On Wed, Oct 2, 2024 at 4:16 AM Ales Musil <[email protected]> wrote:
>
> On Tue, Aug 27, 2024 at 2:56 PM Xavier Simonart <[email protected]> wrote:
>
> > ERROR: AddressSanitizer: SEGV on unknown address 0x000000000020 (pc
> > 0x000000709f68 bp 0x7ffe8546c9b0 sp 0x7ffe8546c9b0 T0)
> > 0 0x709f68 in hmap_insert_fast ./ovs/./include/openvswitch/hmap.h
> > 1 0x702611 in hmap_insert_at ./ovs/./include/openvswitch/hmap.h:307:5
> > 2 0x70444c in ovsdb_idl_txn_write__ ./ovs/lib/ovsdb-idl.c:3631:9
> > 3 0x704141 in ovsdb_idl_txn_write ./ovs/lib/ovsdb-idl.c:3688:5
> > 4 0x6c1cd1 in icsbrec_availability_zone_set_name
> > ./lib/ovn-ic-sb-idl.c:395:5
> > 5 0x5a7f1e in az_run ./ic/ovn-ic.c:144:17
> > 6 0x5a7f1e in main ./ic/ovn-ic.c:2323:62
> > 7 0x7fc605b6db74 in __libc_start_main (/lib64/libc.so.6+0x27b74)
> > 8 0x4c8fed in _start (/root/master-ovn/ic/ovn-ic+0x4c8fed)
> >
> > Fixes: 37fd1dd378ea ("ovn-ic: Handle NB:name updates properly.")
> >
> > Signed-off-by: Xavier Simonart <[email protected]>
> > ---
> >
>
> Hi Xavier,
>
> I have one comment down below.
>
> ic/ovn-ic.c | 24 +++++++++++++-----------
> > 1 file changed, 13 insertions(+), 11 deletions(-)
> >
> > diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
> > index e91010892..84d2ed5a4 100644
> > --- a/ic/ovn-ic.c
> > +++ b/ic/ovn-ic.c
> > @@ -137,19 +137,21 @@ az_run(struct ic_context *ctx)
> > * "ovn-ic-sbctl destroy avail <az>". */
> > static char *az_name;
> > const struct icsbrec_availability_zone *az;
> > - if (az_name && strcmp(az_name, nb_global->name)) {
> > - ICSBREC_AVAILABILITY_ZONE_FOR_EACH (az, ctx->ovnisb_idl) {
> > - /* AZ name update locally need to update az in ISB. */
> > - if (nb_global->name[0] && !strcmp(az->name, az_name)) {
> > - icsbrec_availability_zone_set_name(az, nb_global->name);
> > - break;
> > - } else if (!nb_global->name[0] && !strcmp(az->name, az_name))
> > {
> > - icsbrec_availability_zone_delete(az);
> > - break;
> > + if (ctx->ovnisb_txn) {
> >
>
> I don't think we have to nest this even further, we could add it to the if
> below.
>
I did this change and applied this patch to main and backported till 23.09.
Thanks
Numan
> + if (az_name && strcmp(az_name, nb_global->name)) {
> > + ICSBREC_AVAILABILITY_ZONE_FOR_EACH (az, ctx->ovnisb_idl) {
> > + /* AZ name update locally need to update az in ISB. */
> > + if (nb_global->name[0] && !strcmp(az->name, az_name)) {
> > + icsbrec_availability_zone_set_name(az,
> > nb_global->name);
> > + break;
> > + } else if (!nb_global->name[0] && !strcmp(az->name,
> > az_name)) {
> > + icsbrec_availability_zone_delete(az);
> > + break;
> > + }
> > }
> > + free(az_name);
> > + az_name = NULL;
> > }
> > - free(az_name);
> > - az_name = NULL;
> > }
> >
> > if (!nb_global->name[0]) {
> > --
> > 2.31.1
> >
> > _______________________________________________
> > dev mailing list
> > [email protected]
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
> >
> Thanks,
> Ales
> --
>
> Ales Musil
>
> Senior Software Engineer - OVN Core
>
> Red Hat EMEA <https://www.redhat.com>
>
> [email protected]
> <https://red.ht/sig>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
