From patchwork Tue Jul 7 13:16:22 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1324339 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.136; helo=silver.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 silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4B1NHv71c1z9s1x for ; Tue, 7 Jul 2020 23:16:43 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 810B62916B; Tue, 7 Jul 2020 13:16:41 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id syVZ16TczbN2; Tue, 7 Jul 2020 13:16:39 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by silver.osuosl.org (Postfix) with ESMTP id DC0B22011A; Tue, 7 Jul 2020 13:16:39 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id B5C29C07FF; Tue, 7 Jul 2020 13:16:39 +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 E1F2EC016F for ; Tue, 7 Jul 2020 13:16:38 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id D6E0F87DF9 for ; Tue, 7 Jul 2020 13:16:38 +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 K3jDtTcutkIW for ; Tue, 7 Jul 2020 13:16:36 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net [217.70.183.195]) by whitealder.osuosl.org (Postfix) with ESMTPS id 6895487DEB for ; Tue, 7 Jul 2020 13:16:36 +0000 (UTC) X-Originating-IP: 27.7.98.201 Received: from nusiddiq.home.org.com (unknown [27.7.98.201]) (Authenticated sender: numans@ovn.org) by relay3-d.mail.gandi.net (Postfix) with ESMTPSA id CC2446000D; Tue, 7 Jul 2020 13:16:31 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Tue, 7 Jul 2020 18:46:22 +0530 Message-Id: <20200707131622.581859-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 MIME-Version: 1.0 Cc: Tim Rozet Subject: [ovs-dev] [PATCH ovn] ovn-northd: Fix the missing lflow issue in LS_OUT_PRE_LB. 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 load balancer(s) configured with VIPs are associated to a logical switch, then ovn-northd adds the below logical flow so that the packets in the egress switch pipeline enter the conntrack. table=0 (ls_out_pre_lb ), priority=100 , match=(ip), action=(reg0[[0]] = 1; next;) ovn-northd maintains a local boolean variable 'vip_configured' in build_pre_lb() and adds the above lflow if this is true at the end. But this variable is overriden as -> vip_configured = !!lb->n_vips; when it loops through every load balancer associated with the logical switch. This is wrong and this patch fixes this issue. A test case is addd to test this scenario and the test case fails without the fix in this patch. Fixes: bb9f2b9ce56c("ovn-northd: Consider load balancer active backends in router pipeline") Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1849162 Reported-by: Tim Rozet Signed-off-by: Numan Siddique Acked-by: Dumitru Ceara --- northd/ovn-northd.c | 2 +- tests/ovn-northd.at | 73 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 32ecab851..203391a45 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -5076,7 +5076,7 @@ build_pre_lb(struct ovn_datapath *od, struct hmap *lflows, * table, we will eventually look at L4 information. */ } - vip_configured = !!lb->n_vips; + vip_configured = (vip_configured || lb->n_vips); } /* 'REGBIT_CONNTRACK_DEFRAG' is set to let the pre-stateful table send diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 5693f84de..1e4d20a5d 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -1700,3 +1700,76 @@ action=(eth.dst = eth.src; eth.src = 00:00:00:00:00:02; arp.op = 2; /* ARP reply AT_CHECK([ovn-sbctl lflow-list | grep "xreg0\[[0..47\]]" | grep -vE 'lr_in_admission|lr_in_ip_input'], [1], []) AT_CLEANUP + +# This test case tests that when a logical switch has load balancers associated +# (with VIPs configured), the below logical flow is added by ovn-northd. +# table=0 (ls_out_pre_lb ), priority=100 , match=(ip), action=(reg0[[0]] = 1; next;) +# This test case is added for the BZ - +# https://bugzilla.redhat.com/show_bug.cgi?id=1849162 +# +# ovn-northd was not adding the above lflow if the last load balancer associated +# to the logical switch doesn't have the VIP configured even if other load +# balancers before the last one in the last have VIPs configured. +# So make sure that the above lflow is added even if one load balancer has VIP +# associated. + +AT_SETUP([ovn -- Load balancer - missing ls_out_pre_lb flows]) +ovn_start + +ovn-nbctl ls-add sw0 +ovn-nbctl lsp-add sw0 sw0-p1 + +ovn-nbctl lb-add lb1 "10.0.0.10" "10.0.0.3" +ovn-nbctl lb-add lb2 "10.0.0.11" "10.0.0.4" + +ovn-nbctl --wait=sb sync +AT_CHECK([ovn-sbctl lflow-list | grep "ls_out_pre_lb.*priority=100" | grep reg0 | sort], [0], [dnl +]) + +ovn-nbctl ls-lb-add sw0 lb1 +ovn-nbctl --wait=sb sync +AT_CHECK([ovn-sbctl lflow-list | grep "ls_out_pre_lb.*priority=100" | grep reg0 | sort], [0], [dnl + table=0 (ls_out_pre_lb ), priority=100 , match=(ip), action=(reg0[[0]] = 1; next;) +]) + +ovn-nbctl ls-lb-add sw0 lb2 +ovn-nbctl --wait=sb sync +AT_CHECK([ovn-sbctl lflow-list | grep "ls_out_pre_lb.*priority=100" | grep reg0 | sort], [0], [dnl + table=0 (ls_out_pre_lb ), priority=100 , match=(ip), action=(reg0[[0]] = 1; next;) +]) + +lb1_uuid=$(ovn-nbctl --bare --columns _uuid find load_balancer name=lb1) +lb2_uuid=$(ovn-nbctl --bare --columns _uuid find load_balancer name=lb2) + +ovn-nbctl clear load_balancer $lb1_uuid vips +ovn-nbctl --wait=sb sync +AT_CHECK([ovn-sbctl lflow-list | grep "ls_out_pre_lb.*priority=100" | grep reg0 | sort], [0], [dnl + table=0 (ls_out_pre_lb ), priority=100 , match=(ip), action=(reg0[[0]] = 1; next;) +]) + +ovn-nbctl clear load_balancer $lb2_uuid vips +ovn-nbctl --wait=sb sync +AT_CHECK([ovn-sbctl lflow-list | grep "ls_out_pre_lb.*priority=100" | grep reg0 | sort], [0], [dnl +]) + +ovn-nbctl set load_balancer $lb1_uuid vips:"10.0.0.10"="10.0.0.3" +ovn-nbctl set load_balancer $lb2_uuid vips:"10.0.0.11"="10.0.0.4" + +ovn-nbctl --wait=sb sync +AT_CHECK([ovn-sbctl lflow-list | grep "ls_out_pre_lb.*priority=100" | grep reg0 | sort], [0], [dnl + table=0 (ls_out_pre_lb ), priority=100 , match=(ip), action=(reg0[[0]] = 1; next;) +]) + +# Now reverse the order of clearing the vip. +ovn-nbctl clear load_balancer $lb2_uuid vips +ovn-nbctl --wait=sb sync +AT_CHECK([ovn-sbctl lflow-list | grep "ls_out_pre_lb.*priority=100" | grep reg0 | sort], [0], [dnl + table=0 (ls_out_pre_lb ), priority=100 , match=(ip), action=(reg0[[0]] = 1; next;) +]) + +ovn-nbctl clear load_balancer $lb1_uuid vips +ovn-nbctl --wait=sb sync +AT_CHECK([ovn-sbctl lflow-list | grep "ls_out_pre_lb.*priority=100" | grep reg0 | sort], [0], [dnl +]) + +AT_CLEANUP