From patchwork Mon Sep 11 16:01:18 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1832348 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=patchwork.ozlabs.org) 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 (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4Rks0s0gLKz1ygM for ; Tue, 12 Sep 2023 02:02:05 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 4CF77417BD; Mon, 11 Sep 2023 16:02:03 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 4CF77417BD 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 TyueXVari9Pq; Mon, 11 Sep 2023 16:02:01 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp4.osuosl.org (Postfix) with ESMTPS id 55108404FF; Mon, 11 Sep 2023 16:02:00 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 55108404FF Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 29806C0072; Mon, 11 Sep 2023 16:02:00 +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 C37C0C0032 for ; Mon, 11 Sep 2023 16:01:58 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 94BC841487 for ; Mon, 11 Sep 2023 16:01:58 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 94BC841487 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 6mYh1k3O-VEJ for ; Mon, 11 Sep 2023 16:01:57 +0000 (UTC) Received: from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net [217.70.183.195]) by smtp2.osuosl.org (Postfix) with ESMTPS id E0B9741479 for ; Mon, 11 Sep 2023 16:01:56 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org E0B9741479 Received: by mail.gandi.net (Postfix) with ESMTPSA id D5CDC6001D; Mon, 11 Sep 2023 16:01:22 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Mon, 11 Sep 2023 12:01:18 -0400 Message-ID: <20230911160118.179671-1-numans@ovn.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230911155930.179283-1-numans@ovn.org> References: <20230911155930.179283-1-numans@ovn.org> MIME-Version: 1.0 X-GND-Sasl: numans@ovn.org Subject: [ovs-dev] [PATCH ovn v7 2/4] northd: Handle load balancer group 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 For a given load balancer group 'A', northd engine data maintains a bitmap of datapaths associated to this lb group. So when lb group 'A' gets associated to a logical switch 's1', the bitmap index of 's1' is set in its bitmap. In order to handle the load balancer group changes incrementally for a logical switch, we need to set and clear the bitmap bits accordingly. And this patch does it. Reviewed-by: Ales Musil Acked-by: Mark Michelson Signed-off-by: Numan Siddique --- northd/en-lb-data.c | 102 ++++++++++++++++++++++++++++++++------------ northd/en-lb-data.h | 4 ++ northd/northd.c | 62 +++++++++++++++++++++++---- tests/ovn-northd.at | 30 +++++++++---- 4 files changed, 154 insertions(+), 44 deletions(-) diff --git a/northd/en-lb-data.c b/northd/en-lb-data.c index 02b1bfd7a4..3604ba8226 100644 --- a/northd/en-lb-data.c +++ b/northd/en-lb-data.c @@ -63,6 +63,7 @@ static struct crupdated_lbgrp * static void add_deleted_lbgrp_to_tracked_data( struct ovn_lb_group *, struct tracked_lb_data *); static bool is_ls_lbs_changed(const struct nbrec_logical_switch *nbs); +static bool is_ls_lbgrps_changed(const struct nbrec_logical_switch *nbs); /* 'lb_data' engine node manages the NB load balancers and load balancer * groups. For each NB LB, it creates 'struct ovn_northd_lb' and @@ -271,12 +272,15 @@ lb_data_logical_switch_handler(struct engine_node *node, void *data) destroy_od_lb_data(od_lb_data); } } else { - if (!is_ls_lbs_changed(nbs)) { + bool ls_lbs_changed = is_ls_lbs_changed(nbs); + bool ls_lbgrps_changed = is_ls_lbgrps_changed(nbs); + if (!ls_lbs_changed && !ls_lbgrps_changed) { continue; } struct crupdated_od_lb_data *codlb = xzalloc(sizeof *codlb); codlb->od_uuid = nbs->header_.uuid; uuidset_init(&codlb->assoc_lbs); + uuidset_init(&codlb->assoc_lbgrps); struct od_lb_data *od_lb_data = find_od_lb_data(&lb_data->ls_lb_map, &nbs->header_.uuid); @@ -285,38 +289,66 @@ lb_data_logical_switch_handler(struct engine_node *node, void *data) &nbs->header_.uuid); } - struct uuidset *pre_lb_uuids = od_lb_data->lbs; - od_lb_data->lbs = xzalloc(sizeof *od_lb_data->lbs); - uuidset_init(od_lb_data->lbs); - - for (size_t i = 0; i < nbs->n_load_balancer; i++) { - const struct uuid *lb_uuid = - &nbs->load_balancer[i]->header_.uuid; - uuidset_insert(od_lb_data->lbs, lb_uuid); + if (ls_lbs_changed) { + struct uuidset *pre_lb_uuids = od_lb_data->lbs; + od_lb_data->lbs = xzalloc(sizeof *od_lb_data->lbs); + uuidset_init(od_lb_data->lbs); + + for (size_t i = 0; i < nbs->n_load_balancer; i++) { + const struct uuid *lb_uuid = + &nbs->load_balancer[i]->header_.uuid; + uuidset_insert(od_lb_data->lbs, lb_uuid); + + struct uuidset_node *unode = uuidset_find(pre_lb_uuids, + lb_uuid); + + if (!unode || (nbrec_load_balancer_row_get_seqno( + nbs->load_balancer[i], + OVSDB_IDL_CHANGE_MODIFY) > 0)) { + /* Add this lb to the tracked data. */ + uuidset_insert(&codlb->assoc_lbs, lb_uuid); + changed = true; + } + + if (unode) { + uuidset_delete(pre_lb_uuids, unode); + } + } + if (!uuidset_is_empty(pre_lb_uuids)) { + trk_lb_data->has_dissassoc_lbs_from_od = true; + changed = true; + } - struct uuidset_node *unode = uuidset_find(pre_lb_uuids, - lb_uuid); + uuidset_destroy(pre_lb_uuids); + free(pre_lb_uuids); + } - if (!unode || (nbrec_load_balancer_row_get_seqno( - nbs->load_balancer[i], OVSDB_IDL_CHANGE_MODIFY) > 0)) { - /* Add this lb to the tracked data. */ - uuidset_insert(&codlb->assoc_lbs, lb_uuid); - changed = true; + if (ls_lbgrps_changed) { + struct uuidset *pre_lbgrp_uuids = od_lb_data->lbgrps; + od_lb_data->lbgrps = xzalloc(sizeof *od_lb_data->lbgrps); + uuidset_init(od_lb_data->lbgrps); + for (size_t i = 0; i < nbs->n_load_balancer_group; i++) { + const struct uuid *lbg_uuid = + &nbs->load_balancer_group[i]->header_.uuid; + uuidset_insert(od_lb_data->lbgrps, lbg_uuid); + + if (!uuidset_find_and_delete(pre_lbgrp_uuids, + lbg_uuid)) { + /* Add this lb group to the tracked data. */ + uuidset_insert(&codlb->assoc_lbgrps, lbg_uuid); + changed = true; + } } - if (unode) { - uuidset_delete(pre_lb_uuids, unode); + if (!uuidset_is_empty(pre_lbgrp_uuids)) { + trk_lb_data->has_dissassoc_lbgrps_from_od = true; + changed = true; } - } - if (!uuidset_is_empty(pre_lb_uuids)) { - trk_lb_data->has_dissassoc_lbs_from_od = true; - changed = true; + uuidset_destroy(pre_lbgrp_uuids); + free(pre_lbgrp_uuids); } - uuidset_destroy(pre_lb_uuids); - free(pre_lb_uuids); - ovs_list_insert(&trk_lb_data->crupdated_ls_lbs, &codlb->list_node); } } @@ -412,7 +444,7 @@ build_od_lb_map(const struct nbrec_logical_switch_table *nbrec_ls_table, { const struct nbrec_logical_switch *nbrec_ls; NBREC_LOGICAL_SWITCH_TABLE_FOR_EACH (nbrec_ls, nbrec_ls_table) { - if (!nbrec_ls->n_load_balancer) { + if (!nbrec_ls->n_load_balancer && !nbrec_ls->n_load_balancer_group) { continue; } @@ -422,6 +454,10 @@ build_od_lb_map(const struct nbrec_logical_switch_table *nbrec_ls_table, uuidset_insert(od_lb_data->lbs, &nbrec_ls->load_balancer[i]->header_.uuid); } + for (size_t i = 0; i < nbrec_ls->n_load_balancer_group; i++) { + uuidset_insert(od_lb_data->lbgrps, + &nbrec_ls->load_balancer_group[i]->header_.uuid); + } } } @@ -431,7 +467,9 @@ create_od_lb_data(struct hmap *od_lb_map, const struct uuid *od_uuid) struct od_lb_data *od_lb_data = xzalloc(sizeof *od_lb_data); od_lb_data->od_uuid = *od_uuid; od_lb_data->lbs = xzalloc(sizeof *od_lb_data->lbs); + od_lb_data->lbgrps = xzalloc(sizeof *od_lb_data->lbgrps); uuidset_init(od_lb_data->lbs); + uuidset_init(od_lb_data->lbgrps); hmap_insert(od_lb_map, &od_lb_data->hmap_node, uuid_hash(&od_lb_data->od_uuid)); @@ -456,7 +494,9 @@ static void destroy_od_lb_data(struct od_lb_data *od_lb_data) { uuidset_destroy(od_lb_data->lbs); + uuidset_destroy(od_lb_data->lbgrps); free(od_lb_data->lbs); + free(od_lb_data->lbgrps); free(od_lb_data); } @@ -467,6 +507,7 @@ destroy_tracked_data(struct ed_type_lb_data *lb_data) lb_data->tracked_lb_data.has_health_checks = false; lb_data->tracked_lb_data.has_dissassoc_lbs_from_lbgrps = false; lb_data->tracked_lb_data.has_dissassoc_lbs_from_od = false; + lb_data->tracked_lb_data.has_dissassoc_lbgrps_from_od = false; struct hmapx_node *node; HMAPX_FOR_EACH_SAFE (node, &lb_data->tracked_lb_data.deleted_lbs) { @@ -497,7 +538,7 @@ destroy_tracked_data(struct ed_type_lb_data *lb_data) &lb_data->tracked_lb_data.crupdated_ls_lbs) { ovs_list_remove(&codlb->list_node); uuidset_destroy(&codlb->assoc_lbs); - + uuidset_destroy(&codlb->assoc_lbgrps); free(codlb); } } @@ -552,3 +593,10 @@ is_ls_lbs_changed(const struct nbrec_logical_switch *nbs) { || nbrec_logical_switch_is_updated(nbs, NBREC_LOGICAL_SWITCH_COL_LOAD_BALANCER)); } + +static bool +is_ls_lbgrps_changed(const struct nbrec_logical_switch *nbs) { + return ((nbrec_logical_switch_is_new(nbs) && nbs->n_load_balancer_group) + || nbrec_logical_switch_is_updated(nbs, + NBREC_LOGICAL_SWITCH_COL_LOAD_BALANCER_GROUP)); +} diff --git a/northd/en-lb-data.h b/northd/en-lb-data.h index 022b38523b..d802d000d4 100644 --- a/northd/en-lb-data.h +++ b/northd/en-lb-data.h @@ -33,6 +33,7 @@ struct crupdated_od_lb_data { struct uuid od_uuid; struct uuidset assoc_lbs; + struct uuidset assoc_lbgrps; }; struct tracked_lb_data { @@ -62,6 +63,9 @@ struct tracked_lb_data { /* Indicates if a lb was disassociated from a logical switch. */ bool has_dissassoc_lbs_from_od; + + /* Indicates if a lb group was disassociated from a logical switch. */ + bool has_dissassoc_lbgrps_from_od; }; /* struct which maintains the data of the engine node lb_data. */ diff --git a/northd/northd.c b/northd/northd.c index f767e0b39f..c5e1d1c5a4 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -5075,6 +5075,7 @@ ls_port_create(struct ovsdb_idl_txn *ovnsb_txn, struct hmap *ls_ports, * Presently supports i-p for the below changes: * - logical switch ports. * - load balancers. + * - load balancer groups. */ static bool ls_changes_can_be_handled( @@ -5086,7 +5087,8 @@ ls_changes_can_be_handled( if (nbrec_logical_switch_is_updated(ls, col)) { if (col == NBREC_LOGICAL_SWITCH_COL_ACLS || col == NBREC_LOGICAL_SWITCH_COL_PORTS || - col == NBREC_LOGICAL_SWITCH_COL_LOAD_BALANCER) { + col == NBREC_LOGICAL_SWITCH_COL_LOAD_BALANCER || + col == NBREC_LOGICAL_SWITCH_COL_LOAD_BALANCER_GROUP) { continue; } return false; @@ -5111,12 +5113,6 @@ ls_changes_can_be_handled( return false; } } - 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) { - return false; - } - } for (size_t i = 0; i < ls->n_qos_rules; i++) { if (nbrec_qos_row_get_seqno(ls->qos_rules[i], OVSDB_IDL_CHANGE_MODIFY) > 0) { @@ -5304,7 +5300,11 @@ fail: /* Return true if changes are handled incrementally, false otherwise. * When there are any changes, try to track what's exactly changed and set * northd_data->change_tracked accordingly: change tracked - true, otherwise, - * false. */ + * false. + * + * Note: Changes to load balancer and load balancer groups associated with + * the logical switches are handled separately in the lb_data change handlers. + * */ bool northd_handle_ls_changes(struct ovsdb_idl_txn *ovnsb_idl_txn, const struct northd_input *ni, @@ -5473,6 +5473,12 @@ northd_handle_lb_data_changes(struct tracked_lb_data *trk_lb_data, return false; } + /* Fall back to recompute if any load balancer group has been disassociated + * from a logical switch or router. */ + if (trk_lb_data->has_dissassoc_lbgrps_from_od) { + return false; + } + struct ovn_lb_datapaths *lb_dps; struct ovn_northd_lb *lb; struct ovn_datapath *od; @@ -5543,6 +5549,22 @@ northd_handle_lb_data_changes(struct tracked_lb_data *trk_lb_data, ovn_lb_datapaths_add_ls(lb_dps, 1, &od); } + UUIDSET_FOR_EACH (uuidnode, &codlb->assoc_lbgrps) { + lbgrp_dps = ovn_lb_group_datapaths_find(lbgrp_datapaths_map, + &uuidnode->uuid); + ovs_assert(lbgrp_dps); + ovn_lb_group_datapaths_add_ls(lbgrp_dps, 1, &od); + + /* Associate all the lbs of the lbgrp to the datapath 'od' */ + for (size_t j = 0; j < lbgrp_dps->lb_group->n_lbs; j++) { + const struct uuid *lb_uuid + = &lbgrp_dps->lb_group->lbs[j]->nlb->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); + } + } + /* Re-evaluate 'od->has_lb_vip' */ init_lb_for_datapath(od); } @@ -5562,6 +5584,30 @@ northd_handle_lb_data_changes(struct tracked_lb_data *trk_lb_data, } } + HMAP_FOR_EACH (crupdated_lbgrp, hmap_node, &trk_lb_data->crupdated_lbgrps) { + lbgrp = crupdated_lbgrp->lbgrp; + const struct uuid *lb_uuid = &lbgrp->uuid; + + lbgrp_dps = ovn_lb_group_datapaths_find(lbgrp_datapaths_map, + lb_uuid); + ovs_assert(lbgrp_dps); + + struct hmapx_node *hnode; + HMAPX_FOR_EACH (hnode, &crupdated_lbgrp->assoc_lbs) { + lb = hnode->data; + lb_uuid = &lb->nlb->header_.uuid; + lb_dps = ovn_lb_datapaths_find(lb_datapaths_map, lb_uuid); + ovs_assert(lb_dps); + for (size_t i = 0; i < lbgrp_dps->n_ls; i++) { + od = lbgrp_dps->ls[i]; + ovn_lb_datapaths_add_ls(lb_dps, 1, &od); + + /* Re-evaluate 'od->has_lb_vip' */ + init_lb_for_datapath(od); + } + } + } + return true; } diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index f0f8d1f503..34e10663d0 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -2191,6 +2191,18 @@ check ovn-nbctl --wait=sb sync AT_CHECK([ovn-sbctl lflow-list | grep "ls_out_pre_lb.*priority=100" | grep reg0 | sort], [0], [dnl ]) +# Now associate vip again to lb4 and then delete it. +check ovn-nbctl set load_balancer $lb4 vips:"10.0.0.13"="10.0.0.6" +check ovn-nbctl --wait=sb sync +AT_CHECK([ovn-sbctl lflow-list | grep "ls_out_pre_lb.*priority=100" | grep reg0 | sort], [0], [dnl + table=1 (ls_out_pre_lb ), priority=100 , match=(ip), action=(reg0[[2]] = 1; next;) +]) + +check ovn-nbctl lb-del $lb4 +check ovn-nbctl --wait=sb sync +AT_CHECK([ovn-sbctl lflow-list | grep "ls_out_pre_lb.*priority=100" | grep reg0 | sort], [0], [dnl +]) + AT_CLEANUP ]) @@ -10553,21 +10565,21 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats check ovn-nbctl add logical_switch sw0 load_balancer_group $lbg1_uuid check_engine_stats lb_data norecompute compute -check_engine_stats northd recompute nocompute +check_engine_stats northd norecompute compute check_engine_stats lflow recompute nocompute -# Update lb and this should result in recompute +# Update lb and this should not result in northd recompute check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats check ovn-nbctl --wait=sb set load_balancer . options:bar=foo check_engine_stats lb_data norecompute compute -check_engine_stats northd recompute nocompute +check_engine_stats northd norecompute compute check_engine_stats lflow recompute nocompute # Modify the backend of the lb1 vip check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats check ovn-nbctl --wait=sb set load_balancer lb1 vips:'"10.0.0.10:80"'='"10.0.0.100:80"' check_engine_stats lb_data norecompute compute -check_engine_stats northd recompute nocompute +check_engine_stats northd norecompute compute check_engine_stats lflow recompute nocompute CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -10575,7 +10587,7 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats check ovn-nbctl --wait=sb clear load_Balancer lb1 vips check_engine_stats lb_data norecompute compute -check_engine_stats northd recompute nocompute +check_engine_stats northd norecompute compute check_engine_stats lflow recompute nocompute CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -10583,7 +10595,7 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats check ovn-nbctl --wait=sb lb-add lb1 10.0.0.10:80 10.0.0.3:80 check_engine_stats lb_data norecompute compute -check_engine_stats northd recompute nocompute +check_engine_stats northd norecompute compute check_engine_stats lflow recompute nocompute CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -10591,7 +10603,7 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats check ovn-nbctl --wait=sb lb-add lb1 10.0.0.20:80 10.0.0.30:8080 check_engine_stats lb_data norecompute compute -check_engine_stats northd recompute nocompute +check_engine_stats northd norecompute compute check_engine_stats lflow recompute nocompute CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -10650,7 +10662,7 @@ check_engine_stats lflow recompute nocompute check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats check ovn-nbctl add logical_switch sw0 load_balancer_group $lbg1_uuid check_engine_stats lb_data norecompute compute -check_engine_stats northd recompute nocompute +check_engine_stats northd norecompute compute check_engine_stats lflow recompute nocompute check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats @@ -10691,7 +10703,7 @@ check_engine_stats lflow recompute nocompute check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats check ovn-nbctl set logical_switch sw0 load_balancer_group=$lbg1_uuid check_engine_stats lb_data norecompute compute -check_engine_stats northd recompute nocompute +check_engine_stats northd norecompute compute check_engine_stats lflow recompute nocompute CHECK_NO_CHANGE_AFTER_RECOMPUTE