northd: add condition for stateless nat drop flow in S_ROUTER_IN_GW_R…

…EDIRECT pipeline 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 hairpin scenario with stateless nat. Fix the northd documentation. Fixes: c0224a1 ("northd: fix stateless nat with allowed_ext_ips") Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2094980 Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> Acked-by: Mark Michelson <mmichels@redhat.com> Signed-off-by: Mark Michelson <mmichels@redhat.com>
ovn-org · Jun 29, 2022 · 187ae17 · 187ae17
1 parent c81559a
commit 187ae17
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 18 deletions.
diff --git a/northd/northd.c b/northd/northd.c
@@ -12028,6 +12028,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_;
@@ -12046,22 +12047,42 @@ 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") ||
+                (!nat->nb->allowed_ext_ips && !nat->nb->exempted_ext_ips)) {
+                continue;
+            }
 
-        if (!lrouter_nat_is_stateless(nat->nb) ||
-            strcmp(nat->nb->type, "dnat_and_snat")) {
-           continue;
-        }
+            struct ds match_ext = DS_EMPTY_INITIALIZER;
+            struct nbrec_address_set  *as = nat->nb->allowed_ext_ips
+                ? nat->nb->allowed_ext_ips : nat->nb->exempted_ext_ips;
+            ds_put_format(&match_ext, "%s && ip%s.src == $%s",
+                          ds_cstr(match), nat_entry_is_v6(nat) ? "6" : "4",
+                          as->name);
 
-        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;");
+            if (nat->nb->allowed_ext_ips) {
+                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;
+                }
+            } else if (nat->nb->exempted_ext_ips) {
+                ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_GW_REDIRECT,
+                                        75, ds_cstr(&match_ext), "drop;",
+                                        stage_hint);
+            }
+            ds_destroy(&match_ext);
+        }
     }
 
     /* Packets are allowed by default. */

diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
@@ -2987,8 +2987,7 @@ icmp6 {
           <code>ip &amp;&amp; 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>
@@ -3027,7 +3026,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>
@@ -4161,6 +4160,26 @@ icmp6 {
         external ip and <var>D</var> is NAT external mac.
       </li>
 
+      <li>
+        For each <code>dnat_and_snat</code> NAT rule with
+        <code>stateless=true</code> and <code>allowed_ext_ips</code>
+        configured, a priority-75 flow is programmed with match
+        <code>ip4.dst == <var>B</var></code> and action
+        <code>outport = <var>CR</var>; next;</code> where <var>B</var>
+        is the NAT rule external IP and <var>CR</var> is the
+        <code>chassisredirect</code> port representing the instance
+        of the logical router distributed gateway port on the
+        gateway chassis. Moreover a priority-70 flow is programmed
+        with same match and action <code>drop;</code>.
+        For each <code>dnat_and_snat</code> NAT rule with
+        <code>stateless=true</code> and <code>exempted_ext_ips</code>
+        configured, a priority-75 flow is programmed with match
+        <code>ip4.dst == <var>B</var></code> and action
+        <code>drop;</code> where <var>B</var> is the NAT rule
+        external IP.
+        A similar flow is added for IPv6 traffic.
+      </li>
+
       <li>
         For each NAT rule in the OVN Northbound database that can
         be handled in a distributed manner, a priority-80 logical flow
@@ -4359,8 +4378,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
@@ -6730,6 +6730,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