From patchwork Fri Sep 10 14:13:18 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Anton Ivanov X-Patchwork-Id: 1526530 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.138; helo=smtp1.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4H5dC35Zqlz9sX3 for ; Sat, 11 Sep 2021 00:13:35 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 3152384A8A; Fri, 10 Sep 2021 14:13:33 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id l7UL49z1dp1B; Fri, 10 Sep 2021 14:13:32 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp1.osuosl.org (Postfix) with ESMTPS id 88C1E826FF; Fri, 10 Sep 2021 14:13:31 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 5E843C0011; Fri, 10 Sep 2021 14:13:31 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 99904C000D for ; Fri, 10 Sep 2021 14:13:29 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 882574029B for ; Fri, 10 Sep 2021 14:13:29 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Q7PX2Qcsz7tN for ; Fri, 10 Sep 2021 14:13:28 +0000 (UTC) X-Greylist: from auto-whitelisted by SQLgrey-1.8.0 Received: from www.kot-begemot.co.uk (ivanoab7.miniserver.com [37.128.132.42]) by smtp2.osuosl.org (Postfix) with ESMTPS id 90398404C8 for ; Fri, 10 Sep 2021 14:13:28 +0000 (UTC) Received: from tun252.jain.kot-begemot.co.uk ([192.168.18.6] helo=jain.kot-begemot.co.uk) by www.kot-begemot.co.uk with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mOhHR-0007zt-Na; Fri, 10 Sep 2021 14:13:25 +0000 Received: from jain.kot-begemot.co.uk ([192.168.3.3]) by jain.kot-begemot.co.uk with esmtp (Exim 4.92) (envelope-from ) id 1mOhHO-0003oz-LX; Fri, 10 Sep 2021 15:13:24 +0100 From: anton.ivanov@cambridgegreys.com To: ovs-dev@openvswitch.org Date: Fri, 10 Sep 2021 15:13:18 +0100 Message-Id: <20210910141321.14624-1-anton.ivanov@cambridgegreys.com> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 X-Clacks-Overhead: GNU Terry Pratchett Cc: i.maximets@ovn.org, Anton Ivanov Subject: [ovs-dev] [OVN Patch v7 1/4] northd: Disable parallel processing for logical_dp_groups 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: Anton Ivanov Work on improving processing with dp_groups enabled has discovered that the locking mechanism presently in use is not reliable. Disabling parallel processing if dp_groups are enabled until the root cause is determined and fixed. Signed-off-by: Anton Ivanov --- northd/ovn-northd.c | 2 +- ovs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index ee761cef0..2474d1ca4 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -12974,7 +12974,7 @@ build_lswitch_and_lrouter_flows(struct hmap *datapaths, struct hmap *ports, } } - if (use_parallel_build) { + if (use_parallel_build && (!use_logical_dp_groups)) { struct hmap *lflow_segs; struct lswitch_flow_build_info *lsiv; int index; From patchwork Fri Sep 10 14:13:19 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Anton Ivanov X-Patchwork-Id: 1526531 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=2605:bc80:3010::133; helo=smtp2.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4H5dC95jymz9sW4 for ; Sat, 11 Sep 2021 00:13:41 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id AE9F940653; Fri, 10 Sep 2021 14:13:36 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id wj0M3GewAO4g; Fri, 10 Sep 2021 14:13:35 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp2.osuosl.org (Postfix) with ESMTPS id DA00040645; Fri, 10 Sep 2021 14:13:33 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 0A5A3C0021; Fri, 10 Sep 2021 14:13:32 +0000 (UTC) X-Original-To: ovs-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 48ECBC000D for ; Fri, 10 Sep 2021 14:13:30 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 3904B4012A for ; Fri, 10 Sep 2021 14:13:30 +0000 (UTC) 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 VEAx_yrEDk4l for ; Fri, 10 Sep 2021 14:13:29 +0000 (UTC) X-Greylist: from auto-whitelisted by SQLgrey-1.8.0 Received: from www.kot-begemot.co.uk (ivanoab7.miniserver.com [37.128.132.42]) by smtp4.osuosl.org (Postfix) with ESMTPS id A8E9B40122 for ; Fri, 10 Sep 2021 14:13:29 +0000 (UTC) Received: from tun252.jain.kot-begemot.co.uk ([192.168.18.6] helo=jain.kot-begemot.co.uk) by www.kot-begemot.co.uk with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mOhHU-0007zz-2Z; Fri, 10 Sep 2021 14:13:28 +0000 Received: from jain.kot-begemot.co.uk ([192.168.3.3]) by jain.kot-begemot.co.uk with esmtp (Exim 4.92) (envelope-from ) id 1mOhHQ-0003oz-G6; Fri, 10 Sep 2021 15:13:26 +0100 From: anton.ivanov@cambridgegreys.com To: ovs-dev@openvswitch.org Date: Fri, 10 Sep 2021 15:13:19 +0100 Message-Id: <20210910141321.14624-2-anton.ivanov@cambridgegreys.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20210910141321.14624-1-anton.ivanov@cambridgegreys.com> References: <20210910141321.14624-1-anton.ivanov@cambridgegreys.com> MIME-Version: 1.0 X-Clacks-Overhead: GNU Terry Pratchett Cc: i.maximets@ovn.org, Anton Ivanov Subject: [ovs-dev] [OVN Patch v7 2/4] northd: Resize the hash to correct parameters after build 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: Anton Ivanov Parallel builds may result in suboptimal hash bucket sizing. In the absense of dp-groups this does not matter as the hash is purely storage and not used for lookups during the build. Such a hash needs to be resized to a correct size at the end of the build to ensure that any lookups during the lflow reconcilliation phase are done as fast as possible. Signed-off-by: Anton Ivanov --- northd/ovn-northd.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 2474d1ca4..6dfb4327a 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -13178,6 +13178,11 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths, igmp_groups, meter_groups, lbs, bfd_connections); + /* Parallel build may result in a suboptimal hash. Resize the + * hash to a correct size before doing lookups */ + + hmap_expand(&lflows); + if (hmap_count(&lflows) > max_seen_lflow_size) { max_seen_lflow_size = hmap_count(&lflows); } From patchwork Fri Sep 10 14:13:20 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Anton Ivanov X-Patchwork-Id: 1526533 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=smtp3.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) 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 RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4H5dCM6Mq9z9sX3 for ; Sat, 11 Sep 2021 00:13:51 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 2939D605A9; Fri, 10 Sep 2021 14:13:48 +0000 (UTC) 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 yJvt47qscA1V; Fri, 10 Sep 2021 14:13:42 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTPS id A993B61609; Fri, 10 Sep 2021 14:13:40 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 73558C001D; Fri, 10 Sep 2021 14:13:40 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 3C0F3C000D for ; Fri, 10 Sep 2021 14:13:37 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 5D04C60603 for ; Fri, 10 Sep 2021 14:13:36 +0000 (UTC) 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 3I3vbve3_JjA for ; Fri, 10 Sep 2021 14:13:34 +0000 (UTC) X-Greylist: from auto-whitelisted by SQLgrey-1.8.0 Received: from www.kot-begemot.co.uk (ivanoab7.miniserver.com [37.128.132.42]) by smtp3.osuosl.org (Postfix) with ESMTPS id EDE4560BBC for ; Fri, 10 Sep 2021 14:13:33 +0000 (UTC) Received: from tun252.jain.kot-begemot.co.uk ([192.168.18.6] helo=jain.kot-begemot.co.uk) by www.kot-begemot.co.uk with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mOhHX-000808-2u; Fri, 10 Sep 2021 14:13:32 +0000 Received: from jain.kot-begemot.co.uk ([192.168.3.3]) by jain.kot-begemot.co.uk with esmtp (Exim 4.92) (envelope-from ) id 1mOhHS-0003oz-Ax; Fri, 10 Sep 2021 15:13:29 +0100 From: anton.ivanov@cambridgegreys.com To: ovs-dev@openvswitch.org Date: Fri, 10 Sep 2021 15:13:20 +0100 Message-Id: <20210910141321.14624-3-anton.ivanov@cambridgegreys.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20210910141321.14624-1-anton.ivanov@cambridgegreys.com> References: <20210910141321.14624-1-anton.ivanov@cambridgegreys.com> MIME-Version: 1.0 X-Clacks-Overhead: GNU Terry Pratchett Cc: i.maximets@ovn.org, Anton Ivanov Subject: [ovs-dev] [OVN Patch v7 3/4] northd: Optimize dp groups operations 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: Anton Ivanov Remove full hash walks to form lflow dp_groups and add them to the overall parallelizeable lflow build. Make processing of "with dp groups" and "without" in build_lflows independent to allow these to run in parallel after the updates to the parallel API have been merged. Signed-off-by: Anton Ivanov --- northd/ovn-northd.c | 553 +++++++++++++++++++++++++++----------------- 1 file changed, 344 insertions(+), 209 deletions(-) diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 6dfb4327a..aee5b9508 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -86,6 +86,11 @@ struct northd_state { bool paused; }; +struct lflow_state { + struct hmap single_od; + struct hmap multiple_od; +}; + static const char *ovnnb_db; static const char *ovnsb_db; static const char *unixctl_path; @@ -4306,8 +4311,15 @@ struct ovn_lflow { const char *where; }; -static void ovn_lflow_destroy(struct hmap *lflows, struct ovn_lflow *lflow); -static struct ovn_lflow *ovn_lflow_find(const struct hmap *lflows, +static void ovn_lflow_destroy(struct lflow_state *lflows, + struct ovn_lflow *lflow); +static struct ovn_lflow *do_ovn_lflow_find(const struct hmap *lflows, + const struct ovn_datapath *od, + enum ovn_stage stage, + uint16_t priority, const char *match, + const char *actions, + const char *ctrl_meter, uint32_t hash); +static struct ovn_lflow *ovn_lflow_find(const struct lflow_state *lflows, const struct ovn_datapath *od, enum ovn_stage stage, uint16_t priority, const char *match, @@ -4366,7 +4378,7 @@ static struct hashrow_locks lflow_locks; * Version to use when locking is required. */ static struct ovn_lflow * -do_ovn_lflow_add(struct hmap *lflow_map, struct ovn_datapath *od, +do_ovn_lflow_add(struct lflow_state *lflow_map, struct ovn_datapath *od, uint32_t hash, enum ovn_stage stage, uint16_t priority, const char *match, const char *actions, const char *io_port, const struct ovsdb_idl_row *stage_hint, @@ -4377,10 +4389,33 @@ do_ovn_lflow_add(struct hmap *lflow_map, struct ovn_datapath *od, struct ovn_lflow *lflow; if (use_logical_dp_groups) { - old_lflow = ovn_lflow_find(lflow_map, NULL, stage, priority, match, - actions, ctrl_meter, hash); + /* Look up in multiple first. */ + old_lflow = do_ovn_lflow_find(&lflow_map->multiple_od, NULL, stage, + priority, match, + actions, ctrl_meter, hash); if (old_lflow) { hmapx_add(&old_lflow->od_group, od); + } else { + /* Not found, lookup in single od. */ + old_lflow = do_ovn_lflow_find(&lflow_map->single_od, NULL, + stage, priority, match, + actions, ctrl_meter, hash); + if (old_lflow) { + hmapx_add(&old_lflow->od_group, od); + /* Found, different, od count went up. Move to multiple od. */ + if (hmapx_count(&old_lflow->od_group) > 1) { + hmap_remove(&lflow_map->single_od, &old_lflow->hmap_node); + if (use_parallel_build) { + hmap_insert_fast(&lflow_map->multiple_od, + &old_lflow->hmap_node, hash); + } else { + hmap_insert(&lflow_map->multiple_od, + &old_lflow->hmap_node, hash); + } + } + } + } + if (old_lflow) { return old_lflow; } } @@ -4395,16 +4430,20 @@ do_ovn_lflow_add(struct hmap *lflow_map, struct ovn_datapath *od, nullable_xstrdup(ctrl_meter), ovn_lflow_hint(stage_hint), where); hmapx_add(&lflow->od_group, od); + + /* Insert "fresh" lflows into single_od. */ + if (!use_parallel_build) { - hmap_insert(lflow_map, &lflow->hmap_node, hash); + hmap_insert(&lflow_map->single_od, &lflow->hmap_node, hash); } else { - hmap_insert_fast(lflow_map, &lflow->hmap_node, hash); + hmap_insert_fast(&lflow_map->single_od, &lflow->hmap_node, hash); } return lflow; } static struct ovn_lflow * -ovn_lflow_add_at_with_hash(struct hmap *lflow_map, struct ovn_datapath *od, +ovn_lflow_add_at_with_hash(struct lflow_state *lflow_map, + struct ovn_datapath *od, enum ovn_stage stage, uint16_t priority, const char *match, const char *actions, const char *io_port, const char *ctrl_meter, @@ -4429,7 +4468,7 @@ ovn_lflow_add_at_with_hash(struct hmap *lflow_map, struct ovn_datapath *od, /* Adds a row with the specified contents to the Logical_Flow table. */ static void -ovn_lflow_add_at(struct hmap *lflow_map, struct ovn_datapath *od, +ovn_lflow_add_at(struct lflow_state *lflow_map, struct ovn_datapath *od, enum ovn_stage stage, uint16_t priority, const char *match, const char *actions, const char *io_port, const char *ctrl_meter, @@ -4503,27 +4542,82 @@ ovn_dp_group_add_with_reference(struct ovn_lflow *lflow_ref, ACTIONS, NULL, CTRL_METER, NULL) static struct ovn_lflow * -ovn_lflow_find(const struct hmap *lflows, const struct ovn_datapath *od, +do_ovn_lflow_find(const struct hmap *lflows, const struct ovn_datapath *od, enum ovn_stage stage, uint16_t priority, const char *match, const char *actions, const char *ctrl_meter, uint32_t hash) { struct ovn_lflow *lflow; - HMAP_FOR_EACH_WITH_HASH (lflow, hmap_node, hash, lflows) { - if (ovn_lflow_equal(lflow, od, stage, priority, match, actions, - ctrl_meter)) { - return lflow; + if (lflows) { + HMAP_FOR_EACH_WITH_HASH (lflow, hmap_node, hash, lflows) { + if (ovn_lflow_equal(lflow, od, stage, priority, match, actions, + ctrl_meter)) { + return lflow; + } } } return NULL; } +static struct ovn_lflow * +ovn_lflow_find(const struct lflow_state *lflows, const struct ovn_datapath *od, + enum ovn_stage stage, uint16_t priority, + const char *match, const char *actions, const char *ctrl_meter, + uint32_t hash) +{ + struct ovn_lflow *lflow = + do_ovn_lflow_find(&lflows->single_od, od, stage, + priority, match, actions, + ctrl_meter, hash); + if (!lflow) { + lflow = do_ovn_lflow_find(&lflows->multiple_od, od, stage, + priority, match, + actions, ctrl_meter, hash); + } + + return lflow; +} + +static inline bool +hmap_safe_remove(struct hmap *hmap, struct hmap_node *node) +{ + struct hmap_node **bucket = &hmap->buckets[node->hash & hmap->mask]; + while (*bucket != node && *bucket != NULL) { + bucket = &(*bucket)->next; + } + if (*bucket == node) { + *bucket = node->next; + hmap->n--; + return true; + } + return false; +} + +static void +remove_lflow_from_lflows(struct lflow_state *lflows, struct ovn_lflow *lflow) +{ + if (use_logical_dp_groups && use_parallel_build) { + lock_hash_row(&lflow_locks, lflow->hmap_node.hash); + } + if (hmapx_count(&lflow->od_group) > 1) { + if (!hmap_safe_remove(&lflows->multiple_od, &lflow->hmap_node)) { + hmap_remove(&lflows->single_od, &lflow->hmap_node); + } + } else { + if (!hmap_safe_remove(&lflows->single_od, &lflow->hmap_node)) { + hmap_remove(&lflows->multiple_od, &lflow->hmap_node); + } + } + if (use_logical_dp_groups && use_parallel_build) { + unlock_hash_row(&lflow_locks, lflow->hmap_node.hash); + } +} static void -ovn_lflow_destroy(struct hmap *lflows, struct ovn_lflow *lflow) +ovn_lflow_destroy(struct lflow_state *lflows, struct ovn_lflow *lflow) { if (lflow) { if (lflows) { - hmap_remove(lflows, &lflow->hmap_node); + remove_lflow_from_lflows(lflows, lflow); } hmapx_destroy(&lflow->od_group); free(lflow->match); @@ -4535,6 +4629,7 @@ ovn_lflow_destroy(struct hmap *lflows, struct ovn_lflow *lflow) } } + /* Appends port security constraints on L2 address field 'eth_addr_field' * (e.g. "eth.src" or "eth.dst") to 'match'. 'ps_addrs', with 'n_ps_addrs' * elements, is the collection of port_security constraints from an @@ -4662,7 +4757,7 @@ build_port_security_ipv6_flow( * - Priority 80 flow to drop ARP and IPv6 ND packets. */ static void -build_port_security_nd(struct ovn_port *op, struct hmap *lflows, +build_port_security_nd(struct ovn_port *op, struct lflow_state *lflows, const struct ovsdb_idl_row *stage_hint) { struct ds match = DS_EMPTY_INITIALIZER; @@ -4743,7 +4838,7 @@ build_port_security_nd(struct ovn_port *op, struct hmap *lflows, */ static void build_port_security_ip(enum ovn_pipeline pipeline, struct ovn_port *op, - struct hmap *lflows, + struct lflow_state *lflows, const struct ovsdb_idl_row *stage_hint) { char *port_direction; @@ -5147,7 +5242,7 @@ ls_get_acl_flags(struct ovn_datapath *od) */ static void build_lswitch_input_port_sec_op( - struct ovn_port *op, struct hmap *lflows, + struct ovn_port *op, struct lflow_state *lflows, struct ds *actions, struct ds *match) { @@ -5191,7 +5286,7 @@ build_lswitch_input_port_sec_op( */ static void build_lswitch_input_port_sec_od( - struct ovn_datapath *od, struct hmap *lflows) + struct ovn_datapath *od, struct lflow_state *lflows) { if (od->nbs) { @@ -5202,7 +5297,7 @@ build_lswitch_input_port_sec_od( static void build_lswitch_learn_fdb_op( - struct ovn_port *op, struct hmap *lflows, + struct ovn_port *op, struct lflow_state *lflows, struct ds *actions, struct ds *match) { if (op->nbsp && !op->n_ps_addrs && !strcmp(op->nbsp->type, "") && @@ -5229,7 +5324,7 @@ build_lswitch_learn_fdb_op( static void build_lswitch_learn_fdb_od( - struct ovn_datapath *od, struct hmap *lflows) + struct ovn_datapath *od, struct lflow_state *lflows) { if (od->nbs) { @@ -5250,7 +5345,7 @@ build_lswitch_learn_fdb_od( */ static void build_lswitch_output_port_sec_op(struct ovn_port *op, - struct hmap *lflows, + struct lflow_state *lflows, struct ds *match, struct ds *actions) { @@ -5295,7 +5390,7 @@ build_lswitch_output_port_sec_op(struct ovn_port *op, * (priority 100). */ static void build_lswitch_output_port_sec_od(struct ovn_datapath *od, - struct hmap *lflows) + struct lflow_state *lflows) { if (od->nbs) { ovn_lflow_add(lflows, od, S_SWITCH_OUT_PORT_SEC_IP, 0, "1", "next;"); @@ -5307,7 +5402,7 @@ build_lswitch_output_port_sec_od(struct ovn_datapath *od, static void skip_port_from_conntrack(struct ovn_datapath *od, struct ovn_port *op, enum ovn_stage in_stage, enum ovn_stage out_stage, - uint16_t priority, struct hmap *lflows) + uint16_t priority, struct lflow_state *lflows) { /* Can't use ct() for router ports. Consider the following configuration: * lp1(10.0.0.2) on hostA--ls1--lr0--ls2--lp2(10.0.1.2) on hostB, For a @@ -5337,7 +5432,7 @@ skip_port_from_conntrack(struct ovn_datapath *od, struct ovn_port *op, static void build_stateless_filter(struct ovn_datapath *od, const struct nbrec_acl *acl, - struct hmap *lflows) + struct lflow_state *lflows) { if (!strcmp(acl->direction, "from-lport")) { ovn_lflow_add_with_hint(lflows, od, S_SWITCH_IN_PRE_ACL, @@ -5356,7 +5451,7 @@ build_stateless_filter(struct ovn_datapath *od, static void build_stateless_filters(struct ovn_datapath *od, struct hmap *port_groups, - struct hmap *lflows) + struct lflow_state *lflows) { for (size_t i = 0; i < od->nbs->n_acls; i++) { const struct nbrec_acl *acl = od->nbs->acls[i]; @@ -5380,7 +5475,7 @@ build_stateless_filters(struct ovn_datapath *od, struct hmap *port_groups, static void build_pre_acls(struct ovn_datapath *od, struct hmap *port_groups, - struct hmap *lflows) + struct lflow_state *lflows) { /* Ingress and Egress Pre-ACL Table (Priority 0): Packets are * allowed by default. */ @@ -5509,7 +5604,7 @@ ls_has_lb_vip(struct ovn_datapath *od) } static void -build_pre_lb(struct ovn_datapath *od, struct hmap *lflows, +build_pre_lb(struct ovn_datapath *od, struct lflow_state *lflows, struct hmap *lbs) { /* Do not send ND packets to conntrack */ @@ -5588,7 +5683,7 @@ build_pre_lb(struct ovn_datapath *od, struct hmap *lflows, } static void -build_pre_stateful(struct ovn_datapath *od, struct hmap *lflows) +build_pre_stateful(struct ovn_datapath *od, struct lflow_state *lflows) { /* Ingress and Egress pre-stateful Table (Priority 0): Packets are * allowed by default. */ @@ -5640,7 +5735,7 @@ build_pre_stateful(struct ovn_datapath *od, struct hmap *lflows) } static void -build_acl_hints(struct ovn_datapath *od, struct hmap *lflows) +build_acl_hints(struct ovn_datapath *od, struct lflow_state *lflows) { /* This stage builds hints for the IN/OUT_ACL stage. Based on various * combinations of ct flags packets may hit only a subset of the logical @@ -5813,7 +5908,7 @@ build_acl_log(struct ds *actions, const struct nbrec_acl *acl, } static void -build_reject_acl_rules(struct ovn_datapath *od, struct hmap *lflows, +build_reject_acl_rules(struct ovn_datapath *od, struct lflow_state *lflows, enum ovn_stage stage, struct nbrec_acl *acl, struct ds *extra_match, struct ds *extra_actions, const struct ovsdb_idl_row *stage_hint, @@ -5856,7 +5951,7 @@ build_reject_acl_rules(struct ovn_datapath *od, struct hmap *lflows, } static void -consider_acl(struct hmap *lflows, struct ovn_datapath *od, +consider_acl(struct lflow_state *lflows, struct ovn_datapath *od, struct nbrec_acl *acl, bool has_stateful, const struct shash *meter_groups, struct ds *match, struct ds *actions) @@ -6080,7 +6175,7 @@ build_port_group_lswitches(struct northd_context *ctx, struct hmap *pgs, } static void -build_acls(struct ovn_datapath *od, struct hmap *lflows, +build_acls(struct ovn_datapath *od, struct lflow_state *lflows, struct hmap *port_groups, const struct shash *meter_groups) { bool has_stateful = od->has_stateful_acl || od->has_lb_vip; @@ -6294,7 +6389,7 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows, } static void -build_qos(struct ovn_datapath *od, struct hmap *lflows) { +build_qos(struct ovn_datapath *od, struct lflow_state *lflows) { struct ds action = DS_EMPTY_INITIALIZER; ovn_lflow_add(lflows, od, S_SWITCH_IN_QOS_MARK, 0, "1", "next;"); @@ -6355,7 +6450,7 @@ build_qos(struct ovn_datapath *od, struct hmap *lflows) { } static void -build_lb_rules(struct hmap *lflows, struct ovn_northd_lb *lb, +build_lb_rules(struct lflow_state *lflows, struct ovn_northd_lb *lb, struct ds *match, struct ds *action, struct shash *meter_groups) { @@ -6436,7 +6531,7 @@ build_lb_rules(struct hmap *lflows, struct ovn_northd_lb *lb, } static void -build_stateful(struct ovn_datapath *od, struct hmap *lflows) +build_stateful(struct ovn_datapath *od, struct lflow_state *lflows) { /* Ingress and Egress stateful Table (Priority 0): Packets are * allowed by default. */ @@ -6475,7 +6570,7 @@ build_stateful(struct ovn_datapath *od, struct hmap *lflows) } static void -build_lb_hairpin(struct ovn_datapath *od, struct hmap *lflows) +build_lb_hairpin(struct ovn_datapath *od, struct lflow_state *lflows) { /* Ingress Pre-Hairpin/Nat-Hairpin/Hairpin tabled (Priority 0). * Packets that don't need hairpinning should continue processing. @@ -6533,7 +6628,7 @@ build_lb_hairpin(struct ovn_datapath *od, struct hmap *lflows) /* Build logical flows for the forwarding groups */ static void -build_fwd_group_lflows(struct ovn_datapath *od, struct hmap *lflows) +build_fwd_group_lflows(struct ovn_datapath *od, struct lflow_state *lflows) { if (!(!od->nbs || !od->nbs->n_forwarding_groups)) { @@ -6746,7 +6841,7 @@ static void build_lswitch_rport_arp_req_self_orig_flow(struct ovn_port *op, uint32_t priority, struct ovn_datapath *od, - struct hmap *lflows) + struct lflow_state *lflows) { struct sset all_eth_addrs = SSET_INITIALIZER(&all_eth_addrs); struct ds eth_src = DS_EMPTY_INITIALIZER; @@ -6840,7 +6935,7 @@ arp_nd_ns_match(const char *ips, int addr_family, struct ds *match) static void build_lswitch_rport_arp_req_flow_for_reachable_ip(const char *ips, int addr_family, struct ovn_port *patch_op, struct ovn_datapath *od, - uint32_t priority, struct hmap *lflows, + uint32_t priority, struct lflow_state *lflows, const struct ovsdb_idl_row *stage_hint) { struct ds match = DS_EMPTY_INITIALIZER; @@ -6879,7 +6974,7 @@ build_lswitch_rport_arp_req_flow_for_reachable_ip(const char *ips, static void build_lswitch_rport_arp_req_flow_for_unreachable_ip(const char *ips, int addr_family, struct ovn_datapath *od, uint32_t priority, - struct hmap *lflows, const struct ovsdb_idl_row *stage_hint) + struct lflow_state *lflows, const struct ovsdb_idl_row *stage_hint) { struct ds match = DS_EMPTY_INITIALIZER; @@ -6904,7 +6999,7 @@ static void build_lswitch_rport_arp_req_flows(struct ovn_port *op, struct ovn_datapath *sw_od, struct ovn_port *sw_op, - struct hmap *lflows, + struct lflow_state *lflows, const struct ovsdb_idl_row *stage_hint) { if (!op || !op->nbrp) { @@ -7021,7 +7116,7 @@ build_dhcpv4_options_flows(struct ovn_port *op, struct lport_addresses *lsp_addrs, struct ovn_port *inport, bool is_external, struct shash *meter_groups, - struct hmap *lflows) + struct lflow_state *lflows) { struct ds match = DS_EMPTY_INITIALIZER; @@ -7114,7 +7209,7 @@ build_dhcpv6_options_flows(struct ovn_port *op, struct lport_addresses *lsp_addrs, struct ovn_port *inport, bool is_external, struct shash *meter_groups, - struct hmap *lflows) + struct lflow_state *lflows) { struct ds match = DS_EMPTY_INITIALIZER; @@ -7164,7 +7259,7 @@ build_dhcpv6_options_flows(struct ovn_port *op, static void build_drop_arp_nd_flows_for_unbound_router_ports(struct ovn_port *op, const struct ovn_port *port, - struct hmap *lflows) + struct lflow_state *lflows) { struct ds match = DS_EMPTY_INITIALIZER; @@ -7229,7 +7324,7 @@ is_vlan_transparent(const struct ovn_datapath *od) } static void -build_lswitch_flows(struct hmap *datapaths, struct hmap *lflows) +build_lswitch_flows(struct hmap *datapaths, struct lflow_state *lflows) { /* This flow table structure is documented in ovn-northd(8), so please * update ovn-northd.8.xml if you change anything. */ @@ -7264,7 +7359,7 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap *lflows) static void build_lswitch_lflows_pre_acl_and_acl(struct ovn_datapath *od, struct hmap *port_groups, - struct hmap *lflows, + struct lflow_state *lflows, struct shash *meter_groups, struct hmap *lbs) { @@ -7287,7 +7382,7 @@ build_lswitch_lflows_pre_acl_and_acl(struct ovn_datapath *od, * 100). */ static void build_lswitch_lflows_admission_control(struct ovn_datapath *od, - struct hmap *lflows) + struct lflow_state *lflows) { if (od->nbs) { /* Logical VLANs not supported. */ @@ -7313,7 +7408,7 @@ build_lswitch_lflows_admission_control(struct ovn_datapath *od, static void build_lswitch_arp_nd_responder_skip_local(struct ovn_port *op, - struct hmap *lflows, + struct lflow_state *lflows, struct ds *match) { if (op->nbsp) { @@ -7333,7 +7428,7 @@ build_lswitch_arp_nd_responder_skip_local(struct ovn_port *op, * (priority 50). */ static void build_lswitch_arp_nd_responder_known_ips(struct ovn_port *op, - struct hmap *lflows, + struct lflow_state *lflows, struct hmap *ports, struct shash *meter_groups, struct ds *actions, @@ -7553,7 +7648,7 @@ build_lswitch_arp_nd_responder_known_ips(struct ovn_port *op, * (priority 0)*/ static void build_lswitch_arp_nd_responder_default(struct ovn_datapath *od, - struct hmap *lflows) + struct lflow_state *lflows) { if (od->nbs) { ovn_lflow_add(lflows, od, S_SWITCH_IN_ARP_ND_RSP, 0, "1", "next;"); @@ -7564,7 +7659,7 @@ build_lswitch_arp_nd_responder_default(struct ovn_datapath *od, * (priority 110)*/ static void build_lswitch_arp_nd_service_monitor(struct ovn_northd_lb *lb, - struct hmap *lflows, + struct lflow_state *lflows, struct ds *actions, struct ds *match) { @@ -7612,7 +7707,7 @@ build_lswitch_arp_nd_service_monitor(struct ovn_northd_lb *lb, * priority 100 flows. */ static void build_lswitch_dhcp_options_and_response(struct ovn_port *op, - struct hmap *lflows, + struct lflow_state *lflows, struct shash *meter_groups) { if (op->nbsp) { @@ -7668,7 +7763,7 @@ build_lswitch_dhcp_options_and_response(struct ovn_port *op, * (priority 0). */ static void build_lswitch_dhcp_and_dns_defaults(struct ovn_datapath *od, - struct hmap *lflows) + struct lflow_state *lflows) { if (od->nbs) { ovn_lflow_add(lflows, od, S_SWITCH_IN_DHCP_OPTIONS, 0, "1", "next;"); @@ -7684,7 +7779,7 @@ build_lswitch_dhcp_and_dns_defaults(struct ovn_datapath *od, */ static void build_lswitch_dns_lookup_and_response(struct ovn_datapath *od, - struct hmap *lflows, + struct lflow_state *lflows, struct shash *meter_groups) { if (od->nbs && ls_has_dns_records(od->nbs)) { @@ -7713,7 +7808,7 @@ build_lswitch_dns_lookup_and_response(struct ovn_datapath *od, * binding the external ports. */ static void build_lswitch_external_port(struct ovn_port *op, - struct hmap *lflows) + struct lflow_state *lflows) { if (op->nbsp && lsp_is_external(op->nbsp)) { @@ -7728,7 +7823,7 @@ build_lswitch_external_port(struct ovn_port *op, * (priority 70 - 100). */ static void build_lswitch_destination_lookup_bmcast(struct ovn_datapath *od, - struct hmap *lflows, + struct lflow_state *lflows, struct ds *actions, struct shash *meter_groups) { @@ -7820,7 +7915,7 @@ build_lswitch_destination_lookup_bmcast(struct ovn_datapath *od, * (priority 90). */ static void build_lswitch_ip_mcast_igmp_mld(struct ovn_igmp_group *igmp_group, - struct hmap *lflows, + struct lflow_state *lflows, struct ds *actions, struct ds *match) { @@ -7897,7 +7992,7 @@ static struct ovs_mutex mcgroup_mutex = OVS_MUTEX_INITIALIZER; /* Ingress table 22: Destination lookup, unicast handling (priority 50), */ static void build_lswitch_ip_unicast_lookup(struct ovn_port *op, - struct hmap *lflows, + struct lflow_state *lflows, struct hmap *mcgroups, struct ds *actions, struct ds *match) @@ -8317,7 +8412,7 @@ get_outport_for_routing_policy_nexthop(struct ovn_datapath *od, } static void -build_routing_policy_flow(struct hmap *lflows, struct ovn_datapath *od, +build_routing_policy_flow(struct lflow_state *lflows, struct ovn_datapath *od, struct hmap *ports, const struct nbrec_logical_router_policy *rule, const struct ovsdb_idl_row *stage_hint) @@ -8382,8 +8477,8 @@ build_routing_policy_flow(struct hmap *lflows, struct ovn_datapath *od, } static void -build_ecmp_routing_policy_flows(struct hmap *lflows, struct ovn_datapath *od, - struct hmap *ports, +build_ecmp_routing_policy_flows(struct lflow_state *lflows, + struct ovn_datapath *od, struct hmap *ports, const struct nbrec_logical_router_policy *rule, uint16_t ecmp_group_id) { @@ -8853,7 +8948,7 @@ find_static_route_outport(struct ovn_datapath *od, struct hmap *ports, } static void -add_ecmp_symmetric_reply_flows(struct hmap *lflows, +add_ecmp_symmetric_reply_flows(struct lflow_state *lflows, struct ovn_datapath *od, const char *port_ip, struct ovn_port *out_port, @@ -8933,7 +9028,7 @@ add_ecmp_symmetric_reply_flows(struct hmap *lflows, } static void -build_ecmp_route_flow(struct hmap *lflows, struct ovn_datapath *od, +build_ecmp_route_flow(struct lflow_state *lflows, struct ovn_datapath *od, struct hmap *ports, struct ecmp_groups_node *eg) { @@ -9016,7 +9111,7 @@ build_ecmp_route_flow(struct hmap *lflows, struct ovn_datapath *od, } static void -add_route(struct hmap *lflows, struct ovn_datapath *od, +add_route(struct lflow_state *lflows, struct ovn_datapath *od, const struct ovn_port *op, const char *lrp_addr_s, const char *network_s, int plen, const char *gateway, bool is_src_route, const struct ovsdb_idl_row *stage_hint, @@ -9078,7 +9173,7 @@ add_route(struct hmap *lflows, struct ovn_datapath *od, } static void -build_static_route_flow(struct hmap *lflows, struct ovn_datapath *od, +build_static_route_flow(struct lflow_state *lflows, struct ovn_datapath *od, struct hmap *ports, const struct parsed_route *route_) { @@ -9173,7 +9268,7 @@ static void build_lrouter_nat_flows_for_lb(struct ovn_lb_vip *lb_vip, struct ovn_northd_lb *lb, struct ovn_northd_lb_vip *vips_nb, - struct hmap *lflows, + struct lflow_state *lflows, struct ds *match, struct ds *action, struct shash *meter_groups) { @@ -9383,7 +9478,8 @@ next: } static void -build_lswitch_flows_for_lb(struct ovn_northd_lb *lb, struct hmap *lflows, +build_lswitch_flows_for_lb(struct ovn_northd_lb *lb, + struct lflow_state *lflows, struct shash *meter_groups, struct ds *match, struct ds *action) { @@ -9434,7 +9530,7 @@ build_lswitch_flows_for_lb(struct ovn_northd_lb *lb, struct hmap *lflows, */ static void build_lrouter_defrag_flows_for_lb(struct ovn_northd_lb *lb, - struct hmap *lflows, + struct lflow_state *lflows, struct ds *match) { if (!lb->n_nb_lr) { @@ -9492,7 +9588,7 @@ build_lrouter_defrag_flows_for_lb(struct ovn_northd_lb *lb, static void build_lflows_for_unreachable_vips(struct ovn_northd_lb *lb, struct ovn_lb_vip *lb_vip, - struct hmap *lflows, + struct lflow_state *lflows, struct ds *match) { static const char *action = "outport = \"_MC_flood\"; output;"; @@ -9553,9 +9649,10 @@ build_lflows_for_unreachable_vips(struct ovn_northd_lb *lb, } static void -build_lrouter_flows_for_lb(struct ovn_northd_lb *lb, struct hmap *lflows, - struct shash *meter_groups, struct ds *match, - struct ds *action) +build_lrouter_flows_for_lb(struct ovn_northd_lb *lb, + struct lflow_state *lflows, + struct shash *meter_groups, + struct ds *match, struct ds *action) { if (!lb->n_nb_lr) { return; @@ -9706,7 +9803,7 @@ lrouter_nat_is_stateless(const struct nbrec_nat *nat) */ static inline void lrouter_nat_add_ext_ip_match(struct ovn_datapath *od, - struct hmap *lflows, struct ds *match, + struct lflow_state *lflows, struct ds *match, const struct nbrec_nat *nat, bool is_v6, bool is_src, ovs_be32 mask) { @@ -9773,7 +9870,7 @@ build_lrouter_arp_flow(struct ovn_datapath *od, struct ovn_port *op, const char *ip_address, const char *eth_addr, struct ds *extra_match, bool drop, uint16_t priority, const struct ovsdb_idl_row *hint, - struct hmap *lflows) + struct lflow_state *lflows) { struct ds match = DS_EMPTY_INITIALIZER; struct ds actions = DS_EMPTY_INITIALIZER; @@ -9823,7 +9920,7 @@ build_lrouter_nd_flow(struct ovn_datapath *od, struct ovn_port *op, const char *sn_ip_address, const char *eth_addr, struct ds *extra_match, bool drop, uint16_t priority, const struct ovsdb_idl_row *hint, - struct hmap *lflows, struct shash *meter_groups) + struct lflow_state *lflows, struct shash *meter_groups) { struct ds match = DS_EMPTY_INITIALIZER; struct ds actions = DS_EMPTY_INITIALIZER; @@ -9877,7 +9974,7 @@ build_lrouter_nd_flow(struct ovn_datapath *od, struct ovn_port *op, static void build_lrouter_nat_arp_nd_flow(struct ovn_datapath *od, struct ovn_nat *nat_entry, - struct hmap *lflows, + struct lflow_state *lflows, struct shash *meter_groups) { struct lport_addresses *ext_addrs = &nat_entry->ext_addrs; @@ -9900,7 +9997,7 @@ build_lrouter_nat_arp_nd_flow(struct ovn_datapath *od, static void build_lrouter_port_nat_arp_nd_flow(struct ovn_port *op, struct ovn_nat *nat_entry, - struct hmap *lflows, + struct lflow_state *lflows, struct shash *meter_groups) { struct lport_addresses *ext_addrs = &nat_entry->ext_addrs; @@ -9967,7 +10064,7 @@ build_lrouter_port_nat_arp_nd_flow(struct ovn_port *op, static void build_lrouter_drop_own_dest(struct ovn_port *op, enum ovn_stage stage, uint16_t priority, bool drop_snat_ip, - struct hmap *lflows) + struct lflow_state *lflows) { struct ds match_ips = DS_EMPTY_INITIALIZER; @@ -10024,7 +10121,8 @@ build_lrouter_drop_own_dest(struct ovn_port *op, enum ovn_stage stage, } static void -build_lrouter_force_snat_flows(struct hmap *lflows, struct ovn_datapath *od, +build_lrouter_force_snat_flows(struct lflow_state *lflows, + struct ovn_datapath *od, const char *ip_version, const char *ip_addr, const char *context) { @@ -10051,7 +10149,7 @@ build_lrouter_force_snat_flows(struct hmap *lflows, struct ovn_datapath *od, static void build_lrouter_force_snat_flows_op(struct ovn_port *op, - struct hmap *lflows, + struct lflow_state *lflows, struct ds *match, struct ds *actions) { if (!op->nbrp || !op->peer || !op->od->lb_force_snat_router_ip) { @@ -10122,7 +10220,7 @@ build_lrouter_force_snat_flows_op(struct ovn_port *op, } static void -build_lrouter_bfd_flows(struct hmap *lflows, struct ovn_port *op, +build_lrouter_bfd_flows(struct lflow_state *lflows, struct ovn_port *op, struct shash *meter_groups) { if (!op->has_bfd) { @@ -10177,7 +10275,7 @@ build_lrouter_bfd_flows(struct hmap *lflows, struct ovn_port *op, */ static void build_adm_ctrl_flows_for_lrouter( - struct ovn_datapath *od, struct hmap *lflows) + struct ovn_datapath *od, struct lflow_state *lflows) { if (od->nbr) { /* Logical VLANs not supported. @@ -10196,7 +10294,7 @@ build_check_pkt_len_action_string(struct ovn_port *op, struct ds *actions); */ static void build_adm_ctrl_flows_for_lrouter_port( - struct ovn_port *op, struct hmap *lflows, + struct ovn_port *op, struct lflow_state *lflows, struct ds *match, struct ds *actions) { if (op->nbrp) { @@ -10248,7 +10346,7 @@ build_adm_ctrl_flows_for_lrouter_port( * lflows for logical routers. */ static void build_neigh_learning_flows_for_lrouter( - struct ovn_datapath *od, struct hmap *lflows, + struct ovn_datapath *od, struct lflow_state *lflows, struct ds *match, struct ds *actions, struct shash *meter_groups) { @@ -10352,7 +10450,7 @@ build_neigh_learning_flows_for_lrouter( * for logical router ports. */ static void build_neigh_learning_flows_for_lrouter_port( - struct ovn_port *op, struct hmap *lflows, + struct ovn_port *op, struct lflow_state *lflows, struct ds *match, struct ds *actions) { if (op->nbrp) { @@ -10415,7 +10513,7 @@ build_neigh_learning_flows_for_lrouter_port( * Adv (RA) options and response. */ static void build_ND_RA_flows_for_lrouter_port( - struct ovn_port *op, struct hmap *lflows, + struct ovn_port *op, struct lflow_state *lflows, struct ds *match, struct ds *actions, struct shash *meter_groups) { @@ -10546,7 +10644,8 @@ build_ND_RA_flows_for_lrouter_port( /* Logical router ingress table ND_RA_OPTIONS & ND_RA_RESPONSE: RS * responder, by default goto next. (priority 0). */ static void -build_ND_RA_flows_for_lrouter(struct ovn_datapath *od, struct hmap *lflows) +build_ND_RA_flows_for_lrouter(struct ovn_datapath *od, + struct lflow_state *lflows) { if (od->nbr) { ovn_lflow_add(lflows, od, S_ROUTER_IN_ND_RA_OPTIONS, 0, "1", "next;"); @@ -10572,7 +10671,7 @@ build_ND_RA_flows_for_lrouter(struct ovn_datapath *od, struct hmap *lflows) */ static void build_ip_routing_flows_for_lrouter_port( - struct ovn_port *op, struct hmap *ports,struct hmap *lflows) + struct ovn_port *op, struct hmap *ports,struct lflow_state *lflows) { if (op->nbrp) { @@ -10619,7 +10718,7 @@ build_ip_routing_flows_for_lrouter_port( static void build_static_route_flows_for_lrouter( - struct ovn_datapath *od, struct hmap *lflows, + struct ovn_datapath *od, struct lflow_state *lflows, struct hmap *ports, struct hmap *bfd_connections) { if (od->nbr) { @@ -10673,7 +10772,7 @@ build_static_route_flows_for_lrouter( */ static void build_mcast_lookup_flows_for_lrouter( - struct ovn_datapath *od, struct hmap *lflows, + struct ovn_datapath *od, struct lflow_state *lflows, struct ds *match, struct ds *actions) { if (od->nbr) { @@ -10742,7 +10841,7 @@ build_mcast_lookup_flows_for_lrouter( * advances to the next table for ARP/ND resolution. */ static void build_ingress_policy_flows_for_lrouter( - struct ovn_datapath *od, struct hmap *lflows, + struct ovn_datapath *od, struct lflow_state *lflows, struct hmap *ports) { if (od->nbr) { @@ -10776,7 +10875,7 @@ build_ingress_policy_flows_for_lrouter( /* Local router ingress table ARP_RESOLVE: ARP Resolution. */ static void build_arp_resolve_flows_for_lrouter( - struct ovn_datapath *od, struct hmap *lflows) + struct ovn_datapath *od, struct lflow_state *lflows) { if (od->nbr) { /* Multicast packets already have the outport set so just advance to @@ -10793,7 +10892,8 @@ build_arp_resolve_flows_for_lrouter( } static void -routable_addresses_to_lflows(struct hmap *lflows, struct ovn_port *router_port, +routable_addresses_to_lflows(struct lflow_state *lflows, + struct ovn_port *router_port, struct ovn_port *peer, struct ds *match, struct ds *actions) { @@ -10834,7 +10934,7 @@ routable_addresses_to_lflows(struct hmap *lflows, struct ovn_port *router_port, */ static void build_arp_resolve_flows_for_lrouter_port( - struct ovn_port *op, struct hmap *lflows, + struct ovn_port *op, struct lflow_state *lflows, struct hmap *ports, struct ds *match, struct ds *actions) { @@ -11157,7 +11257,8 @@ build_arp_resolve_flows_for_lrouter_port( } static void -build_icmperr_pkt_big_flows(struct ovn_port *op, int mtu, struct hmap *lflows, +build_icmperr_pkt_big_flows(struct ovn_port *op, int mtu, + struct lflow_state *lflows, struct shash *meter_groups, struct ds *match, struct ds *actions, enum ovn_stage stage, struct ovn_port *outport) @@ -11250,7 +11351,8 @@ build_check_pkt_len_action_string(struct ovn_port *op, struct ds *actions) static void build_check_pkt_len_flows_for_lrp(struct ovn_port *op, - struct hmap *lflows, struct hmap *ports, + struct lflow_state *lflows, + struct hmap *ports, struct shash *meter_groups, struct ds *match, struct ds *actions) { @@ -11303,7 +11405,7 @@ build_check_pkt_len_flows_for_lrp(struct ovn_port *op, * */ static void build_check_pkt_len_flows_for_lrouter( - struct ovn_datapath *od, struct hmap *lflows, + struct ovn_datapath *od, struct lflow_state *lflows, struct hmap *ports, struct ds *match, struct ds *actions, struct shash *meter_groups) @@ -11338,7 +11440,7 @@ build_check_pkt_len_flows_for_lrouter( */ static void build_gateway_redirect_flows_for_lrouter( - struct ovn_datapath *od, struct hmap *lflows, + struct ovn_datapath *od, struct lflow_state *lflows, struct ds *match, struct ds *actions) { if (!od->nbr) { @@ -11376,7 +11478,7 @@ build_gateway_redirect_flows_for_lrouter( * and sends an ARP/IPv6 NA request (priority 100). */ static void build_arp_request_flows_for_lrouter( - struct ovn_datapath *od, struct hmap *lflows, + struct ovn_datapath *od, struct lflow_state *lflows, struct ds *match, struct ds *actions, struct shash *meter_groups) { @@ -11455,7 +11557,7 @@ build_arp_request_flows_for_lrouter( */ static void build_egress_delivery_flows_for_lrouter_port( - struct ovn_port *op, struct hmap *lflows, + struct ovn_port *op, struct lflow_state *lflows, struct ds *match, struct ds *actions) { if (op->nbrp) { @@ -11497,7 +11599,7 @@ build_egress_delivery_flows_for_lrouter_port( static void build_misc_local_traffic_drop_flows_for_lrouter( - struct ovn_datapath *od, struct hmap *lflows) + struct ovn_datapath *od, struct lflow_state *lflows) { if (od->nbr) { /* L3 admission control: drop multicast and broadcast source, localhost @@ -11552,7 +11654,7 @@ build_misc_local_traffic_drop_flows_for_lrouter( static void build_dhcpv6_reply_flows_for_lrouter_port( - struct ovn_port *op, struct hmap *lflows, + struct ovn_port *op, struct lflow_state *lflows, struct ds *match) { if (op->nbrp && (!op->l3dgw_port)) { @@ -11571,7 +11673,7 @@ build_dhcpv6_reply_flows_for_lrouter_port( static void build_ipv6_input_flows_for_lrouter_port( - struct ovn_port *op, struct hmap *lflows, + struct ovn_port *op, struct lflow_state *lflows, struct ds *match, struct ds *actions, struct shash *meter_groups) { @@ -11733,7 +11835,7 @@ build_ipv6_input_flows_for_lrouter_port( static void build_lrouter_arp_nd_for_datapath(struct ovn_datapath *od, - struct hmap *lflows, + struct lflow_state *lflows, struct shash *meter_groups) { if (od->nbr) { @@ -11783,7 +11885,7 @@ build_lrouter_arp_nd_for_datapath(struct ovn_datapath *od, /* Logical router ingress table 3: IP Input for IPv4. */ static void build_lrouter_ipv4_ip_input(struct ovn_port *op, - struct hmap *lflows, + struct lflow_state *lflows, struct ds *match, struct ds *actions, struct shash *meter_groups) { @@ -12071,7 +12173,8 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op, } static void -build_lrouter_in_unsnat_flow(struct hmap *lflows, struct ovn_datapath *od, +build_lrouter_in_unsnat_flow(struct lflow_state *lflows, + struct ovn_datapath *od, const struct nbrec_nat *nat, struct ds *match, struct ds *actions, bool distributed, bool is_v6) { @@ -12134,7 +12237,7 @@ build_lrouter_in_unsnat_flow(struct hmap *lflows, struct ovn_datapath *od, } static void -build_lrouter_in_dnat_flow(struct hmap *lflows, struct ovn_datapath *od, +build_lrouter_in_dnat_flow(struct lflow_state *lflows, struct ovn_datapath *od, const struct nbrec_nat *nat, struct ds *match, struct ds *actions, bool distributed, ovs_be32 mask, bool is_v6) @@ -12221,7 +12324,8 @@ build_lrouter_in_dnat_flow(struct hmap *lflows, struct ovn_datapath *od, } static void -build_lrouter_out_undnat_flow(struct hmap *lflows, struct ovn_datapath *od, +build_lrouter_out_undnat_flow(struct lflow_state *lflows, + struct ovn_datapath *od, const struct nbrec_nat *nat, struct ds *match, struct ds *actions, bool distributed, struct eth_addr mac, bool is_v6) @@ -12268,7 +12372,8 @@ build_lrouter_out_undnat_flow(struct hmap *lflows, struct ovn_datapath *od, } static void -build_lrouter_out_snat_flow(struct hmap *lflows, struct ovn_datapath *od, +build_lrouter_out_snat_flow(struct lflow_state *lflows, + struct ovn_datapath *od, const struct nbrec_nat *nat, struct ds *match, struct ds *actions, bool distributed, struct eth_addr mac, ovs_be32 mask, @@ -12361,7 +12466,7 @@ build_lrouter_out_snat_flow(struct hmap *lflows, struct ovn_datapath *od, } static void -build_lrouter_ingress_flow(struct hmap *lflows, struct ovn_datapath *od, +build_lrouter_ingress_flow(struct lflow_state *lflows, struct ovn_datapath *od, const struct nbrec_nat *nat, struct ds *match, struct ds *actions, struct eth_addr mac, bool distributed, bool is_v6) @@ -12497,7 +12602,8 @@ lrouter_check_nat_entry(struct ovn_datapath *od, const struct nbrec_nat *nat, /* NAT, Defrag and load balancing. */ static void -build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od, struct hmap *lflows, +build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od, + struct lflow_state *lflows, struct hmap *ports, struct ds *match, struct ds *actions) { @@ -12719,7 +12825,7 @@ struct lswitch_flow_build_info { struct hmap *datapaths; struct hmap *ports; struct hmap *port_groups; - struct hmap *lflows; + struct lflow_state *lflows; struct hmap *mcgroups; struct hmap *igmp_groups; struct shash *meter_groups; @@ -12940,12 +13046,6 @@ init_lflows_thread_pool(void) } } -/* TODO: replace hard cutoffs by configurable via commands. These are - * temporary defines to determine single-thread to multi-thread processing - * cutoff. - * Setting to 1 forces "all parallel" lflow build. - */ - static void noop_callback(struct worker_pool *pool OVS_UNUSED, void *fin_result OVS_UNUSED, @@ -12955,10 +13055,24 @@ noop_callback(struct worker_pool *pool OVS_UNUSED, /* Do nothing */ } +static void +lflow_merge_callback(struct worker_pool *pool OVS_UNUSED, + void *fin_result, + void *result_frags, + int index) +{ + struct lflow_state *result = (struct lflow_state *)fin_result; + struct lflow_state *res_frags = (struct lflow_state *)result_frags; + + fast_hmap_merge(&result->single_od, &res_frags[index].single_od); + hmap_destroy(&res_frags[index].single_od); +} + static void build_lswitch_and_lrouter_flows(struct hmap *datapaths, struct hmap *ports, - struct hmap *port_groups, struct hmap *lflows, + struct hmap *port_groups, + struct lflow_state *lflows, struct hmap *mcgroups, struct hmap *igmp_groups, struct shash *meter_groups, struct hmap *lbs, @@ -12975,7 +13089,7 @@ build_lswitch_and_lrouter_flows(struct hmap *datapaths, struct hmap *ports, } if (use_parallel_build && (!use_logical_dp_groups)) { - struct hmap *lflow_segs; + struct lflow_state *lflow_segs; struct lswitch_flow_build_info *lsiv; int index; @@ -12994,7 +13108,9 @@ build_lswitch_and_lrouter_flows(struct hmap *datapaths, struct hmap *ports, * on a per-bucket level instead of merging hash frags */ lsiv[index].lflows = lflows; } else { - fast_hmap_init(&lflow_segs[index], lflows->mask); + fast_hmap_init(&lflow_segs[index].single_od, + lflows->single_od.mask); + hmap_init(&lflow_segs[index].multiple_od); lsiv[index].lflows = &lflow_segs[index]; } @@ -13017,7 +13133,9 @@ build_lswitch_and_lrouter_flows(struct hmap *datapaths, struct hmap *ports, if (use_logical_dp_groups) { run_pool_callback(build_lflows_pool->pool, NULL, NULL, noop_callback); } else { - run_pool_hash(build_lflows_pool->pool, lflows, lflow_segs); + run_pool_callback(build_lflows_pool->pool, + &lflows->single_od, lflow_segs, + lflow_merge_callback); } for (index = 0; index < build_lflows_pool->pool->size; index++) { @@ -13157,6 +13275,57 @@ ovn_sb_set_lflow_logical_dp_group( static ssize_t max_seen_lflow_size = 128; +static void +reconcile_lflow(struct ovn_lflow *lflow, struct northd_context *ctx, + struct hmap *dp_groups, struct lflow_state *lflows) +{ + const struct sbrec_logical_flow *sbflow; + const char *pipeline = ovn_stage_get_pipeline_name(lflow->stage); + uint8_t table = ovn_stage_get_table(lflow->stage); + + sbflow = sbrec_logical_flow_insert(ctx->ovnsb_txn); + if (lflow->od) { + sbrec_logical_flow_set_logical_datapath(sbflow, lflow->od->sb); + } + ovn_sb_set_lflow_logical_dp_group(ctx, dp_groups, + sbflow, &lflow->od_group); + sbrec_logical_flow_set_pipeline(sbflow, pipeline); + sbrec_logical_flow_set_table_id(sbflow, table); + sbrec_logical_flow_set_priority(sbflow, lflow->priority); + sbrec_logical_flow_set_match(sbflow, lflow->match); + sbrec_logical_flow_set_actions(sbflow, lflow->actions); + if (lflow->io_port) { + struct smap tags = SMAP_INITIALIZER(&tags); + smap_add(&tags, "in_out_port", lflow->io_port); + sbrec_logical_flow_set_tags(sbflow, &tags); + smap_destroy(&tags); + } + sbrec_logical_flow_set_controller_meter(sbflow, lflow->ctrl_meter); + + /* Trim the source locator lflow->where, which looks something like + * "ovn/northd/ovn-northd.c:1234", down to just the part following the + * last slash, e.g. "ovn-northd.c:1234". */ + const char *slash = strrchr(lflow->where, '/'); +#if _WIN32 + const char *backslash = strrchr(lflow->where, '\\'); + if (!slash || backslash > slash) { + slash = backslash; + } +#endif + const char *where = slash ? slash + 1 : lflow->where; + + struct smap ids = SMAP_INITIALIZER(&ids); + smap_add(&ids, "stage-name", ovn_stage_to_str(lflow->stage)); + smap_add(&ids, "source", where); + if (lflow->stage_hint) { + smap_add(&ids, "stage-hint", lflow->stage_hint); + } + sbrec_logical_flow_set_external_ids(sbflow, &ids); + smap_destroy(&ids); + + ovn_lflow_destroy(lflows, lflow); +} + /* Updates the Logical_Flow and Multicast_Group tables in the OVN_SB database, * constructing their contents based on the OVN_NB database. */ static void @@ -13167,51 +13336,41 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths, struct hmap *lbs, struct hmap *bfd_connections, bool ovn_internal_version_changed) { - struct hmap lflows; + struct lflow_state lflows; - fast_hmap_size_for(&lflows, max_seen_lflow_size); - if (use_parallel_build) { - update_hashrow_locks(&lflows, &lflow_locks); + fast_hmap_size_for(&lflows.single_od, max_seen_lflow_size); + fast_hmap_size_for(&lflows.multiple_od, max_seen_lflow_size); + if (use_parallel_build && use_logical_dp_groups) { + update_hashrow_locks(&lflows.single_od, &lflow_locks); } + + build_lswitch_and_lrouter_flows(datapaths, ports, port_groups, &lflows, mcgroups, igmp_groups, meter_groups, lbs, bfd_connections); /* Parallel build may result in a suboptimal hash. Resize the - * hash to a correct size before doing lookups */ - - hmap_expand(&lflows); + * hash to a correct size before doing lookups. + * We need to do that only to the multiple_od hash. Single_od + * is recreated during processing, it will be the correct size + * at the point where reconciliation starts lookups. + */ - if (hmap_count(&lflows) > max_seen_lflow_size) { - max_seen_lflow_size = hmap_count(&lflows); - } + hmap_expand(&lflows.multiple_od); stopwatch_start(LFLOWS_DP_GROUPS_STOPWATCH_NAME, time_msec()); - /* Collecting all unique datapath groups. */ + /* Collecting all unique datapath groups. Works only with + * multiple_od hash and is not dependent on single_od. + * Can run in a separate thread after OVN updates to the + * newer version of the parallel API. + */ struct hmap dp_groups = HMAP_INITIALIZER(&dp_groups); - struct hmapx single_dp_lflows = HMAPX_INITIALIZER(&single_dp_lflows); - struct ovn_lflow *lflow; - HMAP_FOR_EACH (lflow, hmap_node, &lflows) { + struct ovn_lflow *lflow, *next_lflow; + + HMAP_FOR_EACH (lflow, hmap_node, &lflows.multiple_od) { uint32_t hash = hash_int(hmapx_count(&lflow->od_group), 0); struct ovn_dp_group *dpg; - - ovs_assert(hmapx_count(&lflow->od_group)); - - if (hmapx_count(&lflow->od_group) == 1) { - /* There is only one datapath, so it should be moved out of the - * group to a single 'od'. */ - const struct hmapx_node *node; - HMAPX_FOR_EACH (node, &lflow->od_group) { - lflow->od = node->data; - break; - } - hmapx_clear(&lflow->od_group); - /* Logical flow should be re-hashed later to allow lookups. */ - hmapx_add(&single_dp_lflows, lflow); - continue; - } - dpg = ovn_dp_group_find(&dp_groups, &lflow->od_group, hash); if (!dpg) { dpg = xzalloc(sizeof *dpg); @@ -13222,18 +13381,34 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths, } /* Adding datapath to the flow hash for logical flows that have only one, - * so they could be found by the southbound db record. */ - const struct hmapx_node *node; + * so they could be found by the southbound db record. + * Not dependent on multiple_od - same comment as above applies here. + */ + + struct hmap processed_single; + hmap_init(&processed_single); + uint32_t hash; - HMAPX_FOR_EACH (node, &single_dp_lflows) { - lflow = node->data; + struct hmapx_node *node; + HMAP_FOR_EACH_POP (lflow, hmap_node, &lflows.single_od) { + HMAPX_FOR_EACH (node, &lflow->od_group) { + lflow->od = node->data; + break; + } + hmapx_clear(&lflow->od_group); hash = hmap_node_hash(&lflow->hmap_node); - hmap_remove(&lflows, &lflow->hmap_node); hash = ovn_logical_flow_hash_datapath(&lflow->od->sb->header_.uuid, hash); - hmap_insert(&lflows, &lflow->hmap_node, hash); + hmap_insert(&processed_single, &lflow->hmap_node, hash); } - hmapx_destroy(&single_dp_lflows); + + hmap_destroy(&lflows.single_od); + + lflows.single_od = processed_single; + hmap_moved(&lflows.single_od); + + max_seen_lflow_size = MAX(hmap_count(&lflows.single_od), + hmap_count(&lflows.multiple_od)); /* Push changes to the Logical_Flow table to database. */ const struct sbrec_logical_flow *sbflow, *next_sbflow; @@ -13366,54 +13541,14 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths, } stopwatch_stop(LFLOWS_DP_GROUPS_STOPWATCH_NAME, time_msec()); - struct ovn_lflow *next_lflow; - HMAP_FOR_EACH_SAFE (lflow, next_lflow, hmap_node, &lflows) { - const char *pipeline = ovn_stage_get_pipeline_name(lflow->stage); - uint8_t table = ovn_stage_get_table(lflow->stage); - - sbflow = sbrec_logical_flow_insert(ctx->ovnsb_txn); - if (lflow->od) { - sbrec_logical_flow_set_logical_datapath(sbflow, lflow->od->sb); - } - ovn_sb_set_lflow_logical_dp_group(ctx, &dp_groups, - sbflow, &lflow->od_group); - sbrec_logical_flow_set_pipeline(sbflow, pipeline); - sbrec_logical_flow_set_table_id(sbflow, table); - sbrec_logical_flow_set_priority(sbflow, lflow->priority); - sbrec_logical_flow_set_match(sbflow, lflow->match); - sbrec_logical_flow_set_actions(sbflow, lflow->actions); - if (lflow->io_port) { - struct smap tags = SMAP_INITIALIZER(&tags); - smap_add(&tags, "in_out_port", lflow->io_port); - sbrec_logical_flow_set_tags(sbflow, &tags); - smap_destroy(&tags); - } - sbrec_logical_flow_set_controller_meter(sbflow, lflow->ctrl_meter); - - /* Trim the source locator lflow->where, which looks something like - * "ovn/northd/ovn-northd.c:1234", down to just the part following the - * last slash, e.g. "ovn-northd.c:1234". */ - const char *slash = strrchr(lflow->where, '/'); -#if _WIN32 - const char *backslash = strrchr(lflow->where, '\\'); - if (!slash || backslash > slash) { - slash = backslash; - } -#endif - const char *where = slash ? slash + 1 : lflow->where; - - struct smap ids = SMAP_INITIALIZER(&ids); - smap_add(&ids, "stage-name", ovn_stage_to_str(lflow->stage)); - smap_add(&ids, "source", where); - if (lflow->stage_hint) { - smap_add(&ids, "stage-hint", lflow->stage_hint); - } - sbrec_logical_flow_set_external_ids(sbflow, &ids); - smap_destroy(&ids); - - ovn_lflow_destroy(&lflows, lflow); + HMAP_FOR_EACH_SAFE (lflow, next_lflow, hmap_node, &lflows.single_od) { + reconcile_lflow(lflow, ctx, &dp_groups, &lflows); + } + HMAP_FOR_EACH_SAFE (lflow, next_lflow, hmap_node, &lflows.multiple_od) { + reconcile_lflow(lflow, ctx, &dp_groups, &lflows); } - hmap_destroy(&lflows); + hmap_destroy(&lflows.single_od); + hmap_destroy(&lflows.multiple_od); struct ovn_dp_group *dpg; HMAP_FOR_EACH_POP (dpg, node, &dp_groups) { From patchwork Fri Sep 10 14:13:21 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Anton Ivanov X-Patchwork-Id: 1526532 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.137; helo=smtp4.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4H5dCB16KCz9sX3 for ; Sat, 11 Sep 2021 00:13:42 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 2D9504018C; Fri, 10 Sep 2021 14:13:40 +0000 (UTC) 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 hnM4Hh-dzoeu; Fri, 10 Sep 2021 14:13:38 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp4.osuosl.org (Postfix) with ESMTPS id EC158401E5; Fri, 10 Sep 2021 14:13:37 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id C3EC2C0011; Fri, 10 Sep 2021 14:13:37 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 33BAFC000D for ; Fri, 10 Sep 2021 14:13:36 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 176A760C15 for ; Fri, 10 Sep 2021 14:13:36 +0000 (UTC) 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 5vpldgxAA9eG for ; Fri, 10 Sep 2021 14:13:34 +0000 (UTC) X-Greylist: from auto-whitelisted by SQLgrey-1.8.0 Received: from www.kot-begemot.co.uk (ivanoab7.miniserver.com [37.128.132.42]) by smtp3.osuosl.org (Postfix) with ESMTPS id 8867C60603 for ; Fri, 10 Sep 2021 14:13:34 +0000 (UTC) Received: from tun252.jain.kot-begemot.co.uk ([192.168.18.6] helo=jain.kot-begemot.co.uk) by www.kot-begemot.co.uk with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mOhHY-00080D-JI; Fri, 10 Sep 2021 14:13:33 +0000 Received: from jain.kot-begemot.co.uk ([192.168.3.3]) by jain.kot-begemot.co.uk with esmtp (Exim 4.92) (envelope-from ) id 1mOhHV-0003oz-0j; Fri, 10 Sep 2021 15:13:31 +0100 From: anton.ivanov@cambridgegreys.com To: ovs-dev@openvswitch.org Date: Fri, 10 Sep 2021 15:13:21 +0100 Message-Id: <20210910141321.14624-4-anton.ivanov@cambridgegreys.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20210910141321.14624-1-anton.ivanov@cambridgegreys.com> References: <20210910141321.14624-1-anton.ivanov@cambridgegreys.com> MIME-Version: 1.0 X-Clacks-Overhead: GNU Terry Pratchett Cc: i.maximets@ovn.org, Anton Ivanov Subject: [ovs-dev] [OVN Patch v7 4/4] northd: Restore parallel build with dp_groups 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: Anton Ivanov Restore parallel build with dp groups using rwlock instead of per row locking as an underlying mechanism. This provides improvement ~ 10% end-to-end on ovn-heater under virutalization despite awakening some qemu gremlin which makes qemu climb to silly CPU usage. The gain on bare metal is likely to be higher. Signed-off-by: Anton Ivanov --- northd/ovn-northd.c | 299 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 234 insertions(+), 65 deletions(-) diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index aee5b9508..16e39ec5e 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -59,6 +59,7 @@ #include "unixctl.h" #include "util.h" #include "uuid.h" +#include "ovs-thread.h" #include "openvswitch/vlog.h" VLOG_DEFINE_THIS_MODULE(ovn_northd); @@ -4372,72 +4373,219 @@ ovn_lflow_init(struct ovn_lflow *lflow, struct ovn_datapath *od, static bool use_logical_dp_groups = false; static bool use_parallel_build = true; -static struct hashrow_locks lflow_locks; +/* This lock is used to lock the lflow table and all related structures. + * It cannot be a mutex, because most of the accesses are read and there is + * only an occasional write change. + */ + +static struct ovs_rwlock flowtable_lock; + +static void ovn_make_multi_lflow(struct ovn_lflow *old_lflow, + struct ovn_datapath *od, + struct lflow_state *lflow_map, + uint32_t hash) +{ + hmapx_add(&old_lflow->od_group, od); + hmap_remove(&lflow_map->single_od, &old_lflow->hmap_node); + if (use_parallel_build) { + hmap_insert_fast(&lflow_map->multiple_od, &old_lflow->hmap_node, hash); + } else { + hmap_insert(&lflow_map->multiple_od, &old_lflow->hmap_node, hash); + } +} /* Adds a row with the specified contents to the Logical_Flow table. - * Version to use when locking is required. + * + * Assumptions: + * + * 1. A large proportion of the operations are lookups (reads). + * 2. RW operations are a small proportion of overall adds. + * + * Principles of operation: + * 1. All accesses to the flow table are protected by a rwlock. + * 2. By default, everyone grabs a rd lock so that multiple threads + * can do lookups simultaneously. + * 3. If a change is needed, the rd lock is released and a wr lock + * is acquired instead (the fact that POSIX does not have an "upgrade" + * on locks is a major pain, but there is nothing we can do - it's not + * there). + * 4. WR lock operations in rd/wr locking have LOWER priority than RD. + * That is by design and spec. So a request for WR lock may wait for a + * considerable amount of time until it is given a change to lock. That + * means that another thread may get there in the meantime and change + * the data. Hence all wr operations MUST be coded to ensure that they + * are not vulnerable to "someone pulled this from under my feet". Re- + * reads, checks for presense, etc. + */ + +/* The code which follows is executed both as single thread and parallel. + * When executed as a single thread locking, re-reading after a lock change + * from rd to wr, etc are not needed and that path does not lock. + * clang thread safety analyzer cannot quite get that idea so we have to + * disable it. */ + static struct ovn_lflow * do_ovn_lflow_add(struct lflow_state *lflow_map, struct ovn_datapath *od, uint32_t hash, enum ovn_stage stage, uint16_t priority, const char *match, const char *actions, const char *io_port, const struct ovsdb_idl_row *stage_hint, const char *where, const char *ctrl_meter) + OVS_NO_THREAD_SAFETY_ANALYSIS { struct ovn_lflow *old_lflow; struct ovn_lflow *lflow; if (use_logical_dp_groups) { - /* Look up in multiple first. */ - old_lflow = do_ovn_lflow_find(&lflow_map->multiple_od, NULL, stage, - priority, match, + if (use_parallel_build) { + /* Fast Path. In case we run in parallel, see if we + * can get away without writing - grab a rdlock and check + * if we can get away with as little work as possible. + */ + ovs_rwlock_rdlock(&flowtable_lock); + } + + /* Look up multiple_od first. That is the more common + * lookup. + */ + + old_lflow = do_ovn_lflow_find(&lflow_map->multiple_od, + NULL, stage, priority, match, actions, ctrl_meter, hash); + if (old_lflow) { - hmapx_add(&old_lflow->od_group, od); + /* Found, amend od_group. */ + if (!use_parallel_build) { + hmapx_add(&old_lflow->od_group, od); + } else { + /* See if we need to add this od before upgrading + * the rd lock to a wr. + */ + if (!hmapx_contains(&old_lflow->od_group, od)) { + if (use_parallel_build) { + /* Upgrade lock to write.*/ + ovs_rwlock_unlock(&flowtable_lock); + ovs_rwlock_wrlock(&flowtable_lock); + } + /* Add the flow to the group. NOOP if it + * exists in it. */ + hmapx_add(&old_lflow->od_group, od); + } + } } else { /* Not found, lookup in single od. */ old_lflow = do_ovn_lflow_find(&lflow_map->single_od, NULL, stage, priority, match, actions, ctrl_meter, hash); if (old_lflow) { - hmapx_add(&old_lflow->od_group, od); - /* Found, different, od count went up. Move to multiple od. */ - if (hmapx_count(&old_lflow->od_group) > 1) { - hmap_remove(&lflow_map->single_od, &old_lflow->hmap_node); + /* We found an old single od flow. See if we need to + * update it. + */ + if (!hmapx_contains(&old_lflow->od_group, od)) { + /* od not in od_group, we need to add it. The flow + * becomes a multi-od flow so it must move to + * multiple_od. */ if (use_parallel_build) { - hmap_insert_fast(&lflow_map->multiple_od, - &old_lflow->hmap_node, hash); - } else { - hmap_insert(&lflow_map->multiple_od, - &old_lflow->hmap_node, hash); + /* Upgrade the lock to write, we are likely to + * modify data. Note - this may take a while, + * so someone may modify the data in the meantime. + */ + ovs_rwlock_unlock(&flowtable_lock); + ovs_rwlock_wrlock(&flowtable_lock); + + /* Check if someone modified the data in the meantime. + */ + if (!hmap_contains(&lflow_map->single_od, + &old_lflow->hmap_node)) { + /* Someone modified the data already. The flow is + * already in multiple_od. Add the od to + * the group. If it exists this is a NOOP. */ + hmapx_add(&old_lflow->od_group, od); + goto done_update_unlock; + } } + ovn_make_multi_lflow(old_lflow, od, lflow_map, hash); } } } +done_update_unlock: + if (use_parallel_build) { + ovs_rwlock_unlock(&flowtable_lock); + } if (old_lflow) { return old_lflow; } } - lflow = xmalloc(sizeof *lflow); - /* While adding new logical flows we're not setting single datapath, but - * collecting a group. 'od' will be updated later for all flows with only - * one datapath in a group, so it could be hashed correctly. */ - ovn_lflow_init(lflow, NULL, stage, priority, - xstrdup(match), xstrdup(actions), - io_port ? xstrdup(io_port) : NULL, - nullable_xstrdup(ctrl_meter), - ovn_lflow_hint(stage_hint), where); - hmapx_add(&lflow->od_group, od); - - /* Insert "fresh" lflows into single_od. */ - + if (use_logical_dp_groups && use_parallel_build) { + /* Slow Path - insert a new flow. + * We could not get away with minimal mostly ro amount of work. + * We are likely to be modifying data, so we go ahead and + * lock with rw and try to do an insert (may end up repeating + * some of what we do for ro). */ + ovs_rwlock_wrlock(&flowtable_lock); + } if (!use_parallel_build) { + lflow = xmalloc(sizeof *lflow); + /* While adding new logical flows we are not setting single datapath, + * but collecting a group. 'od' will be updated later for all flows + * with only one datapath in a group, so it could be hashed correctly. + */ + ovn_lflow_init(lflow, NULL, stage, priority, + xstrdup(match), xstrdup(actions), + io_port ? xstrdup(io_port) : NULL, + nullable_xstrdup(ctrl_meter), + ovn_lflow_hint(stage_hint), where); + hmapx_add(&lflow->od_group, od); hmap_insert(&lflow_map->single_od, &lflow->hmap_node, hash); } else { + if (use_logical_dp_groups) { + /* Search again in case someone added the flow before us. */ + lflow = do_ovn_lflow_find(&lflow_map->single_od, + NULL, stage, priority, match, + actions, ctrl_meter, hash); + if (lflow) { + /* Someone added the flow before us, see if we need to + * convert it into a multi_od flow. */ + if (!hmapx_contains(&lflow->od_group, od)) { + ovn_make_multi_lflow(lflow, od, lflow_map, hash); + } + goto done_add_unlock; + } + /* Unlikely, but possible, check if than one thread got here + * ahead of us while we were wating to acquire a write lock. + * The flow is not just in the database, but it already has + * more than one od assigned to it. + */ + lflow = do_ovn_lflow_find(&lflow_map->multiple_od, NULL, + stage, priority, match, + actions, ctrl_meter, hash); + if (lflow) { + hmapx_add(&lflow->od_group, od); + goto done_add_unlock; + } + } + /* All race possibilities where a flow has been inserted by + * another thread while we are waiting for a lock have been + * handled, we can go ahead, alloc and insert. */ + lflow = xmalloc(sizeof *lflow); + /* While adding new logical flows we're not setting single datapath, + * but collecting a group. 'od' will be updated later for all + * flows with only * one datapath in a group, so it could be hashed + * correctly. */ + ovn_lflow_init(lflow, NULL, stage, priority, + xstrdup(match), xstrdup(actions), + io_port ? xstrdup(io_port) : NULL, + nullable_xstrdup(ctrl_meter), + ovn_lflow_hint(stage_hint), where); + hmapx_add(&lflow->od_group, od); hmap_insert_fast(&lflow_map->single_od, &lflow->hmap_node, hash); } +done_add_unlock: + if (use_logical_dp_groups && use_parallel_build) { + ovs_rwlock_unlock(&flowtable_lock); + } return lflow; } @@ -4450,20 +4598,11 @@ ovn_lflow_add_at_with_hash(struct lflow_state *lflow_map, const struct ovsdb_idl_row *stage_hint, const char *where, uint32_t hash) { - struct ovn_lflow *lflow; - ovs_assert(ovn_stage_to_datapath_type(stage) == ovn_datapath_get_type(od)); - if (use_logical_dp_groups && use_parallel_build) { - lock_hash_row(&lflow_locks, hash); - lflow = do_ovn_lflow_add(lflow_map, od, hash, stage, priority, match, + + return do_ovn_lflow_add(lflow_map, od, hash, stage, priority, match, actions, io_port, stage_hint, where, ctrl_meter); - unlock_hash_row(&lflow_locks, hash); - } else { - lflow = do_ovn_lflow_add(lflow_map, od, hash, stage, priority, match, - actions, io_port, stage_hint, where, ctrl_meter); - } - return lflow; } /* Adds a row with the specified contents to the Logical_Flow table. */ @@ -4484,22 +4623,16 @@ ovn_lflow_add_at(struct lflow_state *lflow_map, struct ovn_datapath *od, io_port, ctrl_meter, stage_hint, where, hash); } + static bool ovn_dp_group_add_with_reference(struct ovn_lflow *lflow_ref, - struct ovn_datapath *od, - uint32_t hash) + struct ovn_datapath *od) { if (!use_logical_dp_groups || !lflow_ref) { return false; } - if (use_parallel_build) { - lock_hash_row(&lflow_locks, hash); - hmapx_add(&lflow_ref->od_group, od); - unlock_hash_row(&lflow_locks, hash); - } else { - hmapx_add(&lflow_ref->od_group, od); - } + hmapx_add(&lflow_ref->od_group, od); return true; } @@ -4595,9 +4728,6 @@ hmap_safe_remove(struct hmap *hmap, struct hmap_node *node) static void remove_lflow_from_lflows(struct lflow_state *lflows, struct ovn_lflow *lflow) { - if (use_logical_dp_groups && use_parallel_build) { - lock_hash_row(&lflow_locks, lflow->hmap_node.hash); - } if (hmapx_count(&lflow->od_group) > 1) { if (!hmap_safe_remove(&lflows->multiple_od, &lflow->hmap_node)) { hmap_remove(&lflows->single_od, &lflow->hmap_node); @@ -4607,9 +4737,6 @@ remove_lflow_from_lflows(struct lflow_state *lflows, struct ovn_lflow *lflow) hmap_remove(&lflows->multiple_od, &lflow->hmap_node); } } - if (use_logical_dp_groups && use_parallel_build) { - unlock_hash_row(&lflow_locks, lflow->hmap_node.hash); - } } static void @@ -6518,8 +6645,17 @@ build_lb_rules(struct lflow_state *lflows, struct ovn_northd_lb *lb, if (reject) { meter = copp_meter_get(COPP_REJECT, od->nbs->copp, meter_groups); - } else if (ovn_dp_group_add_with_reference(lflow_ref, od, hash)) { - continue; + } else if (!use_parallel_build) { + /* We can use this shortcut only if running single threaded. + * If we are running multi-threaded, trying to use it means + * playing with locks in more than one place and that is a + * recipe for trouble. + * If running parallel we let the lflow_add logic sort out + * any duplicates and od groups. + */ + if (ovn_dp_group_add_with_reference(lflow_ref, od)) { + continue; + } } lflow_ref = ovn_lflow_add_at_with_hash(lflows, od, S_SWITCH_IN_STATEFUL, priority, @@ -9572,8 +9708,17 @@ build_lrouter_defrag_flows_for_lb(struct ovn_northd_lb *lb, ds_cstr(match), ds_cstr(&defrag_actions)); for (size_t j = 0; j < lb->n_nb_lr; j++) { struct ovn_datapath *od = lb->nb_lr[j]; - if (ovn_dp_group_add_with_reference(lflow_ref, od, hash)) { - continue; + if (!use_parallel_build) { + /* We can use this shortcut only if running single threaded. + * If we are running multi-threaded, trying to use it means + * playing with locks in more than one place and that is a + * recipe for trouble. + * If running parallel we let the lflow_add logic sort out + * any duplicates and od groups. + */ + if (ovn_dp_group_add_with_reference(lflow_ref, od)) { + continue; + } } lflow_ref = ovn_lflow_add_at_with_hash(lflows, od, S_ROUTER_IN_DEFRAG, prio, @@ -9636,8 +9781,17 @@ build_lflows_for_unreachable_vips(struct ovn_northd_lb *lb, continue; } - if (ovn_dp_group_add_with_reference(lflow_ref, peer->od, hash)) { - continue; + if (!use_parallel_build) { + /* We can use this shortcut only if running single threaded. + * If we are running multi-threaded, trying to use it means + * playing with locks in more than one place and that is a + * recipe for trouble. + * If running parallel we let the lflow_add logic sort out + * any duplicates and od groups. + */ + if (ovn_dp_group_add_with_reference(lflow_ref, peer->od)) { + continue; + } } lflow_ref = ovn_lflow_add_at_with_hash(lflows, peer->od, S_SWITCH_IN_L2_LKUP, 90, @@ -13088,7 +13242,7 @@ build_lswitch_and_lrouter_flows(struct hmap *datapaths, struct hmap *ports, } } - if (use_parallel_build && (!use_logical_dp_groups)) { + if (use_parallel_build) { struct lflow_state *lflow_segs; struct lswitch_flow_build_info *lsiv; int index; @@ -13326,6 +13480,9 @@ reconcile_lflow(struct ovn_lflow *lflow, struct northd_context *ctx, ovn_lflow_destroy(lflows, lflow); } +static bool needs_parallel_init = true; +static bool reset_parallel = false; + /* Updates the Logical_Flow and Multicast_Group tables in the OVN_SB database, * constructing their contents based on the OVN_NB database. */ static void @@ -13340,10 +13497,23 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths, fast_hmap_size_for(&lflows.single_od, max_seen_lflow_size); fast_hmap_size_for(&lflows.multiple_od, max_seen_lflow_size); - if (use_parallel_build && use_logical_dp_groups) { - update_hashrow_locks(&lflows.single_od, &lflow_locks); + + if (reset_parallel) { + /* Due to the way parallel is controlled via commands + * it is nearly impossible to trigger, because + * the command to turn parallel on will nearly always + * come after the first iteration */ + use_parallel_build = true; + reset_parallel = false; } + if (use_parallel_build && use_logical_dp_groups && + needs_parallel_init) { + ovs_rwlock_init(&flowtable_lock); + needs_parallel_init = false; + use_parallel_build = false; + reset_parallel = true; + } build_lswitch_and_lrouter_flows(datapaths, ports, port_groups, &lflows, mcgroups, @@ -13386,7 +13556,7 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths, */ struct hmap processed_single; - hmap_init(&processed_single); + fast_hmap_size_for(&processed_single, hmap_count(&lflows.single_od)); uint32_t hash; struct hmapx_node *node; @@ -15300,7 +15470,6 @@ main(int argc, char *argv[]) daemonize_complete(); - init_hash_row_locks(&lflow_locks); use_parallel_build = can_parallelize_hashes(false); /* We want to detect (almost) all changes to the ovn-nb db. */