network/ndisc: fix removal of unnecessary routes (#35128)

Follow-up for 972f1d17ab461a51142a142609dd3ec50bae8440.

This fixes the logic of removing unnecessary routes configured by the
previously received RAs. Previously, we wrongly handled existing routes
could be updated, and unexpected routes would be kept.

3 files changed, 76 insertions, 30 deletions

diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c
index c1d2373f65..3871618e5f 100644
--- a/src/network/networkd-ndisc.c
+++ b/src/network/networkd-ndisc.c
@@ -196,11 +196,6 @@ static int ndisc_request_route(Route *route, Link *link) {
         assert(link->manager);
         assert(link->network);
 
-        route->source = NETWORK_CONFIG_SOURCE_NDISC;
-
-        if (!route->table_set)
-                route->table = link_get_ndisc_route_table(link);
-
         r = route_metric_set(&route->metric, RTAX_QUICKACK, link->network->ndisc_quickack);
         if (r < 0)
                 return r;
@@ -229,6 +224,9 @@ static int ndisc_request_route(Route *route, Link *link) {
                  * and also an existing pending request, then the source may be updated by the request. So,
                  * we first need to check the source of the requested route. */
                 if (route_get_request(link->manager, route, &req) >= 0) {
+                        route->pref = pref_original;
+                        ndisc_set_route_priority(link, route);
+
                         existing = ASSERT_PTR(req->userdata);
                         if (!route_can_update(existing, route)) {
                                 if (existing->source == NETWORK_CONFIG_SOURCE_STATIC) {
@@ -243,8 +241,14 @@ static int ndisc_request_route(Route *route, Link *link) {
                         }
                 }
 
+                route->pref = pref;
+                ndisc_set_route_priority(link, route);
+
                 /* Then, check if a conflicting route exists. */
                 if (route_get(link->manager, route, &existing) >= 0) {
+                        route->pref = pref_original;
+                        ndisc_set_route_priority(link, route);
+
                         if (!route_can_update(existing, route)) {
                                 if (existing->source == NETWORK_CONFIG_SOURCE_STATIC) {
                                         log_link_debug(link, "Found an existing route that conflicts with new route based on a received RA, ignoring request.");
@@ -274,6 +278,29 @@ static int ndisc_request_route(Route *route, Link *link) {
         return 0;
 }
 
+static void ndisc_route_prepare(Route *route, Link *link) {
+        assert(route);
+        assert(link);
+
+        route->source = NETWORK_CONFIG_SOURCE_NDISC;
+
+        if (!route->table_set)
+                route->table = link_get_ndisc_route_table(link);
+}
+
+static int ndisc_router_route_prepare(Route *route, Link *link, sd_ndisc_router *rt) {
+        assert(route);
+        assert(link);
+        assert(rt);
+
+        ndisc_route_prepare(route, link);
+
+        if (!route->protocol_set)
+                route->protocol = RTPROT_RA;
+
+        return sd_ndisc_router_get_sender_address(rt, &route->provider.in6);
+}
+
 static int ndisc_request_router_route(Route *route, Link *link, sd_ndisc_router *rt) {
         int r;
 
@@ -281,28 +308,20 @@ static int ndisc_request_router_route(Route *route, Link *link, sd_ndisc_router
         assert(link);
         assert(rt);
 
-        r = sd_ndisc_router_get_sender_address(rt, &route->provider.in6);
+        r = ndisc_router_route_prepare(route, link, rt);
         if (r < 0)
                 return r;
 
-        if (!route->protocol_set)
-                route->protocol = RTPROT_RA;
-
         return ndisc_request_route(route, link);
 }
 
 static int ndisc_remove_route(Route *route, Link *link) {
-        int r;
+        int r, ret = 0;
 
         assert(route);
         assert(link);
         assert(link->manager);
 
-        ndisc_set_route_priority(link, route);
-
-        if (!route->table_set)
-                route->table = link_get_ndisc_route_table(link);
-
         r = route_adjust_nexthops(route, link);
         if (r < 0)
                 return r;
@@ -331,9 +350,7 @@ static int ndisc_remove_route(Route *route, Link *link) {
                         if (existing->source == NETWORK_CONFIG_SOURCE_STATIC)
                                 continue;
 
-                        r = route_remove_and_cancel(existing, link->manager);
-                        if (r < 0)
-                                return r;
+                        RET_GATHER(ret, route_remove_and_cancel(existing, link->manager));
                 }
 
                 /* Then, check if the route exists. */
@@ -341,13 +358,25 @@ static int ndisc_remove_route(Route *route, Link *link) {
                         if (existing->source == NETWORK_CONFIG_SOURCE_STATIC)
                                 continue;
 
-                        r = route_remove_and_cancel(existing, link->manager);
-                        if (r < 0)
-                                return r;
+                        RET_GATHER(ret, route_remove_and_cancel(existing, link->manager));
                 }
         }
 
-        return 0;
+        return ret;
+}
+
+static int ndisc_remove_router_route(Route *route, Link *link, sd_ndisc_router *rt) {
+        int r;
+
+        assert(route);
+        assert(link);
+        assert(rt);
+
+        r = ndisc_router_route_prepare(route, link, rt);
+        if (r < 0)
+                return r;
+
+        return ndisc_remove_route(route, link);
 }
 
 static int ndisc_address_handler(sd_netlink *rtnl, sd_netlink_message *m, Request *req, Link *link, Address *address) {
@@ -480,6 +509,8 @@ static int ndisc_remove_redirect_route(Link *link, sd_ndisc_redirect *rd) {
         if (r < 0)
                 return r;
 
+        ndisc_route_prepare(route, link);
+
         return ndisc_remove_route(route, link);
 }
 
@@ -680,6 +711,8 @@ static int ndisc_redirect_handler(Link *link, sd_ndisc_redirect *rd) {
         if (r < 0)
                 return r;
 
+        ndisc_route_prepare(route, link);
+
         return ndisc_request_route(route, link);
 }
 
@@ -755,7 +788,7 @@ static int ndisc_router_drop_default(Link *link, sd_ndisc_router *rt) {
         route->nexthop.family = AF_INET6;
         route->nexthop.gw.in6 = gateway;
 
-        r = ndisc_remove_route(route, link);
+        r = ndisc_remove_router_route(route, link, rt);
         if (r < 0)
                 return log_link_warning_errno(link, r, "Failed to remove the default gateway configured by RA: %m");
 
@@ -775,7 +808,7 @@ static int ndisc_router_drop_default(Link *link, sd_ndisc_router *rt) {
 
                 tmp->nexthop.gw.in6 = gateway;
 
-                r = ndisc_remove_route(tmp, link);
+                r = ndisc_remove_router_route(tmp, link, rt);
                 if (r < 0)
                         return log_link_warning_errno(link, r, "Could not remove semi-static gateway: %m");
         }
@@ -1280,7 +1313,7 @@ static int ndisc_router_process_onlink_prefix(Link *link, sd_ndisc_router *rt) {
          *   received advertisement, reset its invalidation timer to the Valid Lifetime value in the Prefix
          *   Information option. If the new Lifetime value is zero, timeout the prefix immediately. */
         if (lifetime_usec == 0) {
-                r = ndisc_remove_route(route, link);
+                r = ndisc_remove_router_route(route, link, rt);
                 if (r < 0)
                         return log_link_warning_errno(link, r, "Failed to remove prefix route: %m");
         } else {
@@ -1418,7 +1451,7 @@ static int ndisc_router_process_route(Link *link, sd_ndisc_router *rt) {
                 if (r < 0)
                         return log_link_warning_errno(link, r, "Could not request additional route: %m");
         } else {
-                r = ndisc_remove_route(route, link);
+                r = ndisc_remove_router_route(route, link, rt);
                 if (r < 0)
                         return log_link_warning_errno(link, r, "Could not remove additional route with zero lifetime: %m");
         }
@@ -2062,7 +2095,7 @@ static int ndisc_drop_outdated(Link *link, const struct in6_addr *router, usec_t
                 if (route->source != NETWORK_CONFIG_SOURCE_NDISC)
                         continue;
 
-                if (route->nexthop.ifindex != link->ifindex)
+                if (!route_is_bound_to_link(route, link))
                         continue;
 
                 if (route->protocol == RTPROT_REDIRECT)
@@ -2198,7 +2231,7 @@ static int ndisc_setup_expire(Link *link) {
                 if (route->source != NETWORK_CONFIG_SOURCE_NDISC)
                         continue;
 
-                if (route->nexthop.ifindex != link->ifindex)
+                if (!route_is_bound_to_link(route, link))
                         continue;
 
                 if (!route_exists(route))
@@ -2436,7 +2469,7 @@ static int ndisc_neighbor_handle_router_message(Link *link, sd_ndisc_neighbor *n
                 if (route->source != NETWORK_CONFIG_SOURCE_NDISC)
                         continue;
 
-                if (route->nexthop.ifindex != link->ifindex)
+                if (!route_is_bound_to_link(route, link))
                         continue;
 
                 if (!in6_addr_equal(&route->provider.in6, &original_address))
@@ -2729,7 +2762,7 @@ int link_drop_ndisc_config(Link *link, Network *network) {
                         if (route->source != NETWORK_CONFIG_SOURCE_NDISC)
                                 continue;
 
-                        if (route->nexthop.ifindex != link->ifindex)
+                        if (!route_is_bound_to_link(route, link))
                                 continue;
 
                         if (route->protocol == RTPROT_REDIRECT)
diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c
index 5104223396..b85d2a037a 100644
--- a/src/network/networkd-route.c
+++ b/src/network/networkd-route.c
@@ -352,6 +352,18 @@ static int route_get_link(Manager *manager, const Route *route, Link **ret) {
         return route_nexthop_get_link(manager, &route->nexthop, ret);
 }
 
+bool route_is_bound_to_link(const Route *route, Link *link) {
+        assert(route);
+        assert(link);
+        assert(link->manager);
+
+        Link *route_link;
+        if (route_get_link(link->manager, route, &route_link) < 0)
+                return false;
+
+        return route_link->ifindex == link->ifindex;
+}
+
 int route_get_request(Manager *manager, const Route *route, Request **ret) {
         Request *req;
 
diff --git a/src/network/networkd-route.h b/src/network/networkd-route.h
index f391d95714..b9d2dbf9a6 100644
--- a/src/network/networkd-route.h
+++ b/src/network/networkd-route.h
@@ -100,6 +100,7 @@ int route_remove(Route *route, Manager *manager);
 int route_remove_and_cancel(Route *route, Manager *manager);
 
 int route_get(Manager *manager, const Route *route, Route **ret);
+bool route_is_bound_to_link(const Route *route, Link *link);
 int route_get_request(Manager *manager, const Route *route, Request **ret);
 
 bool route_can_update(const Route *existing, const Route *requesting);