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