From b732606950f8726c0280080c7d055a714c2888f5 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 30 Mar 2022 00:52:09 +0900 Subject: [PATCH 1/2] network: automatically determine timeout of waiting for carrier regain The commit 6706ce2fd2a13df0ae5e469b72d688eaf643dac4 made IgnoreCarrierLoss= setting also take timespan, to make users handle issues like #18738 or #20887. But still users needed to explicitly set a timespan. This makes networkd automatically determine the timeout when the situations #18738 or #19832 is detected. Unfortunately, still users have issue #20887 need to specify a value. Closes #19832. --- man/systemd.network.xml | 37 ++++++++++++++++++++++++++++--------- src/network/networkd-link.c | 30 +++++++++++++++++++++++++++--- 2 files changed, 55 insertions(+), 12 deletions(-) diff --git a/man/systemd.network.xml b/man/systemd.network.xml index c849cfc4fe..8c984c5b22 100644 --- a/man/systemd.network.xml +++ b/man/systemd.network.xml @@ -917,17 +917,36 @@ Table=1234 IgnoreCarrierLoss= - Takes a boolean or a timespan. When true, networkd retains both the static and dynamic - configuration of the interface even if its carrier is lost. When a timespan is specified, - networkd waits for the specified timespan, and ignores the carrier loss if the link regain - its carrier within the timespan. Setting a finite timespan may be useful for a wireless - interface connecting to a network which has multiple access points with the same SSID, or an - interface which is reset on changing MTU. When unset, the value specified with - ConfigureWithoutCarrier= is used. + Takes a boolean or a timespan. When true, systemd-networkd retains + both the static and dynamic configuration of the interface even if its carrier is lost. When + false, systemd-networkd drops both the static and dynamic configuration of + the interface. When a timespan is specified, systemd-networkd waits for + the specified timespan, and ignores the carrier loss if the link regain its carrier within + the timespan. Setting 0 seconds is equivalent to no, and + infinite is equivalent to yes. - When ActivationPolicy= is set to always-up, this - is forced to true. + Setting a finite timespan may be useful when e.g. in the following cases: + + + A wireless interface connecting to a network which has multiple access points with + the same SSID. + + + Enslaving a wireless interface to a bond interface, which may disconnect from the + connected access point and causes its carrier to be lost. + + + The driver of the interface resets when the MTU is changed. + + + + When Bond= is specified to a wireless interface, defaults to 3 + seconds. When the DHCPv4 client is enabled and UseMTU= in the [DHCPv4] + sectionis enabled, defaults to 5 seconds. Otherwise, defaults to the value specified with + ConfigureWithoutCarrier=. When ActivationPolicy= is set + to always-up, this is forced to yes, and ignored any + user specified values. diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index d509855f48..73850cdea5 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -1636,6 +1636,8 @@ static int link_carrier_lost_handler(sd_event_source *s, uint64_t usec, void *us } static int link_carrier_lost(Link *link) { + uint16_t dhcp_mtu; + usec_t usec; int r; assert(link); @@ -1651,16 +1653,38 @@ static int link_carrier_lost(Link *link) { if (!link->network) return 0; - if (link->network->ignore_carrier_loss_usec == USEC_INFINITY) + if (link->network->ignore_carrier_loss_set) + /* If IgnoreCarrierLoss= is explicitly specified, then use the specified value. */ + usec = link->network->ignore_carrier_loss_usec; + + else if (link->network->bond && link->wlan_iftype > 0) + /* Enslaving wlan interface to a bond disconnects from the connected AP, and causes its + * carrier to be lost. See #19832. */ + usec = 3 * USEC_PER_SEC; + + else if (link->network->dhcp_use_mtu && + link->dhcp_lease && + sd_dhcp_lease_get_mtu(link->dhcp_lease, &dhcp_mtu) >= 0 && + dhcp_mtu != link->original_mtu) + /* Some drivers reset interfaces when changing MTU. Resetting interfaces by the static + * MTU should not cause any issues, as MTU is changed only once. However, setting MTU + * through DHCP lease causes an infinite loop of resetting the interface. See #18738. */ + usec = 5 * USEC_PER_SEC; + + else + /* Otherwise, use the currently set value. */ + usec = link->network->ignore_carrier_loss_usec; + + if (usec == USEC_INFINITY) return 0; - if (link->network->ignore_carrier_loss_usec == 0) + if (usec == 0) return link_carrier_lost_impl(link); return event_reset_time_relative(link->manager->event, &link->carrier_lost_timer, CLOCK_BOOTTIME, - link->network->ignore_carrier_loss_usec, + usec, 0, link_carrier_lost_handler, link, From da94a69b8d1044c127271d13ac0087ce388749dc Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 30 Mar 2022 01:04:26 +0900 Subject: [PATCH 2/2] network: shorten code a bit Currently, there exist only two MTU sources, static and DHCPv4, and they are exclusive. Hence, it is not necessary to check the existence of the MTU option in the acquired DHCP lease. Let's unconditionally reset the MTU. Note that, if the current and original MTU are equivalent, then link_request_to_set_mtu() handles that gracefully. --- src/network/networkd-dhcp4.c | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index 6bfcf67dd9..d3fd2571e0 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -735,7 +735,6 @@ static int dhcp4_request_routes(Link *link) { } static int dhcp_reset_mtu(Link *link) { - uint16_t mtu; int r; assert(link); @@ -743,18 +742,9 @@ static int dhcp_reset_mtu(Link *link) { if (!link->network->dhcp_use_mtu) return 0; - r = sd_dhcp_lease_get_mtu(link->dhcp_lease, &mtu); - if (r == -ENODATA) - return 0; - if (r < 0) - return log_link_error_errno(link, r, "DHCP error: failed to get MTU from lease: %m"); - - if (link->original_mtu == mtu) - return 0; - r = link_request_to_set_mtu(link, link->original_mtu); if (r < 0) - return log_link_error_errno(link, r, "DHCP error: could not reset MTU: %m"); + return log_link_error_errno(link, r, "DHCP error: Could not queue request to reset MTU: %m"); return 0; }