Hi Sragdhara,
Thanks for the new revision!
On 7/18/26 5:51 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"]],
This is now actually:
"indexes": [["name", "availability_zone"]],
> "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.
>
> Version 3:
> * Fixed a bug where two remote AZs advertising address sets with the same name
> caused a duplicate key collision in ic_remote_as, leading to an OVSDB
> constraint
> violation. Changed ic_remote_as to map each name to an sset * that merges
> addresses from all remote AZs with that name, so a learning AZ produces a
> single aggregated Northbound entry. Added a 3-AZ test to cover this
> scenario.
>
> NEWS | 4 +
> ic/inc-proc-ic.c | 12 ++-
> ic/ovn-ic.c | 199 ++++++++++++++++++++++++++++++++++++++++++
> northd/en-sync-sb.c | 39 ++++++---
> northd/ovn-northd.c | 6 +-
> ovn-ic-sb.ovsschema | 17 +++-
> ovn-ic-sb.xml | 32 +++++++
> ovn-nb.ovsschema | 9 +-
> ovn-nb.xml | 29 ++++++
> ovn-sb.ovsschema | 11 ++-
> ovn-sb.xml | 4 +
> tests/ovn-ic.at | 208 ++++++++++++++++++++++++++++++++++++++++++++
> 12 files changed, 544 insertions(+), 26 deletions(-)
>
...
> +
> +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 {
> + /* Merge addresses from all remote AZs that share the same
> + * address-set name into a single sset so that duplicate names
> + * across AZs are aggregated rather than colliding in the shash.
> + */
> + struct sset *addrs = shash_find_data(&ic_remote_as, ic_as->name);
> + if (!addrs) {
> + addrs = xmalloc(sizeof *addrs);
> + sset_init(addrs);
> + shash_add(&ic_remote_as, ic_as->name, addrs);
> + }
> + for (size_t i = 0; i < ic_as->n_addresses; i++) {
> + sset_add(addrs, ic_as->addresses[i]);
> + }
This loop can be replaced by:
sset_add_array(addrs, ic_as->addresses, ic_as->n_addresses);
> + }
> + }
> +
...
> 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">
Options are not "common columns" IMO, I'd just skip the group all together.
> + <column name="options">
> + </column>
> + </group>
> </table>
>
I took care of the minor things above and applied the patch to main.
Best regards,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
