bbaaef
From d15bb6f025e14fc38415124b0794de40c40c69a2 Mon Sep 17 00:00:00 2001
bbaaef
From: Numan Siddique <numans@ovn.org>
bbaaef
Date: Wed, 16 Oct 2019 22:14:45 +0530
bbaaef
Subject: [PATCH] Fix virtual port binding when the parents are scheduled in
bbaaef
 the same chassis
bbaaef
bbaaef
If a virtual port has 2 parents and if both of them are scheduled
bbaaef
on the same chassis, then virtual port binding doesn't work.
bbaaef
bbaaef
For virtual port binding we have the below logical flows:
bbaaef
inport == p1 && !is_chassis_resident("vip-port") && arp .. actions=bind_vport(vip-port); next;
bbaaef
inport == p2 && !is_chassis_resident("vip-port") && arp .. actions=bind_vport(vip-port); next;
bbaaef
bbaaef
Since we have !is_chassis_resident, as soon as p1 binds the port, the corresponding OF flows
bbaaef
for both the logical flows above are deleted. Because of this, when p2 becomes the
bbaaef
parent of vip-port it can't bind it.
bbaaef
bbaaef
This patch fixes this issue by removing this condition.
bbaaef
bbaaef
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1762341
bbaaef
Acked-by: Dumitru Ceara <dceara@redhat.com>
bbaaef
Signed-off-by: Numan Siddique <numans@ovn.org>
bbaaef
---
bbaaef
 ovn/northd/ovn-northd.8.xml |  2 +-
bbaaef
 ovn/northd/ovn-northd.c     |  3 +--
bbaaef
 tests/ovn.at                | 39 ++++++++++++++++++++++++++++++-------
bbaaef
 3 files changed, 34 insertions(+), 10 deletions(-)
bbaaef
bbaaef
diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
bbaaef
index 38e6ecec5..ad84a5783 100644
bbaaef
--- a/ovn/northd/ovn-northd.8.xml
bbaaef
+++ b/ovn/northd/ovn-northd.8.xml
bbaaef
@@ -580,7 +580,7 @@
bbaaef
           column with the match
bbaaef
         

bbaaef
         
bbaaef
-inport == P && !is_chassis_resident(V) && ((arp.op == 1 && arp.spa == VIP && arp.tpa == VIP) || (arp.op == 2 && arp.spa == VIP))
bbaaef
+inport == P && && ((arp.op == 1 && arp.spa == VIP && arp.tpa == VIP) || (arp.op == 2 && arp.spa == VIP))
bbaaef
         
bbaaef
 
bbaaef
         

bbaaef
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
bbaaef
index 84b2a9ff1..c506f22f4 100644
bbaaef
--- a/ovn/northd/ovn-northd.c
bbaaef
+++ b/ovn/northd/ovn-northd.c
bbaaef
@@ -5255,11 +5255,10 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,
bbaaef
 
bbaaef
                 ds_clear(&match);
bbaaef
                 ds_put_format(&match, "inport == \"%s\" && "
bbaaef
-                              "!is_chassis_resident(%s) && "
bbaaef
                               "((arp.op == 1 && arp.spa == %s && "
bbaaef
                               "arp.tpa == %s) || (arp.op == 2 && "
bbaaef
                               "arp.spa == %s))",
bbaaef
-                              vparent, op->json_key, virtual_ip, virtual_ip,
bbaaef
+                              vparent, virtual_ip, virtual_ip,
bbaaef
                               virtual_ip);
bbaaef
                 ds_clear(&actions);
