If the router has a snat rule and it's external ip isn't lrp address,
when the arp request from other router for this external ip, will
be drop, because of this external ip use same mac address as lrp, so
can not forward to MC_FLOOD.

Fixes: 32f5ebb06226 ("ovn-northd: Limit ARP/ND broadcast domain whenever 
possible.")
Reported-at: https://github.com/ovn-org/ovn/issues/209

Signed-off-by: Daniel Ding <danieldin...@gmail.com>
Acked-by: Dumitru Ceara <dce...@redhat.com>
---
 northd/northd.c     | 18 +++++++++++++++++-
 tests/ovn-northd.at | 12 ++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/northd/northd.c b/northd/northd.c
index e9cb906e2..99fb30f46 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -8974,6 +8974,9 @@ build_lswitch_rport_arp_req_flows(struct ovn_port *op,
         }
     }
 
+    struct sset snat_ips_v4 = SSET_INITIALIZER(&snat_ips_v4);
+    struct sset snat_ips_v6 = SSET_INITIALIZER(&snat_ips_v6);
+
     for (size_t i = 0; i < op->od->nbr->n_nat; i++) {
         struct ovn_nat *nat_entry = &op->od->nat_entries[i];
         const struct nbrec_nat *nat = nat_entry->nb;
@@ -8983,7 +8986,17 @@ build_lswitch_rport_arp_req_flows(struct ovn_port *op,
         }
 
         if (!strcmp(nat->type, "snat")) {
-            continue;
+            if (nat_entry_is_v6(nat_entry)) {
+                if (sset_contains(&snat_ips_v6, nat->external_ip)) {
+                    continue;
+                }
+                sset_add(&snat_ips_v6, nat->external_ip);
+            } else {
+                if (sset_contains(&snat_ips_v4, nat->external_ip)) {
+                    continue;
+                }
+                sset_add(&snat_ips_v4, nat->external_ip);
+            }
         }
 
         /* Check if the ovn port has a network configured on which we could
@@ -9025,6 +9038,9 @@ build_lswitch_rport_arp_req_flows(struct ovn_port *op,
     if (sw_od->n_router_ports != sw_od->nbs->n_ports) {
         build_lswitch_rport_arp_req_self_orig_flow(op, 75, sw_od, lflows);
     }
+
+    sset_destroy(&snat_ips_v4);
+    sset_destroy(&snat_ips_v6);
 }
 
 static void
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 5c9da811f..953e0d829 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -5084,6 +5084,7 @@ AT_CHECK([grep "ls_in_l2_lkup" ls1_lflows | sed 
's/table=../table=??/' | sort],
   table=??(ls_in_l2_lkup      ), priority=70   , match=(eth.mcast), 
action=(outport = "_MC_flood"; output;)
   table=??(ls_in_l2_lkup      ), priority=75   , match=(eth.src == 
{00:00:00:00:01:01} && (arp.op == 1 || rarp.op == 3 || nd_ns)), action=(outport 
= "_MC_flood_l2"; output;)
   table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 10.0.0.100), action=(clone {outport = "ls1-ro1"; 
output; }; outport = "_MC_flood_l2"; output;)
+  table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 10.0.0.200), action=(clone {outport = "ls1-ro1"; 
output; }; outport = "_MC_flood_l2"; output;)
   table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 192.168.1.1), action=(clone {outport = "ls1-ro1"; 
output; }; outport = "_MC_flood_l2"; output;)
   table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
nd_ns && nd.target == fe80::200:ff:fe00:101), action=(clone {outport = 
"ls1-ro1"; output; }; outport = "_MC_flood_l2"; output;)
 ])
@@ -5098,6 +5099,7 @@ AT_CHECK([grep "ls_in_l2_lkup" ls2_lflows | sed 
's/table=../table=??/' | sort],
   table=??(ls_in_l2_lkup      ), priority=75   , match=(eth.src == 
{00:00:00:00:02:01} && (arp.op == 1 || rarp.op == 3 || nd_ns)), action=(outport 
= "_MC_flood_l2"; output;)
   table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 192.168.2.1), action=(clone {outport = "ls2-ro2"; 
output; }; outport = "_MC_flood_l2"; output;)
   table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 20.0.0.100), action=(clone {outport = "ls2-ro2"; 
output; }; outport = "_MC_flood_l2"; output;)
+  table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 20.0.0.200), action=(clone {outport = "ls2-ro2"; 
output; }; outport = "_MC_flood_l2"; output;)
   table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
nd_ns && nd.target == fe80::200:ff:fe00:201), action=(clone {outport = 
"ls2-ro2"; output; }; outport = "_MC_flood_l2"; output;)
 ])
 
@@ -5118,8 +5120,10 @@ AT_CHECK([grep "ls_in_l2_lkup" ls1_lflows | sed 
's/table=../table=??/' | sort],
   table=??(ls_in_l2_lkup      ), priority=70   , match=(eth.mcast), 
action=(outport = "_MC_flood"; output;)
   table=??(ls_in_l2_lkup      ), priority=75   , match=(eth.src == 
{00:00:00:00:01:01} && (arp.op == 1 || rarp.op == 3 || nd_ns)), action=(outport 
= "_MC_flood_l2"; output;)
   table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 10.0.0.100), action=(clone {outport = "ls1-ro1"; 
output; }; outport = "_MC_flood_l2"; output;)
+  table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 10.0.0.200), action=(clone {outport = "ls1-ro1"; 
output; }; outport = "_MC_flood_l2"; output;)
   table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 192.168.1.1), action=(clone {outport = "ls1-ro1"; 
output; }; outport = "_MC_flood_l2"; output;)
   table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 30.0.0.100), action=(clone {outport = "ls1-ro1"; 
output; }; outport = "_MC_flood_l2"; output;)
+  table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 30.0.0.200), action=(clone {outport = "ls1-ro1"; 
output; }; outport = "_MC_flood_l2"; output;)
   table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
nd_ns && nd.target == fe80::200:ff:fe00:101), action=(clone {outport = 
"ls1-ro1"; output; }; outport = "_MC_flood_l2"; output;)
 ])
 
@@ -5133,7 +5137,9 @@ AT_CHECK([grep "ls_in_l2_lkup" ls2_lflows | sed 
's/table=../table=??/' | sort],
   table=??(ls_in_l2_lkup      ), priority=75   , match=(eth.src == 
{00:00:00:00:02:01} && (arp.op == 1 || rarp.op == 3 || nd_ns)), action=(outport 
= "_MC_flood_l2"; output;)
   table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 192.168.2.1), action=(clone {outport = "ls2-ro2"; 
output; }; outport = "_MC_flood_l2"; output;)
   table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 20.0.0.100), action=(clone {outport = "ls2-ro2"; 
output; }; outport = "_MC_flood_l2"; output;)
+  table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 20.0.0.200), action=(clone {outport = "ls2-ro2"; 
output; }; outport = "_MC_flood_l2"; output;)
   table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 40.0.0.100), action=(clone {outport = "ls2-ro2"; 
output; }; outport = "_MC_flood_l2"; output;)
+  table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 40.0.0.200), action=(clone {outport = "ls2-ro2"; 
output; }; outport = "_MC_flood_l2"; output;)
   table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
nd_ns && nd.target == fe80::200:ff:fe00:201), action=(clone {outport = 
"ls2-ro2"; output; }; outport = "_MC_flood_l2"; output;)
 ])
 
@@ -5151,9 +5157,11 @@ AT_CHECK([grep "ls_in_l2_lkup" ls1_lflows | sed 
's/table=../table=??/' | sort],
   table=??(ls_in_l2_lkup      ), priority=70   , match=(eth.mcast), 
action=(outport = "_MC_flood"; output;)
   table=??(ls_in_l2_lkup      ), priority=75   , match=(eth.src == 
{00:00:00:00:01:01} && (arp.op == 1 || rarp.op == 3 || nd_ns)), action=(outport 
= "_MC_flood_l2"; output;)
   table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 10.0.0.100), action=(clone {outport = "ls1-ro1"; 
output; }; outport = "_MC_flood_l2"; output;)
+  table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 10.0.0.200), action=(clone {outport = "ls1-ro1"; 
output; }; outport = "_MC_flood_l2"; output;)
   table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 192.168.1.1), action=(clone {outport = "ls1-ro1"; 
output; }; outport = "_MC_flood_l2"; output;)
   table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 192.168.1.100), action=(clone {outport = "ls1-ro1"; 
output; }; outport = "_MC_flood_l2"; output;)
   table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 30.0.0.100), action=(clone {outport = "ls1-ro1"; 
output; }; outport = "_MC_flood_l2"; output;)
+  table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 30.0.0.200), action=(clone {outport = "ls1-ro1"; 
output; }; outport = "_MC_flood_l2"; output;)
   table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
nd_ns && nd.target == fe80::200:ff:fe00:101), action=(clone {outport = 
"ls1-ro1"; output; }; outport = "_MC_flood_l2"; output;)
 ])
 
@@ -5169,9 +5177,11 @@ AT_CHECK([grep "ls_in_l2_lkup" ls1_lflows | sed 
's/table=../table=??/' | sort],
   table=??(ls_in_l2_lkup      ), priority=70   , match=(eth.mcast), 
action=(outport = "_MC_flood"; output;)
   table=??(ls_in_l2_lkup      ), priority=75   , match=(eth.src == 
{00:00:00:00:01:01} && (arp.op == 1 || rarp.op == 3 || nd_ns)), action=(outport 
= "_MC_flood_l2"; output;)
   table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 10.0.0.100), action=(clone {outport = "ls1-ro1"; 
output; }; outport = "_MC_flood_l2"; output;)
+  table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 10.0.0.200), action=(clone {outport = "ls1-ro1"; 
output; }; outport = "_MC_flood_l2"; output;)
   table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 192.168.1.1), action=(clone {outport = "ls1-ro1"; 
output; }; outport = "_MC_flood_l2"; output;)
   table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 192.168.1.100), action=(clone {outport = "ls1-ro1"; 
output; }; outport = "_MC_flood_l2"; output;)
   table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 30.0.0.100), action=(clone {outport = "ls1-ro1"; 
output; }; outport = "_MC_flood_l2"; output;)
+  table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 30.0.0.200), action=(clone {outport = "ls1-ro1"; 
output; }; outport = "_MC_flood_l2"; output;)
   table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
nd_ns && nd.target == fe80::200:ff:fe00:101), action=(clone {outport = 
"ls1-ro1"; output; }; outport = "_MC_flood_l2"; output;)
 ])
 
@@ -5193,9 +5203,11 @@ AT_CHECK([grep "ls_in_l2_lkup" ls1_lflows | sed 
's/table=../table=??/' | sort],
   table=??(ls_in_l2_lkup      ), priority=70   , match=(eth.mcast), 
action=(outport = "_MC_flood"; output;)
   table=??(ls_in_l2_lkup      ), priority=75   , match=(eth.src == 
{00:00:00:00:01:01} && (arp.op == 1 || rarp.op == 3 || nd_ns)), action=(outport 
= "_MC_flood_l2"; output;)
   table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 10.0.0.100), action=(clone {outport = "ls1-ro1"; 
output; }; outport = "_MC_flood_l2"; output;)
+  table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 10.0.0.200), action=(clone {outport = "ls1-ro1"; 
output; }; outport = "_MC_flood_l2"; output;)
   table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 192.168.1.1), action=(clone {outport = "ls1-ro1"; 
output; }; outport = "_MC_flood_l2"; output;)
   table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 192.168.1.100), action=(clone {outport = "ls1-ro1"; 
output; }; outport = "_MC_flood_l2"; output;)
   table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 30.0.0.100), action=(clone {outport = "ls1-ro1"; 
output; }; outport = "_MC_flood_l2"; output;)
+  table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
arp.op == 1 && arp.tpa == 30.0.0.200), action=(clone {outport = "ls1-ro1"; 
output; }; outport = "_MC_flood_l2"; output;)
   table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && 
nd_ns && nd.target == fe80::200:ff:fe00:101), action=(clone {outport = 
"ls1-ro1"; output; }; outport = "_MC_flood_l2"; output;)
 ])
 
-- 
2.42.0

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to