On distributed routers, the flows that track traffic of an SNAT network
also matched traffic of a stateless dnat_and_snat inside that network,
so it was committed to the SNAT CT zone. Stateless NAT must bypass
conntrack.
Add higher priority flows for stateless dnat_and_snat that just skip
the tracking, reusing the same helper as stateful NAT.
Fixes: 40136a2f2c84 ("northd: Fix direct access to SNAT network.")
Fixes: 79f4cfd9c5d4 ("northd: Avoid committing DNAT traffic to SNAT zone.")
Signed-off-by: Alexandra Rukomoinikova <[email protected]>
---
Documentation/ref/ovn-logical-flows.7.rst | 34 +++++-
northd/northd.c | 128 +++++++++++++---------
tests/ovn-northd.at | 68 ++++++++++++
tests/system-ovn.at | 16 +++
4 files changed, 191 insertions(+), 55 deletions(-)
diff --git a/Documentation/ref/ovn-logical-flows.7.rst
b/Documentation/ref/ovn-logical-flows.7.rst
index 44fd560bf..194074642 100644
--- a/Documentation/ref/ovn-logical-flows.7.rst
+++ b/Documentation/ref/ovn-logical-flows.7.rst
@@ -3784,7 +3784,19 @@ Egress Table 2: Post UNDNAT
- A priority-70 logical flow is added that initiates CT state for traffic that
is configured to be SNATed on Distributed routers. This allows the next
table,
- ``lr_out_snat``, to effectively match on various CT states.
+ ``lr_out_snat``, to effectively match on various CT states. The flow matches
+ on ``ip && ip4.src == A && outport == GW && (!ct.trk || !ct.rpl)`` with an
+ action ``ct_next(snat);``, where *A* is the logical IP or network of the NAT
+ rule and *GW* is the logical router gateway port.
+
+ If the NAT rule is of type dnat_and_snat, the flow is added with priority 75
+ instead, so that the traffic of its logical IP is not tracked by a SNAT rule
+ covering the same network. The action is ``ct_next(dnat);``. If the
+ dnat_and_snat rule has ``stateless=true`` in the options, the flow does not
+ match on CT state and its action is ``next;``, so that the traffic bypasses
+ conntrack.
+
+ These flows are not added if ``options:ct-commit-all`` is set to ``true``.
- A priority-50 logical flow is added that commits any untracked flows from the
previous table :ref:`UNDNAT <lr-out-1>` for Gateway routers. This flow
@@ -3923,7 +3935,17 @@ based on the configuration in the OVN Northbound
database.
``ip4.src == A && outport == GW``, this flow matches on ``ip4.dst == A &&
inport == GW``. A CT state is initiated for this traffic so that the
following
table, ``lr_out_post_snat``, can identify whether the traffic flow was
- initiated from the internal or external network.
+ initiated from the internal or external network. The flow has priority *P*,
+ an additional match ``(!ct.trk || !ct.rpl)`` and an action ``ct_snat;``.
+
+ If the NAT rule is of type dnat_and_snat, the flow is added with priority
+ ``P + 5`` and an action ``ct_dnat;``, so that the traffic of its logical IP
+ is tracked in the DNAT CT zone instead of the SNAT CT zone of a SNAT rule
+ covering the same network. If the dnat_and_snat rule has ``stateless=true``
+ in the options, the flow does not match on CT state and its action is
+ ``next;``, so that the traffic bypasses conntrack.
+
+ This flow is not added if ``options:ct-commit-all`` is set to ``true``.
- If the ``options:ct-commit-all`` is set to ``true`` the following two flows
are configured matching on ``ip && (!ct.trk || !ct.rpl) && flags.unsnat_new
==
@@ -3945,8 +3967,12 @@ Packets reaching this table are processed according to
the flows below:
routers, and was initiated from an external network (i.e. it matches
``ct.new``), is committed to the SNAT CT zone. This ensures that replies
returning from the SNATed network do not have their source address
translated.
- For details about match rules and priority see section :ref:`SNAT on
- Distributed Routers <lr-out-3>`.
+ The action is ``ct_commit_to_zone(snat);``. If the NAT rule is of type
+ dnat_and_snat, the traffic is committed to the DNAT CT zone with an action
+ ``ct_commit_to_zone(dnat);`` instead. Traffic of a dnat_and_snat rule that
+ has ``stateless=true`` in the options is not committed. For details about
+ match rules and priority see section :ref:`SNAT on Distributed Routers
+ <lr-out-3>`.
- A priority-0 logical flow that matches all packets not already handled (match
``1``) and action ``next;``.
diff --git a/northd/northd.c b/northd/northd.c
index f37040b57..4e4849f8c 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -18373,6 +18373,69 @@ build_lrouter_out_snat_match(struct lflow_table
*lflows,
}
}
+static void
+build_lrouter_out_snat_track_flows(struct lflow_table *lflows,
+ const struct ovn_datapath *od,
+ const struct ovn_nat *nat_entry,
+ struct ds *match, struct ds *actions,
+ bool distributed_nat, int cidr_bits,
+ bool is_v6, struct ovn_port *l3dgw_port,
+ struct lflow_ref *lflow_ref,
+ bool commit_all,
+ const struct chassis_features *features,
+ bool stateless)
+{
+ if (!features->ct_commit_to_zone || !features->ct_next_zone ||
+ od->is_gw_router || commit_all || lrouter_use_common_zone(od)) {
+ return;
+ }
+
+ const struct nbrec_nat *nat = nat_entry->nb;
+ uint16_t priority = lrouter_nat_get_priority(od, nat, false, cidr_bits);
+ const char *zone = nat_entry->type == SNAT ? "snat" : "dnat";
+ uint16_t prio_offset = nat_entry->type == SNAT ? 0 : 5;
+
+ build_lrouter_out_snat_match(lflows, od, nat, match, distributed_nat,
+ cidr_bits, is_v6, l3dgw_port, lflow_ref,
+ false);
+ ds_clear(actions);
+ if (stateless) {
+ ds_put_cstr(actions, "next;");
+ } else {
+ ds_put_cstr(match, " && (!ct.trk || !ct.rpl)");
+ ds_put_format(actions, "ct_next(%s);", zone);
+ }
+ ovn_lflow_add(lflows, od, S_ROUTER_OUT_POST_UNDNAT, 70 + prio_offset,
+ ds_cstr(match), ds_cstr(actions), lflow_ref,
+ WITH_HINT(&nat->header_));
+
+ build_lrouter_out_snat_match(lflows, od, nat, match, distributed_nat,
+ cidr_bits, is_v6, l3dgw_port, lflow_ref,
+ true);
+ if (stateless) {
+ ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT, priority + prio_offset,
+ ds_cstr(match), "next;", lflow_ref,
+ WITH_HINT(&nat->header_));
+ return;
+ }
+
+ size_t match_any_state_len = match->length;
+ ds_put_cstr(match, " && (!ct.trk || !ct.rpl)");
+ ds_clear(actions);
+ ds_put_format(actions, "ct_%s;", zone);
+ ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT, priority + prio_offset,
+ ds_cstr(match), ds_cstr(actions), lflow_ref,
+ WITH_HINT(&nat->header_));
+
+ ds_truncate(match, match_any_state_len);
+ ds_put_cstr(match, " && ct.new");
+ ds_clear(actions);
+ ds_put_format(actions, "ct_commit_to_zone(%s);", zone);
+ ovn_lflow_add(lflows, od, S_ROUTER_OUT_POST_SNAT, priority + prio_offset,
+ ds_cstr(match), ds_cstr(actions), lflow_ref,
+ WITH_HINT(&nat->header_));
+}
+
static void
build_lrouter_out_snat_stateless_flow(struct lflow_table *lflows,
const struct ovn_datapath *od,
@@ -18381,7 +18444,9 @@ build_lrouter_out_snat_stateless_flow(struct
lflow_table *lflows,
bool distributed_nat,
struct eth_addr mac, int cidr_bits,
bool is_v6, struct ovn_port *l3dgw_port,
- struct lflow_ref *lflow_ref)
+ struct lflow_ref *lflow_ref,
+ bool commit_all,
+ const struct chassis_features *features)
{
if (!(nat_entry->type == SNAT || nat_entry->type == DNAT_AND_SNAT)) {
return;
@@ -18405,6 +18470,11 @@ build_lrouter_out_snat_stateless_flow(struct
lflow_table *lflows,
ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT, priority, ds_cstr(match),
ds_cstr(actions), lflow_ref, WITH_HINT(&nat->header_));
+
+ build_lrouter_out_snat_track_flows(lflows, od, nat_entry, match, actions,
+ distributed_nat, cidr_bits, is_v6,
+ l3dgw_port, lflow_ref, commit_all,
+ features, true);
}
static void
@@ -18501,55 +18571,10 @@ build_lrouter_out_snat_flow(struct lflow_table
*lflows,
ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT, priority, ds_cstr(match),
ds_cstr(actions), lflow_ref, WITH_HINT(&nat->header_));
- /* 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 && features->ct_next_zone &&
- !od->is_gw_router && !commit_all) {
- const char *zone;
- uint16_t prio_offset;
- if (nat_entry->type == SNAT) {
- /* Traffic to/from hosts behind SNAT is tracked through the
- * SNAT CT zone.*/
- zone = "snat";
- prio_offset = 0;
- } else {
- /* Traffic to/from hosts behind DNAT_AND_SNAT is tracked through
- * the DNAT CT zone with slightly higher priority flows.*/
- zone = "dnat";
- prio_offset = 5;
- }
-
- /* For traffic that comes from the SNAT network, initiate CT state
- * from the correct zone, before entering S_ROUTER_OUT_SNAT to allow
- * matching on various CT states.*/
- ds_clear(actions);
- ds_put_format(actions, "ct_next(%s);", zone);
- ovn_lflow_add(lflows, od, S_ROUTER_OUT_POST_UNDNAT, 70 + prio_offset,
- ds_cstr(match), ds_cstr(actions),
- lflow_ref);
-
- build_lrouter_out_snat_match(lflows, od, nat, match,
- distributed_nat, cidr_bits, is_v6,
- l3dgw_port, lflow_ref, true);
- size_t match_any_state_len = match->length;
- ds_put_cstr(match, " && (!ct.trk || !ct.rpl)");
- ds_clear(actions);
- ds_put_format(actions, "ct_%s;", zone);
- ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT, priority + prio_offset,
- ds_cstr(match), ds_cstr(actions),
- lflow_ref);
-
- /* New traffic that goes into the SNAT network is committed to the
- * correct CT zone to avoid SNAT-ing replies.*/
- ds_truncate(match, match_any_state_len);
- ds_put_cstr(match, " && ct.new");
- ds_clear(actions);
- ds_put_format(actions, "ct_commit_to_zone(%s);", zone);
- ovn_lflow_add(lflows, od, S_ROUTER_OUT_POST_SNAT,
- priority + prio_offset, ds_cstr(match), ds_cstr(actions),
- lflow_ref);
- }
+ build_lrouter_out_snat_track_flows(lflows, od, nat_entry, match, actions,
+ distributed_nat, cidr_bits, is_v6,
+ l3dgw_port, lflow_ref, commit_all,
+ features, false);
}
static void
@@ -19072,7 +19097,8 @@ build_lrouter_nat_defrag_and_lb(
nat_entry->is_distributed,
nat_entry->mac, cidr_bits,
is_v6,
nat_entry->l3dgw_port,
- lflow_ref);
+ lflow_ref, commit_all,
+ features);
} else if (lrouter_use_common_zone(od)) {
build_lrouter_out_snat_in_czone_flow(lflows, od, nat_entry, match,
actions,
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index f8c144918..c41617fd4 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -1431,6 +1431,7 @@ AT_CHECK([grep -e "lr_out_snat" drflows5 |
ovn_strip_lflows], [0], [dnl
table=??(lr_out_snat ), priority=0 , match=(1), action=(next;)
table=??(lr_out_snat ), priority=120 , match=(nd_ns), action=(next;)
table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 50.0.0.11 && outport ==
"DR-S1" && is_chassis_resident("cr-DR-S1") && ip4.dst == $allowed_range),
action=(ip4.src=172.16.1.2; next;)
+ table=??(lr_out_snat ), priority=166 , match=(ip && ip4.dst == 50.0.0.11 && inport == "DR-S1"
&& is_chassis_resident("cr-DR-S1") && ip4.src == $allowed_range), action=(next;)
])
AT_CHECK([grep -e "lr_out_snat" crflows5 | ovn_strip_lflows], [0], [dnl
@@ -1461,6 +1462,7 @@ AT_CHECK([grep -e "lr_out_snat" drflows6 |
ovn_strip_lflows], [0], [dnl
table=??(lr_out_snat ), priority=120 , match=(nd_ns), action=(next;)
table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 50.0.0.11 && outport ==
"DR-S1" && is_chassis_resident("cr-DR-S1")), action=(ip4.src=172.16.1.2; next;)
table=??(lr_out_snat ), priority=163 , match=(ip && ip4.src == 50.0.0.11 && outport ==
"DR-S1" && is_chassis_resident("cr-DR-S1") && ip4.dst == $disallowed_range), action=(next;)
+ table=??(lr_out_snat ), priority=166 , match=(ip && ip4.dst == 50.0.0.11 && inport ==
"DR-S1" && is_chassis_resident("cr-DR-S1")), action=(next;)
])
AT_CHECK([grep -e "lr_out_snat" crflows6 | ovn_strip_lflows], [0], [dnl
@@ -1474,6 +1476,72 @@ OVN_CLEANUP_NORTHD
AT_CLEANUP
])
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([Stateless dnat_and_snat inside SNAT network on distributed router])
+ovn_start
+
+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-next-zone="true"
+
+check ovn-nbctl lr-add DR
+check ovn-nbctl lrp-add DR DR-public 02:ac:10:01:00:01 172.16.1.1/24
+check ovn-nbctl lrp-add DR DR-S1 02:ac:10:01:00:02 50.0.0.1/24
+check ovn-nbctl lrp-set-gateway-chassis DR-public gw1
+
+check ovn-nbctl lr-nat-add DR snat 172.16.1.10 50.0.0.0/24
+check ovn-nbctl --stateless lr-nat-add DR dnat_and_snat 172.16.1.2 50.0.0.11
+check ovn-nbctl --wait=sb sync
+
+dnl Traffic to/from the stateless NAT logical IP must not be tracked by
+dnl the flows of the SNAT covering its network.
+ovn-sbctl dump-flows DR > drflows
+AT_CAPTURE_FILE([drflows])
+
+AT_CHECK([grep -e "lr_out_post_undnat" drflows | 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 == 50.0.0.0/24 && outport ==
"DR-public" && is_chassis_resident("cr-DR-public") && (!ct.trk || !ct.rpl)),
action=(ct_next(snat);)
+ table=??(lr_out_post_undnat ), priority=75 , match=(ip && ip4.src == 50.0.0.11 && outport ==
"DR-public" && is_chassis_resident("cr-DR-public")), action=(next;)
+])
+
+AT_CHECK([grep -e "lr_out_snat" drflows | ovn_strip_lflows], [0], [dnl
+ table=??(lr_out_snat ), priority=0 , match=(1), action=(next;)
+ table=??(lr_out_snat ), priority=120 , match=(nd_ns), action=(next;)
+ table=??(lr_out_snat ), priority=153 , match=(ip && ip4.dst == 50.0.0.0/24 && inport ==
"DR-public" && is_chassis_resident("cr-DR-public") && (!ct.trk || !ct.rpl)), action=(ct_snat;)
+ table=??(lr_out_snat ), priority=153 , match=(ip && ip4.src == 50.0.0.0/24 && outport ==
"DR-public" && is_chassis_resident("cr-DR-public") && (!ct.trk || !ct.rpl)),
action=(ct_snat(172.16.1.10);)
+ table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 50.0.0.11 && outport ==
"DR-public" && is_chassis_resident("cr-DR-public")), action=(ip4.src=172.16.1.2; next;)
+ table=??(lr_out_snat ), priority=166 , match=(ip && ip4.dst == 50.0.0.11 && inport ==
"DR-public" && is_chassis_resident("cr-DR-public")), action=(next;)
+])
+
+AT_CHECK([grep -e "lr_out_post_snat" drflows | ovn_strip_lflows], [0], [dnl
+ table=??(lr_out_post_snat ), priority=0 , match=(1), action=(next;)
+ table=??(lr_out_post_snat ), priority=153 , match=(ip && ip4.dst == 50.0.0.0/24 && inport ==
"DR-public" && is_chassis_resident("cr-DR-public") && ct.new), action=(ct_commit_to_zone(snat);)
+])
+
+dnl With ct-commit-all the SNAT network is not tracked by these flows.
+check ovn-nbctl --wait=sb set logical_router DR options:ct-commit-all="true"
+ovn-sbctl dump-flows DR > drflows2
+AT_CAPTURE_FILE([drflows2])
+
+AT_CHECK([grep -e "lr_out_post_undnat" drflows2 | ovn_strip_lflows], [0], [dnl
+ table=??(lr_out_post_undnat ), priority=0 , match=(1), action=(next;)
+ table=??(lr_out_post_undnat ), priority=10 , match=(ip && (!ct.trk || !ct.rpl) && flags.unsnat_not_tracked ==
1 && outport == "DR-public" && is_chassis_resident("cr-DR-public")), action=(ct_next(snat);)
+ table=??(lr_out_post_undnat ), priority=10 , match=(ip && flags.unsnat_new == 1 && outport ==
"DR-public" && is_chassis_resident("cr-DR-public")), action=(next;)
+])
+
+AT_CHECK([grep -e "lr_out_snat" drflows2 | ovn_strip_lflows], [0], [dnl
+ table=??(lr_out_snat ), priority=0 , match=(1), action=(next;)
+ table=??(lr_out_snat ), priority=10 , match=(ip && (!ct.trk || !ct.rpl) && flags.unsnat_new == 1
&& outport == "DR-public" && is_chassis_resident("cr-DR-public")),
action=(ct_commit_to_zone(snat);)
+ table=??(lr_out_snat ), priority=10 , match=(ip && ct.new && outport == "DR-public"
&& is_chassis_resident("cr-DR-public")), action=(ct_commit_to_zone(snat);)
+ table=??(lr_out_snat ), priority=120 , match=(nd_ns), action=(next;)
+ table=??(lr_out_snat ), priority=153 , match=(ip && ip4.src == 50.0.0.0/24 && outport ==
"DR-public" && is_chassis_resident("cr-DR-public") && (!ct.trk || !ct.rpl)),
action=(ct_snat(172.16.1.10);)
+ table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 50.0.0.11 && outport ==
"DR-public" && is_chassis_resident("cr-DR-public")), action=(ip4.src=172.16.1.2; next;)
+])
+
+OVN_CLEANUP_NORTHD
+AT_CLEANUP
+])
+
OVN_FOR_EACH_NORTHD_NO_HV([
AT_SETUP([check Load balancer health check and Service Monitor sync])
ovn_start
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 13e62bf9b..d9c33b03a 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -3677,6 +3677,22 @@ NS_CHECK_CONNECTIVITY([alice1], [foo1], [192.168.1.2])
# North-South Direct (Bypassing SNAT): 'alice1' reaches 'bar1' using
192.168.2.2
NS_CHECK_CONNECTIVITY([alice1], [bar1], [192.168.2.2])
+AT_CHECK([ovs-appctl dpctl/flush-conntrack])
+
+# Stateless DNAT_AND_SNAT inside the SNAT network: its traffic must not be
+# tracked by the flows of the SNAT covering the network.
+check ovn-nbctl --wait=hv --stateless lr-nat-add R1 dnat_and_snat 172.16.1.5
192.168.2.2
+
+# North-South stateless DNAT: 'alice1' reaches 'bar1' on 172.16.1.5.
+NS_CHECK_CONNECTIVITY([alice1], [bar1], [172.16.1.5])
+
+# South-North stateless SNAT: 'bar1' reaches 'alice1' from 172.16.1.5.
+NS_CHECK_CONNECTIVITY([bar1], [alice1], [172.16.1.2])
+
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(192.168.2.2) | \
+sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl
+])
+
# Try to ping external network
NETNS_START_TCPDUMP([ext-net], [-n -c 3 -i ext-veth dst 172.16.1.3 and icmp],
[ext-net])
AT_CHECK([ovn-nbctl lr-nat-del R1 snat])
