On 7/15/26 10:42 AM, Sragdhara Datta Chaudhuri wrote:
> Advertise address_set from one AZ to others via ovn-ic.
> The address sets from SB are advertised to the IC-SB Address_Set.
> The address sets from IC SB that belong to remote AZ are synced
> to the local NB Address_Set.
>
> A new table Address_Set has been added to IC-SB.
> "Address_Set": {
> "columns": {
> "name": {"type": "string"},
> "addresses": {"type": {"key": "string",
> "min": 0,
> "max": "unlimited"}},
> "availability_zone": {"type": {"key": {"type": "uuid",
> "refTable": "Availability_Zone"}}},
> "external_ids": {
> "type": {"key": "string", "value": "string",
> "min": 0, "max": "unlimited"}}},
> "indexes": [["name"]],
> "isRoot": true}
> }
>
> New "options" field has been added to NB Address_Set table.
> ic-adv - A boolean value that enables advertisement of this
> Address_Set to IC-SB. Default is false.
> New "options" field has been added to SB Address_Set table.
> The options from NB Address_Set gets synced to the SB entry.
>
> Two new nb_global options have been added:
> ic-as-adv - If this is set to true, the Address_Sets in this AZ that have
> option ic-adv set to true get advertised to the IC-SB.
> Default is false.
> ic-as-learn - If this is set to true, the Address_Sets in IC-SB get
> synced to the NB Address_Set table of this AZ. Default
> value of the option is false. The synced address_sets
> would have external-id "ic-learnt" set to true.
>
> The existing northd code that syncs Address_Set from NB to SB has been
> enhanced to sync "options".
>
> New logic has been added in ovn-ic to sync address sets with IC-SB.
> Advertise logic (from SB to IC-SB):
> - Each SB address set that needs to be advertised (ic-adv option set),
> check if it is already present in IC-SB. If not, create new entry in
> IC-SB. Otherwise sync addresses from local address set to IC-SB entry.
> - Delete extra address sets in IC-SB that were earlier learnt from this
> AZ, but is no longer present, or not enabled for advertisement.
>
> Learning logic (from IC-SB to NB):
> - For each NB Address set entries that were earlier learnt from IC-SB
> (external-id "ic-learnt" set to true), check if it is still present
> in IC-SB. If not, delete local entry in NB. If yes, sync addresses
> from IC-SB to NB.
> - Any remote address sets in IC-SB (AZ not same as local AZ) that is not
> present in local AZ, create local entry in NB with external-id
> "ic-learnt" set to true.
>
> Signed-off-by: Sragdhara Datta Chaudhuri <[email protected]>
> Acked-by: Priyankar Jain <[email protected]>
> ---
>
> Version 2:
> * Removed sync_address_set_options. Instead passing options to
> update_sb_addr_set.
> * Added omit alert for the options field.
> * Added AZ to the index to handle clash of address-set names across AZs.
> * Added "wait" in unit tests to avoid race conditions.
>
Hi Sragdhara,
Thanks for the new version!
I have a few more comments. I guess the address_set [name + AZ]
uniqueness is the most important one.
> NEWS | 4 +
> ic/inc-proc-ic.c | 12 ++-
> ic/ovn-ic.c | 185 ++++++++++++++++++++++++++++++++++++++++++++
> northd/en-sync-sb.c | 39 ++++++----
> northd/ovn-northd.c | 6 +-
> ovn-ic-sb.ovsschema | 15 +++-
> ovn-ic-sb.xml | 29 +++++++
> ovn-nb.ovsschema | 9 ++-
> ovn-nb.xml | 29 +++++++
> ovn-sb.ovsschema | 9 ++-
> ovn-sb.xml | 4 +
> tests/ovn-ic.at | 172 ++++++++++++++++++++++++++++++++++++++++
> 12 files changed, 489 insertions(+), 24 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 384e30820..801cb69fd 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,5 +1,9 @@
> Post v26.03.0
> -------------
> + - OVN Interconnection now supports advertising Address_Sets to remote
> + availability zones. Set options:ic-adv=true on an NB Address_Set to
> + have ovn-ic publish it to the IC-SB database and import it into the
> + Northbound databases of other availability zones.
> - Load balancer health checks now "fail closed" for newly added backends:
> ovn-northd initializes a newly created Service_Monitor row to the
> "offline" state, so traffic is not forwarded to a backend until its
> first
> diff --git a/ic/inc-proc-ic.c b/ic/inc-proc-ic.c
> index 2c0420292..bbcbcdd17 100644
> --- a/ic/inc-proc-ic.c
> +++ b/ic/inc-proc-ic.c
> @@ -41,7 +41,8 @@ VLOG_DEFINE_THIS_MODULE(inc_proc_ic);
> NB_NODE(logical_switch, "logical_switch") \
> NB_NODE(logical_switch_port, "logical_switch_port") \
> NB_NODE(load_balancer, "load_balancer") \
> - NB_NODE(load_balancer_group, "load_balancer_group")
> + NB_NODE(load_balancer_group, "load_balancer_group") \
> + NB_NODE(address_set, "address_set")
>
> enum nb_engine_node {
> #define NB_NODE(NAME, NAME_STR) NB_##NAME,
> @@ -66,7 +67,8 @@ VLOG_DEFINE_THIS_MODULE(inc_proc_ic);
> SB_NODE(datapath_binding, "datapath_binding") \
> SB_NODE(port_binding, "port_binding") \
> SB_NODE(service_monitor, "service_monitor") \
> - SB_NODE(learned_route, "learned_route")
> + SB_NODE(learned_route, "learned_route") \
> + SB_NODE(address_set, "address_set")
>
> enum sb_engine_node {
> #define SB_NODE(NAME, NAME_STR) SB_##NAME,
> @@ -114,7 +116,8 @@ VLOG_DEFINE_THIS_MODULE(inc_proc_ic);
> ICSB_NODE(datapath_binding, "datapath_binding") \
> ICSB_NODE(encap, "encap") \
> ICSB_NODE(gateway, "gateway") \
> - ICSB_NODE(port_binding, "port_binding")
> + ICSB_NODE(port_binding, "port_binding") \
> + ICSB_NODE(address_set, "address_set")
>
> enum icsb_engine_node {
> #define ICSB_NODE(NAME, NAME_STR) ICSB_##NAME,
> @@ -176,6 +179,7 @@ void inc_proc_ic_init(struct ovsdb_idl_loop *nb,
> engine_add_input(&en_ic, &en_nb_logical_switch_port, NULL);
> engine_add_input(&en_ic, &en_nb_load_balancer, NULL);
> engine_add_input(&en_ic, &en_nb_load_balancer_group, NULL);
> + engine_add_input(&en_ic, &en_nb_address_set, NULL);
>
> engine_add_input(&en_ic, &en_sb_sb_global, NULL);
> engine_add_input(&en_ic, &en_sb_chassis, NULL);
> @@ -184,6 +188,7 @@ void inc_proc_ic_init(struct ovsdb_idl_loop *nb,
> engine_add_input(&en_ic, &en_sb_port_binding, NULL);
> engine_add_input(&en_ic, &en_sb_service_monitor, NULL);
> engine_add_input(&en_ic, &en_sb_learned_route, NULL);
> + engine_add_input(&en_ic, &en_sb_address_set, NULL);
>
> engine_add_input(&en_ic, &en_icnb_ic_nb_global, NULL);
> engine_add_input(&en_ic, &en_icnb_transit_switch, NULL);
> @@ -198,6 +203,7 @@ void inc_proc_ic_init(struct ovsdb_idl_loop *nb,
> engine_add_input(&en_ic, &en_icsb_gateway, NULL);
> engine_add_input(&en_ic, &en_icsb_route, NULL);
> engine_add_input(&en_ic, &en_icsb_datapath_binding, NULL);
> + engine_add_input(&en_ic, &en_icsb_address_set, NULL);
>
> struct engine_arg engine_arg = {
> .nb_idl = nb->idl,
> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
> index f07e74866..c608726ac 100644
> --- a/ic/ovn-ic.c
> +++ b/ic/ovn-ic.c
> @@ -533,6 +533,170 @@ sync_sb_gw_to_isb(struct ic_context *ctx,
> free(isb_encaps);
> }
>
> +static void
> +nb_addr_set_apply_diff(const void *arg, const char *item, bool add)
> +{
> + const struct nbrec_address_set *as = arg;
> + if (add) {
> + nbrec_address_set_update_addresses_addvalue(as, item);
> + } else {
> + nbrec_address_set_update_addresses_delvalue(as, item);
> + }
> +}
> +
> +static void
> +update_nb_addr_set(const struct icsbrec_address_set *icsb_as,
> + const struct nbrec_address_set *nb_as)
> +{
> + struct sorted_array nb_addrs =
> + sorted_array_from_dbrec(nb_as, addresses);
> + struct sorted_array icsb_addrs =
> + sorted_array_from_dbrec(icsb_as, addresses);
> + sorted_array_apply_diff(&icsb_addrs, &nb_addrs,
> + nb_addr_set_apply_diff, nb_as);
> + sorted_array_destroy(&icsb_addrs);
> + sorted_array_destroy(&nb_addrs);
> +}
> +
> +static void
> +icsb_addr_set_apply_diff(const void *arg, const char *item, bool add)
> +{
> + const struct icsbrec_address_set *as = arg;
> + if (add) {
> + icsbrec_address_set_update_addresses_addvalue(as, item);
> + } else {
> + icsbrec_address_set_update_addresses_delvalue(as, item);
> + }
> +}
> +
> +static void
> +update_icsb_addr_set(struct sorted_array *nb_addrs,
> + const struct icsbrec_address_set *icsb_as)
> +{
> + struct sorted_array icsb_addrs =
> + sorted_array_from_dbrec(icsb_as, addresses);
> + sorted_array_apply_diff(nb_addrs, &icsb_addrs,
> + icsb_addr_set_apply_diff, icsb_as);
> + sorted_array_destroy(&icsb_addrs);
> +}
> +
> +static void
> +sync_addr_set_to_icsb(struct ovsdb_idl_txn *ovnisb_txn,
> + const struct sbrec_address_set *sb_as,
> + const struct icsbrec_address_set *icsb_as,
> + const struct icsbrec_availability_zone *az)
> +{
> + struct sorted_array addrs =
> + sorted_array_from_dbrec(sb_as, addresses);
> + if (!icsb_as) {
> + icsb_as = icsbrec_address_set_insert(ovnisb_txn);
> + icsbrec_address_set_set_name(icsb_as, sb_as->name);
> + icsbrec_address_set_set_availability_zone(icsb_as, az);
> + icsbrec_address_set_set_addresses(icsb_as, addrs.arr, addrs.n);
> + } else {
> + update_icsb_addr_set(&addrs, icsb_as);
> + }
> + sorted_array_destroy(&addrs);
> +}
> +
> +static void
> +sync_addr_set_from_icsb(struct ovsdb_idl_txn *ovnnb_txn,
> + const struct icsbrec_address_set *icsb_as)
Nit: indentation.
> +{
> + struct nbrec_address_set *nb_as;
> + struct sorted_array addrs =
> + sorted_array_from_dbrec(icsb_as, addresses);
> +
> + nb_as = nbrec_address_set_insert(ovnnb_txn);
> + nbrec_address_set_set_name(nb_as, icsb_as->name);
> + nbrec_address_set_update_external_ids_setkey(nb_as, "ic-learnt", "true");
> + nbrec_address_set_set_addresses(nb_as, addrs.arr, addrs.n);
> + sorted_array_destroy(&addrs);
> +}
> +
> +static void
> +address_set_run(struct ic_context *ctx)
> +{
> + if (!ctx->ovnisb_unlocked_txn || !ctx->ovnnb_txn || !ctx->ovnsb_txn) {
> + return;
> + }
> +
> + struct shash ic_local_as = SHASH_INITIALIZER(&ic_local_as);
> + struct shash ic_remote_as = SHASH_INITIALIZER(&ic_remote_as);
> + const struct icsbrec_address_set *ic_as;
> + ICSBREC_ADDRESS_SET_FOR_EACH (ic_as, ctx->ovnisb_unlocked_idl) {
> + if (ic_as->availability_zone == ctx->runned_az) {
> + shash_add(&ic_local_as, ic_as->name, ic_as);
> + } else {
> + shash_add(&ic_remote_as, ic_as->name, ic_as);
Now the IC SB address_set schema enforces the [name + az] unique index.
But that also means we could have two address sets with different the
same name in different remote AZs.
If that happens the second shash_add() will insert a duplicate.
> + }
> + }
> +
> + const struct nbrec_nb_global *nb_global =
> + nbrec_nb_global_first(ctx->ovnnb_idl);
> + ovs_assert(nb_global);
> + bool global_learn = smap_get_bool(&nb_global->options, "ic-as-learn",
> + false);
> + bool global_adv = smap_get_bool(&nb_global->options, "ic-as-adv", false);
> +
> + /* Advertise address set - from SB to IC-SB:
> + * - Each SB address set that needs to be advertised (ic-adv option set),
> + * check if it is already present in IC-SB. If not, create new entry in
> + * IC-SB. Otherwise sync addresses from local address set to IC-SB
> entry.
> + * - Delete extra address sets in IC-SB that were earlier learnt from
> this
> + * AZ, but is no longer present, or not enabled for advertisement.
Nit: the lines after '-' could be indented one more space to the right.
> + */
> + if (global_adv) {
> + const struct sbrec_address_set *sb_as;
> + SBREC_ADDRESS_SET_FOR_EACH (sb_as, ctx->ovnsb_idl) {
> + if (smap_get_bool(&sb_as->options, "ic-adv", false)) {
> + const struct icsbrec_address_set *icsb_as;
> + icsb_as = shash_find_and_delete(&ic_local_as, sb_as->name);
> + sync_addr_set_to_icsb(ctx->ovnisb_unlocked_txn, sb_as,
> icsb_as,
> + ctx->runned_az);
> + }
> + }
> + }
> + struct shash_node *node;
> + SHASH_FOR_EACH (node, &ic_local_as) {
> + icsbrec_address_set_delete(node->data);
> + }
> + shash_destroy(&ic_local_as);
> +
> + /* Learn address set - from IC-SB to NB:
> + * - For each NB Address set entries that were earlier learnt from IC-SB
> + * (external-id "ic-learnt" set to true), check if it is still present
> + * in IC-SB. If not, delete local entry in NB. If yes, sync addresses
> + * from IC-SB to NB.
> + * - Any remote address sets in IC-SB (AZ not same as local AZ) that is
> not
> + * present in local AZ, create local entry in NB with external-id
> + * "ic-learnt" set to true.
> + */
> + const struct nbrec_address_set *nb_as;
> + NBREC_ADDRESS_SET_FOR_EACH_SAFE (nb_as, ctx->ovnnb_idl) {
> + const struct icsbrec_address_set *icsb_as;
> + icsb_as = shash_find_and_delete(&ic_remote_as, nb_as->name);
Here we'd remove the first of the duplicates.
> + if (smap_get_bool(&nb_as->external_ids, "ic-learnt", false)) {
> + if (!icsb_as || !global_learn) {
> + nbrec_address_set_delete(nb_as);
> + } else {
> + update_nb_addr_set(icsb_as, nb_as);
> + }
> + }
> + }
> +
> + if (global_learn) {
> + SHASH_FOR_EACH (node, &ic_remote_as) {
> + /* In case local address-set with same name exists, we
> + * will not overwrite it because such address sets are already
> + * removed from ic_remote_as in the loop above.
> + */
Here we would find the other duplicate AS. And we'll try to insert
trigerring a constraint violation because an address set with the same
name already exists in this AZ's NB database.
I think in the beginning of this function, when populating ic_remote_as,
we should handle duplicates and merge them together into a single
aggregated address set.
What do you think?
> + sync_addr_set_from_icsb(ctx->ovnnb_txn, node->data);
> + }
> + }
> + shash_destroy(&ic_remote_as);
> +}
> +
> static void
> gateway_run(struct ic_context *ctx)
> {
> @@ -3500,6 +3664,7 @@ ovn_db_run(struct ic_context *ctx)
> port_binding_run(ctx);
> route_run(ctx);
> sync_service_monitor(ctx);
> + address_set_run(ctx);
>
> ovn_destroy_tnlids(&dp_tnlids);
> shash_destroy(&isb_ts_dps);
> @@ -3838,6 +4003,17 @@ main(int argc, char *argv[])
> ovsdb_idl_track_add_column(ovnnb_idl_loop.idl,
> &nbrec_load_balancer_group_col_load_balancer);
>
> + ovsdb_idl_add_table(ovnnb_idl_loop.idl, &nbrec_table_address_set);
> + ovsdb_idl_track_add_column(ovnnb_idl_loop.idl,
> + &nbrec_address_set_col_name);
> + ovsdb_idl_track_add_column(ovnnb_idl_loop.idl,
> + &nbrec_address_set_col_addresses);
> + ovsdb_idl_track_add_column(ovnnb_idl_loop.idl,
> + &nbrec_address_set_col_options);
> + ovsdb_idl_track_add_column(ovnnb_idl_loop.idl,
> + &nbrec_address_set_col_external_ids);
> +
> +
Nit: one empty line too many.
> /* ovn-sb db. */
> struct ovsdb_idl_loop ovnsb_idl_loop = OVSDB_IDL_LOOP_INITIALIZER(
> ovsdb_idl_create(ovnsb_db, &sbrec_idl_class, false, true));
> @@ -3923,6 +4099,15 @@ main(int argc, char *argv[])
> &sbrec_learned_route_col_ip_prefix);
> ovsdb_idl_track_add_column(ovnsb_idl_loop.idl,
> &sbrec_learned_route_col_datapath);
> +
> + ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_address_set);
> + ovsdb_idl_track_add_column(ovnsb_idl_loop.idl,
> + &sbrec_address_set_col_name);
> + ovsdb_idl_track_add_column(ovnsb_idl_loop.idl,
> + &sbrec_address_set_col_addresses);
> + ovsdb_idl_track_add_column(ovnsb_idl_loop.idl,
> + &sbrec_address_set_col_options);
> +
> /* Create IDL indexes */
> struct ovsdb_idl_index *nbrec_ls_by_name
> = ovsdb_idl_index_create1(ovnnb_idl_loop.idl,
> diff --git a/northd/en-sync-sb.c b/northd/en-sync-sb.c
> index c00e91764..db9cf5cf3 100644
> --- a/northd/en-sync-sb.c
> +++ b/northd/en-sync-sb.c
> @@ -43,7 +43,8 @@ VLOG_DEFINE_THIS_MODULE(en_sync_to_sb);
>
> static void sync_addr_set(struct ovsdb_idl_txn *ovnsb_txn, const char *name,
> struct sorted_array *addresses,
> - struct shash *sb_address_sets);
> + struct shash *sb_address_sets,
> + const struct smap *options);
> static void sync_addr_sets(struct ovsdb_idl_txn *ovnsb_txn,
> const struct nbrec_address_set_table *,
> const struct nbrec_port_group_table *,
> @@ -55,7 +56,8 @@ static void sync_addr_sets(struct ovsdb_idl_txn *ovnsb_txn,
> 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(struct sorted_array *,
> - const struct sbrec_address_set *);
> + const struct sbrec_address_set *,
> + const struct smap *options);
> static void build_port_group_address_set(const struct nbrec_port_group *,
> struct svec *ipv4_addrs,
> struct svec *ipv6_addrs);
> @@ -151,7 +153,7 @@ sync_to_sb_addr_set_nb_address_set_handler(struct
> engine_node *node,
> }
> struct sorted_array addrs =
> sorted_array_from_dbrec(nb_addr_set, addresses);
> - update_sb_addr_set(&addrs, sb_addr_set);
> + update_sb_addr_set(&addrs, sb_addr_set, &nb_addr_set->options);
> sorted_array_destroy(&addrs);
> }
>
> @@ -204,8 +206,8 @@ sync_to_sb_addr_set_nb_port_group_handler(struct
> engine_node *node,
> struct sorted_array ipv6_addrs_sorted =
> sorted_array_from_svec(&ipv6_addrs);
>
> - update_sb_addr_set(&ipv4_addrs_sorted, sb_addr_set_v4);
> - update_sb_addr_set(&ipv6_addrs_sorted, sb_addr_set_v6);
> + update_sb_addr_set(&ipv4_addrs_sorted, sb_addr_set_v4, NULL);
> + update_sb_addr_set(&ipv6_addrs_sorted, sb_addr_set_v6, NULL);
>
> sorted_array_destroy(&ipv4_addrs_sorted);
> sorted_array_destroy(&ipv6_addrs_sorted);
> @@ -440,7 +442,8 @@ sync_to_sb_pb_lr_stateful_handler(struct engine_node
> *node,
> static void
> sync_addr_set(struct ovsdb_idl_txn *ovnsb_txn, const char *name,
> struct sorted_array *addresses,
> - struct shash *sb_address_sets)
> + struct shash *sb_address_sets,
> + const struct smap *options)
> {
> const struct sbrec_address_set *sb_address_set;
> sb_address_set = shash_find_and_delete(sb_address_sets,
> @@ -450,8 +453,11 @@ sync_addr_set(struct ovsdb_idl_txn *ovnsb_txn, const
> char *name,
> sbrec_address_set_set_name(sb_address_set, name);
> sbrec_address_set_set_addresses(sb_address_set, addresses->arr,
> addresses->n);
> + if (options) {
> + sbrec_address_set_set_options(sb_address_set, options);
> + }
> } else {
> - update_sb_addr_set(addresses, sb_address_set);
> + update_sb_addr_set(addresses, sb_address_set, options);
> }
> }
>
> @@ -491,7 +497,7 @@ sync_addr_sets(struct ovsdb_idl_txn *ovnsb_txn,
> const char *svc_macs[] = {svc_monitor_macp, svc_monitor_macp_dst};
> struct sorted_array svc =
> sorted_array_from_unsorted(svc_macs, ARRAY_SIZE(svc_macs), false);
> - sync_addr_set(ovnsb_txn, "svc_monitor_mac", &svc, &sb_address_sets);
> + sync_addr_set(ovnsb_txn, "svc_monitor_mac", &svc, &sb_address_sets,
> NULL);
> sorted_array_destroy(&svc);
>
> /* sync port group generated address sets first */
> @@ -510,9 +516,9 @@ sync_addr_sets(struct ovsdb_idl_txn *ovnsb_txn,
> sorted_array_from_svec(&ipv6_addrs);
>
> sync_addr_set(ovnsb_txn, ipv4_addrs_name,
> - &ipv4_addrs_sorted, &sb_address_sets);
> + &ipv4_addrs_sorted, &sb_address_sets, NULL);
> sync_addr_set(ovnsb_txn, ipv6_addrs_name,
> - &ipv6_addrs_sorted, &sb_address_sets);
> + &ipv6_addrs_sorted, &sb_address_sets, NULL);
> sorted_array_destroy(&ipv4_addrs_sorted);
> sorted_array_destroy(&ipv6_addrs_sorted);
> svec_destroy(&ipv4_addrs);
> @@ -535,7 +541,7 @@ sync_addr_sets(struct ovsdb_idl_txn *ovnsb_txn,
> &lr_stateful_rec->lb_ips->ips_v4_reachable);
>
> sync_addr_set(ovnsb_txn, ipv4_addrs_name,
> - &ipv4_addrs_sorted, &sb_address_sets);
> + &ipv4_addrs_sorted, &sb_address_sets, NULL);
> sorted_array_destroy(&ipv4_addrs_sorted);
> free(ipv4_addrs_name);
> }
> @@ -547,7 +553,7 @@ sync_addr_sets(struct ovsdb_idl_txn *ovnsb_txn,
> &lr_stateful_rec->lb_ips->ips_v6_reachable);
>
> sync_addr_set(ovnsb_txn, ipv6_addrs_name,
> - &ipv6_addrs_sorted, &sb_address_sets);
> + &ipv6_addrs_sorted, &sb_address_sets, NULL);
> sorted_array_destroy(&ipv6_addrs_sorted);
> free(ipv6_addrs_name);
> }
> @@ -561,7 +567,8 @@ sync_addr_sets(struct ovsdb_idl_txn *ovnsb_txn,
> struct sorted_array addrs =
> sorted_array_from_dbrec(nb_address_set, addresses);
> sync_addr_set(ovnsb_txn, nb_address_set->name,
> - &addrs, &sb_address_sets);
> + &addrs, &sb_address_sets,
> + &nb_address_set->options);
> sorted_array_destroy(&addrs);
> }
>
> @@ -586,13 +593,17 @@ sb_addr_set_apply_diff(const void *arg, const char
> *item, bool add)
>
> static void
> update_sb_addr_set(struct sorted_array *nb_addresses,
> - const struct sbrec_address_set *sb_as)
> + const struct sbrec_address_set *sb_as,
> + const struct smap *options)
> {
> struct sorted_array sb_addresses =
> sorted_array_from_dbrec(sb_as, addresses);
> sorted_array_apply_diff(nb_addresses, &sb_addresses,
> sb_addr_set_apply_diff, sb_as);
> sorted_array_destroy(&sb_addresses);
> + if (options) {
> + sbrec_address_set_set_options(sb_as, options);
> + }
> }
>
> static void
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index c4c71ca81..499cf9edd 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -946,8 +946,10 @@ main(int argc, char *argv[])
> ovsdb_idl_omit_alert(ovnsb_idl_loop.idl, &sbrec_sb_global_col_nb_cfg);
> ovsdb_idl_omit_alert(ovnsb_idl_loop.idl,
> &sbrec_sb_global_col_nb_cfg_timestamp);
> - ovsdb_idl_omit_alert(ovnsb_idl_loop.idl, &sbrec_address_set_col_name);
> - ovsdb_idl_omit_alert(ovnsb_idl_loop.idl,
> &sbrec_address_set_col_addresses);
> + for (size_t i = 0; i < SBREC_ADDRESS_SET_N_COLUMNS; i++) {
> + ovsdb_idl_omit_alert(ovnsb_idl_loop.idl,
> + &sbrec_address_set_columns[i]);
> + }
> for (size_t i = 0; i < SBREC_LOGICAL_FLOW_N_COLUMNS; i++) {
> ovsdb_idl_omit_alert(ovnsb_idl_loop.idl,
> &sbrec_logical_flow_columns[i]);
> diff --git a/ovn-ic-sb.ovsschema b/ovn-ic-sb.ovsschema
> index 9208e7ab5..b11dcfe1f 100644
> --- a/ovn-ic-sb.ovsschema
> +++ b/ovn-ic-sb.ovsschema
> @@ -1,7 +1,7 @@
> {
> "name": "OVN_IC_Southbound",
> "version": "2.6.0",
This should be bumped to 2.7.0 because we're adding a new table.
> - "cksum": "2842701319 9868",
> + "cksum": "2916493883 10519",
> "tables": {
> "IC_SB_Global": {
> "columns": {
> @@ -195,6 +195,19 @@
> "external_ids": {
> "type": {"key": "string", "value": "string",
> "min": 0, "max": "unlimited"}}},
> + "isRoot": true},
> + "Address_Set": {
> + "columns": {
> + "name": {"type": "string"},
> + "addresses": {"type": {"key": "string",
> + "min": 0,
> + "max": "unlimited"}},
> + "availability_zone": {"type": {"key": {"type": "uuid",
> + "refTable": "Availability_Zone"}}},
> + "external_ids": {
> + "type": {"key": "string", "value": "string",
> + "min": 0, "max": "unlimited"}}},
> + "indexes": [["name", "availability_zone"]],
> "isRoot": true}
> }
> }
> diff --git a/ovn-ic-sb.xml b/ovn-ic-sb.xml
> index f0d450681..6e306eb06 100644
> --- a/ovn-ic-sb.xml
> +++ b/ovn-ic-sb.xml
> @@ -802,4 +802,33 @@
> Copy from source SBDB record.
> </column>
> </table>
> +
> + <table name="Address_Set" title="Address Sets">
> + <p>
> + Each row in this table represents an Address_Set advertised.
Maybe rephrase to "an advertised Address_Set"?
> + The combination of <ref column="name"/> and
> + <ref column="availability_zone"/> is unique: each availability zone
> + may advertise at most one address set with a given name.
Maybe we should document the expected behavior if the advertised address
set exist with the same name in different AZs.
> + </p>
> +
> + <column name="name">
> + A name for the address set. Names are ASCII and must match
> + <code>[a-zA-Z_.][a-zA-Z_.0-9]*</code>.
> + </column>
> +
> + <column name="addresses">
> + The set of addresses in string form.
> + </column>
> +
> + <column name="availability_zone">
> + The availability zone that has advertised the Address_Set.
> + </column>
> +
> + <group title="Common Columns">
> + <column name="external_ids">
> + See <em>External IDs</em> at the beginning of this document.
> + </column>
> + </group>
> + </table>
> +
> </database>
> diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
> index e5945b831..00909fff4 100644
> --- a/ovn-nb.ovsschema
> +++ b/ovn-nb.ovsschema
> @@ -1,7 +1,7 @@
> {
> "name": "OVN_Northbound",
> - "version": "7.18.0",
> - "cksum": "1537030958 45190",
> + "version": "7.18.1",
> + "cksum": "2442107800 45407",
> "tables": {
> "NB_Global": {
> "columns": {
> @@ -292,6 +292,11 @@
> "addresses": {"type": {"key": "string",
> "min": 0,
> "max": "unlimited"}},
> + "options": {
> + "type": {"key": "string",
> + "value": "string",
> + "min": 0,
> + "max": "unlimited"}},
> "external_ids": {
> "type": {"key": "string", "value": "string",
> "min": 0, "max": "unlimited"}}},
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index 33a6dc676..05fd75f66 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -583,6 +583,25 @@
> </column>
> </group>
>
> + <group title="Options for interconnection address_set advertisement">
> + <p>
> + These options control whether address_sets propagate from one OVN
> + deployment to another via the global <ref db="OVN_IC_Southbound"/>.
> + </p>
> +
> + <column name="options" key="ic-as-adv">
> + A boolean value that enables address_set advertisement to the
> global
> + <ref db="OVN_IC_Southbound"/> database. Default is
> + <code>false</code>.
> + </column>
> +
> + <column name="options" key="ic-as-learn">
> + A boolean value that enables address_set learning from the global
> + <ref db="OVN_IC_Southbound"/> database. Default is
> + <code>false</code>.
> + </column>
> + </group>
> +
> </group>
>
> <group title="Connection Options">
> @@ -2424,6 +2443,16 @@
> See <em>External IDs</em> at the beginning of this document.
> </column>
> </group>
> +
> + <column name="options">
> + This column provides general key/value settings. The supported
> + options are described individually below.
> + </column>
> +
> + <column name="options" key="ic-adv">
> + A boolean value that enables advertisement of this Address_Set to
> + the global OVN_IC_Southbound database. Default is <code>false</code>.
> + </column>
> </table>
>
> <table name="Port_Group" title="Port Groups">
> diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
> index 3d79a64b4..9c43bb715 100644
> --- a/ovn-sb.ovsschema
> +++ b/ovn-sb.ovsschema
> @@ -1,7 +1,7 @@
> {
> "name": "OVN_Southbound",
> "version": "21.10.0",
This should be bumped to 21.11.0 because we're adding a new column.
> - "cksum": "2298996353 36780",
> + "cksum": "3354272172 36997",
> "tables": {
> "SB_Global": {
> "columns": {
> @@ -78,7 +78,12 @@
> "name": {"type": "string"},
> "addresses": {"type": {"key": "string",
> "min": 0,
> - "max": "unlimited"}}},
> + "max": "unlimited"}},
> + "options": {
> + "type": {"key": "string",
> + "value": "string",
> + "min": 0,
> + "max": "unlimited"}}},
> "indexes": [["name"]],
> "isRoot": true},
> "Port_Group": {
> diff --git a/ovn-sb.xml b/ovn-sb.xml
> index b54c25c91..f8f26bd3e 100644
> --- a/ovn-sb.xml
> +++ b/ovn-sb.xml
> @@ -592,6 +592,10 @@
>
> <column name="name"/>
> <column name="addresses"/>
> + <group title="Common Columns">
> + <column name="options">
> + </column>
> + </group>
> </table>
>
> <table name="Port_Group" title="Port Groups">
> diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
> index f9fceac8d..e54daa1c5 100644
> --- a/tests/ovn-ic.at
> +++ b/tests/ovn-ic.at
> @@ -922,6 +922,178 @@ OVN_CLEANUP_IC([az1], [az2])
> AT_CLEANUP
> ])
>
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([ovn-ic -- address-set sync])
> +
> +ovn_init_ic_db
> +ovn-ic-nbctl ts-add ts1
> +
> +for i in 1 2; do
> + ovn_start az$i
> + ovn_as az$i
> + check ovn-ic-nbctl --wait=sb sync
> + # Enable address-set learning at AZ level
> + check ovn-nbctl set nb_global . options:ic-as-learn=true
> + # Enable address-set advertising at AZ level
> + check ovn-nbctl set nb_global . options:ic-as-adv=true
> +
> + # Create address-set to advertise
> + check_uuid ovn-nbctl create Address_Set name=az$i-as1
> addresses=\"$i.1.1.1\",\"$i.1.1.2\" options:ic-adv=true
> +done
> +
> +wait_row_count ic-sb:Address_Set 2
> +OVS_WAIT_FOR_OUTPUT([ovn-ic-sbctl --columns=addresses find Address_Set
> name=az1-as1], [0], [dnl
> +addresses : [["1.1.1.1", "1.1.1.2"]]
> +])
> +
> +OVS_WAIT_FOR_OUTPUT([ovn-ic-sbctl --columns=addresses find Address_Set
> name=az2-as1], [0], [dnl
> +addresses : [["2.1.1.1", "2.1.1.2"]]
> +])
> +
> +for i in 1 2; do
> + OVS_WAIT_UNTIL([ovn_as az$i ovn-nbctl list Address_Set | grep learnt])
> +done
> +
> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl --columns=addresses,external_ids
> list Address_Set az2-as1], [0], [dnl
> +addresses : [["2.1.1.1", "2.1.1.2"]]
> +external_ids : {ic-learnt="true"}
> +])
> +
> +OVS_WAIT_FOR_OUTPUT([ovn_as az2 ovn-nbctl --columns=addresses,external_ids
> list Address_Set az1-as1], [0], [dnl
> +addresses : [["1.1.1.1", "1.1.1.2"]]
> +external_ids : {ic-learnt="true"}
> +])
> +
> +# Update AZ1 address_set (add 2 new IPs and delete one) and check it is
> synced to IC and AZ2.
> +ovn_as az1 ovn-nbctl set Address_Set az1-as1
> addresses=\"1.1.1.1\",\"1.1.1.3\",\"1.1.1.4\"
> +OVS_WAIT_FOR_OUTPUT([ovn-ic-sbctl --columns=addresses find Address_Set
> name=az1-as1], [0], [dnl
> +addresses : [["1.1.1.1", "1.1.1.3", "1.1.1.4"]]
> +])
> +OVS_WAIT_FOR_OUTPUT([ovn_as az2 ovn-nbctl --columns=addresses,external_ids
> list Address_Set az1-as1], [0], [dnl
> +addresses : [["1.1.1.1", "1.1.1.3", "1.1.1.4"]]
> +external_ids : {ic-learnt="true"}
> +])
> +
> +# Add 2 new address-sets in AZ1 - one to advertise, another not to.
> +ovn_as az1 ovn-nbctl create Address_Set name=az1-as2
> addresses=\"1.1.2.1\",\"1.1.2.2\" options:ic-adv=true
> +ovn_as az1 ovn-nbctl create Address_Set name=az1-as3
> addresses=\"1.1.3.1\",\"1.1.3.2\"
> +wait_row_count ic-sb:Address_Set 3
> +OVS_WAIT_FOR_OUTPUT([ovn-ic-sbctl --columns=addresses find Address_Set
> name=az1-as2], [0], [dnl
> +addresses : [["1.1.2.1", "1.1.2.2"]]
> +])
> +AT_CHECK([ovn-ic-sbctl --columns=addresses find Address_Set name=az1-as3],
> [0], [dnl
> +])
> +OVS_WAIT_FOR_OUTPUT([ovn_as az2 ovn-nbctl --columns=addresses,external_ids
> list Address_Set az1-as2], [0], [dnl
> +addresses : [["1.1.2.1", "1.1.2.2"]]
> +external_ids : {ic-learnt="true"}
> +])
> +AT_CHECK([ovn_as az2 ovn-nbctl --columns=addresses,external_ids find
> Address_Set name=az1-as3], [0], [dnl
> +])
> +
> +# Now set the option field of the above address-set to advertise it.
> +ovn_as az1 ovn-nbctl set Address_Set az1-as3 options={ic-adv=true}
> +wait_row_count ic-sb:Address_Set 4
> +OVS_WAIT_FOR_OUTPUT([ovn-ic-sbctl --columns=addresses find Address_Set
> name=az1-as3], [0], [dnl
> +addresses : [["1.1.3.1", "1.1.3.2"]]
> +])
> +OVS_WAIT_FOR_OUTPUT([ovn_as az2 ovn-nbctl --columns=addresses,external_ids
> list Address_Set az1-as3], [0], [dnl
> +addresses : [["1.1.3.1", "1.1.3.2"]]
> +external_ids : {ic-learnt="true"}
> +])
> +
> +# Change the options of one of the address-sets to not advertise.
> +ovn_as az1 ovn-nbctl set Address_Set az1-as2 options={ic-adv=false}
> +wait_row_count ic-sb:Address_Set 3
> +AT_CHECK([ovn-ic-sbctl --columns=addresses find Address_Set name=az1-as2],
> [0], [dnl
> +])
> +OVS_WAIT_FOR_OUTPUT([ovn_as az2 ovn-nbctl --columns=addresses,external_ids
> find Address_Set name=az1-as2], [0], [dnl
> +])
> +
> +# Delete one of the advertised address-sets in AZ1.
> +ovn_as az1 ovn-nbctl destroy Address_Set az1-as3
> +wait_row_count ic-sb:Address_Set 2
> +AT_CHECK([ovn-ic-sbctl --columns=addresses find Address_Set name=az1-as3],
> [0], [dnl
> +])
> +OVS_WAIT_FOR_OUTPUT([ovn_as az2 ovn-nbctl --columns=addresses,external_ids
> find Address_Set name=az1-as3], [0], [dnl
> +])
> +
> +# Add an IPv6 address-set in AZ2.
> +ovn_as az2 ovn-nbctl create Address_Set name=az2-as2
> addresses=\"2001:0db8:85a3:0000:0000:8a2e:0370:7334\",\"2001:0db8:85a3:0000:0000:8a2e:0370:7335\"
> options:ic-adv=true
> +wait_row_count ic-sb:Address_Set 3
> +OVS_WAIT_FOR_OUTPUT([ovn-ic-sbctl --columns=addresses find Address_Set
> name=az2-as2], [0], [dnl
> +addresses : [["2001:0db8:85a3:0000:0000:8a2e:0370:7334",
> "2001:0db8:85a3:0000:0000:8a2e:0370:7335"]]
> +])
> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl --columns=addresses,external_ids
> list Address_Set az2-as2], [0], [dnl
> +addresses : [["2001:0db8:85a3:0000:0000:8a2e:0370:7334",
> "2001:0db8:85a3:0000:0000:8a2e:0370:7335"]]
> +external_ids : {ic-learnt="true"}
> +])
> +
> +# Set the global option in AZ1 to not learn.
> +ovn_as az1 ovn-nbctl set nb_global . options:ic-as-learn=false
> +OVS_WAIT_WHILE([ovn_as az1 ovn-nbctl list Address_Set | grep learnt])
> +ovn_as az1 wait_row_count nb:Address_Set 2
> +
> +# Set the global option in AZ1 back to learn.
> +ovn_as az1 ovn-nbctl set nb_global . options:ic-as-learn=true
> +OVS_WAIT_UNTIL([ovn_as az1 ovn-nbctl list Address_Set | grep learnt])
> +ovn_as az1 wait_row_count nb:Address_Set 4
> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl --columns=addresses,external_ids
> list Address_Set az2-as2], [0], [dnl
> +addresses : [["2001:0db8:85a3:0000:0000:8a2e:0370:7334",
> "2001:0db8:85a3:0000:0000:8a2e:0370:7335"]]
> +external_ids : {ic-learnt="true"}
> +])
> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl --columns=addresses,external_ids
> list Address_Set az2-as1], [0], [dnl
> +addresses : [["2.1.1.1", "2.1.1.2"]]
> +external_ids : {ic-learnt="true"}
> +])
> +
> +# Set the global option in AZ2 to not advertise.
> +ovn_as az2 ovn-nbctl set nb_global . options:ic-as-adv=false
> +wait_row_count ic-sb:Address_Set 1
> +AT_CHECK([ovn-ic-sbctl --columns=addresses find Address_Set name=az2-as1],
> [0], [dnl])
> +AT_CHECK([ovn-ic-sbctl --columns=addresses find Address_Set name=az2-as2],
> [0], [dnl])
> +OVS_WAIT_WHILE([ovn_as az1 ovn-nbctl list Address_Set | grep learnt])
> +
> +# Set the global option in AZ2 back to advertise.
> +ovn_as az2 ovn-nbctl set nb_global . options:ic-as-adv=true
> +wait_row_count ic-sb:Address_Set 3
> +ovn_as az1 wait_row_count nb:Address_Set 4
> +OVS_WAIT_FOR_OUTPUT([ovn-ic-sbctl --columns=addresses find Address_Set
> name=az2-as1], [0], [dnl
> +addresses : [["2.1.1.1", "2.1.1.2"]]
> +])
> +OVS_WAIT_FOR_OUTPUT([ovn-ic-sbctl --columns=addresses find Address_Set
> name=az2-as2], [0], [dnl
> +addresses : [["2001:0db8:85a3:0000:0000:8a2e:0370:7334",
> "2001:0db8:85a3:0000:0000:8a2e:0370:7335"]]
> +])
> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl --columns=addresses,external_ids
> list Address_Set az2-as1], [0], [dnl
> +addresses : [["2.1.1.1", "2.1.1.2"]]
> +external_ids : {ic-learnt="true"}
> +])
> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl --columns=addresses,external_ids
> list Address_Set az2-as2], [0], [dnl
> +addresses : [["2001:0db8:85a3:0000:0000:8a2e:0370:7334",
> "2001:0db8:85a3:0000:0000:8a2e:0370:7335"]]
> +external_ids : {ic-learnt="true"}
> +])
> +
> +# Create an address-set in AZ1 and then try to advertise an address-set with
> same name from AZ2.
> +# It will sync to IC-SB but will not propagate to AZ1.
> +ovn_as az1 ovn-nbctl create Address_Set name=dup-as addresses=\"5.5.5.1\"
> +ovn_as az2 ovn-nbctl create Address_Set name=dup-as addresses=\"5.5.5.2\"
> options:ic-adv=true
> +wait_row_count ic-sb:Address_Set 4
> +OVS_WAIT_FOR_OUTPUT([ovn-ic-sbctl --columns=addresses find Address_Set
> name=dup-as], [0], [dnl
> +addresses : [["5.5.5.2"]]
> +])
> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl --columns=addresses,external_ids
> list Address_Set dup-as], [0], [dnl
> +addresses : [["5.5.5.1"]]
> +external_ids : {}
> +])
> +OVS_WAIT_FOR_OUTPUT([ovn_as az2 ovn-nbctl --columns=addresses,external_ids
> list Address_Set dup-as], [0], [dnl
> +addresses : [["5.5.5.2"]]
> +external_ids : {}
> +])
> +
> +OVN_CLEANUP_IC([az1], [az2])
> +
> +AT_CLEANUP
> +])
> +
> OVN_FOR_EACH_NORTHD([
> AT_SETUP([ovn-ic -- route sync])
>
Regards,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
