From b2640c4e6eb46c9ba220b05dd02d0d8cb025491e Mon Sep 17 00:00:00 2001 From: Jonas Gorski Date: Tue, 21 May 2024 14:35:08 +0200 Subject: [PATCH 1/3] nl_l3::del_l3_unicast_route(): ignore routes with no nexthops Like we do when adding a route, ignore routes with no nexthops. Signed-off-by: Jonas Gorski --- src/netlink/nl_l3.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/netlink/nl_l3.cc b/src/netlink/nl_l3.cc index be3957f9..0c121268 100644 --- a/src/netlink/nl_l3.cc +++ b/src/netlink/nl_l3.cc @@ -1926,6 +1926,7 @@ int nl_l3::del_l3_unicast_route(rtnl_route *r, bool keep_route) { int rv = 0; auto dst = rtnl_route_get_dst(r); + int nnhs = rtnl_route_get_nnexthops(r); uint16_t vrf_id = rtnl_route_get_table(r); // Routing table 254 id matches the main routing table on linux. // Adding a vrf with this id will collide with the main routing @@ -1933,6 +1934,11 @@ int nl_l3::del_l3_unicast_route(rtnl_route *r, bool keep_route) { if (vrf_id == MAIN_ROUTING_TABLE) vrf_id = 0; + if (nnhs == 0) { + LOG(WARNING) << __FUNCTION__ << ": no neighbours of route " << r; + return -ENOTSUP; + } + if (rtnl_route_get_priority(r) > 0 && rtnl_route_get_protocol(r) != RTPROT_KERNEL) { nl_route_query rq; From af60faa12864ea71436a0d447467eec6f621d72a Mon Sep 17 00:00:00 2001 From: Jonas Gorski Date: Tue, 21 May 2024 14:43:08 +0200 Subject: [PATCH 2/3] nl_l3::add_l3_unicast_route(): fix nnhs handling We already rejected routes with nnhs == 0 earlier, so nnhs cannot be 0 at this stage. Instead, we have only two options, nnhs == 1 (single nexthop or on-link route), or nnhs > 1 (ecmp route). So convert the switch to a if-else and make the 0 for on-link and routes with unresolved neighbor explicit. Signed-off-by: Jonas Gorski --- src/netlink/nl_l3.cc | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/src/netlink/nl_l3.cc b/src/netlink/nl_l3.cc index 0c121268..9e826212 100644 --- a/src/netlink/nl_l3.cc +++ b/src/netlink/nl_l3.cc @@ -1849,22 +1849,23 @@ int nl_l3::add_l3_unicast_route(rtnl_route *r, bool update_route) { this, nh_params{net_params{rtnl_route_get_dst(r), nh.ifindex}, nh}); } - switch (nnhs) { - case 0: - // Not yet determined next-hop, the rule must be installed to the switch - // pointing to controller and then updated after next-hop registration - rv = add_l3_unicast_route(rtnl_route_get_dst(r), 0, false, update_route, - vrf_id); - break; - case 1: - // Single known next-hop - rv = add_l3_unicast_route(rtnl_route_get_dst(r), *l3_interfaces.begin(), - false, update_route, vrf_id); - break; - default: + if (nnhs == 1) { + uint32_t route_dst_interface; + + if (l3_interfaces.size() == 0) { + // Not yet resolved next-hop, or on-link route, the rule must be installed + // to the switch pointing to controller and then updated after neighbor + // registration. + route_dst_interface = 0; + } else { + // single, resolved nexthop + route_dst_interface = *l3_interfaces.begin(); /* resolved nexthop */ + } + rv = add_l3_unicast_route(rtnl_route_get_dst(r), route_dst_interface, false, + update_route, vrf_id); + } else { // create ecmp group rv = add_l3_ecmp_route(r, l3_interfaces, update_route); - break; } if (rv < 0) { From 8e57584ced57ed2a892b4340b950ac02d4003b2e Mon Sep 17 00:00:00 2001 From: Jonas Gorski Date: Tue, 21 May 2024 14:47:24 +0200 Subject: [PATCH 3/3] nl_l3::del_l3_unicast_route(): use nnhs for ecmp determination We used the number of total nexthops for determining ecmp routes on creation, so we need to do the same on deletion. Signed-off-by: Jonas Gorski --- src/netlink/nl_l3.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/netlink/nl_l3.cc b/src/netlink/nl_l3.cc index 9e826212..db0735e9 100644 --- a/src/netlink/nl_l3.cc +++ b/src/netlink/nl_l3.cc @@ -2024,7 +2024,7 @@ int nl_l3::del_l3_unicast_route(rtnl_route *r, bool keep_route) { return rv; } - if (neighs.size() > 1) { + if (nnhs > 1) { // get all l3 interfaces std::set l3_interfaces; // all create l3 interface ids for (auto n : neighs) {