network: don't forget old RAs when a new one arrives

IPv6 Neighbor Discovery lets us autoconfigure a link's IPv6 addresses,
routes, DNS servers, and DNS search domains by listening for Router
Advertisement (RA) packets broadcast by one or more routers on the link.
Each RA can contain zero or more "options," each describing one piece of
configuration (e.g. a single route).

Currently, when we receive an RA from a router, we delete any addresses,
routes, etc. that originated from that router's previous RAs unless
they're also present as options in the new RA.

That behavior is a violation of RFC 4861[1]. In Section 9, the RFC
states that

    Senders MAY send a subset of options in different packets. ... Thus,
    a receiver MUST NOT associate any action with the absence of an
    option in a particular packet. This protocol specifies that
    receivers should only act on the expiration of timers and on the
    information that is received in the packets.

Several other passages in the RFC reiterate this. Section 6.2.3:

    A router MAY choose not to include some or all options when sending
    unsolicited Router Advertisements.

Section 6.3.4:

    Hosts accept the union of all received information; the receipt of a
    Router Advertisement MUST NOT invalidate all information received in
    a previous advertisement or from another source.

At least one consumer router in production today, the Google Nest Wifi,
often sends RAs that omit its global IPv6 prefix. When current versions
of systemd-networkd receive those RAs, they immediately delete the
interface's global IPv6 address, which breaks IPv6 connectivity.

Fix the issue by removing the invalidation logic entirely. It's not
needed at all, since we already invalidate addresses, routes, and DNS
configuration when the interface goes down or their lifetimes expire.

This fix does have the side effect of preventing changes to the .network
file (e.g. denylisted prefixes, whether to add routes from RAs) from
taking effect as soon as a new RA arrives. Instead, a full interface
reconfiguration is needed. But triggering those changes on RA receipt
was already rather arbitrary and out of the administrator's control, so
I think this change is fine.

