Merge pull request #17430 from yuwata/network-drop-foreign-routes-managed-by-manager

network: drop routes managed by Manager when they are not requested

src/network/networkd-ipv6-proxy-ndp.c

@@ -84,7 +84,7 @@ static int ipv6_proxy_ndp_set(Link *link) {
 
         r = sysctl_write_ip_property_boolean(AF_INET6, link->ifname, "proxy_ndp", v);
         if (r < 0)
-                return log_link_warning_errno(link, r, "Cannot configure proxy NDP for the interface: %m");
+                return log_link_warning_errno(link, r, "Cannot configure proxy NDP for the interface, ignoring: %m");
 
         return v;
 }
@@ -100,7 +100,7 @@ int link_set_ipv6_proxy_ndp_addresses(Link *link) {
         /* enable or disable proxy_ndp itself depending on whether ipv6_proxy_ndp_addresses are set or not */
         r = ipv6_proxy_ndp_set(link);
         if (r <= 0)
-                return r;
+                return 0;
 
         SET_FOREACH(address, link->network->ipv6_proxy_ndp_addresses) {
                 r = ipv6_proxy_ndp_address_configure(link, address);

src/network/networkd-route.c

@@ -581,6 +581,110 @@ static int route_add(Manager *manager, Link *link, const Route *in, const Multip
         return 0;
 }
 
+static int route_set_netlink_message(const Route *route, sd_netlink_message *req, Link *link) {
+        unsigned flags;
+        int r;
+
+        assert(route);
+        assert(req);
+
+        /* link may be NULL */
+
+        if (in_addr_is_null(route->gw_family, &route->gw) == 0) {
+                if (route->gw_family == route->family) {
+                        r = netlink_message_append_in_addr_union(req, RTA_GATEWAY, route->gw_family, &route->gw);
+                        if (r < 0)
+                                return log_link_error_errno(link, r, "Could not append RTA_GATEWAY attribute: %m");
+                } else {
+                        RouteVia rtvia = {
+                                .family = route->gw_family,
+                                .address = route->gw,
+                        };
+
+                        r = sd_netlink_message_append_data(req, RTA_VIA, &rtvia, sizeof(rtvia));
+                        if (r < 0)
+                                return log_link_error_errno(link, r, "Could not append RTA_VIA attribute: %m");
+                }
+        }
+
+        if (route->dst_prefixlen > 0) {
+                r = netlink_message_append_in_addr_union(req, RTA_DST, route->family, &route->dst);
+                if (r < 0)
+                        return log_link_error_errno(link, r, "Could not append RTA_DST attribute: %m");
+
+                r = sd_rtnl_message_route_set_dst_prefixlen(req, route->dst_prefixlen);
+                if (r < 0)
+                        return log_link_error_errno(link, r, "Could not set destination prefix length: %m");
+        }
+
+        if (route->src_prefixlen > 0) {
+                r = netlink_message_append_in_addr_union(req, RTA_SRC, route->family, &route->src);
+                if (r < 0)
+                        return log_link_error_errno(link, r, "Could not append RTA_SRC attribute: %m");
+
+                r = sd_rtnl_message_route_set_src_prefixlen(req, route->src_prefixlen);
+                if (r < 0)
+                        return log_link_error_errno(link, r, "Could not set source prefix length: %m");
+        }
+
+        if (in_addr_is_null(route->family, &route->prefsrc) == 0) {
+                r = netlink_message_append_in_addr_union(req, RTA_PREFSRC, route->family, &route->prefsrc);
+                if (r < 0)
+                        return log_link_error_errno(link, r, "Could not append RTA_PREFSRC attribute: %m");
+        }
+
+        r = sd_rtnl_message_route_set_scope(req, route->scope);
+        if (r < 0)
+                return log_link_error_errno(link, r, "Could not set scope: %m");
+
+        flags = route->flags;
+        if (route->gateway_onlink >= 0)
+                SET_FLAG(flags, RTNH_F_ONLINK, route->gateway_onlink);
+
+        r = sd_rtnl_message_route_set_flags(req, flags);
+        if (r < 0)
+                return log_link_error_errno(link, r, "Could not set flags: %m");
+
+        if (route->table != RT_TABLE_MAIN) {
+                if (route->table < 256) {
+                        r = sd_rtnl_message_route_set_table(req, route->table);
+                        if (r < 0)
+                                return log_link_error_errno(link, r, "Could not set route table: %m");
+                } else {
+                        r = sd_rtnl_message_route_set_table(req, RT_TABLE_UNSPEC);
+                        if (r < 0)
+                                return log_link_error_errno(link, r, "Could not set route table: %m");
+
+                        /* Table attribute to allow more than 256. */
+                        r = sd_netlink_message_append_data(req, RTA_TABLE, &route->table, sizeof(route->table));
+                        if (r < 0)
+                                return log_link_error_errno(link, r, "Could not append RTA_TABLE attribute: %m");
+                }
+        }
+
+        r = sd_rtnl_message_route_set_type(req, route->type);
+        if (r < 0)
+                return log_link_error_errno(link, r, "Could not set route type: %m");
+
+        if (!IN_SET(route->type, RTN_UNREACHABLE, RTN_PROHIBIT, RTN_BLACKHOLE, RTN_THROW)) {
+                assert(link); /* Those routes must be attached to a specific link */
+
+                r = sd_netlink_message_append_u32(req, RTA_OIF, link->ifindex);
+                if (r < 0)
+                        return log_link_error_errno(link, r, "Could not append RTA_OIF attribute: %m");
+        }
+
+        r = sd_netlink_message_append_u8(req, RTA_PREF, route->pref);
+        if (r < 0)
+                return log_link_error_errno(link, r, "Could not append RTA_PREF attribute: %m");
+
+        r = sd_netlink_message_append_u32(req, RTA_PRIORITY, route->priority);
+        if (r < 0)
+                return log_link_error_errno(link, r, "Could not append RTA_PRIORITY attribute: %m");
+
+        return 0;
+}
+
 static int route_remove_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) {
         int r;
 
@@ -642,66 +746,11 @@ int route_remove(
                                strna(route_type_to_string(route->type)));
         }
 
-        if (in_addr_is_null(route->gw_family, &route->gw) == 0) {
-                if (route->gw_family == route->family) {
-                        r = netlink_message_append_in_addr_union(req, RTA_GATEWAY, route->gw_family, &route->gw);
-                        if (r < 0)
-                                return log_link_error_errno(link, r, "Could not append RTA_GATEWAY attribute: %m");
-                } else {
-                        RouteVia rtvia = {
-                                .family = route->gw_family,
-                                .address = route->gw,
-                        };
-
-                        r = sd_netlink_message_append_data(req, RTA_VIA, &rtvia, sizeof(rtvia));
-                        if (r < 0)
-                                return log_link_error_errno(link, r, "Could not append RTA_VIA attribute: %m");
-                }
-        }
-
-        if (route->dst_prefixlen) {
-                r = netlink_message_append_in_addr_union(req, RTA_DST, route->family, &route->dst);
-                if (r < 0)
-                        return log_link_error_errno(link, r, "Could not append RTA_DST attribute: %m");
-
-                r = sd_rtnl_message_route_set_dst_prefixlen(req, route->dst_prefixlen);
-                if (r < 0)
-                        return log_link_error_errno(link, r, "Could not set destination prefix length: %m");
-        }
-
-        if (route->src_prefixlen) {
-                r = netlink_message_append_in_addr_union(req, RTA_SRC, route->family, &route->src);
-                if (r < 0)
-                        return log_link_error_errno(link, r, "Could not append RTA_SRC attribute: %m");
-
-                r = sd_rtnl_message_route_set_src_prefixlen(req, route->src_prefixlen);
-                if (r < 0)
-                        return log_link_error_errno(link, r, "Could not set source prefix length: %m");
-        }
-
-        if (in_addr_is_null(route->family, &route->prefsrc) == 0) {
-                r = netlink_message_append_in_addr_union(req, RTA_PREFSRC, route->family, &route->prefsrc);
-                if (r < 0)
-                        return log_link_error_errno(link, r, "Could not append RTA_PREFSRC attribute: %m");
-        }
-
-        r = sd_rtnl_message_route_set_scope(req, route->scope);
+        r = route_set_netlink_message(route, req, link);
         if (r < 0)
-                return log_link_error_errno(link, r, "Could not set scope: %m");
+                return r;
 
-        r = sd_netlink_message_append_u32(req, RTA_PRIORITY, route->priority);
-        if (r < 0)
-                return log_link_error_errno(link, r, "Could not append RTA_PRIORITY attribute: %m");
-
-        if (!IN_SET(route->type, RTN_UNREACHABLE, RTN_PROHIBIT, RTN_BLACKHOLE, RTN_THROW)) {
-                assert(link); /* Those routes must be attached to a specific link */
-
-                r = sd_netlink_message_append_u32(req, RTA_OIF, link->ifindex);
-                if (r < 0)
-                        return log_link_error_errno(link, r, "Could not append RTA_OIF attribute: %m");
-        }
-
-        r = netlink_call_async(link->manager->rtnl, NULL, req,
+        r = netlink_call_async(manager->rtnl, NULL, req,
                                callback ?: route_remove_handler,
                                link_netlink_destroy_callback, link);
         if (r < 0)
@@ -712,7 +761,7 @@ int route_remove(
         return 0;
 }
 
-static bool link_is_static_route_configured(const Link *link, const Route *route) {
+static bool link_has_route(const Link *link, const Route *route) {
         Route *net_route;
 
         assert(link);
@@ -728,11 +777,78 @@ static bool link_is_static_route_configured(const Link *link, const Route *route
         return false;
 }
 
+static bool links_have_route(Manager *manager, const Route *route, const Link *except) {
+        Link *link;
+
+        assert(manager);
+
+        HASHMAP_FOREACH(link, manager->links) {
+                if (link == except)
+                        continue;
+
+                if (link_has_route(link, route))
+                        return true;
+        }
+
+        return false;
+}
+
+static int manager_drop_foreign_routes(Manager *manager) {
+        Route *route;
+        int k, r = 0;
+
+        assert(manager);
+
+        SET_FOREACH(route, manager->routes_foreign) {
+                /* do not touch routes managed by the kernel */
+                if (route->protocol == RTPROT_KERNEL)
+                        continue;
+
+                if (links_have_route(manager, route, NULL))
+                        /* The route will be configured later. */
+                        continue;
+
+                /* The existing links do not have the route. Let's drop this now. It may by
+                 * re-configured later. */
+                k = route_remove(route, manager, NULL, NULL);
+                if (k < 0 && r >= 0)
+                        r = k;
+        }
+
+        return r;
+}
+
+static int manager_drop_routes(Manager *manager, Link *except) {
+        Route *route;
+        int k, r = 0;
+
+        assert(manager);
+
+        SET_FOREACH(route, manager->routes) {
+                /* do not touch routes managed by the kernel */
+                if (route->protocol == RTPROT_KERNEL)
+                        continue;
+
+                if (links_have_route(manager, route, except))
+                        /* The route will be configured later. */
+                        continue;
+
+                /* The existing links do not have the route. Let's drop this now. It may by
+                 * re-configured later. */
+                k = route_remove(route, manager, NULL, NULL);
+                if (k < 0 && r >= 0)
+                        r = k;
+        }
+
+        return r;
+}
+
 int link_drop_foreign_routes(Link *link) {
         Route *route;
         int k, r = 0;
 
         assert(link);
+        assert(link->manager);
 
         SET_FOREACH(route, link->routes_foreign) {
                 /* do not touch routes managed by the kernel */
@@ -756,7 +872,7 @@ int link_drop_foreign_routes(Link *link) {
                     FLAGS_SET(link->network->keep_configuration, KEEP_CONFIGURATION_DHCP))
                         continue;
 
-                if (link_is_static_route_configured(link, route))
+                if (link_has_route(link, route))
                         k = route_add(NULL, link, route, NULL, NULL);
                 else
                         k = route_remove(route, NULL, link, NULL);
@@ -764,6 +880,10 @@ int link_drop_foreign_routes(Link *link) {
                         r = k;
         }
 
+        k = manager_drop_foreign_routes(link->manager);
+        if (k < 0 && r >= 0)
+                r = k;
+
         return r;
 }
 
@@ -783,6 +903,10 @@ int link_drop_routes(Link *link) {
                         r = k;
         }
 
+        k = manager_drop_routes(link->manager, link);
+        if (k < 0 && r >= 0)
+                r = k;
+
         return r;
 }
 
@@ -930,7 +1054,6 @@ int route_configure(
                 Route **ret) {
 
         _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *req = NULL;
-        unsigned flags;
         int r;
 
         assert(link);
@@ -974,85 +1097,9 @@ int route_configure(
         if (r < 0)
                 return log_link_error_errno(link, r, "Could not create RTM_NEWROUTE message: %m");
 
-        if (in_addr_is_null(route->gw_family, &route->gw) == 0) {
-                if (route->gw_family == route->family) {
-                        r = netlink_message_append_in_addr_union(req, RTA_GATEWAY, route->gw_family, &route->gw);
-                        if (r < 0)
-                                return log_link_error_errno(link, r, "Could not append RTA_GATEWAY attribute: %m");
-                } else {
-                        RouteVia rtvia = {
-                                .family = route->gw_family,
-                                .address = route->gw,
-                        };
-
-                        r = sd_netlink_message_append_data(req, RTA_VIA, &rtvia, sizeof(rtvia));
-                        if (r < 0)
-                                return log_link_error_errno(link, r, "Could not append RTA_VIA attribute: %m");
-                }
-        }
-
-        if (route->dst_prefixlen > 0) {
-                r = netlink_message_append_in_addr_union(req, RTA_DST, route->family, &route->dst);
-                if (r < 0)
-                        return log_link_error_errno(link, r, "Could not append RTA_DST attribute: %m");
-
-                r = sd_rtnl_message_route_set_dst_prefixlen(req, route->dst_prefixlen);
-                if (r < 0)
-                        return log_link_error_errno(link, r, "Could not set destination prefix length: %m");
-        }
-
-        if (route->src_prefixlen > 0) {
-                r = netlink_message_append_in_addr_union(req, RTA_SRC, route->family, &route->src);
-                if (r < 0)
-                        return log_link_error_errno(link, r, "Could not append RTA_SRC attribute: %m");
-
-                r = sd_rtnl_message_route_set_src_prefixlen(req, route->src_prefixlen);
-                if (r < 0)
-                        return log_link_error_errno(link, r, "Could not set source prefix length: %m");
-        }
-
-        if (in_addr_is_null(route->family, &route->prefsrc) == 0) {
-                r = netlink_message_append_in_addr_union(req, RTA_PREFSRC, route->family, &route->prefsrc);
-                if (r < 0)
-                        return log_link_error_errno(link, r, "Could not append RTA_PREFSRC attribute: %m");
-        }
-
-        r = sd_rtnl_message_route_set_scope(req, route->scope);
+        r = route_set_netlink_message(route, req, link);
         if (r < 0)
-                return log_link_error_errno(link, r, "Could not set scope: %m");
-
-        flags = route->flags;
-        if (route->gateway_onlink >= 0)
-                SET_FLAG(flags, RTNH_F_ONLINK, route->gateway_onlink);
-
-        r = sd_rtnl_message_route_set_flags(req, flags);
-        if (r < 0)
-                return log_link_error_errno(link, r, "Could not set flags: %m");
-
-        if (route->table != RT_TABLE_MAIN) {
-                if (route->table < 256) {
-                        r = sd_rtnl_message_route_set_table(req, route->table);
-                        if (r < 0)
-                                return log_link_error_errno(link, r, "Could not set route table: %m");
-                } else {
-                        r = sd_rtnl_message_route_set_table(req, RT_TABLE_UNSPEC);
-                        if (r < 0)
-                                return log_link_error_errno(link, r, "Could not set route table: %m");
-
-                        /* Table attribute to allow more than 256. */
-                        r = sd_netlink_message_append_data(req, RTA_TABLE, &route->table, sizeof(route->table));
-                        if (r < 0)
-                                return log_link_error_errno(link, r, "Could not append RTA_TABLE attribute: %m");
-                }
-        }
-
-        r = sd_netlink_message_append_u32(req, RTA_PRIORITY, route->priority);
-        if (r < 0)
-                return log_link_error_errno(link, r, "Could not append RTA_PRIORITY attribute: %m");
-
-        r = sd_netlink_message_append_u8(req, RTA_PREF, route->pref);
-        if (r < 0)
-                return log_link_error_errno(link, r, "Could not append RTA_PREF attribute: %m");
+                return r;
 
         if (route->lifetime != USEC_INFINITY && kernel_route_expiration_supported()) {
                 r = sd_netlink_message_append_u32(req, RTA_EXPIRES,
@@ -1061,16 +1108,6 @@ int route_configure(
                         return log_link_error_errno(link, r, "Could not append RTA_EXPIRES attribute: %m");
         }
 
-        r = sd_rtnl_message_route_set_type(req, route->type);
-        if (r < 0)
-                return log_link_error_errno(link, r, "Could not set route type: %m");
-
-        if (!IN_SET(route->type, RTN_UNREACHABLE, RTN_PROHIBIT, RTN_BLACKHOLE, RTN_THROW)) {
-                r = sd_netlink_message_append_u32(req, RTA_OIF, link->ifindex);
-                if (r < 0)
-                        return log_link_error_errno(link, r, "Could not append RTA_OIF attribute: %m");
-        }
-
         if (route->ttl_propagate >= 0) {
                 r = sd_netlink_message_append_u8(req, RTA_TTL_PROPAGATE, route->ttl_propagate);
                 if (r < 0)

src/network/networkd-sysctl.c

@@ -194,7 +194,7 @@ int link_set_sysctl(Link *link) {
 
         r = link_set_ipv6_privacy_extensions(link);
         if (r < 0)
-                log_link_warning_errno(link, r, "Cannot configure IPv6 privacy extension for interface, ignorign: %m");
+                log_link_warning_errno(link, r, "Cannot configure IPv6 privacy extension for interface, ignoring: %m");
 
         r = link_set_ipv6_accept_ra(link);
         if (r < 0)

test/test-network/systemd-networkd-tests.py

@@ -2230,6 +2230,66 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities):
         self.assertRegex(output, 'via 2001:1234:5:8fff:ff:ff:ff:ff dev dummy98')
         self.assertRegex(output, 'via 2001:1234:5:9fff:ff:ff:ff:ff dev dummy98')
 
+        copy_unit_to_networkd_unit_path('25-address-static.network')
+        check_output(*networkctl_cmd, 'reload', env=env)
+        self.wait_online(['dummy98:routable'])
+
+        # check all routes managed by Manager are removed
+        print('### ip route show type blackhole')
+        output = check_output('ip route show type blackhole')
+        print(output)
+        self.assertEqual(output, '')
+
+        print('### ip route show type unreachable')
+        output = check_output('ip route show type unreachable')
+        print(output)
+        self.assertEqual(output, '')
+
+        print('### ip route show type prohibit')
+        output = check_output('ip route show type prohibit')
+        print(output)
+        self.assertEqual(output, '')
+
+        remove_unit_from_networkd_path(['25-address-static.network'])
+        check_output(*networkctl_cmd, 'reload', env=env)
+        self.wait_online(['dummy98:routable'])
+
+        # check all routes managed by Manager are reconfigured
+        print('### ip route show type blackhole')
+        output = check_output('ip route show type blackhole')
+        print(output)
+        self.assertRegex(output, 'blackhole 202.54.1.2 proto static')
+
+        print('### ip route show type unreachable')
+        output = check_output('ip route show type unreachable')
+        print(output)
+        self.assertRegex(output, 'unreachable 202.54.1.3 proto static')
+
+        print('### ip route show type prohibit')
+        output = check_output('ip route show type prohibit')
+        print(output)
+        self.assertRegex(output, 'prohibit 202.54.1.4 proto static')
+
+        rc = call("ip link del dummy98")
+        self.assertEqual(rc, 0)
+        time.sleep(2)
+
+        # check all routes managed by Manager are removed
+        print('### ip route show type blackhole')
+        output = check_output('ip route show type blackhole')
+        print(output)
+        self.assertEqual(output, '')
+
+        print('### ip route show type unreachable')
+        output = check_output('ip route show type unreachable')
+        print(output)
+        self.assertEqual(output, '')
+
+        print('### ip route show type prohibit')
+        output = check_output('ip route show type prohibit')
+        print(output)
+        self.assertEqual(output, '')
+
     @expectedFailureIfRTA_VIAIsNotSupported()
     def test_route_via_ipv6(self):
         copy_unit_to_networkd_unit_path('25-route-via-ipv6.network', '12-dummy.netdev')