From patchwork Tue Feb 7 11:43:01 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Maximets X-Patchwork-Id: 1738902 X-Patchwork-Delegate: mmichels@redhat.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.138; helo=smtp1.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4PB1VW32CHz23jB for ; Tue, 7 Feb 2023 22:43:47 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 343C581E77; Tue, 7 Feb 2023 11:43:45 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 343C581E77 X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 2XZgjLLkAgeP; Tue, 7 Feb 2023 11:43:43 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp1.osuosl.org (Postfix) with ESMTPS id 1633181E8B; Tue, 7 Feb 2023 11:43:41 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 1633181E8B Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id D0EBCC0033; Tue, 7 Feb 2023 11:43:40 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 078CFC0077 for ; Tue, 7 Feb 2023 11:43:40 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 0FFFD81406 for ; Tue, 7 Feb 2023 11:43:25 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 0FFFD81406 X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id bHWUoBmyYZTP for ; Tue, 7 Feb 2023 11:43:23 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org B4C1681C56 Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by smtp1.osuosl.org (Postfix) with ESMTPS id B4C1681C56 for ; Tue, 7 Feb 2023 11:43:22 +0000 (UTC) Received: (Authenticated sender: i.maximets@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id 507B21C0010; Tue, 7 Feb 2023 11:43:20 +0000 (UTC) From: Ilya Maximets To: ovs-dev@openvswitch.org Date: Tue, 7 Feb 2023 12:43:01 +0100 Message-Id: <20230207114302.1908836-8-i.maximets@ovn.org> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230207114302.1908836-1-i.maximets@ovn.org> References: <20230207114302.1908836-1-i.maximets@ovn.org> MIME-Version: 1.0 Cc: Dumitru Ceara , Ilya Maximets Subject: [ovs-dev] [PATCH ovn v2 7/8] northd: Create metered flows with dp groups if CoPP is not configured. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" Previous change introduced the ovn_lflow_add_with_dp_group() wrapper and applied it to all the non-metered logical flows. Metered ones are a bit more complex, but still can be optimized to use the new wrapper in cases where CoPP is not enabled or enabled only on some datapaths. Iterating over all the datapaths and excluding metered ones from the bitmap. For the remaining datapaths the new wrapper can be used. Metered flows can be created right away. This might be slightly slower for a case where CoPP is enabled due to more hash calculations, but that can be optimized later, if necessary. With this change, there are no more users of ovn_lflow_add_at_with_hash(), so it is inlined into ovn_lflow_add_at(). 'struct lrouter_nat_lb_flow' degraded to just an action string, so inlined into ctx structure. Initializer is no longer needed. Signed-off-by: Ilya Maximets --- northd/northd.c | 182 +++++++++++++++++++----------------------------- 1 file changed, 70 insertions(+), 112 deletions(-) diff --git a/northd/northd.c b/northd/northd.c index 941085e0f..d2acfc502 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -5179,28 +5179,24 @@ static thread_local size_t thread_lflow_counter = 0; /* Adds an OVN datapath to a datapath group of existing logical flow. * Version to use when hash bucket locking is NOT required or the corresponding * hash lock is already taken. */ -static bool +static void ovn_dp_group_add_with_reference(struct ovn_lflow *lflow_ref, const struct ovn_datapath *od, const unsigned long *dp_bitmap) OVS_REQUIRES(fake_hash_mutex) { - if (!lflow_ref) { - return false; - } if (od) { bitmap_set1(lflow_ref->dpg_bitmap, od->index); } if (dp_bitmap) { bitmap_or(lflow_ref->dpg_bitmap, dp_bitmap, n_datapaths); } - return true; } /* Adds a row with the specified contents to the Logical_Flow table. * Version to use when hash bucket locking is NOT required. */ -static struct ovn_lflow * +static void do_ovn_lflow_add(struct hmap *lflow_map, const struct ovn_datapath *od, const unsigned long *dp_bitmap, uint32_t hash, enum ovn_stage stage, uint16_t priority, @@ -5217,7 +5213,7 @@ do_ovn_lflow_add(struct hmap *lflow_map, const struct ovn_datapath *od, actions, ctrl_meter, hash); if (old_lflow) { ovn_dp_group_add_with_reference(old_lflow, od, dp_bitmap); - return old_lflow; + return; } lflow = xmalloc(sizeof *lflow); @@ -5238,32 +5234,6 @@ do_ovn_lflow_add(struct hmap *lflow_map, const struct ovn_datapath *od, hmap_insert_fast(lflow_map, &lflow->hmap_node, hash); thread_lflow_counter++; } - return lflow; -} - -/* Adds a row with the specified contents to the Logical_Flow table. - * Version to use when hash is pre-computed and a hash bucket is already - * locked if necessary. */ -static struct ovn_lflow * -ovn_lflow_add_at_with_hash(struct hmap *lflow_map, - const struct ovn_datapath *od, - const unsigned long *dp_bitmap, - enum ovn_stage stage, uint16_t priority, - const char *match, const char *actions, - const char *io_port, const char *ctrl_meter, - const struct ovsdb_idl_row *stage_hint, - const char *where, uint32_t hash) - OVS_REQUIRES(fake_hash_mutex) -{ - struct ovn_lflow *lflow; - - ovs_assert(!od || - ovn_stage_to_datapath_type(stage) == ovn_datapath_get_type(od)); - - lflow = do_ovn_lflow_add(lflow_map, od, dp_bitmap, hash, stage, priority, - match, actions, io_port, stage_hint, where, - ctrl_meter); - return lflow; } /* Adds a row with the specified contents to the Logical_Flow table. */ @@ -5279,15 +5249,17 @@ ovn_lflow_add_at(struct hmap *lflow_map, const struct ovn_datapath *od, struct ovs_mutex *hash_lock; uint32_t hash; + ovs_assert(!od || + ovn_stage_to_datapath_type(stage) == ovn_datapath_get_type(od)); + hash = ovn_logical_flow_hash(ovn_stage_get_table(stage), ovn_stage_get_pipeline(stage), priority, match, actions); hash_lock = lflow_hash_lock(lflow_map, hash); - ovn_lflow_add_at_with_hash(lflow_map, od, dp_bitmap, stage, priority, - match, actions, io_port, ctrl_meter, - stage_hint, where, hash); + do_ovn_lflow_add(lflow_map, od, dp_bitmap, hash, stage, priority, + match, actions, io_port, stage_hint, where, ctrl_meter); lflow_hash_unlock(hash_lock); } @@ -7514,32 +7486,35 @@ build_lb_rules(struct hmap *lflows, struct ovn_northd_lb *lb, build_lb_affinity_ls_flows(lflows, lb, lb_vip); - struct ovn_lflow *lflow_ref = NULL; - uint32_t hash = ovn_logical_flow_hash( - ovn_stage_get_table(S_SWITCH_IN_LB), - ovn_stage_get_pipeline(S_SWITCH_IN_LB), priority, - ds_cstr(match), ds_cstr(action)); - struct ovs_mutex *hash_lock = lflow_hash_lock(lflows, hash); - size_t index; + unsigned long *dp_non_meter = NULL; + bool build_non_meter = false; + if (reject) { + size_t index; - BITMAP_FOR_EACH_1 (index, n_datapaths, lb->nb_ls_map) { - struct ovn_datapath *od = datapaths_array[index]; + dp_non_meter = bitmap_clone(lb->nb_ls_map, n_datapaths); + BITMAP_FOR_EACH_1 (index, n_datapaths, lb->nb_ls_map) { + struct ovn_datapath *od = datapaths_array[index]; - if (reject) { meter = copp_meter_get(COPP_REJECT, od->nbs->copp, meter_groups); - } - if (meter || !ovn_dp_group_add_with_reference(lflow_ref, - od, NULL)) { - struct ovn_lflow *lflow = ovn_lflow_add_at_with_hash( - lflows, od, NULL, S_SWITCH_IN_LB, priority, + if (!meter) { + build_non_meter = true; + continue; + } + bitmap_set0(dp_non_meter, index); + ovn_lflow_add_with_hint__( + lflows, od, S_SWITCH_IN_LB, priority, ds_cstr(match), ds_cstr(action), - NULL, meter, &lb->nlb->header_, - OVS_SOURCE_LOCATOR, hash); - lflow_ref = meter ? NULL : lflow; + NULL, meter, &lb->nlb->header_); } } - lflow_hash_unlock(hash_lock); + if (!dp_non_meter || build_non_meter) { + ovn_lflow_add_with_dp_group( + lflows, dp_non_meter ? dp_non_meter : lb->nb_ls_map, + S_SWITCH_IN_LB, priority, + ds_cstr(match), ds_cstr(action), &lb->nlb->header_); + } + bitmap_free(dp_non_meter); } } @@ -10442,15 +10417,6 @@ get_force_snat_ip(struct ovn_datapath *od, const char *key_type, return true; } -#define LROUTER_NAT_LB_FLOW_INIT(MATCH, ACTION, PRIO) \ - (struct lrouter_nat_lb_flow) { \ - .action = (ACTION), \ - .hash = ovn_logical_flow_hash( \ - ovn_stage_get_table(S_ROUTER_IN_DNAT), \ - ovn_stage_get_pipeline(S_ROUTER_IN_DNAT), \ - (PRIO), ds_cstr(MATCH), (ACTION)), \ - } - enum lrouter_nat_lb_flow_type { LROUTER_NAT_LB_FLOW_NORMAL = 0, LROUTER_NAT_LB_FLOW_SKIP_SNAT, @@ -10458,14 +10424,9 @@ enum lrouter_nat_lb_flow_type { LROUTER_NAT_LB_FLOW_MAX, }; -struct lrouter_nat_lb_flow { - char *action; - uint32_t hash; -}; - struct lrouter_nat_lb_flows_ctx { - struct lrouter_nat_lb_flow new[LROUTER_NAT_LB_FLOW_MAX]; - struct lrouter_nat_lb_flow est[LROUTER_NAT_LB_FLOW_MAX]; + const char *new_action[LROUTER_NAT_LB_FLOW_MAX]; + const char *est_action[LROUTER_NAT_LB_FLOW_MAX]; struct ds *new_match; struct ds *est_match; @@ -10507,10 +10468,10 @@ build_distr_lrouter_nat_flows_for_lb(struct lrouter_nat_lb_flows_ctx *ctx, } ovn_lflow_add_with_hint__(ctx->lflows, od, S_ROUTER_IN_DNAT, ctx->prio, - ds_cstr(ctx->new_match), ctx->new[type].action, + ds_cstr(ctx->new_match), ctx->new_action[type], NULL, meter, &ctx->lb->nlb->header_); ovn_lflow_add_with_hint(ctx->lflows, od, S_ROUTER_IN_DNAT, ctx->prio, - ds_cstr(ctx->est_match), ctx->est[type].action, + ds_cstr(ctx->est_match), ctx->est_action[type], &ctx->lb->nlb->header_); ds_truncate(ctx->new_match, new_match_len); @@ -10520,8 +10481,8 @@ build_distr_lrouter_nat_flows_for_lb(struct lrouter_nat_lb_flows_ctx *ctx, return; } - char *action = type == LROUTER_NAT_LB_FLOW_NORMAL - ? gw_action : ctx->est[type].action; + const char *action = (type == LROUTER_NAT_LB_FLOW_NORMAL) + ? gw_action : ctx->est_action[type]; ds_put_format(ctx->undnat_match, ") && outport == %s && is_chassis_resident(%s)", @@ -10538,41 +10499,44 @@ build_gw_lrouter_nat_flows_for_lb(struct lrouter_nat_lb_flows_ctx *ctx, enum lrouter_nat_lb_flow_type type, const unsigned long *dp_bitmap) { - struct ovn_lflow *new_lflow_ref = NULL; - struct lrouter_nat_lb_flow *new_flow = &ctx->new[type]; - struct lrouter_nat_lb_flow *est_flow = &ctx->est[type]; - struct ovs_mutex *hash_lock; + unsigned long *dp_non_meter = NULL; + bool build_non_meter = false; size_t index; if (bitmap_is_all_zeros(dp_bitmap, n_datapaths)) { return; } - hash_lock = lflow_hash_lock(ctx->lflows, new_flow->hash); - BITMAP_FOR_EACH_1 (index, n_datapaths, dp_bitmap) { - struct ovn_datapath *od = datapaths_array[index]; - const char *meter = NULL; - struct ovn_lflow *lflow; + if (ctx->reject) { + dp_non_meter = bitmap_clone(dp_bitmap, n_datapaths); + BITMAP_FOR_EACH_1 (index, n_datapaths, dp_bitmap) { + struct ovn_datapath *od = datapaths_array[index]; + const char *meter; - if (ctx->reject) { meter = copp_meter_get(COPP_REJECT, od->nbr->copp, ctx->meter_groups); - } - - if (meter || - !ovn_dp_group_add_with_reference(new_lflow_ref, od, NULL)) { - lflow = ovn_lflow_add_at_with_hash(ctx->lflows, od, NULL, - S_ROUTER_IN_DNAT, ctx->prio, ds_cstr(ctx->new_match), - new_flow->action, NULL, meter, &ctx->lb->nlb->header_, - OVS_SOURCE_LOCATOR, new_flow->hash); - new_lflow_ref = meter ? NULL : lflow; + if (!meter) { + build_non_meter = true; + continue; + } + bitmap_set0(dp_non_meter, index); + ovn_lflow_add_with_hint__(ctx->lflows, od, S_ROUTER_IN_DNAT, + ctx->prio, ds_cstr(ctx->new_match), ctx->new_action[type], + NULL, meter, &ctx->lb->nlb->header_); } } - lflow_hash_unlock(hash_lock); + if (!dp_non_meter || build_non_meter) { + ovn_lflow_add_with_dp_group(ctx->lflows, + dp_non_meter ? dp_non_meter : dp_bitmap, + S_ROUTER_IN_DNAT, ctx->prio, ds_cstr(ctx->new_match), + ctx->new_action[type], &ctx->lb->nlb->header_); + } + bitmap_free(dp_non_meter); ovn_lflow_add_with_dp_group( ctx->lflows, dp_bitmap, S_ROUTER_IN_DNAT, ctx->prio, - ds_cstr(ctx->est_match), est_flow->action, &ctx->lb->nlb->header_); + ds_cstr(ctx->est_match), ctx->est_action[type], + &ctx->lb->nlb->header_); } static void @@ -10666,22 +10630,16 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip *lb_vip, .undnat_match = &undnat_match }; - ctx.new[LROUTER_NAT_LB_FLOW_NORMAL] = - LROUTER_NAT_LB_FLOW_INIT(match, ds_cstr(action), prio); - ctx.est[LROUTER_NAT_LB_FLOW_NORMAL] = - LROUTER_NAT_LB_FLOW_INIT(&est_match, "next;", prio); - - ctx.new[LROUTER_NAT_LB_FLOW_SKIP_SNAT] = - LROUTER_NAT_LB_FLOW_INIT(match, ds_cstr(&skip_snat_act), prio); - ctx.est[LROUTER_NAT_LB_FLOW_SKIP_SNAT] = - LROUTER_NAT_LB_FLOW_INIT(&est_match, - "flags.skip_snat_for_lb = 1; next;", prio); - - ctx.new[LROUTER_NAT_LB_FLOW_FORCE_SNAT] = - LROUTER_NAT_LB_FLOW_INIT(match, ds_cstr(&force_snat_act), prio); - ctx.est[LROUTER_NAT_LB_FLOW_FORCE_SNAT] = - LROUTER_NAT_LB_FLOW_INIT(&est_match, - "flags.force_snat_for_lb = 1; next;", prio); + ctx.new_action[LROUTER_NAT_LB_FLOW_NORMAL] = ds_cstr(action); + ctx.est_action[LROUTER_NAT_LB_FLOW_NORMAL] = "next;"; + + ctx.new_action[LROUTER_NAT_LB_FLOW_SKIP_SNAT] = ds_cstr(&skip_snat_act); + ctx.est_action[LROUTER_NAT_LB_FLOW_SKIP_SNAT] = + "flags.skip_snat_for_lb = 1; next;"; + + ctx.new_action[LROUTER_NAT_LB_FLOW_FORCE_SNAT] = ds_cstr(&force_snat_act); + ctx.est_action[LROUTER_NAT_LB_FLOW_FORCE_SNAT] = + "flags.force_snat_for_lb = 1; next;"; enum { LROUTER_NAT_LB_AFF = LROUTER_NAT_LB_FLOW_MAX,