Commit 4deac4509 introduced the new "czone" variants for ct_dnat and ct_snat. This makes it so that a single conntrack zone is used for both DNAT and SNAT on the datapath. The common CT zone chosen in this commit was the DNAT zone.
The ct-snat-zone option for logical routers makes it so that an SNAT zone can be explicitly configured instead of having a zone selected at random by OVN. The problem is that since the DNAT zone is used as the common zone ID, it means that when SNAT is performed, the SNAT is not performed in the configured ct-snat-zone. The fix for this is to change to using the SNAT zone as the common zone if ct-snat-zone is configured on the logical router. This way, SNAT will use the configured zone as expected. DNAT will also use this zone when possible. Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2061619 Signed-off-by: Mark Michelson <[email protected]> --- controller/lflow.c | 18 +++++++++ include/ovn/actions.h | 2 + include/ovn/logical-fields.h | 2 - lib/actions.c | 4 +- tests/ovn.at | 72 ++++++++++++++++++++++++++++++++++++ tests/test-ovn.c | 1 + 6 files changed, 95 insertions(+), 4 deletions(-) diff --git a/controller/lflow.c b/controller/lflow.c index b7ddeb1c0..3c953e18b 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -1104,6 +1104,23 @@ lflow_parse_ctrl_meter(const struct sbrec_logical_flow *lflow, } } +static int +get_common_nat_zone(const struct local_datapath *ldp) +{ + /* Normally, the common NAT zone defaults to the DNAT zone. However, + * if the "snat-ct-zone" is set on the datapath, the user is + * expecting an explicit CT zone to be used for SNAT. If we default + * to the DNAT zone, then it means SNAT will not use the configured + * value. The way we get around this is to use the SNAT zone as the + * common zone if "snat-ct-zone" is set. + */ + if (smap_get(&ldp->datapath->external_ids, "snat-ct-zone")) { + return MFF_LOG_SNAT_ZONE; + } else { + return MFF_LOG_DNAT_ZONE; + } +} + static void add_matches_to_flow_table(const struct sbrec_logical_flow *lflow, const struct local_datapath *ldp, @@ -1153,6 +1170,7 @@ add_matches_to_flow_table(const struct sbrec_logical_flow *lflow, .fdb_ptable = OFTABLE_GET_FDB, .fdb_lookup_ptable = OFTABLE_LOOKUP_FDB, .ctrl_meter_id = ctrl_meter_id, + .common_nat_ct_zone = get_common_nat_zone(ldp), }; ovnacts_encode(ovnacts->data, ovnacts->size, &ep, &ofpacts); diff --git a/include/ovn/actions.h b/include/ovn/actions.h index f55d77d47..547797584 100644 --- a/include/ovn/actions.h +++ b/include/ovn/actions.h @@ -808,6 +808,8 @@ struct ovnact_encode_params { * 'lookup_fdb' to resubmit. */ uint32_t ctrl_meter_id; /* Meter to be used if the resulting flow sends packets to controller. */ + uint32_t common_nat_ct_zone; /* When performing NAT in a common CT zone, + this determines which CT zone to use */ }; void ovnacts_encode(const struct ovnact[], size_t ovnacts_len, diff --git a/include/ovn/logical-fields.h b/include/ovn/logical-fields.h index 25b5a62a3..18516634e 100644 --- a/include/ovn/logical-fields.h +++ b/include/ovn/logical-fields.h @@ -36,8 +36,6 @@ enum ovn_controller_event { * (32 bits). */ #define MFF_LOG_SNAT_ZONE MFF_REG12 /* conntrack snat zone for gateway router * (32 bits). */ -#define MFF_LOG_NAT_ZONE MFF_LOG_DNAT_ZONE /* conntrack zone for both snat - * and dnat. */ #define MFF_LOG_CT_ZONE MFF_REG13 /* Logical conntrack zone for lports * (32 bits). */ #define MFF_LOG_INPORT MFF_REG14 /* Logical input port (32 bits). */ diff --git a/lib/actions.c b/lib/actions.c index a3cf747db..a9c27600c 100644 --- a/lib/actions.c +++ b/lib/actions.c @@ -1131,7 +1131,7 @@ encode_CT_DNAT_IN_CZONE(const struct ovnact_ct_nat *cn, const struct ovnact_encode_params *ep, struct ofpbuf *ofpacts) { - encode_ct_nat(cn, ep, false, MFF_LOG_NAT_ZONE, ofpacts); + encode_ct_nat(cn, ep, false, ep->common_nat_ct_zone, ofpacts); } static void @@ -1139,7 +1139,7 @@ encode_CT_SNAT_IN_CZONE(const struct ovnact_ct_nat *cn, const struct ovnact_encode_params *ep, struct ofpbuf *ofpacts) { - encode_ct_nat(cn, ep, true, MFF_LOG_NAT_ZONE, ofpacts); + encode_ct_nat(cn, ep, true, ep->common_nat_ct_zone, ofpacts); } static void diff --git a/tests/ovn.at b/tests/ovn.at index 799a6aabd..06c607218 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -30505,3 +30505,75 @@ test_mac_binding_flows 150 00:00:11:22:33:88 0 OVN_CLEANUP([hv1]) AT_CLEANUP + +AT_SETUP([snat-ct-zone with common NAT zone]) +# This test sets up a couple of simple NATs. OVN will program logical +# flows for ct_snat_in_czone() and ct_dnat_in_czone() as a result. We +# want to ensure that the common zone used by these flows is the DNAT +# zone. Then, we will set the snat-ct-zone option on our datapath and +# ensure that the flows now use the SNAT zone as the common zone. + +ovn_start + +net_add n1 +sim_add hv1 +as hv1 +check ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 +check ovs-vsctl add-port br-int ls0-hv -- set Interface ls0-hv external-ids:iface-id=ls0-hv + +check ovn-nbctl lr-add lr0 + +check ovn-nbctl ls-add ls0 +check ovn-nbctl lsp-add ls0 ls0-lr0 +check ovn-nbctl lsp-set-type ls0-lr0 router +check ovn-nbctl lsp-set-addresses ls0-lr0 router +check ovn-nbctl lrp-add lr0 lr0-ls0 00:00:00:00:00:01 10.0.0.1 + +check ovn-nbctl lsp-add ls0 ls0-hv +check ovn-nbctl lsp-set-addresses ls0-hv "00:00:00:00:00:02 10.0.0.2" + +check ovn-nbctl ls-add ext +check ovn-nbctl lsp-add ext ext-lr0 +check ovn-nbctl lsp-set-type ext-lr0 router +check ovn-nbctl lsp-set-addresses ext-lr0 router +check ovn-nbctl lrp-add lr0 lr0-ext 00:00:00:00:01:01 20.0.0.1 + +check ovn-nbctl lrp-set-gateway-chassis lr0-ext hv1 + +check ovn-nbctl lr-nat-add lr0 snat 172.16.0.2 10.0.0.0/24 +check ovn-nbctl lr-nat-add lr0 dnat 172.16.0.2 10.0.0.2 + +check ovn-nbctl --wait=hv sync + +# Use constants so that if tables or registers change, this test can +# be updated easily. +DNAT_TABLE=14 +SNAT_TABLE=43 +DNAT_ZONE_REG="NXM_NX_REG11[[0..15]]" +SNAT_ZONE_REG="NXM_NX_REG12[[0..15]]" + +dnat_zone=$(ovs-ofctl dump-flows br-int | grep table=$DNAT_TABLE | grep -o zone=.*, | cut -d '=' -f 2) +dnat_zone=${dnat_zone::-1} +snat_zone=$(ovs-ofctl dump-flows br-int | grep table=$SNAT_TABLE | grep priority=153 | grep -o zone=.*, | cut -d '=' -f 2) +snat_zone=${snat_zone::-1} + +# For now, we expect that the common zone is the dnat zone + +check test "$dnat_zone" = "$DNAT_ZONE_REG" +check test "$snat_zone" = "$DNAT_ZONE_REG" + +check ovn-nbctl --wait=hv set logical_router lr0 options:snat-ct-zone=666 + +dnat_zone=$(ovs-ofctl dump-flows br-int | grep table=$DNAT_TABLE | grep -o zone=.*, | cut -d '=' -f 2) +dnat_zone=${dnat_zone::-1} +snat_zone=$(ovs-ofctl dump-flows br-int | grep table=$SNAT_TABLE | grep priority=153 | grep -o zone=.*, | cut -d '=' -f 2) +snat_zone=${snat_zone::-1} + +# Now with an snat-ct-zone set, the common zone is the snat zone + +check test "$dnat_zone" = "$SNAT_ZONE_REG" +check test "$snat_zone" = "$SNAT_ZONE_REG" + +OVN_CLEANUP([hv1]) +AT_CLEANUP diff --git a/tests/test-ovn.c b/tests/test-ovn.c index d79c6a5bc..6704f612b 100644 --- a/tests/test-ovn.c +++ b/tests/test-ovn.c @@ -1351,6 +1351,7 @@ test_parse_actions(struct ovs_cmdl_context *ctx OVS_UNUSED) .ct_snat_vip_ptable = OFTABLE_CT_SNAT_HAIRPIN, .fdb_ptable = OFTABLE_GET_FDB, .fdb_lookup_ptable = OFTABLE_LOOKUP_FDB, + .common_nat_ct_zone = MFF_LOG_DNAT_ZONE, }; struct ofpbuf ofpacts; ofpbuf_init(&ofpacts, 0); -- 2.31.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
