On Tue, Apr 29, 2025 at 3:00 PM Xavier Simonart <xsimo...@redhat.com> wrote:
> Hi Ales
>
> Thanks for the patch.
> It looks good to me
>
> Acked-by: Xavier Simonart <xsimo...@redhat.com>
>
> Thanks
> Xavier
>
> On Mon, Apr 28, 2025 at 12:08 PM Ales Musil via dev <
> ovs-dev@openvswitch.org> wrote:
>
>> It can happen that RAFT makes multiple SB LB that are the same.
>> Make sure we catch those cases and reconcile the SB DB to remove the
>> duplicate LB.
>>
>> Fixes: d81e7b4f513c ("northd: Use the same UUID for SB representation of
>> Load_Balancer.")
>> Reported-at: https://issues.redhat.com/browse/FDP-1351
>> Signed-off-by: Ales Musil <amu...@redhat.com>
>> ---
>> northd/en-sync-sb.c | 25 +++++++++++++++++++++++++
>> northd/en-sync-sb.h | 2 ++
>> northd/inc-proc-northd.c | 4 +---
>> tests/ovn-northd.at | 3 +++
>> 4 files changed, 31 insertions(+), 3 deletions(-)
>>
>> diff --git a/northd/en-sync-sb.c b/northd/en-sync-sb.c
>> index b6fb78ee7..de91e7831 100644
>> --- a/northd/en-sync-sb.c
>> +++ b/northd/en-sync-sb.c
>> @@ -344,6 +344,31 @@ sync_to_sb_lb_northd_handler(struct engine_node
>> *node, void *data_)
>> return true;
>> }
>>
>> +bool
>> +sync_to_sb_lb_sb_load_balancer(struct engine_node *node, void *data_)
>> +{
>> + const struct sbrec_load_balancer_table *sb_lb_table =
>> + EN_OVSDB_GET(engine_get_input("SB_load_balancer", node));
>> + struct ed_type_sync_to_sb_lb_data *data = data_;
>> +
>> + /* The only reason to handle SB.Load_Balancer updates is to detect
>> + * spurious records being created in clustered databases due to
>> + * lack of indexing on the SB.Load_Balancer table. All other changes
>> + * are valid and performed by northd, the only write-client for
>> + * this table. */
>> + const struct sbrec_load_balancer *sb_lb;
>> + SBREC_LOAD_BALANCER_TABLE_FOR_EACH_TRACKED (sb_lb, sb_lb_table) {
>> + if (!sbrec_load_balancer_is_new(sb_lb)) {
>> + continue;
>> + }
>> +
>> + if (!sb_lb_table_find(&data->sb_lbs.entries,
>> &sb_lb->header_.uuid)) {
>> + return false;
>> + }
>> + }
>> + return true;
>> +}
>> +
>> /* sync_to_sb_pb engine node functions.
>> * This engine node syncs the SB Port Bindings (partly).
>> * en_northd engine create the SB Port binding rows and
>> diff --git a/northd/en-sync-sb.h b/northd/en-sync-sb.h
>> index f08565eee..3bcbb8259 100644
>> --- a/northd/en-sync-sb.h
>> +++ b/northd/en-sync-sb.h
>> @@ -21,6 +21,8 @@ void *en_sync_to_sb_lb_init(struct engine_node *,
>> struct engine_arg *);
>> void en_sync_to_sb_lb_run(struct engine_node *, void *data);
>> void en_sync_to_sb_lb_cleanup(void *data);
>> bool sync_to_sb_lb_northd_handler(struct engine_node *, void *data
>> OVS_UNUSED);
>> +bool sync_to_sb_lb_sb_load_balancer(struct engine_node *,
>> + void *data OVS_UNUSED);
>>
>> void *en_sync_to_sb_pb_init(struct engine_node *, struct engine_arg *);
>> void en_sync_to_sb_pb_run(struct engine_node *, void *data);
>> diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
>> index 7f92c0cb7..26112cc81 100644
>> --- a/northd/inc-proc-northd.c
>> +++ b/northd/inc-proc-northd.c
>> @@ -374,10 +374,8 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
>> node_global_config_handler);
>> engine_add_input(&en_sync_to_sb_lb, &en_northd,
>> sync_to_sb_lb_northd_handler);
>> - /* There is nothing to handle as duplicates are prevented by using
>> - * UUID mapping between NB and SB. */
>> engine_add_input(&en_sync_to_sb_lb, &en_sb_load_balancer,
>> - engine_noop_handler);
>> + sync_to_sb_lb_sb_load_balancer);
>> engine_add_input(&en_sync_to_sb_lb, &en_sb_logical_dp_group, NULL);
>>
>> engine_add_input(&en_sync_to_sb_pb, &en_northd,
>> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
>> index 82dfe92fd..ec45dd4c2 100644
>> --- a/tests/ovn-northd.at
>> +++ b/tests/ovn-northd.at
>> @@ -6621,8 +6621,11 @@ nlb=$(fetch_column nb:Load_Balancer _uuid)
>> AT_CHECK([ovn-sbctl --id="$nlb" create Load_Balancer name=lb1
>> ls_datapath_group="$dps"], [1], [ignore], [stderr])
>> check grep -q "duplicate uuid" stderr
>>
>> +AT_CHECK([ovn-sbctl create Load_Balancer name=lb1
>> ls_datapath_group="$dps"], [0], [ignore])
>> +
>> check ovn-nbctl --wait=sb sync
>> check_row_count Load_Balancer 1
>> +check test "$nlb" == "$(fetch_column Load_Balancer _uuid)"
>>
>> AT_CLEANUP
>> ])
>> --
>> 2.49.0
>>
>> _______________________________________________
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>>
Thank you Xavier, I went ahead and merged this into main and 25.03.
Regards,
Ales
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
