From 634dd2f5e865c9ec167e0ca4fb04802e06ebd5d0 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Sat, 21 Oct 2023 22:22:15 +0200 Subject: [PATCH 1/2] core: add "reason" argument to NMActiveConnection device_state_changed() NMActiveConnection implements method device_state_changed() that re-emits device state changes as convenience for subclasses. Add the reason for the state change to the handler, as it will be used in the next commit. --- src/core/nm-act-request.c | 3 ++- src/core/nm-active-connection.c | 3 ++- src/core/nm-active-connection.h | 3 ++- src/core/vpn/nm-vpn-connection.c | 3 ++- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/core/nm-act-request.c b/src/core/nm-act-request.c index dce18ba4ec..8fcff63f57 100644 --- a/src/core/nm-act-request.c +++ b/src/core/nm-act-request.c @@ -259,7 +259,8 @@ static void device_state_changed(NMActiveConnection *active, NMDevice *device, NMDeviceState new_state, - NMDeviceState old_state) + NMDeviceState old_state, + NMDeviceStateReason reason) { NMActiveConnectionState cur_ac_state = nm_active_connection_get_state(active); NMActiveConnectionState ac_state = NM_ACTIVE_CONNECTION_STATE_UNKNOWN; diff --git a/src/core/nm-active-connection.c b/src/core/nm-active-connection.c index 36a11f7151..5feab966de 100644 --- a/src/core/nm-active-connection.c +++ b/src/core/nm-active-connection.c @@ -636,7 +636,8 @@ device_state_changed(NMDevice *device, NM_ACTIVE_CONNECTION_GET_CLASS(self)->device_state_changed(self, device, new_state, - old_state); + old_state, + reason); } static void diff --git a/src/core/nm-active-connection.h b/src/core/nm-active-connection.h index 15db68c369..8032294f3c 100644 --- a/src/core/nm-active-connection.h +++ b/src/core/nm-active-connection.h @@ -78,7 +78,8 @@ typedef struct { void (*device_state_changed)(NMActiveConnection *connection, NMDevice *device, NMDeviceState new_state, - NMDeviceState old_state); + NMDeviceState old_state, + NMDeviceStateReason reason); void (*master_failed)(NMActiveConnection *connection); void (*device_changed)(NMActiveConnection *connection, diff --git a/src/core/vpn/nm-vpn-connection.c b/src/core/vpn/nm-vpn-connection.c index d7102a12ef..3dba9ff6c8 100644 --- a/src/core/vpn/nm-vpn-connection.c +++ b/src/core/vpn/nm-vpn-connection.c @@ -1143,7 +1143,8 @@ static void device_state_changed(NMActiveConnection *active, NMDevice *device, NMDeviceState new_state, - NMDeviceState old_state) + NMDeviceState old_state, + NMDeviceStateReason reason) { if (_service_and_connection_can_persist(NM_VPN_CONNECTION(active))) { if (new_state <= NM_DEVICE_STATE_DISCONNECTED || new_state == NM_DEVICE_STATE_FAILED) { From d3db0883c7723a9e150ea9856bd8678480c99874 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Sun, 22 Oct 2023 08:38:55 +0200 Subject: [PATCH 2/2] core: propagate the user-requested reason for act-request disconnection If the device is being disconnected for a user request, at the moment the active connection goes to state DEACTIVATED through the following transitions, independently of the reason for the disconnection: - state: DEACTIVATING, reason: UNKNOWN - state: DEACTIVATED, reason: DEVICE_DISCONNECTED For VPNs, a disconnection is always user-initiated, and the active connection states emitted are: - state: DEACTIVATING, reason: USER_DISCONNECTED - state: DEACTIVATED, reason: USER_DISCONNECTED This difference poses problems for clients that want to handle device and VPNs in the same way, especially because WireGuard is implemented as a device, but is logically a VPN. Let NMActRequest translate the USER_REQUESTED device state reason to USER_DISCONNECTED active connection state reason, in case of disconnection. This is an API change, but the previous behavior of reporting generic uninformative reasons seems a bug. See for example nmc_activation_get_effective_state(), which inspects the AC state reason and in case it's generic (DEVICE_DISCONNECTED), it considers the device state instead. https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1405 --- src/core/nm-act-request.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/core/nm-act-request.c b/src/core/nm-act-request.c index 8fcff63f57..bed7ffde18 100644 --- a/src/core/nm-act-request.c +++ b/src/core/nm-act-request.c @@ -320,14 +320,20 @@ device_state_changed(NMActiveConnection *active, active); break; case NM_DEVICE_STATE_DEACTIVATING: + if (reason == NM_DEVICE_STATE_REASON_USER_REQUESTED) + ac_state_reason = NM_ACTIVE_CONNECTION_STATE_REASON_USER_DISCONNECTED; + ac_state = NM_ACTIVE_CONNECTION_STATE_DEACTIVATING; break; case NM_DEVICE_STATE_FAILED: case NM_DEVICE_STATE_DISCONNECTED: case NM_DEVICE_STATE_UNMANAGED: case NM_DEVICE_STATE_UNAVAILABLE: - ac_state = NM_ACTIVE_CONNECTION_STATE_DEACTIVATED; - ac_state_reason = NM_ACTIVE_CONNECTION_STATE_REASON_DEVICE_DISCONNECTED; + ac_state = NM_ACTIVE_CONNECTION_STATE_DEACTIVATED; + if (reason == NM_DEVICE_STATE_REASON_USER_REQUESTED) + ac_state_reason = NM_ACTIVE_CONNECTION_STATE_REASON_USER_DISCONNECTED; + else + ac_state_reason = NM_ACTIVE_CONNECTION_STATE_REASON_DEVICE_DISCONNECTED; g_signal_handlers_disconnect_by_func(device, G_CALLBACK(device_notify), active); break;