commit 69203fba700e ("network: ndisc: remove old addresses and routes
after at least one SLAAC address becomes ready") introduced this
behavior. commit 50550722e3ba fixed it partially, by preventing one
router's RAs from invalidating another router's configuration.

[1] https://www.rfc-editor.org/rfc/rfc4861

Fixes: 69203fba700e ("network: ndisc: remove old addresses and routes after at least one SLAAC address becomes ready")

diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c
index d7333b634eea5f396cce7930e9631c3628daa8fc..bb1eedc6f1d99c91bcdf3473c1bd444bfc4a4e54 100644 (file)
--- a/src/network/networkd-address.c
+++ b/src/network/networkd-address.c
@@ -160,7 +160,7 @@ bool address_is_ready(const Address *a) {
         return true;
 }
 
-void link_mark_addresses(Link *link, NetworkConfigSource source, const struct in6_addr *router) {
+void link_mark_addresses(Link *link, NetworkConfigSource source) {
         Address *a;
 
         assert(link);
@@ -169,10 +169,6 @@ void link_mark_addresses(Link *link, NetworkConfigSource source, const struct in
                 if (a->source != source)
                         continue;
 
-                if (source == NETWORK_CONFIG_SOURCE_NDISC &&
-                    router && !in6_addr_equal(router, &a->provider.in6))
-                        continue;
-
                 address_mark(a);
         }
 }

diff --git a/src/network/networkd-address.h b/src/network/networkd-address.h
index d1eff9090a9a593dd53212d3b0049514b246c582..ef29caf4e78d5152035841d5fc615ce6499c5ada 100644 (file)
--- a/src/network/networkd-address.h
+++ b/src/network/networkd-address.h
@@ -120,7 +120,7 @@ int address_compare_func(const Address *a1, const Address *a2);
 
 DEFINE_NETWORK_CONFIG_STATE_FUNCTIONS(Address, address);
 
-void link_mark_addresses(Link *link, NetworkConfigSource source, const struct in6_addr *router);
+void link_mark_addresses(Link *link, NetworkConfigSource source);
 
 CONFIG_PARSER_PROTOTYPE(config_parse_address);
 CONFIG_PARSER_PROTOTYPE(config_parse_broadcast);

diff --git a/src/network/networkd-dhcp-prefix-delegation.c b/src/network/networkd-dhcp-prefix-delegation.c
index a78a7e5789ecd1d465dd849104731bad4c0ac077..66c5e979d9fcd58d1e91153d5bf741d3f7be881a 100644 (file)
--- a/src/network/networkd-dhcp-prefix-delegation.c
+++ b/src/network/networkd-dhcp-prefix-delegation.c
@@ -572,8 +572,8 @@ static int dhcp_pd_prepare(Link *link) {
         if (link_radv_enabled(link) && link->network->dhcp_pd_announce && !link->radv)
                 return 0;
 
-        link_mark_addresses(link, NETWORK_CONFIG_SOURCE_DHCP_PD, NULL);
-        link_mark_routes(link, NETWORK_CONFIG_SOURCE_DHCP_PD, NULL);
+        link_mark_addresses(link, NETWORK_CONFIG_SOURCE_DHCP_PD);
+        link_mark_routes(link, NETWORK_CONFIG_SOURCE_DHCP_PD);
 
         return 1;
 }

diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c
index afe80460a0f048038812f39afb2876fd2f8ef755..c5bddb60fd9abb88bd99e805c84c76985a0791f8 100644 (file)
--- a/src/network/networkd-dhcp4.c
+++ b/src/network/networkd-dhcp4.c
@@ -899,8 +899,8 @@ static int dhcp4_request_address_and_routes(Link *link, bool announce) {
 
         assert(link);
 
-        link_mark_addresses(link, NETWORK_CONFIG_SOURCE_DHCP4, NULL);
-        link_mark_routes(link, NETWORK_CONFIG_SOURCE_DHCP4, NULL);
+        link_mark_addresses(link, NETWORK_CONFIG_SOURCE_DHCP4);
+        link_mark_routes(link, NETWORK_CONFIG_SOURCE_DHCP4);
 
         r = dhcp4_request_address(link, announce);
         if (r < 0)

diff --git a/src/network/networkd-dhcp6.c b/src/network/networkd-dhcp6.c
index 086a6f4b6160b28be1d110e900b73c31f2d7ed1b..789868c26aaabbdee3a349386f219d0d3f42da45 100644 (file)
--- a/src/network/networkd-dhcp6.c
+++ b/src/network/networkd-dhcp6.c
@@ -304,8 +304,8 @@ static int dhcp6_lease_ip_acquired(sd_dhcp6_client *client, Link *link) {
         sd_dhcp6_lease *lease;
         int r;
 
-        link_mark_addresses(link, NETWORK_CONFIG_SOURCE_DHCP6, NULL);
-        link_mark_routes(link, NETWORK_CONFIG_SOURCE_DHCP6, NULL);
+        link_mark_addresses(link, NETWORK_CONFIG_SOURCE_DHCP6);
+        link_mark_routes(link, NETWORK_CONFIG_SOURCE_DHCP6);
 
         r = sd_dhcp6_client_get_lease(client, &lease);
         if (r < 0)

diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c
index d0508f93c048c7182e69b897d5b6e8c13aa9ee40..2250235d4eb17932f475ee59fe5b3e58ef798426 100644 (file)
--- a/src/network/networkd-ndisc.c
+++ b/src/network/networkd-ndisc.c
@@ -72,72 +72,6 @@ void network_adjust_ipv6_accept_ra(Network *network) {
                 network->ndisc_deny_listed_route_prefix = set_free_free(network->ndisc_deny_listed_route_prefix);
 }
 
-static int ndisc_remove(Link *link, struct in6_addr *router) {
-        bool updated = false;
-        NDiscDNSSL *dnssl;
-        NDiscRDNSS *rdnss;
-        Address *address;
-        Route *route;
-        int k, r = 0;
-
-        assert(link);
-
-        SET_FOREACH(route, link->routes) {
-                if (route->source != NETWORK_CONFIG_SOURCE_NDISC)
-                        continue;
-                if (!route_is_marked(route))
-                        continue;
-                if (router && !in6_addr_equal(router, &route->provider.in6))
-                        continue;
-
-                k = route_remove(route);
-                if (k < 0)
-                        r = k;
-
-                route_cancel_request(route, link);
-        }
-
-        SET_FOREACH(address, link->addresses) {
-                if (address->source != NETWORK_CONFIG_SOURCE_NDISC)
-                        continue;
-                if (!address_is_marked(address))
-                        continue;
-                if (router && !in6_addr_equal(router, &address->provider.in6))
-                        continue;
-
-                k = address_remove(address);
-                if (k < 0)
-                        r = k;
-
-                address_cancel_request(address);
-        }
-
-        SET_FOREACH(rdnss, link->ndisc_rdnss) {
-                if (!rdnss->marked)
-                        continue;
-                if (router && !in6_addr_equal(router, &rdnss->router))
-                        continue;
-
-                free(set_remove(link->ndisc_rdnss, rdnss));
-                updated = true;
-        }
-
-        SET_FOREACH(dnssl, link->ndisc_dnssl) {
-                if (!dnssl->marked)
-                        continue;
-                if (router && !in6_addr_equal(router, &dnssl->router))
-                        continue;
-
-                free(set_remove(link->ndisc_dnssl, dnssl));
-                updated = true;
-        }
-
-        if (updated)
-                link_dirty(link);
-
-        return r;
-}
-
 static int ndisc_check_ready(Link *link);
 
 static int ndisc_address_ready_callback(Address *address) {
@@ -156,7 +90,6 @@ static int ndisc_address_ready_callback(Address *address) {
 static int ndisc_check_ready(Link *link) {
         bool found = false, ready = false;
         Address *address;
-        int r;
 
         assert(link);
 
@@ -189,10 +122,6 @@ static int ndisc_check_ready(Link *link) {
         link->ndisc_configured = true;
         log_link_debug(link, "SLAAC addresses and routes set.");
 
-        r = ndisc_remove(link, NULL);
-        if (r < 0)
-                return r;
-
         link_check_ready(link);
         return 0;
 }
@@ -216,7 +145,6 @@ static int ndisc_route_handler(sd_netlink *rtnl, sd_netlink_message *m, Request
 static int ndisc_request_route(Route *in, Link *link, sd_ndisc_router *rt) {
         _cleanup_(route_freep) Route *route = in;
         struct in6_addr router;
-        Route *existing;
         int r;
 
         assert(route);
@@ -236,10 +164,8 @@ static int ndisc_request_route(Route *in, Link *link, sd_ndisc_router *rt) {
         if (!route->protocol_set)
                 route->protocol = RTPROT_RA;
 
-        if (route_get(NULL, link, route, &existing) < 0)
+        if (route_get(NULL, link, route, NULL) < 0)
                 link->ndisc_configured = false;
-        else
-                route_unmark(existing);
 
         return link_request_route(link, TAKE_PTR(route), true, &link->ndisc_messages,
                                   ndisc_route_handler, NULL);
@@ -264,7 +190,6 @@ static int ndisc_address_handler(sd_netlink *rtnl, sd_netlink_message *m, Reques
 static int ndisc_request_address(Address *in, Link *link, sd_ndisc_router *rt) {
         _cleanup_(address_freep) Address *address = in;
         struct in6_addr router;
-        Address *existing;
         int r;
 
         assert(address);
@@ -282,10 +207,8 @@ static int ndisc_request_address(Address *in, Link *link, sd_ndisc_router *rt) {
         if (r < 0)
                 return r;
 
-        if (address_get(link, address, &existing) < 0)
+        if (address_get(link, address, NULL) < 0)
                 link->ndisc_configured = false;
-        else
-                address_unmark(existing);
 
         return link_request_address(link, TAKE_PTR(address), true, &link->ndisc_messages,
                                  ndisc_address_handler, NULL);
@@ -723,7 +646,6 @@ static int ndisc_router_process_rdnss(Link *link, sd_ndisc_router *rt) {
 
                 rdnss = set_get(link->ndisc_rdnss, &d);
                 if (rdnss) {
-                        rdnss->marked = false;
                         rdnss->router = router;
                         rdnss->lifetime_usec = lifetime_usec;
                         continue;
@@ -822,7 +744,6 @@ static int ndisc_router_process_dnssl(Link *link, sd_ndisc_router *rt) {
 
                 dnssl = set_get(link->ndisc_dnssl, s);
                 if (dnssl) {
-                        dnssl->marked = false;
                         dnssl->router = router;
                         dnssl->lifetime_usec = lifetime_usec;
                         continue;
@@ -893,25 +814,6 @@ static int ndisc_router_process_options(Link *link, sd_ndisc_router *rt) {
         }
 }
 
-static void ndisc_mark(Link *link, const struct in6_addr *router) {
-        NDiscRDNSS *rdnss;
-        NDiscDNSSL *dnssl;
-
-        assert(link);
-        assert(router);
-
-        link_mark_addresses(link, NETWORK_CONFIG_SOURCE_NDISC, router);
-        link_mark_routes(link, NETWORK_CONFIG_SOURCE_NDISC, router);
-
-        SET_FOREACH(rdnss, link->ndisc_rdnss)
-                if (in6_addr_equal(&rdnss->router, router))
-                        rdnss->marked = true;
-
-        SET_FOREACH(dnssl, link->ndisc_dnssl)
-                if (in6_addr_equal(&dnssl->router, router))
-                        dnssl->marked = true;
-}
-
 static int ndisc_start_dhcp6_client(Link *link, sd_ndisc_router *rt) {
         int r;
 
@@ -978,8 +880,6 @@ static int ndisc_router_handler(Link *link, sd_ndisc_router *rt) {
                 return 0;
         }
 
-        ndisc_mark(link, &router);
-
         r = ndisc_start_dhcp6_client(link, rt);
         if (r < 0)
                 return r;
@@ -992,13 +892,9 @@ static int ndisc_router_handler(Link *link, sd_ndisc_router *rt) {
         if (r < 0)
                 return r;
 
-        if (link->ndisc_messages == 0) {
+        if (link->ndisc_messages == 0)
                 link->ndisc_configured = true;
-
-                r = ndisc_remove(link, &router);
-                if (r < 0)
-                        return r;
-        } else
+        else
                 log_link_debug(link, "Setting SLAAC addresses and router.");
 
         if (!link->ndisc_configured)

diff --git a/src/network/networkd-ndisc.h b/src/network/networkd-ndisc.h
index f88a002c73e333bb48a95aa8be7498de81205a77..bdbe2f41f1d7de8c112fa3ec40234de9bc994c20 100644 (file)
--- a/src/network/networkd-ndisc.h
+++ b/src/network/networkd-ndisc.h
@@ -16,8 +16,6 @@ typedef enum IPv6AcceptRAStartDHCP6Client {
 } IPv6AcceptRAStartDHCP6Client;
 
 typedef struct NDiscRDNSS {
-        /* Used when GC'ing old DNS servers when configuration changes. */
-        bool marked;
         struct in6_addr router;
         /* This is an absolute point in time, and NOT a timespan/duration.
          * Must be specified with clock_boottime_or_monotonic(). */
@@ -26,8 +24,6 @@ typedef struct NDiscRDNSS {
 } NDiscRDNSS;
 
 typedef struct NDiscDNSSL {
-        /* Used when GC'ing old domains when configuration changes. */
-        bool marked;
         struct in6_addr router;
         /* This is an absolute point in time, and NOT a timespan/duration.
          * Must be specified with clock_boottime_or_monotonic(). */

diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c
index 6b3708573c170f4ce940b7a05ea3f480dc66a223..3f460e0ba4b2f28830b47baf98269630c17b536f 100644 (file)
--- a/src/network/networkd-route.c
+++ b/src/network/networkd-route.c
@@ -534,7 +534,7 @@ static int route_convert(Manager *manager, const Route *route, ConvertedRoutes *
         return 1;
 }
 
-void link_mark_routes(Link *link, NetworkConfigSource source, const struct in6_addr *router) {
+void link_mark_routes(Link *link, NetworkConfigSource source) {
         Route *route;
 
         assert(link);
@@ -543,10 +543,6 @@ void link_mark_routes(Link *link, NetworkConfigSource source, const struct in6_a
                 if (route->source != source)
                         continue;
 
-                if (source == NETWORK_CONFIG_SOURCE_NDISC &&
-                    router && !in6_addr_equal(router, &route->provider.in6))
-                        continue;
-
                 route_mark(route);
         }
 }

diff --git a/src/network/networkd-route.h b/src/network/networkd-route.h
index fd4433eae4e54460a456dcced54340dc2517dcfd..071803cefbd8b05475f81e548577bb45b170ac62 100644 (file)
--- a/src/network/networkd-route.h
+++ b/src/network/networkd-route.h
@@ -109,7 +109,7 @@ int network_add_default_route_on_device(Network *network);
 void network_drop_invalid_routes(Network *network);
 
 DEFINE_NETWORK_CONFIG_STATE_FUNCTIONS(Route, route);
-void link_mark_routes(Link *link, NetworkConfigSource source, const struct in6_addr *router);
+void link_mark_routes(Link *link, NetworkConfigSource source);
 
 CONFIG_PARSER_PROTOTYPE(config_parse_gateway);
 CONFIG_PARSER_PROTOTYPE(config_parse_preferred_src);