From patchwork Thu Aug 11 12:21:39 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 658195 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from archives.nicira.com (archives.nicira.com [96.126.127.54]) by ozlabs.org (Postfix) with ESMTP id 3s96Zf6XpZz9t0m for ; Thu, 11 Aug 2016 22:21:46 +1000 (AEST) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id 2557210BDC; Thu, 11 Aug 2016 05:21:46 -0700 (PDT) X-Original-To: dev@openvswitch.org Delivered-To: dev@openvswitch.org Received: from mx3v3.cudamail.com (mx3.cudamail.com [64.34.241.5]) by archives.nicira.com (Postfix) with ESMTPS id F298E10BBD for ; Thu, 11 Aug 2016 05:21:44 -0700 (PDT) Received: from bar6.cudamail.com (localhost [127.0.0.1]) by mx3v3.cudamail.com (Postfix) with ESMTPS id 891B01627AD for ; Thu, 11 Aug 2016 06:21:44 -0600 (MDT) X-ASG-Debug-ID: 1470918103-0b323777e31b0580001-byXFYA Received: from mx1-pf2.cudamail.com ([192.168.24.2]) by bar6.cudamail.com with ESMTP id khm9cOnKAQH55nsK (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Thu, 11 Aug 2016 06:21:43 -0600 (MDT) X-Barracuda-Envelope-From: nusiddiq@redhat.com X-Barracuda-RBL-Trusted-Forwarder: 192.168.24.2 Received: from unknown (HELO mx1.redhat.com) (209.132.183.28) by mx1-pf2.cudamail.com with ESMTPS (DHE-RSA-AES256-SHA encrypted); 11 Aug 2016 12:21:42 -0000 Received-SPF: pass (mx1-pf2.cudamail.com: SPF record at _spf1.redhat.com designates 209.132.183.28 as permitted sender) X-Barracuda-Apparent-Source-IP: 209.132.183.28 X-Barracuda-RBL-IP: 209.132.183.28 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C4E7385543 for ; Thu, 11 Aug 2016 12:21:41 +0000 (UTC) Received: from nusiddiq.blr.redhat.com (dhcp-0-74.blr.redhat.com [10.70.1.74]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u7BCLeWM025662 for ; Thu, 11 Aug 2016 08:21:41 -0400 X-CudaMail-Envelope-Sender: nusiddiq@redhat.com From: Numan Siddique X-CudaMail-Whitelist-To: dev@openvswitch.org X-CudaMail-MID: CM-E2-810010718 X-CudaMail-DTE: 081116 X-CudaMail-Originating-IP: 209.132.183.28 Organization: Red Hat X-ASG-Orig-Subj: [##CM-E2-810010718##][PATCH v2] ovn-controller: Reset flow processing after (re)connection to switch To: ovs dev Message-ID: <107f6263-4c17-80fc-a7ba-cc78cc67bd3e@redhat.com> Date: Thu, 11 Aug 2016 17:51:39 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Thu, 11 Aug 2016 12:21:41 +0000 (UTC) X-Barracuda-Connect: UNKNOWN[192.168.24.2] X-Barracuda-Start-Time: 1470918103 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://web.cudamail.com:443/cgi-mod/mark.cgi X-ASG-Whitelist: Header =?UTF-8?B?eFwtY3VkYW1haWxcLXdoaXRlbGlzdFwtdG8=?= X-Virus-Scanned: by bsmtpd at cudamail.com X-Barracuda-BRTS-Status: 1 Subject: [ovs-dev] [PATCH v2] ovn-controller: Reset flow processing after (re)connection to switch X-BeenThere: dev@openvswitch.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@openvswitch.org Sender: "dev" When ovn-controller reconnects to the ovs-vswitchd, it deletes all the OF flows in the switch. It doesn't install the flows again, leaving the datapath broken unless ovn-controller is restarted or ovn-northd updates the SB DB. The reason for this is - lflow_reset_processing() is not called after the reconnection - the hmap "installed_flows" is not cleared, because of which ofctrl_put skips adding the flows to the switch. This patch fixes the issue and also adds a test case to test this scenario. Signed-off-by: Numan Siddique Acked-by: Ryan Moats --- ovn/controller/ofctrl.c | 7 ++++ tests/ovn.at | 95 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+) v1 -> v2 -------- * Enhanced the test case to restart vswitchd and restore the flows diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c index 9388cb8..26f85db 100644 --- a/ovn/controller/ofctrl.c +++ b/ovn/controller/ofctrl.c @@ -20,6 +20,7 @@ #include "flow.h" #include "hash.h" #include "hindex.h" +#include "lflow.h" #include "ofctrl.h" #include "openflow/openflow.h" #include "openvswitch/dynamic-string.h" @@ -368,6 +369,7 @@ run_S_CLEAR_FLOWS(void) /* Clear installed_flows, to match the state of the switch. */ ovn_flow_table_clear(); + lflow_reset_processing(); /* Clear existing groups, to match the state of the switch. */ if (groups) { @@ -812,6 +814,11 @@ ovn_flow_table_clear(void) hindex_remove(&uuid_flow_table, &f->uuid_hindex_node); ovn_flow_destroy(f); } + + HMAP_FOR_EACH_SAFE (f, next, match_hmap_node, &installed_flows) { + hmap_remove(&installed_flows, &f->match_hmap_node); + ovn_flow_destroy(f); + } } static void diff --git a/tests/ovn.at b/tests/ovn.at index a95e0b2..7e3e25d 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -4072,3 +4072,98 @@ AT_CHECK([cat received2.packets], [0], [expout]) OVN_CLEANUP([hv1]) AT_CLEANUP + +AT_SETUP([ovn -- ovs-vswitchd restart]) +AT_KEYWORDS([vswitchd restart]) +AT_SKIP_IF([test $HAVE_PYTHON = no]) +ovn_start + +ovn-nbctl ls-add ls1 + +ovn-nbctl lsp-add ls1 ls1-lp1 \ +-- lsp-set-addresses ls1-lp1 "f0:00:00:00:00:01 10.0.0.4" + +ovn-nbctl lsp-set-port-security ls1-lp1 "f0:00:00:00:00:01 10.0.0.4" + +net_add n1 +sim_add hv1 + +as hv1 +ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 +ovs-vsctl -- add-port br-int hv1-vif1 -- \ + set interface hv1-vif1 external-ids:iface-id=ls1-lp1 \ + options:tx_pcap=hv1/vif1-tx.pcap \ + options:rxq_pcap=hv1/vif1-rx.pcap \ + ofport-request=1 + +ovn_populate_arp +sleep 2 + +as hv1 ovs-vsctl show + +echo "---------------------" +ovn-sbctl dump-flows +echo "---------------------" + +echo "------ hv1 dump ----------" +as hv1 ovs-ofctl dump-flows br-int +total_flows=`as hv1 ovs-ofctl dump-flows br-int | wc -l` + +echo "Total flows before vswitchd restart = " $total_flows + +# Code taken from ovs-save utility +save_flows () { + echo "ovs-ofctl add-flows br-int - << EOF" > restore_flows.sh + as hv1 ovs-ofctl dump-flows "br-int" | sed -e '/NXST_FLOW/d' \ + -e 's/\(idle\|hard\)_age=[^,]*,//g' >> restore_flows.sh + echo "EOF" >> restore_flows.sh +} + +restart_vswitchd () { + restore_flows=$1 + + if test $restore_flows = true; then + save_flows + fi + + as hv1 + OVS_APP_EXIT_AND_WAIT([ovs-vswitchd]) + + if test $restore_flows = true; then + as hv1 + ovs-vsctl --no-wait set open_vswitch . other_config:flow-restore-wait="true" + fi + + as hv1 + start_daemon ovs-vswitchd --enable-dummy=system -vvconn -vofproto_dpif -vunixctl + ovs-ofctl dump-flows br-int + + if test $restore_flows = true; then + sh ./restore_flows.sh + echo "Flows after restore" + as hv1 + ovs-ofctl dump-flows br-int + ovs-vsctl --no-wait --if-exists remove open_vswitch . other_config \ + flow-restore-wait="true" + fi +} + +# Save the flows, restart vswitchd and restore the flows +restart_vswitchd true +sleep 2 +total_flows_after_restart=`as hv1 ovs-ofctl dump-flows br-int | wc -l` +echo "Total flows after vswitchd restart = " $total_flows_after_restart + +AT_CHECK([test "${total_flows}" = "${total_flows_after_restart}"]) + +# Restart vswitchd without restoring +restart_vswitchd false +sleep 2 +total_flows_after_restart=`as hv1 ovs-ofctl dump-flows br-int | wc -l` +echo "Total flows after vswitchd restart = " $total_flows_after_restart + +AT_CHECK([test "${total_flows}" = "${total_flows_after_restart}"]) + +OVN_CLEANUP([hv1]) +AT_CLEANUP