{"id":2095634,"url":"http://patchwork.ozlabs.org/api/patches/2095634/?format=json","web_url":"http://patchwork.ozlabs.org/project/ovn/patch/20250609173539.1636916-10-mmichels@redhat.com/","project":{"id":68,"url":"http://patchwork.ozlabs.org/api/projects/68/?format=json","name":"Open Virtual Network development","link_name":"ovn","list_id":"ovs-dev.openvswitch.org","list_email":"ovs-dev@openvswitch.org","web_url":"http://openvswitch.org/","scm_url":"","webscm_url":"","list_archive_url":"","list_archive_url_format":"","commit_url_format":""},"msgid":"<20250609173539.1636916-10-mmichels@redhat.com>","list_archive_url":null,"date":"2025-06-09T17:35:28","name":"[ovs-dev,09/14] lflow_ref: Allow multiple datapaths per lflow_ref_node.","commit_ref":null,"pull_url":null,"state":"deferred","archived":false,"hash":"5531df6b8d9d7cbf143e0062fad3e855641b9f85","submitter":{"id":71978,"url":"http://patchwork.ozlabs.org/api/people/71978/?format=json","name":"Mark Michelson","email":"mmichels@redhat.com"},"delegate":null,"mbox":"http://patchwork.ozlabs.org/project/ovn/patch/20250609173539.1636916-10-mmichels@redhat.com/mbox/","series":[{"id":460139,"url":"http://patchwork.ozlabs.org/api/series/460139/?format=json","web_url":"http://patchwork.ozlabs.org/project/ovn/list/?series=460139","date":"2025-06-09T17:35:19","name":"Logical Flow Sync Refactor.","version":1,"mbox":"http://patchwork.ozlabs.org/series/460139/mbox/"}],"comments":"http://patchwork.ozlabs.org/api/patches/2095634/comments/","check":"success","checks":"http://patchwork.ozlabs.org/api/patches/2095634/checks/","tags":{},"related":[],"headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@legolas.ozlabs.org","ovs-dev@lists.linuxfoundation.org"],"Authentication-Results":["legolas.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256\n header.s=mimecast20190719 header.b=RYewbQN/;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org\n (client-ip=140.211.166.136; helo=smtp3.osuosl.org;\n envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org)","smtp3.osuosl.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key)\n header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256\n header.s=mimecast20190719 header.b=RYewbQN/","smtp2.osuosl.org; dmarc=pass (p=quarantine dis=none)\n header.from=redhat.com","smtp2.osuosl.org;\n dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com\n header.a=rsa-sha256 header.s=mimecast20190719 header.b=RYewbQN/"],"Received":["from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4bGJx444wCz1yDx\n\tfor ; Tue, 10 Jun 2025 03:35:51 +1000 (AEST)","from localhost (localhost [127.0.0.1])\n\tby smtp3.osuosl.org (Postfix) with ESMTP id B5297614BA;\n\tMon, 9 Jun 2025 17:36:02 +0000 (UTC)","from smtp3.osuosl.org ([127.0.0.1])\n by localhost (smtp3.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP\n id bf8qVwmnQcMu; Mon, 9 Jun 2025 17:36:00 +0000 (UTC)","from lists.linuxfoundation.org (lf-lists.osuosl.org\n [IPv6:2605:bc80:3010:104::8cd3:938])\n\tby smtp3.osuosl.org (Postfix) with ESMTPS id A08B3614B4;\n\tMon, 9 Jun 2025 17:35:57 +0000 (UTC)","from lf-lists.osuosl.org (localhost [127.0.0.1])\n\tby lists.linuxfoundation.org (Postfix) with ESMTP id 745C2C0AC5;\n\tMon, 9 Jun 2025 17:35:57 +0000 (UTC)","from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.133])\n by lists.linuxfoundation.org (Postfix) with ESMTP id 0EB78C0AC5\n for ; Mon, 9 Jun 2025 17:35:56 +0000 (UTC)","from localhost (localhost [127.0.0.1])\n by smtp2.osuosl.org (Postfix) with ESMTP id 247CC41DDA\n for ; Mon, 9 Jun 2025 17:35:54 +0000 (UTC)","from smtp2.osuosl.org ([127.0.0.1])\n by localhost (smtp2.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP\n id XVhd7nMSmRlb for ;\n Mon, 9 Jun 2025 17:35:52 +0000 (UTC)","from us-smtp-delivery-124.mimecast.com\n (us-smtp-delivery-124.mimecast.com [170.10.129.124])\n by smtp2.osuosl.org (Postfix) with ESMTPS id 0E79D41D74\n for ; Mon, 9 Jun 2025 17:35:50 +0000 (UTC)","from mx-prod-mc-04.mail-002.prod.us-west-2.aws.redhat.com\n (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by\n relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3,\n cipher=TLS_AES_256_GCM_SHA384) id us-mta-609-TJJ8OMADOE602fwnofmBRA-1; Mon,\n 09 Jun 2025 13:35:48 -0400","from mx-prod-int-08.mail-002.prod.us-west-2.aws.redhat.com\n (mx-prod-int-08.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.111])\n (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest\n SHA256)\n (No client certificate requested)\n by mx-prod-mc-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS\n id 0F37619560B0\n for ; Mon, 9 Jun 2025 17:35:48 +0000 (UTC)","from localhost.localdomain.com (unknown [10.22.58.15])\n by mx-prod-int-08.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP\n id 92AAC1800284\n for ; Mon, 9 Jun 2025 17:35:47 +0000 (UTC)"],"X-Virus-Scanned":["amavis at osuosl.org","amavis at osuosl.org"],"X-Comment":"SPF check N/A for local connections -\n client-ip=2605:bc80:3010:104::8cd3:938; helo=lists.linuxfoundation.org;\n envelope-from=ovs-dev-bounces@openvswitch.org; receiver= ","DKIM-Filter":["OpenDKIM Filter v2.11.0 smtp3.osuosl.org A08B3614B4","OpenDKIM Filter v2.11.0 smtp2.osuosl.org 0E79D41D74"],"Received-SPF":"Pass (mailfrom) identity=mailfrom; client-ip=170.10.129.124;\n helo=us-smtp-delivery-124.mimecast.com; envelope-from=mmichels@redhat.com;\n receiver=","DMARC-Filter":"OpenDMARC Filter v1.4.2 smtp2.osuosl.org 0E79D41D74","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n s=mimecast20190719; t=1749490550;\n h=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n to:to:cc:mime-version:mime-version:content-type:content-type:\n content-transfer-encoding:content-transfer-encoding:\n in-reply-to:in-reply-to:references:references;\n bh=WsZmzjR+ohWQO+1bQ6kLTH9/C5vuWGJsg56Cgb/5Xb8=;\n b=RYewbQN//2i/ZpVZYHa3QvvADyBb5UEbeZlG/uyEaONlcvAJuvtE/wItbEaEwY82AeB7xv\n Q07CCNNFI2BsPKE6YxHqLF/3ceHHOiJ4kVGi/Z57IRMKX3w7gye26vLkN04iqisjHhcKjj\n v9PfjM6Gy0G5uw27A3CXwiWvvqa2uE4=","X-MC-Unique":"TJJ8OMADOE602fwnofmBRA-1","X-Mimecast-MFC-AGG-ID":"TJJ8OMADOE602fwnofmBRA_1749490548","To":"dev@openvswitch.org","Date":"Mon, 9 Jun 2025 13:35:28 -0400","Message-ID":"<20250609173539.1636916-10-mmichels@redhat.com>","In-Reply-To":"<20250609173539.1636916-1-mmichels@redhat.com>","References":"<20250609173539.1636916-1-mmichels@redhat.com>","MIME-Version":"1.0","X-Scanned-By":"MIMEDefang 3.4.1 on 10.30.177.111","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"oyEmorKjAwtP2bK059wsvGu-4fVwGoClHa4xsYVkCwU_1749490548","X-Mimecast-Originator":"redhat.com","Subject":"[ovs-dev] [PATCH ovn 09/14] lflow_ref: Allow multiple datapaths per\n lflow_ref_node.","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.30","Precedence":"list","List-Id":"","List-Unsubscribe":",\n ","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n ","From":"Mark Michelson via dev ","Reply-To":"Mark Michelson ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"ovs-dev-bounces@openvswitch.org","Sender":"\"dev\" "},"content":"Prior to this commit, an lflow_ref_node would be created the first time\nthat an lflow was added to an lflow_ref. If this lflow was added for a\nsingle datapath, then the lflow_ref_node would reference that single\ndatapath. If the lflow was added for a datapath_group, then the\nlflow_ref_node would reference all datapaths in the group.\n\nIf the same lflow were added to the same lflow_ref a second time, but\nspecified a different datapath than the first time the lflow was added,\nthen the lflow_ref_node remained unchanged, even though the lflow now\napplied to one or more new datapaths.\n\nIn most cases, this is fine, because lflows are only added once per\nlflow_ref. The IGMP lflow_ref, on the other hand, works differently.\nThere is only a single lflow_ref for all of IGMP. Therefore, it is\npossible (probable, even) for the same lflow to be added multiple times\nto the same lflow_ref, each time for a different datapath.\n\nMost lflow input node handlers start by calling lflow_ref_unlink_lflows().\nThis removes all of the datapath references from the lflow_ref and sets all\nlflow_ref_nodes within the lflow_ref to be unlinked. Then the handler re-builds\nthe relevant lflows, causing any corresponding lflow_ref_nodes to be re-linked,\nbased on the datapath the lflow was added for. Finally, the handler calls\nlflow_ref_sync_lflows(). This causes all lflows in the lflow_ref to be\nsynced with the southbound Logical_Flow table. Any lflow_ref_nodes that were\nunlinked and not re-linked will either have their lflow's corresponding\nLogical_Flow datapath-related columns updated, or they will have their\nrecord deleted entirely if the lflow no longer has any datapath references.\n\nThis particular workflow breaks in the case that the lflow was added\nmultiple times to the lflow_ref, each with a different datapath. The\nlflow_ref_node is only aware of the first datapath for which the lflow\nwas added. So when the lflow_ref_node is unlinked, only the reference to\nthat particular datapath is removed from the lflow. Therefore, the lflow\nmay still have references to datapaths in its dpg_bitmap field that\nshould have been removed when the lflow_ref_node was unlinked. After\nrebuilding the flows and re-syncing, those leftover datapaths will still\nbe in the lflow, resulting in the southbound Logical_Flow referencing\ndatapaths for which the flow should have been removed. This translates\nto the datapath in question having the lflow installed when it should\nhave been removed.\n\nThe IGMP lflow handler gets around this by calling\nlflow_ref_resync_flows() instead of lflow_ref_unlink_lflows() at the\nbeginning. This causes the lflow_ref_nodes to be unlinked and synced\nimmediately before attempting to rebuild the lflows. Since no\nlflow_ref_nodes had a chance to be re-linked, it results in all of the\nlflows in the IGMP lflow_ref being deleted (both in memory and in the\nSouthbound Logical_Flow table), then the lflows are rebuilt from scratch.\nSince the lflows are re-created entirely, there is no possibility of stray\ndatapaths being referenced.\n\nThe next change in this series is going to separate lflow_ref\nunlinking/rebuilding into one engine node and lflow_ref syncing into a\nseparate one. Because of that, the en-lflow IGMP input handler can no longer\ncall lflow_ref_resync_flows(). It needs to use lflow_ref_unlink_lflows() like\nthe other lflow input handlers do. But as stated in an earlier paragraph, this\ncauses stray datapaths to be referenced by the southbound Logical_Flow table.\n\nThis change modifies how lflow_ref_nodes handle their datapath\nreferences. lflow_ref_nodes can now have their datapath references updated\nwhen an lflow is added to an lflow_ref multiple times. To accomplish this, the\nlflow_ref_node dgrp_bitmap and dp_index fields have been combined into a\nbitmap field, dp_bitmap. Whether a single datapath or a datapath group is\nreferenced by the added lflow, we handle it the same in the lflow_ref_node, by\nusing the dp_bitmap.\n\nSigned-off-by: Mark Michelson \n---\n northd/lflow-mgr.c | 79 +++++++++++++++++++---------------------------\n 1 file changed, 33 insertions(+), 46 deletions(-)","diff":"diff --git a/northd/lflow-mgr.c b/northd/lflow-mgr.c\nindex 0f7b989cf..aa8e60478 100644\n--- a/northd/lflow-mgr.c\n+++ b/northd/lflow-mgr.c\n@@ -558,13 +558,11 @@ struct lflow_ref_node {\n /* Indicates whether the lflow was added with a dp_group using the\n * ovn_lflow_add_with_dp_group() macro. */\n bool dpgrp_lflow;\n- /* dpgrp bitmap and bitmap length. Valid only of dpgrp_lflow is true. */\n- unsigned long *dpgrp_bitmap;\n- size_t dpgrp_bitmap_len;\n-\n- /* Index id of the datapath this lflow_ref_node belongs to.\n- * Valid only if dpgrp_lflow is false. */\n- size_t dp_index;\n+ /* dp bitmap and bitmap length. Used to track all datapaths that\n+ * the lflow references\n+ */\n+ unsigned long *dp_bitmap;\n+ size_t dp_bitmap_len;\n \n /* Indicates if the lflow_ref_node for an lflow - L(M, A) is linked\n * to datapath(s) or not.\n@@ -600,10 +598,11 @@ lflow_ref_destroy(struct lflow_ref *lflow_ref)\n }\n \n /* Unlinks the lflows referenced by the 'lflow_ref'.\n- * For each lflow_ref_node (lrn) in the lflow_ref, it basically clears\n- * the datapath id (lrn->dp_index) or all the datapath id bits in the\n- * dp group bitmap (set when ovn_lflow_add_with_dp_group macro was used)\n- * from the lrn->lflow's dpg bitmap\n+ * For each lflow_ref_node (lrn) in the lflow_ref, it removes the\n+ * datapath references in the lrn. If all references are removed\n+ * for a particular datapath index, then the bit is cleared from\n+ * the lflow's dpg_bitmap, indicating the lflow no longer applies\n+ * to the corresponding datapath.\n */\n void\n lflow_ref_unlink_lflows(struct lflow_ref *lflow_ref)\n@@ -611,21 +610,12 @@ lflow_ref_unlink_lflows(struct lflow_ref *lflow_ref)\n struct lflow_ref_node *lrn;\n \n HMAP_FOR_EACH (lrn, ref_node, &lflow_ref->lflow_ref_nodes) {\n- if (lrn->dpgrp_lflow) {\n- size_t index;\n- BITMAP_FOR_EACH_1 (index, lrn->dpgrp_bitmap_len,\n- lrn->dpgrp_bitmap) {\n- if (dp_refcnt_release(&lrn->lflow->dp_refcnts_map, index)) {\n- bitmap_set0(lrn->lflow->dpg_bitmap, index);\n- }\n- }\n- } else {\n- if (dp_refcnt_release(&lrn->lflow->dp_refcnts_map,\n- lrn->dp_index)) {\n- bitmap_set0(lrn->lflow->dpg_bitmap, lrn->dp_index);\n+ size_t index;\n+ BITMAP_FOR_EACH_1 (index, lrn->dp_bitmap_len, lrn->dp_bitmap) {\n+ if (dp_refcnt_release(&lrn->lflow->dp_refcnts_map, index)) {\n+ bitmap_set0(lrn->lflow->dpg_bitmap, index);\n }\n }\n-\n lrn->linked = false;\n }\n }\n@@ -673,9 +663,10 @@ lflow_ref_sync_lflows(struct lflow_ref *lflow_ref,\n *\n * If 'lflow_ref' is not NULL then\n * - it first checks if the lflow is present in the lflow_ref or not\n- * - if present, then it does nothing\n * - if not present, then it creates an lflow_ref_node object for\n * the [L(M, A), dp index] and adds ito the lflow_ref hmap.\n+ * - if present, then the lflow_ref_node object's dp_bitmap is\n+ * updated to include the new datapath.\n *\n * Note that this function is not thread safe for 'lflow_ref'.\n * If 2 or more threads calls this function for the same 'lflow_ref',\n@@ -719,31 +710,27 @@ lflow_table_add_lflow(struct lflow_table *lflow_table,\n lrn = xzalloc(sizeof *lrn);\n lrn->lflow = lflow;\n lrn->lflow_ref = lflow_ref;\n- lrn->dpgrp_lflow = !od;\n- if (lrn->dpgrp_lflow) {\n- lrn->dpgrp_bitmap = bitmap_clone(dp_bitmap, dp_bitmap_len);\n- lrn->dpgrp_bitmap_len = dp_bitmap_len;\n- } else {\n- lrn->dp_index = od->index;\n- }\n+ lrn->dp_bitmap_len = od ? ods_size(od->datapaths) : dp_bitmap_len;\n+ lrn->dp_bitmap = bitmap_allocate(lrn->dp_bitmap_len);\n ovs_list_insert(&lflow->referenced_by, &lrn->ref_list_node);\n hmap_insert(&lflow_ref->lflow_ref_nodes, &lrn->ref_node, hash);\n }\n \n- if (!lrn->linked) {\n- if (lrn->dpgrp_lflow) {\n- ovs_assert(lrn->dpgrp_bitmap_len == dp_bitmap_len);\n- size_t index;\n- BITMAP_FOR_EACH_1 (index, dp_bitmap_len, dp_bitmap) {\n- /* Allocate a reference counter only if already used. */\n- if (bitmap_is_set(lflow->dpg_bitmap, index)) {\n- dp_refcnt_use(&lflow->dp_refcnts_map, index);\n- }\n- }\n- } else {\n+ /* Add refcounts for any datapaths already being used by this\n+ * lflow.\n+ */\n+ if (od) {\n+ bitmap_set1(lrn->dp_bitmap, od->index);\n+ if (bitmap_is_set(lflow->dpg_bitmap, od->index)) {\n+ dp_refcnt_use(&lflow->dp_refcnts_map, od->index);\n+ }\n+ } else {\n+ size_t index;\n+ BITMAP_FOR_EACH_1 (index, dp_bitmap_len, dp_bitmap) {\n+ bitmap_set1(lrn->dp_bitmap, index);\n /* Allocate a reference counter only if already used. */\n- if (bitmap_is_set(lflow->dpg_bitmap, lrn->dp_index)) {\n- dp_refcnt_use(&lflow->dp_refcnts_map, lrn->dp_index);\n+ if (bitmap_is_set(lflow->dpg_bitmap, index)) {\n+ dp_refcnt_use(&lflow->dp_refcnts_map, index);\n }\n }\n }\n@@ -1434,7 +1421,7 @@ lflow_ref_node_destroy(struct lflow_ref_node *lrn)\n hmap_remove(&lrn->lflow_ref->lflow_ref_nodes, &lrn->ref_node);\n ovs_list_remove(&lrn->ref_list_node);\n if (lrn->dpgrp_lflow) {\n- bitmap_free(lrn->dpgrp_bitmap);\n+ bitmap_free(lrn->dp_bitmap);\n }\n free(lrn);\n }\n","prefixes":["ovs-dev","09/14"]}