From patchwork Thu Apr 22 07:30:35 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1469039 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=2605:bc80:3010::133; helo=smtp2.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4FQpyF0BYkz9sTD for ; Thu, 22 Apr 2021 17:31:32 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 45F3840584; Thu, 22 Apr 2021 07:31:26 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id nhc3-i3EvoUG; Thu, 22 Apr 2021 07:31:25 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp2.osuosl.org (Postfix) with ESMTP id 7F2AD40500; Thu, 22 Apr 2021 07:31:24 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 56497C000E; Thu, 22 Apr 2021 07:31:24 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) by lists.linuxfoundation.org (Postfix) with ESMTP id E5044C000B for ; Thu, 22 Apr 2021 07:31:22 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id C7F6440E4A for ; Thu, 22 Apr 2021 07:31:22 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id cIiv9ZDGzJpQ for ; Thu, 22 Apr 2021 07:31:21 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by smtp4.osuosl.org (Postfix) with ESMTPS id 88B10404B5 for ; Thu, 22 Apr 2021 07:31:21 +0000 (UTC) X-Originating-IP: 98.37.104.183 Received: from localhost-live.hsd1.ca.comcast.net (unknown [98.37.104.183]) (Authenticated sender: hzhou@ovn.org) by relay5-d.mail.gandi.net (Postfix) with ESMTPSA id AF1EA1C000B; Thu, 22 Apr 2021 07:31:17 +0000 (UTC) From: Han Zhou To: dev@openvswitch.org Date: Thu, 22 Apr 2021 00:30:35 -0700 Message-Id: <20210422073038.130392-1-hzhou@ovn.org> X-Mailer: git-send-email 2.30.2 MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn 1/4] inc-proc-eng: Call clear_tracked_data before recompute. 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" Cleanup particially tracked data due to some of the change handler executions before falling back to recompute. This is done already in the en_runtime_data_run() implementation, but this patch makes it a generic behavior of the I-P engine. Signed-off-by: Han Zhou --- controller/ovn-controller.c | 17 ----------------- lib/inc-proc-eng.c | 5 +++++ 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 6f7c9ea61..13c03131c 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1412,23 +1412,6 @@ en_runtime_data_run(struct engine_node *node, void *data) struct sset *local_lport_ids = &rt_data->local_lport_ids; struct sset *active_tunnels = &rt_data->active_tunnels; - /* Clear the (stale) tracked data if any. Even though the tracked data - * gets cleared in the beginning of engine_init_run(), - * any of the runtime data handler might have set some tracked - * data and later another runtime data handler might return false - * resulting in full recompute of runtime engine and rendering the tracked - * data stale. - * - * It's possible that engine framework can be enhanced to indicate - * the node handlers (in this case flow_output_runtime_data_handler) - * that its input node had a full recompute. However we would still - * need to clear the tracked data, because we don't want the - * stale tracked data to be accessed outside of the engine, since the - * tracked data is cleared in the engine_init_run() and not at the - * end of the engine run. - * */ - en_runtime_data_clear_tracked_data(data); - static bool first_run = true; if (first_run) { /* don't cleanup since there is no data yet */ diff --git a/lib/inc-proc-eng.c b/lib/inc-proc-eng.c index a6337a1d9..161327404 100644 --- a/lib/inc-proc-eng.c +++ b/lib/inc-proc-eng.c @@ -327,6 +327,11 @@ engine_recompute(struct engine_node *node, bool forced, bool allowed) } /* Run the node handler which might change state. */ + /* Clear tracked data before calling run() so that partially tracked data + * from some of the change handler executions are cleared. */ + if (node->clear_tracked_data) { + node->clear_tracked_data(node->data); + } node->run(node, node->data); node->stats.recompute++; } From patchwork Thu Apr 22 07:30:36 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1469040 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=2605:bc80:3010::133; helo=smtp2.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4FQpyK3Ttxz9sTD for ; Thu, 22 Apr 2021 17:31:37 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 3D79B405B7; Thu, 22 Apr 2021 07:31:29 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id qdacG8pHTni2; Thu, 22 Apr 2021 07:31:28 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp2.osuosl.org (Postfix) with ESMTP id D426E4059C; Thu, 22 Apr 2021 07:31:26 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 44046C001F; Thu, 22 Apr 2021 07:31:25 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 4EF47C000B for ; Thu, 22 Apr 2021 07:31:23 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 31F3683DF0 for ; Thu, 22 Apr 2021 07:31:23 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 4aMjhI2bewUj for ; Thu, 22 Apr 2021 07:31:22 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by smtp1.osuosl.org (Postfix) with ESMTPS id 87E6783DEE for ; Thu, 22 Apr 2021 07:31:21 +0000 (UTC) X-Originating-IP: 98.37.104.183 Received: from localhost-live.hsd1.ca.comcast.net (unknown [98.37.104.183]) (Authenticated sender: hzhou@ovn.org) by relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 059861C0006; Thu, 22 Apr 2021 07:31:18 +0000 (UTC) From: Han Zhou To: dev@openvswitch.org Date: Thu, 22 Apr 2021 00:30:36 -0700 Message-Id: <20210422073038.130392-2-hzhou@ovn.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210422073038.130392-1-hzhou@ovn.org> References: <20210422073038.130392-1-hzhou@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn 2/4] ovn.at: Improve "No ovn-controller assert when generating conjunction flows" 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" This patch improves the test case by binding 2 VIFs on the HV instead of one, to make sure conjunction is still used and the scenario is still tested by this test case when a following patch optimizes conjunction flows. Signed-off-by: Han Zhou --- tests/ovn.at | 94 ++++++++++++++++++++++++++++------------------------ 1 file changed, 50 insertions(+), 44 deletions(-) diff --git a/tests/ovn.at b/tests/ovn.at index 3d0a7f63f..55444bbd7 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -25111,10 +25111,12 @@ ovs-vsctl add-br br-phys ovn_attach n1 br-phys 192.168.0.10 as hv1 -ovs-vsctl \ - -- add-port br-int vif1 \ - -- set Interface vif1 external_ids:iface-id=sw0-p1 \ - ofport-request=1 +for i in 1 2; do + ovs-vsctl \ + -- add-port br-int vif$i \ + -- set Interface vif$i external_ids:iface-id=sw0-p$i \ + ofport-request=$i +done check as hv1 ovs-vsctl set open . external_ids:ovn-monitor-all=true @@ -25122,10 +25124,10 @@ ovs-vsctl set open . external_ids:ovn-monitor-all=true check ovn-nbctl ls-add sw0 check ovn-nbctl pg-add pg1 check ovn-nbctl pg-add pg2 -check ovn-nbctl lsp-add sw0 sw0-p2 -check ovn-nbctl lsp-set-addresses sw0-p2 "00:00:00:00:00:02 192.168.47.2" check ovn-nbctl lsp-add sw0 sw0-p3 check ovn-nbctl lsp-set-addresses sw0-p3 "00:00:00:00:00:03 192.168.47.3" +check ovn-nbctl lsp-add sw0 sw0-p4 +check ovn-nbctl lsp-set-addresses sw0-p4 "00:00:00:00:00:04 192.168.47.4" # Pause ovn-northd. When it is resumed, all the below NB updates # will be sent in one transaction. @@ -25135,8 +25137,10 @@ check as northd-backup ovn-appctl -t NORTHD_TYPE pause check ovn-nbctl lsp-add sw0 sw0-p1 check ovn-nbctl lsp-set-addresses sw0-p1 "00:00:00:00:00:01 192.168.47.1" -check ovn-nbctl pg-set-ports pg1 sw0-p1 sw0-p2 -check ovn-nbctl pg-set-ports pg2 sw0-p3 +check ovn-nbctl lsp-add sw0 sw0-p2 +check ovn-nbctl lsp-set-addresses sw0-p2 "00:00:00:00:00:02 192.168.47.2" +check ovn-nbctl pg-set-ports pg1 sw0-p1 sw0-p2 sw0-p3 +check ovn-nbctl pg-set-ports pg2 sw0-p4 check ovn-nbctl acl-add pg1 to-lport 1002 "outport == @pg1 && ip4 && ip4.src == \$pg2_ip4 && udp && udp.dst >= 1 && udp.dst <= 65535" allow-related # resume ovn-northd now. This should result in a single update message @@ -25144,11 +25148,11 @@ check ovn-nbctl acl-add pg1 to-lport 1002 "outport == @pg1 && ip4 && ip4.src == check as northd ovn-appctl -t NORTHD_TYPE resume AS_BOX([Wait for sw0-p1 to be up]) -wait_for_ports_up sw0-p1 +wait_for_ports_up sw0-p1 sw0-p2 # When the port group pg1 is updated, it should not result in # any assert in ovn-controller. -ovn-nbctl --wait=hv pg-set-ports pg1 sw0-p1 sw0-p2 sw0-p3 +ovn-nbctl --wait=hv pg-set-ports pg1 sw0-p1 sw0-p2 sw0-p3 sw0-p4 AT_CHECK([kill -0 $(cat hv1/ovn-controller.pid)]) check ovn-nbctl --wait=hv sync @@ -25156,40 +25160,42 @@ check ovn-nbctl --wait=hv sync AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | ofctl_strip_all | \ grep "priority=2002" | grep conjunction | \ sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl - table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.3,tp_dst=0x10/0xfff0 actions=conjunction() - table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.3,tp_dst=0x100/0xff00 actions=conjunction() - table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.3,tp_dst=0x1000/0xf000 actions=conjunction() - table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.3,tp_dst=0x2/0xfffe actions=conjunction() - table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.3,tp_dst=0x20/0xffe0 actions=conjunction() - table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.3,tp_dst=0x200/0xfe00 actions=conjunction() - table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.3,tp_dst=0x2000/0xe000 actions=conjunction() - table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.3,tp_dst=0x4/0xfffc actions=conjunction() - table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.3,tp_dst=0x40/0xffc0 actions=conjunction() - table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.3,tp_dst=0x400/0xfc00 actions=conjunction() - table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.3,tp_dst=0x4000/0xc000 actions=conjunction() - table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.3,tp_dst=0x8/0xfff8 actions=conjunction() - table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.3,tp_dst=0x80/0xff80 actions=conjunction() - table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.3,tp_dst=0x800/0xf800 actions=conjunction() - table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.3,tp_dst=0x8000/0x8000 actions=conjunction() - table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.3,tp_dst=1 actions=conjunction() - table=45, priority=2002,udp,reg0=0x100/0x100,reg15=0x3,metadata=0x1,nw_src=192.168.47.3 actions=conjunction() - table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.3,tp_dst=0x10/0xfff0 actions=conjunction() - table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.3,tp_dst=0x100/0xff00 actions=conjunction() - table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.3,tp_dst=0x1000/0xf000 actions=conjunction() - table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.3,tp_dst=0x2/0xfffe actions=conjunction() - table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.3,tp_dst=0x20/0xffe0 actions=conjunction() - table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.3,tp_dst=0x200/0xfe00 actions=conjunction() - table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.3,tp_dst=0x2000/0xe000 actions=conjunction() - table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.3,tp_dst=0x4/0xfffc actions=conjunction() - table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.3,tp_dst=0x40/0xffc0 actions=conjunction() - table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.3,tp_dst=0x400/0xfc00 actions=conjunction() - table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.3,tp_dst=0x4000/0xc000 actions=conjunction() - table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.3,tp_dst=0x8/0xfff8 actions=conjunction() - table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.3,tp_dst=0x80/0xff80 actions=conjunction() - table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.3,tp_dst=0x800/0xf800 actions=conjunction() - table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.3,tp_dst=0x8000/0x8000 actions=conjunction() - table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.3,tp_dst=1 actions=conjunction() - table=45, priority=2002,udp,reg0=0x80/0x80,reg15=0x3,metadata=0x1,nw_src=192.168.47.3 actions=conjunction() + table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.4,tp_dst=0x10/0xfff0 actions=conjunction() + table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.4,tp_dst=0x100/0xff00 actions=conjunction() + table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.4,tp_dst=0x1000/0xf000 actions=conjunction() + table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.4,tp_dst=0x2/0xfffe actions=conjunction() + table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.4,tp_dst=0x20/0xffe0 actions=conjunction() + table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.4,tp_dst=0x200/0xfe00 actions=conjunction() + table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.4,tp_dst=0x2000/0xe000 actions=conjunction() + table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.4,tp_dst=0x4/0xfffc actions=conjunction() + table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.4,tp_dst=0x40/0xffc0 actions=conjunction() + table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.4,tp_dst=0x400/0xfc00 actions=conjunction() + table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.4,tp_dst=0x4000/0xc000 actions=conjunction() + table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.4,tp_dst=0x8/0xfff8 actions=conjunction() + table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.4,tp_dst=0x80/0xff80 actions=conjunction() + table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.4,tp_dst=0x800/0xf800 actions=conjunction() + table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.4,tp_dst=0x8000/0x8000 actions=conjunction() + table=45, priority=2002,udp,reg0=0x100/0x100,metadata=0x1,nw_src=192.168.47.4,tp_dst=1 actions=conjunction() + table=45, priority=2002,udp,reg0=0x100/0x100,reg15=0x3,metadata=0x1,nw_src=192.168.47.4 actions=conjunction() + table=45, priority=2002,udp,reg0=0x100/0x100,reg15=0x4,metadata=0x1,nw_src=192.168.47.4 actions=conjunction() + table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.4,tp_dst=0x10/0xfff0 actions=conjunction() + table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.4,tp_dst=0x100/0xff00 actions=conjunction() + table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.4,tp_dst=0x1000/0xf000 actions=conjunction() + table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.4,tp_dst=0x2/0xfffe actions=conjunction() + table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.4,tp_dst=0x20/0xffe0 actions=conjunction() + table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.4,tp_dst=0x200/0xfe00 actions=conjunction() + table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.4,tp_dst=0x2000/0xe000 actions=conjunction() + table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.4,tp_dst=0x4/0xfffc actions=conjunction() + table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.4,tp_dst=0x40/0xffc0 actions=conjunction() + table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.4,tp_dst=0x400/0xfc00 actions=conjunction() + table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.4,tp_dst=0x4000/0xc000 actions=conjunction() + table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.4,tp_dst=0x8/0xfff8 actions=conjunction() + table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.4,tp_dst=0x80/0xff80 actions=conjunction() + table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.4,tp_dst=0x800/0xf800 actions=conjunction() + table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.4,tp_dst=0x8000/0x8000 actions=conjunction() + table=45, priority=2002,udp,reg0=0x80/0x80,metadata=0x1,nw_src=192.168.47.4,tp_dst=1 actions=conjunction() + table=45, priority=2002,udp,reg0=0x80/0x80,reg15=0x3,metadata=0x1,nw_src=192.168.47.4 actions=conjunction() + table=45, priority=2002,udp,reg0=0x80/0x80,reg15=0x4,metadata=0x1,nw_src=192.168.47.4 actions=conjunction() ]) OVN_CLEANUP([hv1]) From patchwork Thu Apr 22 07:30:37 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1469042 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=2605:bc80:3010::133; helo=smtp2.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4FQpyP0NDbz9sTD for ; Thu, 22 Apr 2021 17:31:40 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 667DF405BB; Thu, 22 Apr 2021 07:31:31 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id nIonZmugcPdg; Thu, 22 Apr 2021 07:31:30 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp2.osuosl.org (Postfix) with ESMTP id 4ABC3405AA; Thu, 22 Apr 2021 07:31:28 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 4A128C002B; Thu, 22 Apr 2021 07:31:26 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.133]) by lists.linuxfoundation.org (Postfix) with ESMTP id A88BBC000B for ; Thu, 22 Apr 2021 07:31:24 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 8E4FF40517 for ; Thu, 22 Apr 2021 07:31:24 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 1ItmIc8akgNE for ; Thu, 22 Apr 2021 07:31:23 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by smtp2.osuosl.org (Postfix) with ESMTPS id 2CAB140143 for ; Thu, 22 Apr 2021 07:31:22 +0000 (UTC) X-Originating-IP: 98.37.104.183 Received: from localhost-live.hsd1.ca.comcast.net (unknown [98.37.104.183]) (Authenticated sender: hzhou@ovn.org) by relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 5E4271C000D; Thu, 22 Apr 2021 07:31:20 +0000 (UTC) From: Han Zhou To: dev@openvswitch.org Date: Thu, 22 Apr 2021 00:30:37 -0700 Message-Id: <20210422073038.130392-3-hzhou@ovn.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210422073038.130392-1-hzhou@ovn.org> References: <20210422073038.130392-1-hzhou@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn 3/4] ovn-controller.c: Reorder addrset and portgroup related functions. 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" Move the logically related functions together, which would also make reviewing the next patch much easier. Signed-off-by: Han Zhou --- controller/ovn-controller.c | 467 ++++++++++++++++++------------------ 1 file changed, 233 insertions(+), 234 deletions(-) diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 13c03131c..7320bd56c 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -433,80 +433,6 @@ get_ovs_chassis_id(const struct ovsrec_open_vswitch_table *ovs_table) return chassis_id; } -/* Iterate address sets in the southbound database. Create and update the - * corresponding symtab entries as necessary. */ -static void -addr_sets_init(const struct sbrec_address_set_table *address_set_table, - struct shash *addr_sets) -{ - const struct sbrec_address_set *as; - SBREC_ADDRESS_SET_TABLE_FOR_EACH (as, address_set_table) { - expr_const_sets_add(addr_sets, as->name, - (const char *const *) as->addresses, - as->n_addresses, true); - } -} - -static void -addr_sets_update(const struct sbrec_address_set_table *address_set_table, - struct shash *addr_sets, struct sset *new, - struct sset *deleted, struct sset *updated) -{ - const struct sbrec_address_set *as; - SBREC_ADDRESS_SET_TABLE_FOR_EACH_TRACKED (as, address_set_table) { - if (sbrec_address_set_is_deleted(as)) { - expr_const_sets_remove(addr_sets, as->name); - sset_add(deleted, as->name); - } else { - expr_const_sets_add(addr_sets, as->name, - (const char *const *) as->addresses, - as->n_addresses, true); - if (sbrec_address_set_is_new(as)) { - sset_add(new, as->name); - } else { - sset_add(updated, as->name); - } - } - } -} - -/* Iterate port groups in the southbound database. Create and update the - * corresponding symtab entries as necessary. */ - static void -port_groups_init(const struct sbrec_port_group_table *port_group_table, - struct shash *port_groups) -{ - const struct sbrec_port_group *pg; - SBREC_PORT_GROUP_TABLE_FOR_EACH (pg, port_group_table) { - expr_const_sets_add(port_groups, pg->name, - (const char *const *) pg->ports, - pg->n_ports, false); - } -} - -static void -port_groups_update(const struct sbrec_port_group_table *port_group_table, - struct shash *port_groups, struct sset *new, - struct sset *deleted, struct sset *updated) -{ - const struct sbrec_port_group *pg; - SBREC_PORT_GROUP_TABLE_FOR_EACH_TRACKED (pg, port_group_table) { - if (sbrec_port_group_is_deleted(pg)) { - expr_const_sets_remove(port_groups, pg->name); - sset_add(deleted, pg->name); - } else { - expr_const_sets_add(port_groups, pg->name, - (const char *const *) pg->ports, - pg->n_ports, false); - if (sbrec_port_group_is_new(pg)) { - sset_add(new, pg->name); - } else { - sset_add(updated, pg->name); - } - } - } -} - static void update_ssl_config(const struct ovsrec_ssl_table *ssl_table) { @@ -1011,166 +937,6 @@ en_ofctrl_is_connected_run(struct engine_node *node, void *data) engine_set_node_state(node, EN_UNCHANGED); } -struct ed_type_addr_sets { - struct shash addr_sets; - bool change_tracked; - struct sset new; - struct sset deleted; - struct sset updated; -}; - -static void * -en_addr_sets_init(struct engine_node *node OVS_UNUSED, - struct engine_arg *arg OVS_UNUSED) -{ - struct ed_type_addr_sets *as = xzalloc(sizeof *as); - - shash_init(&as->addr_sets); - as->change_tracked = false; - sset_init(&as->new); - sset_init(&as->deleted); - sset_init(&as->updated); - return as; -} - -static void -en_addr_sets_cleanup(void *data) -{ - struct ed_type_addr_sets *as = data; - expr_const_sets_destroy(&as->addr_sets); - shash_destroy(&as->addr_sets); - sset_destroy(&as->new); - sset_destroy(&as->deleted); - sset_destroy(&as->updated); -} - -static void -en_addr_sets_run(struct engine_node *node, void *data) -{ - struct ed_type_addr_sets *as = data; - - sset_clear(&as->new); - sset_clear(&as->deleted); - sset_clear(&as->updated); - expr_const_sets_destroy(&as->addr_sets); - - struct sbrec_address_set_table *as_table = - (struct sbrec_address_set_table *)EN_OVSDB_GET( - engine_get_input("SB_address_set", node)); - - addr_sets_init(as_table, &as->addr_sets); - - as->change_tracked = false; - engine_set_node_state(node, EN_UPDATED); -} - -static bool -addr_sets_sb_address_set_handler(struct engine_node *node, void *data) -{ - struct ed_type_addr_sets *as = data; - - sset_clear(&as->new); - sset_clear(&as->deleted); - sset_clear(&as->updated); - - struct sbrec_address_set_table *as_table = - (struct sbrec_address_set_table *)EN_OVSDB_GET( - engine_get_input("SB_address_set", node)); - - addr_sets_update(as_table, &as->addr_sets, &as->new, - &as->deleted, &as->updated); - - if (!sset_is_empty(&as->new) || !sset_is_empty(&as->deleted) || - !sset_is_empty(&as->updated)) { - engine_set_node_state(node, EN_UPDATED); - } else { - engine_set_node_state(node, EN_UNCHANGED); - } - - as->change_tracked = true; - return true; -} - -struct ed_type_port_groups{ - struct shash port_groups; - bool change_tracked; - struct sset new; - struct sset deleted; - struct sset updated; -}; - -static void * -en_port_groups_init(struct engine_node *node OVS_UNUSED, - struct engine_arg *arg OVS_UNUSED) -{ - struct ed_type_port_groups *pg = xzalloc(sizeof *pg); - - shash_init(&pg->port_groups); - pg->change_tracked = false; - sset_init(&pg->new); - sset_init(&pg->deleted); - sset_init(&pg->updated); - return pg; -} - -static void -en_port_groups_cleanup(void *data) -{ - struct ed_type_port_groups *pg = data; - expr_const_sets_destroy(&pg->port_groups); - shash_destroy(&pg->port_groups); - sset_destroy(&pg->new); - sset_destroy(&pg->deleted); - sset_destroy(&pg->updated); -} - -static void -en_port_groups_run(struct engine_node *node, void *data) -{ - struct ed_type_port_groups *pg = data; - - sset_clear(&pg->new); - sset_clear(&pg->deleted); - sset_clear(&pg->updated); - expr_const_sets_destroy(&pg->port_groups); - - struct sbrec_port_group_table *pg_table = - (struct sbrec_port_group_table *)EN_OVSDB_GET( - engine_get_input("SB_port_group", node)); - - port_groups_init(pg_table, &pg->port_groups); - - pg->change_tracked = false; - engine_set_node_state(node, EN_UPDATED); -} - -static bool -port_groups_sb_port_group_handler(struct engine_node *node, void *data) -{ - struct ed_type_port_groups *pg = data; - - sset_clear(&pg->new); - sset_clear(&pg->deleted); - sset_clear(&pg->updated); - - struct sbrec_port_group_table *pg_table = - (struct sbrec_port_group_table *)EN_OVSDB_GET( - engine_get_input("SB_port_group", node)); - - port_groups_update(pg_table, &pg->port_groups, &pg->new, - &pg->deleted, &pg->updated); - - if (!sset_is_empty(&pg->new) || !sset_is_empty(&pg->deleted) || - !sset_is_empty(&pg->updated)) { - engine_set_node_state(node, EN_UPDATED); - } else { - engine_set_node_state(node, EN_UNCHANGED); - } - - pg->change_tracked = true; - return true; -} - struct ed_type_runtime_data { /* Contains "struct local_datapath" nodes. */ struct hmap local_datapaths; @@ -1532,6 +1298,239 @@ runtime_data_sb_datapath_binding_handler(struct engine_node *node OVS_UNUSED, return true; } +struct ed_type_addr_sets { + struct shash addr_sets; + bool change_tracked; + struct sset new; + struct sset deleted; + struct sset updated; +}; + +static void * +en_addr_sets_init(struct engine_node *node OVS_UNUSED, + struct engine_arg *arg OVS_UNUSED) +{ + struct ed_type_addr_sets *as = xzalloc(sizeof *as); + + shash_init(&as->addr_sets); + as->change_tracked = false; + sset_init(&as->new); + sset_init(&as->deleted); + sset_init(&as->updated); + return as; +} + +static void +en_addr_sets_cleanup(void *data) +{ + struct ed_type_addr_sets *as = data; + expr_const_sets_destroy(&as->addr_sets); + shash_destroy(&as->addr_sets); + sset_destroy(&as->new); + sset_destroy(&as->deleted); + sset_destroy(&as->updated); +} + +/* Iterate address sets in the southbound database. Create and update the + * corresponding symtab entries as necessary. */ +static void +addr_sets_init(const struct sbrec_address_set_table *address_set_table, + struct shash *addr_sets) +{ + const struct sbrec_address_set *as; + SBREC_ADDRESS_SET_TABLE_FOR_EACH (as, address_set_table) { + expr_const_sets_add(addr_sets, as->name, + (const char *const *) as->addresses, + as->n_addresses, true); + } +} + +static void +addr_sets_update(const struct sbrec_address_set_table *address_set_table, + struct shash *addr_sets, struct sset *new, + struct sset *deleted, struct sset *updated) +{ + const struct sbrec_address_set *as; + SBREC_ADDRESS_SET_TABLE_FOR_EACH_TRACKED (as, address_set_table) { + if (sbrec_address_set_is_deleted(as)) { + expr_const_sets_remove(addr_sets, as->name); + sset_add(deleted, as->name); + } else { + expr_const_sets_add(addr_sets, as->name, + (const char *const *) as->addresses, + as->n_addresses, true); + if (sbrec_address_set_is_new(as)) { + sset_add(new, as->name); + } else { + sset_add(updated, as->name); + } + } + } +} +static void +en_addr_sets_run(struct engine_node *node, void *data) +{ + struct ed_type_addr_sets *as = data; + + sset_clear(&as->new); + sset_clear(&as->deleted); + sset_clear(&as->updated); + expr_const_sets_destroy(&as->addr_sets); + + struct sbrec_address_set_table *as_table = + (struct sbrec_address_set_table *)EN_OVSDB_GET( + engine_get_input("SB_address_set", node)); + + addr_sets_init(as_table, &as->addr_sets); + + as->change_tracked = false; + engine_set_node_state(node, EN_UPDATED); +} + +static bool +addr_sets_sb_address_set_handler(struct engine_node *node, void *data) +{ + struct ed_type_addr_sets *as = data; + + sset_clear(&as->new); + sset_clear(&as->deleted); + sset_clear(&as->updated); + + struct sbrec_address_set_table *as_table = + (struct sbrec_address_set_table *)EN_OVSDB_GET( + engine_get_input("SB_address_set", node)); + + addr_sets_update(as_table, &as->addr_sets, &as->new, + &as->deleted, &as->updated); + + if (!sset_is_empty(&as->new) || !sset_is_empty(&as->deleted) || + !sset_is_empty(&as->updated)) { + engine_set_node_state(node, EN_UPDATED); + } else { + engine_set_node_state(node, EN_UNCHANGED); + } + + as->change_tracked = true; + return true; +} + +struct ed_type_port_groups{ + struct shash port_groups; + bool change_tracked; + struct sset new; + struct sset deleted; + struct sset updated; +}; + +static void * +en_port_groups_init(struct engine_node *node OVS_UNUSED, + struct engine_arg *arg OVS_UNUSED) +{ + struct ed_type_port_groups *pg = xzalloc(sizeof *pg); + + shash_init(&pg->port_groups); + pg->change_tracked = false; + sset_init(&pg->new); + sset_init(&pg->deleted); + sset_init(&pg->updated); + return pg; +} + +static void +en_port_groups_cleanup(void *data) +{ + struct ed_type_port_groups *pg = data; + expr_const_sets_destroy(&pg->port_groups); + shash_destroy(&pg->port_groups); + sset_destroy(&pg->new); + sset_destroy(&pg->deleted); + sset_destroy(&pg->updated); +} + +/* Iterate port groups in the southbound database. Create and update the + * corresponding symtab entries as necessary. */ + static void +port_groups_init(const struct sbrec_port_group_table *port_group_table, + struct shash *port_groups) +{ + const struct sbrec_port_group *pg; + SBREC_PORT_GROUP_TABLE_FOR_EACH (pg, port_group_table) { + expr_const_sets_add(port_groups, pg->name, + (const char *const *) pg->ports, + pg->n_ports, false); + } +} + +static void +port_groups_update(const struct sbrec_port_group_table *port_group_table, + struct shash *port_groups, struct sset *new, + struct sset *deleted, struct sset *updated) +{ + const struct sbrec_port_group *pg; + SBREC_PORT_GROUP_TABLE_FOR_EACH_TRACKED (pg, port_group_table) { + if (sbrec_port_group_is_deleted(pg)) { + expr_const_sets_remove(port_groups, pg->name); + sset_add(deleted, pg->name); + } else { + expr_const_sets_add(port_groups, pg->name, + (const char *const *) pg->ports, + pg->n_ports, false); + if (sbrec_port_group_is_new(pg)) { + sset_add(new, pg->name); + } else { + sset_add(updated, pg->name); + } + } + } +} + +static void +en_port_groups_run(struct engine_node *node, void *data) +{ + struct ed_type_port_groups *pg = data; + + sset_clear(&pg->new); + sset_clear(&pg->deleted); + sset_clear(&pg->updated); + expr_const_sets_destroy(&pg->port_groups); + + struct sbrec_port_group_table *pg_table = + (struct sbrec_port_group_table *)EN_OVSDB_GET( + engine_get_input("SB_port_group", node)); + + port_groups_init(pg_table, &pg->port_groups); + + pg->change_tracked = false; + engine_set_node_state(node, EN_UPDATED); +} + +static bool +port_groups_sb_port_group_handler(struct engine_node *node, void *data) +{ + struct ed_type_port_groups *pg = data; + + sset_clear(&pg->new); + sset_clear(&pg->deleted); + sset_clear(&pg->updated); + + struct sbrec_port_group_table *pg_table = + (struct sbrec_port_group_table *)EN_OVSDB_GET( + engine_get_input("SB_port_group", node)); + + port_groups_update(pg_table, &pg->port_groups, &pg->new, + &pg->deleted, &pg->updated); + + if (!sset_is_empty(&pg->new) || !sset_is_empty(&pg->deleted) || + !sset_is_empty(&pg->updated)) { + engine_set_node_state(node, EN_UPDATED); + } else { + engine_set_node_state(node, EN_UNCHANGED); + } + + pg->change_tracked = true; + return true; +} + /* Connection tracking zones. */ struct ed_type_ct_zones { unsigned long bitmap[BITMAP_N_LONGS(MAX_CT_ZONES)]; From patchwork Thu Apr 22 07:30:38 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1469041 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=smtp3.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4FQpyM3QZZz9sTD for ; Thu, 22 Apr 2021 17:31:39 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id EA773608EB; Thu, 22 Apr 2021 07:31:36 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 7sanINEYf5QH; Thu, 22 Apr 2021 07:31:33 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp3.osuosl.org (Postfix) with ESMTP id 58F24607BD; Thu, 22 Apr 2021 07:31:30 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id BF7BDC0026; Thu, 22 Apr 2021 07:31:28 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 998B1C001C for ; Thu, 22 Apr 2021 07:31:27 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 7678F60883 for ; Thu, 22 Apr 2021 07:31:27 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id fcZwQXwNvfzV for ; Thu, 22 Apr 2021 07:31:25 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by smtp3.osuosl.org (Postfix) with ESMTPS id 74FAE606C1 for ; Thu, 22 Apr 2021 07:31:25 +0000 (UTC) X-Originating-IP: 98.37.104.183 Received: from localhost-live.hsd1.ca.comcast.net (unknown [98.37.104.183]) (Authenticated sender: hzhou@ovn.org) by relay5-d.mail.gandi.net (Postfix) with ESMTPSA id F07191C0014; Thu, 22 Apr 2021 07:31:21 +0000 (UTC) From: Han Zhou To: dev@openvswitch.org Date: Thu, 22 Apr 2021 00:30:38 -0700 Message-Id: <20210422073038.130392-4-hzhou@ovn.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210422073038.130392-1-hzhou@ovn.org> References: <20210422073038.130392-1-hzhou@ovn.org> MIME-Version: 1.0 Cc: Girish Moodalbail , Dumitru Ceara Subject: [ovs-dev] [PATCH ovn 4/4] ovn-controller: Fix port group conjunction flow explosion problem. 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" For an ACL with match: outport == @PG && ip4.src == $PG_AS, given below scale: P: PG size LP: number of local lports D: number of all datapaths (logical switches) LD: number of datapaths that contain local lports With current OVN implementation, the total number of OF flows is: LP + (P * D) + D The reason is, firstly, datapath is not part of the conjunction, so for each datapath the lflow is reparsed. Secondly, although ovn-controller tries to filter out the flows that are for non-local lports, with the conjunction match, the logic that filters out non-local flows doesn't work for the conjunction part that doesn't have the lport in the match (the P * D part). When there is only one port on each LS it is fine, because no conjunction will be used because SB port groups are splited per datapath, so each port group would have only one port. However, when more than one ports are on each LS the flow explosion happens. This patch deal with the second reason above, by refining the SB port groups to store only locally bound lports: empty const sets will not generate any flows. This reduces the related flow number from LP + (P * D) + D to LP + (P * LD) + LD. Since LD is expected to be small, so even if it is a multiplier, the total number is larged reduced. In particular, in ovn-k8s use cases the LD is always 1, so the formular above becomes LP + P + LD. With a scale of 1k k8s nodes, each has 4 ports for the same PG: P = 4k, LP = 4, D = 1k, LD = 1. The current implementation generates ~4m flows. With this patch it becomes only ~4k. Reported-by: Girish Moodalbail Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2021-March/381082.html Reported-by: Dumitru Ceara Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1944098 Signed-off-by: Han Zhou --- controller/binding.c | 13 +++ controller/binding.h | 5 + controller/ovn-controller.c | 214 ++++++++++++++++++++++++++++++------ include/ovn/expr.h | 2 +- lib/expr.c | 8 +- tests/ovn.at | 53 +++++++++ tests/test-ovn.c | 12 +- utilities/ovn-trace.c | 4 +- 8 files changed, 269 insertions(+), 42 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index 514f5f33f..96f5479d9 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -2987,3 +2987,16 @@ cleanup: return b_lport; } + +struct sset * +binding_collect_local_binding_lports(struct local_binding_data *lbinding_data) +{ + struct sset *lports = xzalloc(sizeof *lports); + sset_init(lports); + struct shash_node *shash_node; + SHASH_FOR_EACH (shash_node, &lbinding_data->lports) { + struct binding_lport *b_lport = shash_node->data; + sset_add(lports, b_lport->name); + } + return lports; +} diff --git a/controller/binding.h b/controller/binding.h index 4fc9ef207..c2a99b0f2 100644 --- a/controller/binding.h +++ b/controller/binding.h @@ -128,4 +128,9 @@ void binding_seqno_run(struct local_binding_data *lbinding_data); void binding_seqno_install(struct local_binding_data *lbinding_data); void binding_seqno_flush(void); void binding_dump_local_bindings(struct local_binding_data *, struct ds *); + +/* Generates a sset of lport names from local_binding_data. + * Note: the caller is responsible for detroying the sset. + */ +struct sset *binding_collect_local_binding_lports(struct local_binding_data *); #endif /* controller/binding.h */ diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 7320bd56c..b948c7186 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1341,7 +1341,7 @@ addr_sets_init(const struct sbrec_address_set_table *address_set_table, SBREC_ADDRESS_SET_TABLE_FOR_EACH (as, address_set_table) { expr_const_sets_add(addr_sets, as->name, (const char *const *) as->addresses, - as->n_addresses, true); + as->n_addresses, true, NULL); } } @@ -1358,7 +1358,7 @@ addr_sets_update(const struct sbrec_address_set_table *address_set_table, } else { expr_const_sets_add(addr_sets, as->name, (const char *const *) as->addresses, - as->n_addresses, true); + as->n_addresses, true, NULL); if (sbrec_address_set_is_new(as)) { sset_add(new, as->name); } else { @@ -1367,6 +1367,7 @@ addr_sets_update(const struct sbrec_address_set_table *address_set_table, } } } + static void en_addr_sets_run(struct engine_node *node, void *data) { @@ -1415,20 +1416,69 @@ addr_sets_sb_address_set_handler(struct engine_node *node, void *data) } struct ed_type_port_groups{ - struct shash port_groups; + /* A copy of SB port_groups, each converted as a sset for efficient lport + * lookup. */ + struct shash port_group_ssets; + + /* Const sets containing local lports, used for expr parsing. */ + struct shash port_groups_cs_local; + bool change_tracked; struct sset new; struct sset deleted; struct sset updated; }; +static void +port_group_ssets_add_or_update(struct shash *port_group_ssets, + const struct sbrec_port_group *pg) +{ + struct sset *lports = shash_find_data(port_group_ssets, pg->name); + if (lports) { + sset_clear(lports); + } else { + lports = xzalloc(sizeof *lports); + sset_init(lports); + shash_add(port_group_ssets, pg->name, lports); + } + + for (size_t i = 0; i < pg->n_ports; i++) { + sset_add(lports, pg->ports[i]); + } +} + +static void +port_group_ssets_delete(struct shash *port_group_ssets, + const char *pg_name) +{ + struct shash_node *node = shash_find(port_group_ssets, pg_name); + if (node) { + sset_destroy((struct sset *)node->data); + shash_delete(port_group_ssets, node); + } +} + +/* Delete and free all ssets in port_group_ssets, but not + * destroying the shash itself. */ +static void +port_group_ssets_clear(struct shash *port_group_ssets) +{ + struct shash_node *node, *next; + SHASH_FOR_EACH_SAFE (node, next, port_group_ssets) { + struct sset *lports = node->data; + shash_delete(port_group_ssets, node); + sset_destroy(lports); + } +} + static void * en_port_groups_init(struct engine_node *node OVS_UNUSED, struct engine_arg *arg OVS_UNUSED) { struct ed_type_port_groups *pg = xzalloc(sizeof *pg); - shash_init(&pg->port_groups); + shash_init(&pg->port_group_ssets); + shash_init(&pg->port_groups_cs_local); pg->change_tracked = false; sset_init(&pg->new); sset_init(&pg->deleted); @@ -1440,41 +1490,52 @@ static void en_port_groups_cleanup(void *data) { struct ed_type_port_groups *pg = data; - expr_const_sets_destroy(&pg->port_groups); - shash_destroy(&pg->port_groups); + + expr_const_sets_destroy(&pg->port_groups_cs_local); + shash_destroy(&pg->port_groups_cs_local); + + port_group_ssets_clear(&pg->port_group_ssets); + shash_destroy(&pg->port_group_ssets); + sset_destroy(&pg->new); sset_destroy(&pg->deleted); sset_destroy(&pg->updated); } -/* Iterate port groups in the southbound database. Create and update the - * corresponding symtab entries as necessary. */ - static void +static void port_groups_init(const struct sbrec_port_group_table *port_group_table, - struct shash *port_groups) + const struct sset *local_lports, + struct shash *port_group_ssets, + struct shash *port_groups_cs_local) { const struct sbrec_port_group *pg; SBREC_PORT_GROUP_TABLE_FOR_EACH (pg, port_group_table) { - expr_const_sets_add(port_groups, pg->name, + port_group_ssets_add_or_update(port_group_ssets, pg); + expr_const_sets_add(port_groups_cs_local, pg->name, (const char *const *) pg->ports, - pg->n_ports, false); + pg->n_ports, false, local_lports); } } static void port_groups_update(const struct sbrec_port_group_table *port_group_table, - struct shash *port_groups, struct sset *new, - struct sset *deleted, struct sset *updated) + const struct sset *local_lports, + struct shash *port_group_ssets, + struct shash *port_groups_cs_local, + struct sset *new, struct sset *deleted, + struct sset *updated) { const struct sbrec_port_group *pg; SBREC_PORT_GROUP_TABLE_FOR_EACH_TRACKED (pg, port_group_table) { if (sbrec_port_group_is_deleted(pg)) { - expr_const_sets_remove(port_groups, pg->name); + expr_const_sets_remove(port_groups_cs_local, pg->name); + port_group_ssets_delete(port_group_ssets, pg->name); sset_add(deleted, pg->name); } else { - expr_const_sets_add(port_groups, pg->name, + expr_const_sets_add(port_groups_cs_local, pg->name, (const char *const *) pg->ports, - pg->n_ports, false); + pg->n_ports, false, local_lports); + port_group_ssets_add_or_update(port_group_ssets, pg); if (sbrec_port_group_is_new(pg)) { sset_add(new, pg->name); } else { @@ -1485,22 +1546,36 @@ port_groups_update(const struct sbrec_port_group_table *port_group_table, } static void -en_port_groups_run(struct engine_node *node, void *data) +en_port_groups_clear_tracked_data(void *data_) { - struct ed_type_port_groups *pg = data; - + struct ed_type_port_groups *pg = data_; sset_clear(&pg->new); sset_clear(&pg->deleted); sset_clear(&pg->updated); - expr_const_sets_destroy(&pg->port_groups); + pg->change_tracked = false; +} + +static void +en_port_groups_run(struct engine_node *node, void *data) +{ + struct ed_type_port_groups *pg = data; + + expr_const_sets_destroy(&pg->port_groups_cs_local); + port_group_ssets_clear(&pg->port_group_ssets); struct sbrec_port_group_table *pg_table = (struct sbrec_port_group_table *)EN_OVSDB_GET( engine_get_input("SB_port_group", node)); - port_groups_init(pg_table, &pg->port_groups); + struct ed_type_runtime_data *rt_data = + engine_get_input_data("runtime_data", node); + + struct sset *local_b_lports = binding_collect_local_binding_lports( + &rt_data->lbinding_data); + port_groups_init(pg_table, local_b_lports, &pg->port_group_ssets, + &pg->port_groups_cs_local); + sset_destroy(local_b_lports); - pg->change_tracked = false; engine_set_node_state(node, EN_UPDATED); } @@ -1509,16 +1584,19 @@ port_groups_sb_port_group_handler(struct engine_node *node, void *data) { struct ed_type_port_groups *pg = data; - sset_clear(&pg->new); - sset_clear(&pg->deleted); - sset_clear(&pg->updated); - struct sbrec_port_group_table *pg_table = (struct sbrec_port_group_table *)EN_OVSDB_GET( engine_get_input("SB_port_group", node)); - port_groups_update(pg_table, &pg->port_groups, &pg->new, - &pg->deleted, &pg->updated); + struct ed_type_runtime_data *rt_data = + engine_get_input_data("runtime_data", node); + + struct sset *local_b_lports = binding_collect_local_binding_lports( + &rt_data->lbinding_data); + port_groups_update(pg_table, local_b_lports, &pg->port_group_ssets, + &pg->port_groups_cs_local, &pg->new, &pg->deleted, + &pg->updated); + sset_destroy(local_b_lports); if (!sset_is_empty(&pg->new) || !sset_is_empty(&pg->deleted) || !sset_is_empty(&pg->updated)) { @@ -1531,6 +1609,74 @@ port_groups_sb_port_group_handler(struct engine_node *node, void *data) return true; } +static bool +port_groups_runtime_data_handler(struct engine_node *node, void *data) +{ + struct ed_type_port_groups *pg = data; + + struct sbrec_port_group_table *pg_table = + (struct sbrec_port_group_table *)EN_OVSDB_GET( + engine_get_input("SB_port_group", node)); + + struct ed_type_runtime_data *rt_data = + engine_get_input_data("runtime_data", node); + + if (!rt_data->tracked) { + return false; + } + + if (hmap_is_empty(&rt_data->tracked_dp_bindings)) { + goto out; + } + + struct sset *local_b_lports = binding_collect_local_binding_lports( + &rt_data->lbinding_data); + + const struct sbrec_port_group *pg_sb; + SBREC_PORT_GROUP_TABLE_FOR_EACH (pg_sb, pg_table) { + + struct sset *pg_lports = shash_find_data(&pg->port_group_ssets, + pg_sb->name); + ovs_assert(pg_lports); + + struct tracked_binding_datapath *tdp; + bool need_update = false; + HMAP_FOR_EACH (tdp, node, &rt_data->tracked_dp_bindings) { + struct shash_node *shash_node; + SHASH_FOR_EACH (shash_node, &tdp->lports) { + struct tracked_binding_lport *lport = shash_node->data; + if (sset_contains(pg_lports, lport->pb->logical_port)) { + /* At least one local port-binding change is related to the + * port_group, so the port_group_cs_local needs update. */ + need_update = true; + break; + } + } + if (need_update) { + break; + } + } + if (need_update) { + expr_const_sets_add(&pg->port_groups_cs_local, pg_sb->name, + (const char *const *) pg_sb->ports, + pg_sb->n_ports, false, local_b_lports); + sset_add(&pg->updated, pg_sb->name); + } + } + + sset_destroy(local_b_lports); + +out: + if (!sset_is_empty(&pg->new) || !sset_is_empty(&pg->deleted) || + !sset_is_empty(&pg->updated)) { + engine_set_node_state(node, EN_UPDATED); + } else { + engine_set_node_state(node, EN_UNCHANGED); + } + pg->change_tracked = true; + return true; +} + /* Connection tracking zones. */ struct ed_type_ct_zones { unsigned long bitmap[BITMAP_N_LONGS(MAX_CT_ZONES)]; @@ -1880,7 +2026,7 @@ static void init_lflow_ctx(struct engine_node *node, struct ed_type_port_groups *pg_data = engine_get_input_data("port_groups", node); - struct shash *port_groups = &pg_data->port_groups; + struct shash *port_groups = &pg_data->port_groups_cs_local; l_ctx_in->sbrec_multicast_group_by_name_datapath = sbrec_mc_group_by_name_dp; @@ -2540,7 +2686,7 @@ main(int argc, char *argv[]) "physical_flow_changes"); ENGINE_NODE(flow_output, "flow_output"); ENGINE_NODE(addr_sets, "addr_sets"); - ENGINE_NODE(port_groups, "port_groups"); + ENGINE_NODE_WITH_CLEAR_TRACK_DATA(port_groups, "port_groups"); #define SB_NODE(NAME, NAME_STR) ENGINE_NODE_SB(NAME, NAME_STR); SB_NODES @@ -2556,6 +2702,10 @@ main(int argc, char *argv[]) addr_sets_sb_address_set_handler); engine_add_input(&en_port_groups, &en_sb_port_group, port_groups_sb_port_group_handler); + /* port_groups computation requires runtime_data's lbinding_data for the + * locally bound ports. */ + engine_add_input(&en_port_groups, &en_runtime_data, + port_groups_runtime_data_handler); /* Engine node physical_flow_changes indicates whether * we can recompute only physical flows or we can @@ -2986,7 +3136,7 @@ main(int argc, char *argv[]) engine_get_data(&en_port_groups); if (br_int && chassis && as_data && pg_data) { char *error = ofctrl_inject_pkt(br_int, pending_pkt.flow_s, - &as_data->addr_sets, &pg_data->port_groups); + &as_data->addr_sets, &pg_data->port_groups_cs_local); if (error) { unixctl_command_reply_error(pending_pkt.conn, error); free(error); diff --git a/include/ovn/expr.h b/include/ovn/expr.h index 032370058..2f7ecbc9c 100644 --- a/include/ovn/expr.h +++ b/include/ovn/expr.h @@ -547,7 +547,7 @@ void expr_constant_set_destroy(struct expr_constant_set *cs); void expr_const_sets_add(struct shash *const_sets, const char *name, const char * const *values, size_t n_values, - bool convert_to_integer); + bool convert_to_integer, const struct sset *filter); void expr_const_sets_remove(struct shash *const_sets, const char *name); void expr_const_sets_destroy(struct shash *const_sets); diff --git a/lib/expr.c b/lib/expr.c index f061a8fbe..b42d78d3e 100644 --- a/lib/expr.c +++ b/lib/expr.c @@ -1065,7 +1065,7 @@ expr_constant_set_destroy(struct expr_constant_set *cs) void expr_const_sets_add(struct shash *const_sets, const char *name, const char *const *values, size_t n_values, - bool convert_to_integer) + bool convert_to_integer, const struct sset *filter) { /* Replace any existing entry for this name. */ expr_const_sets_remove(const_sets, name); @@ -1075,6 +1075,7 @@ expr_const_sets_add(struct shash *const_sets, const char *name, cs->n_values = 0; cs->values = xmalloc(n_values * sizeof *cs->values); if (convert_to_integer) { + ovs_assert(!filter); cs->type = EXPR_C_INTEGER; for (size_t i = 0; i < n_values; i++) { /* Use the lexer to convert each constant set into the proper @@ -1100,6 +1101,11 @@ expr_const_sets_add(struct shash *const_sets, const char *name, } else { cs->type = EXPR_C_STRING; for (size_t i = 0; i < n_values; i++) { + if (filter && !sset_find(filter, values[i])) { + VLOG_DBG("Skip constant set entry '%s' for '%s'", + values[i], name); + continue; + } union expr_constant *c = &cs->values[cs->n_values++]; c->string = xstrdup(values[i]); } diff --git a/tests/ovn.at b/tests/ovn.at index 55444bbd7..aa1d1cf17 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -26265,3 +26265,56 @@ primary lport : [[sw0p2]] OVN_CLEANUP([hv1], [hv2]) AT_CLEANUP ]) + +# Tests the efficiency of conjunction flow generation when port groups are used +# by ACLs. Make sure there is no open flow explosion in table 45 for an ACL +# with self-referencing match condition of a port group: +# +# "outport == @pg1 && ip4.src == $pg1_ip4" +# +# 10 LSes (ls[0-9]), each has 2 LSPs (lsp[0-9][01]). All LSPs belong to port +# group pg1, but only LSPs of LS0 are bound on HV1. +# +# The expected number of conjunction flows is 2 + 20 = 22. +OVN_FOR_EACH_NORTHD([ +AT_SETUP([ovn -- ACL with Port Group conjunction flow efficiency]) +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 + +ovn-nbctl lr-add lr0 + +for i in $(seq 0 9); do + ovn-nbctl ls-add ls$i + ovn-nbctl lrp-add lr0 lrp_lr0_ls$i aa:bb:bb:00:00:0$i 192.168.${i}.1/24 + + ovn-nbctl lsp-add ls$i lsp_ls${i}_lr0 -- \ + lsp-set-addresses lsp_ls${i}_lr0 router -- \ + lsp-set-type lsp_ls${i}_lr0 router -- \ + lsp-set-options lsp_ls${i}_lr0 router-port=lrp_lr0_ls${i} + + for j in 0 1; do + ovn-nbctl lsp-add ls$i lsp${i}-${j} -- \ + lsp-set-addresses lsp${i}-${j} "aa:aa:aa:00:0$i:0$j 192.168.$i.1$j" + done +done + +ovn-nbctl pg-add pg1 +ovn-nbctl pg-set-ports pg1 $(for i in 0 1 2 3 4 5 6 7 8 9; do for j in 0 1; do echo lsp${i}-${j}; done; done) + +ovn-nbctl --type=port-group acl-add pg1 to-lport 100 "outport == @pg1 && ip4.src == \$pg1_ip4" allow + +ovs-vsctl add-port br-int lsp0-0 -- set interface lsp0-0 external_ids:iface-id=lsp0-0 +ovs-vsctl add-port br-int lsp0-1 -- set interface lsp0-1 external_ids:iface-id=lsp0-1 + +check ovn-nbctl --wait=hv sync +AT_CHECK([test $(ovs-ofctl dump-flows br-int table=45 | grep conjunction | wc -l) == 22]) + +OVN_CLEANUP([hv1]) +AT_CLEANUP +]) diff --git a/tests/test-ovn.c b/tests/test-ovn.c index 202a96c5d..55dbbb6b4 100644 --- a/tests/test-ovn.c +++ b/tests/test-ovn.c @@ -227,10 +227,10 @@ create_addr_sets(struct shash *addr_sets) }; static const char *const addrs4[] = { NULL }; - expr_const_sets_add(addr_sets, "set1", addrs1, 3, true); - expr_const_sets_add(addr_sets, "set2", addrs2, 3, true); - expr_const_sets_add(addr_sets, "set3", addrs3, 3, true); - expr_const_sets_add(addr_sets, "set4", addrs4, 0, true); + expr_const_sets_add(addr_sets, "set1", addrs1, 3, true, NULL); + expr_const_sets_add(addr_sets, "set2", addrs2, 3, true, NULL); + expr_const_sets_add(addr_sets, "set3", addrs3, 3, true, NULL); + expr_const_sets_add(addr_sets, "set4", addrs4, 0, true, NULL); } static void @@ -243,8 +243,8 @@ create_port_groups(struct shash *port_groups) }; static const char *const pg2[] = { NULL }; - expr_const_sets_add(port_groups, "0_pg1", pg1, 3, false); - expr_const_sets_add(port_groups, "0_pg_empty", pg2, 0, false); + expr_const_sets_add(port_groups, "0_pg1", pg1, 3, false, NULL); + expr_const_sets_add(port_groups, "0_pg_empty", pg2, 0, false, NULL); } static bool diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c index 6b883886f..63b640db9 100644 --- a/utilities/ovn-trace.c +++ b/utilities/ovn-trace.c @@ -820,7 +820,7 @@ read_address_sets(void) SBREC_ADDRESS_SET_FOR_EACH (sbas, ovnsb_idl) { expr_const_sets_add(&address_sets, sbas->name, (const char *const *) sbas->addresses, - sbas->n_addresses, true); + sbas->n_addresses, true, NULL); } } @@ -833,7 +833,7 @@ read_port_groups(void) SBREC_PORT_GROUP_FOR_EACH (sbpg, ovnsb_idl) { expr_const_sets_add(&port_groups, sbpg->name, (const char *const *) sbpg->ports, - sbpg->n_ports, false); + sbpg->n_ports, false, NULL); } }