From patchwork Wed May 20 19:40:01 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1294687 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.133; helo=hemlock.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 hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 49S34Z5vMqz9sT9 for ; Thu, 21 May 2020 05:40:14 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 270DC88C6C; Wed, 20 May 2020 19:40:13 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id XS8Fm-kuvNpe; Wed, 20 May 2020 19:40:12 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id E7DB388C43; Wed, 20 May 2020 19:40:11 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id CE647C07FF; Wed, 20 May 2020 19:40:11 +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 C6544C0176 for ; Wed, 20 May 2020 19:40:09 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id C253086B48 for ; Wed, 20 May 2020 19:40:09 +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 x4jp6k5cKRtI for ; Wed, 20 May 2020 19:40:08 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay12.mail.gandi.net (relay12.mail.gandi.net [217.70.178.232]) by whitealder.osuosl.org (Postfix) with ESMTPS id 3F87A88366 for ; Wed, 20 May 2020 19:40:08 +0000 (UTC) Received: from nusiddiq.home.org.home.org (unknown [125.99.233.239]) (Authenticated sender: numans@ovn.org) by relay12.mail.gandi.net (Postfix) with ESMTPSA id 46A50200005; Wed, 20 May 2020 19:40:03 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 21 May 2020 01:10:01 +0530 Message-Id: <20200520194001.2351927-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200520193918.2351632-1-numans@ovn.org> References: <20200520193918.2351632-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v7 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). Acked-by: Mark Michelson 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;