From patchwork Fri Aug 27 19:17:42 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Maximets X-Patchwork-Id: 1521778 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.137; helo=smtp4.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4Gx8cs3NZNz9sSs for ; Sat, 28 Aug 2021 05:18:05 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id B0F35427CE; Fri, 27 Aug 2021 19:18:02 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id u_yO3UCs2TRk; Fri, 27 Aug 2021 19:17:58 +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 73BFA427A1; Fri, 27 Aug 2021 19:17:56 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 363A8C0022; Fri, 27 Aug 2021 19:17:56 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 1E85CC001C for ; Fri, 27 Aug 2021 19:17:55 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 0CA324082B for ; Fri, 27 Aug 2021 19:17:55 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id eb4l5p9ijYk0 for ; Fri, 27 Aug 2021 19:17:54 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net [217.70.183.196]) by smtp2.osuosl.org (Postfix) with ESMTPS id B683340207 for ; Fri, 27 Aug 2021 19:17:53 +0000 (UTC) Received: (Authenticated sender: i.maximets@ovn.org) by relay4-d.mail.gandi.net (Postfix) with ESMTPSA id 241BEE0002; Fri, 27 Aug 2021 19:17:50 +0000 (UTC) From: Ilya Maximets To: ovs-dev@openvswitch.org Date: Fri, 27 Aug 2021 21:17:42 +0200 Message-Id: <20210827191742.2307529-3-i.maximets@ovn.org> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210827191742.2307529-1-i.maximets@ovn.org> References: <20210827191742.2307529-1-i.maximets@ovn.org> MIME-Version: 1.0 Cc: Ilya Maximets Subject: [ovs-dev] [PATCH ovn 2/2] ovn-northd: Don't check datapath groups in full if not needed. 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 '/* Push changes to the Logical_Flow table to database. */' loop reads all the datapaths from the datapath group and checks them even if it's not required. To check that flow has at least one valid datapath, we need to find one valid datapath. To find Sb logical flow we need a datapath type and for this we only need one datapath again. Technically, the only place where we need to check all datapaths is when we're checking certain Sb datapath group for the first time. In all other cases we can validate/discard the flow by other characteristics that we already know. The change saves a fair amount of time in this loop. Up to a few seconds in case of a big database. More comments added to make the code easier to read. Signed-off-by: Ilya Maximets Acked-by: Dumitru Ceara --- northd/ovn-northd.c | 73 ++++++++++++++++++++++++++------------------- 1 file changed, 43 insertions(+), 30 deletions(-) diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index ae5874518..b7388d317 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -13231,21 +13231,10 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths, const struct sbrec_logical_flow *sbflow, *next_sbflow; SBREC_LOGICAL_FLOW_FOR_EACH_SAFE (sbflow, next_sbflow, ctx->ovnsb_idl) { struct sbrec_logical_dp_group *dp_group = sbflow->logical_dp_group; - struct ovn_datapath **od, *logical_datapath_od = NULL; - int n_datapaths = 0; + struct ovn_datapath *logical_datapath_od = NULL; size_t i; - od = xmalloc((dp_group ? dp_group->n_datapaths + 1 : 1) * sizeof *od); - /* Check all logical datapaths from the group. */ - for (i = 0; dp_group && i < dp_group->n_datapaths; i++) { - od[n_datapaths] = ovn_datapath_from_sbrec(datapaths, - dp_group->datapaths[i]); - if (!od[n_datapaths] || ovn_datapath_is_stale(od[n_datapaths])) { - continue; - } - n_datapaths++; - } - + /* Find one valid datapath to get the datapath type. */ struct sbrec_datapath_binding *dp = sbflow->logical_datapath; if (dp) { logical_datapath_od = ovn_datapath_from_sbrec(datapaths, dp); @@ -13254,26 +13243,29 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths, logical_datapath_od = NULL; } } + for (i = 0; dp_group && i < dp_group->n_datapaths; i++) { + logical_datapath_od = ovn_datapath_from_sbrec( + datapaths, dp_group->datapaths[i]); + if (logical_datapath_od + && !ovn_datapath_is_stale(logical_datapath_od)) { + break; + } + logical_datapath_od = NULL; + } - if (!n_datapaths && !logical_datapath_od) { + if (!logical_datapath_od) { /* This lflow has no valid logical datapaths. */ sbrec_logical_flow_delete(sbflow); - free(od); continue; } enum ovn_pipeline pipeline = !strcmp(sbflow->pipeline, "ingress") ? P_IN : P_OUT; - enum ovn_datapath_type dp_type; - if (n_datapaths) { - dp_type = od[0]->nbs ? DP_SWITCH : DP_ROUTER; - } else { - dp_type = logical_datapath_od->nbs ? DP_SWITCH : DP_ROUTER; - } lflow = ovn_lflow_find( - &lflows, logical_datapath_od, - ovn_stage_build(dp_type, pipeline, sbflow->table_id), + &lflows, dp_group ? NULL : logical_datapath_od, + ovn_stage_build(ovn_datapath_get_type(logical_datapath_od), + pipeline, sbflow->table_id), sbflow->priority, sbflow->match, sbflow->actions, sbflow->controller_meter, sbflow->hash); if (lflow) { @@ -13307,24 +13299,46 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths, * updates. */ bool update_dp_group = false; - if (n_datapaths != hmapx_count(&lflow->od_group)) { - /* Groups are different. */ + if ((!lflow->dpg && dp_group) || (lflow->dpg && !dp_group)) { + /* Need to add or delete datapath group. */ update_dp_group = true; - } else if (lflow->dpg && lflow->dpg->dp_group) { + } else if (!lflow->dpg && !dp_group) { + /* No datapath group and not needed. */ + } else if (lflow->dpg->dp_group) { /* We know the datapath group in Sb that should be used. */ if (lflow->dpg->dp_group != dp_group) { /* Flow has different datapath group in the database. */ update_dp_group = true; } /* Datapath group is already up to date. */ - } else if (n_datapaths) { + } else { + /* There is a datapath group and we need to perfrom + * a full comparison. */ + struct ovn_datapath **od; + int n_datapaths = 0; + + od = xmalloc(dp_group->n_datapaths * sizeof *od); + /* Check all logical datapaths from the group. */ + for (i = 0; i < dp_group->n_datapaths; i++) { + od[n_datapaths] = ovn_datapath_from_sbrec( + datapaths, dp_group->datapaths[i]); + if (!od[n_datapaths] + || ovn_datapath_is_stale(od[n_datapaths])) { + continue; + } + n_datapaths++; + } + + if (n_datapaths != hmapx_count(&lflow->od_group)) { + update_dp_group = true; + } /* Have to compare datapath groups in full. */ - for (i = 0; i < n_datapaths; i++) { + for (i = 0; !update_dp_group && i < n_datapaths; i++) { if (od[i] && !hmapx_contains(&lflow->od_group, od[i])) { update_dp_group = true; - break; } } + free(od); } if (update_dp_group) { @@ -13341,7 +13355,6 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths, } else { sbrec_logical_flow_delete(sbflow); } - free(od); } struct ovn_lflow *next_lflow;