From a021ee273afeaa5e9324270185bbfe33c64a92cf Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 20 Dec 2022 16:11:16 +0100 Subject: [PATCH 1/1] Revert "policy: track the autoconnect retries in devices for multi-connect" With multi-connect enabled, this can cause infinite retries to autoconnect, see [1]. That has bad consequences for example in initrd, where nm-wait-online-initrd.service would wait up to one hour before failing and blocking boot. This reverts commit 1656d82045343b5af5b86cf129d4f2b12540b277. [1] https://bugzilla.redhat.com/show_bug.cgi?id=2039734#c5 Fixes: 1656d8204534 ('policy: track the autoconnect retries in devices for multi-connect') (cherry picked from commit aec7ae8279cc9b3de7e75140b75fe997f43700fc) (cherry picked from commit ba663123669ae1a5670e4a85ce2cf520fea875f6) --- src/core/devices/nm-device.c | 50 ---------------- src/core/devices/nm-device.h | 5 -- src/core/nm-policy.c | 112 ++++------------------------------- 3 files changed, 10 insertions(+), 157 deletions(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 059e31f298a5..056ca34c1c4d 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -96,8 +96,6 @@ #define NM_DEVICE_AUTH_RETRIES_INFINITY -2 #define NM_DEVICE_AUTH_RETRIES_DEFAULT 3 -#define AUTOCONNECT_RESET_RETRIES_TIMER 300 - /*****************************************************************************/ typedef void (*ActivationHandleFunc)(NMDevice *self); @@ -763,9 +761,6 @@ typedef struct _NMDevicePrivate { GVariant *ports_variant; /* Array of port devices D-Bus path */ char *prop_ip_iface; /* IP interface D-Bus property */ - - int autoconnect_retries; - gint32 autoconnect_retries_blocked_until; } NMDevicePrivate; G_DEFINE_ABSTRACT_TYPE(NMDevice, nm_device, NM_TYPE_DBUS_OBJECT) @@ -16881,49 +16876,6 @@ nm_device_get_initial_hw_address(NMDevice *self) return NM_DEVICE_GET_PRIVATE(self)->hw_addr_initial; } -void -nm_device_set_autoconnect_retries(NMDevice *self, int tries) -{ - NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE(self); - - if (priv->autoconnect_retries != tries) { - _LOGT(LOGD_DEVICE, "autoconnect: retries set %d", tries); - priv->autoconnect_retries = tries; - } - - if (tries) - priv->autoconnect_retries_blocked_until = 0; /* we are not blocked anymore */ - else - priv->autoconnect_retries_blocked_until = - nm_utils_get_monotonic_timestamp_sec() + AUTOCONNECT_RESET_RETRIES_TIMER; -} - -int -nm_device_get_autoconnect_retries(NMDevice *self) -{ - NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE(self); - - return priv->autoconnect_retries; -} - -gint32 -nm_device_autoconnect_retries_blocked_until(NMDevice *self) -{ - NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE(self); - - return priv->autoconnect_retries_blocked_until; -} - -void -nm_device_autoconnect_retries_reset(NMDevice *self) -{ - NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE(self); - - /* default value, we will sync. with connection value when needed */ - priv->autoconnect_retries = -2; - priv->autoconnect_retries_blocked_until = 0; -} - /** * nm_device_spec_match_list: * @self: an #NMDevice @@ -17675,8 +17627,6 @@ nm_device_init(NMDevice *self) priv->sys_iface_state_ = NM_DEVICE_SYS_IFACE_STATE_EXTERNAL; priv->promisc_reset = NM_OPTION_BOOL_DEFAULT; - - priv->autoconnect_retries = -2; } static GObject * diff --git a/src/core/devices/nm-device.h b/src/core/devices/nm-device.h index fea46bb78cae..de850e68f2a1 100644 --- a/src/core/devices/nm-device.h +++ b/src/core/devices/nm-device.h @@ -464,11 +464,6 @@ const char *nm_device_get_permanent_hw_address_full(NMDevice *self, gboolean *out_is_fake); const char *nm_device_get_initial_hw_address(NMDevice *dev); -void nm_device_set_autoconnect_retries(NMDevice *self, int tries); -int nm_device_get_autoconnect_retries(NMDevice *self); -gint32 nm_device_autoconnect_retries_blocked_until(NMDevice *self); -void nm_device_autoconnect_retries_reset(NMDevice *self); - NMDhcpConfig *nm_device_get_dhcp_config(NMDevice *dev, int addr_family); NML3Cfg *nm_device_get_l3cfg(NMDevice *self); diff --git a/src/core/nm-policy.c b/src/core/nm-policy.c index 518fc596dc88..0c07c29e98eb 100644 --- a/src/core/nm-policy.c +++ b/src/core/nm-policy.c @@ -1579,7 +1579,6 @@ nm_policy_unblock_failed_ovs_interfaces(NMPolicy *self) NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE(self); NMSettingsConnection *const *connections = NULL; guint i; - gboolean multiconnect = FALSE; _LOGT(LOGD_DEVICE, "unblocking failed OVS interfaces"); @@ -1587,13 +1586,6 @@ nm_policy_unblock_failed_ovs_interfaces(NMPolicy *self) for (i = 0; connections[i]; i++) { NMSettingsConnection *sett_conn = connections[i]; NMConnection *connection = nm_settings_connection_get_connection(sett_conn); - NMSettingConnection *s_con = nm_connection_get_setting_connection(connection); - - /* Check if any connection is connect multiple */ - if (s_con - && nm_setting_connection_get_multi_connect(s_con) - == NM_CONNECTION_MULTI_CONNECT_MULTIPLE) - multiconnect = TRUE; if (nm_connection_get_setting_ovs_interface(connection)) { nm_settings_connection_autoconnect_retries_reset(sett_conn); @@ -1603,18 +1595,6 @@ nm_policy_unblock_failed_ovs_interfaces(NMPolicy *self) FALSE); } } - - /* There is, at least, one multiconnect connection with an OVS setting. - * Let's check the OVS devices retries */ - if (multiconnect) { - NMDevice *device; - GHashTableIter h_iter; - g_hash_table_iter_init(&h_iter, priv->devices); - while (g_hash_table_iter_next(&h_iter, (gpointer *) &device, NULL)) { - if (nm_device_get_device_type(device) == NM_DEVICE_TYPE_OVS_INTERFACE) - nm_device_autoconnect_retries_reset(device); - } - } } static gboolean @@ -1626,8 +1606,7 @@ reset_autoconnect_all( NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE(self); NMSettingsConnection *const *connections = NULL; guint i; - gboolean changed = FALSE; - gboolean multiconnect = FALSE; + gboolean changed = FALSE; _LOGD(LOGD_DEVICE, "re-enabling autoconnect for all connections%s%s%s", @@ -1638,8 +1617,6 @@ reset_autoconnect_all( connections = nm_settings_get_connections(priv->settings, NULL); for (i = 0; connections[i]; i++) { NMSettingsConnection *sett_conn = connections[i]; - NMSettingConnection *s_con = - nm_connection_get_setting_connection(nm_settings_connection_get_connection(sett_conn)); if (device && !nm_device_check_connection_compatible( @@ -1648,11 +1625,6 @@ reset_autoconnect_all( NULL)) continue; - if (s_con - && nm_setting_connection_get_multi_connect(s_con) - == NM_CONNECTION_MULTI_CONNECT_MULTIPLE) - multiconnect = TRUE; - if (only_no_secrets) { /* we only reset the no-secrets blocked flag. */ if (nm_settings_connection_autoconnect_blocked_reason_set( @@ -1681,20 +1653,6 @@ reset_autoconnect_all( } } } - - /* There is, at least, one multiconnect connection. Let's check the devices retries */ - if (multiconnect) { - NMDevice *device_iter; - GHashTableIter h_iter; - g_hash_table_iter_init(&h_iter, priv->devices); - while (g_hash_table_iter_next(&h_iter, (gpointer *) &device_iter, NULL)) { - if (nm_device_get_autoconnect_retries(device_iter) != -2) { - nm_device_autoconnect_retries_reset(device_iter); - changed = TRUE; - } - } - } - return changed; } @@ -1752,8 +1710,7 @@ reset_connections_retries(gpointer user_data) NMSettingsConnection *const *connections = NULL; guint i; gint32 con_stamp, min_stamp, now; - gboolean changed = FALSE; - gboolean multiconnect = FALSE; + gboolean changed = FALSE; priv->reset_retries_id = 0; @@ -1762,14 +1719,6 @@ reset_connections_retries(gpointer user_data) connections = nm_settings_get_connections(priv->settings, NULL); for (i = 0; connections[i]; i++) { NMSettingsConnection *connection = connections[i]; - NMSettingConnection *s_con = - nm_connection_get_setting_connection(nm_settings_connection_get_connection(connection)); - - /* Check if any connection is connect multiple */ - if (s_con - && nm_setting_connection_get_multi_connect(s_con) - == NM_CONNECTION_MULTI_CONNECT_MULTIPLE) - multiconnect = TRUE; con_stamp = nm_settings_connection_autoconnect_retries_blocked_until(connection); if (con_stamp == 0) @@ -1782,25 +1731,6 @@ reset_connections_retries(gpointer user_data) min_stamp = con_stamp; } - /* There is, at least, one multiconnect connection. Let's check the devices retries */ - if (multiconnect) { - NMDevice *device; - GHashTableIter h_iter; - g_hash_table_iter_init(&h_iter, priv->devices); - while (g_hash_table_iter_next(&h_iter, (gpointer *) &device, NULL)) { - con_stamp = nm_device_autoconnect_retries_blocked_until(device); - - /* default value in device is -2, which means, we do not care */ - if (con_stamp <= 0) - continue; - if (con_stamp <= now) { - nm_device_autoconnect_retries_reset(device); - changed = TRUE; - } else if (min_stamp == 0 || min_stamp > con_stamp) - min_stamp = con_stamp; - } - } - /* Schedule the handler again if there are some stamps left */ if (min_stamp != 0) priv->reset_retries_id = @@ -1814,31 +1744,21 @@ reset_connections_retries(gpointer user_data) } static void -_connection_autoconnect_retries_set(NMPolicy *self, - NMSettingsConnection *connection, - NMDevice *device, - int tries) +_connection_autoconnect_retries_set(NMPolicy *self, NMSettingsConnection *connection, int tries) { - NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE(self); - NMSettingConnection *s_con; - gint32 retry_time; + NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE(self); nm_assert(NM_IS_SETTINGS_CONNECTION(connection)); nm_assert(tries >= 0); - s_con = nm_connection_get_setting_connection(nm_settings_connection_get_connection(connection)); - if (s_con - && nm_setting_connection_get_multi_connect(s_con) == NM_CONNECTION_MULTI_CONNECT_MULTIPLE) { - nm_device_set_autoconnect_retries(device, tries); - retry_time = nm_device_autoconnect_retries_blocked_until(device); - } else { - nm_settings_connection_autoconnect_retries_set(connection, tries); - retry_time = nm_settings_connection_autoconnect_retries_blocked_until(connection); - } + nm_settings_connection_autoconnect_retries_set(connection, tries); if (tries == 0) { /* Schedule a handler to reset retries count */ if (!priv->reset_retries_id) { + gint32 retry_time = + nm_settings_connection_autoconnect_retries_blocked_until(connection); + g_warn_if_fail(retry_time != 0); priv->reset_retries_id = g_timeout_add_seconds(MAX(0, retry_time - nm_utils_get_monotonic_timestamp_sec()), @@ -2100,25 +2020,13 @@ device_state_changed(NMDevice *device, } if (!blocked) { - s_con = nm_connection_get_setting_connection( - nm_settings_connection_get_connection(sett_conn)); - if (s_con - && nm_setting_connection_get_multi_connect(s_con) - == NM_CONNECTION_MULTI_CONNECT_MULTIPLE) { - if (nm_device_get_autoconnect_retries(device) == -2) - nm_device_set_autoconnect_retries( - device, - nm_settings_connection_autoconnect_retries_get(sett_conn)); - - tries = nm_device_get_autoconnect_retries(device); - } else - tries = nm_settings_connection_autoconnect_retries_get(sett_conn); + tries = nm_settings_connection_autoconnect_retries_get(sett_conn); if (tries > 0) { _LOGD(LOGD_DEVICE, "connection '%s' failed to autoconnect; %d tries left", nm_settings_connection_get_id(sett_conn), tries - 1); - _connection_autoconnect_retries_set(self, sett_conn, device, tries - 1); + _connection_autoconnect_retries_set(self, sett_conn, tries - 1); } else if (tries != 0) { _LOGD(LOGD_DEVICE, "connection '%s' failed to autoconnect; infinite tries left", -- 2.38.1