From 9b3e49fc004e5966804e7b181c9cb92b403c50b7 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 23 Oct 2020 17:29:49 +0900 Subject: [PATCH 1/6] network: fix typo --- src/network/networkd-sysctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/network/networkd-sysctl.c b/src/network/networkd-sysctl.c index de4ed95981e..b6839f86eef 100644 --- a/src/network/networkd-sysctl.c +++ b/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) From b1dc5946e2b1ff52c2aeefb7aadac2b9a0904e52 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 23 Oct 2020 17:32:19 +0900 Subject: [PATCH 2/6] network: ignore error in writing proxy_ndp This partially reverts the commit fd773a11d8e2b01e9e6d234ca5693417c0101fca. As, IPv6 may be disabled by kernel. --- src/network/networkd-ipv6-proxy-ndp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/network/networkd-ipv6-proxy-ndp.c b/src/network/networkd-ipv6-proxy-ndp.c index fb8464ff2fa..ce193c0e506 100644 --- a/src/network/networkd-ipv6-proxy-ndp.c +++ b/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); From bac319a79569f80930ea10f9e6caefcf23d24f78 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 23 Oct 2020 22:19:16 +0900 Subject: [PATCH 3/6] network: link may be NULL --- src/network/networkd-route.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index dd05e11067b..075799a7f06 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -701,7 +701,7 @@ int route_remove( 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) From 5f4d7aa45880f0f7741a4547050f785ee04c8357 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 23 Oct 2020 22:40:57 +0900 Subject: [PATCH 4/6] network: specify route type in route_remove() This factors out the common netlink message handling in route_remote() and route_configure() to route_set_netlink_message(). --- src/network/networkd-route.c | 254 +++++++++++++++-------------------- 1 file changed, 108 insertions(+), 146 deletions(-) diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 075799a7f06..148520976e2 100644 --- a/src/network/networkd-route.c +++ b/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,64 +746,9 @@ 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"); - - 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"); - } + return r; r = netlink_call_async(manager->rtnl, NULL, req, callback ?: route_remove_handler, @@ -930,7 +979,6 @@ int route_configure( Route **ret) { _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *req = NULL; - unsigned flags; int r; assert(link); @@ -974,85 +1022,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 +1033,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) From 1132a714ec44c9e244e08b89744122818949ea35 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 23 Oct 2020 17:01:30 +0900 Subject: [PATCH 5/6] network: drop routes managed by Manager when link is removed Also, foreign routes managed by Manager are dropped in link_drop_foreign_routes(). --- src/network/networkd-route.c | 79 +++++++++++++++++++++++++++++++++++- 1 file changed, 77 insertions(+), 2 deletions(-) diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 148520976e2..cd0f0aa707d 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -761,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); @@ -777,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 */ @@ -805,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); @@ -813,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; } @@ -832,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; } From 43d4bc9f561e4c593abfc71324fcd6b537d01a69 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 23 Oct 2020 23:06:13 +0900 Subject: [PATCH 6/6] test-network: add tests for routes managed by Manager --- test/test-network/systemd-networkd-tests.py | 60 +++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 752b2fee435..499677e6e6d 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/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')