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 <william@wkennington.com>
This commit is contained in:
William A. Kennington III 2019-04-18 17:52:28 -07:00 committed by Yu Watanabe
parent 9f4f7fe3b5
commit 0b1cd3e25a
8 changed files with 62 additions and 17 deletions

View File

@ -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;

View File

@ -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");

View File

@ -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;

View File

@ -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");

View File

@ -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);

View File

@ -0,0 +1,6 @@
[Match]
Name=dummy98
[Network]
Address=149.10.124.58/28
Gateway=149.10.124.60

View File

@ -0,0 +1,6 @@
[Match]
Name=dummy98
[Network]
Address=149.10.124.58/28
Gateway=149.10.124.59

View File

@ -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