]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
Merge pull request #31082 from yuwata/network-cleanups-for-removing-routes
authorLennart Poettering <lennart@poettering.net>
Thu, 25 Jan 2024 15:24:19 +0000 (16:24 +0100)
committerGitHub <noreply@github.com>
Thu, 25 Jan 2024 15:24:19 +0000 (16:24 +0100)
network: several cleanups for removing routes

15 files changed:
src/network/netdev/wireguard.c
src/network/networkd-dhcp-prefix-delegation.c
src/network/networkd-dhcp4.c
src/network/networkd-dhcp6.c
src/network/networkd-link.c
src/network/networkd-ndisc.c
src/network/networkd-network.c
src/network/networkd-nexthop.c
src/network/networkd-nexthop.h
src/network/networkd-route-metric.c
src/network/networkd-route-nexthop.c
src/network/networkd-route-nexthop.h
src/network/networkd-route.c
src/network/networkd-route.h
test/test-network/systemd-networkd-tests.py

index 0195dce1bf04c6654623661621e869dcb4b541ca..95c9c8c330093d5e20f666b49d9184c9f937b5b8 100644 (file)
@@ -1190,7 +1190,7 @@ static int wireguard_verify(NetDev *netdev, const char *filename) {
                         continue;
 
                 LIST_FOREACH(ipmasks, ipmask, peer->ipmasks) {
-                        _cleanup_(route_freep) Route *route = NULL;
+                        _cleanup_(route_unrefp) Route *route = NULL;
 
                         r = route_new(&route);
                         if (r < 0)
index b8b03f6d3b94882d0cf11c10091aa7c92e0dab6d..61295d9ce6458b7c57a847d7b4482cd096232918 100644 (file)
@@ -176,8 +176,7 @@ int dhcp_pd_remove(Link *link, bool only_marked) {
 
                         link_remove_dhcp_pd_subnet_prefix(link, &route->dst.in6);
 
-                        RET_GATHER(ret, route_remove(route));
-                        route_cancel_request(route, link);
+                        RET_GATHER(ret, route_remove_and_cancel(route, link->manager));
                 }
         } else {
                 Address *address;
@@ -283,7 +282,7 @@ static int dhcp_pd_route_handler(sd_netlink *rtnl, sd_netlink_message *m, Reques
 }
 
 static int dhcp_pd_request_route(Link *link, const struct in6_addr *prefix, usec_t lifetime_usec) {
-        _cleanup_(route_freep) Route *route = NULL;
+        _cleanup_(route_unrefp) Route *route = NULL;
         Route *existing;
         int r;
 
@@ -612,9 +611,7 @@ void dhcp_pd_prefix_lost(Link *uplink) {
                                           .address = route->dst }))
                         continue;
 
-                (void) route_remove(route);
-
-                route_cancel_request(route, uplink);
+                (void) route_remove_and_cancel(route, uplink->manager);
         }
 
         set_clear(uplink->dhcp_pd_prefixes);
