From patchwork Thu Feb 9 18:01:04 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Maximets X-Patchwork-Id: 1740080 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 4PCPnM1rnmz23jH for ; Fri, 10 Feb 2023 05:01:27 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 68B36611FB; Thu, 9 Feb 2023 18:01:25 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 68B36611FB 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 E6YHfLs38BY7; Thu, 9 Feb 2023 18:01:23 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTPS id 51EE461202; Thu, 9 Feb 2023 18:01:22 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 51EE461202 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 26F86C0071; Thu, 9 Feb 2023 18:01:22 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 9245DC0033 for ; Thu, 9 Feb 2023 18:01:20 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 5F5B982230 for ; Thu, 9 Feb 2023 18:01:20 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 5F5B982230 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 ZvSqfuI01iiR for ; Thu, 9 Feb 2023 18:01:19 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 97AE9821F9 Received: from relay11.mail.gandi.net (relay11.mail.gandi.net [IPv6:2001:4b98:dc4:8::231]) by smtp1.osuosl.org (Postfix) with ESMTPS id 97AE9821F9 for ; Thu, 9 Feb 2023 18:01:18 +0000 (UTC) Received: (Authenticated sender: i.maximets@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id 5ACF2100003; Thu, 9 Feb 2023 18:01:14 +0000 (UTC) From: Ilya Maximets To: ovs-dev@openvswitch.org Date: Thu, 9 Feb 2023 19:01:04 +0100 Message-Id: <20230209180111.2214872-2-i.maximets@ovn.org> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230209180111.2214872-1-i.maximets@ovn.org> References: <20230209180111.2214872-1-i.maximets@ovn.org> MIME-Version: 1.0 Cc: Ilya Maximets , Dumitru Ceara Subject: [ovs-dev] [PATCH ovn v3 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 Acked-by: Mark Michelson 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 Thu Feb 9 18:01:05 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Maximets X-Patchwork-Id: 1740081 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 4PCPnQ0CFrz23jH for ; Fri, 10 Feb 2023 05:01:30 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id EE095611F6; Thu, 9 Feb 2023 18:01:27 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org EE095611F6 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 NGyjwO1XrQ9b; Thu, 9 Feb 2023 18:01:26 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTPS id 1DA8260F95; Thu, 9 Feb 2023 18:01:25 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 1DA8260F95 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id C9001C0078; Thu, 9 Feb 2023 18:01:24 +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 77044C007B for ; Thu, 9 Feb 2023 18:01:23 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 2CB30418D2 for ; Thu, 9 Feb 2023 18:01:23 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 2CB30418D2 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 4O5FHHOmksqm for ; Thu, 9 Feb 2023 18:01:21 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 67482418CA Received: from relay11.mail.gandi.net (relay11.mail.gandi.net [217.70.178.231]) by smtp4.osuosl.org (Postfix) with ESMTPS id 67482418CA for ; Thu, 9 Feb 2023 18:01:21 +0000 (UTC) Received: (Authenticated sender: i.maximets@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id 13730100004; Thu, 9 Feb 2023 18:01:17 +0000 (UTC) From: Ilya Maximets To: ovs-dev@openvswitch.org Date: Thu, 9 Feb 2023 19:01:05 +0100 Message-Id: <20230209180111.2214872-3-i.maximets@ovn.org> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230209180111.2214872-1-i.maximets@ovn.org> References: <20230209180111.2214872-1-i.maximets@ovn.org> MIME-Version: 1.0 Cc: Ilya Maximets , Dumitru Ceara Subject: [ovs-dev] [PATCH ovn v3 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 Acked-by: Mark Michelson 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 Thu Feb 9 18:01:06 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Maximets X-Patchwork-Id: 1740083 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 4PCPnd4N2kz23jH for ; Fri, 10 Feb 2023 05:01:41 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 0C214410D8; Thu, 9 Feb 2023 18:01:39 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 0C214410D8 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 rIPHmdn4hv6Q; Thu, 9 Feb 2023 18:01:35 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp2.osuosl.org (Postfix) with ESMTPS id D0C86410BD; Thu, 9 Feb 2023 18:01:32 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org D0C86410BD Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id C29BDC0033; Thu, 9 Feb 2023 18:01:31 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 52E6FC007B for ; Thu, 9 Feb 2023 18:01:29 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 842D741057 for ; Thu, 9 Feb 2023 18:01:26 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 842D741057 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 RwuQ_giNUVJa for ; Thu, 9 Feb 2023 18:01:25 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 0F17D41054 Received: from relay11.mail.gandi.net (relay11.mail.gandi.net [IPv6:2001:4b98:dc4:8::231]) by smtp2.osuosl.org (Postfix) with ESMTPS id 0F17D41054 for ; Thu, 9 Feb 2023 18:01:23 +0000 (UTC) Received: (Authenticated sender: i.maximets@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id D74EF100009; Thu, 9 Feb 2023 18:01:20 +0000 (UTC) From: Ilya Maximets To: ovs-dev@openvswitch.org Date: Thu, 9 Feb 2023 19:01:06 +0100 Message-Id: <20230209180111.2214872-4-i.maximets@ovn.org> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230209180111.2214872-1-i.maximets@ovn.org> References: <20230209180111.2214872-1-i.maximets@ovn.org> MIME-Version: 1.0 Cc: Ilya Maximets , Dumitru Ceara Subject: [ovs-dev] [PATCH ovn v3 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 | 98 ++++++++++++++++++++++++++++++------------------- 1 file changed, 61 insertions(+), 37 deletions(-) diff --git a/northd/northd.c b/northd/northd.c index 4ddfc3af3..afab12916 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. */ @@ -10827,6 +10847,10 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip *lb_vip, free(lb_aff_force_snat_router); free(lb_aff_router); + + for (size_t i = 0; i < LROUTER_NAT_LB_FLOW_MAX; i++) { + bitmap_free(ctx.dp_bitmap[i]); + } } static void From patchwork Thu Feb 9 18:01:07 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Maximets X-Patchwork-Id: 1740082 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::137; helo=smtp4.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::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 4PCPnV2nWRz23jH for ; Fri, 10 Feb 2023 05:01:34 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 9EB2A41900; Thu, 9 Feb 2023 18:01:32 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 9EB2A41900 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 lKjbml24DsKw; Thu, 9 Feb 2023 18:01:30 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp4.osuosl.org (Postfix) with ESMTPS id 6CB9141902; Thu, 9 Feb 2023 18:01:29 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 6CB9141902 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 05DCAC0033; Thu, 9 Feb 2023 18:01:29 +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 90625C0033 for ; Thu, 9 Feb 2023 18:01:27 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 5CA9D8223D for ; Thu, 9 Feb 2023 18:01:27 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 5CA9D8223D 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 I37A3N_KL33n for ; Thu, 9 Feb 2023 18:01:26 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 05A448223A Received: from relay11.mail.gandi.net (relay11.mail.gandi.net [IPv6:2001:4b98:dc4:8::231]) by smtp1.osuosl.org (Postfix) with ESMTPS id 05A448223A for ; Thu, 9 Feb 2023 18:01:25 +0000 (UTC) Received: (Authenticated sender: i.maximets@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id 582F4100005; Thu, 9 Feb 2023 18:01:23 +0000 (UTC) From: Ilya Maximets To: ovs-dev@openvswitch.org Date: Thu, 9 Feb 2023 19:01:07 +0100 Message-Id: <20230209180111.2214872-5-i.maximets@ovn.org> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230209180111.2214872-1-i.maximets@ovn.org> References: <20230209180111.2214872-1-i.maximets@ovn.org> MIME-Version: 1.0 Cc: Ilya Maximets , Dumitru Ceara Subject: [ovs-dev] [PATCH ovn v3 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 | 144 ++++++++++++++++++++++++------------------------ 1 file changed, 71 insertions(+), 73 deletions(-) diff --git a/northd/northd.c b/northd/northd.c index afab12916..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,11 +10846,8 @@ 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; i++) { - bitmap_free(ctx.dp_bitmap[i]); + for (size_t i = 0; i < LROUTER_NAT_LB_FLOW_MAX + 2; i++) { + bitmap_free(dp_bitmap[i]); } } From patchwork Thu Feb 9 18:01:08 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Maximets X-Patchwork-Id: 1740084 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 4PCPnh4lgGz23jH for ; Fri, 10 Feb 2023 05:01:44 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 9C9F94114D; Thu, 9 Feb 2023 18:01:41 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 9C9F94114D 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 bAYyvR79J0aD; Thu, 9 Feb 2023 18:01:38 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp2.osuosl.org (Postfix) with ESMTPS id B27724105B; Thu, 9 Feb 2023 18:01:35 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org B27724105B Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 857E1C0033; Thu, 9 Feb 2023 18:01:35 +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 1341FC002B for ; Thu, 9 Feb 2023 18:01:34 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 12D3D4190D for ; Thu, 9 Feb 2023 18:01:31 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 12D3D4190D 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 XIzFwmal3-bn for ; Thu, 9 Feb 2023 18:01:29 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org CD486418EE Received: from relay11.mail.gandi.net (relay11.mail.gandi.net [217.70.178.231]) by smtp4.osuosl.org (Postfix) with ESMTPS id CD486418EE for ; Thu, 9 Feb 2023 18:01:28 +0000 (UTC) Received: (Authenticated sender: i.maximets@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id CCD6C100006; Thu, 9 Feb 2023 18:01:25 +0000 (UTC) From: Ilya Maximets To: ovs-dev@openvswitch.org Date: Thu, 9 Feb 2023 19:01:08 +0100 Message-Id: <20230209180111.2214872-6-i.maximets@ovn.org> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230209180111.2214872-1-i.maximets@ovn.org> References: <20230209180111.2214872-1-i.maximets@ovn.org> MIME-Version: 1.0 Cc: Ilya Maximets , Dumitru Ceara Subject: [ovs-dev] [PATCH ovn v3 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. Acked-by: Mark Michelson 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 Thu Feb 9 18:01:09 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Maximets X-Patchwork-Id: 1740085 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.133; helo=smtp2.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.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 4PCPnw1kPKz23jH for ; Fri, 10 Feb 2023 05:01:56 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id F2B4F411A3; Thu, 9 Feb 2023 18:01:51 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org F2B4F411A3 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 5itpy_ycbnqO; Thu, 9 Feb 2023 18:01:46 +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 0426641150; Thu, 9 Feb 2023 18:01:41 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 0426641150 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id B9A60C0033; Thu, 9 Feb 2023 18:01:41 +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 350DAC0078 for ; Thu, 9 Feb 2023 18:01:40 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id A6E1782253 for ; Thu, 9 Feb 2023 18:01:32 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org A6E1782253 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 7H2eaG8zjXhL for ; Thu, 9 Feb 2023 18:01:31 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org E23948221E Received: from relay11.mail.gandi.net (relay11.mail.gandi.net [217.70.178.231]) by smtp1.osuosl.org (Postfix) with ESMTPS id E23948221E for ; Thu, 9 Feb 2023 18:01:30 +0000 (UTC) Received: (Authenticated sender: i.maximets@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id 2CF94100005; Thu, 9 Feb 2023 18:01:27 +0000 (UTC) From: Ilya Maximets To: ovs-dev@openvswitch.org Date: Thu, 9 Feb 2023 19:01:09 +0100 Message-Id: <20230209180111.2214872-7-i.maximets@ovn.org> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230209180111.2214872-1-i.maximets@ovn.org> References: <20230209180111.2214872-1-i.maximets@ovn.org> MIME-Version: 1.0 Cc: Ilya Maximets , Dumitru Ceara Subject: [ovs-dev] [PATCH ovn v3 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. Acked-by: Mark Michelson 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 Thu Feb 9 18:01:10 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Maximets X-Patchwork-Id: 1740086 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.133; helo=smtp2.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.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 4PCPny4qJ8z23yC for ; Fri, 10 Feb 2023 05:01:58 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id D664841129; Thu, 9 Feb 2023 18:01:55 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org D664841129 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 jKrbyrfYQecX; Thu, 9 Feb 2023 18:01:52 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp2.osuosl.org (Postfix) with ESMTPS id D19BA41133; Thu, 9 Feb 2023 18:01:46 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org D19BA41133 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 840BBC0033; Thu, 9 Feb 2023 18:01:45 +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 1CD96C002B for ; Thu, 9 Feb 2023 18:01:44 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 92C404190E for ; Thu, 9 Feb 2023 18:01:35 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 92C404190E 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 nvXpRJFfCjWh for ; Thu, 9 Feb 2023 18:01:33 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 710A041916 Received: from relay11.mail.gandi.net (relay11.mail.gandi.net [IPv6:2001:4b98:dc4:8::231]) by smtp4.osuosl.org (Postfix) with ESMTPS id 710A041916 for ; Thu, 9 Feb 2023 18:01:33 +0000 (UTC) Received: (Authenticated sender: i.maximets@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id 80CEA100006; Thu, 9 Feb 2023 18:01:30 +0000 (UTC) From: Ilya Maximets To: ovs-dev@openvswitch.org Date: Thu, 9 Feb 2023 19:01:10 +0100 Message-Id: <20230209180111.2214872-8-i.maximets@ovn.org> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230209180111.2214872-1-i.maximets@ovn.org> References: <20230209180111.2214872-1-i.maximets@ovn.org> MIME-Version: 1.0 Cc: Ilya Maximets , Dumitru Ceara Subject: [ovs-dev] [PATCH ovn v3 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..f4e046273 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 (!reject || 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 (!ctx->reject || 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 Thu Feb 9 18:01:11 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Maximets X-Patchwork-Id: 1740087 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 4PCPpD3bbXz23yC for ; Fri, 10 Feb 2023 05:02:12 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id A5BB0612C2; Thu, 9 Feb 2023 18:02:10 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org A5BB0612C2 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 k2gj3roEr5Y0; Thu, 9 Feb 2023 18:02:09 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTPS id 09457612A4; Thu, 9 Feb 2023 18:02:04 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 09457612A4 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id B6A82C0033; Thu, 9 Feb 2023 18:02:04 +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 1E517C0033 for ; Thu, 9 Feb 2023 18:02:04 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id A906282274 for ; Thu, 9 Feb 2023 18:01:41 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org A906282274 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 KgYIoJWG_Icj for ; Thu, 9 Feb 2023 18:01:37 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org E602182209 Received: from relay11.mail.gandi.net (relay11.mail.gandi.net [IPv6:2001:4b98:dc4:8::231]) by smtp1.osuosl.org (Postfix) with ESMTPS id E602182209 for ; Thu, 9 Feb 2023 18:01:35 +0000 (UTC) Received: (Authenticated sender: i.maximets@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id 2D587100007; Thu, 9 Feb 2023 18:01:32 +0000 (UTC) From: Ilya Maximets To: ovs-dev@openvswitch.org Date: Thu, 9 Feb 2023 19:01:11 +0100 Message-Id: <20230209180111.2214872-9-i.maximets@ovn.org> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230209180111.2214872-1-i.maximets@ovn.org> References: <20230209180111.2214872-1-i.maximets@ovn.org> MIME-Version: 1.0 Cc: Ilya Maximets , Dumitru Ceara Subject: [ovs-dev] [PATCH ovn v3 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. Acked-by: Mark Michelson 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 f4e046273..6aa1893d6 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)) {