On 12/17/25 8:45 PM, Mark Michelson via dev wrote:
> Thanks for the patch and test, Lucas!
>
> Acked-by: Mark Michelson <[email protected]>
>
> On Tue, Dec 16, 2025 at 11:42 AM Lucas Vargas Dias via dev
> <[email protected]> wrote:
>>
>> If ovn-controller stop or fail, it will not exist more
>> learned route. So, it must be deleted.
>>
>> Signed-off-by: Lucas Vargas Dias <[email protected]>
>> ---
Hi Lucas, Mark,
Thanks for the patch and review!
I think there were some leftover comments from Ales' review on the
previous version (v0), related to using wait/check_row_count in tests.
>> northd/en-learned-route-sync.c | 3 +-
>> northd/northd.h | 25 +++++
>> tests/system-ovn.at | 181 +++++++++++++++++++++++++++++++++
>> 3 files changed, 207 insertions(+), 2 deletions(-)
>>
>> diff --git a/northd/en-learned-route-sync.c b/northd/en-learned-route-sync.c
>> index f22aaa664..753d77b0f 100644
>> --- a/northd/en-learned-route-sync.c
>> +++ b/northd/en-learned-route-sync.c
>> @@ -222,8 +222,7 @@ routes_table_sync(
>> sbrec_learned_route_table) {
>> struct ovn_port *op =
>> ovn_port_find(lr_ports, sb_route->logical_port->logical_port);
>> - if (!op || op->sb != sb_route->logical_port || !op->od ||
>> - !op->od->dynamic_routing) {
>> + if (!ovn_port_must_learn_route(op, sb_route)) {
>> sbrec_learned_route_delete(sb_route);
>> continue;
>> }
>> diff --git a/northd/northd.h b/northd/northd.h
>> index 2869ea97e..ed19f7a21 100644
>> --- a/northd/northd.h
>> +++ b/northd/northd.h
>> @@ -1132,6 +1132,31 @@ lrp_is_l3dgw(const struct ovn_port *op)
>> (op->nbrp->n_gateway_chassis || op->nbrp->ha_chassis_group);
>> }
>>
>> +/* This function returns false if 'op' is NULL, sb port binding different
>> + * from logical port from learned route, datapath has dynamic routing
>> disabled
>> + * or if port is lrp l3dgw with no chassis.
>> + * True otherwise.
>> + */
>> +static inline bool
>> +ovn_port_must_learn_route (const struct ovn_port *op,
Incorrect indentation, no need to add space between function name
and parenthesis.
>> + const struct sbrec_learned_route *sb_route)
>> +{
>> + if (!op) {
>> + return false;
>> + }
>> + if (op->sb != sb_route->logical_port) {
>> + return false;
>> + }
>> + if (!op->od || !op->od->dynamic_routing) {
>> + return false;
>> + }
>> + if (lrp_is_l3dgw(op) && (!op->cr_port->sb ||
>> + !op->cr_port->sb->chassis)) {
This fits on one line.
>> + return false;
>> + }
>> + return true;
>> +}
>> +
>> struct ovn_port *ovn_port_find(const struct hmap *ports, const char *name);
>>
>> void build_igmp_lflows(struct hmap *igmp_groups,
>> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
>> index b60dcae3e..cd192b1e7 100644
>> --- a/tests/system-ovn.at
>> +++ b/tests/system-ovn.at
>> @@ -18999,3 +18999,184 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port
>> patch-.*/d
>> /Couldn't parse IPv6 prefix nexthop.*/d"])
>> AT_CLEANUP
>> ])
>> +
>> +OVN_FOR_EACH_NORTHD([
>> +AT_SETUP([dynamic-routing - BGP learned routes and removing them when
>> ovn-controller stops])
>> +
>> +# This test validates that BGP learned routes work correctly:
>> +# 1. Routes added to the VRF appear in Learned_Route table
>> +# 2. Stopping ovn-controller remove learned routes
>> +#
>> +# Topology:
>> +# +---------+
>> +# | public |
>> +# +----+----+
>> +# |
>> +# +----+---+
>> +# | lr-frr | (in VRF 10)
>> +# +----+---+
>> +# |
>> +# +------+-------+
>> +# |local-bgp-port| (in VRF 10)
>> +# +--------------+
>> +
>> +ovn_start
>> +OVS_TRAFFIC_VSWITCHD_START()
>> +ADD_BR([br-int])
>> +ADD_BR([br-ex])
>> +
>> +check ovs-ofctl add-flow br-ex action=normal
>> +
>> +# Set external-ids in br-int needed for ovn-controller
>> +check ovs-vsctl \
>> + -- set Open_vSwitch . external-ids:system-id=hv1 \
>> + -- set Open_vSwitch .
>> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
>> + -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
>> + -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
>> + -- set bridge br-int fail-mode=secure other-config:disable-in-band=true
>> +
>> +# Configure bridge mappings for localnet
>> +check ovs-vsctl set Open_vSwitch .
>> external-ids:ovn-bridge-mappings=phys:br-ex
>> +
>> +id=10
>> +
>> +# Start ovn-controller
>> +start_daemon ovn-controller
>> +
>> +check ip link add vrf-$id type vrf table $id
>> +on_exit "ip link del vrf-$id"
>> +check ip link set vrf-$id up
>> +
>> +# Create public logical switch with localnet port
>> +check ovn-nbctl ls-add public
>> +check ovn-nbctl lsp-add-localnet-port public ln_port phys
>> +
>> +# Create lr-frr with dynamic routing in VRF $id
>> +check ovn-nbctl lr-add lr-frr \
>> + -- set Logical_Router lr-frr \
>> + options:dynamic-routing=true \
>> + options:dynamic-routing-vrf-id=$id \
>> + options:dynamic-routing-maintain-vrf=false \
>> + options:dynamic-routing-redistribute=static
>> +
>> +check ovn-nbctl lrp-add lr-frr lrp-local-bgp-port 00:00:00:00:00:03
>> 20.0.0.3/8
>> +check ovn-nbctl lrp-set-gateway-chassis lrp-local-bgp-port hv1
>> +check ovn-nbctl lsp-add-router-port public public-lr-frr lrp-local-bgp-port
>> +
>> +# Create local-bgp-port in VRF 10
>> +check ovs-vsctl add-port br-int local-bgp-port \
>> + -- set Interface local-bgp-port type=internal \
>> + -- set Interface local-bgp-port external_ids:iface-id=local-bgp-port
>> +
>> +check ovn-nbctl lsp-add public local-bgp-port \
>> + -- lsp-set-addresses local-bgp-port "00:00:00:00:00:03 20.0.0.3"
>> +
>> +# Configure local-bgp-port interface and add to VRF
>> +AT_CHECK([ip link set local-bgp-port master vrf-$id])
>> +AT_CHECK([ip link set local-bgp-port address 00:00:00:00:00:03])
>> +AT_CHECK([ip addr add dev local-bgp-port 20.0.0.3/8])
>> +AT_CHECK([ip link set local-bgp-port up])
>> +
>> +# Wait for everything to be ready
>> +wait_for_ports_up
>> +check ovn-nbctl --wait=hv sync
>> +
>> +# Check lrp-local-bgp-port has dynamic-routing option set.
>> +check_row_count Port_Binding 1 logical_port=cr-lrp-local-bgp-port
>> 'options:dynamic-routing=true'
>> +
>> +
>> +# Add static routes
>> +check ovn-nbctl lr-route-add lr-frr 10.10.2.1 20.0.0.42 lrp-local-bgp-port
>> +
>> +# Verify advertised routes exist
>> +AS_BOX([Advertised_Route])
>> +OVS_WAIT_UNTIL([ovn-sbctl list Advertised_Route | grep ip_prefix | grep -Fe
>> 10.10.2.1])
>> +
>> +# Add a route to the VRF (simulating BGP learning a route)
>> +AT_CHECK([ip route add 10.10.3.1 via 20.0.0.25 vrf vrf-$id proto zebra])
>> +
>> +# Verify learned route appears in SB database
>> +OVS_WAIT_UNTIL([ovn-sbctl list Learned_Route | grep ip_prefix | grep -Fe
>> 10.10.3.1])
>> +
>> +# Add a second route to the VRF (simulating BGP learning a route)
>> +AT_CHECK([ip route add 10.10.4.1 via 20.0.0.25 vrf vrf-$id proto zebra])
>> +
>> +# Verify both routes appear in SB database.
>> +OVS_WAIT_FOR_OUTPUT([ovn-sbctl list Learned_Route | grep ip_prefix | sort],
>> [0], [dnl
>> +ip_prefix : "10.10.3.1"
>> +ip_prefix : "10.10.4.1"
>> +])
>> +
>> +# Remove one route.
>> +AS_BOX([$(date +%H:%M:%S.%03N) Remove first route])
>> +AT_CHECK([ip route del 10.10.4.1 via 20.0.0.25 vrf vrf-$id proto zebra])
>> +
>> +# Verify only one route remains in SB database.
>> +OVS_WAIT_FOR_OUTPUT([ovn-sbctl list Learned_Route | grep ip_prefix | sort],
>> [0], [dnl
>> +ip_prefix : "10.10.3.1"
>> +])
>> +
>> +# Remove second route.
>> +AS_BOX([$(date +%H:%M:%S.%03N) Remove second route])
>> +AT_CHECK([ip route del 10.10.3.1 via 20.0.0.25 vrf vrf-$id])
>> +
>> +# Verify all routes removed from SB database.
>> +OVS_WAIT_FOR_OUTPUT([ovn-sbctl list Learned_Route | grep ip_prefix | sort],
>> [0], [dnl
>> +])
>> +
>> +# Add again a route to the VRF (simulating BGP learning a route)
>> +AT_CHECK([ip route add 10.10.3.1 via 20.0.0.25 vrf vrf-$id proto zebra])
>> +
>> +# Verify learned route appears in SB database
>> +OVS_WAIT_UNTIL([ovn-sbctl list Learned_Route | grep ip_prefix | grep -Fe
>> 10.10.3.1])
>> +
>> +# Disable dynamic routing
>> +AS_BOX([$(date +%H:%M:%S.%03N) Disable dynamic-routing])
>> +check ovn-nbctl --wait=sb set Logical_Router lr-frr
>> options:dynamic-routing=false
>> +check_row_count Port_Binding 0 logical_port=cr-lrp-local-bgp-port
>> 'options:dynamic-routing=true'
>> +
>> +# Add one more route to the VRF (simulating BGP learning a route)
>> +AT_CHECK([ip route add 10.10.4.1 via 20.0.0.25 vrf vrf-$id proto zebra])
>> +
>> +# Verify learned routes are removed as dynamic-routing=false
>> +OVS_WAIT_FOR_OUTPUT([ovn-sbctl list Learned_Route | grep ip_prefix | sort],
>> [0], [dnl
>> +])
>> +
>> +# Re-enable dynamic routing
>> +check ovn-nbctl --wait=sb set Logical_Router lr-frr
>> options:dynamic-routing=true
>> +check_row_count Port_Binding 1 logical_port=cr-lrp-local-bgp-port
>> 'options:dynamic-routing=true'
>> +
>> +# Verify both routes appear in SB database.
>> +OVS_WAIT_FOR_OUTPUT([ovn-sbctl list Learned_Route | grep ip_prefix | sort],
>> [0], [dnl
>> +ip_prefix : "10.10.3.1"
>> +ip_prefix : "10.10.4.1"
>> +])
>> +
>> +
>> +# Stop ovn-controller
>> +OVN_CLEANUP_CONTROLLER([hv1], [], [], [lr-frr])
>> +check ovn-nbctl --wait=sb sync
>> +
>> +# Verify routes are removed in SB database.
>> +OVS_WAIT_FOR_OUTPUT([ovn-sbctl list Learned_Route | grep ip_prefix | sort],
>> [0], [dnl
>> +])
>> +
>> +# Cleanup VRF route
>> +AT_CHECK([ip route del 10.10.3.1 via 20.0.0.25 vrf vrf-$id])
>> +AT_CHECK([ip route del 10.10.4.1 via 20.0.0.25 vrf vrf-$id])
>> +
>> +as ovn-sb
>> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>> +
>> +as ovn-nb
>> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>> +
>> +as northd
>> +OVS_APP_EXIT_AND_WAIT([ovn-northd])
>> +
>> +as
>> +OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d
>> +/failed to query port patch-.*/d
>> +/.*terminating with signal 15.*/d"])
>> +AT_CLEANUP
>> +])
>> --
>> 2.43.0
>>
I took care of all the minor issues and replaced some more bare
AT_CHECK() calls. I squashed in the following incremental diff
and then applied the patch to main, 25.09 and 25.03.
Regards,
Dumitru
diff --git a/northd/northd.h b/northd/northd.h
index ed19f7a212..87dab77fdd 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -1138,8 +1138,8 @@ lrp_is_l3dgw(const struct ovn_port *op)
* True otherwise.
*/
static inline bool
-ovn_port_must_learn_route (const struct ovn_port *op,
- const struct sbrec_learned_route *sb_route)
+ovn_port_must_learn_route(const struct ovn_port *op,
+ const struct sbrec_learned_route *sb_route)
{
if (!op) {
return false;
@@ -1150,8 +1150,7 @@ ovn_port_must_learn_route (const struct ovn_port *op,
if (!op->od || !op->od->dynamic_routing) {
return false;
}
- if (lrp_is_l3dgw(op) && (!op->cr_port->sb ||
- !op->cr_port->sb->chassis)) {
+ if (lrp_is_l3dgw(op) && (!op->cr_port->sb || !op->cr_port->sb->chassis)) {
return false;
}
return true;
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 16559c53b0..212e409827 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -19665,10 +19665,10 @@ check ovn-nbctl lsp-add public local-bgp-port \
-- lsp-set-addresses local-bgp-port "00:00:00:00:00:03 20.0.0.3"
# Configure local-bgp-port interface and add to VRF
-AT_CHECK([ip link set local-bgp-port master vrf-$id])
-AT_CHECK([ip link set local-bgp-port address 00:00:00:00:00:03])
-AT_CHECK([ip addr add dev local-bgp-port 20.0.0.3/8])
-AT_CHECK([ip link set local-bgp-port up])
+check ip link set local-bgp-port master vrf-$id
+check ip link set local-bgp-port address 00:00:00:00:00:03
+check ip addr add dev local-bgp-port 20.0.0.3/8
+check ip link set local-bgp-port up
# Wait for everything to be ready
wait_for_ports_up
@@ -19677,51 +19677,45 @@ check ovn-nbctl --wait=hv sync
# Check lrp-local-bgp-port has dynamic-routing option set.
check_row_count Port_Binding 1 logical_port=cr-lrp-local-bgp-port
'options:dynamic-routing=true'
-
# Add static routes
check ovn-nbctl lr-route-add lr-frr 10.10.2.1 20.0.0.42 lrp-local-bgp-port
# Verify advertised routes exist
AS_BOX([Advertised_Route])
-OVS_WAIT_UNTIL([ovn-sbctl list Advertised_Route | grep ip_prefix | grep -Fe
10.10.2.1])
+wait_row_count Advertised_Route 1 ip_prefix=10.10.2.1
# Add a route to the VRF (simulating BGP learning a route)
-AT_CHECK([ip route add 10.10.3.1 via 20.0.0.25 vrf vrf-$id proto zebra])
+check ip route add 10.10.3.1 via 20.0.0.25 vrf vrf-$id proto zebra
# Verify learned route appears in SB database
-OVS_WAIT_UNTIL([ovn-sbctl list Learned_Route | grep ip_prefix | grep -Fe
10.10.3.1])
+wait_row_count Learned_Route 1 ip_prefix=10.10.3.1
# Add a second route to the VRF (simulating BGP learning a route)
-AT_CHECK([ip route add 10.10.4.1 via 20.0.0.25 vrf vrf-$id proto zebra])
+ip route add 10.10.4.1 via 20.0.0.25 vrf vrf-$id proto zebra
# Verify both routes appear in SB database.
-OVS_WAIT_FOR_OUTPUT([ovn-sbctl list Learned_Route | grep ip_prefix | sort],
[0], [dnl
-ip_prefix : "10.10.3.1"
-ip_prefix : "10.10.4.1"
-])
+wait_row_count Learned_Route 1 ip_prefix=10.10.4.1
# Remove one route.
AS_BOX([$(date +%H:%M:%S.%03N) Remove first route])
-AT_CHECK([ip route del 10.10.4.1 via 20.0.0.25 vrf vrf-$id proto zebra])
+check ip route del 10.10.4.1 via 20.0.0.25 vrf vrf-$id proto zebra
# Verify only one route remains in SB database.
-OVS_WAIT_FOR_OUTPUT([ovn-sbctl list Learned_Route | grep ip_prefix | sort],
[0], [dnl
-ip_prefix : "10.10.3.1"
-])
+wait_row_count Learned_Route 1
+check_row_count Learned_Route 1 ip_prefix=10.10.3.1
# Remove second route.
AS_BOX([$(date +%H:%M:%S.%03N) Remove second route])
-AT_CHECK([ip route del 10.10.3.1 via 20.0.0.25 vrf vrf-$id])
+check ip route del 10.10.3.1 via 20.0.0.25 vrf vrf-$id
# Verify all routes removed from SB database.
-OVS_WAIT_FOR_OUTPUT([ovn-sbctl list Learned_Route | grep ip_prefix | sort],
[0], [dnl
-])
+wait_row_count Learned_Route 0
# Add again a route to the VRF (simulating BGP learning a route)
-AT_CHECK([ip route add 10.10.3.1 via 20.0.0.25 vrf vrf-$id proto zebra])
+check ip route add 10.10.3.1 via 20.0.0.25 vrf vrf-$id proto zebra
# Verify learned route appears in SB database
-OVS_WAIT_UNTIL([ovn-sbctl list Learned_Route | grep ip_prefix | grep -Fe
10.10.3.1])
+wait_row_count Learned_Route 1 ip_prefix=10.10.3.1
# Disable dynamic routing
AS_BOX([$(date +%H:%M:%S.%03N) Disable dynamic-routing])
@@ -19729,34 +19723,26 @@ check ovn-nbctl --wait=sb set Logical_Router lr-frr
options:dynamic-routing=fals
check_row_count Port_Binding 0 logical_port=cr-lrp-local-bgp-port
'options:dynamic-routing=true'
# Add one more route to the VRF (simulating BGP learning a route)
-AT_CHECK([ip route add 10.10.4.1 via 20.0.0.25 vrf vrf-$id proto zebra])
+check ip route add 10.10.4.1 via 20.0.0.25 vrf vrf-$id proto zebra
# Verify learned routes are removed as dynamic-routing=false
-OVS_WAIT_FOR_OUTPUT([ovn-sbctl list Learned_Route | grep ip_prefix | sort],
[0], [dnl
-])
+wait_row_count Learned_Route 0
# Re-enable dynamic routing
check ovn-nbctl --wait=sb set Logical_Router lr-frr
options:dynamic-routing=true
check_row_count Port_Binding 1 logical_port=cr-lrp-local-bgp-port
'options:dynamic-routing=true'
# Verify both routes appear in SB database.
-OVS_WAIT_FOR_OUTPUT([ovn-sbctl list Learned_Route | grep ip_prefix | sort],
[0], [dnl
-ip_prefix : "10.10.3.1"
-ip_prefix : "10.10.4.1"
-])
-
+wait_row_count Learned_Route 1 ip_prefix=10.10.3.1
+wait_row_count Learned_Route 1 ip_prefix=10.10.4.1
+check_row_count Learned_Route 2
# Stop ovn-controller
OVN_CLEANUP_CONTROLLER([hv1], [], [], [lr-frr])
check ovn-nbctl --wait=sb sync
# Verify routes are removed in SB database.
-OVS_WAIT_FOR_OUTPUT([ovn-sbctl list Learned_Route | grep ip_prefix | sort],
[0], [dnl
-])
-
-# Cleanup VRF route
-AT_CHECK([ip route del 10.10.3.1 via 20.0.0.25 vrf vrf-$id])
-AT_CHECK([ip route del 10.10.4.1 via 20.0.0.25 vrf vrf-$id])
+wait_row_count Learned_Route 0
-as ovn-sb
-OVS_APP_EXIT_AND_WAIT([ovsdb-server])
-
-as ovn-nb
-OVS_APP_EXIT_AND_WAIT([ovsdb-server])
-
-as northd
-OVS_APP_EXIT_AND_WAIT([ovn-northd])
+OVN_CLEANUP_NORTHD
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
