On 2/11/25 9:36 AM, Felix Huettner via dev wrote:
> Previously vrf names where generated using "ovnvrf" + datapath id.
> Now the controller supports the dynamic-routing-vrf-name setting to
> freely configure the vrf name.
> Note that the vrf ID is still the datapath id.
>
> Acked-by: Dumitru Ceara <[email protected]>
> Signed-off-by: Felix Huettner <[email protected]>
> ---
Hi Felix,
I applied this patch to main with the following minor style changes:
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 8b95c305b7..d24e509744 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -16199,8 +16199,9 @@ blackhole 198.51.100.0/24 proto 84 metric 1000
# Changing the vrf name will switch to the new one.
# The old vrf will be removed.
check ovn-nbctl --wait=hv set Logical_Router_Port internet-phys \
- options:dynamic-routing-maintain-vrf=true
-check ovn-nbctl --wait=hv set Logical_Router internet
options:dynamic-routing-vrf-name=ovnvrf1338
+ options:dynamic-routing-maintain-vrf=true
+check ovn-nbctl --wait=hv set Logical_Router internet \
+ options:dynamic-routing-vrf-name=ovnvrf1338
AT_CHECK([ip vrf | grep -q ovnvrf1337], [1], [])
OVS_WAIT_UNTIL_EQUAL([ip route list vrf ovnvrf1338 | awk '{$1=$1};1'], [dnl
blackhole 192.0.2.1 proto 84 metric 1000
--
Regards,
Dumitru
> v5->v6:
> * addressed review comments
>
> controller/route-exchange.c | 14 ++++++--------
> controller/route.c | 16 ++++++++++++++++
> controller/route.h | 2 ++
> tests/system-ovn.at | 26 ++++++++++++++++++++------
> 4 files changed, 44 insertions(+), 14 deletions(-)
>
> diff --git a/controller/route-exchange.c b/controller/route-exchange.c
> index 3c49c60c0..2e5a5cfa1 100644
> --- a/controller/route-exchange.c
> +++ b/controller/route-exchange.c
> @@ -218,27 +218,25 @@ route_exchange_run(struct route_exchange_ctx_in
> *r_ctx_in,
> const struct advertise_datapath_entry *ad;
> HMAP_FOR_EACH (ad, node, r_ctx_in->announce_routes) {
> uint32_t table_id = ad->db->tunnel_key;
> - char vrf_name[IFNAMSIZ + 1];
> - snprintf(vrf_name, sizeof vrf_name, "ovnvrf%"PRIi32, table_id);
>
> if (ad->maintain_vrf) {
> - if (!sset_contains(&old_maintained_vrfs, vrf_name)) {
> - int error = re_nl_create_vrf(vrf_name, table_id);
> + if (!sset_contains(&old_maintained_vrfs, ad->vrf_name)) {
> + int error = re_nl_create_vrf(ad->vrf_name, table_id);
> if (error && error != EEXIST) {
> VLOG_WARN_RL(&rl,
> "Unable to create VRF %s for datapath "
> "%"PRIi32": %s.",
> - vrf_name, table_id,
> + ad->vrf_name, table_id,
> ovs_strerror(error));
> continue;
> }
> }
> - sset_add(&_maintained_vrfs, vrf_name);
> + sset_add(&_maintained_vrfs, ad->vrf_name);
> } else {
> /* A previous maintain-vrf flag was removed. We should therefore
> * also not delete it even if we created it previously. */
> - sset_find_and_delete(&_maintained_vrfs, vrf_name);
> - sset_find_and_delete(&old_maintained_vrfs, vrf_name);
> + sset_find_and_delete(&_maintained_vrfs, ad->vrf_name);
> + sset_find_and_delete(&old_maintained_vrfs, ad->vrf_name);
> }
>
> maintained_route_table_add(table_id);
> diff --git a/controller/route.c b/controller/route.c
> index ed99cbe5a..bfd288503 100644
> --- a/controller/route.c
> +++ b/controller/route.c
> @@ -183,6 +183,22 @@ route_run(struct route_ctx_in *r_ctx_in,
> ad->maintain_vrf |= smap_get_bool(
> &repb->options, "dynamic-routing-maintain-vrf", false);
>
> + const char *vrf_name = smap_get(&repb->options,
> + "dynamic-routing-vrf-name");
> + if (vrf_name && strlen(vrf_name) >= IFNAMSIZ) {
> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5,
> 20);
> + VLOG_WARN_RL(&rl, "Ignoring vrf name %s, since it is too
> long."
> + "Maximum length is %d characters", vrf_name,
> + IFNAMSIZ);
> + vrf_name = NULL;
> + }
> + 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);
> + }
> +
> const char *port_name = smap_get(&repb->options,
> "dynamic-routing-port-name");
> if (!port_name) {
> diff --git a/controller/route.h b/controller/route.h
> index fe87a0d28..02e028c94 100644
> --- a/controller/route.h
> +++ b/controller/route.h
> @@ -20,6 +20,7 @@
>
> #include <stdbool.h>
> #include <netinet/in.h>
> +#include <net/if.h>
> #include "openvswitch/hmap.h"
> #include "sset.h"
> #include "smap.h"
> @@ -58,6 +59,7 @@ struct advertise_datapath_entry {
> struct hmap_node node;
> const struct sbrec_datapath_binding *db;
> bool maintain_vrf;
> + char vrf_name[IFNAMSIZ + 1];
> struct hmap routes;
>
> /* The name of the port bindings locally bound for this datapath and
> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> index 62fd77cdb..fca046b35 100644
> --- a/tests/system-ovn.at
> +++ b/tests/system-ovn.at
> @@ -15921,6 +15921,7 @@ OVN_FOR_EACH_NORTHD([
> AT_SETUP([dynamic-routing - DGP])
>
> VRF_RESERVE([1337])
> +VRF_RESERVE([1338])
>
> # This test uses dynamic routing on a simulated multi-tenant internet
> # connection.
> @@ -16195,10 +16196,25 @@ blackhole 198.51.100.0/24 proto 84 metric 1000
> 233.252.0.0/24 via 192.168.10.10 dev lo onlink
> 233.253.0.0/24 via 192.168.20.20 dev hv1-mll onlink])
>
> +# Changing the vrf name will switch to the new one.
> +# The old vrf will be removed.
> +check ovn-nbctl --wait=hv set Logical_Router_Port internet-phys \
> + options:dynamic-routing-maintain-vrf=true
> +check ovn-nbctl --wait=hv set Logical_Router internet
> options:dynamic-routing-vrf-name=ovnvrf1338
> +AT_CHECK([ip vrf | grep -q ovnvrf1337], [1], [])
> +OVS_WAIT_UNTIL_EQUAL([ip route list vrf ovnvrf1338 | awk '{$1=$1};1'], [dnl
> +blackhole 192.0.2.1 proto 84 metric 1000
> +blackhole 192.0.2.2 proto 84 metric 100
> +blackhole 192.0.2.3 proto 84 metric 100
> +blackhole 192.0.2.10 proto 84 metric 100
> +blackhole 198.51.100.0/24 proto 84 metric 1000
> +233.252.0.0/24 via 192.168.10.10 dev lo onlink
> +233.253.0.0/24 via 192.168.20.20 dev hv1-mll onlink])
> +
> # Stoping with --restart will not touch the routes.
> check ovn-appctl -t ovn-controller exit --restart
> OVS_WAIT_UNTIL([test "$(ovn-appctl -t ovn-controller debug/status)" !=
> "running"])
> -OVS_WAIT_UNTIL_EQUAL([ip route list vrf ovnvrf1337 | awk '{$1=$1};1'], [dnl
> +OVS_WAIT_UNTIL_EQUAL([ip route list vrf ovnvrf1338 | awk '{$1=$1};1'], [dnl
> blackhole 192.0.2.1 proto 84 metric 1000
> blackhole 192.0.2.2 proto 84 metric 100
> blackhole 192.0.2.3 proto 84 metric 100
> @@ -16207,14 +16223,12 @@ blackhole 198.51.100.0/24 proto 84 metric 1000
> 233.252.0.0/24 via 192.168.10.10 dev lo onlink
> 233.253.0.0/24 via 192.168.20.20 dev hv1-mll onlink])
>
> -# Now we set maintain-vrf again and stop the ovn-controller.
> -# It will then remove the VRF.
> +# When we now stop the ovn-controller it will remove the VRF.
> start_daemon ovn-controller
> OVS_WAIT_UNTIL([test "$(ovn-appctl -t ovn-controller debug/status)" ==
> "running"])
> -check ovn-nbctl --wait=hv set Logical_Router_Port internet-phys \
> - options:dynamic-routing-maintain-vrf=true
> +check ovn-nbctl --wait=hv sync
> OVS_APP_EXIT_AND_WAIT([ovn-controller])
> -AT_CHECK([ip vrf | grep -q ovnvrf1337], [1], [])
> +AT_CHECK([ip vrf | grep -q ovnvrf1338], [1], [])
>
> as ovn-sb
> OVS_APP_EXIT_AND_WAIT([ovsdb-server])
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
