northd: fix stateless nat with allowed_ext_ips

When a nat rule is configured in stateless mode there is a 1:1 mapping between external_ip and logical_ip. Do not modify dst/src ips in S_ROUTER_IN_UNSNAT/S_ROUTER_OUT_UNDNAT stages for stateless nat entries since they will be properly modified in S_ROUTER_IN_DNAT/S_ROUTER_OUT_SNAT stages. This path will allow respecting allowed_ext_ips for stateless nat rules. Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2066990 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 · May 19, 2022 · fc2cdb4 · fc2cdb4
1 parent 7d6d67c
commit fc2cdb4
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 10 deletions.
diff --git a/northd/northd.c b/northd/northd.c
@@ -11934,6 +11934,23 @@ build_gateway_redirect_flows_for_lrouter(
                                 ds_cstr(match), ds_cstr(actions),
                                 stage_hint);
     }
+
+    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;
+        }
+
+        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;");
+    }
+
     /* Packets are allowed by default. */
     ovn_lflow_add(lflows, od, S_ROUTER_IN_GW_REDIRECT, 0, "1", "next;");
 }
@@ -12672,8 +12689,7 @@ build_lrouter_in_unsnat_flow(struct hmap *lflows, struct ovn_datapath *od,
         ds_put_format(match, "ip && ip%s.dst == %s",
                       is_v6 ? "6" : "4", nat->external_ip);
         if (!strcmp(nat->type, "dnat_and_snat") && stateless) {
-            ds_put_format(actions, "ip%s.dst=%s; next;",
-                          is_v6 ? "6" : "4", nat->logical_ip);
+            ds_put_format(actions, "next;");
         } else {
             ds_put_cstr(actions, "ct_snat;");
         }
@@ -12698,8 +12714,7 @@ build_lrouter_in_unsnat_flow(struct hmap *lflows, struct ovn_datapath *od,
         }
 
         if (!strcmp(nat->type, "dnat_and_snat") && stateless) {
-            ds_put_format(actions, "ip%s.dst=%s; next;",
-                          is_v6 ? "6" : "4", nat->logical_ip);
+            ds_put_format(actions, "next;");
         } else {
             ds_put_cstr(actions, "ct_snat_in_czone;");
         }
@@ -12852,8 +12867,7 @@ build_lrouter_out_undnat_flow(struct hmap *lflows, struct ovn_datapath *od,
 
     if (!strcmp(nat->type, "dnat_and_snat") &&
         lrouter_nat_is_stateless(nat)) {
-        ds_put_format(actions, "ip%s.src=%s; next;",
-                      is_v6 ? "6" : "4", nat->external_ip);
+        ds_put_format(actions, "next;");
     } else {
         ds_put_format(actions,
                       od->is_gw_router ? "ct_dnat;" : "ct_dnat_in_czone;");

diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
@@ -895,7 +895,7 @@ ovn-nbctl lr-nat-del R1 dnat_and_snat  172.16.1.1
 echo
 echo "IPv4: stateless"
 ovn-nbctl --wait=sb --stateless lr-nat-add R1 dnat_and_snat  172.16.1.1 50.0.0.11
-check_flow_match_sets 2 0 0 2 2 0 0
+check_flow_match_sets 2 0 0 1 1 0 0
 ovn-nbctl lr-nat-del R1 dnat_and_snat  172.16.1.1
 
 echo
@@ -907,7 +907,7 @@ ovn-nbctl lr-nat-del R1 dnat_and_snat  fd01::1
 echo
 echo "IPv6: stateless"
 ovn-nbctl --wait=sb --stateless lr-nat-add R1 dnat_and_snat fd01::1 fd11::2
-check_flow_match_sets 2 0 0 0 0 2 2
+check_flow_match_sets 2 0 0 0 0 1 1
 
 AT_CLEANUP
 ])

diff --git a/tests/ovn.at b/tests/ovn.at
@@ -21917,8 +21917,8 @@ AT_CHECK([for regex in ct_snat ct_dnat ip4.dst= ip4.src=; do
   grep -c "$regex" sbflows;
 done], [0], [0
 0
-2
-2
+1
+1
 ])
 
 echo "----------- Post Traffic hv1 dump -----------"