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]>

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 \

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to