From patchwork Mon Nov 14 16:48:20 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1703675 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.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 ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4N9wHm0yvlz1yqS for ; Tue, 15 Nov 2022 03:48:52 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 3CA0040866; Mon, 14 Nov 2022 16:48:50 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 3CA0040866 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 MpEJ17ZgMjPP; Mon, 14 Nov 2022 16:48:48 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp4.osuosl.org (Postfix) with ESMTPS id 8518A40321; Mon, 14 Nov 2022 16:48:47 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 8518A40321 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 5634FC0033; Mon, 14 Nov 2022 16:48:47 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id C4771C002D for ; Mon, 14 Nov 2022 16:48:46 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 6F99060792 for ; Mon, 14 Nov 2022 16:48:44 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 6F99060792 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 NiFD2CkAQxEo for ; Mon, 14 Nov 2022 16:48:43 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org DCF32600B9 Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net [217.70.183.194]) by smtp3.osuosl.org (Postfix) with ESMTPS id DCF32600B9 for ; Mon, 14 Nov 2022 16:48:42 +0000 (UTC) Received: (Authenticated sender: numans@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id 41CDA40009; Mon, 14 Nov 2022 16:48:38 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Mon, 14 Nov 2022 11:48:20 -0500 Message-Id: <20221114164820.88868-1-numans@ovn.org> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20221114164716.88706-1-numans@ovn.org> References: <20221114164716.88706-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v2 2/2] northd: Add I-P for syncing SB address sets. 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 Updates to NB address sets and NB port groups are handled incrementally for syncing the SB address sets. This patch doesn't support syncing the SB Address sets for the router load balancer vips incrementally, instead a full recompute is triggered for any changes to NB load balancers, NB load balancer groups and NB logical routers. Signed-off-by: Numan Siddique --- northd/en-sb-sync.c | 202 ++++++++++++++++++++++++++++++++++++--- northd/en-sb-sync.h | 6 ++ northd/inc-proc-northd.c | 18 +++- tests/ovn-northd.at | 52 ++++++++++ 4 files changed, 260 insertions(+), 18 deletions(-) diff --git a/northd/en-sb-sync.c b/northd/en-sb-sync.c index c3ba315df..8a17998ec 100644 --- a/northd/en-sb-sync.c +++ b/northd/en-sb-sync.c @@ -22,6 +22,7 @@ #include "openvswitch/util.h" #include "en-sb-sync.h" +#include "include/ovn/expr.h" #include "lib/inc-proc-eng.h" #include "lib/lb.h" #include "lib/ovn-nb-idl.h" @@ -41,6 +42,13 @@ static void sync_address_sets(const struct nbrec_address_set_table *, const struct sbrec_address_set_table *, struct ovsdb_idl_txn *ovnsb_txn, struct hmap *datapaths); +static const struct sbrec_address_set *sb_address_set_lookup_by_name( + struct ovsdb_idl_index *, const char *name); +static void update_sb_addr_set(const char **nb_addresses, size_t n_addresses, + const struct sbrec_address_set *); +static void build_port_group_address_set(const struct nbrec_port_group *, + struct svec *ipv4_addrs, + struct svec *ipv6_addrs); void * en_sb_sync_init(struct engine_node *node OVS_UNUSED, @@ -94,6 +102,98 @@ en_address_set_sync_cleanup(void *data OVS_UNUSED) } +bool +address_set_sync_nb_address_set_handler(struct engine_node *node OVS_UNUSED, + void *data OVS_UNUSED) +{ + const struct nbrec_address_set_table *nb_address_set_table = + EN_OVSDB_GET(engine_get_input("NB_address_set", node)); + + /* Return false if an address set is created or deleted. + * Handle I-P for only updated address sets. */ + const struct nbrec_address_set *nb_addr_set; + NBREC_ADDRESS_SET_TABLE_FOR_EACH_TRACKED (nb_addr_set, + nb_address_set_table) { + if (nbrec_address_set_is_new(nb_addr_set) || + nbrec_address_set_is_deleted(nb_addr_set)) { + return false; + } + } + + struct ovsdb_idl_index *sbrec_address_set_by_name = + engine_ovsdb_node_get_index( + engine_get_input("SB_address_set", node), + "sbrec_address_set_by_name"); + + NBREC_ADDRESS_SET_TABLE_FOR_EACH_TRACKED (nb_addr_set, + nb_address_set_table) { + const struct sbrec_address_set *sb_addr_set = + sb_address_set_lookup_by_name(sbrec_address_set_by_name, + nb_addr_set->name); + if (!sb_addr_set) { + return false; + } + update_sb_addr_set((const char **) nb_addr_set->addresses, + nb_addr_set->n_addresses, sb_addr_set); + } + + return true; +} + +bool +address_set_sync_nb_port_group_handler(struct engine_node *node OVS_UNUSED, + void *data OVS_UNUSED) +{ + const struct nbrec_port_group *nb_pg; + const struct nbrec_port_group_table *nb_port_group_table = + EN_OVSDB_GET(engine_get_input("NB_port_group", node)); + NBREC_PORT_GROUP_TABLE_FOR_EACH_TRACKED (nb_pg, nb_port_group_table) { + if (nbrec_port_group_is_new(nb_pg) || + nbrec_port_group_is_deleted(nb_pg)) { + return false; + } + } + + struct ovsdb_idl_index *sbrec_address_set_by_name = + engine_ovsdb_node_get_index( + engine_get_input("SB_address_set", node), + "sbrec_address_set_by_name"); + NBREC_PORT_GROUP_TABLE_FOR_EACH_TRACKED (nb_pg, nb_port_group_table) { + char *ipv4_addrs_name = xasprintf("%s_ip4", nb_pg->name); + const struct sbrec_address_set *sb_addr_set_v4 = + sb_address_set_lookup_by_name(sbrec_address_set_by_name, + ipv4_addrs_name); + if (!sb_addr_set_v4) { + free(ipv4_addrs_name); + return false; + } + char *ipv6_addrs_name = xasprintf("%s_ip6", nb_pg->name); + const struct sbrec_address_set *sb_addr_set_v6 = + sb_address_set_lookup_by_name(sbrec_address_set_by_name, + ipv6_addrs_name); + if (!sb_addr_set_v6) { + free(ipv4_addrs_name); + free(ipv6_addrs_name); + return false; + } + + struct svec ipv4_addrs = SVEC_EMPTY_INITIALIZER; + struct svec ipv6_addrs = SVEC_EMPTY_INITIALIZER; + build_port_group_address_set(nb_pg, &ipv4_addrs, &ipv6_addrs); + update_sb_addr_set((const char **) ipv4_addrs.names, ipv4_addrs.n, + sb_addr_set_v4); + update_sb_addr_set((const char **) ipv6_addrs.names, ipv6_addrs.n, + sb_addr_set_v6); + + free(ipv4_addrs_name); + free(ipv6_addrs_name); + svec_destroy(&ipv4_addrs); + svec_destroy(&ipv6_addrs); + } + + return true; +} + /* static functions. */ static void sync_address_set(struct ovsdb_idl_txn *ovnsb_txn, const char *name, @@ -106,10 +206,11 @@ sync_address_set(struct ovsdb_idl_txn *ovnsb_txn, const char *name, if (!sb_address_set) { sb_address_set = sbrec_address_set_insert(ovnsb_txn); sbrec_address_set_set_name(sb_address_set, name); + sbrec_address_set_set_addresses(sb_address_set, + addrs, n_addrs); + } else { + update_sb_addr_set(addrs, n_addrs, sb_address_set); } - - sbrec_address_set_set_addresses(sb_address_set, - addrs, n_addrs); } /* OVN_Southbound Address_Set table contains same records as in north @@ -148,18 +249,7 @@ sync_address_sets( nb_port_group_table) { struct svec ipv4_addrs = SVEC_EMPTY_INITIALIZER; struct svec ipv6_addrs = SVEC_EMPTY_INITIALIZER; - for (size_t i = 0; i < nb_port_group->n_ports; i++) { - for (size_t j = 0; j < nb_port_group->ports[i]->n_addresses; j++) { - const char *addrs = nb_port_group->ports[i]->addresses[j]; - if (!is_dynamic_lsp_address(addrs)) { - split_addresses(addrs, &ipv4_addrs, &ipv6_addrs); - } - } - if (nb_port_group->ports[i]->dynamic_addresses) { - split_addresses(nb_port_group->ports[i]->dynamic_addresses, - &ipv4_addrs, &ipv6_addrs); - } - } + build_port_group_address_set(nb_port_group, &ipv4_addrs, &ipv6_addrs); char *ipv4_addrs_name = xasprintf("%s_ip4", nb_port_group->name); char *ipv6_addrs_name = xasprintf("%s_ip6", nb_port_group->name); sync_address_set(ovnsb_txn, ipv4_addrs_name, @@ -228,3 +318,85 @@ sync_address_sets( } shash_destroy(&sb_address_sets); } + +static void +update_sb_addr_set(const char **nb_addresses, size_t n_addresses, + const struct sbrec_address_set *sb_as) +{ + struct expr_constant_set *cs_nb_as = + expr_constant_set_create_integers( + (const char *const *) nb_addresses, n_addresses); + struct expr_constant_set *cs_sb_as = + expr_constant_set_create_integers( + (const char *const *) sb_as->addresses, sb_as->n_addresses); + + struct expr_constant_set *addr_added = NULL; + struct expr_constant_set *addr_deleted = NULL; + expr_constant_set_integers_diff(cs_sb_as, cs_nb_as, &addr_added, + &addr_deleted); + + if (addr_added && addr_added->n_values) { + for (size_t i = 0; i < addr_added->n_values; i++) { + struct ds ds = DS_EMPTY_INITIALIZER; + expr_constant_format(&addr_added->values[i], EXPR_C_INTEGER, &ds); + sbrec_address_set_update_addresses_addvalue(sb_as, ds_cstr(&ds)); + ds_destroy(&ds); + } + } + + if (addr_deleted && addr_deleted->n_values) { + for (size_t i = 0; i < addr_deleted->n_values; i++) { + struct ds ds = DS_EMPTY_INITIALIZER; + expr_constant_format(&addr_deleted->values[i], + EXPR_C_INTEGER, &ds); + sbrec_address_set_update_addresses_delvalue(sb_as, ds_cstr(&ds)); + ds_destroy(&ds); + } + } + + expr_constant_set_destroy(cs_nb_as); + free(cs_nb_as); + expr_constant_set_destroy(cs_sb_as); + free(cs_sb_as); + expr_constant_set_destroy(addr_added); + free(addr_added); + expr_constant_set_destroy(addr_deleted); + free(addr_deleted); +} + +static void +build_port_group_address_set(const struct nbrec_port_group *nb_port_group, + struct svec *ipv4_addrs, + struct svec *ipv6_addrs) +{ + for (size_t i = 0; i < nb_port_group->n_ports; i++) { + for (size_t j = 0; j < nb_port_group->ports[i]->n_addresses; j++) { + const char *addrs = nb_port_group->ports[i]->addresses[j]; + if (!is_dynamic_lsp_address(addrs)) { + split_addresses(addrs, ipv4_addrs, ipv6_addrs); + } + } + if (nb_port_group->ports[i]->dynamic_addresses) { + split_addresses(nb_port_group->ports[i]->dynamic_addresses, + ipv4_addrs, ipv6_addrs); + } + } +} + +/* Finds and returns the address set with the given 'name', or NULL if no such + * address set exists. */ +static const struct sbrec_address_set * +sb_address_set_lookup_by_name(struct ovsdb_idl_index *sbrec_addr_set_by_name, + const char *name) +{ + struct sbrec_address_set *target = sbrec_address_set_index_init_row( + sbrec_addr_set_by_name); + sbrec_address_set_index_set_name(target, name); + + struct sbrec_address_set *retval = sbrec_address_set_index_find( + sbrec_addr_set_by_name, target); + + sbrec_address_set_index_destroy_row(target); + + return retval; +} diff --git a/northd/en-sb-sync.h b/northd/en-sb-sync.h index f99d6a9fc..a63453fe5 100644 --- a/northd/en-sb-sync.h +++ b/northd/en-sb-sync.h @@ -3,12 +3,18 @@ #include "lib/inc-proc-eng.h" +/* en_sb_sync engine node functions. */ void *en_sb_sync_init(struct engine_node *, struct engine_arg *); void en_sb_sync_run(struct engine_node *, void *data); void en_sb_sync_cleanup(void *data); +/* en_address_set_sync engine node functions. */ void *en_address_set_sync_init(struct engine_node *, struct engine_arg *); void en_address_set_sync_run(struct engine_node *, void *data); void en_address_set_sync_cleanup(void *data); +bool address_set_sync_nb_address_set_handler(struct engine_node *, + void *data); +bool address_set_sync_nb_port_group_handler(struct engine_node *, + void *data); #endif diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c index b48f53f17..e2c25046a 100644 --- a/northd/inc-proc-northd.c +++ b/northd/inc-proc-northd.c @@ -238,8 +238,10 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, * the NB database tables. * Right now this engine only syncs the SB Address_Set table. */ - engine_add_input(&en_address_set_sync, &en_nb_address_set, NULL); - engine_add_input(&en_address_set_sync, &en_nb_port_group, NULL); + engine_add_input(&en_address_set_sync, &en_nb_address_set, + address_set_sync_nb_address_set_handler); + engine_add_input(&en_address_set_sync, &en_nb_port_group, + address_set_sync_nb_port_group_handler); engine_add_input(&en_address_set_sync, &en_nb_load_balancer, NULL); engine_add_input(&en_address_set_sync, &en_nb_load_balancer_group, NULL); engine_add_input(&en_address_set_sync, &en_nb_logical_router, NULL); @@ -248,8 +250,12 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, /* We need the en_northd generated data as input to en_address_set_sync * node to access the data generated by it (eg. struct ovn_datapath). + * The handler is noop since en_northd always falls back to full recompute + * (since it has no input handlers) and it doesn't yet indicate what + * changed. It doesn't make sense to add NULL handler for this input, + * otherwise 'en_address_set_sync' will always fall back to full recompute. */ - engine_add_input(&en_address_set_sync, &en_northd, NULL); + engine_add_input(&en_address_set_sync, &en_northd, engine_noop_handler); engine_add_input(&en_sb_sync, &en_address_set_sync, NULL); engine_add_input(&en_northd_output, &en_sb_sync, @@ -300,6 +306,12 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, engine_ovsdb_node_add_index(&en_sb_mac_binding, "sbrec_mac_binding_by_datapath", sbrec_mac_binding_by_datapath); + + struct ovsdb_idl_index *sbrec_address_set_by_name + = ovsdb_idl_index_create1(sb->idl, &sbrec_address_set_col_name); + engine_ovsdb_node_add_index(&en_sb_address_set, + "sbrec_address_set_by_name", + sbrec_address_set_by_name); } void inc_proc_northd_run(struct ovsdb_idl_txn *ovnnb_txn, diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 4f399eccb..f924dcfef 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -7929,3 +7929,55 @@ AT_CHECK([grep "lr_in_arp_resolve" R1flows | grep priority=90 | sort], [0], [dnl AT_CLEANUP ]) + +OVN_FOR_EACH_NORTHD_NO_HV([ +AT_SETUP([Address set incremental processing]) +ovn_start + +foo_as_uuid=$(ovn-nbctl create address_set name=foo addresses=\"1.1.1.1\",\"1.1.1.2\") +ovn-nbctl --wait=sb sync + +check_column '1.1.1.1 1.1.1.2' Address_Set addresses name=foo + +rm -f northd/ovn-northd.log +check as northd ovn-appctl -t NORTHD_TYPE vlog/reopen +check as northd ovn-appctl -t NORTHD_TYPE vlog/set jsonrpc:dbg + +check ovn-nbctl --wait=sb add address_set $foo_as_uuid addresses 1.1.1.3 +check_column '1.1.1.1 1.1.1.2 1.1.1.3' Address_Set addresses name=foo + +AT_CHECK([grep transact northd/ovn-northd.log | grep Address_Set | \ +grep -c mutate], [0], [1 +]) + +check ovn-nbctl --wait=sb add address_set $foo_as_uuid addresses \ +1.1.1.4 -- remove address_set $foo_as_uuid addresses 1.1.1.1 +check_column '1.1.1.2 1.1.1.3 1.1.1.4' Address_Set addresses name=foo + +AT_CHECK([grep transact northd/ovn-northd.log | grep Address_Set | \ +grep -c mutate], [0], [2 +]) + +# Pause ovn-northd and add/remove few addresses. when it is resumed +# it should use mutate for updating the address sets. +check as northd ovn-appctl -t NORTHD_TYPE pause +check as northd-backup ovn-appctl -t NORTHD_TYPE pause + +check ovn-nbctl add address_set $foo_as_uuid addresses 1.1.1.5 +check ovn-nbctl add address_set $foo_as_uuid addresses 1.1.1.6 +check ovn-nbctl remove address_set $foo_as_uuid addresses 1.1.1.2 + +check_column '1.1.1.2 1.1.1.3 1.1.1.4' Address_Set addresses name=foo + +# Resume northd now +check as northd ovn-appctl -t NORTHD_TYPE resume +check ovn-nbctl --wait=sb sync + +check_column '1.1.1.3 1.1.1.4 1.1.1.5 1.1.1.6' Address_Set addresses name=foo + +AT_CHECK([grep transact northd/ovn-northd.log | grep Address_Set | \ +grep -c mutate], [0], [3 +]) + +AT_CLEANUP +])