bbaaef
                 ds_put_format(&actions,
bbaaef
diff --git a/tests/ovn.at b/tests/ovn.at
bbaaef
index e38d14cdf..dad6c2d39 100644
bbaaef
--- a/tests/ovn.at
bbaaef
+++ b/tests/ovn.at
bbaaef
@@ -14503,7 +14503,7 @@ ovn-nbctl lsp-set-addresses sw0-vir "50:54:00:00:00:10 10.0.0.10"
bbaaef
 ovn-nbctl lsp-set-port-security sw0-vir "50:54:00:00:00:10 10.0.0.10"
bbaaef
 ovn-nbctl lsp-set-type sw0-vir virtual
bbaaef
 ovn-nbctl set logical_switch_port sw0-vir options:virtual-ip=10.0.0.10
bbaaef
-ovn-nbctl set logical_switch_port sw0-vir options:virtual-parents=sw0-p1,sw0-p2
bbaaef
+ovn-nbctl set logical_switch_port sw0-vir options:virtual-parents=sw0-p1,sw0-p2,sw0-p3
bbaaef
 
bbaaef
 ovn-nbctl lsp-add sw0 sw0-p1
bbaaef
 ovn-nbctl lsp-set-addresses sw0-p1 "50:54:00:00:00:03 10.0.0.3"
bbaaef
@@ -14515,7 +14515,7 @@ ovn-nbctl lsp-set-port-security sw0-p2 "50:54:00:00:00:04 10.0.0.4 10.0.0.10"
bbaaef
 
bbaaef
 ovn-nbctl lsp-add sw0 sw0-p3
bbaaef
 ovn-nbctl lsp-set-addresses sw0-p3 "50:54:00:00:00:05 10.0.0.5"
bbaaef
-ovn-nbctl lsp-set-port-security sw0-p3 "50:54:00:00:00:05 10.0.0.5"
bbaaef
+ovn-nbctl lsp-set-port-security sw0-p3 "50:54:00:00:00:05 10.0.0.5 10.0.0.10"
bbaaef
 
bbaaef
 # Create the second logical switch with one port
bbaaef
 ovn-nbctl ls-add sw1
bbaaef
@@ -14546,8 +14546,9 @@ ovn-nbctl --wait=hv sync
bbaaef
 ovn-sbctl dump-flows sw0 | grep ls_in_arp_rsp | grep bind_vport > lflows.txt
bbaaef
 
bbaaef
 AT_CHECK([cat lflows.txt], [0], [dnl
bbaaef
-  table=11(ls_in_arp_rsp      ), priority=100  , match=(inport == "sw0-p1" && !is_chassis_resident("sw0-vir") && ((arp.op == 1 && arp.spa == 10.0.0.10 && arp.tpa == 10.0.0.10) || (arp.op == 2 && arp.spa == 10.0.0.10))), action=(bind_vport("sw0-vir", inport); next;)
bbaaef
-  table=11(ls_in_arp_rsp      ), priority=100  , match=(inport == "sw0-p2" && !is_chassis_resident("sw0-vir") && ((arp.op == 1 && arp.spa == 10.0.0.10 && arp.tpa == 10.0.0.10) || (arp.op == 2 && arp.spa == 10.0.0.10))), action=(bind_vport("sw0-vir", inport); next;)
bbaaef
+  table=11(ls_in_arp_rsp      ), priority=100  , match=(inport == "sw0-p1" && ((arp.op == 1 && arp.spa == 10.0.0.10 && arp.tpa == 10.0.0.10) || (arp.op == 2 && arp.spa == 10.0.0.10))), action=(bind_vport("sw0-vir", inport); next;)
bbaaef
+  table=11(ls_in_arp_rsp      ), priority=100  , match=(inport == "sw0-p2" && ((arp.op == 1 && arp.spa == 10.0.0.10 && arp.tpa == 10.0.0.10) || (arp.op == 2 && arp.spa == 10.0.0.10))), action=(bind_vport("sw0-vir", inport); next;)
bbaaef
+  table=11(ls_in_arp_rsp      ), priority=100  , match=(inport == "sw0-p3" && ((arp.op == 1 && arp.spa == 10.0.0.10 && arp.tpa == 10.0.0.10) || (arp.op == 2 && arp.spa == 10.0.0.10))), action=(bind_vport("sw0-vir", inport); next;)
bbaaef
 ])
bbaaef
 
bbaaef
 ovn-sbctl dump-flows lr0 | grep lr_in_arp_resolve | grep "reg0 == 10.0.0.10" \
bbaaef
@@ -14595,6 +14596,29 @@ AT_CHECK([cat lflows.txt], [0], [dnl
bbaaef
   table=11(lr_in_arp_resolve  ), priority=100  , match=(outport == "lr0-sw0" && reg0 == 10.0.0.10), action=(eth.dst = 50:54:00:00:00:03; next;)
bbaaef
 ])
bbaaef
 
bbaaef
+# From sw0-p3 send GARP for 10.0.0.10. hv1 should claim sw0-vir
bbaaef
+# and sw0-p3 should be its virtual_parent.
bbaaef
+eth_src=505400000005
bbaaef
+eth_dst=ffffffffffff
bbaaef
+spa=$(ip_to_hex 10 0 0 10)
bbaaef
+tpa=$(ip_to_hex 10 0 0 10)
bbaaef
+send_garp 1 2 $eth_src $eth_dst $spa $tpa
bbaaef
+
bbaaef
+OVS_WAIT_UNTIL([test x$(ovn-sbctl --bare --columns chassis find port_binding \
bbaaef
+logical_port=sw0-vir) = x$hv1_ch_uuid], [0], [])
bbaaef
+
bbaaef
+OVS_WAIT_UNTIL([test x$(ovn-sbctl --bare --columns virtual_parent find port_binding \
bbaaef
+logical_port=sw0-vir) = xsw0-p3])
bbaaef
+
bbaaef
+ovn-sbctl dump-flows lr0 | grep lr_in_arp_resolve | grep "reg0 == 10.0.0.10" \
bbaaef
+> lflows.txt
bbaaef
+
bbaaef
+# There should be an arp resolve flow to resolve the virtual_ip with the
bbaaef
+# sw0-p2's MAC.
bbaaef
+AT_CHECK([cat lflows.txt], [0], [dnl
bbaaef
+  table=11(lr_in_arp_resolve  ), priority=100  , match=(outport == "lr0-sw0" && reg0 == 10.0.0.10), action=(eth.dst = 50:54:00:00:00:05; next;)
bbaaef
+])
bbaaef
+
bbaaef
 # send the garp from sw0-p2 (in hv2). hv2 should claim sw0-vir
bbaaef
 # and sw0-p2 shpuld be its virtual_parent.
bbaaef
 eth_src=505400000004
bbaaef
@@ -14613,7 +14637,7 @@ ovn-sbctl dump-flows lr0 | grep lr_in_arp_resolve | grep "reg0 == 10.0.0.10" \
bbaaef
 > lflows.txt
bbaaef
 
bbaaef
 # There should be an arp resolve flow to resolve the virtual_ip with the
bbaaef
-# sw0-p2's MAC.
bbaaef
+# sw0-p3's MAC.
bbaaef
 AT_CHECK([cat lflows.txt], [0], [dnl
bbaaef
   table=11(lr_in_arp_resolve  ), priority=100  , match=(outport == "lr0-sw0" && reg0 == 10.0.0.10), action=(eth.dst = 50:54:00:00:00:04; next;)
bbaaef
 ])
bbaaef
@@ -14658,7 +14682,7 @@ AT_CHECK([cat lflows.txt], [0], [dnl
bbaaef
 ])
bbaaef
 
bbaaef
 # Now send arp reply from sw0-p2. hv2 should claim sw0-vir
bbaaef
-# and sw0-p2 shpuld be its virtual_parent.
bbaaef
+# and sw0-p2 should be its virtual_parent.
bbaaef
 eth_src=505400000004
bbaaef
 eth_dst=ffffffffffff
bbaaef
 spa=$(ip_to_hex 10 0 0 10)
bbaaef
@@ -14701,7 +14725,8 @@ ovn-nbctl --wait=hv set logical_switch_port sw0-vir options:virtual-ip=10.0.0.10
bbaaef
 ovn-sbctl dump-flows sw0 | grep ls_in_arp_rsp | grep bind_vport > lflows.txt
bbaaef
 
bbaaef
 AT_CHECK([cat lflows.txt], [0], [dnl
bbaaef
-  table=11(ls_in_arp_rsp      ), priority=100  , match=(inport == "sw0-p1" && !is_chassis_resident("sw0-vir") && ((arp.op == 1 && arp.spa == 10.0.0.10 && arp.tpa == 10.0.0.10) || (arp.op == 2 && arp.spa == 10.0.0.10))), action=(bind_vport("sw0-vir", inport); next;)
bbaaef
+  table=11(ls_in_arp_rsp      ), priority=100  , match=(inport == "sw0-p1" && ((arp.op == 1 && arp.spa == 10.0.0.10 && arp.tpa == 10.0.0.10) || (arp.op == 2 && arp.spa == 10.0.0.10))), action=(bind_vport("sw0-vir", inport); next;)
bbaaef
+  table=11(ls_in_arp_rsp      ), priority=100  , match=(inport == "sw0-p3" && ((arp.op == 1 && arp.spa == 10.0.0.10 && arp.tpa == 10.0.0.10) || (arp.op == 2 && arp.spa == 10.0.0.10))), action=(bind_vport("sw0-vir", inport); next;)
bbaaef
 ])
bbaaef
 
bbaaef
 ovn-nbctl --wait=hv remove logical_switch_port sw0-vir options virtual-parents
bbaaef
-- 
bbaaef
2.23.0
bbaaef