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>

diff --git a/src/network/networkd-dhcp6.c b/src/network/networkd-dhcp6.c
index b20aa78508400bcd4db6a0d7422afd8b742283ee..8ad736a82b3c4539a2777192b1eb84a7323770e9 100644 (file)
--- 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 17d37c1617232abb5b93555ff1d0bb5479950a07..5c0149492f0867e7805f3d43e52046ac6b704e4a 100644 (file)
--- 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 6037e8564fd65b8971f505f5a34c8b1db010cb08..74266ff12ae002f68f98999b0466f19033ee4d3a 100644 (file)
--- 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 25ab527284db5a704a06af52e2979b5b0840c69d..8cc8080b46521f884a72da8d9f3b6922e9237c60 100644 (file)
--- 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 9d9c980d903a96f8636b67a1303e075381d8836a..9bd49915206585cc3743fa7e2dc20500271a1273 100644 (file)
--- 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 (file)
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 (file)
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 76313cf026583c233b4acf194bc71765a0bf5771..f133bf4462fc94886d817c9fe881f11fd7ab4839 100755 (executable)
--- 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