From a021ee273afeaa5e9324270185bbfe33c64a92cf Mon Sep 17 00:00:00 2001

From: Thomas Haller <thaller@redhat.com>

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