From patchwork Tue Aug 29 08:47:53 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ales Musil X-Patchwork-Id: 1827146 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.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=H50gQWJ1; dkim-atps=neutral Authentication-Results: legolas.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=patchwork.ozlabs.org) 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 ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4RZh081yC1z1yZ9 for ; Tue, 29 Aug 2023 18:48:08 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 4329F60ECA; Tue, 29 Aug 2023 08:48:06 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 4329F60ECA Authentication-Results: smtp3.osuosl.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=H50gQWJ1 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 2vmz8PzAsbMt; Tue, 29 Aug 2023 08:48:04 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp3.osuosl.org (Postfix) with ESMTPS id 5976D60EC5; Tue, 29 Aug 2023 08:48:03 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 5976D60EC5 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 40805C0DD5; Tue, 29 Aug 2023 08:48:02 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 3ABDBC0032 for ; Tue, 29 Aug 2023 08:48:00 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 148894017A for ; Tue, 29 Aug 2023 08:48:00 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 148894017A Authentication-Results: smtp4.osuosl.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=H50gQWJ1 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 1nzHUa38en0A for ; Tue, 29 Aug 2023 08:47:59 +0000 (UTC) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by smtp4.osuosl.org (Postfix) with ESMTPS id C146A40156 for ; Tue, 29 Aug 2023 08:47:58 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org C146A40156 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1693298877; 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=voL3c1z8e+zWd0ZDTOiR+moahNWs/dyJSdGwtfTAzZI=; b=H50gQWJ180p6XsWZ+MSYEU7A4zsGQQ2ssbWN/1Y+3rv/4ZdItbddIqogGBCKasOWvL1tlV 836creboFHMLdX04Wwl8ke8ZT5ywLnMUG12csohXOGVKv5fH28QJOc9/ryEAnw918XtIWY tjxEFo0W7gic1SEBuL+3kA2zQ9+L40I= Received: from mimecast-mx02.redhat.com (66.187.233.73 [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-32-Wn3vxohRMIS14rR6Ae3cAQ-1; Tue, 29 Aug 2023 04:47:55 -0400 X-MC-Unique: Wn3vxohRMIS14rR6Ae3cAQ-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 75AA7280BC41 for ; Tue, 29 Aug 2023 08:47:55 +0000 (UTC) Received: from amusil.redhat.com (unknown [10.45.224.117]) by smtp.corp.redhat.com (Postfix) with ESMTP id AB1E95CC01; Tue, 29 Aug 2023 08:47:54 +0000 (UTC) From: Ales Musil To: dev@openvswitch.org Date: Tue, 29 Aug 2023 10:47:53 +0200 Message-ID: <20230829084753.209210-2-amusil@redhat.com> In-Reply-To: <20230829084753.209210-1-amusil@redhat.com> References: <20230829084753.209210-1-amusil@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.5 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn v2 2/2] ofctrl: Prevent conjunction duplication 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" During ofctrl_add_or_append_flow we are able to combine two flows with same match but different conjunctions. However, the function didn't check if the conjunctions already exist in the installed flow, which could result in conjunction duplication and the flow would grow infinitely e.g. actions=conjunction(1,1/2), conjunction(1,1/2) Make sure that we add only conjunctions that are not present in the already existing flow. Reported-at: https://bugzilla.redhat.com/2175928 Signed-off-by: Ales Musil Acked-by: Mark Michelson --- controller/ofctrl.c | 56 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/controller/ofctrl.c b/controller/ofctrl.c index 9de8a145f..e39cef7aa 100644 --- a/controller/ofctrl.c +++ b/controller/ofctrl.c @@ -1273,6 +1273,37 @@ ofctrl_add_flow_metered(struct ovn_desired_flow_table *desired_flows, meter_id, as_info, true); } +struct ofpact_ref { + struct hmap_node hmap_node; + struct ofpact *ofpact; +}; + +static struct ofpact_ref * +ofpact_ref_find(const struct hmap *refs, const struct ofpact *ofpact) +{ + uint32_t hash = hash_bytes(ofpact, ofpact->len, 0); + + struct ofpact_ref *ref; + HMAP_FOR_EACH_WITH_HASH (ref, hmap_node, hash, refs) { + if (ofpacts_equal(ref->ofpact, ref->ofpact->len, + ofpact, ofpact->len)) { + return ref; + } + } + + return NULL; +} + +static void +ofpact_refs_destroy(struct hmap *refs) +{ + struct ofpact_ref *ref; + HMAP_FOR_EACH_POP (ref, hmap_node, refs) { + free(ref); + } + hmap_destroy(refs); +} + /* Either add a new flow, or append actions on an existing flow. If the * flow existed, a new link will also be created between the new sb_uuid * and the existing flow. */ @@ -1292,6 +1323,21 @@ ofctrl_add_or_append_flow(struct ovn_desired_flow_table *desired_flows, meter_id); existing = desired_flow_lookup_conjunctive(desired_flows, &f->flow); if (existing) { + struct hmap existing_conj = HMAP_INITIALIZER(&existing_conj); + + struct ofpact *ofpact; + OFPACT_FOR_EACH (ofpact, existing->flow.ofpacts, + existing->flow.ofpacts_len) { + if (ofpact->type != OFPACT_CONJUNCTION) { + continue; + } + + struct ofpact_ref *ref = xmalloc(sizeof *ref); + ref->ofpact = ofpact; + uint32_t hash = hash_bytes(ofpact, ofpact->len, 0); + hmap_insert(&existing_conj, &ref->hmap_node, hash); + } + /* There's already a flow with this particular match and action * 'conjunction'. Append the action to that flow rather than * adding a new flow. @@ -1301,7 +1347,15 @@ ofctrl_add_or_append_flow(struct ovn_desired_flow_table *desired_flows, ofpbuf_use_stub(&compound, compound_stub, sizeof(compound_stub)); ofpbuf_put(&compound, existing->flow.ofpacts, existing->flow.ofpacts_len); - ofpbuf_put(&compound, f->flow.ofpacts, f->flow.ofpacts_len); + + OFPACT_FOR_EACH (ofpact, f->flow.ofpacts, f->flow.ofpacts_len) { + if (ofpact->type != OFPACT_CONJUNCTION || + !ofpact_ref_find(&existing_conj, ofpact)) { + ofpbuf_put(&compound, ofpact, OFPACT_ALIGN(ofpact->len)); + } + } + + ofpact_refs_destroy(&existing_conj); mem_stats.desired_flow_usage -= desired_flow_size(existing); free(existing->flow.ofpacts);