From patchwork Thu Jul 6 10:01:38 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1804234 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::138; helo=smtp1.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4QxXBb3x3Mz20ZQ for ; Thu, 6 Jul 2023 20:02:14 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 952A881FD9; Thu, 6 Jul 2023 10:02:11 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 952A881FD9 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 PCM8bK3rEevI; Thu, 6 Jul 2023 10:02:10 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp1.osuosl.org (Postfix) with ESMTPS id 681C581F21; Thu, 6 Jul 2023 10:02:09 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 681C581F21 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 4C506C0072; Thu, 6 Jul 2023 10:02:09 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::136]) by lists.linuxfoundation.org (Postfix) with ESMTP id D1783C0032 for ; Thu, 6 Jul 2023 10:02:07 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 9F1F2610B4 for ; Thu, 6 Jul 2023 10:02:07 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 9F1F2610B4 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 F9odOubYEk3y for ; Thu, 6 Jul 2023 10:02:06 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 4061B610AF Received: from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net [IPv6:2001:4b98:dc4:8::228]) by smtp3.osuosl.org (Postfix) with ESMTPS id 4061B610AF for ; Thu, 6 Jul 2023 10:02:06 +0000 (UTC) X-GND-Sasl: hzhou@ovn.org X-GND-Sasl: hzhou@ovn.org X-GND-Sasl: hzhou@ovn.org X-GND-Sasl: hzhou@ovn.org Received: by mail.gandi.net (Postfix) with ESMTPSA id 5ED261BF214; Thu, 6 Jul 2023 10:02:00 +0000 (UTC) From: Han Zhou To: dev@openvswitch.org Date: Thu, 6 Jul 2023 03:01:38 -0700 Message-Id: <20230706100140.699587-1-hzhou@ovn.org> X-Mailer: git-send-email 2.31.1 MIME-Version: 1.0 Cc: Dumitru Ceara Subject: [ovs-dev] [PATCH ovn 1/3] northd: Incremental processing of VIF updates and deletions in 'lflow' node. 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" This patch achieves zero recompute for VIF updates and deletions in common scenarios. The performance gain for these scenarios is similar to the patch "northd: Incremental processing of VIF additions in 'lflow' node." Signed-off-by: Han Zhou Acked-by: Numan Siddique --- northd/northd.c | 235 +++++++++++++++++++++++++++++--------------- tests/ovn-northd.at | 4 +- 2 files changed, 156 insertions(+), 83 deletions(-) diff --git a/northd/northd.c b/northd/northd.c index 9df1a49cbfce..b9605862e498 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -16308,6 +16308,115 @@ void build_lflows(struct ovsdb_idl_txn *ovnsb_txn, hmap_destroy(&mcast_groups); } +static void +sync_lsp_lflows_to_sb(struct ovsdb_idl_txn *ovnsb_txn, + struct lflow_input *lflow_input, + struct hmap *lflows, + struct ovn_lflow *lflow) +{ + size_t n_datapaths; + struct ovn_datapath **datapaths_array; + if (ovn_stage_to_datapath_type(lflow->stage) == DP_SWITCH) { + n_datapaths = ods_size(lflow_input->ls_datapaths); + datapaths_array = lflow_input->ls_datapaths->array; + } else { + n_datapaths = ods_size(lflow_input->lr_datapaths); + datapaths_array = lflow_input->lr_datapaths->array; + } + uint32_t n_ods = bitmap_count1(lflow->dpg_bitmap, n_datapaths); + ovs_assert(n_ods == 1); + /* There is only one datapath, so it should be moved out of the + * group to a single 'od'. */ + size_t index = bitmap_scan(lflow->dpg_bitmap, true, 0, + n_datapaths); + + bitmap_set0(lflow->dpg_bitmap, index); + lflow->od = datapaths_array[index]; + + /* Logical flow should be re-hashed to allow lookups. */ + uint32_t hash = hmap_node_hash(&lflow->hmap_node); + /* Remove from lflows. */ + hmap_remove(lflows, &lflow->hmap_node); + hash = ovn_logical_flow_hash_datapath(&lflow->od->sb->header_.uuid, + hash); + /* Add back. */ + hmap_insert(lflows, &lflow->hmap_node, hash); + + /* Sync to SB. */ + const struct sbrec_logical_flow *sbflow; + /* Note: uuid_random acquires a global mutex. If we parallelize the sync to + * SB this may become a bottleneck. */ + lflow->sb_uuid = uuid_random(); + sbflow = sbrec_logical_flow_insert_persist_uuid(ovnsb_txn, + &lflow->sb_uuid); + const char *pipeline = ovn_stage_get_pipeline_name(lflow->stage); + uint8_t table = ovn_stage_get_table(lflow->stage); + sbrec_logical_flow_set_logical_datapath(sbflow, lflow->od->sb); + sbrec_logical_flow_set_logical_dp_group(sbflow, NULL); + sbrec_logical_flow_set_pipeline(sbflow, pipeline); + sbrec_logical_flow_set_table_id(sbflow, table); + sbrec_logical_flow_set_priority(sbflow, lflow->priority); + sbrec_logical_flow_set_match(sbflow, lflow->match); + sbrec_logical_flow_set_actions(sbflow, lflow->actions); + if (lflow->io_port) { + struct smap tags = SMAP_INITIALIZER(&tags); + smap_add(&tags, "in_out_port", lflow->io_port); + sbrec_logical_flow_set_tags(sbflow, &tags); + smap_destroy(&tags); + } + sbrec_logical_flow_set_controller_meter(sbflow, lflow->ctrl_meter); + /* Trim the source locator lflow->where, which looks something like + * "ovn/northd/northd.c:1234", down to just the part following the + * last slash, e.g. "northd.c:1234". */ + const char *slash = strrchr(lflow->where, '/'); +#if _WIN32 + const char *backslash = strrchr(lflow->where, '\\'); + if (!slash || backslash > slash) { + slash = backslash; + } +#endif + const char *where = slash ? slash + 1 : lflow->where; + + struct smap ids = SMAP_INITIALIZER(&ids); + smap_add(&ids, "stage-name", ovn_stage_to_str(lflow->stage)); + smap_add(&ids, "source", where); + if (lflow->stage_hint) { + smap_add(&ids, "stage-hint", lflow->stage_hint); + } + sbrec_logical_flow_set_external_ids(sbflow, &ids); + smap_destroy(&ids); +} + +static bool +delete_lflow_for_lsp(struct ovn_port *op, bool is_update, + const struct sbrec_logical_flow_table *sb_lflow_table, + struct hmap *lflows) +{ + struct lflow_ref_node *lfrn; + const char *operation = is_update ? "updated" : "deleted"; + LIST_FOR_EACH_SAFE (lfrn, lflow_list_node, &op->lflows) { + VLOG_DBG("Deleting SB lflow "UUID_FMT" for %s port %s", + UUID_ARGS(&lfrn->lflow->sb_uuid), operation, op->key); + + const struct sbrec_logical_flow *sblflow = + sbrec_logical_flow_table_get_for_uuid(sb_lflow_table, + &lfrn->lflow->sb_uuid); + if (sblflow) { + sbrec_logical_flow_delete(sblflow); + } else { + static struct vlog_rate_limit rl = + VLOG_RATE_LIMIT_INIT(1, 1); + VLOG_WARN_RL(&rl, "SB lflow "UUID_FMT" not found when handling " + "%s port %s. Recompute.", + UUID_ARGS(&lfrn->lflow->sb_uuid), operation, op->key); + return false; + } + + ovn_lflow_destroy(lflows, lfrn->lflow); + } + return true; +} + bool lflow_handle_northd_ls_changes(struct ovsdb_idl_txn *ovnsb_txn, struct tracked_ls_changes *ls_changes, struct lflow_input *lflow_input, @@ -16315,13 +16424,6 @@ bool lflow_handle_northd_ls_changes(struct ovsdb_idl_txn *ovnsb_txn, { struct ls_change *ls_change; LIST_FOR_EACH (ls_change, list_node, &ls_changes->updated) { - if (!ovs_list_is_empty(&ls_change->updated_ports) || - !ovs_list_is_empty(&ls_change->deleted_ports)) { - /* XXX: implement lflow index so that we can handle updated and - * deleted LSPs incrementally. */ - return false; - } - const struct sbrec_multicast_group *sbmc_flood = mcast_group_lookup(lflow_input->sbrec_mcast_group_by_name_dp, MC_FLOOD, ls_change->od->sb); @@ -16333,6 +16435,47 @@ bool lflow_handle_northd_ls_changes(struct ovsdb_idl_txn *ovnsb_txn, MC_UNKNOWN, ls_change->od->sb); struct ovn_port *op; + LIST_FOR_EACH (op, list, &ls_change->deleted_ports) { + if (!delete_lflow_for_lsp(op, false, + lflow_input->sbrec_logical_flow_table, + lflows)) { + return false; + } + + /* No need to update SB multicast groups, thanks to weak + * references. */ + } + + LIST_FOR_EACH (op, list, &ls_change->updated_ports) { + /* Delete old lflows. */ + if (!delete_lflow_for_lsp(op, true, + lflow_input->sbrec_logical_flow_table, + lflows)) { + return false; + } + + /* Generate new lflows. */ + struct ds match = DS_EMPTY_INITIALIZER; + struct ds actions = DS_EMPTY_INITIALIZER; + build_lswitch_and_lrouter_iterate_by_lsp(op, lflow_input->ls_ports, + lflow_input->lr_ports, + lflow_input->meter_groups, + &match, &actions, + lflows); + ds_destroy(&match); + ds_destroy(&actions); + + /* SB port_binding is not deleted, so don't update SB multicast + * groups. */ + + /* Sync the new flows to SB. */ + struct lflow_ref_node *lfrn; + LIST_FOR_EACH (lfrn, lflow_list_node, &op->lflows) { + sync_lsp_lflows_to_sb(ovnsb_txn, lflow_input, lflows, + lfrn->lflow); + } + } + LIST_FOR_EACH (op, list, &ls_change->added_ports) { struct ds match = DS_EMPTY_INITIALIZER; struct ds actions = DS_EMPTY_INITIALIZER; @@ -16343,6 +16486,8 @@ bool lflow_handle_northd_ls_changes(struct ovsdb_idl_txn *ovnsb_txn, lflows); ds_destroy(&match); ds_destroy(&actions); + + /* Update SB multicast groups for the new port. */ if (!sbmc_flood) { sbmc_flood = create_sb_multicast_group(ovnsb_txn, ls_change->od->sb, MC_FLOOD, OVN_MCAST_FLOOD_TUNNEL_KEY); @@ -16369,80 +16514,8 @@ bool lflow_handle_northd_ls_changes(struct ovsdb_idl_txn *ovnsb_txn, /* Sync the newly added flows to SB. */ struct lflow_ref_node *lfrn; LIST_FOR_EACH (lfrn, lflow_list_node, &op->lflows) { - struct ovn_lflow *lflow = lfrn->lflow; - size_t n_datapaths; - struct ovn_datapath **datapaths_array; - if (ovn_stage_to_datapath_type(lflow->stage) == DP_SWITCH) { - n_datapaths = ods_size(lflow_input->ls_datapaths); - datapaths_array = lflow_input->ls_datapaths->array; - } else { - n_datapaths = ods_size(lflow_input->lr_datapaths); - datapaths_array = lflow_input->lr_datapaths->array; - } - uint32_t n_ods = bitmap_count1(lflow->dpg_bitmap, n_datapaths); - ovs_assert(n_ods == 1); - /* There is only one datapath, so it should be moved out of the - * group to a single 'od'. */ - size_t index = bitmap_scan(lflow->dpg_bitmap, true, 0, - n_datapaths); - - bitmap_set0(lflow->dpg_bitmap, index); - lflow->od = datapaths_array[index]; - - /* Logical flow should be re-hashed to allow lookups. */ - uint32_t hash = hmap_node_hash(&lflow->hmap_node); - /* Remove from lflows. */ - hmap_remove(lflows, &lflow->hmap_node); - hash = ovn_logical_flow_hash_datapath( - &lflow->od->sb->header_.uuid, hash); - /* Add back. */ - hmap_insert(lflows, &lflow->hmap_node, hash); - - /* Sync to SB. */ - const struct sbrec_logical_flow *sbflow; - /* Note: uuid_random acquires a global mutex. If we parallelize - * the sync to SB this may become a bottleneck. */ - lflow->sb_uuid = uuid_random(); - sbflow = sbrec_logical_flow_insert_persist_uuid( - ovnsb_txn, &lflow->sb_uuid); - const char *pipeline = ovn_stage_get_pipeline_name( - lflow->stage); - uint8_t table = ovn_stage_get_table(lflow->stage); - sbrec_logical_flow_set_logical_datapath(sbflow, lflow->od->sb); - sbrec_logical_flow_set_logical_dp_group(sbflow, NULL); - sbrec_logical_flow_set_pipeline(sbflow, pipeline); - sbrec_logical_flow_set_table_id(sbflow, table); - sbrec_logical_flow_set_priority(sbflow, lflow->priority); - sbrec_logical_flow_set_match(sbflow, lflow->match); - sbrec_logical_flow_set_actions(sbflow, lflow->actions); - if (lflow->io_port) { - struct smap tags = SMAP_INITIALIZER(&tags); - smap_add(&tags, "in_out_port", lflow->io_port); - sbrec_logical_flow_set_tags(sbflow, &tags); - smap_destroy(&tags); - } - sbrec_logical_flow_set_controller_meter(sbflow, - lflow->ctrl_meter); - /* Trim the source locator lflow->where, which looks something - * like "ovn/northd/northd.c:1234", down to just the part - * following the last slash, e.g. "northd.c:1234". */ - const char *slash = strrchr(lflow->where, '/'); -#if _WIN32 - const char *backslash = strrchr(lflow->where, '\\'); - if (!slash || backslash > slash) { - slash = backslash; - } -#endif - const char *where = slash ? slash + 1 : lflow->where; - - struct smap ids = SMAP_INITIALIZER(&ids); - smap_add(&ids, "stage-name", ovn_stage_to_str(lflow->stage)); - smap_add(&ids, "source", where); - if (lflow->stage_hint) { - smap_add(&ids, "stage-hint", lflow->stage_hint); - } - sbrec_logical_flow_set_external_ids(sbflow, &ids); - smap_destroy(&ids); + sync_lsp_lflows_to_sb(ovnsb_txn, lflow_input, lflows, + lfrn->lflow); } } } diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 5d871b2873b6..36f3b91a54c1 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -9540,11 +9540,11 @@ for i in $(seq 10); do check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats check ovn-nbctl --wait=hv lsp-del lsp${i}-1 - check_recompute_counter 0 1 || continue + check_recompute_counter 0 0 || continue check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats check ovn-nbctl --wait=hv lsp-set-addresses lsp${i}-2 "aa:aa:aa:00:00:88 192.168.0.88" - check_recompute_counter 0 1 || continue + check_recompute_counter 0 0 || continue # No change, no recompute check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats