colin/NetworkManager
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

device: fix assuming connections when platform-init arrives late

Browse Source 
When the NM_UNMANAGED_PLATFORM_INIT flag is cleared last in
device_link_changed(), a recheck-assume is scheduled and then the
device goes immediately to UNAVAILABLE. During the state transition,
addresses and routes are removed from the interface. Then,
recheck-assume finds that the device can be assumed but it's too late
since the device was already deconfigured.

This is a problem as the whole point of assuming a device is to
activate a connection while leaving the device untouched.

In the NMCI "dracut_NM_vlan_over_bridge and dracut_NM_vlan_over_bond"
test, NM in real root tries to assume a vlan device that was activated
in initrd. When the interface gets deconfigured in UNAVAILABLE, the
connection to the NFS server breaks and the rootfs becomes
inaccessible.

The fix to this problem is to delay state transitions in
device_link_changed() to a idle handler, so that recheck-assume can
run before.

Fixes-test: @dracut_NM_vlan_over_bridge
Fixes-test: @dracut_NM_vlan_over_bond

https://bugzilla.redhat.com/show_bug.cgi?id=2047302
This commit is contained in:
Beniamino Galvani
2022-02-02 09:39:23 +01:00
parent b2e1bd4436
commit d72a292005
3 changed files with 34 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

47
src/core/devices/nm-device.c
View File

@@ -825,7 +825,8 @@ static void _dev_ipac6_start(NMDevice *self);
static void _dev_ipac6_set_state(NMDevice *self, NMDeviceIPState state);
static void _dev_unamanged_check_external_down(NMDevice *self, gboolean only_if_unmanaged);
static void
_dev_unmanaged_check_external_down(NMDevice *self, gboolean only_if_unmanaged, gboolean now);
static void _dev_ipshared4_start(NMDevice *self);
static void _dev_ipshared4_spawn_dnsmasq(NMDevice *self);
@@ -3914,7 +3915,7 @@ _dev_l3_cfg_notify_cb(NML3Cfg *l3cfg, const NML3ConfigNotifyData *notify_data, N
nmp_object_type_to_flags(NMP_OBJECT_TYPE_LINK)
| nmp_object_type_to_flags(NMP_OBJECT_TYPE_IP4_ADDRESS)
| nmp_object_type_to_flags(NMP_OBJECT_TYPE_IP6_ADDRESS)))
_dev_unamanged_check_external_down(self, TRUE);
_dev_unmanaged_check_external_down(self, TRUE, TRUE);
if (NM_FLAGS_ANY(notify_data->platform_change_on_idle.obj_type_flags,
nmp_object_type_to_flags(NMP_OBJECT_TYPE_IP4_ADDRESS)
@@ -6047,7 +6048,7 @@ _dev_unmanaged_is_external_down(NMDevice *self, gboolean consider_can)
}
static void
_dev_unamanged_check_external_down(NMDevice *self, gboolean only_if_unmanaged)
_dev_unmanaged_check_external_down(NMDevice *self, gboolean only_if_unmanaged, gboolean now)
{
NMUnmanFlagOp ext_flags;
@@ -6067,10 +6068,17 @@ _dev_unamanged_check_external_down(NMDevice *self, gboolean only_if_unmanaged)
nm_device_queue_recheck_assume(self);
}
nm_device_set_unmanaged_by_flags(self,
NM_UNMANAGED_EXTERNAL_DOWN,
ext_flags,
NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED);
if (now) {
nm_device_set_unmanaged_by_flags(self,
NM_UNMANAGED_EXTERNAL_DOWN,
ext_flags,
NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED);
} else {
nm_device_set_unmanaged_by_flags_queue(self,
NM_UNMANAGED_EXTERNAL_DOWN,
ext_flags,
NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED);
}
}
void
@@ -6522,11 +6530,16 @@ device_link_changed(gpointer user_data)
}
}
/* The assume check should happen before the device transitions to
* UNAVAILABLE, because in UNAVAILABLE we already clean up the IP
* configuration. Therefore, this function should never trigger a
* sync state transition.
*/
nm_device_queue_recheck_assume(self);
nm_device_set_unmanaged_by_flags(self, NM_UNMANAGED_PLATFORM_INIT, FALSE, reason);
nm_device_set_unmanaged_by_flags_queue(self, NM_UNMANAGED_PLATFORM_INIT, FALSE, reason);
}
_dev_unamanged_check_external_down(self, FALSE);
_dev_unmanaged_check_external_down(self, FALSE, FALSE);
device_recheck_slave_status(self, pllink);
@@ -6545,7 +6558,7 @@ device_link_changed(gpointer user_data)
}
if (update_unmanaged_specs)
nm_device_set_unmanaged_by_user_settings(self);
nm_device_set_unmanaged_by_user_settings(self, FALSE);
if (got_hw_addr && !priv->up && nm_device_get_state(self) == NM_DEVICE_STATE_UNAVAILABLE) {
/*
@@ -14127,7 +14140,7 @@ nm_device_check_unrealized_device_managed(NMDevice *self)
}
void
nm_device_set_unmanaged_by_user_settings(NMDevice *self)
nm_device_set_unmanaged_by_user_settings(NMDevice *self, gboolean now)
{
gboolean unmanaged;
@@ -14149,11 +14162,13 @@ nm_device_set_unmanaged_by_user_settings(NMDevice *self)
self,
nm_settings_get_unmanaged_specs(NM_DEVICE_GET_PRIVATE(self)->settings));
nm_device_set_unmanaged_by_flags(self,
NM_UNMANAGED_USER_SETTINGS,
!!unmanaged,
unmanaged ? NM_DEVICE_STATE_REASON_NOW_UNMANAGED
: NM_DEVICE_STATE_REASON_NOW_MANAGED);
_set_unmanaged_flags(self,
NM_UNMANAGED_USER_SETTINGS,
!!unmanaged,
TRUE,
now,
unmanaged ? NM_DEVICE_STATE_REASON_NOW_UNMANAGED
: NM_DEVICE_STATE_REASON_NOW_MANAGED);
}
void

2
src/core/devices/nm-device.h
View File

@@ -621,7 +621,7 @@ void nm_device_set_unmanaged_by_flags_queue(NMDevice *self,
NMUnmanagedFlags flags,
NMUnmanFlagOp set_op,
NMDeviceStateReason reason);
void nm_device_set_unmanaged_by_user_settings(NMDevice *self);
void nm_device_set_unmanaged_by_user_settings(NMDevice *self, gboolean now);
void nm_device_set_unmanaged_by_user_udev(NMDevice *self);
void nm_device_set_unmanaged_by_user_conf(NMDevice *self);
void nm_device_set_unmanaged_by_quitting(NMDevice *device);

4
src/core/nm-manager.c
View File

@@ -2266,7 +2266,7 @@ system_unmanaged_devices_changed_cb(NMSettings *settings, GParamSpec *pspec, gpo
NMDevice *device;
c_list_for_each_entry (device, &priv->devices_lst_head, devices_lst)
nm_device_set_unmanaged_by_user_settings(device);
nm_device_set_unmanaged_by_user_settings(device, TRUE);
}
static void
@@ -3304,7 +3304,7 @@ add_device(NMManager *self, NMDevice *device, GError **error)
type_desc = nm_device_get_type_desc(device);
g_assert(type_desc);
nm_device_set_unmanaged_by_user_settings(device);
nm_device_set_unmanaged_by_user_settings(device, TRUE);
nm_device_set_unmanaged_flags(device, NM_UNMANAGED_SLEEPING, manager_sleeping(self));