From 0b1cd3e25abccdb1112d71995d1915696a233baf Mon Sep 17 00:00:00 2001 From: "William A. Kennington III" Date: Thu, 18 Apr 2019 17:52:28 -0700 Subject: [PATCH] networkd: Routes should take the gateway into account Otherwise, changing the default gateway doesn't purge old gateway routes left on the system during daemon restart. This also fixes removing other foreign gateway routes that don't match the expected configuration. Tested: Changed gateway addresses prior to the patch and they lingered on the system during each reconfiguration. Applied this patch and reconfigured gateways and other routes multiple times and it removed the foreign routes that had gateways that didn't match. Signed-off-by: William A. Kennington III --- src/network/networkd-dhcp6.c | 8 ++++---- src/network/networkd-link.c | 4 ++-- src/network/networkd-manager.c | 4 ++-- src/network/networkd-route.c | 23 ++++++++++++++++------ src/network/networkd-route.h | 6 +++--- .../conf/25-gateway-next-static.network | 6 ++++++ test/test-network/conf/25-gateway-static.network | 6 ++++++ test/test-network/systemd-networkd-tests.py | 22 +++++++++++++++++++++ 8 files changed, 62 insertions(+), 17 deletions(-) create mode 100644 test/test-network/conf/25-gateway-next-static.network create mode 100644 test/test-network/conf/25-gateway-static.network diff --git a/src/network/networkd-dhcp6.c b/src/network/networkd-dhcp6.c index b20aa78..8ad736a 100644 --- a/src/network/networkd-dhcp6.c +++ b/src/network/networkd-dhcp6.c @@ -141,7 +141,7 @@ int dhcp6_lease_pd_prefix_lost(sd_dhcp6_client *client, Link* link) { (void) in_addr_to_string(AF_INET6, &pd_prefix, &buf); - r = route_add(link, AF_INET6, &pd_prefix, pd_prefix_len, 0, 0, 0, &route); + r = route_add(link, AF_INET6, &pd_prefix, pd_prefix_len, NULL, 0, 0, 0, &route); if (r < 0) { log_link_warning_errno(link, r, "Failed to add unreachable route to delete for DHCPv6 delegated subnet %s/%u: %m", strnull(buf), @@ -295,7 +295,7 @@ static int dhcp6_lease_pd_prefix_acquired(sd_dhcp6_client *client, Link *link) { table = link_get_dhcp_route_table(link); - r = route_add(link, AF_INET6, &pd_prefix, pd_prefix_len, 0, 0, table, &route); + r = route_add(link, AF_INET6, &pd_prefix, pd_prefix_len, NULL, 0, 0, table, &route); if (r < 0) { log_link_warning_errno(link, r, "Failed to add unreachable route for DHCPv6 delegated subnet %s/%u: %m", strnull(buf), @@ -732,7 +732,7 @@ static int dhcp6_prefix_add(Manager *m, struct in6_addr *addr, Link *link) { assert_return(addr, -EINVAL); r = route_add(link, AF_INET6, (union in_addr_union *) addr, 64, - 0, 0, 0, &route); + NULL, 0, 0, 0, &route); if (r < 0) return r; @@ -799,7 +799,7 @@ int dhcp6_prefix_remove(Manager *m, struct in6_addr *addr) { return -EINVAL; (void) sd_radv_remove_prefix(l->radv, addr, 64); - r = route_get(l, AF_INET6, (union in_addr_union *) addr, 64, 0, 0, 0, &route); + r = route_get(l, AF_INET6, (union in_addr_union *) addr, 64, NULL, 0, 0, 0, &route); if (r < 0) return r; diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 17d37c1..5c01494 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -2441,7 +2441,7 @@ static int link_drop_foreign_config(Link *link) { continue; if (link_is_static_route_configured(link, route)) { - r = route_add(link, route->family, &route->dst, route->dst_prefixlen, route->tos, route->priority, route->table, NULL); + r = route_add(link, route->family, &route->dst, route->dst_prefixlen, &route->gw, route->tos, route->priority, route->table, NULL); if (r < 0) return r; } else { @@ -3014,7 +3014,7 @@ network_file_fail: continue; } - r = route_add(link, family, &route_dst, prefixlen, tos, priority, table, &route); + r = route_add(link, family, &route_dst, prefixlen, NULL, tos, priority, table, &route); if (r < 0) return log_link_error_errno(link, r, "Failed to add route: %m"); diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index 6037e85..74266ff 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -432,7 +432,7 @@ int manager_rtnl_process_route(sd_netlink *rtnl, sd_netlink_message *message, vo return 0; } - (void) route_get(link, family, &dst, dst_prefixlen, tos, priority, table, &route); + (void) route_get(link, family, &dst, dst_prefixlen, &gw, tos, priority, table, &route); if (DEBUG_LOGGING) { _cleanup_free_ char *buf_dst = NULL, *buf_dst_prefixlen = NULL, @@ -466,7 +466,7 @@ int manager_rtnl_process_route(sd_netlink *rtnl, sd_netlink_message *message, vo case RTM_NEWROUTE: if (!route) { /* A route appeared that we did not request */ - r = route_add_foreign(link, family, &dst, dst_prefixlen, tos, priority, table, &route); + r = route_add_foreign(link, family, &dst, dst_prefixlen, &gw, tos, priority, table, &route); if (r < 0) { log_link_warning_errno(link, r, "Failed to remember foreign route, ignoring: %m"); return 0; diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 25ab527..8cc8080 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -198,7 +198,11 @@ static int route_compare_func(const Route *a, const Route *b) { if (r != 0) return r; - return memcmp(&a->dst, &b->dst, FAMILY_ADDRESS_SIZE(a->family)); + r = memcmp(&a->dst, &b->dst, FAMILY_ADDRESS_SIZE(a->family)); + if (r != 0) + return r; + + return memcmp(&a->gw, &b->gw, FAMILY_ADDRESS_SIZE(a->family)); default: /* treat any other address family as AF_UNSPEC */ return 0; @@ -318,6 +322,7 @@ int route_get(Link *link, int family, const union in_addr_union *dst, unsigned char dst_prefixlen, + const union in_addr_union *gw, unsigned char tos, uint32_t priority, uint32_t table, @@ -332,6 +337,7 @@ int route_get(Link *link, .family = family, .dst = *dst, .dst_prefixlen = dst_prefixlen, + .gw = gw ? *gw : IN_ADDR_NULL, .tos = tos, .priority = priority, .table = table, @@ -360,6 +366,7 @@ static int route_add_internal( int family, const union in_addr_union *dst, unsigned char dst_prefixlen, + const union in_addr_union *gw, unsigned char tos, uint32_t priority, uint32_t table, @@ -379,6 +386,8 @@ static int route_add_internal( route->family = family; route->dst = *dst; route->dst_prefixlen = dst_prefixlen; + route->dst = *dst; + route->gw = gw ? *gw : IN_ADDR_NULL; route->tos = tos; route->priority = priority; route->table = table; @@ -406,18 +415,20 @@ int route_add_foreign( int family, const union in_addr_union *dst, unsigned char dst_prefixlen, + const union in_addr_union *gw, unsigned char tos, uint32_t priority, uint32_t table, Route **ret) { - return route_add_internal(link, &link->routes_foreign, family, dst, dst_prefixlen, tos, priority, table, ret); + return route_add_internal(link, &link->routes_foreign, family, dst, dst_prefixlen, gw, tos, priority, table, ret); } int route_add(Link *link, int family, const union in_addr_union *dst, unsigned char dst_prefixlen, + const union in_addr_union *gw, unsigned char tos, uint32_t priority, uint32_t table, @@ -426,10 +437,10 @@ int route_add(Link *link, Route *route; int r; - r = route_get(link, family, dst, dst_prefixlen, tos, priority, table, &route); + r = route_get(link, family, dst, dst_prefixlen, gw, tos, priority, table, &route); if (r == -ENOENT) { /* Route does not exist, create a new one */ - r = route_add_internal(link, &link->routes, family, dst, dst_prefixlen, tos, priority, table, &route); + r = route_add_internal(link, &link->routes, family, dst, dst_prefixlen, gw, tos, priority, table, &route); if (r < 0) return r; } else if (r == 0) { @@ -628,7 +639,7 @@ int route_configure( return 0; } - if (route_get(link, route->family, &route->dst, route->dst_prefixlen, route->tos, route->priority, route->table, NULL) <= 0 && + if (route_get(link, route->family, &route->dst, route->dst_prefixlen, &route->gw, route->tos, route->priority, route->table, NULL) <= 0 && set_size(link->routes) >= routes_max()) return log_link_error_errno(link, SYNTHETIC_ERRNO(E2BIG), "Too many routes are configured, refusing: %m"); @@ -804,7 +815,7 @@ int route_configure( lifetime = route->lifetime; - r = route_add(link, route->family, &route->dst, route->dst_prefixlen, route->tos, route->priority, route->table, &route); + r = route_add(link, route->family, &route->dst, route->dst_prefixlen, &route->gw, route->tos, route->priority, route->table, &route); if (r < 0) return log_link_error_errno(link, r, "Could not add route: %m"); diff --git a/src/network/networkd-route.h b/src/network/networkd-route.h index 9d9c980..9bd4991 100644 --- a/src/network/networkd-route.h +++ b/src/network/networkd-route.h @@ -56,9 +56,9 @@ void route_free(Route *route); int route_configure(Route *route, Link *link, link_netlink_message_handler_t callback); int route_remove(Route *route, Link *link, link_netlink_message_handler_t callback); -int route_get(Link *link, int family, const union in_addr_union *dst, unsigned char dst_prefixlen, unsigned char tos, uint32_t priority, uint32_t table, Route **ret); -int route_add(Link *link, int family, const union in_addr_union *dst, unsigned char dst_prefixlen, unsigned char tos, uint32_t priority, uint32_t table, Route **ret); -int route_add_foreign(Link *link, int family, const union in_addr_union *dst, unsigned char dst_prefixlen, unsigned char tos, uint32_t priority, uint32_t table, Route **ret); +int route_get(Link *link, int family, const union in_addr_union *dst, unsigned char dst_prefixlen, const union in_addr_union *gw, unsigned char tos, uint32_t priority, uint32_t table, Route **ret); +int route_add(Link *link, int family, const union in_addr_union *dst, unsigned char dst_prefixlen, const union in_addr_union *gw, unsigned char tos, uint32_t priority, uint32_t table, Route **ret); +int route_add_foreign(Link *link, int family, const union in_addr_union *dst, unsigned char dst_prefixlen, const union in_addr_union *gw, unsigned char tos, uint32_t priority, uint32_t table, Route **ret); void route_update(Route *route, const union in_addr_union *src, unsigned char src_prefixlen, const union in_addr_union *gw, const union in_addr_union *prefsrc, unsigned char scope, unsigned char protocol, unsigned char type); bool route_equal(Route *r1, Route *r2); diff --git a/test/test-network/conf/25-gateway-next-static.network b/test/test-network/conf/25-gateway-next-static.network new file mode 100644 index 0000000..dfac8f4 --- /dev/null +++ b/test/test-network/conf/25-gateway-next-static.network @@ -0,0 +1,6 @@ +[Match] +Name=dummy98 + +[Network] +Address=149.10.124.58/28 +Gateway=149.10.124.60 diff --git a/test/test-network/conf/25-gateway-static.network b/test/test-network/conf/25-gateway-static.network new file mode 100644 index 0000000..448a21f --- /dev/null +++ b/test/test-network/conf/25-gateway-static.network @@ -0,0 +1,6 @@ +[Match] +Name=dummy98 + +[Network] +Address=149.10.124.58/28 +Gateway=149.10.124.59 diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 76313cf..f133bf4 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -1430,6 +1430,8 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities): '25-link-section-unmanaged.network', '25-route-ipv6-src.network', '25-route-static.network', + '25-gateway-static.network', + '25-gateway-next-static.network', '25-sysctl-disable-ipv6.network', '25-sysctl.network', 'configure-without-carrier.network', @@ -1632,6 +1634,26 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities): print(output) self.assertRegex(output, 'prohibit 202.54.1.4 proto static') + def test_gateway_reconfigure(self): + copy_unit_to_networkd_unit_path('25-gateway-static.network', '12-dummy.netdev') + start_networkd() + self.wait_online(['dummy98:routable']) + print('### ip -4 route show dev dummy98 default') + output = check_output('ip -4 route show dev dummy98 default') + print(output) + self.assertRegex(output, 'default via 149.10.124.59 proto static') + self.assertNotRegex(output, '149.10.124.60') + + remove_unit_from_networkd_path(['25-gateway-static.network']) + copy_unit_to_networkd_unit_path('25-gateway-next-static.network') + restart_networkd(3) + self.wait_online(['dummy98:routable']) + print('### ip -4 route show dev dummy98 default') + output = check_output('ip -4 route show dev dummy98 default') + print(output) + self.assertNotRegex(output, '149.10.124.59') + self.assertRegex(output, 'default via 149.10.124.60 proto static') + def test_ip_route_ipv6_src_route(self): # a dummy device does not make the addresses go through tentative state, so we # reuse a bond from an earlier test, which does make the addresses go through -- 2.7.4