From patchwork Wed Oct 16 17:00:18 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1178048 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=ovn.org Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 46tdpW6yBhz9sPF for ; Thu, 17 Oct 2019 04:00:35 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 25756E2B; Wed, 16 Oct 2019 17:00:33 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 6D3A7E23 for ; Wed, 16 Oct 2019 17:00:31 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay10.mail.gandi.net (relay10.mail.gandi.net [217.70.178.230]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id C6745821 for ; Wed, 16 Oct 2019 17:00:29 +0000 (UTC) Received: from nummac.local (unknown [115.99.168.14]) (Authenticated sender: numans@ovn.org) by relay10.mail.gandi.net (Postfix) with ESMTPSA id BD8CD240008; Wed, 16 Oct 2019 17:00:26 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Wed, 16 Oct 2019 22:30:18 +0530 Message-Id: <20191016170018.12596-1-numans@ovn.org> X-Mailer: git-send-email 2.21.0 MIME-Version: 1.0 X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: [ovs-dev] [PATCH ovn] Fix virtual port binding when the parents are scheduled in the same chassis X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org From: Numan Siddique If a virtual port has 2 parents and if both of them are scheduled on the same chassis, then virtual port binding doesn't work. For virtual port binding we have the below logical flows: inport == p1 && !is_chassis_resident("vip-port") && arp .. actions=bind_vport(vip-port); next; inport == p2 && !is_chassis_resident("vip-port") && arp .. actions=bind_vport(vip-port); next; Since we have !is_chassis_resident, as soon as p1 binds the port, the corresponding OF flows for both the logical flows above are deleted. Because of this, when p2 becomes the parent of vip-port it can't bind it. This patch fixes this issue by removing this condition. Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1762341 Signed-off-by: Numan Siddique Acked-by: Dumitru Ceara Acked-by: Lorenzo Bianconi --- northd/ovn-northd.8.xml | 2 +- northd/ovn-northd.c | 3 +-- tests/ovn.at | 39 ++++++++++++++++++++++++++++++++------- 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml index b5dfcd183..d3e0e5ef2 100644 --- a/northd/ovn-northd.8.xml +++ b/northd/ovn-northd.8.xml @@ -580,7 +580,7 @@ column with the match 

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

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index d0844dd64..ea8ad7c2d 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -5250,11 +5250,10 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap *ports, ds_clear(&match); ds_put_format(&match, "inport == \"%s\" && " - "!is_chassis_resident(%s) && " "((arp.op == 1 && arp.spa == %s && " "arp.tpa == %s) || (arp.op == 2 && " "arp.spa == %s))", - vparent, op->json_key, virtual_ip, virtual_ip, + vparent, virtual_ip, virtual_ip, virtual_ip); ds_clear(&actions); ds_put_format(&actions, diff --git a/tests/ovn.at b/tests/ovn.at index d14136748..6bdb9e8ed 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -15637,7 +15637,7 @@ ovn-nbctl lsp-set-addresses sw0-vir "50:54:00:00:00:10 10.0.0.10" ovn-nbctl lsp-set-port-security sw0-vir "50:54:00:00:00:10 10.0.0.10" ovn-nbctl lsp-set-type sw0-vir virtual ovn-nbctl set logical_switch_port sw0-vir options:virtual-ip=10.0.0.10 -ovn-nbctl set logical_switch_port sw0-vir options:virtual-parents=sw0-p1,sw0-p2 +ovn-nbctl set logical_switch_port sw0-vir options:virtual-parents=sw0-p1,sw0-p2,sw0-p3 ovn-nbctl lsp-add sw0 sw0-p1 ovn-nbctl lsp-set-addresses sw0-p1 "50:54:00:00:00:03 10.0.0.3" @@ -15649,7 +15649,7 @@ ovn-nbctl lsp-set-port-security sw0-p2 "50:54:00:00:00:04 10.0.0.4 10.0.0.10" ovn-nbctl lsp-add sw0 sw0-p3 ovn-nbctl lsp-set-addresses sw0-p3 "50:54:00:00:00:05 10.0.0.5" -ovn-nbctl lsp-set-port-security sw0-p3 "50:54:00:00:00:05 10.0.0.5" +ovn-nbctl lsp-set-port-security sw0-p3 "50:54:00:00:00:05 10.0.0.5 10.0.0.10" # Create the second logical switch with one port ovn-nbctl ls-add sw1 @@ -15680,8 +15680,9 @@ ovn-nbctl --wait=hv sync ovn-sbctl dump-flows sw0 | grep ls_in_arp_rsp | grep bind_vport > lflows.txt AT_CHECK([cat lflows.txt], [0], [dnl - 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;) - 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;) + 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;) + 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;) + 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;) ]) ovn-sbctl dump-flows lr0 | grep lr_in_arp_resolve | grep "reg0 == 10.0.0.10" \ @@ -15729,6 +15730,29 @@ AT_CHECK([cat lflows.txt], [0], [dnl 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;) ]) +# From sw0-p3 send GARP for 10.0.0.10. hv1 should claim sw0-vir +# and sw0-p3 should be its virtual_parent. +eth_src=505400000005 +eth_dst=ffffffffffff +spa=$(ip_to_hex 10 0 0 10) +tpa=$(ip_to_hex 10 0 0 10) +send_garp 1 2 $eth_src $eth_dst $spa $tpa + +OVS_WAIT_UNTIL([test x$(ovn-sbctl --bare --columns chassis find port_binding \ +logical_port=sw0-vir) = x$hv1_ch_uuid], [0], []) + +OVS_WAIT_UNTIL([test x$(ovn-sbctl --bare --columns virtual_parent find port_binding \ +logical_port=sw0-vir) = xsw0-p3]) + +ovn-sbctl dump-flows lr0 | grep lr_in_arp_resolve | grep "reg0 == 10.0.0.10" \ +> lflows.txt + +# There should be an arp resolve flow to resolve the virtual_ip with the +# sw0-p2's MAC. +AT_CHECK([cat lflows.txt], [0], [dnl + 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;) +]) + # send the garp from sw0-p2 (in hv2). hv2 should claim sw0-vir # and sw0-p2 shpuld be its virtual_parent. eth_src=505400000004 @@ -15747,7 +15771,7 @@ ovn-sbctl dump-flows lr0 | grep lr_in_arp_resolve | grep "reg0 == 10.0.0.10" \ > lflows.txt # There should be an arp resolve flow to resolve the virtual_ip with the -# sw0-p2's MAC. +# sw0-p3's MAC. AT_CHECK([cat lflows.txt], [0], [dnl 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;) ]) @@ -15792,7 +15816,7 @@ AT_CHECK([cat lflows.txt], [0], [dnl ]) # Now send arp reply from sw0-p2. hv2 should claim sw0-vir -# and sw0-p2 shpuld be its virtual_parent. +# and sw0-p2 should be its virtual_parent. eth_src=505400000004 eth_dst=ffffffffffff spa=$(ip_to_hex 10 0 0 10) @@ -15835,7 +15859,8 @@ ovn-nbctl --wait=hv set logical_switch_port sw0-vir options:virtual-ip=10.0.0.10 ovn-sbctl dump-flows sw0 | grep ls_in_arp_rsp | grep bind_vport > lflows.txt AT_CHECK([cat lflows.txt], [0], [dnl - 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;) + 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;) + 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;) ]) ovn-nbctl --wait=hv remove logical_switch_port sw0-vir options virtual-parents