Match the drop flow for stateless dnat_and_snat flow in S_ROUTER_IN_GW_REDIRECT
stage just if allowed_ext_ips or exempted_ext_ips conditions do not
match since it breaks the hairping scenario with stateless nat.
Fix the northd documentation.
Fixes: c0224a14f ("northd: fix stateless nat with allowed_ext_ips")
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2094980
Signed-off-by: Lorenzo Bianconi <[email protected]>
---
northd/northd.c | 43 ++++++++++++++++++++++++++++-------------
northd/ovn-northd.8.xml | 20 ++++++++++++++-----
tests/system-ovn.at | 15 ++++++++++++++
3 files changed, 60 insertions(+), 18 deletions(-)
diff --git a/northd/northd.c b/northd/northd.c
index 0207f6ce1..5264dbd71 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -12035,6 +12035,7 @@ build_gateway_redirect_flows_for_lrouter(
}
for (size_t i = 0; i < od->n_l3dgw_ports; i++) {
const struct ovsdb_idl_row *stage_hint = NULL;
+ bool add_def_flow = true;
if (od->l3dgw_ports[i]->nbrp) {
stage_hint = &od->l3dgw_ports[i]->nbrp->header_;
@@ -12053,22 +12054,38 @@ build_gateway_redirect_flows_for_lrouter(
ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_GW_REDIRECT, 50,
ds_cstr(match), ds_cstr(actions),
stage_hint);
- }
+ for (int j = 0; j < od->n_nat_entries; j++) {
+ const struct ovn_nat *nat = &od->nat_entries[j];
- for (int i = 0; i < od->n_nat_entries; i++) {
- const struct ovn_nat *nat = &od->nat_entries[i];
+ if (!lrouter_nat_is_stateless(nat->nb) ||
+ strcmp(nat->nb->type, "dnat_and_snat")) {
+ continue;
+ }
- if (!lrouter_nat_is_stateless(nat->nb) ||
- strcmp(nat->nb->type, "dnat_and_snat")) {
- continue;
- }
+ if (nat->nb->allowed_ext_ips || nat->nb->exempted_ext_ips) {
+ struct nbrec_address_set *as = nat->nb->allowed_ext_ips
+ ? nat->nb->allowed_ext_ips : nat->nb->exempted_ext_ips;
+ struct ds match_ext = DS_EMPTY_INITIALIZER;
- ds_clear(match);
- ds_put_format(match, "ip && ip%s.dst == %s",
- nat_entry_is_v6(nat) ? "6" : "4",
- nat->nb->external_ip);
- ovn_lflow_add(lflows, od, S_ROUTER_IN_GW_REDIRECT, 100,
- ds_cstr(match), "drop;");
+ ds_put_format(&match_ext, "%s && ip%s.src == $%s",
+ ds_cstr(match), nat_entry_is_v6(nat) ? "6" : "4",
+ as->name);
+ ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_GW_REDIRECT,
+ 75, ds_cstr(&match_ext),
+ ds_cstr(actions), stage_hint);
+
+ if (add_def_flow) {
+ ds_clear(&match_ext);
+ ds_put_format(&match_ext, "ip && ip%s.dst == %s",
+ nat_entry_is_v6(nat) ? "6" : "4",
+ nat->nb->external_ip);
+ ovn_lflow_add(lflows, od, S_ROUTER_IN_GW_REDIRECT, 70,
+ ds_cstr(&match_ext), "drop;");
+ add_def_flow = false;
+ }
+ ds_destroy(&match_ext);
+ }
+ }
}
/* Packets are allowed by default. */
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 1f7022490..ad1a66cd4 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -3018,8 +3018,7 @@ icmp6 {
<code>ip && ip6.dst == <var>B</var></code>
with an action <code>ct_snat; </code>. If the NAT rule is of type
dnat_and_snat and has <code>stateless=true</code> in the
- options, then the action would be <code>ip4/6.dst=
- (<var>B</var>)</code>.
+ options, then the action would be <code>next;</code>.
</p>
<p>
@@ -3059,7 +3058,7 @@ icmp6 {
action <code>ct_snat_in_czone;</code> to unSNAT in the common
zone. If the NAT rule is of type dnat_and_snat and has
<code>stateless=true</code> in the options, then the action
- would be <code>ip4/6.dst=(<var>B</var>)</code>.
+ would be <code>next;</code>.
</p>
<p>
@@ -4235,6 +4234,18 @@ icmp6 {
gateway chassis.
</li>
+ <li>
+ For each <code>dnat_and_snat</code> NAT rule with
+ <code>stateless=true</code> and <code>allowed_ext_ips</code>
+ or <code>exempted_ext_ips</code> configured, a similar priority-75
+ flow is programmed with an additional match
+ <code>ip4.src == <var>allowed_ext_ips</var></code> or
+ <code>ip4.src == <var>exempted_ext_ips</var></code>.
+ Moreover a priority-70 drop flow is added with match
+ <code>ip4.dst == <var>B</var></code> where <var>B</var> is the NAT
+ rule external IP. A specular flow is added for IPv6 traffic.
+ </li>
+
<li>
A priority-0 logical flow with match <code>1</code> has actions
<code>next;</code>.
@@ -4415,8 +4426,7 @@ nd_ns {
is the logical router gateway port, with an action
<code>ct_dnat_in_czone;</code>. If the NAT rule is of type
dnat_and_snat and has <code>stateless=true</code> in the
- options, then the action would be <code>ip4/6.src=
- (<var>B</var>)</code>.
+ options, then the action would be <code>next;</code>.
</p>
<p>
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 4bf22593a..df2da3408 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -6741,6 +6741,21 @@ NS_CHECK_EXEC([vm1], [ping -q -c 3 -i 0.3 -w 2
172.18.2.10 | FORMAT_PING], \
[0], [dnl
3 packets transmitted, 3 received, 0% packet loss, time 0ms
])
+
+dnat_and_snat_uuid=$(fetch_column nb:NAT _uuid external_ip=172.18.2.10)
+ovn-nbctl set NAT $dnat_and_snat_uuid options:stateless=true
+
+# A ping from vm1 should hairpin in lr1 and successfully DNAT to vm2
+NS_CHECK_EXEC([vm1], [ping -q -c 3 -i 0.3 -w 2 172.18.2.10 | FORMAT_PING], \
+[0], [dnl
+3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+# A ping from vm2 should hairpin in lr1 and successfully DNAT to vm2
+NS_CHECK_EXEC([vm2], [ping -q -c 3 -i 0.3 -w 2 172.18.2.10 | FORMAT_PING], \
+[0], [dnl
+3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+
kill $(pidof ovn-controller)
as ovn-sb
--
2.35.3
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
