From 72e544c7eca495d0857d0710cc77161cd3b145d0 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Wed, 7 Jun 2017 17:34:47 +0200
Subject: [PATCH 1/1] manager: fix preserving assume state during activation
Originally 850c977 "device: track system interface state in NMDevice",
intended that a connection can only be assumed initially when seeing
a device for the first time. Assuming a connection later was to be
prevented by setting device's sys-iface-state to MANAGED.
That changed too much in behavior, because we used to assume external
connections also when they are activated later on. So this was attempted
to get fixed by
- acf1067 nm-manager: try assuming connections on managed devices
- b6b7d90 manager: avoid generating in memory connections during startup for managed devices
It's probably just wrong to prevent assuming connections based on the
sys-iface-state. So drop the check for sys-iface-state from
recheck_assume_connection(). Now, we can assume anytime on managed,
disconnected interfaces, like previously.
Btw, note that priv->startup is totally wrong to check there, because
priv->startup has the sole purpose of tracking startup-complete property.
Startup, as far as NMManager is concerned, is platform_query_devices().
However, the problem is that we only assume connections (contrary to
doing external activation) when we have a connection-uuid from the state
file or with guess-assume during startup.
When assuming a master device, it can fail with
(nm-bond): ignoring generated connection (IPv6LL-only and not in master-slave relationship)
thus, for internal reason the device cannot be assumed yet.
Fix that by attatching the assume-state to the device, so that on multiple
recheck_assume_connection() calls we still try to assume. Whenever we try
to assume the connection and it fails due to external reasons (like, the connection
no longer matching), we clear the assume state, so that we only try as
long as there are internal reasons why assuming fails.
https://bugzilla.redhat.com/show_bug.cgi?id=1452062
(cherry picked from commit 729de7d7f09c3ad813477b7a822155b4b95dc682)
(cherry picked from commit 06db38b91d627f897b5bdd0de4a06f7b8a220902)
---
src/devices/nm-device.c | 86 ++++++++++++++++++++++++++++++++++++++++++---
src/devices/nm-device.h | 15 +++++++-
src/nm-manager.c | 93 +++++++++++++++++++++++++------------------------
3 files changed, 144 insertions(+), 50 deletions(-)
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index 1aea8cb..404dcf7 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -264,6 +264,9 @@ typedef struct _NMDevicePrivate {
bool nm_owned:1; /* whether the device is a device owned and created by NM */
+ bool assume_state_guess_assume:1;
+ char * assume_state_connection_uuid;
+
GHashTable * available_connections;
char * hw_addr;
char * hw_addr_perm;
@@ -527,6 +530,8 @@ static gboolean dhcp6_start (NMDevice *self, gboolean wait_for_ll);
static void nm_device_start_ip_check (NMDevice *self);
static void realize_start_setup (NMDevice *self,
const NMPlatformLink *plink,
+ gboolean assume_state_guess_assume,
+ const char *assume_state_connection_uuid,
gboolean set_nm_owned,
NMUnmanFlagOp unmanaged_user_explicit);
static void _commit_mtu (NMDevice *self, const NMIP4Config *config);
@@ -711,6 +716,52 @@ nm_device_sys_iface_state_set (NMDevice *self,
/*****************************************************************************/
+void
+nm_device_assume_state_get (NMDevice *self,
+ gboolean *out_assume_state_guess_assume,
+ const char **out_assume_state_connection_uuid)
+{
+ NMDevicePrivate *priv;
+
+ g_return_if_fail (NM_IS_DEVICE (self));
+
+ priv = NM_DEVICE_GET_PRIVATE (self);
+ NM_SET_OUT (out_assume_state_guess_assume, priv->assume_state_guess_assume);
+ NM_SET_OUT (out_assume_state_connection_uuid, priv->assume_state_connection_uuid);
+}
+
+static void
+_assume_state_set (NMDevice *self,
+ gboolean assume_state_guess_assume,
+ const char *assume_state_connection_uuid)
+{
+ NMDevicePrivate *priv;
+
+ nm_assert (NM_IS_DEVICE (self));
+
+ priv = NM_DEVICE_GET_PRIVATE (self);
+ if ( priv->assume_state_guess_assume == !!assume_state_guess_assume
+ && nm_streq0 (priv->assume_state_connection_uuid, assume_state_connection_uuid))
+ return;
+
+ _LOGD (LOGD_DEVICE, "assume-state: set guess-assume=%c, connection=%s%s%s",
+ assume_state_guess_assume ? '1' : '0',
+ NM_PRINT_FMT_QUOTE_STRING (assume_state_connection_uuid));
+ priv->assume_state_guess_assume = assume_state_guess_assume;
+ g_free (priv->assume_state_connection_uuid);
+ priv->assume_state_connection_uuid = g_strdup (assume_state_connection_uuid);
+}
+
+void
+nm_device_assume_state_reset (NMDevice *self)
+{
+ g_return_if_fail (NM_IS_DEVICE (self));
+
+ _assume_state_set (self, FALSE, NULL);
+}
+
+/*****************************************************************************/
+
static void
init_ip4_config_dns_priority (NMDevice *self, NMIP4Config *config)
{
@@ -2741,6 +2792,8 @@ link_type_compatible (NMDevice *self,
* nm_device_realize_start():
* @self: the #NMDevice
* @plink: an existing platform link or %NULL
+ * @assume_state_guess_assume: set the guess_assume state.
+ * @assume_state_connection_uuid: set the connection uuid to assume.
* @set_nm_owned: for software device, if TRUE set nm-owned.
* @unmanaged_user_explicit: the user-explicit unmanaged flag to apply
* on the device initially.
@@ -2759,6 +2812,8 @@ link_type_compatible (NMDevice *self,
gboolean
nm_device_realize_start (NMDevice *self,
const NMPlatformLink *plink,
+ gboolean assume_state_guess_assume,
+ const char *assume_state_connection_uuid,
gboolean set_nm_owned,
NMUnmanFlagOp unmanaged_user_explicit,
gboolean *out_compatible,
@@ -2784,8 +2839,11 @@ nm_device_realize_start (NMDevice *self,
plink_copy = *plink;
plink = &plink_copy;
}
- realize_start_setup (self, plink, set_nm_owned, unmanaged_user_explicit);
-
+ realize_start_setup (self, plink,
+ assume_state_guess_assume,
+ assume_state_connection_uuid,
+ set_nm_owned,
+ unmanaged_user_explicit);
return TRUE;
}
@@ -2824,7 +2882,10 @@ nm_device_create_and_realize (NMDevice *self,
plink = &plink_copy;
}
- realize_start_setup (self, plink, FALSE, NM_UNMAN_FLAG_OP_FORGET);
+ realize_start_setup (self, plink,
+ FALSE, /* assume_state_guess_assume */
+ NULL, /* assume_state_connection_uuid */
+ FALSE, NM_UNMAN_FLAG_OP_FORGET);
nm_device_realize_finish (self, plink);
if (nm_device_get_managed (self, FALSE)) {
@@ -2920,6 +2981,8 @@ realize_start_notify (NMDevice *self,
* realize_start_setup():
* @self: the #NMDevice
* @plink: the #NMPlatformLink if backed by a kernel netdevice
+ * @assume_state_guess_assume: set the guess_assume state.
+ * @assume_state_connection_uuid: set the connection uuid to assume.
* @set_nm_owned: if TRUE and device is a software-device, set nm-owned.
* TRUE.
* @unmanaged_user_explicit: the user-explict unmanaged flag to set.
@@ -2933,6 +2996,8 @@ realize_start_notify (NMDevice *self,
static void
realize_start_setup (NMDevice *self,
const NMPlatformLink *plink,
+ gboolean assume_state_guess_assume,
+ const char *assume_state_connection_uuid,
gboolean set_nm_owned,
NMUnmanFlagOp unmanaged_user_explicit)
{
@@ -2972,6 +3037,8 @@ realize_start_setup (NMDevice *self,
_notify (self, PROP_MTU);
}
+ _assume_state_set (self, assume_state_guess_assume, assume_state_connection_uuid);
+
nm_device_sys_iface_state_set (self, NM_DEVICE_SYS_IFACE_STATE_EXTERNAL);
if (plink) {
@@ -3193,6 +3260,8 @@ nm_device_unrealize (NMDevice *self, gboolean remove_resources, GError **error)
_LOGD (LOGD_DEVICE, "unrealize (ifindex %d)", ifindex > 0 ? ifindex : 0);
+ nm_device_assume_state_reset (self);
+
if (remove_resources) {
if (NM_DEVICE_GET_CLASS (self)->unrealize) {
if (!NM_DEVICE_GET_CLASS (self)->unrealize (self, error))
@@ -4042,7 +4111,7 @@ nm_device_master_update_slave_connection (NMDevice *self,
}
NMConnection *
-nm_device_generate_connection (NMDevice *self, NMDevice *master)
+nm_device_generate_connection (NMDevice *self, NMDevice *master, gboolean *out_maybe_later)
{
NMDeviceClass *klass = NM_DEVICE_GET_CLASS (self);
NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
@@ -4056,6 +4125,8 @@ nm_device_generate_connection (NMDevice *self, NMDevice *master)
GError *error = NULL;
const NMPlatformLink *pllink;
+ NM_SET_OUT (out_maybe_later, FALSE);
+
/* If update_connection() is not implemented, just fail. */
if (!klass->update_connection)
return NULL;
@@ -4131,6 +4202,7 @@ nm_device_generate_connection (NMDevice *self, NMDevice *master)
&& !nm_setting_connection_get_master (NM_SETTING_CONNECTION (s_con))
&& !priv->slaves) {
_LOGD (LOGD_DEVICE, "ignoring generated connection (no IP and not in master-slave relationship)");
+ NM_SET_OUT (out_maybe_later, TRUE);
g_object_unref (connection);
connection = NULL;
}
@@ -4145,6 +4217,7 @@ nm_device_generate_connection (NMDevice *self, NMDevice *master)
&& !priv->slaves
&& !nm_config_data_get_assume_ipv6ll_only (NM_CONFIG_GET_DATA, self)) {
_LOGD (LOGD_DEVICE, "ignoring generated connection (IPv6LL-only and not in master-slave relationship)");
+ NM_SET_OUT (out_maybe_later, TRUE);
g_object_unref (connection);
connection = NULL;
}
@@ -12504,6 +12577,9 @@ _set_state_full (NMDevice *self,
NM_DEVICE_SYS_IFACE_STATE_ASSUME))
nm_device_sys_iface_state_set (self, NM_DEVICE_SYS_IFACE_STATE_MANAGED);
+ if (state > NM_DEVICE_STATE_DISCONNECTED)
+ nm_device_assume_state_reset (self);
+
if (state <= NM_DEVICE_STATE_UNAVAILABLE) {
if (available_connections_del_all (self))
_notify (self, PROP_AVAILABLE_CONNECTIONS);
@@ -13778,6 +13854,8 @@ dispose (GObject *object)
nm_clear_g_cancellable (&priv->deactivating_cancellable);
+ nm_device_assume_state_reset (self);
+
_parent_set_ifindex (self, 0, FALSE);
platform = nm_device_get_platform (self);
diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h
index 74cc230..01e3938 100644
--- a/src/devices/nm-device.h
+++ b/src/devices/nm-device.h
@@ -487,7 +487,9 @@ void nm_device_removed (NMDevice *self, gboolean unconf
gboolean nm_device_is_available (NMDevice *dev, NMDeviceCheckDevAvailableFlags flags);
gboolean nm_device_has_carrier (NMDevice *dev);
-NMConnection * nm_device_generate_connection (NMDevice *self, NMDevice *master);
+NMConnection * nm_device_generate_connection (NMDevice *self,
+ NMDevice *master,
+ gboolean *out_maybe_later);
gboolean nm_device_master_update_slave_connection (NMDevice *master,
NMDevice *slave,
@@ -609,8 +611,19 @@ gboolean nm_device_is_nm_owned (NMDevice *device);
gboolean nm_device_has_capability (NMDevice *self, NMDeviceCapabilities caps);
+/*****************************************************************************/
+
+void nm_device_assume_state_get (NMDevice *self,
+ gboolean *out_assume_state_guess_assume,
+ const char **out_assume_state_connection_uuid);
+void nm_device_assume_state_reset (NMDevice *self);
+
+/*****************************************************************************/
+
gboolean nm_device_realize_start (NMDevice *device,
const NMPlatformLink *plink,
+ gboolean assume_state_guess_assume,
+ const char *assume_state_connection_uuid,
gboolean set_nm_owned,
NMUnmanFlagOp unmanaged_user_explicit,
gboolean *out_compatible,
diff --git a/src/nm-manager.c b/src/nm-manager.c
index 1daf633..9a7b123 100644
--- a/src/nm-manager.c
+++ b/src/nm-manager.c
@@ -1691,11 +1691,6 @@ done:
* get_existing_connection:
* @manager: #NMManager instance
* @device: #NMDevice instance
- * @guess_assume: whether to employ a heuristic to search for a matching
- * connection to assume.
- * @assume_connection_uuid: if present, try to assume a connection with this
- * UUID. If no uuid is given or no matching connection is found, we
- * only do external activation.
* @out_generated: (allow-none): return TRUE, if the connection was generated.
*
* Returns: a #NMSettingsConnection to be assumed by the device, or %NULL if
@@ -1704,8 +1699,6 @@ done:
static NMSettingsConnection *
get_existing_connection (NMManager *self,
NMDevice *device,
- gboolean guess_assume,
- const char *assume_connection_uuid,
gboolean *out_generated)
{
NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self);
@@ -1716,6 +1709,9 @@ get_existing_connection (NMManager *self,
int ifindex = nm_device_get_ifindex (device);
NMSettingsConnection *matched;
NMSettingsConnection *connection_checked = NULL;
+ gboolean assume_state_guess_assume = FALSE;
+ const char *assume_state_connection_uuid = NULL;
+ gboolean maybe_later;
if (out_generated)
*out_generated = FALSE;
@@ -1746,9 +1742,16 @@ get_existing_connection (NMManager *self,
* update_connection() implemented, otherwise nm_device_generate_connection()
* returns NULL.
*/
- connection = nm_device_generate_connection (device, master);
- if (!connection)
+ connection = nm_device_generate_connection (device, master, &maybe_later);
+ if (!connection) {
+ if (!maybe_later)
+ nm_device_assume_state_reset (device);
return NULL;
+ }
+
+ nm_device_assume_state_get (device,
+ &assume_state_guess_assume,
+ &assume_state_connection_uuid);
/* Now we need to compare the generated connection to each configured
* connection. The comparison function is the heart of the connection
@@ -1759,8 +1762,8 @@ get_existing_connection (NMManager *self,
* When no configured connection matches the generated connection, we keep
* the generated connection instead.
*/
- if ( assume_connection_uuid
- && (connection_checked = nm_settings_get_connection_by_uuid (priv->settings, assume_connection_uuid))
+ if ( assume_state_connection_uuid
+ && (connection_checked = nm_settings_get_connection_by_uuid (priv->settings, assume_state_connection_uuid))
&& !active_connection_find_first (self, connection_checked, NULL,
NM_ACTIVE_CONNECTION_STATE_DEACTIVATING)
&& nm_device_check_connection_compatible (device, NM_CONNECTION (connection_checked))) {
@@ -1778,7 +1781,7 @@ get_existing_connection (NMManager *self,
} else
matched = NULL;
- if (!matched && guess_assume) {
+ if (!matched && assume_state_guess_assume) {
gs_free NMSettingsConnection **connections = NULL;
guint len, i, j;
@@ -1812,9 +1815,10 @@ get_existing_connection (NMManager *self,
nm_device_get_iface (device),
nm_settings_connection_get_id (matched),
nm_settings_connection_get_uuid (matched),
- assume_connection_uuid && nm_streq (assume_connection_uuid, nm_settings_connection_get_uuid (matched))
+ assume_state_connection_uuid && nm_streq (assume_state_connection_uuid, nm_settings_connection_get_uuid (matched))
? " (indicated)" : " (guessed)");
g_object_unref (connection);
+ nm_device_assume_state_reset (device);
return matched;
}
@@ -1822,6 +1826,8 @@ get_existing_connection (NMManager *self,
nm_device_get_iface (device),
nm_connection_get_id (connection));
+ nm_device_assume_state_reset (device);
+
added = nm_settings_add_connection (priv->settings, connection, FALSE, &error);
if (added) {
nm_settings_connection_set_flags (NM_SETTINGS_CONNECTION (added),
@@ -1844,34 +1850,28 @@ get_existing_connection (NMManager *self,
static gboolean
recheck_assume_connection (NMManager *self,
- NMDevice *device,
- gboolean guess_assume,
- const char *assume_connection_uuid)
+ NMDevice *device)
{
NMSettingsConnection *connection;
gboolean was_unmanaged = FALSE;
gboolean generated = FALSE;
NMDeviceState state;
- NMDeviceSysIfaceState if_state;
- NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self);
g_return_val_if_fail (NM_IS_MANAGER (self), FALSE);
g_return_val_if_fail (NM_IS_DEVICE (device), FALSE);
- if (!nm_device_get_managed (device, FALSE))
+ if (!nm_device_get_managed (device, FALSE)) {
+ nm_device_assume_state_reset (device);
return FALSE;
+ }
state = nm_device_get_state (device);
- if (state > NM_DEVICE_STATE_DISCONNECTED)
- return FALSE;
-
- if_state = nm_device_sys_iface_state_get (device);
- if (!priv->startup && (if_state == NM_DEVICE_SYS_IFACE_STATE_MANAGED))
- nm_assert (!guess_assume && (assume_connection_uuid == NULL));
- else if (if_state != NM_DEVICE_SYS_IFACE_STATE_EXTERNAL)
+ if (state > NM_DEVICE_STATE_DISCONNECTED) {
+ nm_device_assume_state_reset (device);
return FALSE;
+ }
- connection = get_existing_connection (self, device, guess_assume, assume_connection_uuid, &generated);
+ connection = get_existing_connection (self, device, &generated);
if (!connection) {
_LOGD (LOGD_DEVICE, "(%s): can't assume; no connection",
nm_device_get_iface (device));
@@ -1951,9 +1951,9 @@ recheck_assume_connection (NMManager *self,
}
static void
-recheck_assume_connection_cb (NMDevice *device, gpointer user_data)
+recheck_assume_connection_cb (NMManager *self, NMDevice *device)
{
- recheck_assume_connection (user_data, device, FALSE, NULL);
+ recheck_assume_connection (self, device);
}
static void
@@ -2046,19 +2046,19 @@ device_connectivity_changed (NMDevice *device,
static void
_device_realize_finish (NMManager *self,
NMDevice *device,
- const NMPlatformLink *plink,
- gboolean guess_assume,
- const char *connection_uuid_to_assume)
+ const NMPlatformLink *plink)
{
g_return_if_fail (NM_IS_MANAGER (self));
g_return_if_fail (NM_IS_DEVICE (device));
nm_device_realize_finish (device, plink);
- if (!nm_device_get_managed (device, FALSE))
+ if (!nm_device_get_managed (device, FALSE)) {
+ nm_device_assume_state_reset (device);
return;
+ }
- if (recheck_assume_connection (self, device, guess_assume, connection_uuid_to_assume))
+ if (recheck_assume_connection (self, device))
return;
/* if we failed to assume a connection for the managed device, but the device
@@ -2128,9 +2128,9 @@ add_device (NMManager *self, NMDevice *device, GError **error)
G_CALLBACK (device_removed_cb),
self);
- g_signal_connect (device, NM_DEVICE_RECHECK_ASSUME,
- G_CALLBACK (recheck_assume_connection_cb),
- self);
+ g_signal_connect_data (device, NM_DEVICE_RECHECK_ASSUME,
+ G_CALLBACK (recheck_assume_connection_cb),
+ self, NULL, G_CONNECT_SWAPPED);
g_signal_connect (device, "notify::" NM_DEVICE_IP_IFACE,
G_CALLBACK (device_ip_iface_changed),
@@ -2212,12 +2212,14 @@ factory_device_added_cb (NMDeviceFactory *factory,
if (nm_device_realize_start (device,
NULL,
- FALSE,
+ FALSE, /* assume_state_guess_assume */
+ NULL, /* assume_state_connection_uuid */
+ FALSE, /* set_nm_owned */
NM_UNMAN_FLAG_OP_FORGET,
NULL,
&error)) {
add_device (self, device, NULL);
- _device_realize_finish (self, device, NULL, FALSE, NULL);
+ _device_realize_finish (self, device, NULL);
} else {
_LOGW (LOGD_DEVICE, "(%s): failed to realize device: %s",
nm_device_get_iface (device), error->message);
@@ -2291,12 +2293,13 @@ platform_link_added (NMManager *self,
return;
} else if (nm_device_realize_start (candidate,
plink,
- FALSE,
+ FALSE, /* assume_state_guess_assume */
+ NULL, /* assume_state_connection_uuid */
+ FALSE, /* set_nm_owned */
NM_UNMAN_FLAG_OP_FORGET,
&compatible,
&error)) {
- /* Success */
- _device_realize_finish (self, candidate, plink, FALSE, NULL);
+ _device_realize_finish (self, candidate, plink);
return;
}
@@ -2363,14 +2366,14 @@ platform_link_added (NMManager *self,
if (nm_device_realize_start (device,
plink,
+ guess_assume,
+ dev_state ? dev_state->connection_uuid : NULL,
dev_state ? (dev_state->nm_owned == 1) : FALSE,
unmanaged_user_explicit,
NULL,
&error)) {
add_device (self, device, NULL);
- _device_realize_finish (self, device, plink,
- guess_assume,
- dev_state ? dev_state->connection_uuid : NULL);
+ _device_realize_finish (self, device, plink);
} else {
_LOGW (LOGD_DEVICE, "%s: failed to realize device: %s",
plink->name, error->message);
--
2.9.4