@@ -673,7 +670,7 @@ static int dhcp_request_unreachable_route(
                 route_netlink_handler_t callback,
                 bool *configured) {
 
-        _cleanup_(route_freep) Route *route = NULL;
+        _cleanup_(route_unrefp) Route *route = NULL;
         Route *existing;
         int r;
 
@@ -784,7 +781,7 @@ static int dhcp_pd_prefix_add(Link *link, const struct in6_addr *prefix, uint8_t
 }
 
 static int dhcp4_pd_request_default_gateway_on_6rd_tunnel(Link *link, const struct in_addr *br_address, usec_t lifetime_usec) {
-        _cleanup_(route_freep) Route *route = NULL;
+        _cleanup_(route_unrefp) Route *route = NULL;
         Route *existing;
         int r;
 
index be7abcf6f3849f53e166cfc1cc878bd74b2f7952..f9baf0d535675c7420bcf04d852dd6e3b46f9526 100644 (file)
@@ -256,8 +256,7 @@ static int dhcp4_remove_address_and_routes(Link *link, bool only_marked) {
                 if (only_marked && !route_is_marked(route))
                         continue;
 
-                RET_GATHER(ret, route_remove(route));
-                route_cancel_request(route, link);
+                RET_GATHER(ret, route_remove_and_cancel(route, link->manager));
         }
 
         SET_FOREACH(address, link->addresses) {
@@ -411,7 +410,7 @@ static bool link_prefixroute(Link *link) {
 }
 
 static int dhcp4_request_prefix_route(Link *link) {
-        _cleanup_(route_freep) Route *route = NULL;
+        _cleanup_(route_unrefp) Route *route = NULL;
         int r;
 
         assert(link);
@@ -439,7 +438,7 @@ static int dhcp4_request_prefix_route(Link *link) {
 }
 
 static int dhcp4_request_route_to_gateway(Link *link, const struct in_addr *gw) {
-        _cleanup_(route_freep) Route *route = NULL;
+        _cleanup_(route_unrefp) Route *route = NULL;
         struct in_addr address;
         int r;
 
@@ -557,7 +556,7 @@ static int dhcp4_request_classless_static_or_static_routes(Link *link) {
                 return r;
 
         FOREACH_ARRAY(e, routes, n_routes) {
-                _cleanup_(route_freep) Route *route = NULL;
+                _cleanup_(route_unrefp) Route *route = NULL;
                 struct in_addr gw;
 
                 r = route_new(&route);
@@ -585,7 +584,7 @@ static int dhcp4_request_classless_static_or_static_routes(Link *link) {
 }
 
 static int dhcp4_request_default_gateway(Link *link) {
-        _cleanup_(route_freep) Route *route = NULL;
+        _cleanup_(route_unrefp) Route *route = NULL;
         struct in_addr address, router;
         int r;
 
@@ -640,7 +639,7 @@ static int dhcp4_request_semi_static_routes(Link *link) {
         assert(link->network);
 
         HASHMAP_FOREACH(rt, link->network->routes_by_section) {
-                _cleanup_(route_freep) Route *route = NULL;
+                _cleanup_(route_unrefp) Route *route = NULL;
                 struct in_addr gw;
 
                 if (!rt->gateway_from_dhcp_or_ra)
@@ -691,7 +690,7 @@ static int dhcp4_request_routes_to_servers(
         assert(servers || n_servers == 0);
 
         FOREACH_ARRAY(dst, servers, n_servers) {
-                _cleanup_(route_freep) Route *route = NULL;
+                _cleanup_(route_unrefp) Route *route = NULL;
                 struct in_addr gw;
 
                 if (in4_addr_is_null(dst))
index 26ed034bbeae03243782198d64f3a3d601980b72..eb1420be5c0691314bc45b174eb18e30466fbc29 100644 (file)
@@ -62,8 +62,7 @@ static int dhcp6_remove(Link *link, bool only_marked) {
                 if (only_marked && !route_is_marked(route))
                         continue;
 
-                RET_GATHER(ret, route_remove(route));
-                route_cancel_request(route, link);
+                RET_GATHER(ret, route_remove_and_cancel(route, link->manager));
         }
 
         SET_FOREACH(address, link->addresses) {
index a79ab0d43c12bac6c595119071851aec2c89f011..194d31eafe89dea0e5540f0444d8bd2323a9f9fe 100644 (file)
@@ -997,6 +997,13 @@ static int link_drop_requests(Link *link) {
                                         RET_GATHER(ret, nexthop_remove(nexthop, link->manager));
                                 break;
                         }
+                        case REQUEST_TYPE_ROUTE: {
+                                Route *route = ASSERT_PTR(req->userdata);
+
+                                if (route_get(link->manager, route, NULL) < 0)
+                                        RET_GATHER(ret, route_remove(route, link->manager));
+                                break;
+                        }
                         default:
                                 ;
                         }
index 7186187fd828f379247a31cbc595dea4840b11cc..7a67c552993a5aaf8a998f11e0f824fbee9378a6 100644 (file)
@@ -316,7 +316,7 @@ static int ndisc_router_process_default(Link *link, sd_ndisc_router *rt) {
                 return log_link_warning_errno(link, r, "Failed to get default router preference from RA: %m");
 
         if (link->network->ipv6_accept_ra_use_gateway) {
-                _cleanup_(route_freep) Route *route = NULL;
+                _cleanup_(route_unrefp) Route *route = NULL;
 
                 r = route_new(&route);
                 if (r < 0)
@@ -335,7 +335,7 @@ static int ndisc_router_process_default(Link *link, sd_ndisc_router *rt) {
 
         Route *route_gw;
         HASHMAP_FOREACH(route_gw, link->network->routes_by_section) {
-                _cleanup_(route_freep) Route *route = NULL;
+                _cleanup_(route_unrefp) Route *route = NULL;
 
                 if (!route_gw->gateway_from_dhcp_or_ra)
                         continue;
@@ -498,7 +498,7 @@ static int ndisc_router_process_autonomous_prefix(Link *link, sd_ndisc_router *r
 }
 
 static int ndisc_router_process_onlink_prefix(Link *link, sd_ndisc_router *rt) {
-        _cleanup_(route_freep) Route *route = NULL;
+        _cleanup_(route_unrefp) Route *route = NULL;
         unsigned prefixlen, preference;
         usec_t lifetime_usec;
         struct in6_addr prefix;
@@ -600,7 +600,7 @@ static int ndisc_router_process_prefix(Link *link, sd_ndisc_router *rt) {
 }
 
 static int ndisc_router_process_route(Link *link, sd_ndisc_router *rt) {
-        _cleanup_(route_freep) Route *route = NULL;
+        _cleanup_(route_unrefp) Route *route = NULL;
         unsigned preference, prefixlen;
         struct in6_addr gateway, dst;
         usec_t lifetime_usec;
@@ -1173,7 +1173,7 @@ static int ndisc_drop_outdated(Link *link, usec_t timestamp_usec) {
                 if (route->lifetime_usec >= timestamp_usec)
                         continue; /* the route is still valid */
 
-                r = route_remove_and_drop(route);
+                r = route_remove_and_cancel(route, link->manager);
                 if (r < 0)
                         RET_GATHER(ret, log_link_warning_errno(link, r, "Failed to remove outdated SLAAC route, ignoring: %m"));
         }
index 16c679b34388e640106c9b2c05f1884f709c6826..300fba5dd9968be7987864fd4478befe11b0376e 100644 (file)
@@ -188,7 +188,7 @@ int network_verify(Network *network) {
                                     network->filename);
 
                 network->addresses_by_section = ordered_hashmap_free(network->addresses_by_section);
-                network->routes_by_section = hashmap_free_with_destructor(network->routes_by_section, route_free);
+                network->routes_by_section = hashmap_free(network->routes_by_section);
         }
 
         if (network->link_local < 0) {
@@ -782,7 +782,7 @@ static Network *network_free(Network *network) {
         /* static configs */
         set_free_free(network->ipv6_proxy_ndp_addresses);
         ordered_hashmap_free(network->addresses_by_section);
-        hashmap_free_with_destructor(network->routes_by_section, route_free);
+        hashmap_free(network->routes_by_section);
         ordered_hashmap_free(network->nexthops_by_section);
         hashmap_free_with_destructor(network->bridge_fdb_entries_by_section, bridge_fdb_free);
         hashmap_free_with_destructor(network->bridge_mdb_entries_by_section, bridge_mdb_free);
index cf62e0e82dc666450a42faf5bf2d87ec4c1bc980..07a971a0e9265b71e37c7ac56fe0d31643be5cb2 100644 (file)
@@ -12,6 +12,7 @@
 #include "networkd-network.h"
 #include "networkd-nexthop.h"
 #include "networkd-queue.h"
+#include "networkd-route.h"
 #include "networkd-route-util.h"
 #include "parse-util.h"
 #include "set.h"
@@ -97,6 +98,7 @@ static NextHop* nexthop_free(NextHop *nexthop) {
         config_section_free(nexthop->section);
         hashmap_free_free(nexthop->group);
         set_free(nexthop->nexthops);
+        set_free(nexthop->routes);
 
         return mfree(nexthop);
 }
@@ -488,7 +490,7 @@ static int nexthop_remove_dependents(NextHop *nexthop, Manager *manager) {
         assert(nexthop);
         assert(manager);
 
-        /* If a nexthop is removed, the kernel silently removes nexthops that depend on the
+        /* If a nexthop is removed, the kernel silently removes nexthops and routes that depend on the
          * removed nexthop. Let's remove them for safety (though, they are already removed in the kernel,
          * hence that should fail), and forget them. */
 
@@ -502,6 +504,10 @@ static int nexthop_remove_dependents(NextHop *nexthop, Manager *manager) {
                 RET_GATHER(r, nexthop_remove(nh, manager));
         }
 
+        Route *route;
+        SET_FOREACH(route, nexthop->routes)
+                RET_GATHER(r, route_remove(route, manager));
+
         return r;
 }
 
index 74b23bd77203f0349e0f9ea98248256c1ba476ee..9ce36c6cd9b18d3f19bc8b73da1e555d8264cb79 100644 (file)
@@ -41,8 +41,9 @@ typedef struct NextHop {
         /* Only used in conf parser and nexthop_section_verify(). */
         int onlink;
 
-        /* For managing nexthops that depend on this nexthop. */
+        /* For managing routes and nexthops that depend on this nexthop. */
         Set *nexthops;
+        Set *routes;
 } NextHop;
 
 NextHop* nexthop_ref(NextHop *nexthop);
index b27b3c12948e81220e17e0ff980b453b6a243077..9c417a4fc4f26ed76b96ff8e9bb72eaefce01752 100644 (file)
@@ -374,7 +374,7 @@ static int config_parse_route_metric_boolean_impl(
                         void *userdata) {                               \
                                                                         \
                 Network *network = ASSERT_PTR(userdata);                \
-                _cleanup_(route_free_or_set_invalidp) Route *route = NULL; \
+                _cleanup_(route_unref_or_set_invalidp) Route *route = NULL; \
                 uint16_t attr_type = ltype;                             \
                 int r;                                                  \
                                                                         \
@@ -436,7 +436,7 @@ int config_parse_route_metric_tcp_congestion(
                 void *userdata) {
 
         Network *network = ASSERT_PTR(userdata);
-        _cleanup_(route_free_or_set_invalidp) Route *route = NULL;
+        _cleanup_(route_unref_or_set_invalidp) Route *route = NULL;
         int r;
 
         assert(filename);
index 5f1da286046456c1215c70a5af671a897c6fed13..f7a2201b6b0a0b7ed70a50b777d1328e95488c7d 100644 (file)
@@ -5,6 +5,7 @@
 #include "alloc-util.h"
 #include "extract-word.h"
 #include "netlink-util.h"
+#include "networkd-manager.h"
 #include "networkd-network.h"
 #include "networkd-nexthop.h"
 #include "networkd-route.h"
 #include "parse-util.h"
 #include "string-util.h"
 
+void route_detach_from_nexthop(Route *route) {
+        NextHop *nh;
+
+        assert(route);
+        assert(route->manager);
+
+        if (route->nexthop_id == 0)
+                return;
+
+        if (nexthop_get_by_id(route->manager, route->nexthop_id, &nh) < 0)
+                return;
+
+        route_unref(set_remove(nh->routes, route));
+}
+
+void route_attach_to_nexthop(Route *route) {
+        NextHop *nh;
+        int r;
+
+        assert(route);
+        assert(route->manager);
+
+        if (route->nexthop_id == 0)
+                return;
+
+        r = nexthop_get_by_id(route->manager, route->nexthop_id, &nh);
+        if (r < 0) {
+                if (route->manager->manage_foreign_nexthops)
+                        log_debug_errno(r, "Route has unknown nexthop ID (%"PRIu32"), ignoring.",
+                                        route->nexthop_id);
+                return;
+        }
+
+        r = set_ensure_put(&nh->routes, &route_hash_ops_unref, route);
+        if (r < 0)
+                return (void) log_debug_errno(r, "Failed to save route to nexthop, ignoring: %m");
+        if (r == 0)
+                return (void) log_debug("Duplicated route assigned to nexthop, ignoring.");
+
+        route_ref(route);
+}
+
 static void route_nexthop_done(RouteNextHop *nh) {
         assert(nh);
 
@@ -899,7 +942,7 @@ int config_parse_gateway(
                 void *userdata) {
 
         Network *network = userdata;
-        _cleanup_(route_free_or_set_invalidp) Route *route = NULL;
+        _cleanup_(route_unref_or_set_invalidp) Route *route = NULL;
         int r;
 
         assert(filename);
@@ -986,7 +1029,7 @@ int config_parse_route_gateway_onlink(
                 void *userdata) {
 
         Network *network = userdata;
-        _cleanup_(route_free_or_set_invalidp) Route *route = NULL;
+        _cleanup_(route_unref_or_set_invalidp) Route *route = NULL;
         int r;
 
         assert(filename);
@@ -1026,7 +1069,7 @@ int config_parse_route_nexthop(
                 void *userdata) {
 
         Network *network = userdata;
-        _cleanup_(route_free_or_set_invalidp) Route *route = NULL;
+        _cleanup_(route_unref_or_set_invalidp) Route *route = NULL;
         uint32_t id;
         int r;
 
@@ -1079,7 +1122,7 @@ int config_parse_multipath_route(
                 void *userdata) {
 
         _cleanup_(route_nexthop_freep) RouteNextHop *nh = NULL;
-        _cleanup_(route_free_or_set_invalidp) Route *route = NULL;
+        _cleanup_(route_unref_or_set_invalidp) Route *route = NULL;
         _cleanup_free_ char *word = NULL;
         Network *network = userdata;
         const char *p;
index 5e4602d3cd369d3808b68f2cb6a524192a651b61..f9a147839db9c5eec4d80033e6731a288b10f505 100644 (file)
@@ -26,6 +26,9 @@ typedef struct RouteNextHop {
 
 #define ROUTE_NEXTHOP_NULL ((const RouteNextHop) {})
 
+void route_detach_from_nexthop(Route *route);
+void route_attach_to_nexthop(Route *route);
+
 RouteNextHop* route_nexthop_free(RouteNextHop *nh);
 DEFINE_TRIVIAL_CLEANUP_FUNC(RouteNextHop*, route_nexthop_free);
 
index 3a33769f8b5094bb0d4aa822852e434a0f59e959..80e20a286b0ba7a139e80ae68b2850d0fc4b6577 100644 (file)
 #include "vrf.h"
 #include "wireguard.h"
 
-Route* route_free(Route *route) {
-        if (!route)
-                return NULL;
+static Route* route_detach_impl(Route *route) {
+        assert(route);
+        assert(!!route->network + !!route->manager + !!route->wireguard <= 1);
 
         if (route->network) {
                 assert(route->section);
                 hashmap_remove(route->network->routes_by_section, route->section);
+                route->network = NULL;
+                return route;
         }
 
-        if (route->manager)
+        if (route->manager) {
+                route_detach_from_nexthop(route);
                 set_remove(route->manager->routes, route);
+                route->manager = NULL;
+                return route;
+        }
 
-        if (route->wireguard)
+        if (route->wireguard) {
                 set_remove(route->wireguard->routes, route);
+                route->wireguard = NULL;
+                return route;
+        }
+
+        return NULL;
+}
+
+static void route_detach(Route *route) {
+        route_unref(route_detach_impl(route));
+}
+
+static Route* route_free(Route *route) {
+        if (!route)
+                return NULL;
+
+        route_detach_impl(route);
 
         config_section_free(route->section);
         route_nexthops_done(route);
@@ -44,6 +66,8 @@ Route* route_free(Route *route) {
         return mfree(route);
 }
 
+DEFINE_TRIVIAL_REF_UNREF_FUNC(Route, route, route_free);
+
 static void route_hash_func(const Route *route, struct siphash *state) {
         assert(route);
 
@@ -195,16 +219,32 @@ DEFINE_HASH_OPS_WITH_KEY_DESTRUCTOR(
                 Route,
                 route_hash_func,
                 route_compare_func,
-                route_free);
+                route_detach);
+
+DEFINE_HASH_OPS_WITH_KEY_DESTRUCTOR(
+                route_hash_ops_unref,
+                Route,
+                route_hash_func,
+                route_compare_func,
+                route_unref);
+
+DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR(
+                route_section_hash_ops,
+                ConfigSection,
+                config_section_hash_func,
+                config_section_compare_func,
+                Route,
+                route_detach);
 
 int route_new(Route **ret) {
-        _cleanup_(route_freep) Route *route = NULL;
+        _cleanup_(route_unrefp) Route *route = NULL;
 
         route = new(Route, 1);
         if (!route)
                 return -ENOMEM;
 
         *route = (Route) {
+                .n_ref = 1,
                 .family = AF_UNSPEC,
                 .scope = RT_SCOPE_UNIVERSE,
                 .protocol = RTPROT_UNSPEC,
@@ -221,7 +261,7 @@ int route_new(Route **ret) {
 
 int route_new_static(Network *network, const char *filename, unsigned section_line, Route **ret) {
         _cleanup_(config_section_freep) ConfigSection *n = NULL;
-        _cleanup_(route_freep) Route *route = NULL;
+        _cleanup_(route_unrefp) Route *route = NULL;
         int r;
 
         assert(network);
@@ -251,7 +291,7 @@ int route_new_static(Network *network, const char *filename, unsigned section_li
         route->section = TAKE_PTR(n);
         route->source = NETWORK_CONFIG_SOURCE_STATIC;
 
-        r = hashmap_ensure_put(&network->routes_by_section, &config_section_hash_ops, route->section, route);
+        r = hashmap_ensure_put(&network->routes_by_section, &route_section_hash_ops, route->section, route);
         if (r < 0)
                 return r;
 
@@ -259,7 +299,7 @@ int route_new_static(Network *network, const char *filename, unsigned section_li
         return 0;
 }
 
-static int route_add(Manager *manager, Route *route) {
+static int route_attach(Manager *manager, Route *route) {
         int r;
 
         assert(manager);
@@ -334,7 +374,7 @@ static int route_get_request(Manager *manager, const Route *route, Request **ret
 }
 
 int route_dup(const Route *src, const RouteNextHop *nh, Route **ret) {
-        _cleanup_(route_freep) Route *dest = NULL;
+        _cleanup_(route_unrefp) Route *dest = NULL;
         int r;
 
         assert(src);
@@ -344,7 +384,8 @@ int route_dup(const Route *src, const RouteNextHop *nh, Route **ret) {
         if (!dest)
                 return -ENOMEM;
 
-        /* Unset all pointers */
+        /* Unset number of reference and all pointers */
+        dest->n_ref = 1;
         dest->manager = NULL;
         dest->network = NULL;
         dest->wireguard = NULL;
@@ -493,30 +534,46 @@ static int route_set_netlink_message(const Route *route, sd_netlink_message *m)
         return 0;
 }
 
-static int route_remove_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) {
+static int route_remove_handler(sd_netlink *rtnl, sd_netlink_message *m, RemoveRequest *rreq) {
         int r;
 
         assert(m);
+        assert(rreq);
 
-        /* link may be NULL. */
-
-        if (link && IN_SET(link->state, LINK_STATE_FAILED, LINK_STATE_LINGER))
-                return 0;
+        Manager *manager = ASSERT_PTR(rreq->manager);
+        Route *route = ASSERT_PTR(rreq->userdata);
 
         r = sd_netlink_message_get_errno(m);
-        if (r < 0 && r != -ESRCH)
-                log_link_message_warning_errno(link, m, r, "Could not drop route, ignoring");
+        if (r < 0) {
+                log_message_full_errno(m,
+                                       (r == -ESRCH || /* the route is already removed? */
+                                        (r == -EINVAL && route->nexthop_id != 0) || /* The nexthop is already removed? */
+                                        !route->manager) ? /* already detached? */
+                                       LOG_DEBUG : LOG_WARNING,
+                                       r, "Could not drop route, ignoring");
+
+                if (route->manager) {
+                        /* If the route cannot be removed, then assume the route is already removed. */
+                        log_route_debug(route, "Forgetting", manager);
+
+                        Request *req;
+                        if (route_get_request(manager, route, &req) >= 0)
+                                route_enter_removed(req->userdata);
+
+                        route_detach(route);
+                }
+        }
 
         return 1;
 }
 
-int route_remove(Route *route) {
+int route_remove(Route *route, Manager *manager) {
         _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *m = NULL;
         Link *link = NULL;
         int r;
 
         assert(route);
-        Manager *manager = ASSERT_PTR(route->manager);
+        assert(manager);
 
         log_route_debug(route, "Removing", manager);
 
@@ -530,28 +587,35 @@ int route_remove(Route *route) {
         if (r < 0)
                 return log_link_warning_errno(link, r, "Could not fill netlink message: %m");
 
-        r = netlink_call_async(manager->rtnl, NULL, m, route_remove_handler,
-                               link ? link_netlink_destroy_callback : NULL, link);
+        r = manager_remove_request_add(manager, route, route, manager->rtnl, m, route_remove_handler);
         if (r < 0)
-                return log_link_warning_errno(link, r, "Could not send netlink message: %m");
-
-        link_ref(link);
+                return log_link_warning_errno(link, r, "Could not queue rtnetlink message: %m");
 
         route_enter_removing(route);
         return 0;
 }
 
-int route_remove_and_drop(Route *route) {
-        if (!route)
-                return 0;
+int route_remove_and_cancel(Route *route, Manager *manager) {
+        bool waiting = false;
+        Request *req;
 
-        route_cancel_request(route, NULL);
+        assert(route);
+        assert(manager);
 
-        if (route_exists(route))
-                return route_remove(route);
+        /* If the route is remembered by the manager, then use the remembered object. */
+        (void) route_get(manager, route, &route);
 
-        if (route->state == 0)
-                route_free(route);
+        /* Cancel the request for the route. If the request is already called but we have not received the
+         * notification about the request, then explicitly remove the route. */
+        if (route_get_request(manager, route, &req) >= 0) {
+                waiting = req->waiting_reply;
+                request_detach(req);
+                route_cancel_requesting(route);
+        }
+
+        /* If we know that the route will come or already exists, remove it. */
+        if (waiting || (route->manager && route_exists(route)))
+                return route_remove(route, manager);
 
         return 0;
 }
@@ -560,9 +624,10 @@ static int route_expire_handler(sd_event_source *s, uint64_t usec, void *userdat
         Route *route = ASSERT_PTR(userdata);
         int r;
 
-        assert(route->manager);
+        if (!route->manager)
+                return 0; /* already detached. */
 
-        r = route_remove(route);
+        r = route_remove(route, route->manager);
         if (r < 0) {
                 Link *link = NULL;
                 (void) route_get_link(route->manager, route, &link);
@@ -673,7 +738,7 @@ static int route_configure(const Route *route, uint32_t lifetime_sec, Link *link
 
 static int route_requeue_request(Request *req, Link *link, const Route *route) {
         _unused_ _cleanup_(request_unrefp) Request *req_unref = NULL;
-        _cleanup_(route_freep) Route *tmp = NULL;
+        _cleanup_(route_unrefp) Route *tmp = NULL;
         int r;
 
         assert(req);
@@ -796,7 +861,7 @@ static int link_request_route_one(
                 unsigned *message_counter,
                 route_netlink_handler_t netlink_handler) {
 
-        _cleanup_(route_freep) Route *tmp = NULL;
+        _cleanup_(route_unrefp) Route *tmp = NULL;
         Route *existing = NULL;
         int r;
 
@@ -819,7 +884,7 @@ static int link_request_route_one(
         log_route_debug(tmp, "Requesting", link->manager);
         r = link_queue_request_safe(link, REQUEST_TYPE_ROUTE,
                                     tmp,
-                                    route_free,
+                                    route_unref,
                                     route_hash_func,
                                     route_compare_func,
                                     route_process_request,
@@ -944,27 +1009,12 @@ int link_request_static_routes(Link *link, bool only_ipv4) {
         return 0;
 }
 
-void route_cancel_request(Route *route, Link *link) {
-        assert(route);
-        Manager *manager = ASSERT_PTR(route->manager ?: ASSERT_PTR(link)->manager);
-
-        if (!route_is_requesting(route))
-                return;
-
-        Request *req;
-        if (route_get_request(manager, route, &req) >= 0)
-                request_detach(req);
-
-        route_cancel_requesting(route);
-}
-
 static int process_route_one(
                 Manager *manager,
                 uint16_t type,
-                Route *in,
+                Route *tmp,
                 const struct rta_cacheinfo *cacheinfo) {
 
-        _cleanup_(route_freep) Route *tmp = in;
         Request *req = NULL;
         Route *route = NULL;
         Link *link = NULL;
@@ -991,13 +1041,13 @@ static int process_route_one(
                         }
 
                         /* If we do not know the route, then save it. */
-                        r = route_add(manager, tmp);
+                        r = route_attach(manager, tmp);
                         if (r < 0) {
                                 log_link_warning_errno(link, r, "Failed to remember foreign route, ignoring: %m");
                                 return 0;
                         }
 
-                        route = TAKE_PTR(tmp);
+                        route = route_ref(tmp);
                         is_new = true;
 
                 } else
@@ -1024,7 +1074,7 @@ static int process_route_one(
                 if (route) {
                         route_enter_removed(route);
                         log_route_debug(route, "Forgetting removed", manager);
-                        route_free(route);
+                        route_detach(route);
                 } else
                         log_route_debug(tmp,
                                         manager->manage_foreign_routes ? "Kernel removed unknown" : "Ignoring received",
@@ -1051,7 +1101,7 @@ static int process_route_one(
 }
 
 int manager_rtnl_process_route(sd_netlink *rtnl, sd_netlink_message *message, Manager *m) {
-        _cleanup_(route_freep) Route *tmp = NULL;
+        _cleanup_(route_unrefp) Route *tmp = NULL;
         int r;
 
         assert(rtnl);
@@ -1194,17 +1244,17 @@ int manager_rtnl_process_route(sd_netlink *rtnl, sd_netlink_message *message, Ma
         has_cacheinfo = r >= 0;
 
         if (tmp->family == AF_INET || ordered_set_isempty(tmp->nexthops))
-                return process_route_one(m, type, TAKE_PTR(tmp), has_cacheinfo ? &cacheinfo : NULL);
+                return process_route_one(m, type, tmp, has_cacheinfo ? &cacheinfo : NULL);
 
         RouteNextHop *nh;
         ORDERED_SET_FOREACH(nh, tmp->nexthops) {
-                _cleanup_(route_freep) Route *dup = NULL;
+                _cleanup_(route_unrefp) Route *dup = NULL;
 
                 r = route_dup(tmp, nh, &dup);
                 if (r < 0)
                         return log_oom();
 
-                r = process_route_one(m, type, TAKE_PTR(dup), has_cacheinfo ? &cacheinfo : NULL);
+                r = process_route_one(m, type, dup, has_cacheinfo ? &cacheinfo : NULL);
                 if (r < 0)
                         return r;
         }
@@ -1252,7 +1302,7 @@ static bool route_by_kernel(const Route *route) {
 }
 
 static int link_unmark_route(Link *link, const Route *route, const RouteNextHop *nh) {
-        _cleanup_(route_freep) Route *tmp = NULL;
+        _cleanup_(route_unrefp) Route *tmp = NULL;
         Route *existing;
         int r;
 
@@ -1376,7 +1426,7 @@ int link_drop_routes(Link *link, bool foreign) {
                 if (!route_is_marked(route))
                         continue;
 
-                RET_GATHER(r, route_remove(route));
+                RET_GATHER(r, route_remove(route, link->manager));
         }
 
         return r;
@@ -1404,7 +1454,7 @@ int link_foreignize_routes(Link *link) {
 }
 
 int network_add_ipv4ll_route(Network *network) {
-        _cleanup_(route_free_or_set_invalidp) Route *route = NULL;
+        _cleanup_(route_unref_or_set_invalidp) Route *route = NULL;
         unsigned section_line;
         int r;
 
@@ -1439,7 +1489,7 @@ int network_add_ipv4ll_route(Network *network) {
 }
 
 int network_add_default_route_on_device(Network *network) {
-        _cleanup_(route_free_or_set_invalidp) Route *route = NULL;
+        _cleanup_(route_unref_or_set_invalidp) Route *route = NULL;
         unsigned section_line;
         int r;
 
@@ -1479,7 +1529,7 @@ int config_parse_preferred_src(
                 void *userdata) {
 
         Network *network = userdata;
-        _cleanup_(route_free_or_set_invalidp) Route *route = NULL;
+        _cleanup_(route_unref_or_set_invalidp) Route *route = NULL;
         int r;
 
         assert(filename);
@@ -1524,7 +1574,7 @@ int config_parse_destination(
                 void *userdata) {
 
         Network *network = userdata;
-        _cleanup_(route_free_or_set_invalidp) Route *route = NULL;
+        _cleanup_(route_unref_or_set_invalidp) Route *route = NULL;
         union in_addr_union *buffer;
         unsigned char *prefixlen;
         int r;
@@ -1582,7 +1632,7 @@ int config_parse_route_priority(
                 void *userdata) {
 
         Network *network = userdata;
-        _cleanup_(route_free_or_set_invalidp) Route *route = NULL;
+        _cleanup_(route_unref_or_set_invalidp) Route *route = NULL;
         int r;
 
         assert(filename);
@@ -1625,7 +1675,7 @@ int config_parse_route_scope(
                 void *userdata) {
 
         Network *network = userdata;
-        _cleanup_(route_free_or_set_invalidp) Route *route = NULL;
+        _cleanup_(route_unref_or_set_invalidp) Route *route = NULL;
         int r;
 
         assert(filename);
@@ -1667,7 +1717,7 @@ int config_parse_route_table(
                 void *data,
                 void *userdata) {
 
-        _cleanup_(route_free_or_set_invalidp) Route *route = NULL;
+        _cleanup_(route_unref_or_set_invalidp) Route *route = NULL;
         Network *network = userdata;
         int r;
 
@@ -1711,7 +1761,7 @@ int config_parse_ipv6_route_preference(
                 void *userdata) {
 
         Network *network = userdata;
-        _cleanup_(route_free_or_set_invalidp) Route *route = NULL;
+        _cleanup_(route_unref_or_set_invalidp) Route *route = NULL;
         int r;
 
         r = route_new_static(network, filename, section_line, &route);
@@ -1752,7 +1802,7 @@ int config_parse_route_protocol(
                 void *userdata) {
 
         Network *network = userdata;
-        _cleanup_(route_free_or_set_invalidp) Route *route = NULL;
+        _cleanup_(route_unref_or_set_invalidp) Route *route = NULL;
         int r;
 
         r = route_new_static(network, filename, section_line, &route);
@@ -1790,7 +1840,7 @@ int config_parse_route_type(
                 void *userdata) {
 
         Network *network = userdata;
-        _cleanup_(route_free_or_set_invalidp) Route *route = NULL;
+        _cleanup_(route_unref_or_set_invalidp) Route *route = NULL;
         int t, r;
 
         r = route_new_static(network, filename, section_line, &route);
@@ -1875,5 +1925,5 @@ void network_drop_invalid_routes(Network *network) {
 
         HASHMAP_FOREACH(route, network->routes_by_section)
                 if (route_section_verify(route) < 0)
-                        route_free(route);
+                        route_detach(route);
 }
index 090bf34a61aadf34f4f028ae37169ef23e053c45..c5c2693ac4ff425c8066042ab521ce5b5cda5879 100644 (file)
@@ -35,6 +35,8 @@ struct Route {
         NetworkConfigState state;
         union in_addr_union provider; /* DHCP server or router address */
 
+        unsigned n_ref;
+
         /* rtmsg header */
         int family;
         unsigned char dst_prefixlen;
@@ -79,17 +81,19 @@ struct Route {
 };
 
 extern const struct hash_ops route_hash_ops;
+extern const struct hash_ops route_hash_ops_unref;
 
-Route* route_free(Route *route);
-DEFINE_SECTION_CLEANUP_FUNCTIONS(Route, route_free);
+Route* route_ref(Route *route);
+Route* route_unref(Route *route);
+DEFINE_SECTION_CLEANUP_FUNCTIONS(Route, route_unref);
 
 int route_new(Route **ret);
 int route_new_static(Network *network, const char *filename, unsigned section_line, Route **ret);
 int route_dup(const Route *src, const RouteNextHop *nh, Route **ret);
 
 int route_configure_handler_internal(sd_netlink *rtnl, sd_netlink_message *m, Link *link, Route *route, const char *error_msg);
-int route_remove(Route *route);
-int route_remove_and_drop(Route *route);
+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);
 
@@ -102,7 +106,6 @@ static inline int link_drop_foreign_routes(Link *link) {
 }
 int link_foreignize_routes(Link *link);
 
-void route_cancel_request(Route *route, Link *link);
 int link_request_route(
                 Link *link,
                 const Route *route,
index 064ca53193b47b33c2032b36c947c9a2231a70c8..20c5f91a5d6bf99b5b108fcdc40f45cf96ddc520 100755 (executable)
@@ -4082,7 +4082,9 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities):
             self.assertIn('nexthop via 192.168.5.3 dev veth99 weight 3', output)
         self.assertIn('nexthop via 192.168.20.1 dev dummy98 weight 1', output)
 
-        check_json(networkctl_json())
+        output = networkctl_json()
+        check_json(output)
+        self.assertNotIn('"Destination":[10.10.10.14]', output)
 
     def _test_nexthop(self, manage_foreign_nexthops):
         if not manage_foreign_nexthops:
@@ -4141,6 +4143,11 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities):
         output = networkctl_status('test1')
         self.assertIn('State: routable (configuring)', output)
 
+        # Check if the route which needs nexthop 20 and 21 are forgotten.
+        output = networkctl_json()
+        check_json(output)
+        self.assertNotIn('"Destination":[10.10.10.14]', output)
+
         # Reconfigure the interface that has nexthop with ID 20 and 21,
         # then the route requested by test1 can be configured.
         networkctl_reconfigure('dummy98')