On Mon, Nov 3, 2025 at 2:02 PM Felix Huettner <[email protected]>
wrote:
> On Thu, Oct 30, 2025 at 03:26:22PM +0100, Dumitru Ceara wrote:
> > On 10/30/25 10:37 AM, Ales Musil wrote:
> > > Add an LR option, called "dynamic-routing-vrf-id", that allows CMS to
> > > specify vrf table_id. The table_id will be used instead of tunnel_key
> > > for given datapath corresponding to the LR. This also allows CMS to
> > > share one vrf between multiple LRs.
> > >
> > > Fixes: cb7ba9e9057c ("controller: Support user defined vrf names.")
> > > Fixes: faf4df563f1d ("controller: Announce routes via route-exchange.")
> > > Reported-at: https://issues.redhat.com/browse/FDP-1886
> > > Signed-off-by: Ales Musil <[email protected]>
> > > ---
> > > v2: Add warning and skip duplicate table_id.
> > > Remove the extra import.
> > > ---
> >
> > Hi Ales,
> >
> > Thanks for v2! I have a few nits below but those can be addressed
> > without a v3 when merging the patch. It might be nice if Felix has
> > another look at this version too.
> >
> > With the small nits taken care of:
> >
> > Acked-by: Dumitru Ceara <[email protected]>
>
> Hi Alex, Hi Dumitru,
>
> all of the 3 patches look good to me as well:
>
> Acked-by: Felix Huettner <[email protected]>
>
> Thanks a lot,
> Felix
>
>
> >
> > Regards,
> > Dumitru
> >
> > > NEWS | 2 ++
> > > controller/route-exchange.c | 17 +++++++++++++----
> > > controller/route.c | 12 ++++++++++--
> > > controller/route.h | 2 ++
> > > northd/en-datapath-logical-router.c | 7 +++++++
> > > ovn-nb.xml | 29 +++++++++++++++++++++++++++--
> > > tests/multinode-bgp-macros.at | 2 +-
> > > tests/system-ovn.at | 22 +++++++++++-----------
> > > 8 files changed, 73 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/NEWS b/NEWS
> > > index 06cc18b41..1e2abfa1c 100644
> > > --- a/NEWS
> > > +++ b/NEWS
> > > @@ -17,6 +17,8 @@ Post v25.09.0
> > > * Add the "other_config:dynamic-routing-redistribute=ip" to
> Logical
> > > Switches to announce local IPs/MAC bindings belonging to the
> same
> > > EVPN domain.
> > > + * Add the option "dynamic-routing-vrf-id" to Logical Routers
> which allows
> > > + CMS to specify the table_id for given vrf.
> >
> > Nit: s/table_id for given vrf/Linux routing table id for a given vrf/
> >
> > > - Add support for Network Function insertion in OVN with stateful
> traffic
> > > redirection capability in Logical Switch datapath. The feature
> introduces
> > > three new NB database tables:
> > > diff --git a/controller/route-exchange.c b/controller/route-exchange.c
> > > index 161385aa4..829d1b73f 100644
> > > --- a/controller/route-exchange.c
> > > +++ b/controller/route-exchange.c
> > > @@ -232,6 +232,7 @@ void
> > > route_exchange_run(const struct route_exchange_ctx_in *r_ctx_in,
> > > struct route_exchange_ctx_out *r_ctx_out)
> > > {
> > > + struct hmap table_ids = HMAP_INITIALIZER(&table_ids);
> > > struct sset old_maintained_vrfs =
> SSET_INITIALIZER(&old_maintained_vrfs);
> > > sset_swap(&_maintained_vrfs, &old_maintained_vrfs);
> > > struct hmap old_maintained_route_table =
> > > @@ -242,7 +243,14 @@ route_exchange_run(const struct
> route_exchange_ctx_in *r_ctx_in,
> > > CLEAR_ROUTE_EXCHANGE_NL_STATUS();
> > > const struct advertise_datapath_entry *ad;
> > > HMAP_FOR_EACH (ad, node, r_ctx_in->announce_routes) {
> > > - uint32_t table_id = ad->db->tunnel_key;
> > > + uint32_t table_id = route_get_table_id(ad->db);
> > > +
> > > + if (!ovn_add_tnlid(&table_ids, table_id)) {
> > > + VLOG_WARN_RL(&rl, "Unable to sync routes for datapath
> "UUID_FMT": "
> > > + "duplicate table id: %"PRIu32,
> > > + UUID_ARGS(&ad->db->header_.uuid), table_id);
> > > + continue;
> > > + }
> > >
> > > if (ad->maintain_vrf) {
> > > if (!sset_contains(&old_maintained_vrfs, ad->vrf_name)) {
> > > @@ -250,8 +258,8 @@ route_exchange_run(const struct
> route_exchange_ctx_in *r_ctx_in,
> > > if (error && error != EEXIST) {
> > > VLOG_WARN_RL(&rl,
> > > "Unable to create VRF %s for
> datapath "
> > > - "%"PRIi32": %s.",
> > > - ad->vrf_name, table_id,
> > > + UUID_FMT": %s.", ad->vrf_name,
> > > + UUID_ARGS(&ad->db->header_.uuid),
> > > ovs_strerror(error));
> > > SET_ROUTE_EXCHANGE_NL_STATUS(error);
> > > continue;
> > > @@ -270,7 +278,7 @@ route_exchange_run(const struct
> route_exchange_ctx_in *r_ctx_in,
> > > struct vector received_routes =
> > > VECTOR_EMPTY_INITIALIZER(struct
> re_nl_received_route_node);
> > >
> > > - error = re_nl_sync_routes(ad->db->tunnel_key, &ad->routes,
> > > + error = re_nl_sync_routes(table_id, &ad->routes,
> > > &received_routes, ad->db);
> > > SET_ROUTE_EXCHANGE_NL_STATUS(error);
> > >
> > > @@ -315,6 +323,7 @@ route_exchange_run(const struct
> route_exchange_ctx_in *r_ctx_in,
> > > sset_delete(&old_maintained_vrfs,
> SSET_NODE_FROM_NAME(vrf_name));
> > > }
> > > sset_destroy(&old_maintained_vrfs);
> > > + ovn_destroy_tnlids(&table_ids);
> > > }
> > >
> > > void
> > > diff --git a/controller/route.c b/controller/route.c
> > > index db4aa4122..7b0f3a872 100644
> > > --- a/controller/route.c
> > > +++ b/controller/route.c
> > > @@ -218,8 +218,8 @@ route_run(struct route_ctx_in *r_ctx_in,
> > > if (vrf_name) {
> > > memcpy(ad->vrf_name, vrf_name, strlen(vrf_name) + 1);
> > > } else {
> > > - snprintf(ad->vrf_name, sizeof ad->vrf_name,
> "ovnvrf%"PRIi64,
> > > - ad->db->tunnel_key);
> > > + snprintf(ad->vrf_name, sizeof ad->vrf_name,
> "ovnvrf%"PRIu32,
> > > + route_get_table_id(ad->db));
> > > }
> > >
> > > if (!port_name) {
> > > @@ -323,3 +323,11 @@ route_cleanup(struct hmap *announce_routes)
> > > advertise_datapath_cleanup(ad);
> > > }
> > > }
> > > +
> > > +uint32_t
> > > +route_get_table_id(const struct sbrec_datapath_binding *dp)
> > > +{
> > > + int64_t vrf_id = ovn_smap_get_llong(&dp->external_ids,
> > > + "dynamic-routing-vrf-id", -1);
> > > + return (vrf_id >= 1 && vrf_id <= UINT32_MAX) ? vrf_id :
> dp->tunnel_key;
> > > +}
> > > diff --git a/controller/route.h b/controller/route.h
> > > index c2c92e70b..f6cb5993c 100644
> > > --- a/controller/route.h
> > > +++ b/controller/route.h
> > > @@ -29,6 +29,7 @@ struct hmap;
> > > struct ovsdb_idl_index;
> > > struct sbrec_chassis;
> > > struct sbrec_port_binding;
> > > +struct sbrec_datapath_binding;
> > >
> > > struct route_ctx_in {
> > > const struct sbrec_advertised_route_table *advertised_route_table;
> > > @@ -85,5 +86,6 @@ const struct sbrec_port_binding
> *route_exchange_find_port(
> > > uint32_t advertise_route_hash(const struct in6_addr *dst, unsigned
> int plen);
> > > void route_run(struct route_ctx_in *, struct route_ctx_out *);
> > > void route_cleanup(struct hmap *announce_routes);
> > > +uint32_t route_get_table_id(const struct sbrec_datapath_binding *dp);
> >
> > Nit: 'dp' not needed.
> >
> > >
> > > #endif /* ROUTE_H */
> > > diff --git a/northd/en-datapath-logical-router.c
> b/northd/en-datapath-logical-router.c
> > > index f6f49422f..4625edfd0 100644
> > > --- a/northd/en-datapath-logical-router.c
> > > +++ b/northd/en-datapath-logical-router.c
> > > @@ -88,6 +88,13 @@ gather_external_ids(const struct
> nbrec_logical_router *nbr,
> > > smap_add_format(external_ids, "disable_garp_rarp",
> > > disable_garp_rarp ? "true" : "false");
> > >
> > > + int64_t vrf_id = ovn_smap_get_llong(&nbr->options,
> > > + "dynamic-routing-vrf-id", -1);
> > > + if (vrf_id > 0) {
> > > + smap_add_format(external_ids, "dynamic-routing-vrf-id",
> "%"PRId64,
> > > + vrf_id);
> > > + }
> > > +
> > > /* For backwards-compatibility, also store the NB UUID in
> > > * external-ids:logical-router. This is useful if ovn-controller
> > > * has not updated and expects this to be where to find the
> > > diff --git a/ovn-nb.xml b/ovn-nb.xml
> > > index 36d7accaf..f4b47f213 100644
> > > --- a/ovn-nb.xml
> > > +++ b/ovn-nb.xml
> > > @@ -3383,7 +3383,9 @@ or
> > > <p>
> > > This defines the name of the vrf the ovn-controller will
> use to
> > > advertise and learn routes. If not set the vrf will be
> named "ovnvrf"
> > > - with the datapath id of the Logical Router appended to it.
> > > + with the <ref column="options" key="dynamic-routing-vrf-id"
> > > + table="Logical_Router"/> or datapath id of the Logical
> Router
> > > + appended to it.
> > > </p>
> > >
> > > <p>
> > > @@ -3398,6 +3400,27 @@ or
> > > </p>
> > > </column>
> > >
> > > + <column name="options" key="dynamic-routing-vrf-id"
> > > + type='{"type": "integer", "minInteger": 1,
> > > + "maxInteger": 4294967295}'>
> > > + <p>
> > > + Only relevant if <ref column="options"
> key="dynamic-routing"/>
> > > + is set to <code>true</code>.
> > > + </p>
> > > +
> > > + <p>
> > > + This defines the vrf table id used by the specified vrf.
> When not
> > > + set or set to invalid value datapath id of the Logical
> Router
> > > + will be used instead.
> > > + </p>
> > > +
> > > + <p>
> > > + This can also affect the vrf name. For details see
> > > + <ref column="options" key="dynamic-routing-vrf-name"
> > > + table="Logical_Router"/> on the Logical_Router.
> > > + </p>
> > > + </column>
> > > +
> > > <column name="options" key="ct-commit-all" type='{"type":
> "boolean"}'>
> > > When enabled the LR will commit traffic in a zone that is
> stateful.
> > > The traffic is not commited to both zones but it is selective
> based
> > > @@ -4416,7 +4439,9 @@ or
> > > this LRP.
> > > Unless <ref column="options" key="dynamic-routing-vrf-name"
> > > table="Logical_Router"/> is set the vrf will be named
> "ovnvrf" with
> > > - the datapath id of the Logical Router appended to it.
> > > + the <ref column="options" key="dynamic-routing-vrf-id"
> > > + table="Logical_Router"/> or datapath id of the Logical
> Router
> > > + appended to it.
> > > </p>
> > >
> > > <p>
> > > diff --git a/tests/multinode-bgp-macros.at b/tests/
> multinode-bgp-macros.at
> > > index fb0906005..2ea9136c7 100644
> > > --- a/tests/multinode-bgp-macros.at
> > > +++ b/tests/multinode-bgp-macros.at
> > > @@ -275,7 +275,7 @@ m_setup_ovn_frr_router() {
> > >
> > > check multinode_nbctl lr-add $lr
> > > check multinode_nbctl set Logical_Router $lr
> options:chassis=$node \
> > > - options:dynamic-routing=true options:requested-tnl-key=$vni
> > > + options:dynamic-routing=true
> options:dynamic-routing-vrf-id=$vni
> > >
> > > check multinode_nbctl lrp-add $lr $lrp $bgp_mac
> > > check multinode_nbctl lrp-set-options $lrp \
> > > diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> > > index 2b880ec37..98dcc60e9 100644
> > > --- a/tests/system-ovn.at
> > > +++ b/tests/system-ovn.at
> > > @@ -15821,7 +15821,7 @@ check ovn-nbctl ls-add p2
> > >
> > > check ovn-nbctl lr-add internet \
> > > -- set Logical_Router internet options:dynamic-routing=true \
> > > - options:requested-tnl-key=1337
> > > + options:dynamic-routing-vrf-id=1337
> > >
> > > check ovn-nbctl lrp-add internet internet-public \
> > > 00:00:02:01:02:03 192.0.2.1/24 \
> > > @@ -15845,7 +15845,7 @@ check ovn-nbctl lsp-add phys phys-internet \
> > > # LR pr1 setup.
> > >
> > > check ovn-nbctl lr-add pr1 \
> > > - -- set Logical_Router pr1 options:requested-tnl-key=1338
> > > + -- set Logical_Router pr1 options:dynamic-routing-vrf-id=1338
> > >
> > > check ovn-nbctl lrp-add pr1 pr1-public \
> > > 00:00:02:01:02:04 192.0.2.2/24
> > > @@ -15867,7 +15867,7 @@ check ovn-nbctl lr-route-add pr1 0.0.0.0/0
> 192.0.2.1
> > > # LR pr2 setup.
> > >
> > > check ovn-nbctl lr-add pr2 \
> > > - -- set Logical_Router pr2 options:requested-tnl-key=1339
> > > + -- set Logical_Router pr2 options:dynamic-routing-vrf-id=1339
> > >
> > > check ovn-nbctl lrp-add pr2 pr2-public \
> > > 00:00:02:01:02:05 192.0.2.3/24
> > > @@ -16177,7 +16177,7 @@ check ovn-nbctl ls-add p2
> > >
> > > check ovn-nbctl lr-add internet \
> > > -- set Logical_Router internet options:dynamic-routing=true \
> > > - options:requested-tnl-key=1337 \
> > > +
> options:dynamic-routing-vrf-id=1337 \
> > > options:chassis=hv1
> > >
> > > check ovn-nbctl lrp-add internet internet-public \
> > > @@ -16201,7 +16201,7 @@ check ovn-nbctl lsp-add phys phys-internet \
> > > # LR pr1 setup.
> > >
> > > check ovn-nbctl lr-add pr1 \
> > > - -- set Logical_Router pr1 options:requested-tnl-key=1338
> > > + -- set Logical_Router pr1 options:dynamic-routing-vrf-id=1338
> > >
> > > check ovn-nbctl lrp-add pr1 pr1-public \
> > > 00:00:02:01:02:04 192.0.2.2/24
> > > @@ -16223,7 +16223,7 @@ check ovn-nbctl lr-route-add pr1 0.0.0.0/0
> 192.0.2.1
> > > # LR pr2 setup.
> > >
> > > check ovn-nbctl lr-add pr2 \
> > > - -- set Logical_Router pr2 options:requested-tnl-key=1339
> > > + -- set Logical_Router pr2 options:dynamic-routing-vrf-id=1339
> > >
> > > check ovn-nbctl lrp-add pr2 pr2-public \
> > > 00:00:02:01:02:05 192.0.2.3/24
> > > @@ -16484,7 +16484,7 @@ OVS_WAIT_WHILE([ip link | grep -q
> ovnvrf$vrf:.*UP])
> > > check ovn-nbctl \
> > > -- lr-add R1 \
> > > -- set Logical_Router R1 \
> > > - options:requested-tnl-key=$vrf \
> > > + options:dynamic-routing-vrf-id=$vrf \
> > > options:dynamic-routing=true \
> > > -- ls-add sw0 \
> > > -- ls-add public \
> > > @@ -16642,7 +16642,7 @@ OVS_WAIT_WHILE([ip link | grep -q
> ovnvrf$vrf:.*UP])
> > > check ovn-nbctl \
> > > -- lr-add R1 \
> > > -- set Logical_Router R1 \
> > > - options:requested-tnl-key=$vrf \
> > > + options:dynamic-routing-vrf-id=$vrf \
> > > options:dynamic-routing=true \
> > > -- ls-add sw0 \
> > > -- ls-add public \
> > > @@ -16802,7 +16802,7 @@ OVS_WAIT_WHILE([ip link | grep -q
> ovnvrf$vrf:.*UP])
> > > check ovn-nbctl \
> > > -- lr-add R1 \
> > > -- set Logical_Router R1 \
> > > - options:requested-tnl-key=$vrf \
> > > + options:dynamic-routing-vrf-id=$vrf \
> > > options:dynamic-routing=true \
> > > -- ls-add sw0 \
> > > -- ls-add public \
> > > @@ -17032,7 +17032,7 @@ OVS_WAIT_WHILE([ip link | grep -q
> ovnvrf$vrf:.*UP])
> > > check ovn-nbctl \
> > > -- lr-add R1 \
> > > -- set Logical_Router R1 \
> > > - options:requested-tnl-key=$vrf \
> > > + options:dynamic-routing-vrf-id=$vrf \
> > > options:dynamic-routing=true \
> > > -- ls-add sw0 \
> > > -- ls-add public \
> > > @@ -17297,7 +17297,7 @@ check ovn-nbctl
> \
> > > check ovn-nbctl \
> > > -- lr-add R2 \
> > > -- set Logical_Router R2 \
> > > - options:requested-tnl-key=$vrf \
> > > + options:dynamic-routing-vrf-id=$vrf \
> > > options:dynamic-routing=true \
> > > options:dynamic-routing-redistribute=lb,nat \
> > > -- lrp-add R2 R2-PUB1 00:02:02:01:02:03 172.16.1.2/24 \
> >
>
>
Thank you Dumitru and Felix,
I have addressed the nits, went ahead, merged this into and backported all
the way down to 25.03.
Regards,
Ales
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
