On 17/09/2020 03:16, Ankur Sharma wrote: > From: Ankur Sharma <[email protected]> > > 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 <[email protected]> > --- > 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 > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
