On Wednesday, August 13, 2025, Han Zhou <hz...@ovn.org
<mailto:hz...@ovn.org>> wrote:
On Wed, Aug 13, 2025 at 1:20 AM Dumitru Ceara <dce...@redhat.com
<mailto:dce...@redhat.com>> wrote:
>
> Hi Mark, Han, Numan,
>
> On 8/12/25 8:32 PM, Mark Michelson via dev wrote:
> > On 8/11/25 2:36 PM, Han Zhou wrote:
> >>
> >>
> >> On Mon, Aug 11, 2025 at 10:22 AM Numan Siddique
<num...@ovn.org <mailto:num...@ovn.org>
> >> <mailto:num...@ovn.org <mailto:num...@ovn.org>>> wrote:
> >> >
> >> > On Mon, Aug 11, 2025 at 10:00 PM Han Zhou <hz...@ovn.org
<mailto:hz...@ovn.org>
> >> <mailto:hz...@ovn.org <mailto:hz...@ovn.org>>> wrote:
> >> > >
> >> > > On Mon, Aug 11, 2025 at 7:59 AM Mark Michelson via dev <
> >> > > ovs-dev@openvswitch.org <mailto:ovs-dev@openvswitch.org>
<mailto:ovs-dev@openvswitch.org <mailto:ovs-dev@openvswitch.org>>>
wrote:
> >> > > >
> >> > > > Prior to this commit, if you wanted to find the
corresponding
> >> northbound
> >> > > > UUID for a southbound Logical Datapath, you could find
it in
> >> one of two
> >> > > > places:
> >> > > >
> >> > > > For logical switches, it was in external-ids:logical-
switch.
> >> > > > For logical routers, it was in external-ids:logical-router.
> >> > > >
> >> > > > With this commit, we are separating the type and UUID into
> >> separate
> >> > > > fields. This way, no matter the type of the datapath,
you can
> >> find the
> >> > > > UUID. And you can find the type of the datapath without
having to
> >> > > > potentially check multiple external-ids keys to do so.
> >> > > >
> >> > > > These fields are going to be used pretty heavily by
northd in
> >> upcoming
> >> > > > patches, so instead of making them external-ids, these
are now
> >> > > > full-fledged columns on the southbound
Datapath_Binding. The
> >> UUID of the
> >> > > > northbound logical datapath is in a column called
"nb_uuid" and
> >> the type
> >> > > > of the logical datapath is stored in an enumerated column
> >> called "type".
> >> > >
> >> > > Thanks Mark. Sorry for reviewing this late. While still
reviewing
> >> the rest
> >> > > of the patches, I want to raise a question regarding this
patch.
> >> Would it
> >> > > be more straightforward and efficient to use NB UUIDs
directly in
> >> SB for
> >> > > both LS and LR? I saw that there was an old patch from
Ales which
> >> mentioned
> >> > > this potential change [0]. It mentioned the necessity of
"collision
> >> > > detection", but I don't think it is necessary, because
UUIDs are
> >> just
> >> > > designed to be globally unique. Did I miss any other
discussions
> >> for this?
> >> > >
> >> > > [0] https://www.mail-archive.com/ovs-
dev%40openvswitch.org/ <https://www.mail-archive.com/ovs-
dev%40openvswitch.org/>
> >> msg89347.html <https://www.mail-archive.com/ovs-
dev%40openvswitch.org/ <https://www.mail-archive.com/ovs-
dev%40openvswitch.org/>
> >> msg89347.html>
> >> > >
> >> >
> >> > I had included this change of using the same NB UUIDs in SB for
> >> > Datapath-bindings here [1]
> >> > as part of this series - [2]. Please see the comments in [1].
> >> >
> >> > Later Lorenzo took over this patch series and dropped the
patch [1]
> >> from it.
> >> >
> >> > [1] - https://patchwork.ozlabs.org/project/ovn/ <https://
patchwork.ozlabs.org/project/ovn/>
> >> patch/20250110162652.3550775-1-num...@ovn.org/
<http://20250110162652.3550775-1-num...@ovn.org/> <https://
> >> patchwork.ozlabs.org/project/ovn/
patch/20250110162652.3550775-1- <http://patchwork.ozlabs.org/
project/ovn/patch/20250110162652.3550775-1->
> >> num...@ovn.org/ <http://num...@ovn.org/>>
> >> > [2] - https://patchwork.ozlabs.org/project/ovn/list/
<https://patchwork.ozlabs.org/project/ovn/list/>?
> >> series=439760&state=* <https://patchwork.ozlabs.org/project/
ovn/list/ <https://patchwork.ozlabs.org/project/ovn/list/>?
> >> series=439760&state=*>
> >> >
> >> > The main concern is that both LS and LR can have the same uuid.
> >> > IMO this can happen only if LS and LR were created by passing
> >> > "--id=<SAME_UUID>".
> >> >
> >> Using the same explicit UUID for LS and LR creation is clearly a
> >> misconfiguration.
> >>
> >> > I personally think we can overlook this corner case and
document it
> >> > and suggest the users to recreate such logical switches/
routers before
> >> > upgrading.
> >>
> >> I agree, and I assume legitimate deployments would not have such
> >> conflicts.
> >
> > I have been looking at the datapath syncing code *a lot* lately.
> > Existing code in northd/northd.c in the main branch already
complains if
> > UUIDs are shared between logical switches and logical routers.
> >
> > This is from join_datapaths() in northd/northd.c in main:
> >
> > SBREC_DATAPATH_BINDING_TABLE_FOR_EACH_SAFE (sb,
sbrec_dp_table) {
> > struct uuid key;
> > if (!smap_get_uuid(&sb->external_ids, "logical-
switch", &key) &&
> > !smap_get_uuid(&sb->external_ids, "logical-
router", &key)) {
> > ovsdb_idl_txn_add_comment(
> > ovnsb_txn,
> > "deleting Datapath_Binding "UUID_FMT" that
lacks "
> > "external-ids:logical-switch and "
> > "external-ids:logical-router",
> > UUID_ARGS(&sb->header_.uuid));
> > 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(
> > &rl, "deleting Datapath_Binding "UUID_FMT" with "
> > "duplicate external-ids:logical-switch/router
"UUID_FMT,
> > UUID_ARGS(&sb->header_.uuid), UUID_ARGS(&key));
> > sbrec_datapath_binding_delete(sb);
> > continue;
> > }
> >
> > struct ovn_datapath *od =
ovn_datapath_create(datapaths, &key,
> > NULL,
NULL, sb);
> > ovs_list_push_back(sb_only, &od->list);
> > }
> >
> >
> > Notice the call to `ovn_datapath_find_()` here. If you have a
logical
> > switch and logical router with the same UUID, then northd will
complain
> > and not create an ovn_datapath for the southbound datapath
binding with
> > the duplicated UUID.
> >
> > Therefore, I think it's perfectly fine to propagate the
northbound UUID
> > to the southbound, since the UUID collision would already be
treated as
> > an error, and we would not break anything by doing so.
> >
>
> Unfortunately, while looking at your v16 where we actually
propagate the
> NB UUID to the SB, I realized this might introduce a significant
problem
> during upgrades.
>
> Due to the "UUID sync" we now have to recreate _all_
SB.Datapath_binding
> records when ovn-northd upgrades to a version that includes that
change.
> That essentially means we need to recreate most of the SB contents
> (datapaths, lflows, datapath groups).
>
> That SB change will likely be a single transaction (ovn-northd ->
SB) so
> it will also be sent as a single jsonrpc update to ovn-controllers.
>
> In large(r) setups, e.g., with a few hundred chassis, this will
likely
> create a performance issue as the SB ovsdb-server will have to
send that
> huge transaction to all ovn-controllers. While that could be
mitigated
> by using ovsdb-relay for example, it's quite likely that our
users don't
> have that in place.
>
> So my recommendation is to _not_ sync NB.UUID to
> SB.Datapath_Binding.UUID. I don't think the extra column (the v14)
> approach was too much overhead, it's just one UUID per datapath
record
> and the northd code doesn't seem to become way more complex.
>
Hi Dumitru,
Hi Han,
Thanks for sharing your concern. However, we did have similar
changes in the past that resulted in large SB updates, for example,
"northd.c: Generate and maintain SB lflow uuid in ovn_lflow." that
changed UUIDs of all logical flows, which is the biggest part of SB
DB. In addition, whenever there is a SB DB schema change, during the
upgrade it would anyway resend all the DB data to all ovn-
controllers, and this time we are changing the
schema as well.
Thanks for the pointer to the precedent. I’d argue that datapath binding
resync is slightly more costly than just lflow but you’re right it’s
probably comparable.
Thanks,
Dumitru
Thanks,
Han
> Regards,
> Dumitru
>
> >>
> >> >
> >> > Thanks
> >> > Numan
> >> >
> >> > > Thanks,
> >> > > Han
> >> > >
> >> > > >
> >> > > > Note that there is a seemingly-unrelated change in the
check
> >> packet
> >> > > > length test in tests/ovn.at <http://ovn.at> <http://
ovn.at <http://ovn.at>>. This test started
> >> failing because its use
> >> > > > of `grep` was picking up the new "nb_uuid" column
accidentally.
> >> The
> >> > > > change to the test uses `fetch_column` since it is more
precise.
> >> > > >
> >> > > > Signed-off-by: Mark Michelson <mmich...@redhat.com
<mailto:mmich...@redhat.com>
> >> <mailto:mmich...@redhat.com <mailto:mmich...@redhat.com>>>
> >> > > > Acked-by: Ales Musil <amu...@redhat.com
<mailto:amu...@redhat.com>
> >> <mailto:amu...@redhat.com <mailto:amu...@redhat.com>>>
> >> > > > ---
> >> > > > v14
> >> > > > * Rebased.
> >> > > >
> >> > > > v13
> >> > > > * Rebased.
> >> > > >
> >> > > > v12
> >> > > > * Rebased.
> >> > > > * Added Ales Musil's ack.
> >> > > >
> >> > > > v11
> >> > > > * Rebased.
> >> > > >
> >> > > > v10
> >> > > > * Rebased.
> >> > > > * Maintained backwards-compatibility with external_ids
method of
> >> > > > storing NB UUID and type. It was recommended to continue
> >> setting
> >> > > > the external-ids in addition to the new fields. This
patch
> >> does not
> >> > > > do that because that code would get immediately
wiped out by
> >> the
> >> > > > next patch in the series. The next patch in the
series does
> >> write
> >> > > > the old external_ids values, though.
> >> > > >
> >> > > > v9
> >> > > > * Rebased.
> >> > > >
> >> > > > v8
> >> > > > * Rebased.
> >> > > > * Changed the types of the new columns. The nb_uuid is
type
> >> "uuid" and
> >> > > > the type is an enum.
> >> > > >
> >> > > > v7
> >> > > > * Rebased.
> >> > > > * Changed from using external-ids for the nb_uuid and
type to
> >> using
> >> > > > new columns in the Datapath_Binding table.
> >> > > >
> >> > > > v6
> >> > > > * This is the first version of the series to contain
this patch.
> >> > > > ---
> >> > > > controller/local_data.c | 2 +-
> >> > > > ic/ovn-ic.c | 5 +++--
> >> > > > lib/ovn-util.c | 45 +++++++++++++++++++++++++
++++++++
> >> ++ ++++++
> >> > > > lib/ovn-util.h | 8 ++++++++
> >> > > > northd/northd.c | 38 ++++++++++++++++
+-----------------
> >> > > > northd/northd.h | 5 +++--
> >> > > > ovn-sb.ovsschema | 11 ++++++++--
> >> > > > ovn-sb.xml | 21 ++++++++++---------
> >> > > > tests/ovn-controller.at <http://ovn-controller.at>
<http://ovn-controller.at <http://ovn-controller.at>> | 4 ++++
> >> > > > tests/ovn-northd.at <http://ovn-northd.at> <http://
ovn-northd.at <http://ovn-northd.at>> | 6 +++---
> >> > > > tests/ovn.at <http://ovn.at> <http://ovn.at <http://
ovn.at>> | 6 ++----
> >> > > > utilities/ovn-sbctl.c | 4 ++--
> >> > > > utilities/ovn-trace.c | 3 +--
> >> > > > 13 files changed, 112 insertions(+), 46 deletions(-)
> >> > > >
> >> > > > diff --git a/controller/local_data.c b/controller/
local_data.c
> >> > > > index 00383d091..a35d3fa5a 100644
> >> > > > --- a/controller/local_data.c
> >> > > > +++ b/controller/local_data.c
> >> > > > @@ -708,7 +708,7 @@ local_datapath_peer_port_add(struct
> >> local_datapath
> >> > > *ld,
> >> > > > static bool
> >> > > > datapath_is_switch(const struct sbrec_datapath_binding
*ldp)
> >> > > > {
> >> > > > - return smap_get(&ldp->external_ids, "logical-
switch") !=
> >> NULL;
> >> > > > + return strcmp(datapath_get_nb_type(ldp), "logical-
switch")
> >> == 0;
> >> > > > }
> >> > > >
> >> > > > static bool
> >> > > > diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
> >> > > > index caffa6fe0..0cd78e720 100644
> >> > > > --- a/ic/ovn-ic.c
> >> > > > +++ b/ic/ovn-ic.c
> >> > > > @@ -613,8 +613,7 @@ get_router_uuid_by_sb_pb(struct
ic_context
> >> *ctx,
> >> > > > return NULL;
> >> > > > }
> >> > > >
> >> > > > - return smap_get_uuid(&router_pb->datapath-
>external_ids,
> >> > > "logical-router",
> >> > > > - router_uuid);
> >> > > > + return datapath_get_nb_uuid(router_pb->datapath,
> >> router_uuid);
> >> > > > }
> >> > > >
> >> > > > static void
> >> > > > @@ -2582,6 +2581,8 @@ main(int argc, char *argv[])
> >> > > > ovsdb_idl_add_table(ovnsb_idl_loop.idl,
> >> > > &sbrec_table_datapath_binding);
> >> > > > ovsdb_idl_add_column(ovnsb_idl_loop.idl,
> >> > > >
> >> &sbrec_datapath_binding_col_external_ids);
> >> > > > + ovsdb_idl_add_column(ovnsb_idl_loop.idl,
> >> > > > +
&sbrec_datapath_binding_col_nb_uuid);
> >> > > >
> >> > > > ovsdb_idl_add_table(ovnsb_idl_loop.idl,
> >> &sbrec_table_port_binding);
> >> > > > ovsdb_idl_add_column(ovnsb_idl_loop.idl,
> >> > > > diff --git a/lib/ovn-util.c b/lib/ovn-util.c
> >> > > > index ab5c4fc47..44a1082d4 100644
> >> > > > --- a/lib/ovn-util.c
> >> > > > +++ b/lib/ovn-util.c
> >> > > > @@ -1503,3 +1503,48 @@ put_load(uint64_t value, enum
> >> mf_field_id dst,
> >> > > size_t ofs, size_t n_bits,
> >> > > > ovs_be64 n_value = htonll(value);
> >> > > > put_load_bytes(&n_value, 8, dst, ofs, n_bits,
ofpacts);
> >> > > > }
> >> > > > +
> >> > > > +bool
> >> > > > +datapath_get_nb_uuid_and_type(const struct
> >> sbrec_datapath_binding *sb,
> >> > > > + struct uuid *nb_uuid,
const char
> >> **type)
> >> > > > +{
> >> > > > + if (sb->nb_uuid) {
> >> > > > + /* New style. The UUID and type are direct
columns, so
> >> use
> >> > > those. */
> >> > > > + *nb_uuid = *sb->nb_uuid;
> >> > > > + *type = sb->type;
> >> > > > + return true;
> >> > > > + }
> >> > > > +
> >> > > > + /* Old style. The UUID is stored in external_ids,
and the key
> >> > > > + * corresponds to the datapath type. This only
works with
> >> > > > + * logical switches and logical routers.
> >> > > > + */
> >> > > > + *type = "logical-switch";
> >> > > > + if (smap_get_uuid(&sb->external_ids, *type,
nb_uuid)) {
> >> > > > + return true;
> >> > > > + }
> >> > > > + *type = "logical-router";
> >> > > > + if (smap_get_uuid(&sb->external_ids, *type,
nb_uuid)) {
> >> > > > + return true;
> >> > > > + }
> >> > > > + *type = "";
> >> > > > + *nb_uuid = UUID_ZERO;
> >> > > > + return false;
> >> > > > +}
> >> > > > +
> >> > > > +bool
> >> > > > +datapath_get_nb_uuid(const struct
sbrec_datapath_binding *sb,
> >> > > > + struct uuid *nb_uuid)
> >> > > > +{
> >> > > > + const char *type;
> >> > > > + return datapath_get_nb_uuid_and_type(sb, nb_uuid,
&type);
> >> > > > +}
> >> > > > +
> >> > > > +const char *
> >> > > > +datapath_get_nb_type(const struct
sbrec_datapath_binding *sb)
> >> > > > +{
> >> > > > + const char *type;
> >> > > > + struct uuid nb_uuid;
> >> > > > + datapath_get_nb_uuid_and_type(sb, &nb_uuid, &type);
> >> > > > + return type;
> >> > > > +}
> >> > > > diff --git a/lib/ovn-util.h b/lib/ovn-util.h
> >> > > > index bde40666a..18ebea912 100644
> >> > > > --- a/lib/ovn-util.h
> >> > > > +++ b/lib/ovn-util.h
> >> > > > @@ -546,4 +546,12 @@ void put_load(uint64_t value, enum
> >> mf_field_id dst,
> >> > > size_t ofs, size_t n_bits,
> >> > > > #define _VFUNC(name, n) _VFUNC_(name, n)
> >> > > > #define VFUNC(func, ...) _VFUNC(func,
__NARG__(__VA_ARGS__))
> >> > > (__VA_ARGS__)
> >> > > >
> >> > > > +bool datapath_get_nb_uuid_and_type(const struct
> >> sbrec_datapath_binding
> >> > > *sb,
> >> > > > + struct uuid
*nb_uuid, const
> >> char
> >> > > **type);
> >> > > > +
> >> > > > +bool datapath_get_nb_uuid(const struct
sbrec_datapath_binding
> >> *sb,
> >> > > > + struct uuid *nb_uuid);
> >> > > > +
> >> > > > +const char *datapath_get_nb_type(const struct
> >> sbrec_datapath_binding
> >> > > *sb);
> >> > > > +
> >> > > > #endif /* OVN_UTIL_H */
> >> > > > diff --git a/northd/northd.c b/northd/northd.c
> >> > > > index 2cb69f9aa..241af1d3c 100644
> >> > > > --- a/northd/northd.c
> >> > > > +++ b/northd/northd.c
> >> > > > @@ -608,19 +608,21 @@ ovn_datapath_from_sbrec(const
struct hmap
> >> > > *ls_datapaths,
> >> > > > const struct hmap *lr_datapaths,
> >> > > > const struct
sbrec_datapath_binding *sb)
> >> > > > {
> >> > > > - struct uuid key;
> >> > > > const struct hmap *dps;
> >> > > > + struct uuid key;
> >> > > > + const char *type;
> >> > > > + if (!datapath_get_nb_uuid_and_type(sb, &key, &type)) {
> >> > > > + return NULL;
> >> > > > + }
> >> > > >
> >> > > > - if (smap_get_uuid(&sb->external_ids, "logical-switch",
> >> &key)) {
> >> > > > + if (!strcmp(type, "logical-switch")) {
> >> > > > dps = ls_datapaths;
> >> > > > - } else if (smap_get_uuid(&sb->external_ids, "logical-
> >> router", &key))
> >> > > {
> >> > > > + } else if (!strcmp(type, "logical-router")) {
> >> > > > dps = lr_datapaths;
> >> > > > } else {
> >> > > > return NULL;
> >> > > > }
> >> > > > - if (!dps) {
> >> > > > - return NULL;
> >> > > > - }
> >> > > > +
> >> > > > struct ovn_datapath *od = ovn_datapath_find_(dps,
&key);
> >> > > > if (od && (od->sb == sb)) {
> >> > > > return od;
> >> > > > @@ -768,11 +770,9 @@
store_mcast_info_for_switch_datapath(const
> >> struct
> >> > > sbrec_ip_multicast *sb,
> >> > > > static void
> >> > > > ovn_datapath_update_external_ids(struct ovn_datapath *od)
> >> > > > {
> >> > > > - /* Get the logical-switch or logical-router UUID
to set in
> >> > > > - * external-ids. */
> >> > > > + /* Get the NB UUID to set in external-ids. */
> >> > > > char uuid_s[UUID_LEN + 1];
> >> > > > sprintf(uuid_s, UUID_FMT, UUID_ARGS(&od->key));
> >> > > > - const char *key = od->nbs ? "logical-switch" :
"logical-
> >> router";
> >> > > >
> >> > > > /* Get names to set in external-ids. */
> >> > > > const char *name = od->nbs ? od->nbs->name : od-
>nbr->name;
> >> > > > @@ -784,7 +784,6 @@ ovn_datapath_update_external_ids(struct
> >> ovn_datapath
> >> > > *od)
> >> > > >
> >> > > > /* Set external-ids. */
> >> > > > struct smap ids = SMAP_INITIALIZER(&ids);
> >> > > > - smap_add(&ids, key, uuid_s);
> >> > > > smap_add(&ids, "name", name);
> >> > > > if (name2 && name2[0]) {
> >> > > > smap_add(&ids, "name2", name2);
> >> > > > @@ -912,13 +911,10 @@ join_datapaths(const struct
> >> > > nbrec_logical_switch_table *nbrec_ls_table,
> >> > > > const struct sbrec_datapath_binding *sb;
> >> > > > SBREC_DATAPATH_BINDING_TABLE_FOR_EACH_SAFE (sb,
> >> sbrec_dp_table) {
> >> > > > struct uuid key;
> >> > > > - if (!smap_get_uuid(&sb->external_ids, "logical-
> >> switch", &key) &&
> >> > > > - !smap_get_uuid(&sb->external_ids, "logical-
> >> router", &key)) {
> >> > > > + if (!datapath_get_nb_uuid(sb, &key)) {
> >> > > > ovsdb_idl_txn_add_comment(
> >> > > > ovnsb_txn,
> >> > > > - "deleting Datapath_Binding "UUID_FMT" that
> >> lacks "
> >> > > > - "external-ids:logical-switch and "
> >> > > > - "external-ids:logical-router",
> >> > > > + "deleting Datapath_Binding "UUID_FMT"
that lacks
> >> > > nb_uuid",
> >> > > > UUID_ARGS(&sb->header_.uuid));
> >> > > > sbrec_datapath_binding_delete(sb);
> >> > > > continue;
> >> > > > @@ -928,13 +924,13 @@ join_datapaths(const struct
> >> > > nbrec_logical_switch_table *nbrec_ls_table,
> >> > > > static struct vlog_rate_limit rl =
> >> VLOG_RATE_LIMIT_INIT(5,
> >> > > 1);
> >> > > > VLOG_INFO_RL(
> >> > > > &rl, "deleting Datapath_Binding
"UUID_FMT" with "
> >> > > > - "duplicate external-ids:logical-
switch/router
> >> "UUID_FMT,
> >> > > > - UUID_ARGS(&sb->header_.uuid),
UUID_ARGS(&key));
> >> > > > + "duplicate nb_uuid "UUID_FMT,
> >> > > > + UUID_ARGS(&sb->header_.uuid),
UUID_ARGS(sb-
> >> >nb_uuid));
> >> > > > sbrec_datapath_binding_delete(sb);
> >> > > > continue;
> >> > > > }
> >> > > >
> >> > > > - struct ovn_datapath *od =
> >> ovn_datapath_create(datapaths, &key,
> >> > > > + struct ovn_datapath *od =
ovn_datapath_create(datapaths,
> >> > > sb->nb_uuid,
> >> > > >
NULL,
> >> NULL, sb);
> >> > > > ovs_list_push_back(sb_only, &od->list);
> >> > > > }
> >> > > > @@ -1140,11 +1136,17 @@ build_datapaths(struct
ovsdb_idl_txn
> >> *ovnsb_txn,
> >> > > > sbrec_datapath_binding_set_tunnel_key(od->sb,
> >> > > od->tunnel_key);
> >> > > > }
> >> > > > ovn_datapath_update_external_ids(od);
> >> > > > + sbrec_datapath_binding_set_nb_uuid(od->sb,
&od->key, 1);
> >> > > > + sbrec_datapath_binding_set_type(od->sb, od->nbs ?
> >> > > "logical-switch" :
> >> > > > + "logical-router");
> >> > > > }
> >> > > > LIST_FOR_EACH (od, list, &nb_only) {
> >> > > > od->sb = sbrec_datapath_binding_insert(ovnsb_txn);
> >> > > > ovn_datapath_update_external_ids(od);
> >> > > > sbrec_datapath_binding_set_tunnel_key(od->sb, od-
> >> >tunnel_key);
> >> > > > + sbrec_datapath_binding_set_nb_uuid(od->sb,
&od->key, 1);
> >> > > > + sbrec_datapath_binding_set_type(od->sb, od->nbs ?
> >> > > "logical-switch" :
> >> > > > + "logical-router");
> >> > > > }
> >> > > > ovn_destroy_tnlids(&dp_tnlids);
> >> > > >
> >> > > > diff --git a/northd/northd.h b/northd/northd.h
> >> > > > index 1108793d7..d3b268129 100644
> >> > > > --- a/northd/northd.h
> >> > > > +++ b/northd/northd.h
> >> > > > @@ -351,7 +351,7 @@ DRR_MODES
> >> > > > #undef DRR_MODE
> >> > > >
> >> > > > /* The 'key' comes from nbs->header_.uuid or nbr-
>header_.uuid or
> >> > > > - * sb->external_ids:logical-switch. */
> >> > > > + * sb->nb_uuid. */
> >> > > > struct ovn_datapath {
> >> > > > struct hmap_node key_node; /* Index on 'key'. */
> >> > > > struct uuid key; /* (nbs/nbr)-
>header_.uuid. */
> >> > > > @@ -454,7 +454,8 @@ ovn_datapath_is_stale(const struct
> >> ovn_datapath *od)
> >> > > > /* The two purposes for which ovn-northd uses OVN logical
> >> datapaths. */
> >> > > > enum ovn_datapath_type {
> >> > > > DP_SWITCH, /* OVN logical switch. */
> >> > > > - DP_ROUTER /* OVN logical router. */
> >> > > > + DP_ROUTER, /* OVN logical router. */
> >> > > > + DP_MAX,
> >> > > > };
> >> > > >
> >> > > > /* Returns an "enum ovn_stage" built from the arguments.
> >> > > > diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
> >> > > > index 4c24f5b51..8bc7bbba3 100644
> >> > > > --- a/ovn-sb.ovsschema
> >> > > > +++ b/ovn-sb.ovsschema
> >> > > > @@ -1,7 +1,7 @@
> >> > > > {
> >> > > > "name": "OVN_Southbound",
> >> > > > - "version": "21.2.0",
> >> > > > - "cksum": "29145795 34859",
> >> > > > + "version": "21.3.0",
> >> > > > + "cksum": "3179330761 35286",
> >> > > > "tables": {
> >> > > > "SB_Global": {
> >> > > > "columns": {
> >> > > > @@ -196,6 +196,13 @@
> >> > > > "load_balancers": {"type": {"key":
{"type":
> >> "uuid"},
> >> > > > "min": 0,
> >> > > > "max":
"unlimited"}},
> >> > > > + "type": {"type": {"key": {"type":
"string",
> >> > > > + "enum": ["set",
> >> > > > +
> >> ["logical- switch",
> >> > > > +
> >> > > "logical-router"]]}}},
> >> > > > + "nb_uuid": {"type": {"key": {"type":
"uuid"},
> >> > > > + "min": 0,
> >> > > > + "max": 1}},
> >> > > > "external_ids": {
> >> > > > "type": {"key": "string", "value":
"string",
> >> > > > "min": 0, "max":
"unlimited"}}},
> >> > > > diff --git a/ovn-sb.xml b/ovn-sb.xml
> >> > > > index 395deae83..dcade18fc 100644
> >> > > > --- a/ovn-sb.xml
> >> > > > +++ b/ovn-sb.xml
> >> > > > @@ -3236,18 +3236,19 @@ tcp.flags = RST;
> >> > > > db="OVN_Northbound"/> database.
> >> > > > </p>
> >> > > >
> >> > > > - <column name="external_ids" key="logical-switch"
> >> type='{"type":
> >> > > "uuid"}'>
> >> > > > - For a logical datapath that represents a
logical switch,
> >> > > > - <code>ovn-northd</code> stores in this key the
UUID of
> >> the
> >> > > > - corresponding <ref table="Logical_Switch"
> >> db="OVN_Northbound"/>
> >> > > row in
> >> > > > - the <ref db="OVN_Northbound"/> database.
> >> > > > + <column name="type" type='{"type": "string"}'>
> >> > > > + This represents the type of the northbound logical
> >> datapath that
> >> > > is
> >> > > > + represented by this southbound
> >> <code>Datapath_Binding</ code>. The
> >> > > > + possible values for this are:
> >> > > > + <ul>
> >> > > > + <li><code>logical-switch</code></li>
> >> > > > + <li><code>logical-router</code></li>
> >> > > > + </ul>
> >> > > > </column>
> >> > > >
> >> > > > - <column name="external_ids" key="logical-router"
> >> type='{"type":
> >> > > "uuid"}'>
> >> > > > - For a logical datapath that represents a
logical router,
> >> > > > - <code>ovn-northd</code> stores in this key the
UUID of
> >> the
> >> > > > - corresponding <ref table="Logical_Router"
> >> db="OVN_Northbound"/>
> >> > > row in
> >> > > > - the <ref db="OVN_Northbound"/> database.
> >> > > > + <column name="nb_uuid" type='{"type": "string"}'>
> >> > > > + This is the UUID of the corresponding northbound
> >> logical datapath
> >> > > > + represented by this southbound
> >> <code>Datapath_Binding</ code>.
> >> > > > </column>
> >> > > >
> >> > > > <column name="external_ids" key="interconn-ts"
> >> type='{"type":
> >> > > "string"}'>
> >> > > > diff --git a/tests/ovn-controller.at <http://ovn-
controller.at> <http://ovn-controller.at <http://ovn-controller.at>>
> >> b/tests/ovn-controller.at <http://ovn-controller.at> <http://
ovn-controller.at <http://ovn-controller.at>>
> >> > > > index afdc76a32..95fbcc0be 100644
> >> > > > --- a/tests/ovn-controller.at <http://ovn-
controller.at> <http://ovn-controller.at <http://ovn-controller.at>>
> >> > > > +++ b/tests/ovn-controller.at <http://ovn-
controller.at> <http://ovn-controller.at <http://ovn-controller.at>>
> >> > > > @@ -123,6 +123,7 @@ check_patches
> >> > > > # another logical port bound to this chassis.
> >> > > > check_uuid ovn-sbctl \
> >> > > > -- --id=@dp101 create Datapath_Binding tunnel_key=101
> >> > > external_ids:name=dp101 \
> >> > > > + type=logical-switch \
> >> > > > -- create Port_Binding datapath=@dp101
logical_port=localnet1
> >> > > tunnel_key=1 \
> >> > > > type=localnet options:network_name=physnet1
> >> > > > check_patches
> >> > > > @@ -131,6 +132,7 @@ check_patches
> >> > > > # Now we should get some patch ports created.
> >> > > > check_uuid ovn-sbctl \
> >> > > > -- --id=@dp102 create Datapath_Binding tunnel_key=102
> >> > > external_ids:name=dp102 \
> >> > > > + type=logical-switch \
> >> > > > -- create Port_Binding datapath=@dp102
logical_port=localnet2
> >> > > tunnel_key=1 \
> >> > > > type=localnet options:network_name=physnet1 \
> >> > > > -- create Port_Binding datapath=@dp102
logical_port=localvif2
> >> > > tunnel_key=2
> >> > > > @@ -145,7 +147,9 @@ check_patches \
> >> > > > # the set of OVS patch ports doesn't change.
> >> > > > AT_CHECK([ovn-sbctl \
> >> > > > -- --id=@dp1 create Datapath_Binding tunnel_key=1
> >> > > external_ids:name=dp1 \
> >> > > > + type=logical-switch \
> >> > > > -- --id=@dp2 create Datapath_Binding tunnel_key=2
> >> > > external_ids:name=dp2 \
> >> > > > + type=logical-switch \
> >> > > > -- create Port_Binding datapath=@dp1 logical_port=foo
> >> tunnel_key=1
> >> > > type=patch options:peer=bar \
> >> > > > -- create Port_Binding datapath=@dp2 logical_port=bar
> >> tunnel_key=2
> >> > > type=patch options:peer=foo \
> >> > > > -- create Port_Binding datapath=@dp1
logical_port=dp1vif
> >> > > tunnel_key=3 \
> >> > > > diff --git a/tests/ovn-northd.at <http://ovn-northd.at>
<http://ovn-northd.at <http://ovn-northd.at>> b/
> >> tests/ ovn-northd.at <http://ovn-northd.at> <http://ovn-
northd.at <http://ovn-northd.at>>
> >> > > > index ec7009b28..6bfd09483 100644
> >> > > > --- a/tests/ovn-northd.at <http://ovn-northd.at>
<http://ovn-northd.at <http://ovn-northd.at>>
> >> > > > +++ b/tests/ovn-northd.at <http://ovn-northd.at>
<http://ovn-northd.at <http://ovn-northd.at>>
> >> > > > @@ -1893,8 +1893,8 @@ check ovn-nbctl --wait=sb \
> >> > > >
> >> > > > check_row_count Datapath_Binding 1
> >> > > >
> >> > > > -nb_uuid=$(ovn-sbctl get Datapath_Binding .
> >> external_ids:logical- router)
> >> > > > -lr_uuid=\"$(ovn-nbctl get Logical_Router . _uuid)\"
> >> > > > +nb_uuid=$(ovn-sbctl get Datapath_Binding . nb_uuid)
> >> > > > +lr_uuid=$(ovn-nbctl get Logical_Router . _uuid)
> >> > > > echo nb_uuid="$nb_uuid" lr_uuid="$lr_uuid"
> >> > > > AT_CHECK([test "${nb_uuid}" = "${lr_uuid}"])
> >> > > >
> >> > > > @@ -7595,7 +7595,7 @@ ls1_uuid=$(fetch_column
nb:Logical_Switch
> >> _uuid)
> >> > > >
> >> > > > # create a duplicated sb datapath (and an IP_Mulicast
record that
> >> > > references
> >> > > > # it) on purpose.
> >> > > > -AT_CHECK([ovn-sbctl --id=@dp create Datapath_Binding
> >> > > external_ids:logical-switch=$ls1_uuid external_ids:name=ls1
> >> tunnel_key=123
> >> > > -- create IP_Multicast datapath=@dp], [0], [ignore])
> >> > > > +AT_CHECK([ovn-sbctl --id=@dp create Datapath_Binding
> >> nb_uuid=$ls1_uuid
> >> > > type=logical-switch external_ids:name=ls1 tunnel_key=123
-- create
> >> > > IP_Multicast datapath=@dp], [0], [ignore])
> >> > > >
> >> > > > # northd should delete one of the datapaths in the end
> >> > > > wait_row_count Datapath_Binding 1
> >> > > > diff --git a/tests/ovn.at <http://ovn.at> <http://
ovn.at <http://ovn.at>> b/tests/ovn.at <http://ovn.at>
> >> <http:// ovn.at <http://ovn.at>>
> >> > > > index fadc62a76..bba57d9b4 100644
> >> > > > --- a/tests/ovn.at <http://ovn.at> <http://ovn.at
<http://ovn.at>>
> >> > > > +++ b/tests/ovn.at <http://ovn.at> <http://ovn.at
<http://ovn.at>>
> >> > > > @@ -22232,8 +22232,7 @@ AT_CAPTURE_FILE([sbflows])
> >> > > > AT_CHECK([as hv1 ovs-ofctl dump-flows br-int \
> >> > > > | grep "check_pkt_larger" | wc -l], [0], [[0
> >> > > > ]])
> >> > > > -dp_uuid=$(ovn-sbctl find datapath_binding | grep lr0 -B2 |
> >> grep _uuid | \
> >> > > > -awk '{print $3}')
> >> > > > +dp_uuid=$(fetch_column Datapath_Binding _uuid external-
> >> ids:name=lr0)
> >> > > > check_uuid ovn-sbctl create MAC_Binding ip=172.168.0.3
> >> datapath=$dp_uuid
> >> > > \
> >> > > > logical_port=lr0-public mac="00\:00\:00\:12\:af\:11"
> >> > > >
> >> > > > @@ -22303,8 +22302,7 @@ check ovn-nbctl lr-nat-add lr1 snat
> >> 172.168.0.100
> >> > > 10.0.0.0/24 <http://10.0.0.0/24> <http://10.0.0.0/24
<http://10.0.0.0/24>>
> >> > > > check ovn-nbctl lr-nat-add lr1 snat 2000::1 1000::/64
> >> > > > check ovn-nbctl --wait=sb sync
> >> > > >
> >> > > > -dp_uuid=$(ovn-sbctl find datapath_binding | grep lr1 -B2 |
> >> grep _uuid | \
> >> > > > -awk '{print $3}')
> >> > > > +dp_uuid=$(fetch_column Datapath_Binding _uuid external-
> >> ids:name=lr1)
> >> > > > check_uuid ovn-sbctl create MAC_Binding ip=172.168.0.3
> >> datapath=$dp_uuid
> >> > > \
> >> > > > logical_port=lr1-public mac="00\:00\:00\:12\:af\:11"
> >> > > >
> >> > > > diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c
> >> > > > index 60cc39149..74f759699 100644
> >> > > > --- a/utilities/ovn-sbctl.c
> >> > > > +++ b/utilities/ovn-sbctl.c
> >> > > > @@ -382,6 +382,7 @@ pre_get_info(struct ctl_context *ctx)
> >> > > > ovsdb_idl_add_column(ctx->idl,
> >> > > &sbrec_logical_dp_group_col_datapaths);
> >> > > >
> >> > > > ovsdb_idl_add_column(ctx->idl,
> >> > > &sbrec_datapath_binding_col_external_ids);
> >> > > > + ovsdb_idl_add_column(ctx->idl,
> >> &sbrec_datapath_binding_col_nb_uuid);
> >> > > >
> >> > > > ovsdb_idl_add_column(ctx->idl,
> >> &sbrec_ip_multicast_col_datapath);
> >> > > > ovsdb_idl_add_column(ctx->idl,
> >> &sbrec_ip_multicast_col_seq_no);
> >> > > > @@ -1493,8 +1494,7 @@ static const struct ctl_table_class
> >> > > tables[SBREC_N_TABLES] = {
> >> > > > [SBREC_TABLE_DATAPATH_BINDING].row_ids
> >> > > > = {{&sbrec_datapath_binding_col_external_ids,
"name", NULL},
> >> > > > {&sbrec_datapath_binding_col_external_ids,
"name2",
> >> NULL},
> >> > > > - {&sbrec_datapath_binding_col_external_ids,
"logical-
> >> switch",
> >> > > NULL},
> >> > > > - {&sbrec_datapath_binding_col_external_ids,
"logical-
> >> router",
> >> > > NULL}},
> >> > > > + {&sbrec_datapath_binding_col_nb_uuid, NULL,
NULL}},
> >> > > >
> >> > > > [SBREC_TABLE_PORT_BINDING].row_ids
> >> > > > = {{&sbrec_port_binding_col_logical_port, NULL,
NULL},
> >> > > > diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c
> >> > > > index a63a3be19..9ce29f096 100644
> >> > > > --- a/utilities/ovn-trace.c
> >> > > > +++ b/utilities/ovn-trace.c
> >> > > > @@ -703,8 +703,7 @@ read_datapaths(void)
> >> > > > const struct smap *ids = &sbdb->external_ids;
> >> > > >
> >> > > > dp->sb_uuid = sbdb->header_.uuid;
> >> > > > - if (!smap_get_uuid(ids, "logical-switch", &dp-
> >> >nb_uuid) &&
> >> > > > - !smap_get_uuid(ids, "logical-router", &dp-
> >> >nb_uuid)) {
> >> > > > + if (!datapath_get_nb_uuid(sbdb, &dp->nb_uuid)) {
> >> > > > dp->nb_uuid = dp->sb_uuid;
> >> > > > }
> >> > > >
> >> > > > --
> >> > > > 2.49.0
> >> > > >
> >> > > > _______________________________________________
> >> > > > dev mailing list
> >> > > > d...@openvswitch.org <mailto:d...@openvswitch.org>
<mailto:d...@openvswitch.org <mailto:d...@openvswitch.org>>
> >> > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
<https://mail.openvswitch.org/mailman/listinfo/ovs-dev> <https://
> >> mail.openvswitch.org/mailman/listinfo/ovs-dev <http://
mail.openvswitch.org/mailman/listinfo/ovs-dev>>
> >> > > _______________________________________________
> >> > > dev mailing list
> >> > > d...@openvswitch.org <mailto:d...@openvswitch.org>
<mailto:d...@openvswitch.org <mailto:d...@openvswitch.org>>
> >> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
<https://mail.openvswitch.org/mailman/listinfo/ovs-dev> <https://
> >> mail.openvswitch.org/mailman/listinfo/ovs-dev <http://
mail.openvswitch.org/mailman/listinfo/ovs-dev>>
> >
> > _______________________________________________
> > dev mailing list
> > d...@openvswitch.org <mailto:d...@openvswitch.org>
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev <https://
mail.openvswitch.org/mailman/listinfo/ovs-dev>
