| Message ID | 1600309019-99938-3-git-send-email-svc.mail.git@nutanix.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | NAT port range support | expand |
On 17/09/2020 03:16, Ankur Sharma wrote: > From: Ankur Sharma <ankur.sharma@nutanix.com> > > This patch has following changes: > > a. Northd changes to put port range hash in the > logical flow based on configuration. > > b. Changes to parse the logical flow, which specifies > port_range_hash along with port_range for ct_nat action. > > Example logical flow: > ct_snat(10.15.24.135,1-30000, hash) Thanks for the patch. > > Signed-off-by: Ankur Sharma <ankur.sharma@nutanix.com> > --- > include/ovn/actions.h | 1 + > lib/actions.c | 51 +++++++++++++++++++++++++++++++++++++-- > northd/ovn-northd.c | 16 +++++++++++++ > tests/ovn-northd.at | 31 ++++++++++++++++++++++++ > tests/ovn.at | 66 ++++++++++++++++++++++++++++++++++++++++++++------- > 5 files changed, 155 insertions(+), 10 deletions(-) > > diff --git a/include/ovn/actions.h b/include/ovn/actions.h > index 636cb4b..101cd7a 100644 > --- a/include/ovn/actions.h > +++ b/include/ovn/actions.h > @@ -235,6 +235,7 @@ struct ovnact_ct_nat { > bool exists; > uint16_t port_lo; > uint16_t port_hi; > + char *port_hash; I wonder could the name be changed to something like 'port_hash_type' or 'port_hash_algorthm'. 'port_hash' reads more like the result of the hash rather than the type of hash. This would need to be changed across all your changes (e.g. 'external_port_hash' -> 'external_port_hash_type', etc) > } port_range; > > uint8_t ltable; /* Logical table ID of next table. */ > diff --git a/lib/actions.c b/lib/actions.c > index 5fe0a38..c8e0767 100644 > --- a/lib/actions.c > +++ b/lib/actions.c > @@ -707,6 +707,8 @@ parse_ct_nat(struct action_context *ctx, const char *name, > > if (lexer_match(ctx->lexer, LEX_T_COMMA)) { > > + cn->port_range.port_hash = NULL; > + > if (ctx->lexer->token.type != LEX_T_INTEGER || > ctx->lexer->token.format != LEX_F_DECIMAL) { > lexer_syntax_error(ctx->lexer, "expecting Integer for port " > @@ -733,8 +735,40 @@ parse_ct_nat(struct action_context *ctx, const char *name, > "greater than range low"); > } > lexer_get(ctx->lexer); > + > + if (lexer_match(ctx->lexer, LEX_T_COMMA)) { > + if (ctx->lexer->token.type != LEX_T_ID) { > + lexer_syntax_error(ctx->lexer, "expecting string for " > + "port hash"); > + } > + > + if (strcmp(ctx->lexer->token.s, "hash") && > + strcmp(ctx->lexer->token.s, "random")) { > + lexer_syntax_error(ctx->lexer, "Invalid value for " > + "port hash"); > + } > + > + cn->port_range.port_hash = xstrdup(ctx->lexer->token.s); > + lexer_get(ctx->lexer); > + } > } else { > cn->port_range.port_hi = 0; > + > + if (lexer_match(ctx->lexer, LEX_T_COMMA)) { > + if (ctx->lexer->token.type != LEX_T_ID) { > + lexer_syntax_error(ctx->lexer, "expecting string for " > + "port hash"); > + } > + > + if (strcmp(ctx->lexer->token.s, "hash") && > + strcmp(ctx->lexer->token.s, "random")) { > + lexer_syntax_error(ctx->lexer, "Invalid value for " > + "port hash"); > + } > + > + cn->port_range.port_hash = xstrdup(ctx->lexer->token.s); > + lexer_get(ctx->lexer); > + } > } > > cn->port_range.exists = true; > @@ -777,6 +811,10 @@ format_ct_nat(const struct ovnact_ct_nat *cn, const char *name, struct ds *s) > if (cn->port_range.port_hi) { > ds_put_format(s, "-%d", cn->port_range.port_hi); > } > + > + if (cn->port_range.port_hash) { > + ds_put_format(s, ",%s", cn->port_range.port_hash); > + } > ds_put_char(s, ')'); > } > > @@ -843,8 +881,17 @@ 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; > + const char *port_hash = cn->port_range.port_hash; > + nat->range.proto.min = cn->port_range.port_lo; > + nat->range.proto.max = cn->port_range.port_hi; > + > + if (port_hash) { > + if (!strcmp(port_hash, "hash")) { > + nat->flags |= NX_NAT_F_PROTO_HASH; > + } else { > + nat->flags |= NX_NAT_F_PROTO_RANDOM; > + } > + } > } > > ofpacts->header = ofpbuf_push_uninit(ofpacts, nat_offset); > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index db14909..2d8bc8b 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -9601,6 +9601,10 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > if (nat->external_port_range[0]) { > ds_put_format(&actions, ",%s", > nat->external_port_range); > + if (nat->external_port_hash[0]) { > + ds_put_format(&actions, ",%s", > + nat->external_port_hash); > + } > } > ds_put_format(&actions, ");"); > } > @@ -9638,6 +9642,10 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > if (nat->external_port_range[0]) { > ds_put_format(&actions, ",%s", > nat->external_port_range); > + if (nat->external_port_hash[0]) { > + ds_put_format(&actions, ",%s", > + nat->external_port_hash); > + } > } > ds_put_format(&actions, ");"); > } > @@ -9755,6 +9763,10 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > if (nat->external_port_range[0]) { > ds_put_format(&actions, ",%s", > nat->external_port_range); > + if (nat->external_port_hash[0]) { > + ds_put_format(&actions, ",%s", > + nat->external_port_hash); > + } > } > ds_put_format(&actions, ");"); > } > @@ -9804,6 +9816,10 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > if (nat->external_port_range[0]) { > ds_put_format(&actions, ",%s", > nat->external_port_range); > + if (nat->external_port_hash[0]) { > + ds_put_format(&actions, ",%s", > + nat->external_port_hash); > + } > } > ds_put_format(&actions, ");"); > } > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 99a9204..960ce00 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -2010,3 +2010,34 @@ ovn-nbctl --wait=sb set NB_Global . options:ignore_lsp_down=true > AT_CHECK([ovn-sbctl lflow-list | grep arp | grep 10\.0\.0\.1], [0], [ignore]) > > AT_CLEANUP > + > +AT_SETUP([ovn -- Port Range and Hash in NAT entries]) > +AT_SKIP_IF([test $HAVE_PYTHON = no]) > +ovn_start > + > +ovn-nbctl lr-add lr0 > +ovn-nbctl lrp-add lr0 lr0-public 00:00:01:01:02:04 192.168.2.1/24 > +ovn-nbctl lrp-add lr0 lr0-join 00:00:01:01:02:04 10.10.0.1/24 > + > +ovn-nbctl set logical_router lr0 options:chassis=ch1 > + > +ovn-nbctl --portrange lr-nat-add lr0 snat 192.168.2.1 10.0.0.0/24 1-1000 hash > +ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 192.168.2.4 10.0.0.4 1100-2000 random > +ovn-nbctl --portrange lr-nat-add lr0 dnat 192.168.2.5 10.0.0.5 2100-3000 > + > +ovn-sbctl dump-flows lr0 > + > +AT_CHECK([ovn-sbctl dump-flows lr0 | grep lr_out_snat | \ > +grep "ct_snat(192.168.2.1,1-1000,hash)" | wc -l], [0], [1 > +]) > +AT_CHECK([ovn-sbctl dump-flows lr0 | grep lr_in_dnat | \ > +grep "ct_dnat(10.0.0.4,1100-2000,random)" | wc -l], [0], [1 > +]) > +AT_CHECK([ovn-sbctl dump-flows lr0 | grep lr_out_snat | \ > +grep "ct_snat(192.168.2.4,1100-2000,random)" | wc -l], [0], [1 > +]) > +AT_CHECK([ovn-sbctl dump-flows lr0 | grep lr_in_dnat | \ > +grep "ct_dnat(10.0.0.5,2100-3000)" | wc -l], [0], [1 > +]) > + > +AT_CLEANUP > diff --git a/tests/ovn.at b/tests/ovn.at > index 41fe577..75b79b0 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -1123,6 +1123,22 @@ 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, 1000); > + formats as ct_dnat(192.168.1.2,1000); > + encodes as ct(commit,table=19,zone=NXM_NX_REG11[0..15],nat(dst=192.168.1.2:1000)) > + has prereqs ip > +ct_dnat(192.168.1.2, 1-3000, hash); > + formats as ct_dnat(192.168.1.2,1-3000,hash); > + encodes as ct(commit,table=19,zone=NXM_NX_REG11[0..15],nat(dst=192.168.1.2:1-3000,hash)) > + has prereqs ip > +ct_dnat(192.168.1.2, 1-3000, random); > + formats as ct_dnat(192.168.1.2,1-3000,random); > + encodes as ct(commit,table=19,zone=NXM_NX_REG11[0..15],nat(dst=192.168.1.2:1-3000,random)) > + has prereqs ip > +ct_dnat(192.168.1.2, 1000, hash); > + formats as ct_dnat(192.168.1.2,1000,hash); > + encodes as ct(commit,table=19,zone=NXM_NX_REG11[0..15],nat(dst=192.168.1.2:1000,hash)) > + has prereqs ip > > ct_dnat(192.168.1.2, 192.168.1.3); > Syntax error at `192.168.1.3' expecting Integer for port range. > @@ -1136,12 +1152,20 @@ 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); > - formats as ct_dnat(192.168.1.2,1000); > - encodes as ct(commit,table=19,zone=NXM_NX_REG11[0..15],nat(dst=192.168.1.2:1000)) > - has prereqs ip > ct_dnat(192.168.1.2, 1000-100); > Syntax error at `100' range high should be greater than range low. > +ct_dnat(192.168.1.2, hash); > + Syntax error at `hash' expecting Integer for port range. > +ct_dnat(192.168.1.2, random); > + Syntax error at `random' expecting Integer for port range. > +ct_dnat(192.168.1.2, 192.168.1.3, hash); > + Syntax error at `192.168.1.3' expecting Integer for port range. > +ct_dnat(192.168.1.2, foo, hash); > + Syntax error at `foo' expecting Integer for port range. > +ct_dnat(192.168.1.2, 1000-foo, hash); > + Syntax error at `foo' expecting Integer for port range. > +ct_dnat(192.168.1.2, 1000-100, hash); > + Syntax error at `100' range high should be greater than range low. > > # ct_snat > ct_snat; > @@ -1157,6 +1181,22 @@ 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, 1000); > + formats as ct_snat(192.168.1.2,1000); > + encodes as ct(commit,table=19,zone=NXM_NX_REG12[0..15],nat(src=192.168.1.2:1000)) > + has prereqs ip > +ct_snat(192.168.1.2, 1-3000, hash); > + formats as ct_snat(192.168.1.2,1-3000,hash); > + encodes as ct(commit,table=19,zone=NXM_NX_REG12[0..15],nat(src=192.168.1.2:1-3000,hash)) > + has prereqs ip > +ct_snat(192.168.1.2, 1-3000, random); > + formats as ct_snat(192.168.1.2,1-3000,random); > + encodes as ct(commit,table=19,zone=NXM_NX_REG12[0..15],nat(src=192.168.1.2:1-3000,random)) > + has prereqs ip > +ct_snat(192.168.1.2, 1000, hash); > + formats as ct_snat(192.168.1.2,1000,hash); > + encodes as ct(commit,table=19,zone=NXM_NX_REG12[0..15],nat(src=192.168.1.2:1000,hash)) > + has prereqs ip > > ct_snat(192.168.1.2, 192.168.1.3); > Syntax error at `192.168.1.3' expecting Integer for port range. > @@ -1170,12 +1210,22 @@ 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); > - formats as ct_snat(192.168.1.2,1000); > - encodes as ct(commit,table=19,zone=NXM_NX_REG12[0..15],nat(src=192.168.1.2:1000)) > - has prereqs ip > ct_snat(192.168.1.2, 1000-100); > Syntax error at `100' range high should be greater than range low. > +ct_snat(192.168.1.2, hash); > + Syntax error at `hash' expecting Integer for port range. > +ct_snat(192.168.1.2, random); > + Syntax error at `random' expecting Integer for port range. > +ct_snat(192.168.1.2, 192.168.1.3, hash); > + Syntax error at `192.168.1.3' expecting Integer for port range. > +ct_snat(192.168.1.2, foo, hash); > + Syntax error at `foo' expecting Integer for port range. > +ct_snat(192.168.1.2, 1000-foo, hash); > + Syntax error at `foo' expecting Integer for port range. > +ct_snat(192.168.1.2, 1000-100, hash); > + Syntax error at `100' range high should be greater than range low. > + > + > # ct_clear > ct_clear; > encodes as ct_clear >
On Mon, Sep 21, 2020 at 7:49 PM Mark Gray <mark.d.gray@redhat.com> wrote: > On 17/09/2020 03:16, Ankur Sharma wrote: > > From: Ankur Sharma <ankur.sharma@nutanix.com> > > > > This patch has following changes: > > > > a. Northd changes to put port range hash in the > > logical flow based on configuration. > > > > b. Changes to parse the logical flow, which specifies > > port_range_hash along with port_range for ct_nat action. > > > > Example logical flow: > > ct_snat(10.15.24.135,1-30000, hash) > > Thanks for the patch. > Hi Ankur, Thanks for the patch. Please see below for a few comments. > > > > > Signed-off-by: Ankur Sharma <ankur.sharma@nutanix.com> > > --- > > include/ovn/actions.h | 1 + > > lib/actions.c | 51 +++++++++++++++++++++++++++++++++++++-- > > northd/ovn-northd.c | 16 +++++++++++++ > > tests/ovn-northd.at | 31 ++++++++++++++++++++++++ > > tests/ovn.at | 66 > ++++++++++++++++++++++++++++++++++++++++++++------- > > 5 files changed, 155 insertions(+), 10 deletions(-) > > > > diff --git a/include/ovn/actions.h b/include/ovn/actions.h > > index 636cb4b..101cd7a 100644 > > --- a/include/ovn/actions.h > > +++ b/include/ovn/actions.h > > @@ -235,6 +235,7 @@ struct ovnact_ct_nat { > > bool exists; > > uint16_t port_lo; > > uint16_t port_hi; > > + char *port_hash; > > I wonder could the name be changed to something like 'port_hash_type' or > 'port_hash_algorthm'. 'port_hash' reads more like the result of the hash > rather than the type of hash. This would need to be changed across all > your changes (e.g. 'external_port_hash' -> 'external_port_hash_type', etc) > I think instead of 'port_hash' you can have an integer with the name 'nat_flags' ? And during parsing, you can set the value to 'NX_NAT_F_PROTO_HASH', 'NX_NAT_F_PROTO_RANDOM' or 0. Right now you are allocating memory for this during parsing, but you're not freeing it. With integer type, there is no need to allocate any memory. > > } port_range; > > > > uint8_t ltable; /* Logical table ID of next table. */ > > diff --git a/lib/actions.c b/lib/actions.c > > index 5fe0a38..c8e0767 100644 > > --- a/lib/actions.c > > +++ b/lib/actions.c > > @@ -707,6 +707,8 @@ parse_ct_nat(struct action_context *ctx, const char > *name, > > > > if (lexer_match(ctx->lexer, LEX_T_COMMA)) { > > > > + cn->port_range.port_hash = NULL; > > + > > if (ctx->lexer->token.type != LEX_T_INTEGER || > > ctx->lexer->token.format != LEX_F_DECIMAL) { > > lexer_syntax_error(ctx->lexer, "expecting Integer for > port " > > @@ -733,8 +735,40 @@ parse_ct_nat(struct action_context *ctx, const char > *name, > > "greater than range low"); > > } > > lexer_get(ctx->lexer); > > + > > + if (lexer_match(ctx->lexer, LEX_T_COMMA)) { > > + if (ctx->lexer->token.type != LEX_T_ID) { > > + lexer_syntax_error(ctx->lexer, "expecting string > for " > > + "port hash"); > > + } > > + > > + if (strcmp(ctx->lexer->token.s, "hash") && > > + strcmp(ctx->lexer->token.s, "random")) { > > + lexer_syntax_error(ctx->lexer, "Invalid value > for " > > + "port hash"); > > + } > > + > > + cn->port_range.port_hash = > xstrdup(ctx->lexer->token.s); > > + lexer_get(ctx->lexer); > > + } > > } else { > > cn->port_range.port_hi = 0; > > + > > + if (lexer_match(ctx->lexer, LEX_T_COMMA)) { > > + if (ctx->lexer->token.type != LEX_T_ID) { > > + lexer_syntax_error(ctx->lexer, "expecting string > for " > > + "port hash"); > > + } > > + > > + if (strcmp(ctx->lexer->token.s, "hash") && > > + strcmp(ctx->lexer->token.s, "random")) { > > + lexer_syntax_error(ctx->lexer, "Invalid value > for " > > + "port hash"); > > + } > > + > > + cn->port_range.port_hash = > xstrdup(ctx->lexer->token.s); > > + lexer_get(ctx->lexer); > > + } > > } > > > > cn->port_range.exists = true; > > @@ -777,6 +811,10 @@ format_ct_nat(const struct ovnact_ct_nat *cn, const > char *name, struct ds *s) > > if (cn->port_range.port_hi) { > > ds_put_format(s, "-%d", cn->port_range.port_hi); > > } > > + > > + if (cn->port_range.port_hash) { > > + ds_put_format(s, ",%s", cn->port_range.port_hash); > > + } > > ds_put_char(s, ')'); > > } > > > > @@ -843,8 +881,17 @@ 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; > > + const char *port_hash = cn->port_range.port_hash; > > + nat->range.proto.min = cn->port_range.port_lo; > > + nat->range.proto.max = cn->port_range.port_hi; > > + > > + if (port_hash) { > > + if (!strcmp(port_hash, "hash")) { > > + nat->flags |= NX_NAT_F_PROTO_HASH; > > + } else { > > + nat->flags |= NX_NAT_F_PROTO_RANDOM; > > + } > > + } > > } > > > > ofpacts->header = ofpbuf_push_uninit(ofpacts, nat_offset); > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > > index db14909..2d8bc8b 100644 > > --- a/northd/ovn-northd.c > > +++ b/northd/ovn-northd.c > > @@ -9601,6 +9601,10 @@ build_lrouter_flows(struct hmap *datapaths, > struct hmap *ports, > > if (nat->external_port_range[0]) { > > ds_put_format(&actions, ",%s", > > nat->external_port_range); > > + if (nat->external_port_hash[0]) { > > + ds_put_format(&actions, ",%s", > > + nat->external_port_hash); > > + } > > } > > ds_put_format(&actions, ");"); > > } > > @@ -9638,6 +9642,10 @@ build_lrouter_flows(struct hmap *datapaths, > struct hmap *ports, > > if (nat->external_port_range[0]) { > > ds_put_format(&actions, ",%s", > > nat->external_port_range); > > + if (nat->external_port_hash[0]) { > > + ds_put_format(&actions, ",%s", > > + nat->external_port_hash); > > + } > > } > > ds_put_format(&actions, ");"); > > } > > @@ -9755,6 +9763,10 @@ build_lrouter_flows(struct hmap *datapaths, > struct hmap *ports, > > if (nat->external_port_range[0]) { > > ds_put_format(&actions, ",%s", > > nat->external_port_range); > > + if (nat->external_port_hash[0]) { > > + ds_put_format(&actions, ",%s", > > + nat->external_port_hash); > > + } > > } > > ds_put_format(&actions, ");"); > > } > > @@ -9804,6 +9816,10 @@ build_lrouter_flows(struct hmap *datapaths, > struct hmap *ports, > > if (nat->external_port_range[0]) { > > ds_put_format(&actions, ",%s", > > nat->external_port_range); > > + if (nat->external_port_hash[0]) { > > + ds_put_format(&actions, ",%s", > > + nat->external_port_hash); > > + } > > } > > ds_put_format(&actions, ");"); > > } > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > > index 99a9204..960ce00 100644 > > --- a/tests/ovn-northd.at > > +++ b/tests/ovn-northd.at > > @@ -2010,3 +2010,34 @@ ovn-nbctl --wait=sb set NB_Global . > options:ignore_lsp_down=true > > AT_CHECK([ovn-sbctl lflow-list | grep arp | grep 10\.0\.0\.1], [0], > [ignore]) > > > > AT_CLEANUP > > + > > +AT_SETUP([ovn -- Port Range and Hash in NAT entries]) > > +AT_SKIP_IF([test $HAVE_PYTHON = no]) > > +ovn_start > > + > > +ovn-nbctl lr-add lr0 > > +ovn-nbctl lrp-add lr0 lr0-public 00:00:01:01:02:04 192.168.2.1/24 > > +ovn-nbctl lrp-add lr0 lr0-join 00:00:01:01:02:04 10.10.0.1/24 > > + > > +ovn-nbctl set logical_router lr0 options:chassis=ch1 > > + > > +ovn-nbctl --portrange lr-nat-add lr0 snat 192.168.2.1 10.0.0.0/24 > 1-1000 hash > > +ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 192.168.2.4 10.0.0.4 > 1100-2000 random > > +ovn-nbctl --portrange lr-nat-add lr0 dnat 192.168.2.5 10.0.0.5 2100-3000 > > + > > +ovn-sbctl dump-flows lr0 > > + > > +AT_CHECK([ovn-sbctl dump-flows lr0 | grep lr_out_snat | \ > > +grep "ct_snat(192.168.2.1,1-1000,hash)" | wc -l], [0], [1 > > +]) > > +AT_CHECK([ovn-sbctl dump-flows lr0 | grep lr_in_dnat | \ > > +grep "ct_dnat(10.0.0.4,1100-2000,random)" | wc -l], [0], [1 > > +]) > > +AT_CHECK([ovn-sbctl dump-flows lr0 | grep lr_out_snat | \ > > +grep "ct_snat(192.168.2.4,1100-2000,random)" | wc -l], [0], [1 > > +]) > > +AT_CHECK([ovn-sbctl dump-flows lr0 | grep lr_in_dnat | \ > > +grep "ct_dnat(10.0.0.5,2100-3000)" | wc -l], [0], [1 > > +]) > > + > > +AT_CLEANUP > > diff --git a/tests/ovn.at b/tests/ovn.at > > index 41fe577..75b79b0 100644 > > --- a/tests/ovn.at > > +++ b/tests/ovn.at > > @@ -1123,6 +1123,22 @@ 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, 1000); > > + formats as ct_dnat(192.168.1.2,1000); > > + encodes as ct(commit,table=19,zone=NXM_NX_REG11[0..15],nat(dst= > 192.168.1.2:1000)) > > + has prereqs ip > > +ct_dnat(192.168.1.2, 1-3000, hash); > > + formats as ct_dnat(192.168.1.2,1-3000,hash); > > + encodes as > ct(commit,table=19,zone=NXM_NX_REG11[0..15],nat(dst=192.168.1.2:1 > -3000,hash)) > > + has prereqs ip > > +ct_dnat(192.168.1.2, 1-3000, random); > > + formats as ct_dnat(192.168.1.2,1-3000,random); > > + encodes as > ct(commit,table=19,zone=NXM_NX_REG11[0..15],nat(dst=192.168.1.2:1 > -3000,random)) > > + has prereqs ip > > +ct_dnat(192.168.1.2, 1000, hash); > > + formats as ct_dnat(192.168.1.2,1000,hash); > > + encodes as ct(commit,table=19,zone=NXM_NX_REG11[0..15],nat(dst= > 192.168.1.2:1000,hash)) > > + has prereqs ip > > > > ct_dnat(192.168.1.2, 192.168.1.3); > > Syntax error at `192.168.1.3' expecting Integer for port range. > > @@ -1136,12 +1152,20 @@ 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); > > - formats as ct_dnat(192.168.1.2,1000); > > - encodes as ct(commit,table=19,zone=NXM_NX_REG11[0..15],nat(dst= > 192.168.1.2:1000)) > > - has prereqs ip > > ct_dnat(192.168.1.2, 1000-100); > > Syntax error at `100' range high should be greater than range low. > > +ct_dnat(192.168.1.2, hash); > > + Syntax error at `hash' expecting Integer for port range. > > +ct_dnat(192.168.1.2, random); > > + Syntax error at `random' expecting Integer for port range. > > +ct_dnat(192.168.1.2, 192.168.1.3, hash); > > + Syntax error at `192.168.1.3' expecting Integer for port range. > > +ct_dnat(192.168.1.2, foo, hash); > > + Syntax error at `foo' expecting Integer for port range. > > +ct_dnat(192.168.1.2, 1000-foo, hash); > > + Syntax error at `foo' expecting Integer for port range. > > +ct_dnat(192.168.1.2, 1000-100, hash); > > + Syntax error at `100' range high should be greater than range low. > > > > # ct_snat > > ct_snat; > > @@ -1157,6 +1181,22 @@ 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, 1000); > > + formats as ct_snat(192.168.1.2,1000); > > + encodes as ct(commit,table=19,zone=NXM_NX_REG12[0..15],nat(src= > 192.168.1.2:1000)) > > + has prereqs ip > > +ct_snat(192.168.1.2, 1-3000, hash); > > + formats as ct_snat(192.168.1.2,1-3000,hash); > > + encodes as > ct(commit,table=19,zone=NXM_NX_REG12[0..15],nat(src=192.168.1.2:1 > -3000,hash)) > > + has prereqs ip > > +ct_snat(192.168.1.2, 1-3000, random); > > + formats as ct_snat(192.168.1.2,1-3000,random); > > + encodes as > ct(commit,table=19,zone=NXM_NX_REG12[0..15],nat(src=192.168.1.2:1 > -3000,random)) > > + has prereqs ip > > +ct_snat(192.168.1.2, 1000, hash); > > + formats as ct_snat(192.168.1.2,1000,hash); > > + encodes as ct(commit,table=19,zone=NXM_NX_REG12[0..15],nat(src= > 192.168.1.2:1000,hash)) > > + has prereqs ip > > > > ct_snat(192.168.1.2, 192.168.1.3); > > Syntax error at `192.168.1.3' expecting Integer for port range. > > @@ -1170,12 +1210,22 @@ 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); > > - formats as ct_snat(192.168.1.2,1000); > > - encodes as ct(commit,table=19,zone=NXM_NX_REG12[0..15],nat(src= > 192.168.1.2:1000)) > > - has prereqs ip > > ct_snat(192.168.1.2, 1000-100); > > Syntax error at `100' range high should be greater than range low. > > +ct_snat(192.168.1.2, hash); > > + Syntax error at `hash' expecting Integer for port range. > > +ct_snat(192.168.1.2, random); > > + Syntax error at `random' expecting Integer for port range. > > +ct_snat(192.168.1.2, 192.168.1.3, hash); > > + Syntax error at `192.168.1.3' expecting Integer for port range. > > +ct_snat(192.168.1.2, foo, hash); > > + Syntax error at `foo' expecting Integer for port range. > > +ct_snat(192.168.1.2, 1000-foo, hash); > > + Syntax error at `foo' expecting Integer for port range. > > +ct_snat(192.168.1.2, 1000-100, hash); > > + Syntax error at `100' range high should be greater than range low. > > + > > + > > # ct_clear > > ct_clear; > > encodes as ct_clear > Can you please add more tests which checks that the proper OF flows are added and the NAT flags are set properly ? Thanks Numan > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
diff --git a/include/ovn/actions.h b/include/ovn/actions.h index 636cb4b..101cd7a 100644 --- a/include/ovn/actions.h +++ b/include/ovn/actions.h @@ -235,6 +235,7 @@ struct ovnact_ct_nat { bool exists; uint16_t port_lo; uint16_t port_hi; + char *port_hash; } port_range; uint8_t ltable; /* Logical table ID of next table. */ diff --git a/lib/actions.c b/lib/actions.c index 5fe0a38..c8e0767 100644 --- a/lib/actions.c +++ b/lib/actions.c @@ -707,6 +707,8 @@ parse_ct_nat(struct action_context *ctx, const char *name, if (lexer_match(ctx->lexer, LEX_T_COMMA)) { + cn->port_range.port_hash = NULL; + if (ctx->lexer->token.type != LEX_T_INTEGER || ctx->lexer->token.format != LEX_F_DECIMAL) { lexer_syntax_error(ctx->lexer, "expecting Integer for port " @@ -733,8 +735,40 @@ parse_ct_nat(struct action_context *ctx, const char *name, "greater than range low"); } lexer_get(ctx->lexer); + + if (lexer_match(ctx->lexer, LEX_T_COMMA)) { + if (ctx->lexer->token.type != LEX_T_ID) { + lexer_syntax_error(ctx->lexer, "expecting string for " + "port hash"); + } + + if (strcmp(ctx->lexer->token.s, "hash") && + strcmp(ctx->lexer->token.s, "random")) { + lexer_syntax_error(ctx->lexer, "Invalid value for " + "port hash"); + } + + cn->port_range.port_hash = xstrdup(ctx->lexer->token.s); + lexer_get(ctx->lexer); + } } else { cn->port_range.port_hi = 0; + + if (lexer_match(ctx->lexer, LEX_T_COMMA)) { + if (ctx->lexer->token.type != LEX_T_ID) { + lexer_syntax_error(ctx->lexer, "expecting string for " + "port hash"); + } + + if (strcmp(ctx->lexer->token.s, "hash") && + strcmp(ctx->lexer->token.s, "random")) { + lexer_syntax_error(ctx->lexer, "Invalid value for " + "port hash"); + } + + cn->port_range.port_hash = xstrdup(ctx->lexer->token.s); + lexer_get(ctx->lexer); + } } cn->port_range.exists = true; @@ -777,6 +811,10 @@ format_ct_nat(const struct ovnact_ct_nat *cn, const char *name, struct ds *s) if (cn->port_range.port_hi) { ds_put_format(s, "-%d", cn->port_range.port_hi); } + + if (cn->port_range.port_hash) { + ds_put_format(s, ",%s", cn->port_range.port_hash); + } ds_put_char(s, ')'); } @@ -843,8 +881,17 @@ 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; + const char *port_hash = cn->port_range.port_hash; + nat->range.proto.min = cn->port_range.port_lo; + nat->range.proto.max = cn->port_range.port_hi; + + if (port_hash) { + if (!strcmp(port_hash, "hash")) { + nat->flags |= NX_NAT_F_PROTO_HASH; + } else { + nat->flags |= NX_NAT_F_PROTO_RANDOM; + } + } } ofpacts->header = ofpbuf_push_uninit(ofpacts, nat_offset); diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index db14909..2d8bc8b 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -9601,6 +9601,10 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, if (nat->external_port_range[0]) { ds_put_format(&actions, ",%s", nat->external_port_range); + if (nat->external_port_hash[0]) { + ds_put_format(&actions, ",%s", + nat->external_port_hash); + } } ds_put_format(&actions, ");"); } @@ -9638,6 +9642,10 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, if (nat->external_port_range[0]) { ds_put_format(&actions, ",%s", nat->external_port_range); + if (nat->external_port_hash[0]) { + ds_put_format(&actions, ",%s", + nat->external_port_hash); + } } ds_put_format(&actions, ");"); } @@ -9755,6 +9763,10 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, if (nat->external_port_range[0]) { ds_put_format(&actions, ",%s", nat->external_port_range); + if (nat->external_port_hash[0]) { + ds_put_format(&actions, ",%s", + nat->external_port_hash); + } } ds_put_format(&actions, ");"); } @@ -9804,6 +9816,10 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, if (nat->external_port_range[0]) { ds_put_format(&actions, ",%s", nat->external_port_range); + if (nat->external_port_hash[0]) { + ds_put_format(&actions, ",%s", + nat->external_port_hash); + } } ds_put_format(&actions, ");"); } diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 99a9204..960ce00 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -2010,3 +2010,34 @@ ovn-nbctl --wait=sb set NB_Global . options:ignore_lsp_down=true AT_CHECK([ovn-sbctl lflow-list | grep arp | grep 10\.0\.0\.1], [0], [ignore]) AT_CLEANUP + +AT_SETUP([ovn -- Port Range and Hash in NAT entries]) +AT_SKIP_IF([test $HAVE_PYTHON = no]) +ovn_start + +ovn-nbctl lr-add lr0 +ovn-nbctl lrp-add lr0 lr0-public 00:00:01:01:02:04 192.168.2.1/24 +ovn-nbctl lrp-add lr0 lr0-join 00:00:01:01:02:04 10.10.0.1/24 + +ovn-nbctl set logical_router lr0 options:chassis=ch1 + +ovn-nbctl --portrange lr-nat-add lr0 snat 192.168.2.1 10.0.0.0/24 1-1000 hash +ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 192.168.2.4 10.0.0.4 1100-2000 random +ovn-nbctl --portrange lr-nat-add lr0 dnat 192.168.2.5 10.0.0.5 2100-3000 + +ovn-sbctl dump-flows lr0 + +AT_CHECK([ovn-sbctl dump-flows lr0 | grep lr_out_snat | \ +grep "ct_snat(192.168.2.1,1-1000,hash)" | wc -l], [0], [1 +]) +AT_CHECK([ovn-sbctl dump-flows lr0 | grep lr_in_dnat | \ +grep "ct_dnat(10.0.0.4,1100-2000,random)" | wc -l], [0], [1 +]) +AT_CHECK([ovn-sbctl dump-flows lr0 | grep lr_out_snat | \ +grep "ct_snat(192.168.2.4,1100-2000,random)" | wc -l], [0], [1 +]) +AT_CHECK([ovn-sbctl dump-flows lr0 | grep lr_in_dnat | \ +grep "ct_dnat(10.0.0.5,2100-3000)" | wc -l], [0], [1 +]) + +AT_CLEANUP diff --git a/tests/ovn.at b/tests/ovn.at index 41fe577..75b79b0 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -1123,6 +1123,22 @@ 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, 1000); + formats as ct_dnat(192.168.1.2,1000); + encodes as ct(commit,table=19,zone=NXM_NX_REG11[0..15],nat(dst=192.168.1.2:1000)) + has prereqs ip +ct_dnat(192.168.1.2, 1-3000, hash); + formats as ct_dnat(192.168.1.2,1-3000,hash); + encodes as ct(commit,table=19,zone=NXM_NX_REG11[0..15],nat(dst=192.168.1.2:1-3000,hash)) + has prereqs ip +ct_dnat(192.168.1.2, 1-3000, random); + formats as ct_dnat(192.168.1.2,1-3000,random); + encodes as ct(commit,table=19,zone=NXM_NX_REG11[0..15],nat(dst=192.168.1.2:1-3000,random)) + has prereqs ip +ct_dnat(192.168.1.2, 1000, hash); + formats as ct_dnat(192.168.1.2,1000,hash); + encodes as ct(commit,table=19,zone=NXM_NX_REG11[0..15],nat(dst=192.168.1.2:1000,hash)) + has prereqs ip ct_dnat(192.168.1.2, 192.168.1.3); Syntax error at `192.168.1.3' expecting Integer for port range. @@ -1136,12 +1152,20 @@ 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); - formats as ct_dnat(192.168.1.2,1000); - encodes as ct(commit,table=19,zone=NXM_NX_REG11[0..15],nat(dst=192.168.1.2:1000)) - has prereqs ip ct_dnat(192.168.1.2, 1000-100); Syntax error at `100' range high should be greater than range low. +ct_dnat(192.168.1.2, hash); + Syntax error at `hash' expecting Integer for port range. +ct_dnat(192.168.1.2, random); + Syntax error at `random' expecting Integer for port range. +ct_dnat(192.168.1.2, 192.168.1.3, hash); + Syntax error at `192.168.1.3' expecting Integer for port range. +ct_dnat(192.168.1.2, foo, hash); + Syntax error at `foo' expecting Integer for port range. +ct_dnat(192.168.1.2, 1000-foo, hash); + Syntax error at `foo' expecting Integer for port range. +ct_dnat(192.168.1.2, 1000-100, hash); + Syntax error at `100' range high should be greater than range low. # ct_snat ct_snat; @@ -1157,6 +1181,22 @@ 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, 1000); + formats as ct_snat(192.168.1.2,1000); + encodes as ct(commit,table=19,zone=NXM_NX_REG12[0..15],nat(src=192.168.1.2:1000)) + has prereqs ip +ct_snat(192.168.1.2, 1-3000, hash); + formats as ct_snat(192.168.1.2,1-3000,hash); + encodes as ct(commit,table=19,zone=NXM_NX_REG12[0..15],nat(src=192.168.1.2:1-3000,hash)) + has prereqs ip +ct_snat(192.168.1.2, 1-3000, random); + formats as ct_snat(192.168.1.2,1-3000,random); + encodes as ct(commit,table=19,zone=NXM_NX_REG12[0..15],nat(src=192.168.1.2:1-3000,random)) + has prereqs ip +ct_snat(192.168.1.2, 1000, hash); + formats as ct_snat(192.168.1.2,1000,hash); + encodes as ct(commit,table=19,zone=NXM_NX_REG12[0..15],nat(src=192.168.1.2:1000,hash)) + has prereqs ip ct_snat(192.168.1.2, 192.168.1.3); Syntax error at `192.168.1.3' expecting Integer for port range. @@ -1170,12 +1210,22 @@ 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); - formats as ct_snat(192.168.1.2,1000); - encodes as ct(commit,table=19,zone=NXM_NX_REG12[0..15],nat(src=192.168.1.2:1000)) - has prereqs ip ct_snat(192.168.1.2, 1000-100); Syntax error at `100' range high should be greater than range low. +ct_snat(192.168.1.2, hash); + Syntax error at `hash' expecting Integer for port range. +ct_snat(192.168.1.2, random); + Syntax error at `random' expecting Integer for port range. +ct_snat(192.168.1.2, 192.168.1.3, hash); + Syntax error at `192.168.1.3' expecting Integer for port range. +ct_snat(192.168.1.2, foo, hash); + Syntax error at `foo' expecting Integer for port range. +ct_snat(192.168.1.2, 1000-foo, hash); + Syntax error at `foo' expecting Integer for port range. +ct_snat(192.168.1.2, 1000-100, hash); + Syntax error at `100' range high should be greater than range low. + + # ct_clear ct_clear; encodes as ct_clear