| Message ID | 20240821085507.394179-1-amusil@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev] northd, controller: Use ct_next to get the CT state for direct SNAT. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On Wed, Aug 21, 2024 at 10:55 AM Ales Musil <amusil@redhat.com> wrote: > In order to get the direct SNAT access working we need to commit new > connections so the reply is not marked as invalid. The CT state to > determine if the connection should be committed was populated by > ct_snat action, however this action also performs the NAT part > if the connection is already known and there is an entry for that. > This would cause issues when the there is combination of FIPs and > SNAT with very broad logical IP. To prevent that use ct_next only > which will populate the state but won't do the NAT part which can > be done later on if needed according to the conditions. > > At the same time add support for ct_next in SNAT zone as ct_next > was assuming that the zone is always DNAT. > > Fixes: 40136a2f2c84 ("northd: Fix direct access to SNAT network.") > Reported-at: https://issues.redhat.com/browse/FDP-744 > Signed-off-by: Ales Musil <amusil@redhat.com> > --- > controller/chassis.c | 8 ++++++++ > include/ovn/actions.h | 1 + > include/ovn/features.h | 1 + > lib/actions.c | 33 +++++++++++++++++++++++++++++---- > northd/en-global-config.c | 10 ++++++++++ > northd/en-global-config.h | 1 + > northd/northd.c | 6 +++--- > ovn-sb.xml | 2 ++ > tests/ovn-northd.at | 23 ++++++++++++++--------- > tests/ovn.at | 16 ++++++++++++++++ > tests/system-ovn.at | 10 ++++------ > 11 files changed, 89 insertions(+), 22 deletions(-) > > diff --git a/controller/chassis.c b/controller/chassis.c > index 2991a0af3..ee839084a 100644 > --- a/controller/chassis.c > +++ b/controller/chassis.c > @@ -390,6 +390,7 @@ chassis_build_other_config(const struct > ovs_chassis_cfg *ovs_cfg, > smap_replace(config, OVN_FEATURE_CT_COMMIT_TO_ZONE, "true"); > smap_replace(config, OVN_FEATURE_SAMPLE_WITH_REGISTERS, > ovs_cfg->sample_with_regs ? "true" : "false"); > + smap_replace(config, OVN_FEATURE_CT_NEXT_ZONE, "true"); > } > > /* > @@ -549,6 +550,12 @@ chassis_other_config_changed(const struct > ovs_chassis_cfg *ovs_cfg, > return true; > } > > + if (!smap_get_bool(&chassis_rec->other_config, > + OVN_FEATURE_CT_NEXT_ZONE, > + false)) { > + return true; > + } > + > return false; > } > > @@ -706,6 +713,7 @@ update_supported_sset(struct sset *supported) > sset_add(supported, OVN_FEATURE_CT_COMMIT_NAT_V2); > sset_add(supported, OVN_FEATURE_CT_COMMIT_TO_ZONE); > sset_add(supported, OVN_FEATURE_SAMPLE_WITH_REGISTERS); > + sset_add(supported, OVN_FEATURE_CT_NEXT_ZONE); > } > > static void > diff --git a/include/ovn/actions.h b/include/ovn/actions.h > index c8dd66ed8..a95a0daf7 100644 > --- a/include/ovn/actions.h > +++ b/include/ovn/actions.h > @@ -260,6 +260,7 @@ struct ovnact_push_pop { > /* OVNACT_CT_NEXT. */ > struct ovnact_ct_next { > struct ovnact ovnact; > + bool dnat_zone; > uint8_t ltable; /* Logical table ID of next table. */ > }; > > diff --git a/include/ovn/features.h b/include/ovn/features.h > index 4275f7526..3566ab60f 100644 > --- a/include/ovn/features.h > +++ b/include/ovn/features.h > @@ -30,6 +30,7 @@ > #define OVN_FEATURE_CT_COMMIT_NAT_V2 "ct-commit-nat-v2" > #define OVN_FEATURE_CT_COMMIT_TO_ZONE "ct-commit-to-zone" > #define OVN_FEATURE_SAMPLE_WITH_REGISTERS "ovn-sample-with-registers" > +#define OVN_FEATURE_CT_NEXT_ZONE "ct-next-zone" > > /* OVS datapath supported features. Based on availability OVN might > generate > * different types of openflows. > diff --git a/lib/actions.c b/lib/actions.c > index c12d087e7..2e05d4134 100644 > --- a/lib/actions.c > +++ b/lib/actions.c > @@ -701,13 +701,32 @@ parse_CT_NEXT(struct action_context *ctx) > } > > add_prerequisite(ctx, "ip"); > - ovnact_put_CT_NEXT(ctx->ovnacts)->ltable = ctx->pp->cur_ltable + 1; > + struct ovnact_ct_next *ct_next = ovnact_put_CT_NEXT(ctx->ovnacts); > + ct_next->dnat_zone = true; > + ct_next->ltable = ctx->pp->cur_ltable + 1; > + > + if (!lexer_match(ctx->lexer, LEX_T_LPAREN)) { > + return; > + } > + > + if (lexer_match_id(ctx->lexer, "dnat")) { > + ct_next->dnat_zone = true; > + } else if (lexer_match_id(ctx->lexer, "snat")) { > + ct_next->dnat_zone = false; > + } else { > + lexer_error(ctx->lexer, "\"ct_next\" action accepts only" > + " \"dnat\" or \"snat\" parameter."); > + return; > + } > + > + lexer_force_match(ctx->lexer, LEX_T_RPAREN); > } > > static void > format_CT_NEXT(const struct ovnact_ct_next *ct_next OVS_UNUSED, struct ds > *s) > { > - ds_put_cstr(s, "ct_next;"); > + ds_put_cstr(s, "ct_next"); > + ds_put_cstr(s, ct_next->dnat_zone ? "(dnat);" : "(snat);"); > } > > static void > @@ -719,11 +738,17 @@ encode_CT_NEXT(const struct ovnact_ct_next *ct_next, > > struct ofpact_conntrack *ct = ofpact_put_CT(ofpacts); > ct->recirc_table = first_ptable(ep, ep->pipeline) + ct_next->ltable; > - ct->zone_src.field = ep->is_switch ? mf_from_id(MFF_LOG_CT_ZONE) > - : mf_from_id(MFF_LOG_DNAT_ZONE); > ct->zone_src.ofs = 0; > ct->zone_src.n_bits = 16; > > + if (ep->is_switch) { > + ct->zone_src.field = mf_from_id(MFF_LOG_CT_ZONE); > + } else { > + ct->zone_src.field = mf_from_id(ct_next->dnat_zone > + ? MFF_LOG_DNAT_ZONE > + : MFF_LOG_SNAT_ZONE); > + } > + > ct = ofpbuf_at_assert(ofpacts, ct_offset, sizeof *ct); > ofpacts->header = ct; > ofpact_finish_CT(ofpacts, &ct); > diff --git a/northd/en-global-config.c b/northd/en-global-config.c > index 0ce7f8308..fff2aaa16 100644 > --- a/northd/en-global-config.c > +++ b/northd/en-global-config.c > @@ -382,6 +382,7 @@ northd_enable_all_features(struct > ed_type_global_config *data) > .ct_commit_nat_v2 = true, > .ct_commit_to_zone = true, > .sample_with_reg = true, > + .ct_next_zone = true, > }; > } > > @@ -452,6 +453,15 @@ build_chassis_features(const struct > sbrec_chassis_table *sbrec_chassis_table, > chassis_features->sample_with_reg) { > chassis_features->sample_with_reg = false; > } > + > + bool ct_next_zone = > + smap_get_bool(&chassis->other_config, > + OVN_FEATURE_CT_NEXT_ZONE, > + false); > + if (!ct_next_zone && > + chassis_features->ct_next_zone) { > + chassis_features->ct_next_zone = false; > + } > } > } > > diff --git a/northd/en-global-config.h b/northd/en-global-config.h > index 0cf34482a..767810542 100644 > --- a/northd/en-global-config.h > +++ b/northd/en-global-config.h > @@ -20,6 +20,7 @@ struct chassis_features { > bool ct_commit_nat_v2; > bool ct_commit_to_zone; > bool sample_with_reg; > + bool ct_next_zone; > }; > > struct global_config_tracked_data { > diff --git a/northd/northd.c b/northd/northd.c > index 73367b910..19a484ceb 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -16079,14 +16079,14 @@ build_lrouter_out_snat_flow(struct lflow_table > *lflows, > /* For the SNAT networks, we need to make sure that connections are > * properly tracked so we can decide whether to perform SNAT on > traffic > * exiting the network. */ > - if (features->ct_commit_to_zone && !strcmp(nat->type, "snat") && > - !od->is_gw_router) { > + if (features->ct_commit_to_zone && features->ct_next_zone && > + !strcmp(nat->type, "snat") && !od->is_gw_router) { > /* For traffic that comes from SNAT network, initiate CT state > before > * entering S_ROUTER_OUT_SNAT to allow matching on various CT > states. > */ > ds_truncate(match, original_match_len); > ovn_lflow_add(lflows, od, S_ROUTER_OUT_POST_UNDNAT, 70, > - ds_cstr(match), "ct_snat;", > + ds_cstr(match), "ct_next(snat);", > lflow_ref); > > build_lrouter_out_snat_match(lflows, od, nat, match, > diff --git a/ovn-sb.xml b/ovn-sb.xml > index c11296d7c..95116ac56 100644 > --- a/ovn-sb.xml > +++ b/ovn-sb.xml > @@ -1369,6 +1369,8 @@ > </dd> > > <dt><code>ct_next;</code></dt> > + <dt><code>ct_next(dnat);</code></dt> > + <dt><code>ct_next(snat);</code></dt> > <dd> > <p> > Apply connection tracking to the flow, initializing > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 93ccbce6b..8816749c4 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -1133,7 +1133,8 @@ ovn_start > # DR is connected to S1 and CR is connected to S2 > > check ovn-sbctl chassis-add gw1 geneve 127.0.0.1 \ > - -- set chassis gw1 other_config:ct-commit-to-zone="true" > + -- set chassis gw1 other_config:ct-commit-to-zone="true" \ > + -- set chassis gw1 other_config:ct-next-zone="true" > > check ovn-nbctl lr-add DR > check ovn-nbctl lrp-add DR DR-S1 02:ac:10:01:00:01 172.16.1.1/24 > @@ -5721,7 +5722,8 @@ AT_CHECK([grep "lr_out_snat" lr0flows | > ovn_strip_lflows], [0], [dnl > ]) > > check ovn-sbctl chassis-add gw1 geneve 127.0.0.1 \ > - -- set chassis gw1 other_config:ct-commit-to-zone="true" > + -- set chassis gw1 other_config:ct-commit-to-zone="true" \ > + -- set chassis gw1 other_config:ct-next-zone="true" > > # Create a distributed gw port on lr0 > check ovn-nbctl ls-add public > @@ -5822,8 +5824,8 @@ AT_CHECK([grep "lr_out_undnat" lr0flows | > ovn_strip_lflows], [0], [dnl > > AT_CHECK([grep "lr_out_post_undnat" lr0flows | ovn_strip_lflows], [0], > [dnl > table=??(lr_out_post_undnat ), priority=0 , match=(1), action=(next;) > - table=??(lr_out_post_undnat ), priority=70 , match=(ip && ip4.src == > 10.0.0.0/24 && outport == "lr0-public" && > is_chassis_resident("cr-lr0-public")), action=(ct_snat;) > - table=??(lr_out_post_undnat ), priority=70 , match=(ip && ip4.src == > 10.0.0.10 && outport == "lr0-public" && > is_chassis_resident("cr-lr0-public")), action=(ct_snat;) > + table=??(lr_out_post_undnat ), priority=70 , match=(ip && ip4.src == > 10.0.0.0/24 && outport == "lr0-public" && > is_chassis_resident("cr-lr0-public")), action=(ct_next(snat);) > + table=??(lr_out_post_undnat ), priority=70 , match=(ip && ip4.src == > 10.0.0.10 && outport == "lr0-public" && > is_chassis_resident("cr-lr0-public")), action=(ct_next(snat);) > ]) > > AT_CHECK([grep "lr_out_snat" lr0flows | ovn_strip_lflows], [0], [dnl > @@ -5980,8 +5982,8 @@ AT_CHECK([grep "lr_out_undnat" lr0flows | > ovn_strip_lflows], [0], [dnl > > AT_CHECK([grep "lr_out_post_undnat" lr0flows | ovn_strip_lflows], [0], > [dnl > table=??(lr_out_post_undnat ), priority=0 , match=(1), action=(next;) > - table=??(lr_out_post_undnat ), priority=70 , match=(ip && ip4.src == > 10.0.0.0/24 && outport == "lr0-public" && > is_chassis_resident("cr-lr0-public")), action=(ct_snat;) > - table=??(lr_out_post_undnat ), priority=70 , match=(ip && ip4.src == > 10.0.0.10 && outport == "lr0-public" && > is_chassis_resident("cr-lr0-public")), action=(ct_snat;) > + table=??(lr_out_post_undnat ), priority=70 , match=(ip && ip4.src == > 10.0.0.0/24 && outport == "lr0-public" && > is_chassis_resident("cr-lr0-public")), action=(ct_next(snat);) > + table=??(lr_out_post_undnat ), priority=70 , match=(ip && ip4.src == > 10.0.0.10 && outport == "lr0-public" && > is_chassis_resident("cr-lr0-public")), action=(ct_next(snat);) > ]) > > AT_CHECK([grep "lr_out_snat" lr0flows | ovn_strip_lflows], [0], [dnl > @@ -7876,13 +7878,16 @@ ovn_start > # distributed gateway LRPs. > > check ovn-sbctl chassis-add gw1 geneve 127.0.0.1 \ > - -- set chassis gw1 other_config:ct-commit-to-zone="true" > + -- set chassis gw1 other_config:ct-commit-to-zone="true" \ > + -- set chassis gw1 other_config:ct-next-zone="true" > > check ovn-sbctl chassis-add gw2 geneve 128.0.0.1 \ > - -- set chassis gw2 other_config:ct-commit-to-zone="true" > + -- set chassis gw2 other_config:ct-commit-to-zone="true" \ > + -- set chassis gw2 other_config:ct-next-zone="true" > > check ovn-sbctl chassis-add gw3 geneve 129.0.0.1 \ > - -- set chassis gw3 other_config:ct-commit-to-zone="true" > + -- set chassis gw3 other_config:ct-commit-to-zone="true" \ > + -- set chassis gw3 other_config:ct-next-zone="true" > > check ovn-nbctl lr-add DR > check ovn-nbctl lrp-add DR DR-S1 02:ac:10:01:00:01 172.16.1.1/24 > diff --git a/tests/ovn.at b/tests/ovn.at > index 50c9f04da..632f060cc 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -1263,11 +1263,27 @@ ct_lb_mark(backends=192.168.1.2:80,192.168.1.3:80; > hash_fields="eth_src,eth_dst, > > # ct_next > ct_next; > + formats as ct_next(dnat); > + encodes as ct(table=oflow_in_table,zone=NXM_NX_REG13[[0..15]]) > + has prereqs ip > +ct_next(dnat); > + encodes as ct(table=oflow_in_table,zone=NXM_NX_REG13[[0..15]]) > + has prereqs ip > +ct_next(snat); > encodes as ct(table=oflow_in_table,zone=NXM_NX_REG13[[0..15]]) > has prereqs ip > ct_clear; ct_next; > + formats as ct_clear; ct_next(dnat); > encodes as > ct_clear,ct(table=oflow_in_table,zone=NXM_NX_REG13[[0..15]]) > has prereqs ip > +ct_next(snat, dnat); > + Syntax error at `,' expecting `)'. > +ct_next(dnat, ignore); > + Syntax error at `,' expecting `)'. > +ct_next(ignore); > + "ct_next" action accepts only "dnat" or "snat" parameter. > +ct_next(); > + "ct_next" action accepts only "dnat" or "snat" parameter. > > # ct_commit > ct_commit; > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > index 6e4ec4247..01f71161a 100644 > --- a/tests/system-ovn.at > +++ b/tests/system-ovn.at > @@ -3518,7 +3518,7 @@ AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat > 172.16.1.3 192.168.1.2 foo1 00:0 > AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.4 192.168.1.3 > foo2 00:00:02:02:03:05]) > > # Add a SNAT rule > -AT_CHECK([ovn-nbctl lr-nat-add R1 snat 172.16.1.1 192.168.0.0/16]) > +AT_CHECK([ovn-nbctl lr-nat-add R1 snat 172.16.1.1 0.0.0.0/0]) > > # Add default route to ext-net > AT_CHECK([ovn-nbctl lr-route-add R1 10.0.0.0/24 172.16.1.2]) > @@ -3724,8 +3724,7 @@ AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat > fd20::3 fd11::2 foo1 00:00:02:02 > AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat fd20::4 fd11::3 foo2 > 00:00:02:02:03:05]) > > # Add a SNAT rule > -AT_CHECK([ovn-nbctl lr-nat-add R1 snat fd20::1 fd11::/64]) > -AT_CHECK([ovn-nbctl lr-nat-add R1 snat fd20::1 fd12::/64]) > +AT_CHECK([ovn-nbctl lr-nat-add R1 snat fd20::1 ::/0]) > > ovn-nbctl --wait=hv sync > OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int | grep 'nat(src=fd20::1)']) > @@ -3920,7 +3919,7 @@ AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat > 172.16.1.3 192.168.1.2 foo1 00:0 > AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.4 192.168.2.2 > bar1 00:00:02:02:03:05]) > > # Add a SNAT rule > -AT_CHECK([ovn-nbctl lr-nat-add R1 snat 172.16.1.1 192.168.0.0/16]) > +AT_CHECK([ovn-nbctl lr-nat-add R1 snat 172.16.1.1 0.0.0.0/0]) > > ovn-nbctl --wait=hv sync > OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int | grep 'nat(src=172.16.1.1)']) > @@ -4104,8 +4103,7 @@ AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat > fd20::3 fd11::2 foo1 00:00:02:02 > AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat fd20::4 fd12::2 bar1 > 00:00:02:02:03:05]) > > # Add a SNAT rule > -AT_CHECK([ovn-nbctl lr-nat-add R1 snat fd20::1 fd11::/64]) > -AT_CHECK([ovn-nbctl lr-nat-add R1 snat fd20::1 fd12::/64]) > +AT_CHECK([ovn-nbctl lr-nat-add R1 snat fd20::1 ::/0]) > > ovn-nbctl --wait=hv sync > OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int | grep 'nat(src=fd20::1)']) > -- > 2.46.0 > > There is still an issue when external ports tries to access the FIP with SNAT being 0.0.0.0/0. I'll send v2 eventually, please ignore this for now. Thanks, Ales
Hi Ales, thanks for getting to the bottom of this. The ct_snat was indeed used opportunistically as an easiest way to commit connection to the CT, without realising that it has side effects. The change overall looks good to me. Martin. > On 21 Aug 2024, at 10:55, Ales Musil <amusil@redhat.com> wrote: > > In order to get the direct SNAT access working we need to commit new > connections so the reply is not marked as invalid. The CT state to > determine if the connection should be committed was populated by > ct_snat action, however this action also performs the NAT part > if the connection is already known and there is an entry for that. > This would cause issues when the there is combination of FIPs and > SNAT with very broad logical IP. To prevent that use ct_next only > which will populate the state but won't do the NAT part which can > be done later on if needed according to the conditions. > > At the same time add support for ct_next in SNAT zone as ct_next > was assuming that the zone is always DNAT. > > Fixes: 40136a2f2c84 ("northd: Fix direct access to SNAT network.") > Reported-at: https://issues.redhat.com/browse/FDP-744 > Signed-off-by: Ales Musil <amusil@redhat.com> > --- > controller/chassis.c | 8 ++++++++ > include/ovn/actions.h | 1 + > include/ovn/features.h | 1 + > lib/actions.c | 33 +++++++++++++++++++++++++++++---- > northd/en-global-config.c | 10 ++++++++++ > northd/en-global-config.h | 1 + > northd/northd.c | 6 +++--- > ovn-sb.xml | 2 ++ > tests/ovn-northd.at | 23 ++++++++++++++--------- > tests/ovn.at | 16 ++++++++++++++++ > tests/system-ovn.at | 10 ++++------ > 11 files changed, 89 insertions(+), 22 deletions(-) > > diff --git a/controller/chassis.c b/controller/chassis.c > index 2991a0af3..ee839084a 100644 > --- a/controller/chassis.c > +++ b/controller/chassis.c > @@ -390,6 +390,7 @@ chassis_build_other_config(const struct ovs_chassis_cfg *ovs_cfg, > smap_replace(config, OVN_FEATURE_CT_COMMIT_TO_ZONE, "true"); > smap_replace(config, OVN_FEATURE_SAMPLE_WITH_REGISTERS, > ovs_cfg->sample_with_regs ? "true" : "false"); > + smap_replace(config, OVN_FEATURE_CT_NEXT_ZONE, "true"); > } > > /* > @@ -549,6 +550,12 @@ chassis_other_config_changed(const struct ovs_chassis_cfg *ovs_cfg, > return true; > } > > + if (!smap_get_bool(&chassis_rec->other_config, > + OVN_FEATURE_CT_NEXT_ZONE, > + false)) { > + return true; > + } > + > return false; > } > > @@ -706,6 +713,7 @@ update_supported_sset(struct sset *supported) > sset_add(supported, OVN_FEATURE_CT_COMMIT_NAT_V2); > sset_add(supported, OVN_FEATURE_CT_COMMIT_TO_ZONE); > sset_add(supported, OVN_FEATURE_SAMPLE_WITH_REGISTERS); > + sset_add(supported, OVN_FEATURE_CT_NEXT_ZONE); > } > > static void > diff --git a/include/ovn/actions.h b/include/ovn/actions.h > index c8dd66ed8..a95a0daf7 100644 > --- a/include/ovn/actions.h > +++ b/include/ovn/actions.h > @@ -260,6 +260,7 @@ struct ovnact_push_pop { > /* OVNACT_CT_NEXT. */ > struct ovnact_ct_next { > struct ovnact ovnact; > + bool dnat_zone; > uint8_t ltable; /* Logical table ID of next table. */ > }; > > diff --git a/include/ovn/features.h b/include/ovn/features.h > index 4275f7526..3566ab60f 100644 > --- a/include/ovn/features.h > +++ b/include/ovn/features.h > @@ -30,6 +30,7 @@ > #define OVN_FEATURE_CT_COMMIT_NAT_V2 "ct-commit-nat-v2" > #define OVN_FEATURE_CT_COMMIT_TO_ZONE "ct-commit-to-zone" > #define OVN_FEATURE_SAMPLE_WITH_REGISTERS "ovn-sample-with-registers" > +#define OVN_FEATURE_CT_NEXT_ZONE "ct-next-zone" > > /* OVS datapath supported features. Based on availability OVN might generate > * different types of openflows. > diff --git a/lib/actions.c b/lib/actions.c > index c12d087e7..2e05d4134 100644 > --- a/lib/actions.c > +++ b/lib/actions.c > @@ -701,13 +701,32 @@ parse_CT_NEXT(struct action_context *ctx) > } > > add_prerequisite(ctx, "ip"); > - ovnact_put_CT_NEXT(ctx->ovnacts)->ltable = ctx->pp->cur_ltable + 1; > + struct ovnact_ct_next *ct_next = ovnact_put_CT_NEXT(ctx->ovnacts); > + ct_next->dnat_zone = true; > + ct_next->ltable = ctx->pp->cur_ltable + 1; > + > + if (!lexer_match(ctx->lexer, LEX_T_LPAREN)) { > + return; > + } > + > + if (lexer_match_id(ctx->lexer, "dnat")) { > + ct_next->dnat_zone = true; > + } else if (lexer_match_id(ctx->lexer, "snat")) { > + ct_next->dnat_zone = false; > + } else { > + lexer_error(ctx->lexer, "\"ct_next\" action accepts only" > + " \"dnat\" or \"snat\" parameter."); > + return; > + } > + > + lexer_force_match(ctx->lexer, LEX_T_RPAREN); > } > > static void > format_CT_NEXT(const struct ovnact_ct_next *ct_next OVS_UNUSED, struct ds *s) > { > - ds_put_cstr(s, "ct_next;"); > + ds_put_cstr(s, "ct_next"); > + ds_put_cstr(s, ct_next->dnat_zone ? "(dnat);" : "(snat);"); > } > > static void > @@ -719,11 +738,17 @@ encode_CT_NEXT(const struct ovnact_ct_next *ct_next, > > struct ofpact_conntrack *ct = ofpact_put_CT(ofpacts); > ct->recirc_table = first_ptable(ep, ep->pipeline) + ct_next->ltable; > - ct->zone_src.field = ep->is_switch ? mf_from_id(MFF_LOG_CT_ZONE) > - : mf_from_id(MFF_LOG_DNAT_ZONE); > ct->zone_src.ofs = 0; > ct->zone_src.n_bits = 16; > > + if (ep->is_switch) { > + ct->zone_src.field = mf_from_id(MFF_LOG_CT_ZONE); > + } else { > + ct->zone_src.field = mf_from_id(ct_next->dnat_zone > + ? MFF_LOG_DNAT_ZONE > + : MFF_LOG_SNAT_ZONE); > + } > + > ct = ofpbuf_at_assert(ofpacts, ct_offset, sizeof *ct); > ofpacts->header = ct; > ofpact_finish_CT(ofpacts, &ct); > diff --git a/northd/en-global-config.c b/northd/en-global-config.c > index 0ce7f8308..fff2aaa16 100644 > --- a/northd/en-global-config.c > +++ b/northd/en-global-config.c > @@ -382,6 +382,7 @@ northd_enable_all_features(struct ed_type_global_config *data) > .ct_commit_nat_v2 = true, > .ct_commit_to_zone = true, > .sample_with_reg = true, > + .ct_next_zone = true, > }; > } > > @@ -452,6 +453,15 @@ build_chassis_features(const struct sbrec_chassis_table *sbrec_chassis_table, > chassis_features->sample_with_reg) { > chassis_features->sample_with_reg = false; > } > + > + bool ct_next_zone = > + smap_get_bool(&chassis->other_config, > + OVN_FEATURE_CT_NEXT_ZONE, > + false); > + if (!ct_next_zone && > + chassis_features->ct_next_zone) { > + chassis_features->ct_next_zone = false; > + } > } > } > > diff --git a/northd/en-global-config.h b/northd/en-global-config.h > index 0cf34482a..767810542 100644 > --- a/northd/en-global-config.h > +++ b/northd/en-global-config.h > @@ -20,6 +20,7 @@ struct chassis_features { > bool ct_commit_nat_v2; > bool ct_commit_to_zone; > bool sample_with_reg; > + bool ct_next_zone; > }; > > struct global_config_tracked_data { > diff --git a/northd/northd.c b/northd/northd.c > index 73367b910..19a484ceb 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -16079,14 +16079,14 @@ build_lrouter_out_snat_flow(struct lflow_table *lflows, > /* For the SNAT networks, we need to make sure that connections are > * properly tracked so we can decide whether to perform SNAT on traffic > * exiting the network. */ > - if (features->ct_commit_to_zone && !strcmp(nat->type, "snat") && > - !od->is_gw_router) { > + if (features->ct_commit_to_zone && features->ct_next_zone && > + !strcmp(nat->type, "snat") && !od->is_gw_router) { > /* For traffic that comes from SNAT network, initiate CT state before > * entering S_ROUTER_OUT_SNAT to allow matching on various CT states. > */ > ds_truncate(match, original_match_len); > ovn_lflow_add(lflows, od, S_ROUTER_OUT_POST_UNDNAT, 70, > - ds_cstr(match), "ct_snat;", > + ds_cstr(match), "ct_next(snat);", > lflow_ref); > > build_lrouter_out_snat_match(lflows, od, nat, match, > diff --git a/ovn-sb.xml b/ovn-sb.xml > index c11296d7c..95116ac56 100644 > --- a/ovn-sb.xml > +++ b/ovn-sb.xml > @@ -1369,6 +1369,8 @@ > </dd> > > <dt><code>ct_next;</code></dt> > + <dt><code>ct_next(dnat);</code></dt> > + <dt><code>ct_next(snat);</code></dt> > <dd> > <p> > Apply connection tracking to the flow, initializing > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 93ccbce6b..8816749c4 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -1133,7 +1133,8 @@ ovn_start > # DR is connected to S1 and CR is connected to S2 > > check ovn-sbctl chassis-add gw1 geneve 127.0.0.1 \ > - -- set chassis gw1 other_config:ct-commit-to-zone="true" > + -- set chassis gw1 other_config:ct-commit-to-zone="true" \ > + -- set chassis gw1 other_config:ct-next-zone="true" > > check ovn-nbctl lr-add DR > check ovn-nbctl lrp-add DR DR-S1 02:ac:10:01:00:01 172.16.1.1/24 > @@ -5721,7 +5722,8 @@ AT_CHECK([grep "lr_out_snat" lr0flows | ovn_strip_lflows], [0], [dnl > ]) > > check ovn-sbctl chassis-add gw1 geneve 127.0.0.1 \ > - -- set chassis gw1 other_config:ct-commit-to-zone="true" > + -- set chassis gw1 other_config:ct-commit-to-zone="true" \ > + -- set chassis gw1 other_config:ct-next-zone="true" > > # Create a distributed gw port on lr0 > check ovn-nbctl ls-add public > @@ -5822,8 +5824,8 @@ AT_CHECK([grep "lr_out_undnat" lr0flows | ovn_strip_lflows], [0], [dnl > > AT_CHECK([grep "lr_out_post_undnat" lr0flows | ovn_strip_lflows], [0], [dnl > table=??(lr_out_post_undnat ), priority=0 , match=(1), action=(next;) > - table=??(lr_out_post_undnat ), priority=70 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat;) > - table=??(lr_out_post_undnat ), priority=70 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat;) > + table=??(lr_out_post_undnat ), priority=70 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_next(snat);) > + table=??(lr_out_post_undnat ), priority=70 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_next(snat);) > ]) > > AT_CHECK([grep "lr_out_snat" lr0flows | ovn_strip_lflows], [0], [dnl > @@ -5980,8 +5982,8 @@ AT_CHECK([grep "lr_out_undnat" lr0flows | ovn_strip_lflows], [0], [dnl > > AT_CHECK([grep "lr_out_post_undnat" lr0flows | ovn_strip_lflows], [0], [dnl > table=??(lr_out_post_undnat ), priority=0 , match=(1), action=(next;) > - table=??(lr_out_post_undnat ), priority=70 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat;) > - table=??(lr_out_post_undnat ), priority=70 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat;) > + table=??(lr_out_post_undnat ), priority=70 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_next(snat);) > + table=??(lr_out_post_undnat ), priority=70 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_next(snat);) > ]) > > AT_CHECK([grep "lr_out_snat" lr0flows | ovn_strip_lflows], [0], [dnl > @@ -7876,13 +7878,16 @@ ovn_start > # distributed gateway LRPs. > > check ovn-sbctl chassis-add gw1 geneve 127.0.0.1 \ > - -- set chassis gw1 other_config:ct-commit-to-zone="true" > + -- set chassis gw1 other_config:ct-commit-to-zone="true" \ > + -- set chassis gw1 other_config:ct-next-zone="true" > > check ovn-sbctl chassis-add gw2 geneve 128.0.0.1 \ > - -- set chassis gw2 other_config:ct-commit-to-zone="true" > + -- set chassis gw2 other_config:ct-commit-to-zone="true" \ > + -- set chassis gw2 other_config:ct-next-zone="true" > > check ovn-sbctl chassis-add gw3 geneve 129.0.0.1 \ > - -- set chassis gw3 other_config:ct-commit-to-zone="true" > + -- set chassis gw3 other_config:ct-commit-to-zone="true" \ > + -- set chassis gw3 other_config:ct-next-zone="true" > > check ovn-nbctl lr-add DR > check ovn-nbctl lrp-add DR DR-S1 02:ac:10:01:00:01 172.16.1.1/24 > diff --git a/tests/ovn.at b/tests/ovn.at > index 50c9f04da..632f060cc 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -1263,11 +1263,27 @@ ct_lb_mark(backends=192.168.1.2:80,192.168.1.3:80; hash_fields="eth_src,eth_dst, > > # ct_next > ct_next; > + formats as ct_next(dnat); > + encodes as ct(table=oflow_in_table,zone=NXM_NX_REG13[[0..15]]) > + has prereqs ip > +ct_next(dnat); > + encodes as ct(table=oflow_in_table,zone=NXM_NX_REG13[[0..15]]) > + has prereqs ip > +ct_next(snat); > encodes as ct(table=oflow_in_table,zone=NXM_NX_REG13[[0..15]]) > has prereqs ip > ct_clear; ct_next; > + formats as ct_clear; ct_next(dnat); > encodes as ct_clear,ct(table=oflow_in_table,zone=NXM_NX_REG13[[0..15]]) > has prereqs ip > +ct_next(snat, dnat); > + Syntax error at `,' expecting `)'. > +ct_next(dnat, ignore); > + Syntax error at `,' expecting `)'. > +ct_next(ignore); > + "ct_next" action accepts only "dnat" or "snat" parameter. > +ct_next(); > + "ct_next" action accepts only "dnat" or "snat" parameter. > > # ct_commit > ct_commit; > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > index 6e4ec4247..01f71161a 100644 > --- a/tests/system-ovn.at > +++ b/tests/system-ovn.at > @@ -3518,7 +3518,7 @@ AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.3 192.168.1.2 foo1 00:0 > AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.4 192.168.1.3 foo2 00:00:02:02:03:05]) > > # Add a SNAT rule > -AT_CHECK([ovn-nbctl lr-nat-add R1 snat 172.16.1.1 192.168.0.0/16]) > +AT_CHECK([ovn-nbctl lr-nat-add R1 snat 172.16.1.1 0.0.0.0/0]) > > # Add default route to ext-net > AT_CHECK([ovn-nbctl lr-route-add R1 10.0.0.0/24 172.16.1.2]) > @@ -3724,8 +3724,7 @@ AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat fd20::3 fd11::2 foo1 00:00:02:02 > AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat fd20::4 fd11::3 foo2 00:00:02:02:03:05]) > > # Add a SNAT rule > -AT_CHECK([ovn-nbctl lr-nat-add R1 snat fd20::1 fd11::/64]) > -AT_CHECK([ovn-nbctl lr-nat-add R1 snat fd20::1 fd12::/64]) > +AT_CHECK([ovn-nbctl lr-nat-add R1 snat fd20::1 ::/0]) > > ovn-nbctl --wait=hv sync > OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int | grep 'nat(src=fd20::1)']) > @@ -3920,7 +3919,7 @@ AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.3 192.168.1.2 foo1 00:0 > AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.4 192.168.2.2 bar1 00:00:02:02:03:05]) > > # Add a SNAT rule > -AT_CHECK([ovn-nbctl lr-nat-add R1 snat 172.16.1.1 192.168.0.0/16]) > +AT_CHECK([ovn-nbctl lr-nat-add R1 snat 172.16.1.1 0.0.0.0/0]) > > ovn-nbctl --wait=hv sync > OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int | grep 'nat(src=172.16.1.1)']) > @@ -4104,8 +4103,7 @@ AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat fd20::3 fd11::2 foo1 00:00:02:02 > AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat fd20::4 fd12::2 bar1 00:00:02:02:03:05]) > > # Add a SNAT rule > -AT_CHECK([ovn-nbctl lr-nat-add R1 snat fd20::1 fd11::/64]) > -AT_CHECK([ovn-nbctl lr-nat-add R1 snat fd20::1 fd12::/64]) > +AT_CHECK([ovn-nbctl lr-nat-add R1 snat fd20::1 ::/0]) > > ovn-nbctl --wait=hv sync > OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int | grep 'nat(src=fd20::1)']) > -- > 2.46.0 >
diff --git a/controller/chassis.c b/controller/chassis.c index 2991a0af3..ee839084a 100644 --- a/controller/chassis.c +++ b/controller/chassis.c @@ -390,6 +390,7 @@ chassis_build_other_config(const struct ovs_chassis_cfg *ovs_cfg, smap_replace(config, OVN_FEATURE_CT_COMMIT_TO_ZONE, "true"); smap_replace(config, OVN_FEATURE_SAMPLE_WITH_REGISTERS, ovs_cfg->sample_with_regs ? "true" : "false"); + smap_replace(config, OVN_FEATURE_CT_NEXT_ZONE, "true"); } /* @@ -549,6 +550,12 @@ chassis_other_config_changed(const struct ovs_chassis_cfg *ovs_cfg, return true; } + if (!smap_get_bool(&chassis_rec->other_config, + OVN_FEATURE_CT_NEXT_ZONE, + false)) { + return true; + } + return false; } @@ -706,6 +713,7 @@ update_supported_sset(struct sset *supported) sset_add(supported, OVN_FEATURE_CT_COMMIT_NAT_V2); sset_add(supported, OVN_FEATURE_CT_COMMIT_TO_ZONE); sset_add(supported, OVN_FEATURE_SAMPLE_WITH_REGISTERS); + sset_add(supported, OVN_FEATURE_CT_NEXT_ZONE); } static void diff --git a/include/ovn/actions.h b/include/ovn/actions.h index c8dd66ed8..a95a0daf7 100644 --- a/include/ovn/actions.h +++ b/include/ovn/actions.h @@ -260,6 +260,7 @@ struct ovnact_push_pop { /* OVNACT_CT_NEXT. */ struct ovnact_ct_next { struct ovnact ovnact; + bool dnat_zone; uint8_t ltable; /* Logical table ID of next table. */ }; diff --git a/include/ovn/features.h b/include/ovn/features.h index 4275f7526..3566ab60f 100644 --- a/include/ovn/features.h +++ b/include/ovn/features.h @@ -30,6 +30,7 @@ #define OVN_FEATURE_CT_COMMIT_NAT_V2 "ct-commit-nat-v2" #define OVN_FEATURE_CT_COMMIT_TO_ZONE "ct-commit-to-zone" #define OVN_FEATURE_SAMPLE_WITH_REGISTERS "ovn-sample-with-registers" +#define OVN_FEATURE_CT_NEXT_ZONE "ct-next-zone" /* OVS datapath supported features. Based on availability OVN might generate * different types of openflows. diff --git a/lib/actions.c b/lib/actions.c index c12d087e7..2e05d4134 100644 --- a/lib/actions.c +++ b/lib/actions.c @@ -701,13 +701,32 @@ parse_CT_NEXT(struct action_context *ctx) } add_prerequisite(ctx, "ip"); - ovnact_put_CT_NEXT(ctx->ovnacts)->ltable = ctx->pp->cur_ltable + 1; + struct ovnact_ct_next *ct_next = ovnact_put_CT_NEXT(ctx->ovnacts); + ct_next->dnat_zone = true; + ct_next->ltable = ctx->pp->cur_ltable + 1; + + if (!lexer_match(ctx->lexer, LEX_T_LPAREN)) { + return; + } + + if (lexer_match_id(ctx->lexer, "dnat")) { + ct_next->dnat_zone = true; + } else if (lexer_match_id(ctx->lexer, "snat")) { + ct_next->dnat_zone = false; + } else { + lexer_error(ctx->lexer, "\"ct_next\" action accepts only" + " \"dnat\" or \"snat\" parameter."); + return; + } + + lexer_force_match(ctx->lexer, LEX_T_RPAREN); } static void format_CT_NEXT(const struct ovnact_ct_next *ct_next OVS_UNUSED, struct ds *s) { - ds_put_cstr(s, "ct_next;"); + ds_put_cstr(s, "ct_next"); + ds_put_cstr(s, ct_next->dnat_zone ? "(dnat);" : "(snat);"); } static void @@ -719,11 +738,17 @@ encode_CT_NEXT(const struct ovnact_ct_next *ct_next, struct ofpact_conntrack *ct = ofpact_put_CT(ofpacts); ct->recirc_table = first_ptable(ep, ep->pipeline) + ct_next->ltable; - ct->zone_src.field = ep->is_switch ? mf_from_id(MFF_LOG_CT_ZONE) - : mf_from_id(MFF_LOG_DNAT_ZONE); ct->zone_src.ofs = 0; ct->zone_src.n_bits = 16; + if (ep->is_switch) { + ct->zone_src.field = mf_from_id(MFF_LOG_CT_ZONE); + } else { + ct->zone_src.field = mf_from_id(ct_next->dnat_zone + ? MFF_LOG_DNAT_ZONE + : MFF_LOG_SNAT_ZONE); + } + ct = ofpbuf_at_assert(ofpacts, ct_offset, sizeof *ct); ofpacts->header = ct; ofpact_finish_CT(ofpacts, &ct); diff --git a/northd/en-global-config.c b/northd/en-global-config.c index 0ce7f8308..fff2aaa16 100644 --- a/northd/en-global-config.c +++ b/northd/en-global-config.c @@ -382,6 +382,7 @@ northd_enable_all_features(struct ed_type_global_config *data) .ct_commit_nat_v2 = true, .ct_commit_to_zone = true, .sample_with_reg = true, + .ct_next_zone = true, }; } @@ -452,6 +453,15 @@ build_chassis_features(const struct sbrec_chassis_table *sbrec_chassis_table, chassis_features->sample_with_reg) { chassis_features->sample_with_reg = false; } + + bool ct_next_zone = + smap_get_bool(&chassis->other_config, + OVN_FEATURE_CT_NEXT_ZONE, + false); + if (!ct_next_zone && + chassis_features->ct_next_zone) { + chassis_features->ct_next_zone = false; + } } } diff --git a/northd/en-global-config.h b/northd/en-global-config.h index 0cf34482a..767810542 100644 --- a/northd/en-global-config.h +++ b/northd/en-global-config.h @@ -20,6 +20,7 @@ struct chassis_features { bool ct_commit_nat_v2; bool ct_commit_to_zone; bool sample_with_reg; + bool ct_next_zone; }; struct global_config_tracked_data { diff --git a/northd/northd.c b/northd/northd.c index 73367b910..19a484ceb 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -16079,14 +16079,14 @@ build_lrouter_out_snat_flow(struct lflow_table *lflows, /* For the SNAT networks, we need to make sure that connections are * properly tracked so we can decide whether to perform SNAT on traffic * exiting the network. */ - if (features->ct_commit_to_zone && !strcmp(nat->type, "snat") && - !od->is_gw_router) { + if (features->ct_commit_to_zone && features->ct_next_zone && + !strcmp(nat->type, "snat") && !od->is_gw_router) { /* For traffic that comes from SNAT network, initiate CT state before * entering S_ROUTER_OUT_SNAT to allow matching on various CT states. */ ds_truncate(match, original_match_len); ovn_lflow_add(lflows, od, S_ROUTER_OUT_POST_UNDNAT, 70, - ds_cstr(match), "ct_snat;", + ds_cstr(match), "ct_next(snat);", lflow_ref); build_lrouter_out_snat_match(lflows, od, nat, match, diff --git a/ovn-sb.xml b/ovn-sb.xml index c11296d7c..95116ac56 100644 --- a/ovn-sb.xml +++ b/ovn-sb.xml @@ -1369,6 +1369,8 @@ </dd> <dt><code>ct_next;</code></dt> + <dt><code>ct_next(dnat);</code></dt> + <dt><code>ct_next(snat);</code></dt> <dd> <p> Apply connection tracking to the flow, initializing diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 93ccbce6b..8816749c4 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -1133,7 +1133,8 @@ ovn_start # DR is connected to S1 and CR is connected to S2 check ovn-sbctl chassis-add gw1 geneve 127.0.0.1 \ - -- set chassis gw1 other_config:ct-commit-to-zone="true" + -- set chassis gw1 other_config:ct-commit-to-zone="true" \ + -- set chassis gw1 other_config:ct-next-zone="true" check ovn-nbctl lr-add DR check ovn-nbctl lrp-add DR DR-S1 02:ac:10:01:00:01 172.16.1.1/24 @@ -5721,7 +5722,8 @@ AT_CHECK([grep "lr_out_snat" lr0flows | ovn_strip_lflows], [0], [dnl ]) check ovn-sbctl chassis-add gw1 geneve 127.0.0.1 \ - -- set chassis gw1 other_config:ct-commit-to-zone="true" + -- set chassis gw1 other_config:ct-commit-to-zone="true" \ + -- set chassis gw1 other_config:ct-next-zone="true" # Create a distributed gw port on lr0 check ovn-nbctl ls-add public @@ -5822,8 +5824,8 @@ AT_CHECK([grep "lr_out_undnat" lr0flows | ovn_strip_lflows], [0], [dnl AT_CHECK([grep "lr_out_post_undnat" lr0flows | ovn_strip_lflows], [0], [dnl table=??(lr_out_post_undnat ), priority=0 , match=(1), action=(next;) - table=??(lr_out_post_undnat ), priority=70 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat;) - table=??(lr_out_post_undnat ), priority=70 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat;) + table=??(lr_out_post_undnat ), priority=70 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_next(snat);) + table=??(lr_out_post_undnat ), priority=70 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_next(snat);) ]) AT_CHECK([grep "lr_out_snat" lr0flows | ovn_strip_lflows], [0], [dnl @@ -5980,8 +5982,8 @@ AT_CHECK([grep "lr_out_undnat" lr0flows | ovn_strip_lflows], [0], [dnl AT_CHECK([grep "lr_out_post_undnat" lr0flows | ovn_strip_lflows], [0], [dnl table=??(lr_out_post_undnat ), priority=0 , match=(1), action=(next;) - table=??(lr_out_post_undnat ), priority=70 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat;) - table=??(lr_out_post_undnat ), priority=70 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat;) + table=??(lr_out_post_undnat ), priority=70 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_next(snat);) + table=??(lr_out_post_undnat ), priority=70 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_next(snat);) ]) AT_CHECK([grep "lr_out_snat" lr0flows | ovn_strip_lflows], [0], [dnl @@ -7876,13 +7878,16 @@ ovn_start # distributed gateway LRPs. check ovn-sbctl chassis-add gw1 geneve 127.0.0.1 \ - -- set chassis gw1 other_config:ct-commit-to-zone="true" + -- set chassis gw1 other_config:ct-commit-to-zone="true" \ + -- set chassis gw1 other_config:ct-next-zone="true" check ovn-sbctl chassis-add gw2 geneve 128.0.0.1 \ - -- set chassis gw2 other_config:ct-commit-to-zone="true" + -- set chassis gw2 other_config:ct-commit-to-zone="true" \ + -- set chassis gw2 other_config:ct-next-zone="true" check ovn-sbctl chassis-add gw3 geneve 129.0.0.1 \ - -- set chassis gw3 other_config:ct-commit-to-zone="true" + -- set chassis gw3 other_config:ct-commit-to-zone="true" \ + -- set chassis gw3 other_config:ct-next-zone="true" check ovn-nbctl lr-add DR check ovn-nbctl lrp-add DR DR-S1 02:ac:10:01:00:01 172.16.1.1/24 diff --git a/tests/ovn.at b/tests/ovn.at index 50c9f04da..632f060cc 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -1263,11 +1263,27 @@ ct_lb_mark(backends=192.168.1.2:80,192.168.1.3:80; hash_fields="eth_src,eth_dst, # ct_next ct_next; + formats as ct_next(dnat); + encodes as ct(table=oflow_in_table,zone=NXM_NX_REG13[[0..15]]) + has prereqs ip +ct_next(dnat); + encodes as ct(table=oflow_in_table,zone=NXM_NX_REG13[[0..15]]) + has prereqs ip +ct_next(snat); encodes as ct(table=oflow_in_table,zone=NXM_NX_REG13[[0..15]]) has prereqs ip ct_clear; ct_next; + formats as ct_clear; ct_next(dnat); encodes as ct_clear,ct(table=oflow_in_table,zone=NXM_NX_REG13[[0..15]]) has prereqs ip +ct_next(snat, dnat); + Syntax error at `,' expecting `)'. +ct_next(dnat, ignore); + Syntax error at `,' expecting `)'. +ct_next(ignore); + "ct_next" action accepts only "dnat" or "snat" parameter. +ct_next(); + "ct_next" action accepts only "dnat" or "snat" parameter. # ct_commit ct_commit; diff --git a/tests/system-ovn.at b/tests/system-ovn.at index 6e4ec4247..01f71161a 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -3518,7 +3518,7 @@ AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.3 192.168.1.2 foo1 00:0 AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.4 192.168.1.3 foo2 00:00:02:02:03:05]) # Add a SNAT rule -AT_CHECK([ovn-nbctl lr-nat-add R1 snat 172.16.1.1 192.168.0.0/16]) +AT_CHECK([ovn-nbctl lr-nat-add R1 snat 172.16.1.1 0.0.0.0/0]) # Add default route to ext-net AT_CHECK([ovn-nbctl lr-route-add R1 10.0.0.0/24 172.16.1.2]) @@ -3724,8 +3724,7 @@ AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat fd20::3 fd11::2 foo1 00:00:02:02 AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat fd20::4 fd11::3 foo2 00:00:02:02:03:05]) # Add a SNAT rule -AT_CHECK([ovn-nbctl lr-nat-add R1 snat fd20::1 fd11::/64]) -AT_CHECK([ovn-nbctl lr-nat-add R1 snat fd20::1 fd12::/64]) +AT_CHECK([ovn-nbctl lr-nat-add R1 snat fd20::1 ::/0]) ovn-nbctl --wait=hv sync OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int | grep 'nat(src=fd20::1)']) @@ -3920,7 +3919,7 @@ AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.3 192.168.1.2 foo1 00:0 AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.4 192.168.2.2 bar1 00:00:02:02:03:05]) # Add a SNAT rule -AT_CHECK([ovn-nbctl lr-nat-add R1 snat 172.16.1.1 192.168.0.0/16]) +AT_CHECK([ovn-nbctl lr-nat-add R1 snat 172.16.1.1 0.0.0.0/0]) ovn-nbctl --wait=hv sync OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int | grep 'nat(src=172.16.1.1)']) @@ -4104,8 +4103,7 @@ AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat fd20::3 fd11::2 foo1 00:00:02:02 AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat fd20::4 fd12::2 bar1 00:00:02:02:03:05]) # Add a SNAT rule -AT_CHECK([ovn-nbctl lr-nat-add R1 snat fd20::1 fd11::/64]) -AT_CHECK([ovn-nbctl lr-nat-add R1 snat fd20::1 fd12::/64]) +AT_CHECK([ovn-nbctl lr-nat-add R1 snat fd20::1 ::/0]) ovn-nbctl --wait=hv sync OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int | grep 'nat(src=fd20::1)'])
In order to get the direct SNAT access working we need to commit new connections so the reply is not marked as invalid. The CT state to determine if the connection should be committed was populated by ct_snat action, however this action also performs the NAT part if the connection is already known and there is an entry for that. This would cause issues when the there is combination of FIPs and SNAT with very broad logical IP. To prevent that use ct_next only which will populate the state but won't do the NAT part which can be done later on if needed according to the conditions. At the same time add support for ct_next in SNAT zone as ct_next was assuming that the zone is always DNAT. Fixes: 40136a2f2c84 ("northd: Fix direct access to SNAT network.") Reported-at: https://issues.redhat.com/browse/FDP-744 Signed-off-by: Ales Musil <amusil@redhat.com> --- controller/chassis.c | 8 ++++++++ include/ovn/actions.h | 1 + include/ovn/features.h | 1 + lib/actions.c | 33 +++++++++++++++++++++++++++++---- northd/en-global-config.c | 10 ++++++++++ northd/en-global-config.h | 1 + northd/northd.c | 6 +++--- ovn-sb.xml | 2 ++ tests/ovn-northd.at | 23 ++++++++++++++--------- tests/ovn.at | 16 ++++++++++++++++ tests/system-ovn.at | 10 ++++------ 11 files changed, 89 insertions(+), 22 deletions(-)