From patchwork Fri Jul 7 05:54:16 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1804607 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 4Qy2fb3gD9z20b8 for ; Fri, 7 Jul 2023 15:54:47 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id B3F374118A; Fri, 7 Jul 2023 05:54:45 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org B3F374118A 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 iIAty296O_x8; Fri, 7 Jul 2023 05:54:44 +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 72C6D4051D; Fri, 7 Jul 2023 05:54:43 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 72C6D4051D Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 4B989C0072; Fri, 7 Jul 2023 05:54:43 +0000 (UTC) X-Original-To: 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 8B90AC0032 for ; Fri, 7 Jul 2023 05:54:42 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 3F6CE40577 for ; Fri, 7 Jul 2023 05:54:26 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 3F6CE40577 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 fzjJw66oW3H7 for ; Fri, 7 Jul 2023 05:54:25 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org C647840962 Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by smtp2.osuosl.org (Postfix) with ESMTPS id C647840962 for ; Fri, 7 Jul 2023 05:54:24 +0000 (UTC) X-GND-Sasl: numans@ovn.org X-GND-Sasl: numans@ovn.org Received: by mail.gandi.net (Postfix) with ESMTPSA id C2C261C0005; Fri, 7 Jul 2023 05:54:21 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Fri, 7 Jul 2023 11:24:16 +0530 Message-Id: <20230707055416.961752-1-numans@ovn.org> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20230707055147.961462-1-numans@ovn.org> References: <20230707055147.961462-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v2 5/8] northd: Handle load balancer changes for a logical switch. 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" From: Numan Siddique Logical switch change handler of the 'northd' engine node now also handles changes to load balancers. Signed-off-by: Numan Siddique --- lib/lb.c | 17 +++- lib/lb.h | 2 + northd/northd.c | 212 +++++++++++++++++++++++++++++++++----------- northd/northd.h | 9 ++ tests/ovn-northd.at | 8 +- 5 files changed, 191 insertions(+), 57 deletions(-) diff --git a/lib/lb.c b/lib/lb.c index a51fe225fa..b2b6473c1d 100644 --- a/lib/lb.c +++ b/lib/lb.c @@ -1091,7 +1091,22 @@ ovn_lb_datapaths_add_ls(struct ovn_lb_datapaths *lb_dps, size_t n, struct ovn_datapath **ods) { for (size_t i = 0; i < n; i++) { - bitmap_set1(lb_dps->nb_ls_map, ods[i]->index); + if (!bitmap_is_set(lb_dps->nb_ls_map, ods[i]->index)) { + bitmap_set1(lb_dps->nb_ls_map, ods[i]->index); + lb_dps->n_nb_ls++; + } + } +} + +void +ovn_lb_datapaths_remove_ls(struct ovn_lb_datapaths *lb_dps, size_t n, + struct ovn_datapath **ods) +{ + for (size_t i = 0; i < n; i++) { + if (bitmap_is_set(lb_dps->nb_ls_map, ods[i]->index)) { + bitmap_set0(lb_dps->nb_ls_map, ods[i]->index); + lb_dps->n_nb_ls--; + } } } diff --git a/lib/lb.h b/lib/lb.h index 74905c73b7..ac33333a7f 100644 --- a/lib/lb.h +++ b/lib/lb.h @@ -179,6 +179,8 @@ void ovn_lb_datapaths_add_lr(struct ovn_lb_datapaths *, size_t n, struct ovn_datapath **); void ovn_lb_datapaths_add_ls(struct ovn_lb_datapaths *, size_t n, struct ovn_datapath **); +void ovn_lb_datapaths_remove_ls(struct ovn_lb_datapaths *, size_t n, + struct ovn_datapath **); struct ovn_lb_group_datapaths { struct hmap_node hmap_node; diff --git a/northd/northd.c b/northd/northd.c index fc98ab7bfd..bf02190f7c 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -853,7 +853,8 @@ ovn_datapath_destroy(struct hmap *datapaths, struct ovn_datapath *od) ovn_ls_port_group_destroy(&od->nb_pgs); destroy_mcast_info_for_datapath(od); destroy_ports_for_datapath(od); - + free(od->lb_uuids); + free(od->lb_group_uuids); free(od); } } @@ -4080,6 +4081,48 @@ build_lb_vip_actions(const struct ovn_northd_lb *lb, return reject; } +static void +associate_ls_lbs(struct ovn_datapath *ls_od, struct hmap *lb_datapaths_map) +{ + struct ovn_lb_datapaths *lb_dps; + + free(ls_od->lb_uuids); + ls_od->lb_uuids = xcalloc(ls_od->nbs->n_load_balancer, + sizeof *ls_od->lb_uuids); + ls_od->n_lb_uuids = ls_od->nbs->n_load_balancer; + for (size_t i = 0; i < ls_od->nbs->n_load_balancer; i++) { + const struct uuid *lb_uuid = + &ls_od->nbs->load_balancer[i]->header_.uuid; + lb_dps = ovn_lb_datapaths_find(lb_datapaths_map, lb_uuid); + ovs_assert(lb_dps); + ovn_lb_datapaths_add_ls(lb_dps, 1, &ls_od); + ls_od->lb_uuids[i] = *lb_uuid; + } +} + +static void +associate_ls_lb_groups(struct ovn_datapath *ls_od, + struct hmap *lb_group_datapaths_map) +{ + const struct nbrec_load_balancer_group *nbrec_lb_group; + struct ovn_lb_group_datapaths *lb_group_dps; + + free(ls_od->lb_group_uuids); + ls_od->lb_group_uuids = xcalloc(ls_od->nbs->n_load_balancer_group, + sizeof *ls_od->lb_group_uuids); + ls_od->n_lb_group_uuids = ls_od->nbs->n_load_balancer_group; + for (size_t i = 0; i < ls_od->nbs->n_load_balancer_group; i++) { + nbrec_lb_group = ls_od->nbs->load_balancer_group[i]; + const struct uuid *lb_group_uuid = &nbrec_lb_group->header_.uuid; + lb_group_dps = + ovn_lb_group_datapaths_find(lb_group_datapaths_map, + lb_group_uuid); + ovs_assert(lb_group_dps); + ovn_lb_group_datapaths_add_ls(lb_group_dps, 1, &ls_od); + ls_od->lb_group_uuids[i] = *lb_group_uuid; + } +} + static void build_lb_datapaths(const struct hmap *lbs, const struct hmap *lb_groups, struct ovn_datapaths *ls_datapaths, @@ -4115,24 +4158,8 @@ build_lb_datapaths(const struct hmap *lbs, const struct hmap *lb_groups, if (!od->nbs) { continue; } - - for (size_t i = 0; i < od->nbs->n_load_balancer; i++) { - const struct uuid *lb_uuid = - &od->nbs->load_balancer[i]->header_.uuid; - lb_dps = ovn_lb_datapaths_find(lb_datapaths_map, lb_uuid); - ovs_assert(lb_dps); - ovn_lb_datapaths_add_ls(lb_dps, 1, &od); - } - - for (size_t i = 0; i < od->nbs->n_load_balancer_group; i++) { - nbrec_lb_group = od->nbs->load_balancer_group[i]; - const struct uuid *lb_group_uuid = &nbrec_lb_group->header_.uuid; - lb_group_dps = - ovn_lb_group_datapaths_find(lb_group_datapaths_map, - lb_group_uuid); - ovs_assert(lb_group_dps); - ovn_lb_group_datapaths_add_ls(lb_group_dps, 1, &od); - } + associate_ls_lbs(od, lb_datapaths_map); + associate_ls_lb_groups(od, lb_group_datapaths_map); } HMAP_FOR_EACH (od, key_node, &lr_datapaths->datapaths) { @@ -4891,23 +4918,29 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn, sset_destroy(&active_ha_chassis_grps); } +static void +destroy_tracked_ls_change(struct ls_change *ls_change) +{ + struct ovn_port *op; + LIST_FOR_EACH (op, list, &ls_change->added_ports) { + ovs_list_remove(&op->list); + } + LIST_FOR_EACH (op, list, &ls_change->updated_ports) { + ovs_list_remove(&op->list); + } + LIST_FOR_EACH_SAFE (op, list, &ls_change->deleted_ports) { + ovs_list_remove(&op->list); + ovn_port_destroy_orphan(op); + } +} + void destroy_northd_data_tracked_changes(struct northd_data *nd) { struct ls_change *ls_change; LIST_FOR_EACH_SAFE (ls_change, list_node, &nd->tracked_ls_changes.updated) { - struct ovn_port *op; - LIST_FOR_EACH (op, list, &ls_change->added_ports) { - ovs_list_remove(&op->list); - } - LIST_FOR_EACH (op, list, &ls_change->updated_ports) { - ovs_list_remove(&op->list); - } - LIST_FOR_EACH_SAFE (op, list, &ls_change->deleted_ports) { - ovs_list_remove(&op->list); - ovn_port_destroy_orphan(op); - } + destroy_tracked_ls_change(ls_change); ovs_list_remove(&ls_change->list_node); free(ls_change); } @@ -5028,6 +5061,7 @@ ls_port_create(struct ovsdb_idl_txn *ovnsb_txn, struct hmap *ls_ports, * incrementally handled. * Presently supports i-p for the below changes: * - logical switch ports. + * - load balancers. */ static bool check_unsupported_inc_proc_for_ls_changes( @@ -5036,8 +5070,11 @@ check_unsupported_inc_proc_for_ls_changes( /* Check if the columns are changed in this row. */ enum nbrec_logical_switch_column_id col; for (col = 0; col < NBREC_LOGICAL_SWITCH_N_COLUMNS; col++) { - if (nbrec_logical_switch_is_updated(ls, col) && - col != NBREC_LOGICAL_SWITCH_COL_PORTS) { + if (nbrec_logical_switch_is_updated(ls, col)) { + if (col == NBREC_LOGICAL_SWITCH_COL_PORTS || + col == NBREC_LOGICAL_SWITCH_COL_LOAD_BALANCER) { + continue; + } return true; } } @@ -5066,12 +5103,6 @@ check_unsupported_inc_proc_for_ls_changes( return true; } } - for (size_t i = 0; i < ls->n_load_balancer; i++) { - if (nbrec_load_balancer_row_get_seqno(ls->load_balancer[i], - OVSDB_IDL_CHANGE_MODIFY) > 0) { - return true; - } - } for (size_t i = 0; i < ls->n_load_balancer_group; i++) { if (nbrec_load_balancer_group_row_get_seqno(ls->load_balancer_group[i], OVSDB_IDL_CHANGE_MODIFY) > 0) { @@ -5130,7 +5161,9 @@ ls_check_and_handle_lsp_changes(struct ovsdb_idl_txn *ovnsb_idl_txn, const struct nbrec_logical_switch *changed_ls, const struct northd_input *ni, struct northd_data *nd, - struct ovn_datapath *od) + struct ovn_datapath *od, + struct ls_change *ls_change, + bool *updated) { bool ls_ports_changed = false; if (!nbrec_logical_switch_is_updated(changed_ls, @@ -5151,12 +5184,6 @@ ls_check_and_handle_lsp_changes(struct ovsdb_idl_txn *ovnsb_idl_txn, return true; } - struct ls_change *ls_change = xzalloc(sizeof *ls_change); - ls_change->od = od; - ovs_list_init(&ls_change->added_ports); - ovs_list_init(&ls_change->deleted_ports); - ovs_list_init(&ls_change->updated_ports); - struct ovn_port *op; HMAP_FOR_EACH (op, dp_node, &od->ports) { op->visited = false; @@ -5246,10 +5273,7 @@ ls_check_and_handle_lsp_changes(struct ovsdb_idl_txn *ovnsb_idl_txn, if (!ovs_list_is_empty(&ls_change->added_ports) || !ovs_list_is_empty(&ls_change->updated_ports) || !ovs_list_is_empty(&ls_change->deleted_ports)) { - ovs_list_push_back(&nd->tracked_ls_changes.updated, - &ls_change->list_node); - } else { - free(ls_change); + *updated = true; } return true; @@ -5260,10 +5284,60 @@ fail_clean_deleted: } fail: - free(ls_change); + + LIST_FOR_EACH (op, list, &ls_change->added_ports) { + ovs_list_remove(&op->list); + } + LIST_FOR_EACH (op, list, &ls_change->updated_ports) { + ovs_list_remove(&op->list); + } + LIST_FOR_EACH_SAFE (op, list, &ls_change->deleted_ports) { + ovs_list_remove(&op->list); + ovn_port_destroy_orphan(op); + } return false; } +static bool +ls_check_and_handle_lb_changes(const struct nbrec_logical_switch *changed_ls, + struct hmap *lb_datapaths_map, + struct ovn_datapath *od, + struct ls_change *ls_change, + bool *updated) +{ + + if (!nbrec_logical_switch_is_updated(changed_ls, + NBREC_LOGICAL_SWITCH_COL_LOAD_BALANCER)) { + for (size_t i = 0; i < changed_ls->n_load_balancer; i++) { + if (nbrec_load_balancer_row_get_seqno(changed_ls->load_balancer[i], + OVSDB_IDL_CHANGE_MODIFY) > 0) { + ls_change->lbs_changed = true; + *updated = true; + /* Re-evaluate 'od->has_lb_vip' */ + init_lb_for_datapath(od); + break; + } + } + + return true; + } + + struct ovn_lb_datapaths *lb_dps; + for (size_t i = 0; i < od->n_lb_uuids; i++) { + lb_dps = ovn_lb_datapaths_find(lb_datapaths_map, &od->lb_uuids[i]); + if (lb_dps) { + ovn_lb_datapaths_remove_ls(lb_dps, 1, &od); + } + } + + associate_ls_lbs(od, lb_datapaths_map); + ls_change->lbs_changed = true; + *updated = true; + /* Re-evaluate 'od->has_lb_vip' */ + init_lb_for_datapath(od); + return true; +} + /* Return true if changes are handled incrementally, false otherwise. * When there are any changes, try to track what's exactly changed and set @@ -5293,20 +5367,47 @@ northd_handle_ls_changes(struct ovsdb_idl_txn *ovnsb_idl_txn, goto fail; } - /* Now only able to handle lsp changes. */ + /* Now only able to handle lsp, load balancerload and + * load balancer group changes. */ if (check_unsupported_inc_proc_for_ls_changes(changed_ls)) { goto fail; } + struct ls_change *ls_change = xzalloc(sizeof *ls_change); + ls_change->od = od; + ovs_list_init(&ls_change->added_ports); + ovs_list_init(&ls_change->deleted_ports); + ovs_list_init(&ls_change->updated_ports); + + bool updated = false; if (!ls_check_and_handle_lsp_changes(ovnsb_idl_txn, changed_ls, - ni, nd, od)) { + ni, nd, od, ls_change, + &updated)) { + destroy_tracked_ls_change(ls_change); + free(ls_change); + goto fail; + } + + if (!ls_check_and_handle_lb_changes(changed_ls, + &nd->lb_datapaths_map, od, + ls_change, &updated)) { + destroy_tracked_ls_change(ls_change); + free(ls_change); goto fail; } + + if (updated) { + ovs_list_push_back(&nd->tracked_ls_changes.updated, + &ls_change->list_node); + } else { + free(ls_change); + } } if (!ovs_list_is_empty(&nd->tracked_ls_changes.updated)) { nd->change_tracked = true; } + return true; fail: @@ -16628,6 +16729,13 @@ bool lflow_handle_northd_ls_changes(struct ovsdb_idl_txn *ovnsb_txn, struct hmap *lflows) { struct ls_change *ls_change; + + LIST_FOR_EACH (ls_change, list_node, &ls_changes->updated) { + if (ls_change->lbs_changed) { + return false; + } + } + LIST_FOR_EACH (ls_change, list_node, &ls_changes->updated) { const struct sbrec_multicast_group *sbmc_flood = mcast_group_lookup(lflow_input->sbrec_mcast_group_by_name_dp, diff --git a/northd/northd.h b/northd/northd.h index 7d17921fa2..3afa4eaff2 100644 --- a/northd/northd.h +++ b/northd/northd.h @@ -94,6 +94,7 @@ struct ls_change { struct ovs_list added_ports; struct ovs_list deleted_ports; struct ovs_list updated_ports; + bool lbs_changed; }; /* Track what's changed for logical switches. @@ -318,6 +319,14 @@ struct ovn_datapath { /* Map of ovn_port objects belonging to this datapath. * This map doesn't include derived ports. */ struct hmap ports; + + /* LB uuids associated with this datapath. */ + struct uuid *lb_uuids; + size_t n_lb_uuids; + + /* LB group uuids associated with this datapath. */ + struct uuid *lb_group_uuids; + size_t n_lb_group_uuids; }; void ovnnb_db_run(struct northd_input *input_data, diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index dd0bd8b36a..d9f3917a3e 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -9750,15 +9750,15 @@ check ovn-nbctl ls-add sw0 check ovn-nbctl --wait=sb lr-add lr0 check_engine_stats recompute recompute -# Associate lb1 to sw0. There should be a full recompute of northd engine node +# Associate lb1 to sw0. There should be no recompute of northd engine node check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats check ovn-nbctl --wait=sb ls-lb-add sw0 lb1 -check_engine_stats recompute recompute +check_engine_stats norecompute recompute -# Disassociate lb1 from sw0. There should be a full recompute of northd engine node. +# Disassociate lb1 from sw0. There should be a no recompute of northd engine node. check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats check ovn-nbctl --wait=sb ls-lb-del sw0 lb1 -check_engine_stats recompute recompute +check_engine_stats norecompute recompute # Test load balancer group now check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats