Hi Dumitru, thanks for the review. Em sex., 12 de dez. de 2025 às 12:29, Dumitru Ceara <[email protected]> escreveu:
> 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? > > You're right, it's more appropriate to add the code in ovn controller and northd will not generate flows. However, I would like to avoid the garbage on the southbound. Regards, Lucas > 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]) > > > > -- _‘Esta mensagem é direcionada apenas para os endereços constantes no cabeçalho inicial. Se você não está listado nos endereços constantes no cabeçalho, pedimos-lhe que desconsidere completamente o conteúdo dessa mensagem e cuja cópia, encaminhamento e/ou execução das ações citadas estão imediatamente anuladas e proibidas’._ * **‘Apesar do Magazine Luiza tomar todas as precauções razoáveis para assegurar que nenhum vírus esteja presente nesse e-mail, a empresa não poderá aceitar a responsabilidade por quaisquer perdas ou danos causados por esse e-mail ou por seus anexos’.* _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
