Commit 40136a2f2c84 introduced the ability to directly reach networks behind
SNAT and DNAT_AND_SNAT on Distributed Routers, matching the behavior
of Gateway Routers. This was achieved by committing the incoming traffic into
the SNAT TC zone, which allowed the router to SNAT only traffic that originated
from inside the SNATed network.
An unintended consequence was that incoming traffic destined for the external
IP of a DNAT_AND_SNAT rule would also be committed to the SNAT CT zone,
resulting in an unnecessary entry lingering in the table until it expired.
This change attempts to fix that by handling traffic matching DNAT_AND_SNAT
rule with slightly higher priority than the simple SNAT, and committing it to
DNAT CT zone instead.
Fixes: 40136a2f2c84 ("northd: Fix direct access to SNAT network.")
Signed-off-by: Martin Kalcok <martin.kal...@canonical.com>
---
northd/northd.c | 48 +++++++++++++++++++++++++++++++++------------
tests/ovn-northd.at | 27 ++++++++++++++++---------
tests/system-ovn.at | 2 --
3 files changed, 53 insertions(+), 24 deletions(-)
diff --git a/northd/northd.c b/northd/northd.c
index 8b5413ef3..6db6c7aed 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -16611,27 +16611,49 @@ build_lrouter_out_snat_flow(struct lflow_table
*lflows,
* 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 &&
- nat_entry->type == SNAT && !od->is_gw_router && !commit_all) {
- /* For traffic that comes from SNAT network, initiate CT state before
- * entering S_ROUTER_OUT_SNAT to allow matching on various CT states.
- */
- ovn_lflow_add(lflows, od, S_ROUTER_OUT_POST_UNDNAT, 70,
- ds_cstr(match), "ct_next(snat);",
+ !od->is_gw_router && !commit_all) {
+ const char *zone;
+ uint16_t prio_offset;
+ /* Traffic to/from hosts behind SNAT is tracked through the
+ * SNAT CT zone.*/
+ if (nat_entry->type == SNAT) {
+ zone = "snat";
+ prio_offset = 0;
+ /* Traffic to/from hosts behind DNAT_AND_SNAT is tracked through the
+ * DNAT CT zone with slightly higher priority flows.*/
+ } else {
+ 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);
+ /* New traffic that goes into the SNAT network is committed to the
+ * correct CT zone to avoid SNAT-ing replies.*/
build_lrouter_out_snat_match(lflows, od, nat, match,
distributed_nat, cidr_bits, is_v6,
l3dgw_port, lflow_ref, true);
-
- /* New traffic that goes into SNAT network is committed to CT to avoid
- * SNAT-ing replies.*/
- ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT, priority,
- ds_cstr(match), "ct_snat;",
+ 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);
+ ds_truncate(match, match_any_state_len);
ds_put_cstr(match, " && ct.new");
- ovn_lflow_add(lflows, od, S_ROUTER_OUT_POST_SNAT, priority,
- ds_cstr(match), "ct_commit_to_zone(snat);",
+ 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);
}
}
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index c9e998129..d99815dd6 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -1242,7 +1242,7 @@ AT_CAPTURE_FILE([crflows])
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=161 , match=(ip && ip4.dst ==
50.0.0.11 && inport == "DR-S1" && is_chassis_resident("cr-DR-S1") && ip4.src ==
$allowed_range), action=(ct_snat;)
+ table=??(lr_out_snat ), priority=161 , match=(ip && ip4.dst ==
50.0.0.11 && inport == "DR-S1" && is_chassis_resident("cr-DR-S1") && ip4.src ==
$allowed_range && (!ct.trk || !ct.rpl)), action=(ct_snat;)
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 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.16.1.1);)
])
@@ -1281,7 +1281,7 @@ AT_CAPTURE_FILE([crflows2])
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=120 , match=(nd_ns), action=(next;)
- table=??(lr_out_snat ), priority=161 , match=(ip && ip4.dst ==
50.0.0.11 && inport == "DR-S1" && is_chassis_resident("cr-DR-S1")),
action=(ct_snat;)
+ table=??(lr_out_snat ), priority=161 , match=(ip && ip4.dst ==
50.0.0.11 && inport == "DR-S1" && is_chassis_resident("cr-DR-S1") && (!ct.trk
|| !ct.rpl)), action=(ct_snat;)
table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src ==
50.0.0.11 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1") && (!ct.trk
|| !ct.rpl)), action=(ct_snat(172.16.1.1);)
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;)
])
@@ -1321,10 +1321,12 @@ AT_CHECK([grep -e "lr_out_snat" drflows3 |
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 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.16.1.2);)
+ 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 && (!ct.trk || !ct.rpl)), action=(ct_dnat;)
])
AT_CHECK([grep -e "lr_out_post_snat" drflows3 | ovn_strip_lflows], [0], [dnl
table=??(lr_out_post_snat ), priority=0 , match=(1), action=(next;)
+ table=??(lr_out_post_snat ), priority=166 , match=(ip && ip4.dst ==
50.0.0.11 && inport == "DR-S1" && is_chassis_resident("cr-DR-S1") && ip4.src ==
$allowed_range && ct.new), action=(ct_commit_to_zone(dnat);)
])
AT_CHECK([grep -e "lr_out_snat" crflows3 | ovn_strip_lflows], [0], [dnl
@@ -1357,6 +1359,7 @@ AT_CHECK([grep -e "lr_out_snat" drflows4 |
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") && (!ct.trk
|| !ct.rpl)), action=(ct_snat(172.16.1.2);)
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") && (!ct.trk
|| !ct.rpl)), action=(ct_dnat;)
])
AT_CHECK([grep -e "lr_out_snat" crflows4 | ovn_strip_lflows], [0], [dnl
@@ -5999,22 +6002,25 @@ 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")
&& (!ct.trk || !ct.rpl)), 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") &&
(!ct.trk || !ct.rpl)), action=(ct_next(snat);)
+ table=??(lr_out_post_undnat ), priority=75 , match=(ip && ip4.src ==
10.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") &&
(!ct.trk || !ct.rpl)), action=(ct_next(dnat);)
])
AT_CHECK([grep "lr_out_snat" lr0flows | 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 ==
10.0.0.0/24 && inport == "lr0-public" && is_chassis_resident("cr-lr0-public")),
action=(ct_snat;)
+ table=??(lr_out_snat ), priority=153 , match=(ip && ip4.dst ==
10.0.0.0/24 && inport == "lr0-public" && is_chassis_resident("cr-lr0-public")
&& (!ct.trk || !ct.rpl)), action=(ct_snat;)
table=??(lr_out_snat ), priority=153 , match=(ip && ip4.src ==
10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")
&& (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.10);)
- table=??(lr_out_snat ), priority=161 , match=(ip && ip4.dst ==
10.0.0.10 && inport == "lr0-public" && is_chassis_resident("cr-lr0-public")),
action=(ct_snat;)
+ table=??(lr_out_snat ), priority=161 , match=(ip && ip4.dst ==
10.0.0.10 && inport == "lr0-public" && is_chassis_resident("cr-lr0-public") &&
(!ct.trk || !ct.rpl)), action=(ct_snat;)
table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src ==
10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") &&
(!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.30);)
table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src ==
10.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") &&
(!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.20);)
+ table=??(lr_out_snat ), priority=166 , match=(ip && ip4.dst ==
10.0.0.3 && inport == "lr0-public" && is_chassis_resident("cr-lr0-public") &&
(!ct.trk || !ct.rpl)), action=(ct_dnat;)
])
AT_CHECK([grep "lr_out_post_snat" lr0flows | 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 ==
10.0.0.0/24 && inport == "lr0-public" && is_chassis_resident("cr-lr0-public")
&& ct.new), action=(ct_commit_to_zone(snat);)
table=??(lr_out_post_snat ), priority=161 , match=(ip && ip4.dst ==
10.0.0.10 && inport == "lr0-public" && is_chassis_resident("cr-lr0-public") &&
ct.new), action=(ct_commit_to_zone(snat);)
+ table=??(lr_out_post_snat ), priority=166 , match=(ip && ip4.dst ==
10.0.0.3 && inport == "lr0-public" && is_chassis_resident("cr-lr0-public") &&
ct.new), action=(ct_commit_to_zone(dnat);)
])
# Associate load balancer to lr0
@@ -6157,22 +6163,25 @@ 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")
&& (!ct.trk || !ct.rpl)), 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") &&
(!ct.trk || !ct.rpl)), action=(ct_next(snat);)
+ table=??(lr_out_post_undnat ), priority=75 , match=(ip && ip4.src ==
10.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") &&
(!ct.trk || !ct.rpl)), action=(ct_next(dnat);)
])
AT_CHECK([grep "lr_out_snat" lr0flows | 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 ==
10.0.0.0/24 && inport == "lr0-public" && is_chassis_resident("cr-lr0-public")),
action=(ct_snat;)
+ table=??(lr_out_snat ), priority=153 , match=(ip && ip4.dst ==
10.0.0.0/24 && inport == "lr0-public" && is_chassis_resident("cr-lr0-public")
&& (!ct.trk || !ct.rpl)), action=(ct_snat;)
table=??(lr_out_snat ), priority=153 , match=(ip && ip4.src ==
10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")
&& (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.10);)
- table=??(lr_out_snat ), priority=161 , match=(ip && ip4.dst ==
10.0.0.10 && inport == "lr0-public" && is_chassis_resident("cr-lr0-public")),
action=(ct_snat;)
+ table=??(lr_out_snat ), priority=161 , match=(ip && ip4.dst ==
10.0.0.10 && inport == "lr0-public" && is_chassis_resident("cr-lr0-public") &&
(!ct.trk || !ct.rpl)), action=(ct_snat;)
table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src ==
10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") &&
(!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.30);)
table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src ==
10.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") &&
(!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.20);)
+ table=??(lr_out_snat ), priority=166 , match=(ip && ip4.dst ==
10.0.0.3 && inport == "lr0-public" && is_chassis_resident("cr-lr0-public") &&
(!ct.trk || !ct.rpl)), action=(ct_dnat;)
])
AT_CHECK([grep "lr_out_post_snat" lr0flows | 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 ==
10.0.0.0/24 && inport == "lr0-public" && is_chassis_resident("cr-lr0-public")
&& ct.new), action=(ct_commit_to_zone(snat);)
table=??(lr_out_post_snat ), priority=161 , match=(ip && ip4.dst ==
10.0.0.10 && inport == "lr0-public" && is_chassis_resident("cr-lr0-public") &&
ct.new), action=(ct_commit_to_zone(snat);)
+ table=??(lr_out_post_snat ), priority=166 , match=(ip && ip4.dst ==
10.0.0.3 && inport == "lr0-public" && is_chassis_resident("cr-lr0-public") &&
ct.new), action=(ct_commit_to_zone(dnat);)
])
# Make the logical router as Gateway router
@@ -8389,9 +8398,9 @@ AT_CHECK([grep lr_in_unsnat lrflows | grep ct_snat |
ovn_strip_lflows], [0], [dn
])
AT_CHECK([grep lr_out_snat lrflows | grep ct_snat | ovn_strip_lflows], [0],
[dnl
- table=??(lr_out_snat ), priority=161 , match=(ip && ip4.dst ==
20.0.0.10 && inport == "DR-S1" && is_chassis_resident("cr-DR-S1")),
action=(ct_snat;)
- table=??(lr_out_snat ), priority=161 , match=(ip && ip4.dst ==
20.0.0.10 && inport == "DR-S2" && is_chassis_resident("cr-DR-S2")),
action=(ct_snat;)
- table=??(lr_out_snat ), priority=161 , match=(ip && ip4.dst ==
20.0.0.10 && inport == "DR-S3" && is_chassis_resident("cr-DR-S3")),
action=(ct_snat;)
+ table=??(lr_out_snat ), priority=161 , match=(ip && ip4.dst ==
20.0.0.10 && inport == "DR-S1" && is_chassis_resident("cr-DR-S1") && (!ct.trk
|| !ct.rpl)), action=(ct_snat;)
+ table=??(lr_out_snat ), priority=161 , match=(ip && ip4.dst ==
20.0.0.10 && inport == "DR-S2" && is_chassis_resident("cr-DR-S2") && (!ct.trk
|| !ct.rpl)), action=(ct_snat;)
+ table=??(lr_out_snat ), priority=161 , match=(ip && ip4.dst ==
20.0.0.10 && inport == "DR-S3" && is_chassis_resident("cr-DR-S3") && (!ct.trk
|| !ct.rpl)), action=(ct_snat;)
table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src ==
20.0.0.10 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1") && (!ct.trk
|| !ct.rpl)), action=(ct_snat(172.16.1.10);)
table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src ==
20.0.0.10 && outport == "DR-S2" && is_chassis_resident("cr-DR-S2") && (!ct.trk
|| !ct.rpl)), action=(ct_snat(10.0.0.10);)
table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src ==
20.0.0.10 && outport == "DR-S3" && is_chassis_resident("cr-DR-S3") && (!ct.trk
|| !ct.rpl)), action=(ct_snat(192.168.0.10);)
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index e6688c381..0d01582a4 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -4176,7 +4176,6 @@ NS_CHECK_EXEC([foo2], [ping -q -c 3 -i 0.3 -w 2
172.16.1.4 | FORMAT_PING], \
AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep icmp | FORMAT_CT(172.16.1.1)
| \
sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl
icmp,orig=(src=172.16.1.1,dst=172.16.1.4,id=<cleared>,type=8,code=0),reply=(src=192.168.2.2,dst=172.16.1.1,id=<cleared>,type=0,code=0),zone=<cleared>
-icmp,orig=(src=172.16.1.1,dst=192.168.2.2,id=<cleared>,type=8,code=0),reply=(src=192.168.2.2,dst=172.16.1.1,id=<cleared>,type=0,code=0),zone=<cleared>
icmp,orig=(src=192.168.1.3,dst=172.16.1.4,id=<cleared>,type=8,code=0),reply=(src=172.16.1.4,dst=172.16.1.1,id=<cleared>,type=0,code=0),zone=<cleared>
])
@@ -4344,7 +4343,6 @@ NS_CHECK_EXEC([foo2], [ping -q -c 3 -i 0.3 -w 2 fd20::4 |
FORMAT_PING], \
AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(fd20::1) | \
sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl
icmpv6,orig=(src=fd11::3,dst=fd20::4,id=<cleared>,type=128,code=0),reply=(src=fd20::4,dst=fd20::1,id=<cleared>,type=129,code=0),zone=<cleared>
-icmpv6,orig=(src=fd20::1,dst=fd12::2,id=<cleared>,type=128,code=0),reply=(src=fd12::2,dst=fd20::1,id=<cleared>,type=129,code=0),zone=<cleared>
icmpv6,orig=(src=fd20::1,dst=fd20::4,id=<cleared>,type=128,code=0),reply=(src=fd12::2,dst=fd20::1,id=<cleared>,type=129,code=0),zone=<cleared>
])
--
2.48.1
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
