diff options
author | Thomas Haller <thaller@redhat.com> | 2023-05-04 10:34:41 +0200 |
---|---|---|
committer | Thomas Haller <thaller@redhat.com> | 2023-05-04 10:34:41 +0200 |
commit | a7409312045b48e1cce253e13636d234d3ff6cd9 (patch) | |
tree | 56d8b72c7cbe0cd528198f3571681f8c951961e0 | |
parent | 2e3fabae50be1429e451c85f32ef9b7011b8feae (diff) | |
parent | 6e229a852fd93fc0dd5d52fcc0884d462a243249 (diff) | |
download | NetworkManager-a7409312045b48e1cce253e13636d234d3ff6cd9.tar.gz |
core: merge branch 'th/autoconnect-cleanups'
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1615
-rw-r--r-- | src/core/nm-manager.c | 188 | ||||
-rw-r--r-- | src/core/nm-manager.h | 6 | ||||
-rw-r--r-- | src/core/nm-policy.c | 35 | ||||
-rw-r--r-- | src/core/settings/nm-settings-connection.c | 28 |
4 files changed, 163 insertions, 94 deletions
diff --git a/src/core/nm-manager.c b/src/core/nm-manager.c index 02dabaf6fe..c6cf2cb943 100644 --- a/src/core/nm-manager.c +++ b/src/core/nm-manager.c @@ -89,6 +89,16 @@ typedef struct { } DevConData; +#define DEV_CON_DATA_LOG_FMT \ + "device[" NM_HASH_OBFUSCATE_PTR_FMT ",%s]-profile[" NM_HASH_OBFUSCATE_PTR_FMT ",%s]" + +/* This is an unsafe macro (it evaluates the macro arguments multiple times and is non-function-like). */ +#define DEV_CON_DATA_LOG_ARGS(device, sett_conn) \ + NM_HASH_OBFUSCATE_PTR(device), nm_device_get_iface(device), NM_HASH_OBFUSCATE_PTR(sett_conn), \ + nm_settings_connection_get_id(sett_conn) + +#define DEV_CON_DATA_LOG_ARGS_DATA(data) DEV_CON_DATA_LOG_ARGS((data)->device, (data)->sett_conn) + typedef enum { ASYNC_OP_TYPE_AC_AUTH_ACTIVATE_INTERNAL, ASYNC_OP_TYPE_AC_AUTH_ACTIVATE_USER, @@ -440,7 +450,8 @@ static void _rfkill_update(NMManager *self, NMRfkillType rtype); static DevConData *_devcon_lookup_data(NMManager *self, NMDevice *device, NMSettingsConnection *sett_conn, - gboolean create); + gboolean create, + gboolean log_creation); /*****************************************************************************/ @@ -1252,49 +1263,58 @@ _autoconnect_retries_initial(NMSettingsConnection *sett_conn) if (retries == -1) retries = nm_config_data_get_autoconnect_retries_default(NM_CONFIG_GET_DATA); - nm_assert(retries >= 0 && ((guint) retries) <= ((guint) G_MAXINT32)); + nm_assert(retries >= 0 && retries <= G_MAXINT32); if (retries == 0) return NM_AUTOCONNECT_RETRIES_FOREVER; return (guint32) retries; } -static void +static gboolean _autoconnect_retries_set(NMManager *self, DevConData *data, guint32 retries, gboolean is_reset) { + gboolean changed = FALSE; + gint32 blocked_until_sec; + nm_assert(data); - if (data->autoconnect.retries != retries || !data->autoconnect.initialized) { - _LOGT(LOGD_SETTINGS, - "autoconnect: device[%p] (%s): retries set %d%s", - data->device, - nm_device_get_ip_iface(data->device), - retries, - is_reset ? " (reset)" : ""); + if (!data->autoconnect.initialized || data->autoconnect.retries != retries) { data->autoconnect.initialized = TRUE; data->autoconnect.retries = retries; + changed = TRUE; } if (retries != 0) { - _LOGT(LOGD_SETTINGS, - "autoconnect: device[%p] (%s): no longer block autoconnect (due to retry count) %s", - data->device, - nm_device_get_ip_iface(data->device), - is_reset ? " (reset)" : ""); - data->autoconnect.blocked_until_sec = 0; + blocked_until_sec = 0; } else { /* NOTE: the blocked time must be identical for all connections, otherwise * the tracking of resetting the retry count in NMPolicy needs adjustment * in _connection_autoconnect_retries_set() (as it would need to re-evaluate * the next-timeout every time a connection gets blocked). */ - data->autoconnect.blocked_until_sec = + blocked_until_sec = nm_utils_get_monotonic_timestamp_sec() + AUTOCONNECT_RESET_RETRIES_TIMER_SEC; + } + + if (data->autoconnect.blocked_until_sec != blocked_until_sec) { + data->autoconnect.blocked_until_sec = blocked_until_sec; + changed = TRUE; + } + + if (changed) { + char sbuf[200]; + _LOGT(LOGD_SETTINGS, - "autoconnect: device[%p] (%s): block autoconnect due to retry count for %d seconds", - data->device, - nm_device_get_ip_iface(data->device), - AUTOCONNECT_RESET_RETRIES_TIMER_SEC); + "block-autoconnect: " DEV_CON_DATA_LOG_FMT ": retries set %u%s%s", + DEV_CON_DATA_LOG_ARGS_DATA(data), + retries, + is_reset ? " (is-reset)" : "", + blocked_until_sec == 0 ? "" + : nm_sprintf_buf(sbuf, + " (blocked for %d sec)", + AUTOCONNECT_RESET_RETRIES_TIMER_SEC)); } + + return changed; } /** @@ -1320,11 +1340,10 @@ nm_manager_devcon_autoconnect_retries_get(NMManager *self, nm_assert(self == nm_device_get_manager(device)); nm_assert(self == nm_settings_connection_get_manager(sett_conn)); - data = _devcon_lookup_data(self, device, sett_conn, TRUE); + data = _devcon_lookup_data(self, device, sett_conn, TRUE, FALSE); - if (G_UNLIKELY(!data->autoconnect.initialized)) { + if (G_UNLIKELY(!data->autoconnect.initialized)) _autoconnect_retries_set(self, data, _autoconnect_retries_initial(sett_conn), FALSE); - } return data->autoconnect.retries; } @@ -1336,29 +1355,37 @@ nm_manager_devcon_autoconnect_retries_set(NMManager *self, guint32 retries) { _autoconnect_retries_set(self, - _devcon_lookup_data(self, device, sett_conn, TRUE), + _devcon_lookup_data(self, device, sett_conn, TRUE, FALSE), retries, FALSE); } -void +gboolean nm_manager_devcon_autoconnect_retries_reset(NMManager *self, NMDevice *device, NMSettingsConnection *sett_conn) { DevConData *data; + guint32 retries_initial; + gboolean changed = FALSE; nm_assert(NM_IS_SETTINGS_CONNECTION(sett_conn)); - if (device) - _autoconnect_retries_set(self, - _devcon_lookup_data(self, device, sett_conn, TRUE), - _autoconnect_retries_initial(sett_conn), - TRUE); - else { - c_list_for_each_entry (data, &sett_conn->devcon_con_lst_head, con_lst) - _autoconnect_retries_set(self, data, _autoconnect_retries_initial(sett_conn), TRUE); + retries_initial = _autoconnect_retries_initial(sett_conn); + + if (device) { + return _autoconnect_retries_set(self, + _devcon_lookup_data(self, device, sett_conn, TRUE, FALSE), + retries_initial, + TRUE); + } + + c_list_for_each_entry (data, &sett_conn->devcon_con_lst_head, con_lst) { + if (_autoconnect_retries_set(self, data, retries_initial, TRUE)) + changed = TRUE; } + + return changed; } /** @@ -1398,19 +1425,19 @@ nm_manager_devcon_autoconnect_reset_reconnect_all(NMManager *self, return changed; } - /* we reset the tries-count and any blocked-reason */ + /* we reset the tries-count and any blocked-reason... */ + nm_manager_devcon_autoconnect_retries_reset(self, NULL, sett_conn); - /* if there is a device and we changed the state, then something changed. */ - if (device - && nm_manager_devcon_autoconnect_blocked_reason_set( - self, - device, - sett_conn, - NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_FAILED - | NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_USER_REQUEST, - FALSE)) - changed = TRUE; + if (device) { + if (nm_manager_devcon_autoconnect_blocked_reason_set( + self, + device, + sett_conn, + NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_FAILED, + FALSE)) + changed = TRUE; + } /* we remove all the blocked reason from the connection, if something * happened, then it means the status changed */ @@ -1435,18 +1462,23 @@ nm_manager_devcon_autoconnect_retries_blocked_until(NMManager *self, nm_assert(NM_IS_SETTINGS_CONNECTION(sett_conn)); if (device) { - data = _devcon_lookup_data(self, device, sett_conn, TRUE); + data = _devcon_lookup_data(self, device, sett_conn, FALSE, FALSE); + + if (!data) + return 0; + return data->autoconnect.blocked_until_sec; - } else { - min_stamp = 0; - c_list_for_each_entry (data, &sett_conn->devcon_con_lst_head, con_lst) { - gint32 condev_stamp = data->autoconnect.blocked_until_sec; - if (condev_stamp == 0) - continue; + } - if (min_stamp == 0 || min_stamp > condev_stamp) - min_stamp = condev_stamp; - } + min_stamp = 0; + c_list_for_each_entry (data, &sett_conn->devcon_con_lst_head, con_lst) { + gint32 condev_stamp = data->autoconnect.blocked_until_sec; + + if (condev_stamp == 0) + continue; + + if (min_stamp == 0 || min_stamp > condev_stamp) + min_stamp = condev_stamp; } return min_stamp; @@ -1465,11 +1497,15 @@ nm_manager_devcon_autoconnect_is_blocked(NMManager *self, if (nm_settings_connection_autoconnect_is_blocked(sett_conn)) return TRUE; - data = _devcon_lookup_data(self, device, sett_conn, TRUE); + data = _devcon_lookup_data(self, device, sett_conn, FALSE, FALSE); + + if (!data) + return FALSE; + if (data->autoconnect.blocked_reason != NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_NONE) return TRUE; - if (data->autoconnect.retries == 0 && data->autoconnect.initialized) + if (data->autoconnect.initialized && data->autoconnect.retries == 0) return TRUE; return FALSE; @@ -1487,22 +1523,23 @@ nm_manager_devcon_autoconnect_blocked_reason_set(NMManager gboolean changed = FALSE; char buf[100]; - nm_assert(value); - nm_assert(sett_conn); + nm_assert(NM_IS_SETTINGS_CONNECTION(sett_conn)); + nm_assert(value != NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_NONE); + nm_assert(!NM_FLAGS_ANY(value, ~(NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_FAILED))); if (device) { - data = _devcon_lookup_data(self, device, sett_conn, TRUE); + data = _devcon_lookup_data(self, device, sett_conn, TRUE, TRUE); v = data->autoconnect.blocked_reason; v = NM_FLAGS_ASSIGN(v, value, set); if (data->autoconnect.blocked_reason == v) return FALSE; - _LOGT(LOGD_SETTINGS, - "autoconnect: blocked reason: %s for device %s", - nm_settings_autoconnect_blocked_reason_to_string(v, buf, sizeof(buf)), - nm_device_get_ip_iface(device)); data->autoconnect.blocked_reason = v; + _LOGT(LOGD_SETTINGS, + "block-autoconnect: " DEV_CON_DATA_LOG_FMT ": set blocked reason %s", + DEV_CON_DATA_LOG_ARGS_DATA(data), + nm_settings_autoconnect_blocked_reason_to_string(v, buf, sizeof(buf))); return TRUE; } @@ -1512,10 +1549,11 @@ nm_manager_devcon_autoconnect_blocked_reason_set(NMManager if (data->autoconnect.blocked_reason == v) continue; + _LOGT(LOGD_SETTINGS, - "autoconnect: blocked reason: %s for device %s", - nm_settings_autoconnect_blocked_reason_to_string(v, buf, sizeof(buf)), - nm_device_get_ip_iface(data->device)); + "block-autoconnect: " DEV_CON_DATA_LOG_FMT ": set blocked reason %s", + DEV_CON_DATA_LOG_ARGS_DATA(data), + nm_settings_autoconnect_blocked_reason_to_string(v, buf, sizeof(buf))); data->autoconnect.blocked_reason = v; changed = TRUE; } @@ -1554,7 +1592,8 @@ static DevConData * _devcon_lookup_data(NMManager *self, NMDevice *device, NMSettingsConnection *sett_conn, - gboolean create) + gboolean create, + gboolean log_creation) { NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE(self); DevConData *data; @@ -1581,7 +1620,10 @@ _devcon_lookup_data(NMManager *self, .sett_conn = sett_conn, .autoconnect = { - .initialized = FALSE, + .initialized = FALSE, + .retries = 0, + .blocked_until_sec = 0, + .blocked_reason = NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_NONE, }, }; c_list_link_tail(&device->devcon_dev_lst_head, &data->dev_lst); @@ -1589,6 +1631,12 @@ _devcon_lookup_data(NMManager *self, g_hash_table_add(priv->devcon_data_dict, data); + if (log_creation) { + _LOGT(LOGD_SETTINGS, + "block-autoconnect: " DEV_CON_DATA_LOG_FMT ": entry created (not initialized)", + DEV_CON_DATA_LOG_ARGS_DATA(data)); + } + return data; } @@ -1600,7 +1648,7 @@ _devcon_remove_data(NMManager *self, DevConData *data) nm_assert(data); nm_assert(NM_IS_DEVICE(data->device)); nm_assert(NM_IS_SETTINGS_CONNECTION(data->sett_conn)); - nm_assert(data == _devcon_lookup_data(self, data->device, data->sett_conn, FALSE)); + nm_assert(data == _devcon_lookup_data(self, data->device, data->sett_conn, FALSE, FALSE)); c_list_unlink_stale(&data->dev_lst); c_list_unlink_stale(&data->con_lst); diff --git a/src/core/nm-manager.h b/src/core/nm-manager.h index caa83e4546..3028eb7ebe 100644 --- a/src/core/nm-manager.h +++ b/src/core/nm-manager.h @@ -243,9 +243,9 @@ void nm_manager_devcon_autoconnect_retries_set(NMManager *self, NMSettingsConnection *sett_conn, guint32 retries); -void nm_manager_devcon_autoconnect_retries_reset(NMManager *self, - NMDevice *device, - NMSettingsConnection *sett_conn); +gboolean nm_manager_devcon_autoconnect_retries_reset(NMManager *self, + NMDevice *device, + NMSettingsConnection *sett_conn); gboolean nm_manager_devcon_autoconnect_reset_reconnect_all(NMManager *self, NMDevice *device, diff --git a/src/core/nm-policy.c b/src/core/nm-policy.c index 9309026e56..bad8d32f6d 100644 --- a/src/core/nm-policy.c +++ b/src/core/nm-policy.c @@ -1758,11 +1758,13 @@ _connection_autoconnect_retries_set(NMPolicy *self, if (tries == 0) { /* Schedule a handler to reset retries count */ if (!priv->reset_connections_retries_idle_source) { - gint32 retry_time = nm_manager_devcon_autoconnect_retries_blocked_until(priv->manager, - device, - connection); + gint32 retry_time; + + retry_time = nm_manager_devcon_autoconnect_retries_blocked_until(priv->manager, + device, + connection); + nm_assert(retry_time != 0); - g_warn_if_fail(retry_time != 0); priv->reset_connections_retries_idle_source = nm_g_timeout_add_seconds_source( MAX(0, retry_time - nm_utils_get_monotonic_timestamp_sec()), reset_connections_retries, @@ -1825,9 +1827,8 @@ activate_slave_connections(NMPolicy *self, NMDevice *device) continue; if (!internal_activation) { - nm_manager_devcon_autoconnect_retries_reset(priv->manager, NULL, sett_conn); - /* we cannot know if they changed or not, so considering we did a reset, let's consider they changed. */ - changed = TRUE; + if (nm_manager_devcon_autoconnect_retries_reset(priv->manager, NULL, sett_conn)) + changed = TRUE; } /* unblock the devices associated with that connection */ if (nm_manager_devcon_autoconnect_blocked_reason_set( @@ -1981,7 +1982,8 @@ device_state_changed(NMDevice *device, gboolean blocked = FALSE; guint64 con_v; - if (nm_device_state_reason_check(reason) == NM_DEVICE_STATE_REASON_NO_SECRETS) { + switch (nm_device_state_reason_check(reason)) { + case NM_DEVICE_STATE_REASON_NO_SECRETS: /* we want to block the connection from auto-connect if it failed due to no-secrets. * However, if a secret-agent registered, since the connection made the last * secret-request, we do not block it. The new secret-agent might not yet @@ -1998,7 +2000,8 @@ device_state_changed(NMDevice *device, con_v = nm_settings_connection_get_last_secret_agent_version_id(sett_conn); if (con_v == 0 || con_v == nm_agent_manager_get_agent_version_id(priv->agent_mgr)) { _LOGD(LOGD_DEVICE, - "connection '%s' now blocked from autoconnect due to no secrets", + "block-autoconnect: connection '%s' now blocked from autoconnect due to " + "no secrets", nm_settings_connection_get_id(sett_conn)); nm_settings_connection_autoconnect_blocked_reason_set( sett_conn, @@ -2006,8 +2009,8 @@ device_state_changed(NMDevice *device, TRUE); blocked = TRUE; } - } else if (nm_device_state_reason_check(reason) - == NM_DEVICE_STATE_REASON_DEPENDENCY_FAILED) { + break; + case NM_DEVICE_STATE_REASON_DEPENDENCY_FAILED: /* A connection that fails due to dependency-failed is not * able to reconnect until the master connection activates * again; when this happens, the master clears the blocked @@ -2017,7 +2020,8 @@ device_state_changed(NMDevice *device, * dependency-failed. */ _LOGD(LOGD_DEVICE, - "autoconnect: connection[%p] (%s) now blocked from autoconnect due to failed " + "block-autoconnect: connection[%p] (%s) now blocked from autoconnect due to " + "failed " "dependency", sett_conn, nm_settings_connection_get_id(sett_conn)); @@ -2028,6 +2032,9 @@ device_state_changed(NMDevice *device, NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_FAILED, TRUE); blocked = TRUE; + break; + default: + break; } if (!blocked) { @@ -2102,7 +2109,7 @@ device_state_changed(NMDevice *device, } if (blocked_reason != NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_NONE) { _LOGD(LOGD_DEVICE, - "blocking autoconnect of connection '%s': %s", + "block-autoconnect: blocking autoconnect of connection '%s': %s", nm_settings_connection_get_id(sett_conn), NM_UTILS_LOOKUP_STR_A(nm_device_state_reason_to_string, nm_device_state_reason_check(reason))); @@ -2158,7 +2165,7 @@ device_state_changed(NMDevice *device, priv->manager, device, sett_conn, - NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_ALL, + NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_FAILED, FALSE); break; case NM_DEVICE_STATE_SECONDARIES: diff --git a/src/core/settings/nm-settings-connection.c b/src/core/settings/nm-settings-connection.c index 8d9ee08a99..da0f272872 100644 --- a/src/core/settings/nm-settings-connection.c +++ b/src/core/settings/nm-settings-connection.c @@ -2523,10 +2523,13 @@ nm_settings_connection_autoconnect_blocked_reason_set(NMSettingsConnection { NMSettingsAutoconnectBlockedReason v; NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE(self); - char buf1[100]; - char buf2[100]; + char buf1[200]; + char buf2[200]; - nm_assert(reason); + nm_assert(reason != NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_NONE); + nm_assert(!NM_FLAGS_ANY(reason, + ~(NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_USER_REQUEST + | NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_NO_SECRETS))); v = priv->autoconnect_blocked_reason; v = NM_FLAGS_ASSIGN(v, reason, set); @@ -2534,10 +2537,21 @@ nm_settings_connection_autoconnect_blocked_reason_set(NMSettingsConnection if (priv->autoconnect_blocked_reason == v) return FALSE; - _LOGD("autoconnect: %s blocked reason: %s (now %s)", - set ? "set" : "unset", - nm_settings_autoconnect_blocked_reason_to_string(reason, buf1, sizeof(buf1)), - nm_settings_autoconnect_blocked_reason_to_string(v, buf2, sizeof(buf2))); + if (set) { + _LOGT("block-autoconnect: profile: blocked with reason %s (%s %s)", + nm_settings_autoconnect_blocked_reason_to_string(v, buf1, sizeof(buf1)), + "just blocked", + nm_settings_autoconnect_blocked_reason_to_string(reason, buf2, sizeof(buf2))); + } else if (v != NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_NONE) { + _LOGT("block-autoconnect: profile: blocked with reason %s (%s %s)", + nm_settings_autoconnect_blocked_reason_to_string(v, buf1, sizeof(buf1)), + "just unblocked", + nm_settings_autoconnect_blocked_reason_to_string(reason, buf2, sizeof(buf2))); + } else { + _LOGT("block-autoconnect: profile: not blocked (unblocked %s)", + nm_settings_autoconnect_blocked_reason_to_string(reason, buf1, sizeof(buf1))); + } + priv->autoconnect_blocked_reason = v; return TRUE; } |