From patchwork Wed Jul 15 17:56:15 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1329749 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.166.137; helo=fraxinus.osuosl.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 fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4B6Q7724Ghz9sTX for ; Thu, 16 Jul 2020 03:56:34 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id C7C6D8955D; Wed, 15 Jul 2020 17:56:32 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Q9ZM1Hxk4OEr; Wed, 15 Jul 2020 17:56:31 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id B1FB38937D; Wed, 15 Jul 2020 17:56:31 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 8D19FC07FF; Wed, 15 Jul 2020 17:56:31 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 2D0B4C0733 for ; Wed, 15 Jul 2020 17:56:30 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 0B3968B5F0 for ; Wed, 15 Jul 2020 17:56:30 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id tH65iJa+3vbe for ; Wed, 15 Jul 2020 17:56:28 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net [217.70.183.201]) by whitealder.osuosl.org (Postfix) with ESMTPS id 7B7728B5E6 for ; Wed, 15 Jul 2020 17:56:28 +0000 (UTC) X-Originating-IP: 27.7.7.206 Received: from nusiddiq.home.org.home.org (unknown [27.7.7.206]) (Authenticated sender: numans@ovn.org) by relay8-d.mail.gandi.net (Postfix) with ESMTPSA id 1B8AE1BF205; Wed, 15 Jul 2020 17:56:23 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Wed, 15 Jul 2020 23:26:15 +0530 Message-Id: <20200715175615.481433-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn] ovn-controller: Fix the missing flows with monitor-all set to True X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" From: Numan Siddique When the config ovn-monitor-all is set to True, then ovn-controller is not programming the flows completely when it binds the logical port of a logical switch until a full recompute is triggered. This issue was introduced by the commit in Fixes - which added incremental procesing for runtime data changes. This patch fixes this issue. Fixes: 3d096f833c4a("ovn-controller: Handle runtime data changes in flow output engine") Reported-by: Dumitru Ceara Signed-off-by: Numan Siddique Acked-by: Dumitru Ceara --- controller/binding.c | 17 ++++++-- tests/ovn.at | 101 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 3 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index b2c388146d..8f6d68006e 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -103,9 +103,20 @@ add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key, ld->localnet_port = NULL; ld->has_local_l3gateway = has_local_l3gateway; - if (tracked_datapaths && - !tracked_binding_datapath_find(tracked_datapaths, datapath)) { - tracked_binding_datapath_create(datapath, true, tracked_datapaths); + if (tracked_datapaths) { + struct tracked_binding_datapath *tdp = + tracked_binding_datapath_find(tracked_datapaths, datapath); + if (!tdp) { + tracked_binding_datapath_create(datapath, true, tracked_datapaths); + } else { + /* Its possible that there is already an entry in tracked datapaths + * for this 'datapath'. tracked_binding_datapath_lport_add() may + * have created it. Since the 'datapath' is added to the + * local datapaths, set the tdp->is_new to true so that the flows + * for this datapath are programmed properly. + * */ + tdp->is_new = true; + } } if (depth >= 100) { diff --git a/tests/ovn.at b/tests/ovn.at index 24d93bc245..ba1a534e92 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -20601,3 +20601,104 @@ OVS_WAIT_UNTIL([ OVN_CLEANUP([hv1], [hv2]) AT_CLEANUP + +AT_SETUP([ovn -- controller I-P handling with monitoring disabled]) +AT_KEYWORDS([lb]) +ovn_start + +net_add n1 + +sim_add hv1 +as hv1 +ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 + + +sim_add hv2 +as hv2 +ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.2 + +ovn-nbctl ls-add sw0 + +ovn-nbctl lsp-add sw0 sw0-p1 +ovn-nbctl lsp-set-addresses sw0-p1 "50:54:00:00:00:03 10.0.0.3" +ovn-nbctl lsp-set-port-security sw0-p1 "50:54:00:00:00:03 10.0.0.3" + +ovn-nbctl lsp-add sw0 sw0-p2 +ovn-nbctl lsp-set-addresses sw0-p2 "50:54:00:00:00:04 10.0.0.4" +ovn-nbctl lsp-set-port-security sw0-p2 "50:54:00:00:00:04 10.0.0.4" + +as hv1 +ovs-vsctl -- add-port br-int hv1-vif1 -- \ + set interface hv1-vif1 external-ids:iface-id=sw0-p1 \ + options:tx_pcap=hv1/vif1-tx.pcap \ + options:rxq_pcap=hv1/vif1-rx.pcap \ + ofport-request=1 + +OVS_WAIT_UNTIL([test x$(ovn-nbctl lsp-get-up sw0-p1) = xup]) + +# Get the number of OF flows in hv1 and hv2 +hv1_offlows=$(as hv1 ovs-ofctl dump-flows br-int | wc -l) +echo "hv1 flows : $hv1_offlows" +AT_CHECK([test $hv1_offlows -gt 0]) + +as hv2 +ovs-vsctl -- add-port br-int hv2-vif1 -- \ + set interface hv2-vif1 external-ids:iface-id=sw0-p2 \ + options:tx_pcap=hv2/vif1-tx.pcap \ + options:rxq_pcap=hv2/vif1-rx.pcap \ + ofport-request=1 + +OVS_WAIT_UNTIL([test x$(ovn-nbctl lsp-get-up sw0-p2) = xup]) + +hv2_offlows=$(as hv2 ovs-ofctl dump-flows br-int | wc -l) +echo "hv2 flows : $hv2_offlows" +AT_CHECK([test $hv2_offlows -gt 0]) + +ovn-nbctl ls-del sw0 +as hv1 ovs-vsctl del-port hv1-vif1 +as hv2 ovs-vsctl del-port hv2-vif1 + +as hv1 ovs-vsctl set open . external_ids:ovn-monitor-all=true +as hv2 ovs-vsctl set open . external_ids:ovn-monitor-all=true + +ovn-nbctl ls-add sw0 + +ovn-nbctl lsp-add sw0 sw0-p1 +ovn-nbctl lsp-set-addresses sw0-p1 "50:54:00:00:00:03 10.0.0.3" +ovn-nbctl lsp-set-port-security sw0-p1 "50:54:00:00:00:03 10.0.0.3" + +ovn-nbctl lsp-add sw0 sw0-p2 +ovn-nbctl lsp-set-addresses sw0-p2 "50:54:00:00:00:04 10.0.0.4" +ovn-nbctl lsp-set-port-security sw0-p2 "50:54:00:00:00:04 10.0.0.4" + +as hv1 +ovs-vsctl -- add-port br-int hv1-vif1 -- \ + set interface hv1-vif1 external-ids:iface-id=sw0-p1 \ + options:tx_pcap=hv1/vif1-tx.pcap \ + options:rxq_pcap=hv1/vif1-rx.pcap \ + ofport-request=1 + +OVS_WAIT_UNTIL([test x$(ovn-nbctl lsp-get-up sw0-p1) = xup]) + +# Get the number of OF flows in hv1 and hv2 +hv1_offlows_mon=$(as hv1 ovs-ofctl dump-flows br-int | wc -l) +echo "hv1 flows after monitor-all=true : $hv1_offlows" +AT_CHECK([test "$hv1_offlows" = "$hv1_offlows_mon"]) + +as hv2 +ovs-vsctl -- add-port br-int hv2-vif1 -- \ + set interface hv2-vif1 external-ids:iface-id=sw0-p2 \ + options:tx_pcap=hv2/vif1-tx.pcap \ + options:rxq_pcap=hv2/vif1-rx.pcap \ + ofport-request=1 + +OVS_WAIT_UNTIL([test x$(ovn-nbctl lsp-get-up sw0-p2) = xup]) + +hv2_offlows_mon=$(as hv2 ovs-ofctl dump-flows br-int | wc -l) +echo "hv2 flows after monitor-all=true : $hv2_offlows" +AT_CHECK([test "$hv2_offlows" = "$hv2_offlows_mon"]) + +OVN_CLEANUP([hv1], [hv2]) +AT_CLEANUP