On 12/5/25 7:20 PM, Lucas Vargas Dias via dev wrote:
> Learned routes must have the nexthop reachable. So, if logical router
> has two ports in differents subnets, route must be learned in just
> one which is the port in the same subnet from nexthop.
>
> Signed-off-by: Lucas Vargas Dias <[email protected]>
> ---
Hi Lucas,
Thanks for the patch!
I'm not sure I understand the problem though, please see below.
> northd/en-learned-route-sync.c | 44 ++++++++++++++++++++
> tests/ovn-northd.at | 14 ++++++-
> tests/system-ovn.at | 74 +++++++++++++++++-----------------
> 3 files changed, 93 insertions(+), 39 deletions(-)
>
> diff --git a/northd/en-learned-route-sync.c b/northd/en-learned-route-sync.c
> index f22aaa664..4c9de8651 100644
> --- a/northd/en-learned-route-sync.c
> +++ b/northd/en-learned-route-sync.c
> @@ -227,6 +227,50 @@ routes_table_sync(
> sbrec_learned_route_delete(sb_route);
> continue;
> }
> + bool is_same_subnet = false;
> + for (size_t i = 0; !is_same_subnet &&
> + i < sb_route->logical_port->n_mac;
> + i++) {
> + struct lport_addresses logical_port_addrs;
> + if (!extract_lsp_addresses(sb_route->logical_port->mac[i],
> + &logical_port_addrs)) {
> + destroy_lport_addresses(&logical_port_addrs);
> + continue;
> + }
> + ovs_be32 neigh_prefix_v4;
> + struct in6_addr neigh_prefix_v6;
> +
> + if (ip_parse(sb_route->nexthop, &neigh_prefix_v4)) {
> + for (size_t j = 0; j < logical_port_addrs.n_ipv4_addrs;
> + j++) {
> + struct ipv4_netaddr address =
> + logical_port_addrs.ipv4_addrs[j];
> + if (address.network ==
> + (neigh_prefix_v4 & address.mask)) {
> + is_same_subnet = true;
> + break;
> + }
> + }
> + } else if (ipv6_parse(sb_route->nexthop, &neigh_prefix_v6)) {
> + for (size_t j = 0; j < logical_port_addrs.n_ipv6_addrs; j++)
> {
> + struct ipv6_netaddr address =
> + logical_port_addrs.ipv6_addrs[j];
> + struct in6_addr neigh_prefix =
> + ipv6_addr_bitand(&neigh_prefix_v6, &address.mask);
> + if (ipv6_addr_equals(&address.network, &neigh_prefix)) {
> + is_same_subnet = true;
> + break;
> + }
> + }
> + }
> + destroy_lport_addresses(&logical_port_addrs);
> + }
> +
> +
> + if (!is_same_subnet) {
> + sbrec_learned_route_delete(sb_route);
> + continue;
> + }
> parse_route_from_sbrec_route(parsed_routes_out, lr_ports,
> &lr_datapaths->datapaths,
> sb_route);
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 864854c56..0d1d06196 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -15578,7 +15578,7 @@ AT_CHECK([grep -w "lr_in_ip_routing" lr0flows |
> ovn_strip_lflows], [0], [dnl
> ])
>
> # Learn a route to 2001:db8:2::/64 via 2001:db8:ffff::20 learned on lr0-sw1.
> -# This is not reachable so will not produce a lflow.
> +# This is not reachable so will not produce a lflow. Also, it'll be removed
> by northd
As the comment used to say, while the route is learned, ovn-northd
doesn't actually produce logical flows for it because the next hop is
not reachable on any network configured on lr0-sw1.
So yes, the route is in SB.Learned_Route, but it doesn't affect routing
in any way. It's exactly the same as configuring a NB Static_Route with
a wrong next hop.
Moreover, your code removes the Learned_Route in northd. But
Learned_Routes are fully managed (created & deleted) by ovn-controller.
So next time ovn-controller runs it will just relearn the route
(re-create the SB.Learned_Route record). So this only creates
unnecessary churn as far as I understand.
Or is there a use case I missed?
Regards,
Dumitru
> check_uuid ovn-sbctl create Learned_Route \
> datapath=$datapath \
> logical_port=$sw1 \
> @@ -15586,7 +15586,9 @@ check_uuid ovn-sbctl create Learned_Route \
> nexthop=\"2001:db8:ffff::20\"
> check ovn-nbctl --wait=sb sync
> check_row_count Advertised_Route 4
> -check_row_count Learned_Route 2
> +check_row_count Learned_Route 1
> +check_row_count Learned_Route 0 logical_port=$sw1
> ip_prefix=\"2001:db8:2::/64\"
> +
> ovn-sbctl dump-flows lr0 > lr0flows
> AT_CHECK([grep -w "lr_in_ip_routing" lr0flows | ovn_strip_lflows], [0], [dnl
> table=??(lr_in_ip_routing ), priority=0 , match=(1), action=(drop;)
> @@ -15605,6 +15607,14 @@ AT_CHECK([grep -w "lr_in_ip_routing" lr0flows |
> ovn_strip_lflows], [0], [dnl
> # active.
> check ovn-nbctl --wait=sb set Logical_Router_Port lr0-sw1 \
> networks="\"2001:db8::1/64\" \"2001:db8:ffff::1/64\""
> +
> +# Learn a route to 2001:db8:2::/64 via 2001:db8:ffff::20 learned on lr0-sw1.
> +# Northd doesn not remove now because lrp have address in same subnet from
> nexthop
> +check_uuid ovn-sbctl create Learned_Route \
> + datapath=$datapath \
> + logical_port=$sw1 \
> + ip_prefix=\"2001:db8:2::/64\" \
> + nexthop=\"2001:db8:ffff::20\"
> check_row_count Advertised_Route 5
> check_row_count Learned_Route 2
> ovn-sbctl dump-flows lr0 > lr0flows
> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> index 1cbbdfa58..6e87fb266 100644
> --- a/tests/system-ovn.at
> +++ b/tests/system-ovn.at
> @@ -15404,10 +15404,10 @@ wait_for_ports_up mylearninglsp
> check ovn-nbctl --wait=hv set Logical_Router_Port internet-phys \
> options:dynamic-routing-port-name=mylearninglsp
>
> -check ip route add 233.253.0.0/24 via 192.168.20.20 dev hv1-mll onlink vrf
> ovnvrf1337 metric 30 proto zebra
> -check ip route add 233.253.0.0/24 via 192.168.20.20 dev hv1-mll onlink vrf
> ovnvrf1337 metric 40 proto zebra
> +check ip route add 233.253.0.0/24 via 192.168.10.20 dev hv1-mll onlink vrf
> ovnvrf1337 metric 30 proto zebra
> +check ip route add 233.253.0.0/24 via 192.168.10.20 dev hv1-mll onlink vrf
> ovnvrf1337 metric 40 proto zebra
> check ovn-nbctl --wait=hv sync
> -check_row_count Learned_Route 1 ip_prefix=233.253.0.0/24
> nexthop=192.168.20.20
> +check_row_count Learned_Route 1 ip_prefix=233.253.0.0/24
> nexthop=192.168.10.20
>
> # Stopping the ovn-controller will clean up the route entries created by it.
> # We first need to unset dynamic-routing-maintain-vrf as otherwise it will
> @@ -15417,8 +15417,8 @@ check ovn-nbctl --wait=hv set Logical_Router_Port
> internet-phys \
> OVN_CLEANUP_CONTROLLER([hv1])
> OVN_ROUTE_EQUAL([ovnvrf1337], [dnl
> 233.252.0.0/24 via 192.168.10.10 dev lo proto zebra onlink
> -233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 30 onlink
> -233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 40 onlink])
> +233.253.0.0/24 via 192.168.10.20 dev hv1-mll proto zebra metric 30 onlink
> +233.253.0.0/24 via 192.168.10.20 dev hv1-mll proto zebra metric 40 onlink])
>
> # Starting it again will add the routes again.
> start_daemon ovn-controller
> @@ -15431,8 +15431,8 @@ blackhole 192.0.2.10 proto ovn metric 100
> blackhole 192.0.2.20 proto ovn metric 100
> blackhole 198.51.100.0/24 proto ovn metric 1000
> 233.252.0.0/24 via 192.168.10.10 dev lo proto zebra onlink
> -233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 30 onlink
> -233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 40 onlink])
> +233.253.0.0/24 via 192.168.10.20 dev hv1-mll proto zebra metric 30 onlink
> +233.253.0.0/24 via 192.168.10.20 dev hv1-mll proto zebra metric 40 onlink])
>
> # Changing the vrf name will switch to the new one.
> # The old vrf will be removed.
> @@ -15449,8 +15449,8 @@ blackhole 192.0.2.10 proto ovn metric 100
> blackhole 192.0.2.20 proto ovn metric 100
> blackhole 198.51.100.0/24 proto ovn metric 1000
> 233.252.0.0/24 via 192.168.10.10 dev lo proto zebra onlink
> -233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 30 onlink
> -233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 40 onlink])
> +233.253.0.0/24 via 192.168.10.20 dev hv1-mll proto zebra metric 30 onlink
> +233.253.0.0/24 via 192.168.10.20 dev hv1-mll proto zebra metric 40 onlink])
>
> # Stopping with --restart will not touch the routes.
> OVN_CONTROLLER_EXIT([],[--restart])
> @@ -15462,8 +15462,8 @@ blackhole 192.0.2.10 proto ovn metric 100
> blackhole 192.0.2.20 proto ovn metric 100
> blackhole 198.51.100.0/24 proto ovn metric 1000
> 233.252.0.0/24 via 192.168.10.10 dev lo proto zebra onlink
> -233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 30 onlink
> -233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 40 onlink])
> +233.253.0.0/24 via 192.168.10.20 dev hv1-mll proto zebra metric 30 onlink
> +233.253.0.0/24 via 192.168.10.20 dev hv1-mll proto zebra metric 40 onlink])
>
> # When we now stop the ovn-controller it will remove the VRF.
> start_daemon ovn-controller
> @@ -15494,8 +15494,8 @@ blackhole 192.0.2.20 proto ovn metric 100
> blackhole 192.0.2.21 proto ovn metric 100
> blackhole 198.51.100.0/24 proto ovn metric 1000
> 233.252.0.0/24 via 192.168.10.10 dev lo proto zebra onlink
> -233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 30 onlink
> -233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 40 onlink])
> +233.253.0.0/24 via 192.168.10.20 dev hv1-mll proto zebra metric 30 onlink
> +233.253.0.0/24 via 192.168.10.20 dev hv1-mll proto zebra metric 40 onlink])
>
> # Bind "vip" port locally and check the virtual IP is added in the VRF.
> NS_EXEC([vif4], [arping -U -c 1 -w 2 -I vif4 192.0.2.30])
> @@ -15511,8 +15511,8 @@ blackhole 192.0.2.21 proto ovn metric 100
> blackhole 192.0.2.30 proto ovn metric 100
> blackhole 198.51.100.0/24 proto ovn metric 1000
> 233.252.0.0/24 via 192.168.10.10 dev lo proto zebra onlink
> -233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 30 onlink
> -233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 40 onlink])
> +233.253.0.0/24 via 192.168.10.20 dev hv1-mll proto zebra metric 30 onlink
> +233.253.0.0/24 via 192.168.10.20 dev hv1-mll proto zebra metric 40 onlink])
>
> check ovn-sbctl clear Port_Binding vip virtual-parent
> OVN_ROUTE_EQUAL([ovnvrf1338], [dnl
> @@ -15524,8 +15524,8 @@ blackhole 192.0.2.20 proto ovn metric 100
> blackhole 192.0.2.21 proto ovn metric 100
> blackhole 198.51.100.0/24 proto ovn metric 1000
> 233.252.0.0/24 via 192.168.10.10 dev lo proto zebra onlink
> -233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 30 onlink
> -233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 40 onlink])
> +233.253.0.0/24 via 192.168.10.20 dev hv1-mll proto zebra metric 30 onlink
> +233.253.0.0/24 via 192.168.10.20 dev hv1-mll proto zebra metric 40 onlink])
>
> # Remove the backoff period, so we can bind it right away.
> check ovn-sbctl remove Port_Binding vip options vport-backoff
> @@ -15543,8 +15543,8 @@ blackhole 192.0.2.21 proto ovn metric 100
> blackhole 192.0.2.30 proto ovn metric 100
> blackhole 198.51.100.0/24 proto ovn metric 1000
> 233.252.0.0/24 via 192.168.10.10 dev lo proto zebra onlink
> -233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 30 onlink
> -233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 40 onlink])
> +233.253.0.0/24 via 192.168.10.20 dev hv1-mll proto zebra metric 30 onlink
> +233.253.0.0/24 via 192.168.10.20 dev hv1-mll proto zebra metric 40 onlink])
>
> # Simulate "vip" bound to a different chassis.
> check ovn-sbctl clear Port_Binding vip virtual-parent
> @@ -15559,8 +15559,8 @@ blackhole 192.0.2.20 proto ovn metric 100
> blackhole 192.0.2.21 proto ovn metric 100
> blackhole 198.51.100.0/24 proto ovn metric 1000
> 233.252.0.0/24 via 192.168.10.10 dev lo proto zebra onlink
> -233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 30 onlink
> -233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 40 onlink])
> +233.253.0.0/24 via 192.168.10.20 dev hv1-mll proto zebra metric 30 onlink
> +233.253.0.0/24 via 192.168.10.20 dev hv1-mll proto zebra metric 40 onlink])
>
> # Check with dynamic-routing-redistribute-local-only=false.
> check ovn-nbctl --wait=hv set logical_router_port internet-public \
> @@ -15576,8 +15576,8 @@ blackhole 192.0.2.22 proto ovn metric 1000
> blackhole 192.0.2.30 proto ovn metric 1000
> blackhole 198.51.100.0/24 proto ovn metric 1000
> 233.252.0.0/24 via 192.168.10.10 dev lo proto zebra onlink
> -233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 30 onlink
> -233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 40 onlink])
> +233.253.0.0/24 via 192.168.10.20 dev hv1-mll proto zebra metric 30 onlink
> +233.253.0.0/24 via 192.168.10.20 dev hv1-mll proto zebra metric 40 onlink])
>
> # Remove the backoff period, so we can bind it right away.
> check ovn-sbctl remove Port_Binding vip options vport-backoff
> @@ -15596,8 +15596,8 @@ blackhole 192.0.2.22 proto ovn metric 1000
> blackhole 192.0.2.30 proto ovn metric 100
> blackhole 198.51.100.0/24 proto ovn metric 1000
> 233.252.0.0/24 via 192.168.10.10 dev lo proto zebra onlink
> -233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 30 onlink
> -233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 40 onlink])
> +233.253.0.0/24 via 192.168.10.20 dev hv1-mll proto zebra metric 30 onlink
> +233.253.0.0/24 via 192.168.10.20 dev hv1-mll proto zebra metric 40 onlink])
>
> OVN_CLEANUP_CONTROLLER([hv1])
> AT_CHECK([ip vrf | grep -q ovnvrf1338], [1], [])
> @@ -15857,7 +15857,7 @@ check ip route add 233.253.0.0/24 via 192.168.10.10
> dev lo onlink vrf ovnvrf1337
> check ovn-nbctl --wait=hv sync
> # With a Gateway Router all LRPs are locally bound, and without explicit
> # mapping/filtering they will all learn the route.
> -check_row_count Learned_Route 2
> +check_row_count Learned_Route 1
> lp=$(fetch_column port_binding _uuid logical_port=internet-phys)
> check_row_count Learned_Route 1 logical_port=$lp ip_prefix=233.252.0.0/24
> nexthop=192.168.10.10
>
> @@ -15892,10 +15892,10 @@ wait_for_ports_up mylearninglsp
> check ovn-nbctl --wait=hv set Logical_Router_Port internet-phys \
> options:dynamic-routing-port-name=mylearninglsp
>
> -check ip route add 233.253.0.0/24 via 192.168.20.20 dev hv1-mll onlink vrf
> ovnvrf1337 metric 30 proto zebra
> -check ip route add 233.253.0.0/24 via 192.168.20.20 dev hv1-mll onlink vrf
> ovnvrf1337 metric 40 proto zebra
> +check ip route add 233.253.0.0/24 via 192.168.10.20 dev hv1-mll onlink vrf
> ovnvrf1337 metric 30 proto zebra
> +check ip route add 233.253.0.0/24 via 192.168.10.20 dev hv1-mll onlink vrf
> ovnvrf1337 metric 40 proto zebra
> check ovn-nbctl --wait=hv sync
> -check_row_count Learned_Route 1 ip_prefix=233.253.0.0/24
> nexthop=192.168.20.20
> +check_row_count Learned_Route 1 ip_prefix=233.253.0.0/24
> nexthop=192.168.10.20
>
> # Stopping the ovn-controller will clean up the route entries created by it.
> # We first need to unset dynamic-routing-maintain-vrf as otherwise it will
> @@ -15905,8 +15905,8 @@ check ovn-nbctl --wait=hv set Logical_Router_Port
> internet-phys \
> OVN_CLEANUP_CONTROLLER([hv1])
> OVN_ROUTE_EQUAL([ovnvrf1337], [dnl
> 233.252.0.0/24 via 192.168.10.10 dev lo proto zebra onlink
> -233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 30 onlink
> -233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 40 onlink])
> +233.253.0.0/24 via 192.168.10.20 dev hv1-mll proto zebra metric 30 onlink
> +233.253.0.0/24 via 192.168.10.20 dev hv1-mll proto zebra metric 40 onlink])
>
> # Starting it again will add the routes again.
> start_daemon ovn-controller
> @@ -15919,8 +15919,8 @@ blackhole 192.0.2.10 proto ovn metric 100
> blackhole 192.0.2.20 proto ovn metric 100
> blackhole 198.51.100.0/24 proto ovn metric 1000
> 233.252.0.0/24 via 192.168.10.10 dev lo proto zebra onlink
> -233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 30 onlink
> -233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 40 onlink])
> +233.253.0.0/24 via 192.168.10.20 dev hv1-mll proto zebra metric 30 onlink
> +233.253.0.0/24 via 192.168.10.20 dev hv1-mll proto zebra metric 40 onlink])
>
> # Stopping with --restart will not touch the routes.
> OVN_CONTROLLER_EXIT([],[--restart])
> @@ -15932,8 +15932,8 @@ blackhole 192.0.2.10 proto ovn metric 100
> blackhole 192.0.2.20 proto ovn metric 100
> blackhole 198.51.100.0/24 proto ovn metric 1000
> 233.252.0.0/24 via 192.168.10.10 dev lo proto zebra onlink
> -233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 30 onlink
> -233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 40 onlink])
> +233.253.0.0/24 via 192.168.10.20 dev hv1-mll proto zebra metric 30 onlink
> +233.253.0.0/24 via 192.168.10.20 dev hv1-mll proto zebra metric 40 onlink])
>
> # Now we set maintain-vrf again and stop the ovn-controller.
> # It will then remove the VRF.
> @@ -16925,13 +16925,13 @@ check_row_count Learned_Route 1
> AS_BOX([No dynamic-routing-port-name: routes learned on lrp1 and lrp2])
> check ovn-nbctl --wait=hv \
> remove logical_router_port lrp2 options dynamic-routing-port-name
> -check_row_count Learned_Route 1 ip_prefix=3.3.3.0/24 \
> +check_row_count Learned_Route 0 ip_prefix=3.3.3.0/24 \
> nexthop=2.2.2.2 \
> logical_port=$lrp1
> check_row_count Learned_Route 1 ip_prefix=3.3.3.0/24 \
> nexthop=2.2.2.2 \
> logical_port=$lrp2
> -check_row_count Learned_Route 2
> +check_row_count Learned_Route 1
>
> OVN_CLEANUP_CONTROLLER([hv1])
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
