From patchwork Sun Oct 11 12:05:31 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1380313 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=fail (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Q6zHk1kK; dkim-atps=neutral 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 4C8L9t1xZdz9sS8 for ; Sun, 11 Oct 2020 23:05:52 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 75DB9204AB; Sun, 11 Oct 2020 12:05:50 +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 msk-t4iTfhQg; Sun, 11 Oct 2020 12:05:41 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by silver.osuosl.org (Postfix) with ESMTP id B393220482; Sun, 11 Oct 2020 12:05:41 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 93954C07FF; Sun, 11 Oct 2020 12:05:41 +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 7EF0AC0051 for ; Sun, 11 Oct 2020 12:05:40 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 6DEEC86B31 for ; Sun, 11 Oct 2020 12:05:40 +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 5xdKbYucOFaz for ; Sun, 11 Oct 2020 12:05:39 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by fraxinus.osuosl.org (Postfix) with ESMTPS id 4F71886A47 for ; Sun, 11 Oct 2020 12:05:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1602417938; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=0hEChl73AQ9aZu0GE+wX/jKTvnimAPIYhslzqnBN98U=; b=Q6zHk1kKgS/JNNL8Qfrxe24UlWpRmXvR6ih+mZ+fnAK30T6lDrjLE2Ykfvjbh7LuJJmtBk 0w7xqEI83v0/8TNDvmSD9hvQHTlmMVUGgOOPv5Kv1xfTWgoNq8NZaWVJyFo/s3+o3AOA9L UM6uRArYbfercP4f9ouLTJEAqxTESrg= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-104-kmwYQkXcNH2tKh3lnmGt2g-1; Sun, 11 Oct 2020 08:05:36 -0400 X-MC-Unique: kmwYQkXcNH2tKh3lnmGt2g-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 2DE1F56BE2; Sun, 11 Oct 2020 12:05:35 +0000 (UTC) Received: from dceara.remote.csb (ovpn-112-46.ams2.redhat.com [10.36.112.46]) by smtp.corp.redhat.com (Postfix) with ESMTP id 268815D9DC; Sun, 11 Oct 2020 12:05:33 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Sun, 11 Oct 2020 14:05:31 +0200 Message-Id: <20201011120522.3374.57876.stgit@dceara.remote.csb> In-Reply-To: <20201011120456.3374.30821.stgit@dceara.remote.csb> References: <20201011120456.3374.30821.stgit@dceara.remote.csb> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dceara@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Cc: hzhou@ovn.org Subject: [ovs-dev] [PATCH v5 ovn 1/4] ofctrl.c: Only merge actions for conjunctive 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" In ofctrl_add_or_append_flow() when merging flow actions make sure we only do that for conjunctive flows. All other actions can not be merged with action "conjunction". CC: Mark Michelson Fixes: e659bab31a91 ("Combine conjunctions with identical matches into one flow.") Signed-off-by: Dumitru Ceara --- controller/ofctrl.c | 124 +++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 99 insertions(+), 25 deletions(-) diff --git a/controller/ofctrl.c b/controller/ofctrl.c index 4425d98..24b55fc 100644 --- a/controller/ofctrl.c +++ b/controller/ofctrl.c @@ -206,6 +206,9 @@ struct installed_flow { struct desired_flow *desired_flow; }; +typedef bool +(*desired_flow_match_cb)(const struct desired_flow *candidate, + const void *arg); static struct desired_flow *desired_flow_alloc( uint8_t table_id, uint16_t priority, @@ -214,8 +217,14 @@ static struct desired_flow *desired_flow_alloc( const struct ofpbuf *actions); static struct desired_flow *desired_flow_lookup( struct ovn_desired_flow_table *, + const struct ovn_flow *target); +static struct desired_flow *desired_flow_lookup_check_uuid( + struct ovn_desired_flow_table *, const struct ovn_flow *target, - const struct uuid *sb_uuid); + const struct uuid *); +static struct desired_flow *desired_flow_lookup_conjunctive( + struct ovn_desired_flow_table *, + const struct ovn_flow *target); static void desired_flow_destroy(struct desired_flow *); static struct installed_flow *installed_flow_lookup( @@ -806,6 +815,19 @@ desired_flow_set_active(struct desired_flow *d) d->installed_flow->desired_flow = d; } +static bool +flow_action_has_conj(const struct ovn_flow *f) +{ + const struct ofpact *a = NULL; + + OFPACT_FOR_EACH (a, f->ofpacts, f->ofpacts_len) { + if (a->type == OFPACT_CONJUNCTION) { + return true; + } + } + return false; +} + /* Adds the desired flow to the list of desired flows that have same match * conditions as the installed flow. * @@ -962,7 +984,7 @@ ofctrl_check_and_add_flow(struct ovn_desired_flow_table *flow_table, struct desired_flow *f = desired_flow_alloc(table_id, priority, cookie, match, actions); - if (desired_flow_lookup(flow_table, &f->flow, sb_uuid)) { + if (desired_flow_lookup_check_uuid(flow_table, &f->flow, sb_uuid)) { if (log_duplicate_flow) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5); if (!VLOG_DROP_DBG(&rl)) { @@ -1002,14 +1024,15 @@ ofctrl_add_or_append_flow(struct ovn_desired_flow_table *desired_flows, const struct ofpbuf *actions, const struct uuid *sb_uuid) { - struct desired_flow *f = desired_flow_alloc(table_id, priority, cookie, - match, actions); - struct desired_flow *existing; - existing = desired_flow_lookup(desired_flows, &f->flow, NULL); + struct desired_flow *f; + + f = desired_flow_alloc(table_id, priority, cookie, match, actions); + existing = desired_flow_lookup_conjunctive(desired_flows, &f->flow); if (existing) { - /* There's already a flow with this particular match. Append the - * action to that flow rather than adding a new flow + /* There's already a flow with this particular match and action + * 'conjunction'. Append the action to that flow rather than + * adding a new flow. */ uint64_t compound_stub[64 / 8]; struct ofpbuf compound; @@ -1248,15 +1271,11 @@ installed_flow_dup(struct desired_flow *src) return dst; } -/* Finds and returns a desired_flow in 'flow_table' whose key is identical to - * 'target''s key, or NULL if there is none. - * - * If sb_uuid is not NULL, the function will also check if the found flow is - * referenced by the sb_uuid. */ static struct desired_flow * -desired_flow_lookup(struct ovn_desired_flow_table *flow_table, - const struct ovn_flow *target, - const struct uuid *sb_uuid) +desired_flow_lookup__(struct ovn_desired_flow_table *flow_table, + const struct ovn_flow *target, + desired_flow_match_cb match_cb, + const void *arg) { struct desired_flow *d; HMAP_FOR_EACH_WITH_HASH (d, match_hmap_node, target->hash, @@ -1265,20 +1284,76 @@ desired_flow_lookup(struct ovn_desired_flow_table *flow_table, if (f->table_id == target->table_id && f->priority == target->priority && minimatch_equal(&f->match, &target->match)) { - if (!sb_uuid) { + + if (!match_cb || match_cb(d, arg)) { return d; } - struct sb_flow_ref *sfr; - LIST_FOR_EACH (sfr, sb_list, &d->references) { - if (uuid_equals(sb_uuid, &sfr->sb_uuid)) { - return d; - } - } } } return NULL; } +/* Finds and returns a desired_flow in 'flow_table' whose key is identical to + * 'target''s key, or NULL if there is none. + */ +static struct desired_flow * +desired_flow_lookup(struct ovn_desired_flow_table *flow_table, + const struct ovn_flow *target) +{ + return desired_flow_lookup__(flow_table, target, NULL, NULL); +} + +static bool +flow_lookup_match_uuid_cb(const struct desired_flow *candidate, + const void *arg) +{ + const struct uuid *sb_uuid = arg; + struct sb_flow_ref *sfr; + + LIST_FOR_EACH (sfr, sb_list, &candidate->references) { + if (uuid_equals(sb_uuid, &sfr->sb_uuid)) { + return true; + } + } + return false; +} + +/* Finds and returns a desired_flow in 'flow_table' whose key is identical to + * 'target''s key, or NULL if there is none. + * + * The function will also check if the found flow is referenced by the + * 'sb_uuid'. + */ +static struct desired_flow * +desired_flow_lookup_check_uuid(struct ovn_desired_flow_table *flow_table, + const struct ovn_flow *target, + const struct uuid *sb_uuid) +{ + return desired_flow_lookup__(flow_table, target, flow_lookup_match_uuid_cb, + sb_uuid); +} + +static bool +flow_lookup_match_conj_cb(const struct desired_flow *candidate, + const void *arg OVS_UNUSED) +{ + return flow_action_has_conj(&candidate->flow); +} + +/* Finds and returns a desired_flow in 'flow_table' whose key is identical to + * 'target''s key, or NULL if there is none. + * + * The function will only return a matching flow if it contains action + * 'conjunction'. + */ +static struct desired_flow * +desired_flow_lookup_conjunctive(struct ovn_desired_flow_table *flow_table, + const struct ovn_flow *target) +{ + return desired_flow_lookup__(flow_table, target, flow_lookup_match_conj_cb, + NULL); +} + /* Finds and returns an installed_flow in installed_flows whose key is * identical to 'target''s key, or NULL if there is none. */ static struct installed_flow * @@ -1676,8 +1751,7 @@ update_installed_flows_by_compare(struct ovn_desired_flow_table *flow_table, struct installed_flow *i, *next; HMAP_FOR_EACH_SAFE (i, next, match_hmap_node, &installed_flows) { unlink_all_refs_for_installed_flow(i); - struct desired_flow *d = - desired_flow_lookup(flow_table, &i->flow, NULL); + struct desired_flow *d = desired_flow_lookup(flow_table, &i->flow); if (!d) { /* Installed flow is no longer desirable. Delete it from the * switch and from installed_flows. */