diff mbox series

[ovs-dev,4/5] northd: Use the same UUID for SB representation of Mirror.

Message ID 20241018100930.826293-5-amusil@redhat.com
State Superseded
Headers show
Series Reuse UUID for SB rows if there is 1:1 mapping | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes success github build: passed

Commit Message

Ales Musil Oct. 18, 2024, 10:09 a.m. UTC 
The NB Mirror has exactly one corresponding row in SB database.
Use the NB UUID for the SB representation which makes the processing easier
and the link between the rows is obvious at first glance.

Signed-off-by: Ales Musil <amusil@redhat.com>
---
 northd/northd.c | 41 ++++++++++++++++-------------------------
 1 file changed, 16 insertions(+), 25 deletions(-)

Comments

Mark Michelson Oct. 18, 2024, 8:18 p.m. UTC | #1 
Acked-by: Mark Michelson <mmichels@redhat.com>

On 10/18/24 06:09, Ales Musil wrote:
> The NB Mirror has exactly one corresponding row in SB database.
> Use the NB UUID for the SB representation which makes the processing easier
> and the link between the rows is obvious at first glance.
> 
> Signed-off-by: Ales Musil <amusil@redhat.com>
> ---
>   northd/northd.c | 41 ++++++++++++++++-------------------------
>   1 file changed, 16 insertions(+), 25 deletions(-)
> 
> diff --git a/northd/northd.c b/northd/northd.c
> index a81defe49..3b9db5b3c 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -18096,23 +18096,21 @@ mirror_needs_update(const struct nbrec_mirror *nb_mirror,
>   }
>   
>   static void
> -sync_mirrors_iterate_nb_mirror(struct ovsdb_idl_txn *ovnsb_txn,
> -                               const char *mirror_name,
> -                               const struct nbrec_mirror *nb_mirror,
> -                               struct shash *sb_mirrors)
> +sync_nb_and_sb_mirror(struct ovsdb_idl_txn *ovnsb_txn,
> +                      const char *mirror_name,
> +                      const struct nbrec_mirror *nb_mirror,
> +                      const struct sbrec_mirror_table *sbrec_mirror_table)
>   {
> -    const struct sbrec_mirror *sb_mirror;
> -    bool new_sb_mirror = false;
> +    const struct uuid *nb_uuid = &nb_mirror->header_.uuid;
> +    const struct sbrec_mirror *sb_mirror =
> +        sbrec_mirror_table_get_for_uuid(sbrec_mirror_table, nb_uuid);
>   
> -    sb_mirror = shash_find_data(sb_mirrors, mirror_name);
>       if (!sb_mirror) {
> -        sb_mirror = sbrec_mirror_insert(ovnsb_txn);
> +        sb_mirror = sbrec_mirror_insert_persist_uuid(ovnsb_txn, nb_uuid);
>           sbrec_mirror_set_name(sb_mirror, mirror_name);
> -        shash_add(sb_mirrors, sb_mirror->name, sb_mirror);
> -        new_sb_mirror = true;
>       }
>   
> -    if (new_sb_mirror || mirror_needs_update(nb_mirror, sb_mirror)) {
> +    if (mirror_needs_update(nb_mirror, sb_mirror)) {
>           sbrec_mirror_set_filter(sb_mirror, nb_mirror->filter);
>           sbrec_mirror_set_index(sb_mirror, nb_mirror->index);
>           sbrec_mirror_set_sink(sb_mirror, nb_mirror->sink);
> @@ -18125,26 +18123,19 @@ sync_mirrors(struct ovsdb_idl_txn *ovnsb_txn,
>                const struct nbrec_mirror_table *nbrec_mirror_table,
>                const struct sbrec_mirror_table *sbrec_mirror_table)
>   {
> -    struct shash sb_mirrors = SHASH_INITIALIZER(&sb_mirrors);
> -
>       const struct sbrec_mirror *sb_mirror;
> -    SBREC_MIRROR_TABLE_FOR_EACH (sb_mirror, sbrec_mirror_table) {
> -        shash_add(&sb_mirrors, sb_mirror->name, sb_mirror);
> +    SBREC_MIRROR_TABLE_FOR_EACH_SAFE (sb_mirror, sbrec_mirror_table) {
> +        if (!nbrec_mirror_table_get_for_uuid(nbrec_mirror_table,
> +                                             &sb_mirror->header_.uuid)) {
> +            sbrec_mirror_delete(sb_mirror);
> +        }
>       }
>   
>       const struct nbrec_mirror *nb_mirror;
>       NBREC_MIRROR_TABLE_FOR_EACH (nb_mirror, nbrec_mirror_table) {
> -        sync_mirrors_iterate_nb_mirror(ovnsb_txn, nb_mirror->name, nb_mirror,
> -                                       &sb_mirrors);
> -        shash_find_and_delete(&sb_mirrors, nb_mirror->name);
> -    }
> -
> -    struct shash_node *node, *next;
> -    SHASH_FOR_EACH_SAFE (node, next, &sb_mirrors) {
> -        sbrec_mirror_delete(node->data);
> -        shash_delete(&sb_mirrors, node);
> +        sync_nb_and_sb_mirror(ovnsb_txn, nb_mirror->name,
> +                              nb_mirror, sbrec_mirror_table);
>       }
> -    shash_destroy(&sb_mirrors);
>   }
>   
>   /*
Numan Siddique Nov. 13, 2024, 4:49 p.m. UTC | #2 
On Fri, Oct 18, 2024 at 4:18 PM Mark Michelson <mmichels@redhat.com> wrote:
>
> Acked-by: Mark Michelson <mmichels@redhat.com>

Acked-by: Numan Siddique <numans@ovn.org>

Numan

>
> On 10/18/24 06:09, Ales Musil wrote:
> > The NB Mirror has exactly one corresponding row in SB database.
> > Use the NB UUID for the SB representation which makes the processing easier
> > and the link between the rows is obvious at first glance.
> >
> > Signed-off-by: Ales Musil <amusil@redhat.com>
> > ---
> >   northd/northd.c | 41 ++++++++++++++++-------------------------
> >   1 file changed, 16 insertions(+), 25 deletions(-)
> >
> > diff --git a/northd/northd.c b/northd/northd.c
> > index a81defe49..3b9db5b3c 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -18096,23 +18096,21 @@ mirror_needs_update(const struct nbrec_mirror *nb_mirror,
> >   }
> >
> >   static void
> > -sync_mirrors_iterate_nb_mirror(struct ovsdb_idl_txn *ovnsb_txn,
> > -                               const char *mirror_name,
> > -                               const struct nbrec_mirror *nb_mirror,
> > -                               struct shash *sb_mirrors)
> > +sync_nb_and_sb_mirror(struct ovsdb_idl_txn *ovnsb_txn,
> > +                      const char *mirror_name,
> > +                      const struct nbrec_mirror *nb_mirror,
> > +                      const struct sbrec_mirror_table *sbrec_mirror_table)
> >   {
> > -    const struct sbrec_mirror *sb_mirror;
> > -    bool new_sb_mirror = false;
> > +    const struct uuid *nb_uuid = &nb_mirror->header_.uuid;
> > +    const struct sbrec_mirror *sb_mirror =
> > +        sbrec_mirror_table_get_for_uuid(sbrec_mirror_table, nb_uuid);
> >
> > -    sb_mirror = shash_find_data(sb_mirrors, mirror_name);
> >       if (!sb_mirror) {
> > -        sb_mirror = sbrec_mirror_insert(ovnsb_txn);
> > +        sb_mirror = sbrec_mirror_insert_persist_uuid(ovnsb_txn, nb_uuid);
> >           sbrec_mirror_set_name(sb_mirror, mirror_name);
> > -        shash_add(sb_mirrors, sb_mirror->name, sb_mirror);
> > -        new_sb_mirror = true;
> >       }
> >
> > -    if (new_sb_mirror || mirror_needs_update(nb_mirror, sb_mirror)) {
> > +    if (mirror_needs_update(nb_mirror, sb_mirror)) {
> >           sbrec_mirror_set_filter(sb_mirror, nb_mirror->filter);
> >           sbrec_mirror_set_index(sb_mirror, nb_mirror->index);
> >           sbrec_mirror_set_sink(sb_mirror, nb_mirror->sink);
> > @@ -18125,26 +18123,19 @@ sync_mirrors(struct ovsdb_idl_txn *ovnsb_txn,
> >                const struct nbrec_mirror_table *nbrec_mirror_table,
> >                const struct sbrec_mirror_table *sbrec_mirror_table)
> >   {
> > -    struct shash sb_mirrors = SHASH_INITIALIZER(&sb_mirrors);
> > -
> >       const struct sbrec_mirror *sb_mirror;
> > -    SBREC_MIRROR_TABLE_FOR_EACH (sb_mirror, sbrec_mirror_table) {
> > -        shash_add(&sb_mirrors, sb_mirror->name, sb_mirror);
> > +    SBREC_MIRROR_TABLE_FOR_EACH_SAFE (sb_mirror, sbrec_mirror_table) {
> > +        if (!nbrec_mirror_table_get_for_uuid(nbrec_mirror_table,
> > +                                             &sb_mirror->header_.uuid)) {
> > +            sbrec_mirror_delete(sb_mirror);
> > +        }
> >       }
> >
> >       const struct nbrec_mirror *nb_mirror;
> >       NBREC_MIRROR_TABLE_FOR_EACH (nb_mirror, nbrec_mirror_table) {
> > -        sync_mirrors_iterate_nb_mirror(ovnsb_txn, nb_mirror->name, nb_mirror,
> > -                                       &sb_mirrors);
> > -        shash_find_and_delete(&sb_mirrors, nb_mirror->name);
> > -    }
> > -
> > -    struct shash_node *node, *next;
> > -    SHASH_FOR_EACH_SAFE (node, next, &sb_mirrors) {
> > -        sbrec_mirror_delete(node->data);
> > -        shash_delete(&sb_mirrors, node);
> > +        sync_nb_and_sb_mirror(ovnsb_txn, nb_mirror->name,
> > +                              nb_mirror, sbrec_mirror_table);
> >       }
> > -    shash_destroy(&sb_mirrors);
> >   }
> >
> >   /*
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox series

Patch

diff --git a/northd/northd.c b/northd/northd.c
index a81defe49..3b9db5b3c 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -18096,23 +18096,21 @@  mirror_needs_update(const struct nbrec_mirror *nb_mirror,
 }
 
 static void
-sync_mirrors_iterate_nb_mirror(struct ovsdb_idl_txn *ovnsb_txn,
-                               const char *mirror_name,
-                               const struct nbrec_mirror *nb_mirror,
-                               struct shash *sb_mirrors)
+sync_nb_and_sb_mirror(struct ovsdb_idl_txn *ovnsb_txn,
+                      const char *mirror_name,
+                      const struct nbrec_mirror *nb_mirror,
+                      const struct sbrec_mirror_table *sbrec_mirror_table)
 {
-    const struct sbrec_mirror *sb_mirror;
-    bool new_sb_mirror = false;
+    const struct uuid *nb_uuid = &nb_mirror->header_.uuid;
+    const struct sbrec_mirror *sb_mirror =
+        sbrec_mirror_table_get_for_uuid(sbrec_mirror_table, nb_uuid);
 
-    sb_mirror = shash_find_data(sb_mirrors, mirror_name);
     if (!sb_mirror) {
-        sb_mirror = sbrec_mirror_insert(ovnsb_txn);
+        sb_mirror = sbrec_mirror_insert_persist_uuid(ovnsb_txn, nb_uuid);
         sbrec_mirror_set_name(sb_mirror, mirror_name);
-        shash_add(sb_mirrors, sb_mirror->name, sb_mirror);
-        new_sb_mirror = true;
     }
 
-    if (new_sb_mirror || mirror_needs_update(nb_mirror, sb_mirror)) {
+    if (mirror_needs_update(nb_mirror, sb_mirror)) {
         sbrec_mirror_set_filter(sb_mirror, nb_mirror->filter);
         sbrec_mirror_set_index(sb_mirror, nb_mirror->index);
         sbrec_mirror_set_sink(sb_mirror, nb_mirror->sink);
@@ -18125,26 +18123,19 @@  sync_mirrors(struct ovsdb_idl_txn *ovnsb_txn,
              const struct nbrec_mirror_table *nbrec_mirror_table,
              const struct sbrec_mirror_table *sbrec_mirror_table)
 {
-    struct shash sb_mirrors = SHASH_INITIALIZER(&sb_mirrors);
-
     const struct sbrec_mirror *sb_mirror;
-    SBREC_MIRROR_TABLE_FOR_EACH (sb_mirror, sbrec_mirror_table) {
-        shash_add(&sb_mirrors, sb_mirror->name, sb_mirror);
+    SBREC_MIRROR_TABLE_FOR_EACH_SAFE (sb_mirror, sbrec_mirror_table) {
+        if (!nbrec_mirror_table_get_for_uuid(nbrec_mirror_table,
+                                             &sb_mirror->header_.uuid)) {
+            sbrec_mirror_delete(sb_mirror);
+        }
     }
 
     const struct nbrec_mirror *nb_mirror;
     NBREC_MIRROR_TABLE_FOR_EACH (nb_mirror, nbrec_mirror_table) {
-        sync_mirrors_iterate_nb_mirror(ovnsb_txn, nb_mirror->name, nb_mirror,
-                                       &sb_mirrors);
-        shash_find_and_delete(&sb_mirrors, nb_mirror->name);
-    }
-
-    struct shash_node *node, *next;
-    SHASH_FOR_EACH_SAFE (node, next, &sb_mirrors) {
-        sbrec_mirror_delete(node->data);
-        shash_delete(&sb_mirrors, node);
+        sync_nb_and_sb_mirror(ovnsb_txn, nb_mirror->name,
+                              nb_mirror, sbrec_mirror_table);
     }
-    shash_destroy(&sb_mirrors);
 }
 
 /*