From patchwork Sat May 16 17:56:47 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1291948 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 49PXzb532lz9sTT for ; Sun, 17 May 2020 03:57:15 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 0DE64227FC; Sat, 16 May 2020 17:57:14 +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 6ZyyWK4xhl6s; Sat, 16 May 2020 17:57:04 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by silver.osuosl.org (Postfix) with ESMTP id 9FBBF20532; Sat, 16 May 2020 17:57:00 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 923DCC088F; Sat, 16 May 2020 17:57:00 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 4F8EDC016F for ; Sat, 16 May 2020 17:56:59 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 48E0D873A3 for ; Sat, 16 May 2020 17:56:59 +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 ryBcznyla9-v for ; Sat, 16 May 2020 17:56:58 +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 fraxinus.osuosl.org (Postfix) with ESMTPS id 98E4C874AB for ; Sat, 16 May 2020 17:56:57 +0000 (UTC) X-Originating-IP: 115.99.184.184 Received: from nusiddiq.home.org.com (unknown [115.99.184.184]) (Authenticated sender: numans@ovn.org) by relay3-d.mail.gandi.net (Postfix) with ESMTPSA id 4CB3160002; Sat, 16 May 2020 17:56:52 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Sat, 16 May 2020 23:26:47 +0530 Message-Id: <20200516175647.4003845-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200516175539.4003530-1-numans@ovn.org> References: <20200516175539.4003530-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v6 4/9] ofctrl: Replace the actions of an existing flow if actions have changed. 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 If ofctrl_check_and_add_flow(F') is called where flow F' has match-actions (M, A2) and if there already exists a flow F with match-actions (M, A1) in the desired flow table, then (1) If F and F' share the same 'sb_uuid', this function doesn't update the existing flow F with new actions A2. (2) If F and F' don't share the same 'sb_uuid', this function adds F' also into the flow table with F and F' having the same hash. While installing the flows only one will be considered. This patch fixes (1) by updating the F with the new actions A2. This is required for the upcoming patch in this series which adds incremental processing for OVS interface in the flow output stage. Since we will be not be clearing the flow output data if these changes are handled incrementally, some of the existing flows will be updated with new actions. One such example would be flows in physical table OFTABLE_LOCAL_OUTPUT (table 33). This patch is required to update such flows. Otherwise we will have incomplete actions installed. We should also address (2) and it should be fixed - by logging the duplicate flow F' - and discarding F'. Scenario (2) can happen when - either ovn-northd installs 2 logical flows with same match but with different actions due to some bug in ovn-northd - CMS adds 2 ACLs with the same match and priority, but with different actions. However this patch doesn't attempt to fix (2). Signed-off-by: Numan Siddique --- controller/ofctrl.c | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/controller/ofctrl.c b/controller/ofctrl.c index b8a9c2da8..edc22824f 100644 --- a/controller/ofctrl.c +++ b/controller/ofctrl.c @@ -642,6 +642,19 @@ ofctrl_recv(const struct ofp_header *oh, enum ofptype type) } } +static void +ofctrl_swap_flow_actions(struct ovn_flow *a, struct ovn_flow *b) +{ + struct ofpact *tmp = a->ofpacts; + size_t tmp_len = a->ofpacts_len; + + a->ofpacts = b->ofpacts; + a->ofpacts_len = b->ofpacts_len; + + b->ofpacts = tmp; + b->ofpacts_len = tmp_len; +} + /* Flow table interfaces to the rest of ovn-controller. */ /* Adds a flow to 'desired_flows' with the specified 'match' and 'actions' to @@ -667,14 +680,21 @@ ofctrl_check_and_add_flow(struct ovn_desired_flow_table *flow_table, ovn_flow_log(f, "ofctrl_add_flow"); - if (ovn_flow_lookup(&flow_table->match_flow_table, f, true)) { - if (log_duplicate_flow) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5); - if (!VLOG_DROP_DBG(&rl)) { - char *s = ovn_flow_to_string(f); - VLOG_DBG("dropping duplicate flow: %s", s); - free(s); + struct ovn_flow *existing_f = + ovn_flow_lookup(&flow_table->match_flow_table, f, true); + if (existing_f) { + if (ofpacts_equal(f->ofpacts, f->ofpacts_len, + existing_f->ofpacts, existing_f->ofpacts_len)) { + if (log_duplicate_flow) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5); + if (!VLOG_DROP_DBG(&rl)) { + char *s = ovn_flow_to_string(f); + VLOG_DBG("dropping duplicate flow: %s", s); + free(s); + } } + } else { + ofctrl_swap_flow_actions(f, existing_f); } ovn_flow_destroy(f); return;