[ovs-dev] [PATCH v1 3/3 ovn] NAT: Parsing port_range options in ct_*nat actions

Mark Michelson mmichels at redhat.com
Mon Mar 30 19:21:20 UTC 2020


This and patch 2 should be combined into one patch. Patch 2 in isolation 
adds invalid ct_snat and ct_dnat commands since the code for encoding 
them is not added until patch 3.

See below for additional in-line comments.

On 3/27/20 7:18 PM, Ankur Sharma wrote:
> From: Ankur Sharma <ankur.sharma at nutanix.com>
> 
> Changes to parse the logical flow, which specifies port_range
> for ct_nat action.
> 
> Example logical flow:
> ct_snat(10.15.24.135,1-30000)
> 
> Signed-off-by: Ankur Sharma <ankur.sharma at nutanix.com>
> ---
>   include/ovn/actions.h |  7 +++++++
>   include/ovn/lex.h     |  1 +
>   lib/actions.c         | 39 +++++++++++++++++++++++++++++++++++++++
>   lib/lex.c             |  5 ++++-
>   tests/ovn.at          | 24 ++++++++++++++++++++++--
>   5 files changed, 73 insertions(+), 3 deletions(-)
> 
> diff --git a/include/ovn/actions.h b/include/ovn/actions.h
> index 9b01492..2cec369 100644
> --- a/include/ovn/actions.h
> +++ b/include/ovn/actions.h
> @@ -233,6 +233,13 @@ struct ovnact_ct_nat {
>           struct in6_addr ipv6;
>           ovs_be32 ipv4;
>       };
> +
> +    struct {
> +       bool exists;
> +       uint16_t port_lo;
> +       uint16_t port_hi;
> +    } port_range;
> +
>       uint8_t ltable;             /* Logical table ID of next table. */
>   };
>   
> diff --git a/include/ovn/lex.h b/include/ovn/lex.h
> index 8d55857..1da6ccc 100644
> --- a/include/ovn/lex.h
> +++ b/include/ovn/lex.h
> @@ -63,6 +63,7 @@ enum lex_type {
>       LEX_T_EXCHANGE,             /* <-> */
>       LEX_T_DECREMENT,            /* -- */
>       LEX_T_COLON,                /* : */
> +    LEX_T_HYPHEN,               /* - */
>   };
>   
>   /* Subtype for LEX_T_INTEGER and LEX_T_MASKED_INTEGER tokens.
> diff --git a/lib/actions.c b/lib/actions.c
> index f22acdd..07fbed8 100644
> --- a/lib/actions.c
> +++ b/lib/actions.c
> @@ -770,6 +770,33 @@ parse_ct_nat(struct action_context *ctx, const char *name,
>           }
>           lexer_get(ctx->lexer);
>   
> +        if (lexer_match(ctx->lexer, LEX_T_COMMA)) {
> +
> +           if (ctx->lexer->token.type != LEX_T_INTEGER ||
> +               ctx->lexer->token.format != LEX_F_DECIMAL) {
> +              lexer_syntax_error(ctx->lexer, "expecting Integer for port "
> +                                 "range");
> +           }
> +
> +           cn->port_range.port_lo = ntohll(ctx->lexer->token.value.integer);
> +           lexer_get(ctx->lexer);
> +
> +           if (!lexer_match(ctx->lexer, LEX_T_HYPHEN)) {
> +              lexer_syntax_error(ctx->lexer, "expecting - for port range");
> +              return;
> +           }
> +
> +           if (ctx->lexer->token.type != LEX_T_INTEGER) {
> +              lexer_syntax_error(ctx->lexer, "expecting Integer for port "
> +                                 "range");
> +           }
> +
> +           cn->port_range.port_hi = ntohll(ctx->lexer->token.value.integer);

You should probably ensure that cn->port_range.port_hi is greater than 
cn->port_range.port_lo.

> +           lexer_get(ctx->lexer);
> +
> +           cn->port_range.exists = true;
> +        }
> +
>           if (!lexer_force_match(ctx->lexer, LEX_T_RPAREN)) {
>               return;
>           }
> @@ -799,6 +826,13 @@ format_ct_nat(const struct ovnact_ct_nat *cn, const char *name, struct ds *s)
>           ipv6_format_addr(&cn->ipv6, s);
>           ds_put_char(s, ')');
>       }
> +
> +    if (cn->port_range.exists) {
> +        ds_chomp(s, ')');
> +        ds_put_format(s, ",%d-%d)", cn->port_range.port_lo,
> +                      cn->port_range.port_hi);
> +    }
> +
>       ds_put_char(s, ';');
>   }
>   
> @@ -861,6 +895,11 @@ encode_ct_nat(const struct ovnact_ct_nat *cn,
>           }
>       }
>   
> +    if (cn->port_range.exists) {
> +       nat->range.proto.min = cn->port_range.port_lo;
> +       nat->range.proto.max = cn->port_range.port_hi;
> +    }
> +
>       ofpacts->header = ofpbuf_push_uninit(ofpacts, nat_offset);
>       ct = ofpacts->header;
>       if (cn->family == AF_INET || cn->family == AF_INET6) {
> diff --git a/lib/lex.c b/lib/lex.c
> index 7a2ab41..94f6c77 100644
> --- a/lib/lex.c
> +++ b/lib/lex.c
> @@ -301,6 +301,9 @@ lex_token_format(const struct lex_token *token, struct ds *s)
>       case LEX_T_COLON:
>           ds_put_char(s, ':');
>           break;
> +    case LEX_T_HYPHEN:
> +        ds_put_char(s, '-');
> +        break;
>       default:
>           OVS_NOT_REACHED();
>       }
> @@ -757,7 +760,7 @@ next:
>               token->type = LEX_T_DECREMENT;
>               p++;
>           } else {
> -            lex_error(token, "`-' is only valid as part of `--'.");
> +           token->type = LEX_T_HYPHEN;
>           }
>           break;
>   
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 9a44f0a..d76db6f 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -1049,15 +1049,25 @@ ct_dnat(192.168.1.2);
>   ct_dnat(fd11::2);
>       encodes as ct(commit,table=19,zone=NXM_NX_REG11[0..15],nat(dst=fd11::2))
>       has prereqs ip
> +ct_dnat(192.168.1.2, 1-3000);
> +    formats as ct_dnat(192.168.1.2,1-3000);
> +    encodes as ct(commit,table=19,zone=NXM_NX_REG11[0..15],nat(dst=192.168.1.2:1-3000))
> +    has prereqs ip
>   
>   ct_dnat(192.168.1.2, 192.168.1.3);
> -    Syntax error at `,' expecting `)'.
> +    Syntax error at `192.168.1.3' expecting Integer for port range.
>   ct_dnat(foo);
>       Syntax error at `foo' expecting IPv4 or IPv6 address.
>   ct_dnat(foo, bar);
>       Syntax error at `foo' expecting IPv4 or IPv6 address.
>   ct_dnat();
>       Syntax error at `)' expecting IPv4 or IPv6 address.
> +ct_dnat(192.168.1.2, foo);
> +    Syntax error at `foo' expecting Integer for port range.
> +ct_dnat(192.168.1.2, 1000-foo);
> +    Syntax error at `foo' expecting Integer for port range.
> +ct_dnat(192.168.1.2, 1000);
> +    Syntax error at `)' expecting - for port range.

The underlying OVS nat action allows for a single port to be specified. 
Shouldn't we allow it for ct_snat/ct_dnat as well?

>   
>   # ct_snat
>   ct_snat;
> @@ -1069,15 +1079,25 @@ ct_snat(192.168.1.2);
>   ct_snat(fd11::2);
>       encodes as ct(commit,table=19,zone=NXM_NX_REG12[0..15],nat(src=fd11::2))
>       has prereqs ip
> +ct_snat(192.168.1.2, 1-3000);
> +    formats as ct_snat(192.168.1.2,1-3000);
> +    encodes as ct(commit,table=19,zone=NXM_NX_REG12[0..15],nat(src=192.168.1.2:1-3000))
> +    has prereqs ip
>   
>   ct_snat(192.168.1.2, 192.168.1.3);
> -    Syntax error at `,' expecting `)'.
> +    Syntax error at `192.168.1.3' expecting Integer for port range.
>   ct_snat(foo);
>       Syntax error at `foo' expecting IPv4 or IPv6 address.
>   ct_snat(foo, bar);
>       Syntax error at `foo' expecting IPv4 or IPv6 address.
>   ct_snat();
>       Syntax error at `)' expecting IPv4 or IPv6 address.
> +ct_snat(192.168.1.2, foo);
> +    Syntax error at `foo' expecting Integer for port range.
> +ct_snat(192.168.1.2, 1000-foo);
> +    Syntax error at `foo' expecting Integer for port range.
> +ct_snat(192.168.1.2, 1000);
> +    Syntax error at `)' expecting - for port range.
>   
>   # ct_clear
>   ct_clear;
> 



More information about the dev mailing list