From patchwork Tue Feb 7 11:42:55 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Maximets X-Patchwork-Id: 1738896 X-Patchwork-Delegate: mmichels@redhat.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.136; helo=smtp3.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4PB1Tz31L5z23jB for ; Tue, 7 Feb 2023 22:43:19 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id BC19B60E9D; Tue, 7 Feb 2023 11:43:16 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org BC19B60E9D 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 1nK0mTsI3ark; Tue, 7 Feb 2023 11:43:14 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp3.osuosl.org (Postfix) with ESMTPS id B622360B95; Tue, 7 Feb 2023 11:43:13 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org B622360B95 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id A2C10C007E; Tue, 7 Feb 2023 11:43:12 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) by lists.linuxfoundation.org (Postfix) with ESMTP id B76B1C007E for ; Tue, 7 Feb 2023 11:43:11 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 86AE9414C8 for ; Tue, 7 Feb 2023 11:43:11 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 86AE9414C8 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 BJ27NSAC4W4V for ; Tue, 7 Feb 2023 11:43:09 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 59F1C410C4 Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by smtp4.osuosl.org (Postfix) with ESMTPS id 59F1C410C4 for ; Tue, 7 Feb 2023 11:43:09 +0000 (UTC) Received: (Authenticated sender: i.maximets@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id DAF571C000F; Tue, 7 Feb 2023 11:43:06 +0000 (UTC) From: Ilya Maximets To: ovs-dev@openvswitch.org Date: Tue, 7 Feb 2023 12:42:55 +0100 Message-Id: <20230207114302.1908836-2-i.maximets@ovn.org> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230207114302.1908836-1-i.maximets@ovn.org> References: <20230207114302.1908836-1-i.maximets@ovn.org> MIME-Version: 1.0 Cc: Dumitru Ceara , Ilya Maximets Subject: [ovs-dev] [PATCH ovn v2 1/8] northd: Use larger hash bucket locks instead of dp group ones. 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" Datapath group locks are taken very frequently while generating logical flows. Every time one new datapath is added to the group, northd takes a mutex, updates a bitmap and unlocks the mutex. The chances for contention on a datapath group lock are very low, but even without any contention these constant lock/unlock operations may take more than 20% of CPU cycles. We have another type of locks in northd - hash bucket locks which protect portions of the lflows hash map. The same locks can be used to protect datapath groups of logical flows in that hash map. As long as we're holding the lock for a portion of a hash map where this lflow is located, it's safe to modify the logical flow itself. Splitting out a pair of lflow_hash_lock/unlock() functions that can be used to lock a particular lflow hash bucket. Once the hash is calculated, we can now lock it, then preform an initial lflow creation and all the datapath group updates, and then unlock. This eliminates most of the lock/unlock operations while still maintaining a very low chance of contention. Testing with ovn-heater and 4 threads shows 10-25% performance improvement depending on a weight of lflow node processing in a particular test scenario. Hash bucket locks are conditional, so the thread safety analysis is disabled to avoid spurious warnings [1]. Also, since the order of taking two locks depends on a "random" hash value, no two locks can be nested, otherwise we'll risk facing ABBA deadlocks. For that reason, some of the loops are split in two. CoPP meters do not affect the hash value, so they do not affect the locking scheme. Unfortunately, the build_lrouter_nat_flows_for_lb() is too complex to benefit from the change right away. It will be re-worked separately in the next commits. [1] https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#no-conditionally-held-locks Signed-off-by: Ilya Maximets --- northd/northd.c | 163 ++++++++++++++++++++++++++++-------------------- 1 file changed, 94 insertions(+), 69 deletions(-) diff --git a/northd/northd.c b/northd/northd.c index 77e105b86..f5e18ef15 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -5006,7 +5006,6 @@ struct ovn_lflow { struct hmap_node hmap_node; struct ovn_datapath *od; /* 'logical_datapath' in SB schema. */ - struct ovs_mutex dpg_lock; /* Lock guarding access to 'dpg_bitmap' */ unsigned long *dpg_bitmap; /* Bitmap of all datapaths by their 'index'.*/ enum ovn_stage stage; uint16_t priority; @@ -5073,29 +5072,6 @@ ovn_lflow_init(struct ovn_lflow *lflow, struct ovn_datapath *od, lflow->ctrl_meter = ctrl_meter; lflow->dpg = NULL; lflow->where = where; - if (parallelization_state != STATE_NULL) { - ovs_mutex_init(&lflow->dpg_lock); - } -} - -static bool -ovn_dp_group_add_with_reference(struct ovn_lflow *lflow_ref, - struct ovn_datapath *od) - OVS_NO_THREAD_SAFETY_ANALYSIS -{ - if (!lflow_ref) { - return false; - } - - if (parallelization_state == STATE_USE_PARALLELIZATION) { - ovs_mutex_lock(&lflow_ref->dpg_lock); - bitmap_set1(lflow_ref->dpg_bitmap, od->index); - ovs_mutex_unlock(&lflow_ref->dpg_lock); - } else { - bitmap_set1(lflow_ref->dpg_bitmap, od->index); - } - - return true; } /* The lflow_hash_lock is a mutex array that protects updates to the shared @@ -5136,6 +5112,30 @@ lflow_hash_lock_destroy(void) lflow_hash_lock_initialized = false; } +static struct ovs_mutex * +lflow_hash_lock(const struct hmap *lflow_map, uint32_t hash) + OVS_NO_THREAD_SAFETY_ANALYSIS +{ + struct ovs_mutex *hash_lock = NULL; + + if (parallelization_state == STATE_USE_PARALLELIZATION) { + hash_lock = + &lflow_hash_locks[hash & lflow_map->mask & LFLOW_HASH_LOCK_MASK]; + ovs_mutex_lock(hash_lock); + } + return hash_lock; +} + +static void +lflow_hash_unlock(struct ovs_mutex *hash_lock) + OVS_NO_THREAD_SAFETY_ANALYSIS +{ + if (hash_lock) { + ovs_mutex_unlock(hash_lock); + } +} + + /* This thread-local var is used for parallel lflow building when dp-groups is * enabled. It maintains the number of lflows inserted by the current thread to * the shared lflow hmap in the current iteration. It is needed because the @@ -5148,6 +5148,22 @@ lflow_hash_lock_destroy(void) * */ static thread_local size_t thread_lflow_counter = 0; +/* Adds an OVN datapath to a datapath group of existing logical flow. + * Version to use when hash bucket locking is NOT required or the corresponding + * hash lock is already taken. */ +static bool +ovn_dp_group_add_with_reference(struct ovn_lflow *lflow_ref, + struct ovn_datapath *od) + OVS_NO_THREAD_SAFETY_ANALYSIS +{ + if (!lflow_ref) { + return false; + } + + bitmap_set1(lflow_ref->dpg_bitmap, od->index); + return true; +} + /* Adds a row with the specified contents to the Logical_Flow table. * Version to use when hash bucket locking is NOT required. */ @@ -5189,28 +5205,8 @@ do_ovn_lflow_add(struct hmap *lflow_map, struct ovn_datapath *od, } /* Adds a row with the specified contents to the Logical_Flow table. - * Version to use when hash bucket locking IS required. - */ -static struct ovn_lflow * -do_ovn_lflow_add_pd(struct hmap *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) -{ - - struct ovn_lflow *lflow; - struct ovs_mutex *hash_lock = - &lflow_hash_locks[hash & lflow_map->mask & LFLOW_HASH_LOCK_MASK]; - - ovs_mutex_lock(hash_lock); - lflow = do_ovn_lflow_add(lflow_map, od, hash, stage, priority, match, - actions, io_port, stage_hint, where, ctrl_meter); - ovs_mutex_unlock(hash_lock); - return lflow; -} - + * Version to use when hash is pre-computed and a hash bucket is already + * locked if necessary. */ static struct ovn_lflow * ovn_lflow_add_at_with_hash(struct hmap *lflow_map, struct ovn_datapath *od, enum ovn_stage stage, uint16_t priority, @@ -5222,14 +5218,9 @@ ovn_lflow_add_at_with_hash(struct hmap *lflow_map, struct ovn_datapath *od, struct ovn_lflow *lflow; ovs_assert(ovn_stage_to_datapath_type(stage) == ovn_datapath_get_type(od)); - if (parallelization_state == STATE_USE_PARALLELIZATION) { - lflow = do_ovn_lflow_add_pd(lflow_map, od, hash, stage, priority, - match, actions, io_port, stage_hint, where, - ctrl_meter); - } else { - lflow = do_ovn_lflow_add(lflow_map, od, hash, stage, priority, match, - actions, io_port, stage_hint, where, ctrl_meter); - } + + lflow = do_ovn_lflow_add(lflow_map, od, hash, stage, priority, match, + actions, io_port, stage_hint, where, ctrl_meter); return lflow; } @@ -5241,14 +5232,18 @@ ovn_lflow_add_at(struct hmap *lflow_map, struct ovn_datapath *od, const char *ctrl_meter, const struct ovsdb_idl_row *stage_hint, const char *where) { + struct ovs_mutex *hash_lock; uint32_t hash; hash = ovn_logical_flow_hash(ovn_stage_get_table(stage), ovn_stage_get_pipeline(stage), priority, match, actions); + + hash_lock = lflow_hash_lock(lflow_map, hash); ovn_lflow_add_at_with_hash(lflow_map, od, stage, priority, match, actions, io_port, ctrl_meter, stage_hint, where, hash); + lflow_hash_unlock(hash_lock); } static void @@ -5322,9 +5317,6 @@ static void ovn_lflow_destroy(struct hmap *lflows, struct ovn_lflow *lflow) { if (lflow) { - if (parallelization_state != STATE_NULL) { - ovs_mutex_destroy(&lflow->dpg_lock); - } if (lflows) { hmap_remove(lflows, &lflow->hmap_node); } @@ -7064,6 +7056,7 @@ build_lb_rules_pre_stateful(struct hmap *lflows, struct ovn_northd_lb *lb, ovn_stage_get_table(S_SWITCH_IN_PRE_STATEFUL), ovn_stage_get_pipeline(S_SWITCH_IN_PRE_STATEFUL), 120, ds_cstr(match), ds_cstr(action)); + struct ovs_mutex *hash_lock = lflow_hash_lock(lflows, hash); for (size_t j = 0; j < lb->n_nb_ls; j++) { struct ovn_datapath *od = lb->nb_ls[j]; @@ -7076,6 +7069,7 @@ build_lb_rules_pre_stateful(struct hmap *lflows, struct ovn_northd_lb *lb, OVS_SOURCE_LOCATOR, hash); } } + lflow_hash_unlock(hash_lock); } } @@ -7134,6 +7128,7 @@ build_lb_affinity_lr_flows(struct hmap *lflows, struct ovn_northd_lb *lb, ovn_stage_get_table(S_ROUTER_IN_LB_AFF_CHECK), ovn_stage_get_pipeline(S_ROUTER_IN_LB_AFF_CHECK), 100, new_lb_match, aff_check); + struct ovs_mutex *hash_lock = lflow_hash_lock(lflows, hash_aff_check); for (size_t i = 0; i < n_dplist; i++) { if (!ovn_dp_group_add_with_reference(lflow_ref_aff_check, dplist[i])) { @@ -7143,6 +7138,7 @@ build_lb_affinity_lr_flows(struct hmap *lflows, struct ovn_northd_lb *lb, OVS_SOURCE_LOCATOR, hash_aff_check); } } + lflow_hash_unlock(hash_lock); struct ds aff_action = DS_EMPTY_INITIALIZER; struct ds aff_action_learn = DS_EMPTY_INITIALIZER; @@ -7244,12 +7240,8 @@ build_lb_affinity_lr_flows(struct hmap *lflows, struct ovn_northd_lb *lb, ovn_stage_get_table(S_ROUTER_IN_LB_AFF_LEARN), ovn_stage_get_pipeline(S_ROUTER_IN_LB_AFF_LEARN), 100, ds_cstr(&aff_match_learn), ds_cstr(&aff_action_learn)); - - struct ovn_lflow *lflow_ref_aff_lb = NULL; - uint32_t hash_aff_lb = ovn_logical_flow_hash( - ovn_stage_get_table(S_ROUTER_IN_DNAT), - ovn_stage_get_pipeline(S_ROUTER_IN_DNAT), - 150, ds_cstr(&aff_match), ds_cstr(&aff_action)); + struct ovs_mutex *hash_lock_learn = lflow_hash_lock(lflows, + hash_aff_learn); for (size_t j = 0; j < n_dplist; j++) { /* Forward to OFTABLE_CHK_LB_AFFINITY table to store flow tuple. */ @@ -7261,6 +7253,17 @@ build_lb_affinity_lr_flows(struct hmap *lflows, struct ovn_northd_lb *lb, NULL, NULL, &lb->nlb->header_, OVS_SOURCE_LOCATOR, hash_aff_learn); } + } + lflow_hash_unlock(hash_lock_learn); + + struct ovn_lflow *lflow_ref_aff_lb = NULL; + uint32_t hash_aff_lb = ovn_logical_flow_hash( + ovn_stage_get_table(S_ROUTER_IN_DNAT), + ovn_stage_get_pipeline(S_ROUTER_IN_DNAT), + 150, ds_cstr(&aff_match), ds_cstr(&aff_action)); + struct ovs_mutex *hash_lock_lb = lflow_hash_lock(lflows, hash_aff_lb); + + for (size_t j = 0; j < n_dplist; j++) { /* Use already selected backend within affinity timeslot. */ if (!ovn_dp_group_add_with_reference(lflow_ref_aff_lb, dplist[j])) { @@ -7271,6 +7274,7 @@ build_lb_affinity_lr_flows(struct hmap *lflows, struct ovn_northd_lb *lb, hash_aff_lb); } } + lflow_hash_unlock(hash_lock_lb); ds_truncate(&aff_action, aff_action_len); ds_truncate(&aff_action_learn, aff_action_learn_len); @@ -7357,6 +7361,7 @@ build_lb_affinity_ls_flows(struct hmap *lflows, struct ovn_northd_lb *lb, ovn_stage_get_table(S_SWITCH_IN_LB_AFF_CHECK), ovn_stage_get_pipeline(S_SWITCH_IN_LB_AFF_CHECK), 100, ds_cstr(&new_lb_match), aff_check); + struct ovs_mutex *hash_lock = lflow_hash_lock(lflows, hash_aff_check); for (size_t i = 0; i < lb->n_nb_ls; i++) { if (!ovn_dp_group_add_with_reference(lflow_ref_aff_check, @@ -7367,6 +7372,7 @@ build_lb_affinity_ls_flows(struct hmap *lflows, struct ovn_northd_lb *lb, &lb->nlb->header_, OVS_SOURCE_LOCATOR, hash_aff_check); } } + lflow_hash_unlock(hash_lock); ds_destroy(&new_lb_match); struct ds aff_action = DS_EMPTY_INITIALIZER; @@ -7457,12 +7463,8 @@ build_lb_affinity_ls_flows(struct hmap *lflows, struct ovn_northd_lb *lb, ovn_stage_get_table(S_SWITCH_IN_LB_AFF_LEARN), ovn_stage_get_pipeline(S_SWITCH_IN_LB_AFF_LEARN), 100, ds_cstr(&aff_match_learn), ds_cstr(&aff_action_learn)); - - struct ovn_lflow *lflow_ref_aff_lb = NULL; - uint32_t hash_aff_lb = ovn_logical_flow_hash( - ovn_stage_get_table(S_SWITCH_IN_LB), - ovn_stage_get_pipeline(S_SWITCH_IN_LB), - 150, ds_cstr(&aff_match), ds_cstr(&aff_action)); + struct ovs_mutex *hash_lock_learn = lflow_hash_lock(lflows, + hash_aff_learn); for (size_t j = 0; j < lb->n_nb_ls; j++) { /* Forward to OFTABLE_CHK_LB_AFFINITY table to store flow tuple. */ @@ -7474,6 +7476,17 @@ build_lb_affinity_ls_flows(struct hmap *lflows, struct ovn_northd_lb *lb, NULL, NULL, &lb->nlb->header_, OVS_SOURCE_LOCATOR, hash_aff_learn); } + } + lflow_hash_unlock(hash_lock_learn); + + struct ovn_lflow *lflow_ref_aff_lb = NULL; + uint32_t hash_aff_lb = ovn_logical_flow_hash( + ovn_stage_get_table(S_SWITCH_IN_LB), + ovn_stage_get_pipeline(S_SWITCH_IN_LB), + 150, ds_cstr(&aff_match), ds_cstr(&aff_action)); + struct ovs_mutex *hash_lock_lb = lflow_hash_lock(lflows, hash_aff_lb); + + for (size_t j = 0; j < lb->n_nb_ls; j++) { /* Use already selected backend within affinity timeslot. */ if (!ovn_dp_group_add_with_reference(lflow_ref_aff_lb, lb->nb_ls[j])) { @@ -7484,6 +7497,7 @@ build_lb_affinity_ls_flows(struct hmap *lflows, struct ovn_northd_lb *lb, hash_aff_lb); } } + lflow_hash_unlock(hash_lock_lb); ds_truncate(&aff_action, aff_action_len); ds_truncate(&aff_action_learn, aff_action_learn_len); @@ -7555,6 +7569,7 @@ build_lb_rules(struct hmap *lflows, struct ovn_northd_lb *lb, ovn_stage_get_table(S_SWITCH_IN_LB), ovn_stage_get_pipeline(S_SWITCH_IN_LB), priority, ds_cstr(match), ds_cstr(action)); + struct ovs_mutex *hash_lock = lflow_hash_lock(lflows, hash); for (size_t j = 0; j < lb->n_nb_ls; j++) { struct ovn_datapath *od = lb->nb_ls[j]; @@ -7572,6 +7587,7 @@ build_lb_rules(struct hmap *lflows, struct ovn_northd_lb *lb, lflow_ref = meter ? NULL : lflow; } } + lflow_hash_unlock(hash_lock); } } @@ -10574,10 +10590,13 @@ build_gw_lrouter_nat_flows_for_lb(struct lrouter_nat_lb_flows_ctx *ctx, const char *meter = NULL; struct lrouter_nat_lb_flow *new_flow = &ctx->new[type]; struct lrouter_nat_lb_flow *est_flow = &ctx->est[type]; + struct ovs_mutex *hash_lock; if (ctx->reject) { meter = copp_meter_get(COPP_REJECT, od->nbr->copp, ctx->meter_groups); } + + hash_lock = lflow_hash_lock(ctx->lflows, new_flow->hash); if (meter || !ovn_dp_group_add_with_reference(new_flow->lflow_ref, od)) { struct ovn_lflow *lflow = ovn_lflow_add_at_with_hash(ctx->lflows, od, S_ROUTER_IN_DNAT, ctx->prio, ds_cstr(ctx->new_match), @@ -10585,13 +10604,16 @@ build_gw_lrouter_nat_flows_for_lb(struct lrouter_nat_lb_flows_ctx *ctx, OVS_SOURCE_LOCATOR, new_flow->hash); new_flow->lflow_ref = meter ? NULL : lflow; } + lflow_hash_unlock(hash_lock); + hash_lock = lflow_hash_lock(ctx->lflows, est_flow->hash); if (!ovn_dp_group_add_with_reference(est_flow->lflow_ref, od)) { est_flow->lflow_ref = ovn_lflow_add_at_with_hash(ctx->lflows, od, S_ROUTER_IN_DNAT, ctx->prio, ds_cstr(ctx->est_match), est_flow->action, NULL, NULL, &ctx->lb->nlb->header_, OVS_SOURCE_LOCATOR, est_flow->hash); } + lflow_hash_unlock(hash_lock); } static void @@ -10878,6 +10900,8 @@ build_lrouter_defrag_flows_for_lb(struct ovn_northd_lb *lb, ovn_stage_get_table(S_ROUTER_IN_DEFRAG), ovn_stage_get_pipeline(S_ROUTER_IN_DEFRAG), prio, ds_cstr(match), ds_cstr(&defrag_actions)); + struct ovs_mutex *hash_lock = lflow_hash_lock(lflows, hash); + 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)) { @@ -10889,6 +10913,7 @@ build_lrouter_defrag_flows_for_lb(struct ovn_northd_lb *lb, NULL, NULL, &lb->nlb->header_, OVS_SOURCE_LOCATOR, hash); } + lflow_hash_unlock(hash_lock); } ds_destroy(&defrag_actions); } From patchwork Tue Feb 7 11:42:56 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Maximets X-Patchwork-Id: 1738897 X-Patchwork-Delegate: mmichels@redhat.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.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 ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4PB1V22Stpz23jB for ; Tue, 7 Feb 2023 22:43:22 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 666FB4174A; Tue, 7 Feb 2023 11:43:19 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 666FB4174A 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 O7768H9rGf-y; Tue, 7 Feb 2023 11:43:18 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp4.osuosl.org (Postfix) with ESMTPS id 74BCC417DC; Tue, 7 Feb 2023 11:43:15 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 74BCC417DC Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 956D3C0077; Tue, 7 Feb 2023 11:43:14 +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 BD76BC0077 for ; Tue, 7 Feb 2023 11:43:13 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 959FE40639 for ; Tue, 7 Feb 2023 11:43:13 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 959FE40639 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 R_EOH3DtgTNW for ; Tue, 7 Feb 2023 11:43:12 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 53AB540734 Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [IPv6:2001:4b98:dc4:8::225]) by smtp2.osuosl.org (Postfix) with ESMTPS id 53AB540734 for ; Tue, 7 Feb 2023 11:43:12 +0000 (UTC) Received: (Authenticated sender: i.maximets@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id 685FD1C0004; Tue, 7 Feb 2023 11:43:09 +0000 (UTC) From: Ilya Maximets To: ovs-dev@openvswitch.org Date: Tue, 7 Feb 2023 12:42:56 +0100 Message-Id: <20230207114302.1908836-3-i.maximets@ovn.org> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230207114302.1908836-1-i.maximets@ovn.org> References: <20230207114302.1908836-1-i.maximets@ovn.org> MIME-Version: 1.0 Cc: Dumitru Ceara , Ilya Maximets Subject: [ovs-dev] [PATCH ovn v2 2/8] northd: Add thread safety analysis to lflow hash locks. 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" Even though it's not possible to enable a full thread-safety analysis due to conditional mutex taking and unpredictable order [1], we can add most of the functionality with a fake mutex. It can detect nesting of hash locks, incorrect locking sequences and some other issues. More details are given in a form of a comment in the code to give this fake mutex more visibility. The fake mutex is declared as extern to not actually use any memory or trigger 'unused' warnings. No variables or structure fields are marked as GUARDED, because we still need a lockless access once the parallel part is over. [1] https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#no-conditionally-held-locks Signed-off-by: Ilya Maximets --- northd/northd.c | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/northd/northd.c b/northd/northd.c index f5e18ef15..4ddfc3af3 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -5112,8 +5112,28 @@ lflow_hash_lock_destroy(void) lflow_hash_lock_initialized = false; } +/* Full thread safety analysis is not possible with hash locks, because + * they are taken conditionally based on the 'parallelization_state' and + * a flow hash. Also, the order in which two hash locks are taken is not + * predictable during the static analysis. + * + * Since the order of taking two locks depends on a random hash, to avoid + * ABBA deadlocks, no two hash locks can be nested. In that sense an array + * of hash locks is similar to a single mutex. + * + * Using a fake mutex to partially simulate thread safety restrictions, as + * if it were actually a single mutex. + * + * OVS_NO_THREAD_SAFETY_ANALYSIS below allows us to ignore conditional + * nature of the lock. Unlike other attributes, it applies to the + * implementation and not to the interface. So, we can define a function + * that acquires the lock without analysing the way it does that. + */ +extern struct ovs_mutex fake_hash_mutex; + static struct ovs_mutex * lflow_hash_lock(const struct hmap *lflow_map, uint32_t hash) + OVS_ACQUIRES(fake_hash_mutex) OVS_NO_THREAD_SAFETY_ANALYSIS { struct ovs_mutex *hash_lock = NULL; @@ -5128,6 +5148,7 @@ lflow_hash_lock(const struct hmap *lflow_map, uint32_t hash) static void lflow_hash_unlock(struct ovs_mutex *hash_lock) + OVS_RELEASES(fake_hash_mutex) OVS_NO_THREAD_SAFETY_ANALYSIS { if (hash_lock) { @@ -5154,7 +5175,7 @@ static thread_local size_t thread_lflow_counter = 0; static bool ovn_dp_group_add_with_reference(struct ovn_lflow *lflow_ref, struct ovn_datapath *od) - OVS_NO_THREAD_SAFETY_ANALYSIS + OVS_REQUIRES(fake_hash_mutex) { if (!lflow_ref) { return false; @@ -5173,6 +5194,7 @@ do_ovn_lflow_add(struct hmap *lflow_map, struct ovn_datapath *od, 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_REQUIRES(fake_hash_mutex) { struct ovn_lflow *old_lflow; @@ -5214,6 +5236,7 @@ ovn_lflow_add_at_with_hash(struct hmap *lflow_map, struct ovn_datapath *od, const char *io_port, const char *ctrl_meter, const struct ovsdb_idl_row *stage_hint, const char *where, uint32_t hash) + OVS_REQUIRES(fake_hash_mutex) { struct ovn_lflow *lflow; @@ -5231,6 +5254,7 @@ ovn_lflow_add_at(struct hmap *lflow_map, struct ovn_datapath *od, const char *match, const char *actions, const char *io_port, const char *ctrl_meter, const struct ovsdb_idl_row *stage_hint, const char *where) + OVS_EXCLUDED(fake_hash_mutex) { struct ovs_mutex *hash_lock; uint32_t hash; From patchwork Tue Feb 7 11:42:57 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Maximets X-Patchwork-Id: 1738898 X-Patchwork-Delegate: mmichels@redhat.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::138; helo=smtp1.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4PB1V42xRXz23jB for ; Tue, 7 Feb 2023 22:43:24 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id D1E7B81E1A; Tue, 7 Feb 2023 11:43:20 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org D1E7B81E1A 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 4VX6DhU590PY; Tue, 7 Feb 2023 11:43:19 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp1.osuosl.org (Postfix) with ESMTPS id 6203A81DFB; Tue, 7 Feb 2023 11:43:17 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 6203A81DFB Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 5B632C007F; Tue, 7 Feb 2023 11:43:16 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) by lists.linuxfoundation.org (Postfix) with ESMTP id D783CC0033 for ; Tue, 7 Feb 2023 11:43:15 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id A08A1417DF for ; Tue, 7 Feb 2023 11:43:15 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org A08A1417DF 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 6fJgopWAtO0n for ; Tue, 7 Feb 2023 11:43:14 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 1646F41583 Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [IPv6:2001:4b98:dc4:8::225]) by smtp4.osuosl.org (Postfix) with ESMTPS id 1646F41583 for ; Tue, 7 Feb 2023 11:43:13 +0000 (UTC) Received: (Authenticated sender: i.maximets@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id 71BF71C0003; Tue, 7 Feb 2023 11:43:11 +0000 (UTC) From: Ilya Maximets To: ovs-dev@openvswitch.org Date: Tue, 7 Feb 2023 12:42:57 +0100 Message-Id: <20230207114302.1908836-4-i.maximets@ovn.org> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230207114302.1908836-1-i.maximets@ovn.org> References: <20230207114302.1908836-1-i.maximets@ovn.org> MIME-Version: 1.0 Cc: Dumitru Ceara , Ilya Maximets Subject: [ovs-dev] [PATCH ovn v2 3/8] northd: Optimize locking pattern for GW LR NAT flows for LB. 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" build_gw_lrouter_nat_flows_for_lb() function generates or adds one datapath to a group for two logical flows for one of 3 different flow types. This means that if we don't want to lock/unlcok for every operation on every call, we need to hold 6 different lflow hash locks. But hash locks can not be nested for thread-safety reasons. Instead, collecting all the affected datapaths into bitmaps per flow type and creating all the similar flows together, so we don't need to hold more than one hash lock at a time. Using bitmaps, since they require much less memory than arrays of pointers and generally easier to work with. This change on its own doesn't provide significant performance benefits, because constructing extra bitmaps takes extra time. But it provides necessary infrastructure for the next commits. Signed-off-by: Ilya Maximets --- northd/northd.c | 94 ++++++++++++++++++++++++++++++------------------- 1 file changed, 57 insertions(+), 37 deletions(-) diff --git a/northd/northd.c b/northd/northd.c index 4ddfc3af3..e06977f7c 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -10514,13 +10514,14 @@ get_force_snat_ip(struct ovn_datapath *od, const char *key_type, return true; } -#define LROUTER_NAT_LB_FLOW_INIT(MATCH, ACTION, PRIO) \ - (struct lrouter_nat_lb_flow) \ - { .action = (ACTION), .lflow_ref = NULL, \ - .hash = ovn_logical_flow_hash( \ - ovn_stage_get_table(S_ROUTER_IN_DNAT), \ - ovn_stage_get_pipeline(S_ROUTER_IN_DNAT), \ - (PRIO), ds_cstr(MATCH), (ACTION)) } +#define LROUTER_NAT_LB_FLOW_INIT(MATCH, ACTION, PRIO) \ + (struct lrouter_nat_lb_flow) { \ + .action = (ACTION), \ + .hash = ovn_logical_flow_hash( \ + ovn_stage_get_table(S_ROUTER_IN_DNAT), \ + ovn_stage_get_pipeline(S_ROUTER_IN_DNAT), \ + (PRIO), ds_cstr(MATCH), (ACTION)), \ + } enum lrouter_nat_lb_flow_type { LROUTER_NAT_LB_FLOW_NORMAL = 0, @@ -10531,8 +10532,6 @@ enum lrouter_nat_lb_flow_type { struct lrouter_nat_lb_flow { char *action; - struct ovn_lflow *lflow_ref; - uint32_t hash; }; @@ -10540,6 +10539,8 @@ struct lrouter_nat_lb_flows_ctx { struct lrouter_nat_lb_flow new[LROUTER_NAT_LB_FLOW_MAX]; struct lrouter_nat_lb_flow est[LROUTER_NAT_LB_FLOW_MAX]; + unsigned long *dp_bitmap[LROUTER_NAT_LB_FLOW_MAX]; + struct ds *new_match; struct ds *est_match; struct ds *undnat_match; @@ -10607,37 +10608,50 @@ build_distr_lrouter_nat_flows_for_lb(struct lrouter_nat_lb_flows_ctx *ctx, } static void -build_gw_lrouter_nat_flows_for_lb(struct lrouter_nat_lb_flows_ctx *ctx, - enum lrouter_nat_lb_flow_type type, - struct ovn_datapath *od) -{ - const char *meter = NULL; - struct lrouter_nat_lb_flow *new_flow = &ctx->new[type]; - struct lrouter_nat_lb_flow *est_flow = &ctx->est[type]; - struct ovs_mutex *hash_lock; +build_gw_lrouter_nat_flows_for_lb(struct lrouter_nat_lb_flows_ctx *ctx) +{ + for (size_t type = 0; type < LROUTER_NAT_LB_FLOW_MAX; type++) { + struct ovn_lflow *new_lflow_ref = NULL, *est_lflow_ref = NULL; + struct lrouter_nat_lb_flow *new_flow = &ctx->new[type]; + struct lrouter_nat_lb_flow *est_flow = &ctx->est[type]; + struct ovs_mutex *hash_lock; + size_t index; + + hash_lock = lflow_hash_lock(ctx->lflows, new_flow->hash); + BITMAP_FOR_EACH_1 (index, n_datapaths, ctx->dp_bitmap[type]) { + struct ovn_datapath *od = datapaths_array[index]; + const char *meter = NULL; + struct ovn_lflow *lflow; + + if (ctx->reject) { + meter = copp_meter_get(COPP_REJECT, od->nbr->copp, + ctx->meter_groups); + } - if (ctx->reject) { - meter = copp_meter_get(COPP_REJECT, od->nbr->copp, ctx->meter_groups); - } + if (meter || + !ovn_dp_group_add_with_reference(new_lflow_ref, od)) { + lflow = ovn_lflow_add_at_with_hash(ctx->lflows, od, + S_ROUTER_IN_DNAT, ctx->prio, ds_cstr(ctx->new_match), + new_flow->action, NULL, meter, &ctx->lb->nlb->header_, + OVS_SOURCE_LOCATOR, new_flow->hash); + new_lflow_ref = meter ? NULL : lflow; + } + } + lflow_hash_unlock(hash_lock); - hash_lock = lflow_hash_lock(ctx->lflows, new_flow->hash); - if (meter || !ovn_dp_group_add_with_reference(new_flow->lflow_ref, od)) { - struct ovn_lflow *lflow = ovn_lflow_add_at_with_hash(ctx->lflows, od, - S_ROUTER_IN_DNAT, ctx->prio, ds_cstr(ctx->new_match), - new_flow->action, NULL, meter, &ctx->lb->nlb->header_, - OVS_SOURCE_LOCATOR, new_flow->hash); - new_flow->lflow_ref = meter ? NULL : lflow; - } - lflow_hash_unlock(hash_lock); + hash_lock = lflow_hash_lock(ctx->lflows, est_flow->hash); + BITMAP_FOR_EACH_1 (index, n_datapaths, ctx->dp_bitmap[type]) { + struct ovn_datapath *od = datapaths_array[index]; - hash_lock = lflow_hash_lock(ctx->lflows, est_flow->hash); - if (!ovn_dp_group_add_with_reference(est_flow->lflow_ref, od)) { - est_flow->lflow_ref = ovn_lflow_add_at_with_hash(ctx->lflows, od, - S_ROUTER_IN_DNAT, ctx->prio, ds_cstr(ctx->est_match), - est_flow->action, NULL, NULL, &ctx->lb->nlb->header_, - OVS_SOURCE_LOCATOR, est_flow->hash); + if (!ovn_dp_group_add_with_reference(est_lflow_ref, od)) { + est_lflow_ref = ovn_lflow_add_at_with_hash(ctx->lflows, od, + S_ROUTER_IN_DNAT, ctx->prio, ds_cstr(ctx->est_match), + est_flow->action, NULL, NULL, &ctx->lb->nlb->header_, + OVS_SOURCE_LOCATOR, est_flow->hash); + } + } + lflow_hash_unlock(hash_lock); } - lflow_hash_unlock(hash_lock); } static void @@ -10748,6 +10762,10 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip *lb_vip, LROUTER_NAT_LB_FLOW_INIT(&est_match, "flags.force_snat_for_lb = 1; next;", prio); + for (size_t i = 0; i < LROUTER_NAT_LB_FLOW_MAX; i++) { + ctx.dp_bitmap[i] = bitmap_allocate(n_datapaths); + } + struct ovn_datapath **lb_aff_force_snat_router = xcalloc(lb->n_nb_lr, sizeof *lb_aff_force_snat_router); int n_lb_aff_force_snat_router = 0; @@ -10773,7 +10791,7 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip *lb_vip, } if (!od->n_l3dgw_ports) { - build_gw_lrouter_nat_flows_for_lb(&ctx, type, od); + bitmap_set1(ctx.dp_bitmap[type], od->index); } else { build_distr_lrouter_nat_flows_for_lb(&ctx, type, od); } @@ -10803,6 +10821,8 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip *lb_vip, } } + build_gw_lrouter_nat_flows_for_lb(&ctx); + /* LB affinity flows for datapaths where CMS has specified * force_snat_for_lb floag option. */ From patchwork Tue Feb 7 11:42:58 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Maximets X-Patchwork-Id: 1738899 X-Patchwork-Delegate: mmichels@redhat.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.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 ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4PB1V63x7nz23jB for ; Tue, 7 Feb 2023 22:43:26 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 5E411409E9; Tue, 7 Feb 2023 11:43:24 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 5E411409E9 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 gwr1HqEX1iUG; Tue, 7 Feb 2023 11:43:22 +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 600F840913; Tue, 7 Feb 2023 11:43:21 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 600F840913 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 16B5CC0077; Tue, 7 Feb 2023 11:43:21 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 13179C0033 for ; Tue, 7 Feb 2023 11:43:20 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 809F1417C5 for ; Tue, 7 Feb 2023 11:43:18 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 809F1417C5 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 4d5NJB4gqkDs for ; Tue, 7 Feb 2023 11:43:16 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 14670417DF Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by smtp4.osuosl.org (Postfix) with ESMTPS id 14670417DF for ; Tue, 7 Feb 2023 11:43:15 +0000 (UTC) Received: (Authenticated sender: i.maximets@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id 952581C0008; Tue, 7 Feb 2023 11:43:13 +0000 (UTC) From: Ilya Maximets To: ovs-dev@openvswitch.org Date: Tue, 7 Feb 2023 12:42:58 +0100 Message-Id: <20230207114302.1908836-5-i.maximets@ovn.org> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230207114302.1908836-1-i.maximets@ovn.org> References: <20230207114302.1908836-1-i.maximets@ovn.org> MIME-Version: 1.0 Cc: Dumitru Ceara , Ilya Maximets Subject: [ovs-dev] [PATCH ovn v2 4/8] northd: Use bitmaps for LB affinity flows. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" Previous commit used bitmaps to collect datapath groups for LR NAT LB flows. The same pattern can be applied to LB affinity flows in the same function. Signed-off-by: Ilya Maximets --- northd/northd.c | 142 ++++++++++++++++++++++++------------------------ 1 file changed, 72 insertions(+), 70 deletions(-) diff --git a/northd/northd.c b/northd/northd.c index e06977f7c..c0a63e4dc 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -7137,8 +7137,7 @@ build_lb_rules_pre_stateful(struct hmap *lflows, struct ovn_northd_lb *lb, static void build_lb_affinity_lr_flows(struct hmap *lflows, struct ovn_northd_lb *lb, struct ovn_lb_vip *lb_vip, char *new_lb_match, - char *lb_action, struct ovn_datapath **dplist, - int n_dplist) + char *lb_action, unsigned long *dp_bitmap) { if (!lb->affinity_timeout) { return; @@ -7153,11 +7152,14 @@ build_lb_affinity_lr_flows(struct hmap *lflows, struct ovn_northd_lb *lb, ovn_stage_get_pipeline(S_ROUTER_IN_LB_AFF_CHECK), 100, new_lb_match, aff_check); struct ovs_mutex *hash_lock = lflow_hash_lock(lflows, hash_aff_check); + size_t index; - for (size_t i = 0; i < n_dplist; i++) { - if (!ovn_dp_group_add_with_reference(lflow_ref_aff_check, dplist[i])) { + BITMAP_FOR_EACH_1 (index, n_datapaths, dp_bitmap) { + struct ovn_datapath *od = datapaths_array[index]; + + if (!ovn_dp_group_add_with_reference(lflow_ref_aff_check, od)) { lflow_ref_aff_check = ovn_lflow_add_at_with_hash( - lflows, dplist[i], S_ROUTER_IN_LB_AFF_CHECK, 100, + lflows, od, S_ROUTER_IN_LB_AFF_CHECK, 100, new_lb_match, aff_check, NULL, NULL, &lb->nlb->header_, OVS_SOURCE_LOCATOR, hash_aff_check); } @@ -7267,12 +7269,13 @@ build_lb_affinity_lr_flows(struct hmap *lflows, struct ovn_northd_lb *lb, struct ovs_mutex *hash_lock_learn = lflow_hash_lock(lflows, hash_aff_learn); - for (size_t j = 0; j < n_dplist; j++) { + BITMAP_FOR_EACH_1 (index, n_datapaths, dp_bitmap) { + struct ovn_datapath *od = datapaths_array[index]; + /* Forward to OFTABLE_CHK_LB_AFFINITY table to store flow tuple. */ - if (!ovn_dp_group_add_with_reference(lflow_ref_aff_learn, - dplist[j])) { + if (!ovn_dp_group_add_with_reference(lflow_ref_aff_learn, od)) { lflow_ref_aff_learn = ovn_lflow_add_at_with_hash( - lflows, dplist[j], S_ROUTER_IN_LB_AFF_LEARN, 100, + lflows, od, S_ROUTER_IN_LB_AFF_LEARN, 100, ds_cstr(&aff_match_learn), ds_cstr(&aff_action_learn), NULL, NULL, &lb->nlb->header_, OVS_SOURCE_LOCATOR, hash_aff_learn); @@ -7287,12 +7290,13 @@ build_lb_affinity_lr_flows(struct hmap *lflows, struct ovn_northd_lb *lb, 150, ds_cstr(&aff_match), ds_cstr(&aff_action)); struct ovs_mutex *hash_lock_lb = lflow_hash_lock(lflows, hash_aff_lb); - for (size_t j = 0; j < n_dplist; j++) { + BITMAP_FOR_EACH_1 (index, n_datapaths, dp_bitmap) { + struct ovn_datapath *od = datapaths_array[index]; + /* Use already selected backend within affinity timeslot. */ - if (!ovn_dp_group_add_with_reference(lflow_ref_aff_lb, - dplist[j])) { + if (!ovn_dp_group_add_with_reference(lflow_ref_aff_lb, od)) { lflow_ref_aff_lb = ovn_lflow_add_at_with_hash( - lflows, dplist[j], S_ROUTER_IN_DNAT, 150, + lflows, od, S_ROUTER_IN_DNAT, 150, ds_cstr(&aff_match), ds_cstr(&aff_action), NULL, NULL, &lb->nlb->header_, OVS_SOURCE_LOCATOR, hash_aff_lb); @@ -10539,8 +10543,6 @@ struct lrouter_nat_lb_flows_ctx { struct lrouter_nat_lb_flow new[LROUTER_NAT_LB_FLOW_MAX]; struct lrouter_nat_lb_flow est[LROUTER_NAT_LB_FLOW_MAX]; - unsigned long *dp_bitmap[LROUTER_NAT_LB_FLOW_MAX]; - struct ds *new_match; struct ds *est_match; struct ds *undnat_match; @@ -10608,50 +10610,50 @@ build_distr_lrouter_nat_flows_for_lb(struct lrouter_nat_lb_flows_ctx *ctx, } static void -build_gw_lrouter_nat_flows_for_lb(struct lrouter_nat_lb_flows_ctx *ctx) +build_gw_lrouter_nat_flows_for_lb(struct lrouter_nat_lb_flows_ctx *ctx, + enum lrouter_nat_lb_flow_type type, + unsigned long *dp_bitmap) { - for (size_t type = 0; type < LROUTER_NAT_LB_FLOW_MAX; type++) { - struct ovn_lflow *new_lflow_ref = NULL, *est_lflow_ref = NULL; - struct lrouter_nat_lb_flow *new_flow = &ctx->new[type]; - struct lrouter_nat_lb_flow *est_flow = &ctx->est[type]; - struct ovs_mutex *hash_lock; - size_t index; - - hash_lock = lflow_hash_lock(ctx->lflows, new_flow->hash); - BITMAP_FOR_EACH_1 (index, n_datapaths, ctx->dp_bitmap[type]) { - struct ovn_datapath *od = datapaths_array[index]; - const char *meter = NULL; - struct ovn_lflow *lflow; + struct ovn_lflow *new_lflow_ref = NULL, *est_lflow_ref = NULL; + struct lrouter_nat_lb_flow *new_flow = &ctx->new[type]; + struct lrouter_nat_lb_flow *est_flow = &ctx->est[type]; + struct ovs_mutex *hash_lock; + size_t index; - if (ctx->reject) { - meter = copp_meter_get(COPP_REJECT, od->nbr->copp, - ctx->meter_groups); - } + hash_lock = lflow_hash_lock(ctx->lflows, new_flow->hash); + BITMAP_FOR_EACH_1 (index, n_datapaths, dp_bitmap) { + struct ovn_datapath *od = datapaths_array[index]; + const char *meter = NULL; + struct ovn_lflow *lflow; - if (meter || - !ovn_dp_group_add_with_reference(new_lflow_ref, od)) { - lflow = ovn_lflow_add_at_with_hash(ctx->lflows, od, - S_ROUTER_IN_DNAT, ctx->prio, ds_cstr(ctx->new_match), - new_flow->action, NULL, meter, &ctx->lb->nlb->header_, - OVS_SOURCE_LOCATOR, new_flow->hash); - new_lflow_ref = meter ? NULL : lflow; - } + if (ctx->reject) { + meter = copp_meter_get(COPP_REJECT, od->nbr->copp, + ctx->meter_groups); } - lflow_hash_unlock(hash_lock); - hash_lock = lflow_hash_lock(ctx->lflows, est_flow->hash); - BITMAP_FOR_EACH_1 (index, n_datapaths, ctx->dp_bitmap[type]) { - struct ovn_datapath *od = datapaths_array[index]; + if (meter || + !ovn_dp_group_add_with_reference(new_lflow_ref, od)) { + lflow = ovn_lflow_add_at_with_hash(ctx->lflows, od, + S_ROUTER_IN_DNAT, ctx->prio, ds_cstr(ctx->new_match), + new_flow->action, NULL, meter, &ctx->lb->nlb->header_, + OVS_SOURCE_LOCATOR, new_flow->hash); + new_lflow_ref = meter ? NULL : lflow; + } + } + lflow_hash_unlock(hash_lock); - if (!ovn_dp_group_add_with_reference(est_lflow_ref, od)) { - est_lflow_ref = ovn_lflow_add_at_with_hash(ctx->lflows, od, - S_ROUTER_IN_DNAT, ctx->prio, ds_cstr(ctx->est_match), - est_flow->action, NULL, NULL, &ctx->lb->nlb->header_, - OVS_SOURCE_LOCATOR, est_flow->hash); - } + hash_lock = lflow_hash_lock(ctx->lflows, est_flow->hash); + BITMAP_FOR_EACH_1 (index, n_datapaths, dp_bitmap) { + struct ovn_datapath *od = datapaths_array[index]; + + if (!ovn_dp_group_add_with_reference(est_lflow_ref, od)) { + est_lflow_ref = ovn_lflow_add_at_with_hash(ctx->lflows, od, + S_ROUTER_IN_DNAT, ctx->prio, ds_cstr(ctx->est_match), + est_flow->action, NULL, NULL, &ctx->lb->nlb->header_, + OVS_SOURCE_LOCATOR, est_flow->hash); } - lflow_hash_unlock(hash_lock); } + lflow_hash_unlock(hash_lock); } static void @@ -10762,17 +10764,15 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip *lb_vip, LROUTER_NAT_LB_FLOW_INIT(&est_match, "flags.force_snat_for_lb = 1; next;", prio); - for (size_t i = 0; i < LROUTER_NAT_LB_FLOW_MAX; i++) { - ctx.dp_bitmap[i] = bitmap_allocate(n_datapaths); - } - - struct ovn_datapath **lb_aff_force_snat_router = - xcalloc(lb->n_nb_lr, sizeof *lb_aff_force_snat_router); - int n_lb_aff_force_snat_router = 0; + enum { + LROUTER_NAT_LB_AFF = LROUTER_NAT_LB_FLOW_MAX, + LROUTER_NAT_LB_AFF_FORCE_SNAT = LROUTER_NAT_LB_FLOW_MAX + 1, + }; + unsigned long *dp_bitmap[LROUTER_NAT_LB_FLOW_MAX + 2]; - struct ovn_datapath **lb_aff_router = - xcalloc(lb->n_nb_lr, sizeof *lb_aff_router); - int n_lb_aff_router = 0; + for (size_t i = 0; i < LROUTER_NAT_LB_FLOW_MAX + 2; i++) { + dp_bitmap[i] = bitmap_allocate(n_datapaths); + } /* Group gw router since we do not have datapath dependency in * lflow generation for them. @@ -10791,16 +10791,16 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip *lb_vip, } if (!od->n_l3dgw_ports) { - bitmap_set1(ctx.dp_bitmap[type], od->index); + bitmap_set1(dp_bitmap[type], od->index); } else { build_distr_lrouter_nat_flows_for_lb(&ctx, type, od); } if (!lport_addresses_is_empty(&od->lb_force_snat_addrs) || od->lb_force_snat_router_ip) { - lb_aff_force_snat_router[n_lb_aff_force_snat_router++] = od; + bitmap_set1(dp_bitmap[LROUTER_NAT_LB_AFF_FORCE_SNAT], od->index); } else { - lb_aff_router[n_lb_aff_router++] = od; + bitmap_set1(dp_bitmap[LROUTER_NAT_LB_AFF], od->index); } if (sset_contains(&od->external_ips, lb_vip->vip_str)) { @@ -10821,15 +10821,16 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip *lb_vip, } } - build_gw_lrouter_nat_flows_for_lb(&ctx); + for (size_t type = 0; type < LROUTER_NAT_LB_FLOW_MAX; type++) { + build_gw_lrouter_nat_flows_for_lb(&ctx, type, dp_bitmap[type]); + } /* LB affinity flows for datapaths where CMS has specified * force_snat_for_lb floag option. */ build_lb_affinity_lr_flows(lflows, lb, lb_vip, ds_cstr(match), "flags.force_snat_for_lb = 1; ", - lb_aff_force_snat_router, - n_lb_aff_force_snat_router); + dp_bitmap[LROUTER_NAT_LB_AFF_FORCE_SNAT]); /* LB affinity flows for datapaths where CMS has specified * skip_snat_for_lb floag option or regular datapaths. @@ -10837,7 +10838,7 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip *lb_vip, char *lb_aff_action = lb->skip_snat ? "flags.skip_snat_for_lb = 1; " : NULL; build_lb_affinity_lr_flows(lflows, lb, lb_vip, ds_cstr(match), - lb_aff_action, lb_aff_router, n_lb_aff_router); + lb_aff_action, dp_bitmap[LROUTER_NAT_LB_AFF]); ds_destroy(&unsnat_match); ds_destroy(&undnat_match); @@ -10845,8 +10846,9 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip *lb_vip, ds_destroy(&skip_snat_act); ds_destroy(&force_snat_act); - free(lb_aff_force_snat_router); - free(lb_aff_router); + for (size_t i = 0; i < LROUTER_NAT_LB_FLOW_MAX + 2; i++) { + bitmap_free(dp_bitmap[i]); + } } static void From patchwork Tue Feb 7 11:42:59 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Maximets X-Patchwork-Id: 1738900 X-Patchwork-Delegate: mmichels@redhat.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::136; helo=smtp3.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::136]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4PB1VG6Y3kz23jB for ; Tue, 7 Feb 2023 22:43:34 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id CBFFC60FBC; Tue, 7 Feb 2023 11:43:32 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org CBFFC60FBC 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 EQdoxCR4coWL; Tue, 7 Feb 2023 11:43:29 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTPS id 5DC2260E7F; Tue, 7 Feb 2023 11:43:27 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 5DC2260E7F Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 0EA5BC0033; Tue, 7 Feb 2023 11:43:27 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 0DC39C002B for ; Tue, 7 Feb 2023 11:43:26 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 1050E81E29 for ; Tue, 7 Feb 2023 11:43:21 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 1050E81E29 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 7mjt-3dBWiFE for ; Tue, 7 Feb 2023 11:43:18 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 330C481E08 Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by smtp1.osuosl.org (Postfix) with ESMTPS id 330C481E08 for ; Tue, 7 Feb 2023 11:43:17 +0000 (UTC) Received: (Authenticated sender: i.maximets@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id A524D1C0007; Tue, 7 Feb 2023 11:43:15 +0000 (UTC) From: Ilya Maximets To: ovs-dev@openvswitch.org Date: Tue, 7 Feb 2023 12:42:59 +0100 Message-Id: <20230207114302.1908836-6-i.maximets@ovn.org> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230207114302.1908836-1-i.maximets@ovn.org> References: <20230207114302.1908836-1-i.maximets@ovn.org> MIME-Version: 1.0 Cc: Dumitru Ceara , Ilya Maximets Subject: [ovs-dev] [PATCH ovn v2 5/8] northd: Use bitmaps for LB lists of switches and routers. 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" Lists of switches and routers to which a certain load balancer is applied are frequently iterated in order to add all these datapaths into datapath group bitmap. They would be easier to operate with, if they were bitmaps themselves in the first place. This change also lays out the infrastructure for more elegant creation of logical flows with datapath groups in the next commits. Signed-off-by: Ilya Maximets --- lib/lb.c | 26 +++++------ lib/lb.h | 9 ++-- northd/northd.c | 112 +++++++++++++++++++++++++++++------------------- 3 files changed, 86 insertions(+), 61 deletions(-) diff --git a/lib/lb.c b/lib/lb.c index 43628bba7..e0e97572f 100644 --- a/lib/lb.c +++ b/lib/lb.c @@ -19,10 +19,12 @@ #include "lib/ovn-nb-idl.h" #include "lib/ovn-sb-idl.h" #include "lib/ovn-util.h" +#include "northd/northd.h" #include "ovn/lex.h" /* OpenvSwitch lib includes. */ #include "openvswitch/vlog.h" +#include "lib/bitmap.h" #include "lib/smap.h" VLOG_DEFINE_THIS_MODULE(lb); @@ -495,7 +497,8 @@ ovn_lb_get_health_check(const struct nbrec_load_balancer *nbrec_lb, } struct ovn_northd_lb * -ovn_northd_lb_create(const struct nbrec_load_balancer *nbrec_lb) +ovn_northd_lb_create(const struct nbrec_load_balancer *nbrec_lb, + size_t n_datapaths) { bool template = smap_get_bool(&nbrec_lb->options, "template", false); bool is_udp = nullable_string_is_equal(nbrec_lb->protocol, "udp"); @@ -533,6 +536,9 @@ ovn_northd_lb_create(const struct nbrec_load_balancer *nbrec_lb) } lb->affinity_timeout = affinity_timeout; + lb->nb_ls_map = bitmap_allocate(n_datapaths); + lb->nb_lr_map = bitmap_allocate(n_datapaths); + sset_init(&lb->ips_v4); sset_init(&lb->ips_v6); struct smap_node *node; @@ -608,24 +614,18 @@ void ovn_northd_lb_add_lr(struct ovn_northd_lb *lb, size_t n, struct ovn_datapath **ods) { - while (lb->n_allocated_nb_lr <= lb->n_nb_lr + n) { - lb->nb_lr = x2nrealloc(lb->nb_lr, &lb->n_allocated_nb_lr, - sizeof *lb->nb_lr); + for (size_t i = 0; i < n; i++) { + bitmap_set1(lb->nb_lr_map, ods[i]->index); } - memcpy(&lb->nb_lr[lb->n_nb_lr], ods, n * sizeof *ods); - lb->n_nb_lr += n; } void ovn_northd_lb_add_ls(struct ovn_northd_lb *lb, size_t n, struct ovn_datapath **ods) { - while (lb->n_allocated_nb_ls <= lb->n_nb_ls + n) { - lb->nb_ls = x2nrealloc(lb->nb_ls, &lb->n_allocated_nb_ls, - sizeof *lb->nb_ls); + for (size_t i = 0; i < n; i++) { + bitmap_set1(lb->nb_ls_map, ods[i]->index); } - memcpy(&lb->nb_ls[lb->n_nb_ls], ods, n * sizeof *ods); - lb->n_nb_ls += n; } void @@ -640,8 +640,8 @@ ovn_northd_lb_destroy(struct ovn_northd_lb *lb) sset_destroy(&lb->ips_v4); sset_destroy(&lb->ips_v6); free(lb->selection_fields); - free(lb->nb_lr); - free(lb->nb_ls); + bitmap_free(lb->nb_lr_map); + bitmap_free(lb->nb_ls_map); free(lb); } diff --git a/lib/lb.h b/lib/lb.h index 55a41ae0b..7594553d5 100644 --- a/lib/lb.h +++ b/lib/lb.h @@ -75,12 +75,10 @@ struct ovn_northd_lb { struct sset ips_v6; size_t n_nb_ls; - size_t n_allocated_nb_ls; - struct ovn_datapath **nb_ls; + unsigned long *nb_ls_map; size_t n_nb_lr; - size_t n_allocated_nb_lr; - struct ovn_datapath **nb_lr; + unsigned long *nb_lr_map; }; struct ovn_lb_vip { @@ -127,7 +125,8 @@ struct ovn_northd_lb_backend { const struct sbrec_service_monitor *sbrec_monitor; }; -struct ovn_northd_lb *ovn_northd_lb_create(const struct nbrec_load_balancer *); +struct ovn_northd_lb *ovn_northd_lb_create(const struct nbrec_load_balancer *, + size_t n_datapaths); struct ovn_northd_lb *ovn_northd_lb_find(const struct hmap *, const struct uuid *); void ovn_northd_lb_destroy(struct ovn_northd_lb *); diff --git a/northd/northd.c b/northd/northd.c index c0a63e4dc..16a208d85 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -3979,7 +3979,8 @@ build_lbs(struct northd_input *input_data, struct hmap *datapaths, const struct nbrec_load_balancer *nbrec_lb; NBREC_LOAD_BALANCER_TABLE_FOR_EACH (nbrec_lb, input_data->nbrec_load_balancer_table) { - struct ovn_northd_lb *lb_nb = ovn_northd_lb_create(nbrec_lb); + struct ovn_northd_lb *lb_nb = ovn_northd_lb_create(nbrec_lb, + n_datapaths); hmap_insert(lbs, &lb_nb->hmap_node, uuid_hash(&nbrec_lb->header_.uuid)); } @@ -4237,9 +4238,11 @@ build_lswitch_lbs_from_lrouter(struct hmap *lbs, struct hmap *lb_groups) } struct ovn_northd_lb *lb; + size_t index; + HMAP_FOR_EACH (lb, hmap_node, lbs) { - for (size_t i = 0; i < lb->n_nb_lr; i++) { - struct ovn_datapath *od = lb->nb_lr[i]; + BITMAP_FOR_EACH_1 (index, n_datapaths, lb->nb_lr_map) { + struct ovn_datapath *od = datapaths_array[index]; ovn_northd_lb_add_ls(lb, od->n_ls_peers, od->ls_peers); } } @@ -4257,6 +4260,17 @@ build_lswitch_lbs_from_lrouter(struct hmap *lbs, struct hmap *lb_groups) } } +static void +build_lb_count_dps(struct hmap *lbs) +{ + struct ovn_northd_lb *lb; + + HMAP_FOR_EACH (lb, hmap_node, lbs) { + lb->n_nb_lr = bitmap_count1(lb->nb_lr_map, n_datapaths); + lb->n_nb_ls = bitmap_count1(lb->nb_ls_map, n_datapaths); + } +} + /* This must be called after all ports have been processed, i.e., after * build_ports() because the reachability check requires the router ports * networks to have been parsed. @@ -4419,25 +4433,18 @@ sync_lbs(struct northd_input *input_data, struct ovsdb_idl_txn *ovnsb_txn, } /* Find datapath group for this load balancer. */ - unsigned long *lb_dps_bitmap; struct ovn_dp_group *dpg; uint32_t hash; - lb_dps_bitmap = bitmap_allocate(n_datapaths); - for (size_t i = 0; i < lb->n_nb_ls; i++) { - bitmap_set1(lb_dps_bitmap, lb->nb_ls[i]->index); - } - - hash = hash_int(bitmap_count1(lb_dps_bitmap, n_datapaths), 0); - dpg = ovn_dp_group_find(&dp_groups, lb_dps_bitmap, hash); + hash = hash_int(lb->n_nb_ls, 0); + dpg = ovn_dp_group_find(&dp_groups, lb->nb_ls_map, hash); if (!dpg) { dpg = xzalloc(sizeof *dpg); dpg->dp_group = ovn_sb_insert_logical_dp_group(ovnsb_txn, - lb_dps_bitmap); - dpg->bitmap = bitmap_clone(lb_dps_bitmap, n_datapaths); + lb->nb_ls_map); + dpg->bitmap = bitmap_clone(lb->nb_ls_map, n_datapaths); hmap_insert(&dp_groups, &dpg->node, hash); } - bitmap_free(lb_dps_bitmap); /* Update columns. */ sbrec_load_balancer_set_name(lb->slb, lb->nlb->name); @@ -7081,9 +7088,10 @@ build_lb_rules_pre_stateful(struct hmap *lflows, struct ovn_northd_lb *lb, ovn_stage_get_pipeline(S_SWITCH_IN_PRE_STATEFUL), 120, ds_cstr(match), ds_cstr(action)); struct ovs_mutex *hash_lock = lflow_hash_lock(lflows, hash); + size_t index; - for (size_t j = 0; j < lb->n_nb_ls; j++) { - struct ovn_datapath *od = lb->nb_ls[j]; + BITMAP_FOR_EACH_1 (index, n_datapaths, lb->nb_ls_map) { + struct ovn_datapath *od = datapaths_array[index]; if (!ovn_dp_group_add_with_reference(lflow_ref, od)) { lflow_ref = ovn_lflow_add_at_with_hash( @@ -7390,12 +7398,14 @@ build_lb_affinity_ls_flows(struct hmap *lflows, struct ovn_northd_lb *lb, ovn_stage_get_pipeline(S_SWITCH_IN_LB_AFF_CHECK), 100, ds_cstr(&new_lb_match), aff_check); struct ovs_mutex *hash_lock = lflow_hash_lock(lflows, hash_aff_check); + size_t index; - for (size_t i = 0; i < lb->n_nb_ls; i++) { - if (!ovn_dp_group_add_with_reference(lflow_ref_aff_check, - lb->nb_ls[i])) { + BITMAP_FOR_EACH_1 (index, n_datapaths, lb->nb_ls_map) { + struct ovn_datapath *od = datapaths_array[index]; + + if (!ovn_dp_group_add_with_reference(lflow_ref_aff_check, od)) { lflow_ref_aff_check = ovn_lflow_add_at_with_hash( - lflows, lb->nb_ls[i], S_SWITCH_IN_LB_AFF_CHECK, 100, + lflows, od, S_SWITCH_IN_LB_AFF_CHECK, 100, ds_cstr(&new_lb_match), aff_check, NULL, NULL, &lb->nlb->header_, OVS_SOURCE_LOCATOR, hash_aff_check); } @@ -7494,12 +7504,13 @@ build_lb_affinity_ls_flows(struct hmap *lflows, struct ovn_northd_lb *lb, struct ovs_mutex *hash_lock_learn = lflow_hash_lock(lflows, hash_aff_learn); - for (size_t j = 0; j < lb->n_nb_ls; j++) { + BITMAP_FOR_EACH_1 (index, n_datapaths, lb->nb_ls_map) { + struct ovn_datapath *od = datapaths_array[index]; + /* Forward to OFTABLE_CHK_LB_AFFINITY table to store flow tuple. */ - if (!ovn_dp_group_add_with_reference(lflow_ref_aff_learn, - lb->nb_ls[j])) { + if (!ovn_dp_group_add_with_reference(lflow_ref_aff_learn, od)) { lflow_ref_aff_learn = ovn_lflow_add_at_with_hash( - lflows, lb->nb_ls[j], S_SWITCH_IN_LB_AFF_LEARN, 100, + lflows, od, S_SWITCH_IN_LB_AFF_LEARN, 100, ds_cstr(&aff_match_learn), ds_cstr(&aff_action_learn), NULL, NULL, &lb->nlb->header_, OVS_SOURCE_LOCATOR, hash_aff_learn); @@ -7514,12 +7525,13 @@ build_lb_affinity_ls_flows(struct hmap *lflows, struct ovn_northd_lb *lb, 150, ds_cstr(&aff_match), ds_cstr(&aff_action)); struct ovs_mutex *hash_lock_lb = lflow_hash_lock(lflows, hash_aff_lb); - for (size_t j = 0; j < lb->n_nb_ls; j++) { + BITMAP_FOR_EACH_1 (index, n_datapaths, lb->nb_ls_map) { + struct ovn_datapath *od = datapaths_array[index]; + /* Use already selected backend within affinity timeslot. */ - if (!ovn_dp_group_add_with_reference(lflow_ref_aff_lb, - lb->nb_ls[j])) { + if (!ovn_dp_group_add_with_reference(lflow_ref_aff_lb, od)) { lflow_ref_aff_lb = ovn_lflow_add_at_with_hash( - lflows, lb->nb_ls[j], S_SWITCH_IN_LB, 150, + lflows, od, S_SWITCH_IN_LB, 150, ds_cstr(&aff_match), ds_cstr(&aff_action), NULL, NULL, &lb->nlb->header_, OVS_SOURCE_LOCATOR, hash_aff_lb); @@ -7598,9 +7610,10 @@ build_lb_rules(struct hmap *lflows, struct ovn_northd_lb *lb, ovn_stage_get_pipeline(S_SWITCH_IN_LB), priority, ds_cstr(match), ds_cstr(action)); struct ovs_mutex *hash_lock = lflow_hash_lock(lflows, hash); + size_t index; - for (size_t j = 0; j < lb->n_nb_ls; j++) { - struct ovn_datapath *od = lb->nb_ls[j]; + BITMAP_FOR_EACH_1 (index, n_datapaths, lb->nb_ls_map) { + struct ovn_datapath *od = datapaths_array[index]; if (reject) { meter = copp_meter_get(COPP_REJECT, od->nbs->copp, @@ -10777,9 +10790,10 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip *lb_vip, /* Group gw router since we do not have datapath dependency in * lflow generation for them. */ - for (size_t i = 0; i < lb->n_nb_lr; i++) { + size_t index; + BITMAP_FOR_EACH_1 (index, n_datapaths, lb->nb_lr_map) { + struct ovn_datapath *od = datapaths_array[index]; enum lrouter_nat_lb_flow_type type; - struct ovn_datapath *od = lb->nb_lr[i]; if (lb->skip_snat) { type = LROUTER_NAT_LB_FLOW_SKIP_SNAT; @@ -10791,16 +10805,16 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip *lb_vip, } if (!od->n_l3dgw_ports) { - bitmap_set1(dp_bitmap[type], od->index); + bitmap_set1(dp_bitmap[type], index); } else { build_distr_lrouter_nat_flows_for_lb(&ctx, type, od); } if (!lport_addresses_is_empty(&od->lb_force_snat_addrs) || od->lb_force_snat_router_ip) { - bitmap_set1(dp_bitmap[LROUTER_NAT_LB_AFF_FORCE_SNAT], od->index); + bitmap_set1(dp_bitmap[LROUTER_NAT_LB_AFF_FORCE_SNAT], index); } else { - bitmap_set1(dp_bitmap[LROUTER_NAT_LB_AFF], od->index); + bitmap_set1(dp_bitmap[LROUTER_NAT_LB_AFF], index); } if (sset_contains(&od->external_ips, lb_vip->vip_str)) { @@ -10868,8 +10882,11 @@ build_lswitch_flows_for_lb(struct ovn_northd_lb *lb, struct hmap *lflows, if (!build_empty_lb_event_flow(lb_vip, lb, match, action)) { continue; } - for (size_t j = 0; j < lb->n_nb_ls; j++) { - struct ovn_datapath *od = lb->nb_ls[j]; + + size_t index; + BITMAP_FOR_EACH_1 (index, n_datapaths, lb->nb_ls_map) { + struct ovn_datapath *od = datapaths_array[index]; + ovn_lflow_add_with_hint__(lflows, od, S_SWITCH_IN_PRE_LB, 130, ds_cstr(match), ds_cstr(action), @@ -10947,9 +10964,11 @@ build_lrouter_defrag_flows_for_lb(struct ovn_northd_lb *lb, ovn_stage_get_pipeline(S_ROUTER_IN_DEFRAG), prio, ds_cstr(match), ds_cstr(&defrag_actions)); struct ovs_mutex *hash_lock = lflow_hash_lock(lflows, hash); + size_t index; + + BITMAP_FOR_EACH_1 (index, n_datapaths, lb->nb_lr_map) { + struct ovn_datapath *od = datapaths_array[index]; - 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)) { continue; } @@ -10970,6 +10989,8 @@ build_lrouter_flows_for_lb(struct ovn_northd_lb *lb, struct hmap *lflows, const struct chassis_features *features, struct ds *match, struct ds *action) { + size_t index; + if (!lb->n_nb_lr) { return; } @@ -10984,8 +11005,10 @@ build_lrouter_flows_for_lb(struct ovn_northd_lb *lb, struct hmap *lflows, if (!build_empty_lb_event_flow(lb_vip, lb, match, action)) { continue; } - for (size_t j = 0; j < lb->n_nb_lr; j++) { - struct ovn_datapath *od = lb->nb_lr[j]; + + BITMAP_FOR_EACH_1 (index, n_datapaths, lb->nb_lr_map) { + struct ovn_datapath *od = datapaths_array[index]; + ovn_lflow_add_with_hint__(lflows, od, S_ROUTER_IN_DNAT, 130, ds_cstr(match), ds_cstr(action), NULL, @@ -10997,8 +11020,10 @@ build_lrouter_flows_for_lb(struct ovn_northd_lb *lb, struct hmap *lflows, } if (lb->skip_snat) { - for (size_t i = 0; i < lb->n_nb_lr; i++) { - ovn_lflow_add(lflows, lb->nb_lr[i], S_ROUTER_OUT_SNAT, 120, + BITMAP_FOR_EACH_1 (index, n_datapaths, lb->nb_lr_map) { + struct ovn_datapath *od = datapaths_array[index]; + + ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT, 120, "flags.skip_snat_for_lb == 1 && ip", "next;"); } } @@ -16323,6 +16348,7 @@ ovnnb_db_run(struct northd_input *input_data, &data->datapaths, &data->ports); build_lb_port_related_data(&data->datapaths, &data->ports, &data->lbs, &data->lb_groups, input_data, ovnsb_txn); + build_lb_count_dps(&data->lbs); build_ipam(&data->datapaths, &data->ports); build_port_group_lswitches(input_data, &data->port_groups, &data->ports); build_lrouter_groups(&data->ports, &data->lr_list); From patchwork Tue Feb 7 11:43:00 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Maximets X-Patchwork-Id: 1738901 X-Patchwork-Delegate: mmichels@redhat.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.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 ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4PB1VR1gtyz23jB for ; Tue, 7 Feb 2023 22:43:43 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 35FFE40A2A; Tue, 7 Feb 2023 11:43:40 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 35FFE40A2A 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 eg3rk56GHyig; Tue, 7 Feb 2023 11:43:37 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp2.osuosl.org (Postfix) with ESMTPS id 162A340990; Tue, 7 Feb 2023 11:43:35 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 162A340990 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id D09ADC0033; Tue, 7 Feb 2023 11:43:34 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 4EF05C002B for ; Tue, 7 Feb 2023 11:43:34 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id D2619813BC for ; Tue, 7 Feb 2023 11:43:23 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org D2619813BC 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 6xuB1R1REgjd for ; Tue, 7 Feb 2023 11:43:21 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org D117081E0E Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [IPv6:2001:4b98:dc4:8::225]) by smtp1.osuosl.org (Postfix) with ESMTPS id D117081E0E for ; Tue, 7 Feb 2023 11:43:20 +0000 (UTC) Received: (Authenticated sender: i.maximets@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id 05BB91C0003; Tue, 7 Feb 2023 11:43:17 +0000 (UTC) From: Ilya Maximets To: ovs-dev@openvswitch.org Date: Tue, 7 Feb 2023 12:43:00 +0100 Message-Id: <20230207114302.1908836-7-i.maximets@ovn.org> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230207114302.1908836-1-i.maximets@ovn.org> References: <20230207114302.1908836-1-i.maximets@ovn.org> MIME-Version: 1.0 Cc: Dumitru Ceara , Ilya Maximets Subject: [ovs-dev] [PATCH ovn v2 6/8] northd: Create logical flows with datapath 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" The same pattern of logical flow creation is repeated multiple times in the code: 1. Calculate the hash. 2. Lock the hash. 3. Iterate over all datapaths in a group. 4. Create an lflow with ovn_lflow_add_at_with_hash() for the first time. 5. Add all other datapaths with ovn_dp_group_add_with_reference(). 6. Unlock the hash. Adding an extra 'dp_bitmap' parameter to the logical flow creation functions and a new ovn_lflow_add_with_dp_group() wrapper, so we can create a flow with all the datapaths we need at once and get rid of most of the code duplications. Signed-off-by: Ilya Maximets --- northd/northd.c | 292 +++++++++++++++--------------------------------- 1 file changed, 90 insertions(+), 202 deletions(-) diff --git a/northd/northd.c b/northd/northd.c index 16a208d85..941085e0f 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -5181,14 +5181,19 @@ static thread_local size_t thread_lflow_counter = 0; * hash lock is already taken. */ static bool ovn_dp_group_add_with_reference(struct ovn_lflow *lflow_ref, - struct ovn_datapath *od) + const struct ovn_datapath *od, + const unsigned long *dp_bitmap) OVS_REQUIRES(fake_hash_mutex) { if (!lflow_ref) { return false; } - - bitmap_set1(lflow_ref->dpg_bitmap, od->index); + if (od) { + bitmap_set1(lflow_ref->dpg_bitmap, od->index); + } + if (dp_bitmap) { + bitmap_or(lflow_ref->dpg_bitmap, dp_bitmap, n_datapaths); + } return true; } @@ -5196,7 +5201,8 @@ ovn_dp_group_add_with_reference(struct ovn_lflow *lflow_ref, * Version to use when hash bucket locking is NOT required. */ static struct ovn_lflow * -do_ovn_lflow_add(struct hmap *lflow_map, struct ovn_datapath *od, +do_ovn_lflow_add(struct hmap *lflow_map, const struct ovn_datapath *od, + const unsigned long *dp_bitmap, 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, @@ -5210,7 +5216,7 @@ do_ovn_lflow_add(struct hmap *lflow_map, struct ovn_datapath *od, old_lflow = ovn_lflow_find(lflow_map, NULL, stage, priority, match, actions, ctrl_meter, hash); if (old_lflow) { - ovn_dp_group_add_with_reference(old_lflow, od); + ovn_dp_group_add_with_reference(old_lflow, od, dp_bitmap); return old_lflow; } @@ -5223,7 +5229,9 @@ do_ovn_lflow_add(struct hmap *lflow_map, struct ovn_datapath *od, io_port ? xstrdup(io_port) : NULL, nullable_xstrdup(ctrl_meter), ovn_lflow_hint(stage_hint), where); - bitmap_set1(lflow->dpg_bitmap, od->index); + + ovn_dp_group_add_with_reference(lflow, od, dp_bitmap); + if (parallelization_state != STATE_USE_PARALLELIZATION) { hmap_insert(lflow_map, &lflow->hmap_node, hash); } else { @@ -5237,7 +5245,9 @@ do_ovn_lflow_add(struct hmap *lflow_map, struct ovn_datapath *od, * Version to use when hash is pre-computed and a hash bucket is already * locked if necessary. */ 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 hmap *lflow_map, + const struct ovn_datapath *od, + const unsigned long *dp_bitmap, enum ovn_stage stage, uint16_t priority, const char *match, const char *actions, const char *io_port, const char *ctrl_meter, @@ -5247,16 +5257,19 @@ ovn_lflow_add_at_with_hash(struct hmap *lflow_map, struct ovn_datapath *od, { struct ovn_lflow *lflow; - ovs_assert(ovn_stage_to_datapath_type(stage) == ovn_datapath_get_type(od)); + ovs_assert(!od || + ovn_stage_to_datapath_type(stage) == ovn_datapath_get_type(od)); - lflow = do_ovn_lflow_add(lflow_map, od, hash, stage, priority, match, - actions, io_port, stage_hint, where, ctrl_meter); + lflow = do_ovn_lflow_add(lflow_map, od, dp_bitmap, 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. */ static void -ovn_lflow_add_at(struct hmap *lflow_map, struct ovn_datapath *od, +ovn_lflow_add_at(struct hmap *lflow_map, const struct ovn_datapath *od, + const unsigned long *dp_bitmap, enum ovn_stage stage, uint16_t priority, const char *match, const char *actions, const char *io_port, const char *ctrl_meter, @@ -5272,8 +5285,9 @@ ovn_lflow_add_at(struct hmap *lflow_map, struct ovn_datapath *od, actions); hash_lock = lflow_hash_lock(lflow_map, hash); - ovn_lflow_add_at_with_hash(lflow_map, od, stage, priority, match, actions, - io_port, ctrl_meter, stage_hint, where, hash); + ovn_lflow_add_at_with_hash(lflow_map, od, dp_bitmap, stage, priority, + match, actions, io_port, ctrl_meter, + stage_hint, where, hash); lflow_hash_unlock(hash_lock); } @@ -5283,7 +5297,8 @@ __ovn_lflow_add_default_drop(struct hmap *lflow_map, enum ovn_stage stage, const char *where) { - ovn_lflow_add_at(lflow_map, od, stage, 0, "1", debug_drop_action(), + ovn_lflow_add_at(lflow_map, od, NULL, stage, 0, "1", + debug_drop_action(), NULL, NULL, NULL, where ); } @@ -5291,14 +5306,19 @@ __ovn_lflow_add_default_drop(struct hmap *lflow_map, #define ovn_lflow_add_with_hint__(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, \ ACTIONS, IN_OUT_PORT, CTRL_METER, \ STAGE_HINT) \ - ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, \ + ovn_lflow_add_at(LFLOW_MAP, OD, NULL, STAGE, PRIORITY, MATCH, ACTIONS, \ IN_OUT_PORT, CTRL_METER, STAGE_HINT, OVS_SOURCE_LOCATOR) #define ovn_lflow_add_with_hint(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, \ ACTIONS, STAGE_HINT) \ - ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, \ + ovn_lflow_add_at(LFLOW_MAP, OD, NULL, STAGE, PRIORITY, MATCH, ACTIONS, \ NULL, NULL, STAGE_HINT, OVS_SOURCE_LOCATOR) +#define ovn_lflow_add_with_dp_group(LFLOW_MAP, DP_BITMAP, STAGE, PRIORITY, \ + MATCH, ACTIONS, STAGE_HINT) \ + ovn_lflow_add_at(LFLOW_MAP, NULL, DP_BITMAP, STAGE, PRIORITY, MATCH, \ + ACTIONS, NULL, NULL, STAGE_HINT, OVS_SOURCE_LOCATOR) + #define ovn_lflow_add_default_drop(LFLOW_MAP, OD, STAGE) \ __ovn_lflow_add_default_drop(LFLOW_MAP, OD, STAGE, OVS_SOURCE_LOCATOR) @@ -5316,11 +5336,11 @@ __ovn_lflow_add_default_drop(struct hmap *lflow_map, #define ovn_lflow_add_with_lport_and_hint(LFLOW_MAP, OD, STAGE, PRIORITY, \ MATCH, ACTIONS, IN_OUT_PORT, \ STAGE_HINT) \ - ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, \ + ovn_lflow_add_at(LFLOW_MAP, OD, NULL, STAGE, PRIORITY, MATCH, ACTIONS, \ IN_OUT_PORT, NULL, STAGE_HINT, OVS_SOURCE_LOCATOR) #define ovn_lflow_add(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS) \ - ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, \ + ovn_lflow_add_at(LFLOW_MAP, OD, NULL, STAGE, PRIORITY, MATCH, ACTIONS, \ NULL, NULL, NULL, OVS_SOURCE_LOCATOR) #define ovn_lflow_metered(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, \ @@ -7038,6 +7058,10 @@ build_lb_rules_pre_stateful(struct hmap *lflows, struct ovn_northd_lb *lb, bool ct_lb_mark, struct ds *match, struct ds *action) { + if (!lb->n_nb_ls) { + return; + } + for (size_t i = 0; i < lb->n_vips; i++) { struct ovn_lb_vip *lb_vip = &lb->vips[i]; ds_clear(action); @@ -7082,26 +7106,9 @@ build_lb_rules_pre_stateful(struct hmap *lflows, struct ovn_northd_lb *lb, ds_put_format(match, " && %s.dst == %s", proto, lb_vip->port_str); } - struct ovn_lflow *lflow_ref = NULL; - uint32_t hash = ovn_logical_flow_hash( - ovn_stage_get_table(S_SWITCH_IN_PRE_STATEFUL), - ovn_stage_get_pipeline(S_SWITCH_IN_PRE_STATEFUL), 120, - ds_cstr(match), ds_cstr(action)); - struct ovs_mutex *hash_lock = lflow_hash_lock(lflows, hash); - size_t index; - - BITMAP_FOR_EACH_1 (index, n_datapaths, lb->nb_ls_map) { - struct ovn_datapath *od = datapaths_array[index]; - - if (!ovn_dp_group_add_with_reference(lflow_ref, od)) { - lflow_ref = ovn_lflow_add_at_with_hash( - lflows, od, S_SWITCH_IN_PRE_STATEFUL, 120, - ds_cstr(match), ds_cstr(action), - NULL, NULL, &lb->nlb->header_, - OVS_SOURCE_LOCATOR, hash); - } - } - lflow_hash_unlock(hash_lock); + ovn_lflow_add_with_dp_group( + lflows, lb->nb_ls_map, S_SWITCH_IN_PRE_STATEFUL, 120, + ds_cstr(match), ds_cstr(action), &lb->nlb->header_); } } @@ -7145,34 +7152,18 @@ build_lb_rules_pre_stateful(struct hmap *lflows, struct ovn_northd_lb *lb, static void build_lb_affinity_lr_flows(struct hmap *lflows, struct ovn_northd_lb *lb, struct ovn_lb_vip *lb_vip, char *new_lb_match, - char *lb_action, unsigned long *dp_bitmap) + char *lb_action, const unsigned long *dp_bitmap) { - if (!lb->affinity_timeout) { + if (!lb->affinity_timeout || + bitmap_is_all_zeros(dp_bitmap, n_datapaths)) { return; } static char *aff_check = REGBIT_KNOWN_LB_SESSION" = chk_lb_aff(); next;"; - struct ovn_lflow *lflow_ref_aff_check = NULL; - /* Check if we have already a enstablished connection for this - * tuple and we are in affinity timeslot. */ - uint32_t hash_aff_check = ovn_logical_flow_hash( - ovn_stage_get_table(S_ROUTER_IN_LB_AFF_CHECK), - ovn_stage_get_pipeline(S_ROUTER_IN_LB_AFF_CHECK), 100, - new_lb_match, aff_check); - struct ovs_mutex *hash_lock = lflow_hash_lock(lflows, hash_aff_check); - size_t index; - BITMAP_FOR_EACH_1 (index, n_datapaths, dp_bitmap) { - struct ovn_datapath *od = datapaths_array[index]; - - if (!ovn_dp_group_add_with_reference(lflow_ref_aff_check, od)) { - lflow_ref_aff_check = ovn_lflow_add_at_with_hash( - lflows, od, S_ROUTER_IN_LB_AFF_CHECK, 100, - new_lb_match, aff_check, NULL, NULL, &lb->nlb->header_, - OVS_SOURCE_LOCATOR, hash_aff_check); - } - } - lflow_hash_unlock(hash_lock); + ovn_lflow_add_with_dp_group( + lflows, dp_bitmap, S_ROUTER_IN_LB_AFF_CHECK, 100, + new_lb_match, aff_check, &lb->nlb->header_); struct ds aff_action = DS_EMPTY_INITIALIZER; struct ds aff_action_learn = DS_EMPTY_INITIALIZER; @@ -7269,48 +7260,16 @@ build_lb_affinity_lr_flows(struct hmap *lflows, struct ovn_northd_lb *lb, ds_put_format(&aff_action_learn, ", timeout = %d); /* drop */", lb->affinity_timeout); - struct ovn_lflow *lflow_ref_aff_learn = NULL; - uint32_t hash_aff_learn = ovn_logical_flow_hash( - ovn_stage_get_table(S_ROUTER_IN_LB_AFF_LEARN), - ovn_stage_get_pipeline(S_ROUTER_IN_LB_AFF_LEARN), - 100, ds_cstr(&aff_match_learn), ds_cstr(&aff_action_learn)); - struct ovs_mutex *hash_lock_learn = lflow_hash_lock(lflows, - hash_aff_learn); - - BITMAP_FOR_EACH_1 (index, n_datapaths, dp_bitmap) { - struct ovn_datapath *od = datapaths_array[index]; - - /* Forward to OFTABLE_CHK_LB_AFFINITY table to store flow tuple. */ - if (!ovn_dp_group_add_with_reference(lflow_ref_aff_learn, od)) { - lflow_ref_aff_learn = ovn_lflow_add_at_with_hash( - lflows, od, S_ROUTER_IN_LB_AFF_LEARN, 100, - ds_cstr(&aff_match_learn), ds_cstr(&aff_action_learn), - NULL, NULL, &lb->nlb->header_, OVS_SOURCE_LOCATOR, - hash_aff_learn); - } - } - lflow_hash_unlock(hash_lock_learn); - - struct ovn_lflow *lflow_ref_aff_lb = NULL; - uint32_t hash_aff_lb = ovn_logical_flow_hash( - ovn_stage_get_table(S_ROUTER_IN_DNAT), - ovn_stage_get_pipeline(S_ROUTER_IN_DNAT), - 150, ds_cstr(&aff_match), ds_cstr(&aff_action)); - struct ovs_mutex *hash_lock_lb = lflow_hash_lock(lflows, hash_aff_lb); + /* Forward to OFTABLE_CHK_LB_AFFINITY table to store flow tuple. */ + ovn_lflow_add_with_dp_group( + lflows, dp_bitmap, S_ROUTER_IN_LB_AFF_LEARN, 100, + ds_cstr(&aff_match_learn), ds_cstr(&aff_action_learn), + &lb->nlb->header_); - BITMAP_FOR_EACH_1 (index, n_datapaths, dp_bitmap) { - struct ovn_datapath *od = datapaths_array[index]; - - /* Use already selected backend within affinity timeslot. */ - if (!ovn_dp_group_add_with_reference(lflow_ref_aff_lb, od)) { - lflow_ref_aff_lb = ovn_lflow_add_at_with_hash( - lflows, od, S_ROUTER_IN_DNAT, 150, - ds_cstr(&aff_match), ds_cstr(&aff_action), NULL, NULL, - &lb->nlb->header_, OVS_SOURCE_LOCATOR, - hash_aff_lb); - } - } - lflow_hash_unlock(hash_lock_lb); + /* Use already selected backend within affinity timeslot. */ + ovn_lflow_add_with_dp_group( + lflows, dp_bitmap, S_ROUTER_IN_DNAT, 150, + ds_cstr(&aff_match), ds_cstr(&aff_action), &lb->nlb->header_); ds_truncate(&aff_action, aff_action_len); ds_truncate(&aff_action_learn, aff_action_learn_len); @@ -7369,7 +7328,7 @@ static void build_lb_affinity_ls_flows(struct hmap *lflows, struct ovn_northd_lb *lb, struct ovn_lb_vip *lb_vip) { - if (!lb->affinity_timeout) { + if (!lb->affinity_timeout || !lb->n_nb_ls) { return; } @@ -7390,27 +7349,10 @@ build_lb_affinity_ls_flows(struct hmap *lflows, struct ovn_northd_lb *lb, } static char *aff_check = REGBIT_KNOWN_LB_SESSION" = chk_lb_aff(); next;"; - struct ovn_lflow *lflow_ref_aff_check = NULL; - /* Check if we have already a enstablished connection for this - * tuple and we are in affinity timeslot. */ - uint32_t hash_aff_check = ovn_logical_flow_hash( - ovn_stage_get_table(S_SWITCH_IN_LB_AFF_CHECK), - ovn_stage_get_pipeline(S_SWITCH_IN_LB_AFF_CHECK), 100, - ds_cstr(&new_lb_match), aff_check); - struct ovs_mutex *hash_lock = lflow_hash_lock(lflows, hash_aff_check); - size_t index; - - BITMAP_FOR_EACH_1 (index, n_datapaths, lb->nb_ls_map) { - struct ovn_datapath *od = datapaths_array[index]; - if (!ovn_dp_group_add_with_reference(lflow_ref_aff_check, od)) { - lflow_ref_aff_check = ovn_lflow_add_at_with_hash( - lflows, od, S_SWITCH_IN_LB_AFF_CHECK, 100, - ds_cstr(&new_lb_match), aff_check, NULL, NULL, - &lb->nlb->header_, OVS_SOURCE_LOCATOR, hash_aff_check); - } - } - lflow_hash_unlock(hash_lock); + ovn_lflow_add_with_dp_group( + lflows, lb->nb_ls_map, S_SWITCH_IN_LB_AFF_CHECK, 100, + ds_cstr(&new_lb_match), aff_check, &lb->nlb->header_); ds_destroy(&new_lb_match); struct ds aff_action = DS_EMPTY_INITIALIZER; @@ -7496,48 +7438,16 @@ build_lb_affinity_ls_flows(struct hmap *lflows, struct ovn_northd_lb *lb, ds_put_format(&aff_action_learn, ", timeout = %d); /* drop */", lb->affinity_timeout); - struct ovn_lflow *lflow_ref_aff_learn = NULL; - uint32_t hash_aff_learn = ovn_logical_flow_hash( - ovn_stage_get_table(S_SWITCH_IN_LB_AFF_LEARN), - ovn_stage_get_pipeline(S_SWITCH_IN_LB_AFF_LEARN), - 100, ds_cstr(&aff_match_learn), ds_cstr(&aff_action_learn)); - struct ovs_mutex *hash_lock_learn = lflow_hash_lock(lflows, - hash_aff_learn); + /* Forward to OFTABLE_CHK_LB_AFFINITY table to store flow tuple. */ + ovn_lflow_add_with_dp_group( + lflows, lb->nb_ls_map, S_SWITCH_IN_LB_AFF_LEARN, 100, + ds_cstr(&aff_match_learn), ds_cstr(&aff_action_learn), + &lb->nlb->header_); - BITMAP_FOR_EACH_1 (index, n_datapaths, lb->nb_ls_map) { - struct ovn_datapath *od = datapaths_array[index]; - - /* Forward to OFTABLE_CHK_LB_AFFINITY table to store flow tuple. */ - if (!ovn_dp_group_add_with_reference(lflow_ref_aff_learn, od)) { - lflow_ref_aff_learn = ovn_lflow_add_at_with_hash( - lflows, od, S_SWITCH_IN_LB_AFF_LEARN, 100, - ds_cstr(&aff_match_learn), ds_cstr(&aff_action_learn), - NULL, NULL, &lb->nlb->header_, OVS_SOURCE_LOCATOR, - hash_aff_learn); - } - } - lflow_hash_unlock(hash_lock_learn); - - struct ovn_lflow *lflow_ref_aff_lb = NULL; - uint32_t hash_aff_lb = ovn_logical_flow_hash( - ovn_stage_get_table(S_SWITCH_IN_LB), - ovn_stage_get_pipeline(S_SWITCH_IN_LB), - 150, ds_cstr(&aff_match), ds_cstr(&aff_action)); - struct ovs_mutex *hash_lock_lb = lflow_hash_lock(lflows, hash_aff_lb); - - BITMAP_FOR_EACH_1 (index, n_datapaths, lb->nb_ls_map) { - struct ovn_datapath *od = datapaths_array[index]; - - /* Use already selected backend within affinity timeslot. */ - if (!ovn_dp_group_add_with_reference(lflow_ref_aff_lb, od)) { - lflow_ref_aff_lb = ovn_lflow_add_at_with_hash( - lflows, od, S_SWITCH_IN_LB, 150, - ds_cstr(&aff_match), ds_cstr(&aff_action), NULL, NULL, - &lb->nlb->header_, OVS_SOURCE_LOCATOR, - hash_aff_lb); - } - } - lflow_hash_unlock(hash_lock_lb); + /* Use already selected backend within affinity timeslot. */ + ovn_lflow_add_with_dp_group( + lflows, lb->nb_ls_map, S_SWITCH_IN_LB, 150, + ds_cstr(&aff_match), ds_cstr(&aff_action), &lb->nlb->header_); ds_truncate(&aff_action, aff_action_len); ds_truncate(&aff_action_learn, aff_action_learn_len); @@ -7619,9 +7529,10 @@ build_lb_rules(struct hmap *lflows, struct ovn_northd_lb *lb, meter = copp_meter_get(COPP_REJECT, od->nbs->copp, meter_groups); } - if (meter || !ovn_dp_group_add_with_reference(lflow_ref, od)) { + if (meter || !ovn_dp_group_add_with_reference(lflow_ref, + od, NULL)) { struct ovn_lflow *lflow = ovn_lflow_add_at_with_hash( - lflows, od, S_SWITCH_IN_LB, priority, + lflows, od, NULL, S_SWITCH_IN_LB, priority, ds_cstr(match), ds_cstr(action), NULL, meter, &lb->nlb->header_, OVS_SOURCE_LOCATOR, hash); @@ -10625,14 +10536,18 @@ build_distr_lrouter_nat_flows_for_lb(struct lrouter_nat_lb_flows_ctx *ctx, static void build_gw_lrouter_nat_flows_for_lb(struct lrouter_nat_lb_flows_ctx *ctx, enum lrouter_nat_lb_flow_type type, - unsigned long *dp_bitmap) + const unsigned long *dp_bitmap) { - struct ovn_lflow *new_lflow_ref = NULL, *est_lflow_ref = NULL; + struct ovn_lflow *new_lflow_ref = NULL; struct lrouter_nat_lb_flow *new_flow = &ctx->new[type]; struct lrouter_nat_lb_flow *est_flow = &ctx->est[type]; struct ovs_mutex *hash_lock; size_t index; + if (bitmap_is_all_zeros(dp_bitmap, n_datapaths)) { + return; + } + hash_lock = lflow_hash_lock(ctx->lflows, new_flow->hash); BITMAP_FOR_EACH_1 (index, n_datapaths, dp_bitmap) { struct ovn_datapath *od = datapaths_array[index]; @@ -10645,8 +10560,8 @@ build_gw_lrouter_nat_flows_for_lb(struct lrouter_nat_lb_flows_ctx *ctx, } if (meter || - !ovn_dp_group_add_with_reference(new_lflow_ref, od)) { - lflow = ovn_lflow_add_at_with_hash(ctx->lflows, od, + !ovn_dp_group_add_with_reference(new_lflow_ref, od, NULL)) { + lflow = ovn_lflow_add_at_with_hash(ctx->lflows, od, NULL, S_ROUTER_IN_DNAT, ctx->prio, ds_cstr(ctx->new_match), new_flow->action, NULL, meter, &ctx->lb->nlb->header_, OVS_SOURCE_LOCATOR, new_flow->hash); @@ -10655,18 +10570,9 @@ build_gw_lrouter_nat_flows_for_lb(struct lrouter_nat_lb_flows_ctx *ctx, } lflow_hash_unlock(hash_lock); - hash_lock = lflow_hash_lock(ctx->lflows, est_flow->hash); - BITMAP_FOR_EACH_1 (index, n_datapaths, dp_bitmap) { - struct ovn_datapath *od = datapaths_array[index]; - - if (!ovn_dp_group_add_with_reference(est_lflow_ref, od)) { - est_lflow_ref = ovn_lflow_add_at_with_hash(ctx->lflows, od, - S_ROUTER_IN_DNAT, ctx->prio, ds_cstr(ctx->est_match), - est_flow->action, NULL, NULL, &ctx->lb->nlb->header_, - OVS_SOURCE_LOCATOR, est_flow->hash); - } - } - lflow_hash_unlock(hash_lock); + ovn_lflow_add_with_dp_group( + ctx->lflows, dp_bitmap, S_ROUTER_IN_DNAT, ctx->prio, + ds_cstr(ctx->est_match), est_flow->action, &ctx->lb->nlb->header_); } static void @@ -10958,27 +10864,9 @@ build_lrouter_defrag_flows_for_lb(struct ovn_northd_lb *lb, ds_put_format(&defrag_actions, "ct_dnat;"); - struct ovn_lflow *lflow_ref = NULL; - uint32_t hash = ovn_logical_flow_hash( - ovn_stage_get_table(S_ROUTER_IN_DEFRAG), - ovn_stage_get_pipeline(S_ROUTER_IN_DEFRAG), prio, - ds_cstr(match), ds_cstr(&defrag_actions)); - struct ovs_mutex *hash_lock = lflow_hash_lock(lflows, hash); - size_t index; - - BITMAP_FOR_EACH_1 (index, n_datapaths, lb->nb_lr_map) { - struct ovn_datapath *od = datapaths_array[index]; - - 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, - ds_cstr(match), ds_cstr(&defrag_actions), - NULL, NULL, &lb->nlb->header_, - OVS_SOURCE_LOCATOR, hash); - } - lflow_hash_unlock(hash_lock); + ovn_lflow_add_with_dp_group( + lflows, lb->nb_lr_map, S_ROUTER_IN_DEFRAG, prio, + ds_cstr(match), ds_cstr(&defrag_actions), &lb->nlb->header_); } ds_destroy(&defrag_actions); } From patchwork Tue Feb 7 11:43:01 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Maximets X-Patchwork-Id: 1738902 X-Patchwork-Delegate: mmichels@redhat.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.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 ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4PB1VW32CHz23jB for ; Tue, 7 Feb 2023 22:43:47 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 343C581E77; Tue, 7 Feb 2023 11:43:45 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 343C581E77 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 2XZgjLLkAgeP; Tue, 7 Feb 2023 11:43:43 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp1.osuosl.org (Postfix) with ESMTPS id 1633181E8B; Tue, 7 Feb 2023 11:43:41 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 1633181E8B Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id D0EBCC0033; Tue, 7 Feb 2023 11:43:40 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 078CFC0077 for ; Tue, 7 Feb 2023 11:43:40 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 0FFFD81406 for ; Tue, 7 Feb 2023 11:43:25 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 0FFFD81406 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 bHWUoBmyYZTP for ; Tue, 7 Feb 2023 11:43:23 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org B4C1681C56 Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by smtp1.osuosl.org (Postfix) with ESMTPS id B4C1681C56 for ; Tue, 7 Feb 2023 11:43:22 +0000 (UTC) Received: (Authenticated sender: i.maximets@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id 507B21C0010; Tue, 7 Feb 2023 11:43:20 +0000 (UTC) From: Ilya Maximets To: ovs-dev@openvswitch.org Date: Tue, 7 Feb 2023 12:43:01 +0100 Message-Id: <20230207114302.1908836-8-i.maximets@ovn.org> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230207114302.1908836-1-i.maximets@ovn.org> References: <20230207114302.1908836-1-i.maximets@ovn.org> MIME-Version: 1.0 Cc: Dumitru Ceara , Ilya Maximets Subject: [ovs-dev] [PATCH ovn v2 7/8] northd: Create metered flows with dp groups if CoPP is not configured. 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" Previous change introduced the ovn_lflow_add_with_dp_group() wrapper and applied it to all the non-metered logical flows. Metered ones are a bit more complex, but still can be optimized to use the new wrapper in cases where CoPP is not enabled or enabled only on some datapaths. Iterating over all the datapaths and excluding metered ones from the bitmap. For the remaining datapaths the new wrapper can be used. Metered flows can be created right away. This might be slightly slower for a case where CoPP is enabled due to more hash calculations, but that can be optimized later, if necessary. With this change, there are no more users of ovn_lflow_add_at_with_hash(), so it is inlined into ovn_lflow_add_at(). 'struct lrouter_nat_lb_flow' degraded to just an action string, so inlined into ctx structure. Initializer is no longer needed. Signed-off-by: Ilya Maximets --- northd/northd.c | 182 +++++++++++++++++++----------------------------- 1 file changed, 70 insertions(+), 112 deletions(-) diff --git a/northd/northd.c b/northd/northd.c index 941085e0f..d2acfc502 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -5179,28 +5179,24 @@ static thread_local size_t thread_lflow_counter = 0; /* Adds an OVN datapath to a datapath group of existing logical flow. * Version to use when hash bucket locking is NOT required or the corresponding * hash lock is already taken. */ -static bool +static void ovn_dp_group_add_with_reference(struct ovn_lflow *lflow_ref, const struct ovn_datapath *od, const unsigned long *dp_bitmap) OVS_REQUIRES(fake_hash_mutex) { - if (!lflow_ref) { - return false; - } if (od) { bitmap_set1(lflow_ref->dpg_bitmap, od->index); } if (dp_bitmap) { bitmap_or(lflow_ref->dpg_bitmap, dp_bitmap, n_datapaths); } - return true; } /* Adds a row with the specified contents to the Logical_Flow table. * Version to use when hash bucket locking is NOT required. */ -static struct ovn_lflow * +static void do_ovn_lflow_add(struct hmap *lflow_map, const struct ovn_datapath *od, const unsigned long *dp_bitmap, uint32_t hash, enum ovn_stage stage, uint16_t priority, @@ -5217,7 +5213,7 @@ do_ovn_lflow_add(struct hmap *lflow_map, const struct ovn_datapath *od, actions, ctrl_meter, hash); if (old_lflow) { ovn_dp_group_add_with_reference(old_lflow, od, dp_bitmap); - return old_lflow; + return; } lflow = xmalloc(sizeof *lflow); @@ -5238,32 +5234,6 @@ do_ovn_lflow_add(struct hmap *lflow_map, const struct ovn_datapath *od, hmap_insert_fast(lflow_map, &lflow->hmap_node, hash); thread_lflow_counter++; } - return lflow; -} - -/* Adds a row with the specified contents to the Logical_Flow table. - * Version to use when hash is pre-computed and a hash bucket is already - * locked if necessary. */ -static struct ovn_lflow * -ovn_lflow_add_at_with_hash(struct hmap *lflow_map, - const struct ovn_datapath *od, - const unsigned long *dp_bitmap, - enum ovn_stage stage, uint16_t priority, - const char *match, const char *actions, - const char *io_port, const char *ctrl_meter, - const struct ovsdb_idl_row *stage_hint, - const char *where, uint32_t hash) - OVS_REQUIRES(fake_hash_mutex) -{ - struct ovn_lflow *lflow; - - ovs_assert(!od || - ovn_stage_to_datapath_type(stage) == ovn_datapath_get_type(od)); - - lflow = do_ovn_lflow_add(lflow_map, od, dp_bitmap, 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. */ @@ -5279,15 +5249,17 @@ ovn_lflow_add_at(struct hmap *lflow_map, const struct ovn_datapath *od, struct ovs_mutex *hash_lock; uint32_t hash; + ovs_assert(!od || + ovn_stage_to_datapath_type(stage) == ovn_datapath_get_type(od)); + hash = ovn_logical_flow_hash(ovn_stage_get_table(stage), ovn_stage_get_pipeline(stage), priority, match, actions); hash_lock = lflow_hash_lock(lflow_map, hash); - ovn_lflow_add_at_with_hash(lflow_map, od, dp_bitmap, stage, priority, - match, actions, io_port, ctrl_meter, - stage_hint, where, hash); + do_ovn_lflow_add(lflow_map, od, dp_bitmap, hash, stage, priority, + match, actions, io_port, stage_hint, where, ctrl_meter); lflow_hash_unlock(hash_lock); } @@ -7514,32 +7486,35 @@ build_lb_rules(struct hmap *lflows, struct ovn_northd_lb *lb, build_lb_affinity_ls_flows(lflows, lb, lb_vip); - struct ovn_lflow *lflow_ref = NULL; - uint32_t hash = ovn_logical_flow_hash( - ovn_stage_get_table(S_SWITCH_IN_LB), - ovn_stage_get_pipeline(S_SWITCH_IN_LB), priority, - ds_cstr(match), ds_cstr(action)); - struct ovs_mutex *hash_lock = lflow_hash_lock(lflows, hash); - size_t index; + unsigned long *dp_non_meter = NULL; + bool build_non_meter = false; + if (reject) { + size_t index; - BITMAP_FOR_EACH_1 (index, n_datapaths, lb->nb_ls_map) { - struct ovn_datapath *od = datapaths_array[index]; + dp_non_meter = bitmap_clone(lb->nb_ls_map, n_datapaths); + BITMAP_FOR_EACH_1 (index, n_datapaths, lb->nb_ls_map) { + struct ovn_datapath *od = datapaths_array[index]; - if (reject) { meter = copp_meter_get(COPP_REJECT, od->nbs->copp, meter_groups); - } - if (meter || !ovn_dp_group_add_with_reference(lflow_ref, - od, NULL)) { - struct ovn_lflow *lflow = ovn_lflow_add_at_with_hash( - lflows, od, NULL, S_SWITCH_IN_LB, priority, + if (!meter) { + build_non_meter = true; + continue; + } + bitmap_set0(dp_non_meter, index); + ovn_lflow_add_with_hint__( + lflows, od, S_SWITCH_IN_LB, priority, ds_cstr(match), ds_cstr(action), - NULL, meter, &lb->nlb->header_, - OVS_SOURCE_LOCATOR, hash); - lflow_ref = meter ? NULL : lflow; + NULL, meter, &lb->nlb->header_); } } - lflow_hash_unlock(hash_lock); + if (!dp_non_meter || build_non_meter) { + ovn_lflow_add_with_dp_group( + lflows, dp_non_meter ? dp_non_meter : lb->nb_ls_map, + S_SWITCH_IN_LB, priority, + ds_cstr(match), ds_cstr(action), &lb->nlb->header_); + } + bitmap_free(dp_non_meter); } } @@ -10442,15 +10417,6 @@ get_force_snat_ip(struct ovn_datapath *od, const char *key_type, return true; } -#define LROUTER_NAT_LB_FLOW_INIT(MATCH, ACTION, PRIO) \ - (struct lrouter_nat_lb_flow) { \ - .action = (ACTION), \ - .hash = ovn_logical_flow_hash( \ - ovn_stage_get_table(S_ROUTER_IN_DNAT), \ - ovn_stage_get_pipeline(S_ROUTER_IN_DNAT), \ - (PRIO), ds_cstr(MATCH), (ACTION)), \ - } - enum lrouter_nat_lb_flow_type { LROUTER_NAT_LB_FLOW_NORMAL = 0, LROUTER_NAT_LB_FLOW_SKIP_SNAT, @@ -10458,14 +10424,9 @@ enum lrouter_nat_lb_flow_type { LROUTER_NAT_LB_FLOW_MAX, }; -struct lrouter_nat_lb_flow { - char *action; - uint32_t hash; -}; - struct lrouter_nat_lb_flows_ctx { - struct lrouter_nat_lb_flow new[LROUTER_NAT_LB_FLOW_MAX]; - struct lrouter_nat_lb_flow est[LROUTER_NAT_LB_FLOW_MAX]; + const char *new_action[LROUTER_NAT_LB_FLOW_MAX]; + const char *est_action[LROUTER_NAT_LB_FLOW_MAX]; struct ds *new_match; struct ds *est_match; @@ -10507,10 +10468,10 @@ build_distr_lrouter_nat_flows_for_lb(struct lrouter_nat_lb_flows_ctx *ctx, } ovn_lflow_add_with_hint__(ctx->lflows, od, S_ROUTER_IN_DNAT, ctx->prio, - ds_cstr(ctx->new_match), ctx->new[type].action, + ds_cstr(ctx->new_match), ctx->new_action[type], NULL, meter, &ctx->lb->nlb->header_); ovn_lflow_add_with_hint(ctx->lflows, od, S_ROUTER_IN_DNAT, ctx->prio, - ds_cstr(ctx->est_match), ctx->est[type].action, + ds_cstr(ctx->est_match), ctx->est_action[type], &ctx->lb->nlb->header_); ds_truncate(ctx->new_match, new_match_len); @@ -10520,8 +10481,8 @@ build_distr_lrouter_nat_flows_for_lb(struct lrouter_nat_lb_flows_ctx *ctx, return; } - char *action = type == LROUTER_NAT_LB_FLOW_NORMAL - ? gw_action : ctx->est[type].action; + const char *action = (type == LROUTER_NAT_LB_FLOW_NORMAL) + ? gw_action : ctx->est_action[type]; ds_put_format(ctx->undnat_match, ") && outport == %s && is_chassis_resident(%s)", @@ -10538,41 +10499,44 @@ build_gw_lrouter_nat_flows_for_lb(struct lrouter_nat_lb_flows_ctx *ctx, enum lrouter_nat_lb_flow_type type, const unsigned long *dp_bitmap) { - struct ovn_lflow *new_lflow_ref = NULL; - struct lrouter_nat_lb_flow *new_flow = &ctx->new[type]; - struct lrouter_nat_lb_flow *est_flow = &ctx->est[type]; - struct ovs_mutex *hash_lock; + unsigned long *dp_non_meter = NULL; + bool build_non_meter = false; size_t index; if (bitmap_is_all_zeros(dp_bitmap, n_datapaths)) { return; } - hash_lock = lflow_hash_lock(ctx->lflows, new_flow->hash); - BITMAP_FOR_EACH_1 (index, n_datapaths, dp_bitmap) { - struct ovn_datapath *od = datapaths_array[index]; - const char *meter = NULL; - struct ovn_lflow *lflow; + if (ctx->reject) { + dp_non_meter = bitmap_clone(dp_bitmap, n_datapaths); + BITMAP_FOR_EACH_1 (index, n_datapaths, dp_bitmap) { + struct ovn_datapath *od = datapaths_array[index]; + const char *meter; - if (ctx->reject) { meter = copp_meter_get(COPP_REJECT, od->nbr->copp, ctx->meter_groups); - } - - if (meter || - !ovn_dp_group_add_with_reference(new_lflow_ref, od, NULL)) { - lflow = ovn_lflow_add_at_with_hash(ctx->lflows, od, NULL, - S_ROUTER_IN_DNAT, ctx->prio, ds_cstr(ctx->new_match), - new_flow->action, NULL, meter, &ctx->lb->nlb->header_, - OVS_SOURCE_LOCATOR, new_flow->hash); - new_lflow_ref = meter ? NULL : lflow; + if (!meter) { + build_non_meter = true; + continue; + } + bitmap_set0(dp_non_meter, index); + ovn_lflow_add_with_hint__(ctx->lflows, od, S_ROUTER_IN_DNAT, + ctx->prio, ds_cstr(ctx->new_match), ctx->new_action[type], + NULL, meter, &ctx->lb->nlb->header_); } } - lflow_hash_unlock(hash_lock); + if (!dp_non_meter || build_non_meter) { + ovn_lflow_add_with_dp_group(ctx->lflows, + dp_non_meter ? dp_non_meter : dp_bitmap, + S_ROUTER_IN_DNAT, ctx->prio, ds_cstr(ctx->new_match), + ctx->new_action[type], &ctx->lb->nlb->header_); + } + bitmap_free(dp_non_meter); ovn_lflow_add_with_dp_group( ctx->lflows, dp_bitmap, S_ROUTER_IN_DNAT, ctx->prio, - ds_cstr(ctx->est_match), est_flow->action, &ctx->lb->nlb->header_); + ds_cstr(ctx->est_match), ctx->est_action[type], + &ctx->lb->nlb->header_); } static void @@ -10666,22 +10630,16 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip *lb_vip, .undnat_match = &undnat_match }; - ctx.new[LROUTER_NAT_LB_FLOW_NORMAL] = - LROUTER_NAT_LB_FLOW_INIT(match, ds_cstr(action), prio); - ctx.est[LROUTER_NAT_LB_FLOW_NORMAL] = - LROUTER_NAT_LB_FLOW_INIT(&est_match, "next;", prio); - - ctx.new[LROUTER_NAT_LB_FLOW_SKIP_SNAT] = - LROUTER_NAT_LB_FLOW_INIT(match, ds_cstr(&skip_snat_act), prio); - ctx.est[LROUTER_NAT_LB_FLOW_SKIP_SNAT] = - LROUTER_NAT_LB_FLOW_INIT(&est_match, - "flags.skip_snat_for_lb = 1; next;", prio); - - ctx.new[LROUTER_NAT_LB_FLOW_FORCE_SNAT] = - LROUTER_NAT_LB_FLOW_INIT(match, ds_cstr(&force_snat_act), prio); - ctx.est[LROUTER_NAT_LB_FLOW_FORCE_SNAT] = - LROUTER_NAT_LB_FLOW_INIT(&est_match, - "flags.force_snat_for_lb = 1; next;", prio); + ctx.new_action[LROUTER_NAT_LB_FLOW_NORMAL] = ds_cstr(action); + ctx.est_action[LROUTER_NAT_LB_FLOW_NORMAL] = "next;"; + + ctx.new_action[LROUTER_NAT_LB_FLOW_SKIP_SNAT] = ds_cstr(&skip_snat_act); + ctx.est_action[LROUTER_NAT_LB_FLOW_SKIP_SNAT] = + "flags.skip_snat_for_lb = 1; next;"; + + ctx.new_action[LROUTER_NAT_LB_FLOW_FORCE_SNAT] = ds_cstr(&force_snat_act); + ctx.est_action[LROUTER_NAT_LB_FLOW_FORCE_SNAT] = + "flags.force_snat_for_lb = 1; next;"; enum { LROUTER_NAT_LB_AFF = LROUTER_NAT_LB_FLOW_MAX, From patchwork Tue Feb 7 11:43:02 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Maximets X-Patchwork-Id: 1738903 X-Patchwork-Delegate: mmichels@redhat.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::138; helo=smtp1.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4PB1Vg0V43z23jB for ; Tue, 7 Feb 2023 22:43:55 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 2608881EC0; Tue, 7 Feb 2023 11:43:53 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 2608881EC0 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 SqVkvjNohJKI; Tue, 7 Feb 2023 11:43:51 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp1.osuosl.org (Postfix) with ESMTPS id B187D81E9C; Tue, 7 Feb 2023 11:43:49 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org B187D81E9C Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 97836C0033; Tue, 7 Feb 2023 11:43:49 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 1A6C1C0077 for ; Tue, 7 Feb 2023 11:43:48 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id A032981E00 for ; Tue, 7 Feb 2023 11:43:26 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org A032981E00 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 I7QrmpIafPko for ; Tue, 7 Feb 2023 11:43:25 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 8CE2781C56 Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [IPv6:2001:4b98:dc4:8::225]) by smtp1.osuosl.org (Postfix) with ESMTPS id 8CE2781C56 for ; Tue, 7 Feb 2023 11:43:25 +0000 (UTC) Received: (Authenticated sender: i.maximets@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id 026211C0004; Tue, 7 Feb 2023 11:43:22 +0000 (UTC) From: Ilya Maximets To: ovs-dev@openvswitch.org Date: Tue, 7 Feb 2023 12:43:02 +0100 Message-Id: <20230207114302.1908836-9-i.maximets@ovn.org> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230207114302.1908836-1-i.maximets@ovn.org> References: <20230207114302.1908836-1-i.maximets@ovn.org> MIME-Version: 1.0 Cc: Dumitru Ceara , Ilya Maximets Subject: [ovs-dev] [PATCH ovn v2 8/8] northd: Don't collect datapath groups for LB affinity if disabled. 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" If affinity timeout is not set, these datapath groups will not be used, so no need to collect them. Signed-off-by: Ilya Maximets --- northd/northd.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/northd/northd.c b/northd/northd.c index d2acfc502..6edea36ff 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -10674,11 +10674,13 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip *lb_vip, build_distr_lrouter_nat_flows_for_lb(&ctx, type, od); } - if (!lport_addresses_is_empty(&od->lb_force_snat_addrs) || - od->lb_force_snat_router_ip) { - bitmap_set1(dp_bitmap[LROUTER_NAT_LB_AFF_FORCE_SNAT], index); - } else { - bitmap_set1(dp_bitmap[LROUTER_NAT_LB_AFF], index); + if (lb->affinity_timeout) { + if (!lport_addresses_is_empty(&od->lb_force_snat_addrs) || + od->lb_force_snat_router_ip) { + bitmap_set1(dp_bitmap[LROUTER_NAT_LB_AFF_FORCE_SNAT], index); + } else { + bitmap_set1(dp_bitmap[LROUTER_NAT_LB_AFF], index); + } } if (sset_contains(&od->external_ips, lb_vip->vip_str)) {