diff mbox series

[ovs-dev,v1,2/4] northd: Use the same LS/LR UUID for SB datapath_bindings.

Message ID 20250110162652.3550775-1-numans@ovn.org
State Changes Requested
Headers show
Series northd I-P for logical switch creation | 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

Numan Siddique Jan. 10, 2025, 4:26 p.m. UTC
From: Numan Siddique <numans@ovn.org>

Signed-off-by: Numan Siddique <numans@ovn.org>
---
 northd/northd.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Comments

Ales Musil Jan. 17, 2025, 2:01 p.m. UTC | #1
On Fri, Jan 10, 2025 at 5:27 PM <numans@ovn.org> wrote:

> From: Numan Siddique <numans@ovn.org>
>
> Signed-off-by: Numan Siddique <numans@ovn.org>
> ---
>

Hi Numan,

as per discussion during the u/s meeting I would like to raise the
potential problem with UUID conflicts because we could have the same
UUID for LS and LR. I'm not sure what would be the proper way to
solve this to be honest. We could have a fallback to random UUID when
the conflict is detected, however that defeats the benefit of having
the same UUID. How hard would it be to adjust this series to not use
the same UUID between LS and DP?


>  northd/northd.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 15956210a0..07cc332a03 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -847,6 +847,16 @@ join_datapaths(const struct
> nbrec_logical_switch_table *nbrec_ls_table,
>              continue;
>          }
>
> +        if (!uuid_equals(&sb->header_.uuid, &key)) {
> +            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> +            VLOG_INFO_RL(
> +                &rl, "deleting Datapath_Binding "UUID_FMT" with "
> +                "mismatching external-ids:logical-switch/router "UUID_FMT,
> +                UUID_ARGS(&sb->header_.uuid), UUID_ARGS(&key));
> +            sbrec_datapath_binding_delete(sb);
> +            continue;
> +        }
> +
>          if (ovn_datapath_find_(datapaths, &key)) {
>              static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
>              VLOG_INFO_RL(
> @@ -1064,7 +1074,8 @@ build_datapaths(struct ovsdb_idl_txn *ovnsb_txn,
>          ovn_datapath_update_external_ids(od);
>      }
>      LIST_FOR_EACH (od, list, &nb_only) {
> -        od->sb = sbrec_datapath_binding_insert(ovnsb_txn);
> +        od->sb = sbrec_datapath_binding_insert_persist_uuid(ovnsb_txn,
> +                                                            &od->key);
>          ovn_datapath_update_external_ids(od);
>          sbrec_datapath_binding_set_tunnel_key(od->sb, od->tunnel_key);
>      }
> --
> 2.47.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Thanks,
Ales
Numan Siddique Jan. 17, 2025, 9:40 p.m. UTC | #2
On Fri, Jan 17, 2025 at 9:01 AM Ales Musil <amusil@redhat.com> wrote:
>
>
>
> On Fri, Jan 10, 2025 at 5:27 PM <numans@ovn.org> wrote:
>>
>> From: Numan Siddique <numans@ovn.org>
>>
>> Signed-off-by: Numan Siddique <numans@ovn.org>
>> ---
>
>
> Hi Numan,
>
> as per discussion during the u/s meeting I would like to raise the
> potential problem with UUID conflicts because we could have the same
> UUID for LS and LR. I'm not sure what would be the proper way to
> solve this to be honest. We could have a fallback to random UUID when
> the conflict is detected, however that defeats the benefit of having
> the same UUID. How hard would it be to adjust this series to not use
> the same UUID between LS and DP?

Hi Ales,

Thanks for the comments.  I think it should not be hard to drop this patch.
I'll look into it.

Thanks
Numan

>
>>
>>  northd/northd.c | 13 ++++++++++++-
>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/northd/northd.c b/northd/northd.c
>> index 15956210a0..07cc332a03 100644
>> --- a/northd/northd.c
>> +++ b/northd/northd.c
>> @@ -847,6 +847,16 @@ join_datapaths(const struct nbrec_logical_switch_table *nbrec_ls_table,
>>              continue;
>>          }
>>
>> +        if (!uuid_equals(&sb->header_.uuid, &key)) {
>> +            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
>> +            VLOG_INFO_RL(
>> +                &rl, "deleting Datapath_Binding "UUID_FMT" with "
>> +                "mismatching external-ids:logical-switch/router "UUID_FMT,
>> +                UUID_ARGS(&sb->header_.uuid), UUID_ARGS(&key));
>> +            sbrec_datapath_binding_delete(sb);
>> +            continue;
>> +        }
>> +
>>          if (ovn_datapath_find_(datapaths, &key)) {
>>              static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
>>              VLOG_INFO_RL(
>> @@ -1064,7 +1074,8 @@ build_datapaths(struct ovsdb_idl_txn *ovnsb_txn,
>>          ovn_datapath_update_external_ids(od);
>>      }
>>      LIST_FOR_EACH (od, list, &nb_only) {
>> -        od->sb = sbrec_datapath_binding_insert(ovnsb_txn);
>> +        od->sb = sbrec_datapath_binding_insert_persist_uuid(ovnsb_txn,
>> +                                                            &od->key);
>>          ovn_datapath_update_external_ids(od);
>>          sbrec_datapath_binding_set_tunnel_key(od->sb, od->tunnel_key);
>>      }
>> --
>> 2.47.1
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>
> Thanks,
> Ales
diff mbox series

Patch

diff --git a/northd/northd.c b/northd/northd.c
index 15956210a0..07cc332a03 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -847,6 +847,16 @@  join_datapaths(const struct nbrec_logical_switch_table *nbrec_ls_table,
             continue;
         }
 
+        if (!uuid_equals(&sb->header_.uuid, &key)) {
+            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
+            VLOG_INFO_RL(
+                &rl, "deleting Datapath_Binding "UUID_FMT" with "
+                "mismatching external-ids:logical-switch/router "UUID_FMT,
+                UUID_ARGS(&sb->header_.uuid), UUID_ARGS(&key));
+            sbrec_datapath_binding_delete(sb);
+            continue;
+        }
+
         if (ovn_datapath_find_(datapaths, &key)) {
             static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
             VLOG_INFO_RL(
@@ -1064,7 +1074,8 @@  build_datapaths(struct ovsdb_idl_txn *ovnsb_txn,
         ovn_datapath_update_external_ids(od);
     }
     LIST_FOR_EACH (od, list, &nb_only) {
-        od->sb = sbrec_datapath_binding_insert(ovnsb_txn);
+        od->sb = sbrec_datapath_binding_insert_persist_uuid(ovnsb_txn,
+                                                            &od->key);
         ovn_datapath_update_external_ids(od);
         sbrec_datapath_binding_set_tunnel_key(od->sb, od->tunnel_key);
     }