From d3e5eab73423a7df099d4f9317e5159ee47e056a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 27 Feb 2020 15:43:28 +0100 Subject: [PATCH 1/8] shared: add nm_g_variant_is_of_type() helper --- shared/nm-glib-aux/nm-shared-utils.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/shared/nm-glib-aux/nm-shared-utils.h b/shared/nm-glib-aux/nm-shared-utils.h index 115f9179d5..bba60954b1 100644 --- a/shared/nm-glib-aux/nm-shared-utils.h +++ b/shared/nm-glib-aux/nm-shared-utils.h @@ -1062,6 +1062,14 @@ nm_g_variant_lookup_value (GVariant *dictionary, : NULL; } +static inline gboolean +nm_g_variant_is_of_type (GVariant *value, + const GVariantType *type) +{ + return value + && g_variant_is_of_type (value, type); +} + static inline void nm_g_source_destroy_and_unref (GSource *source) { From 607a22b900cb81fa122fdf323fe4b6c1ba5a56e2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 27 Feb 2020 16:36:50 +0100 Subject: [PATCH 2/8] libnm-core: minor cleanup checks in "nm-setting-adsl.c"'s verify() --- libnm-core/nm-setting-adsl.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/libnm-core/nm-setting-adsl.c b/libnm-core/nm-setting-adsl.c index e9b8666974..c952353d7b 100644 --- a/libnm-core/nm-setting-adsl.c +++ b/libnm-core/nm-setting-adsl.c @@ -158,7 +158,8 @@ verify (NMSetting *setting, NMConnection *connection, GError **error) _("property is missing")); g_prefix_error (error, "%s.%s: ", NM_SETTING_ADSL_SETTING_NAME, NM_SETTING_ADSL_USERNAME); return FALSE; - } else if (!strlen (priv->username)) { + } + if (!priv->username[0]) { g_set_error_literal (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_INVALID_PROPERTY, @@ -167,10 +168,9 @@ verify (NMSetting *setting, NMConnection *connection, GError **error) return FALSE; } - if ( !priv->protocol - || ( strcmp (priv->protocol, NM_SETTING_ADSL_PROTOCOL_PPPOA) - && strcmp (priv->protocol, NM_SETTING_ADSL_PROTOCOL_PPPOE) - && strcmp (priv->protocol, NM_SETTING_ADSL_PROTOCOL_IPOATM))){ + if (!NM_IN_STRSET (priv->protocol, NM_SETTING_ADSL_PROTOCOL_PPPOA, + NM_SETTING_ADSL_PROTOCOL_PPPOE, + NM_SETTING_ADSL_PROTOCOL_IPOATM)) { g_set_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_INVALID_PROPERTY, @@ -180,9 +180,9 @@ verify (NMSetting *setting, NMConnection *connection, GError **error) return FALSE; } - if ( priv->encapsulation - && ( strcmp (priv->encapsulation, NM_SETTING_ADSL_ENCAPSULATION_VCMUX) - && strcmp (priv->encapsulation, NM_SETTING_ADSL_ENCAPSULATION_LLC) )) { + if (!NM_IN_STRSET (priv->encapsulation, NULL, + NM_SETTING_ADSL_ENCAPSULATION_VCMUX, + NM_SETTING_ADSL_ENCAPSULATION_LLC)) { g_set_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_INVALID_PROPERTY, From aa991916dcc280c8ff16584db57ff49639e30596 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 27 Feb 2020 16:45:16 +0100 Subject: [PATCH 3/8] device: various code cleanups in devices Mostly just cleanups, there should be no significant change in behavior. --- src/devices/adsl/nm-device-adsl.c | 103 ++++++++++++------------- src/devices/nm-device-bridge.c | 52 ++++++------- src/devices/nm-device-ethernet.c | 83 ++++++++++---------- src/devices/nm-device-macsec.c | 19 ++--- src/devices/nm-device-ppp.c | 19 +++-- src/devices/nm-device-wireguard.c | 26 +++---- src/devices/nm-device.c | 9 ++- src/devices/wifi/nm-device-iwd.c | 43 +++++------ src/devices/wifi/nm-device-olpc-mesh.c | 2 +- src/devices/wifi/nm-device-wifi-p2p.c | 4 +- src/devices/wifi/nm-device-wifi.c | 48 +++++------- src/devices/wwan/nm-device-modem.c | 1 - 12 files changed, 193 insertions(+), 216 deletions(-) diff --git a/src/devices/adsl/nm-device-adsl.c b/src/devices/adsl/nm-device-adsl.c index 1d39ad5334..fe4055eb66 100644 --- a/src/devices/adsl/nm-device-adsl.c +++ b/src/devices/adsl/nm-device-adsl.c @@ -228,7 +228,8 @@ link_changed_cb (NMPlatform *platform, /* This only gets called for PPPoE connections and "nas" interfaces */ - if (priv->nas_ifindex > 0 && ifindex == priv->nas_ifindex) { + if ( priv->nas_ifindex > 0 + && ifindex == priv->nas_ifindex) { /* NAS device went away for some reason; kill the connection */ _LOGD (LOGD_ADSL, "br2684 interface disappeared"); nm_device_state_changed (device, @@ -273,11 +274,17 @@ nas_update_cb (gpointer user_data) NMDeviceAdslPrivate *priv = NM_DEVICE_ADSL_GET_PRIVATE (self); NMDevice *device = NM_DEVICE (self); - g_assert (priv->nas_ifname); + nm_assert (priv->nas_ifname); priv->nas_update_count++; - if (priv->nas_update_count > 10) { + nm_assert (priv->nas_ifindex <= 0); + priv->nas_ifindex = nm_platform_link_get_ifindex (nm_device_get_platform (device), priv->nas_ifname); + if (priv->nas_ifindex <= 0) { + if (priv->nas_update_count <= 10) { + /* Keep waiting for it to appear */ + return G_SOURCE_CONTINUE; + } priv->nas_update_id = 0; _LOGW (LOGD_ADSL, "failed to find br2684 interface %s ifindex after timeout", priv->nas_ifname); nm_device_state_changed (device, @@ -286,31 +293,22 @@ nas_update_cb (gpointer user_data) return G_SOURCE_REMOVE; } - g_warn_if_fail (priv->nas_ifindex < 0); - priv->nas_ifindex = nm_platform_link_get_ifindex (nm_device_get_platform (device), priv->nas_ifname); - if (priv->nas_ifindex < 0) { - /* Keep waiting for it to appear */ - return G_SOURCE_CONTINUE; - } - priv->nas_update_id = 0; _LOGD (LOGD_ADSL, "using br2684 iface '%s' index %d", priv->nas_ifname, priv->nas_ifindex); - if (pppoe_vcc_config (self)) { - nm_device_activate_schedule_stage3_ip_config_start (device); - } else { + if (!pppoe_vcc_config (self)) { nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_BR2684_FAILED); + return G_SOURCE_REMOVE; } + nm_device_activate_schedule_stage3_ip_config_start (device); return G_SOURCE_REMOVE; } -static NMActStageReturn -br2684_create_iface (NMDeviceAdsl *self, - NMSettingAdsl *s_adsl, - NMDeviceStateReason *out_failure_reason) +static gboolean +br2684_create_iface (NMDeviceAdsl *self) { NMDeviceAdslPrivate *priv = NM_DEVICE_ADSL_GET_PRIVATE (self); struct atm_newif_br2684 ni; @@ -318,19 +316,14 @@ br2684_create_iface (NMDeviceAdsl *self, int err, errsv; guint num = 0; - g_return_val_if_fail (s_adsl != NULL, FALSE); - - if (priv->nas_update_id) { - g_warn_if_fail (priv->nas_update_id == 0); - nm_clear_g_source (&priv->nas_update_id); - } + if (nm_clear_g_source (&priv->nas_update_id)) + nm_assert_not_reached (); fd = socket (PF_ATMPVC, SOCK_DGRAM | SOCK_CLOEXEC, ATM_AAL5); if (fd < 0) { errsv = errno; _LOGE (LOGD_ADSL, "failed to open ATM control socket (%d)", errsv); - NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_BR2684_FAILED); - return NM_ACT_STAGE_RETURN_FAILURE; + return FALSE; } memset (&ni, 0, sizeof (ni)); @@ -343,36 +336,32 @@ br2684_create_iface (NMDeviceAdsl *self, * cannot return that name to us. Since we want to know the name right * away, just brute-force it. */ - while (num < 10000) { + while (TRUE) { memset (&ni.ifname, 0, sizeof (ni.ifname)); - g_snprintf (ni.ifname, sizeof (ni.ifname), "nas%d", num++); + g_snprintf (ni.ifname, sizeof (ni.ifname), "nas%u", num++); err = ioctl (fd, ATM_NEWBACKENDIF, &ni); - if (err == 0) { - g_free (priv->nas_ifname); - priv->nas_ifname = g_strdup (ni.ifname); - _LOGD (LOGD_ADSL, "waiting for br2684 iface '%s' to appear", priv->nas_ifname); + if (err != 0) { + errsv = errno; + if (errsv == EEXIST) + continue; - priv->nas_update_count = 0; - priv->nas_update_id = g_timeout_add (100, nas_update_cb, self); - return NM_ACT_STAGE_RETURN_POSTPONE; - } - errsv = errno; - if (errsv != EEXIST) { _LOGW (LOGD_ADSL, "failed to create br2684 interface (%d)", errsv); - break; + return FALSE; } - } - NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_BR2684_FAILED); - return NM_ACT_STAGE_RETURN_FAILURE; + nm_utils_strdup_reset (&priv->nas_ifname, ni.ifname); + _LOGD (LOGD_ADSL, "waiting for br2684 iface '%s' to appear", priv->nas_ifname); + priv->nas_update_count = 0; + priv->nas_update_id = g_timeout_add (100, nas_update_cb, self); + return TRUE; + } } static NMActStageReturn act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) { NMDeviceAdsl *self = NM_DEVICE_ADSL (device); - NMActStageReturn ret = NM_ACT_STAGE_RETURN_FAILURE; NMSettingAdsl *s_adsl; const char *protocol; @@ -383,16 +372,22 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) protocol = nm_setting_adsl_get_protocol (s_adsl); _LOGD (LOGD_ADSL, "using ADSL protocol '%s'", protocol); - if (g_strcmp0 (protocol, NM_SETTING_ADSL_PROTOCOL_PPPOE) == 0) { - /* PPPoE needs RFC2684 bridging before we can do PPP over it */ - ret = br2684_create_iface (self, s_adsl, out_failure_reason); - } else if (g_strcmp0 (protocol, NM_SETTING_ADSL_PROTOCOL_PPPOA) == 0) { + if (nm_streq0 (protocol, NM_SETTING_ADSL_PROTOCOL_PPPOA)) { /* PPPoA doesn't need anything special */ - ret = NM_ACT_STAGE_RETURN_SUCCESS; - } else - _LOGW (LOGD_ADSL, "unhandled ADSL protocol '%s'", protocol); + return NM_ACT_STAGE_RETURN_SUCCESS; + } - return ret; + if (nm_streq0 (protocol, NM_SETTING_ADSL_PROTOCOL_PPPOE)) { + /* PPPoE needs RFC2684 bridging before we can do PPP over it */ + if (!br2684_create_iface (self)) { + NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_BR2684_FAILED); + return NM_ACT_STAGE_RETURN_FAILURE; + } + return NM_ACT_STAGE_RETURN_POSTPONE; + } + + _LOGW (LOGD_ADSL, "unhandled ADSL protocol '%s'", protocol); + return NM_ACT_STAGE_RETURN_SUCCESS; } static void @@ -460,8 +455,8 @@ act_stage3_ip4_config_start (NMDevice *device, g_return_val_if_fail (s_adsl, NM_ACT_STAGE_RETURN_FAILURE); /* PPPoE uses the NAS interface, not the ATM interface */ - if (g_strcmp0 (nm_setting_adsl_get_protocol (s_adsl), NM_SETTING_ADSL_PROTOCOL_PPPOE) == 0) { - g_assert (priv->nas_ifname); + if (nm_streq0 (nm_setting_adsl_get_protocol (s_adsl), NM_SETTING_ADSL_PROTOCOL_PPPOE)) { + nm_assert (priv->nas_ifname); ppp_iface = priv->nas_ifname; _LOGD (LOGD_ADSL, "starting PPPoE on br2684 interface %s", priv->nas_ifname); @@ -540,8 +535,8 @@ adsl_cleanup (NMDeviceAdsl *self) * so it gets leaked. It does get destroyed when it's no longer in use, * but we have no control over that. */ - priv->nas_ifindex = -1; - g_clear_pointer (&priv->nas_ifname, g_free); + priv->nas_ifindex = 0; + nm_clear_g_free (&priv->nas_ifname); } static void diff --git a/src/devices/nm-device-bridge.c b/src/devices/nm-device-bridge.c index 53221080c0..5eaf5a98c9 100644 --- a/src/devices/nm-device-bridge.c +++ b/src/devices/nm-device-bridge.c @@ -561,40 +561,38 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) NMDeviceBridge *self = NM_DEVICE_BRIDGE (device); NMConnection *connection; NMSettingBluetooth *s_bt; + gs_free_error GError *error = NULL; connection = nm_device_get_applied_connection (device); s_bt = _nm_connection_get_setting_bluetooth_for_nap (connection); - if (s_bt) { - gs_free_error GError *error = NULL; + if (!s_bt) + return NM_ACT_STAGE_RETURN_SUCCESS; - if (!nm_bt_vtable_network_server) { - _LOGD (LOGD_DEVICE, "bluetooth NAP server failed because bluetooth plugin not available"); - *out_failure_reason = NM_DEVICE_STATE_REASON_BT_FAILED; - return NM_ACT_STAGE_RETURN_FAILURE; - } - - if (self->bt_cancellable) - return NM_ACT_STAGE_RETURN_POSTPONE; - - self->bt_cancellable = g_cancellable_new (); - if (!nm_bt_vtable_network_server->register_bridge (nm_bt_vtable_network_server, - nm_setting_bluetooth_get_bdaddr (s_bt), - device, - self->bt_cancellable, - _bt_register_bridge_cb, - device, - &error)) { - _LOGD (LOGD_DEVICE, "bluetooth NAP server failed to register bridge: %s", error->message); - *out_failure_reason = NM_DEVICE_STATE_REASON_BT_FAILED; - return NM_ACT_STAGE_RETURN_FAILURE; - } - - self->bt_registered = TRUE; - return NM_ACT_STAGE_RETURN_POSTPONE; + if (!nm_bt_vtable_network_server) { + _LOGD (LOGD_DEVICE, "bluetooth NAP server failed because bluetooth plugin not available"); + *out_failure_reason = NM_DEVICE_STATE_REASON_BT_FAILED; + return NM_ACT_STAGE_RETURN_FAILURE; } - return NM_ACT_STAGE_RETURN_SUCCESS; + if (self->bt_cancellable) + return NM_ACT_STAGE_RETURN_POSTPONE; + + self->bt_cancellable = g_cancellable_new (); + if (!nm_bt_vtable_network_server->register_bridge (nm_bt_vtable_network_server, + nm_setting_bluetooth_get_bdaddr (s_bt), + device, + self->bt_cancellable, + _bt_register_bridge_cb, + device, + &error)) { + _LOGD (LOGD_DEVICE, "bluetooth NAP server failed to register bridge: %s", error->message); + *out_failure_reason = NM_DEVICE_STATE_REASON_BT_FAILED; + return NM_ACT_STAGE_RETURN_FAILURE; + } + + self->bt_registered = TRUE; + return NM_ACT_STAGE_RETURN_POSTPONE; } static void diff --git a/src/devices/nm-device-ethernet.c b/src/devices/nm-device-ethernet.c index 0f66fe8680..d54350d24b 100644 --- a/src/devices/nm-device-ethernet.c +++ b/src/devices/nm-device-ethernet.c @@ -734,7 +734,7 @@ supplicant_iface_state_cb (NMSupplicantInterface *iface, supplicant_iface_state_is_completed (self, new_state); } -static NMActStageReturn +static gboolean handle_auth_or_fail (NMDeviceEthernet *self, NMActRequest *req, gboolean new_secrets) @@ -743,7 +743,7 @@ handle_auth_or_fail (NMDeviceEthernet *self, NMConnection *applied_connection; if (!nm_device_auth_retries_try_next (NM_DEVICE (self))) - return NM_ACT_STAGE_RETURN_FAILURE; + return FALSE; nm_device_state_changed (NM_DEVICE (self), NM_DEVICE_STATE_NEED_AUTH, NM_DEVICE_STATE_REASON_NONE); @@ -753,7 +753,7 @@ handle_auth_or_fail (NMDeviceEthernet *self, setting_name = nm_connection_need_secrets (applied_connection, NULL); if (!setting_name) { _LOGI (LOGD_DEVICE, "Cleared secrets, but setting didn't need any secrets."); - return NM_ACT_STAGE_RETURN_FAILURE; + return FALSE; } _LOGI (LOGD_DEVICE | LOGD_ETHER, "Activation: (ethernet) asking for new secrets"); @@ -768,7 +768,7 @@ handle_auth_or_fail (NMDeviceEthernet *self, wired_secrets_get_secrets (self, setting_name, NM_SECRET_AGENT_GET_SECRETS_FLAG_ALLOW_INTERACTION | (new_secrets ? NM_SECRET_AGENT_GET_SECRETS_FLAG_REQUEST_NEW : 0)); - return NM_ACT_STAGE_RETURN_POSTPONE; + return TRUE; } static gboolean @@ -800,7 +800,7 @@ supplicant_connection_timeout_cb (gpointer user_data) if (nm_settings_connection_get_timestamp (connection, ×tamp)) new_secrets = !timestamp; - if (handle_auth_or_fail (self, req, new_secrets) == NM_ACT_STAGE_RETURN_FAILURE) { + if (!handle_auth_or_fail (self, req, new_secrets)) { wired_auth_cond_fail (self, NM_DEVICE_STATE_REASON_NO_SECRETS); return G_SOURCE_REMOVE; } @@ -1003,7 +1003,6 @@ supplicant_check_secrets_needed (NMDeviceEthernet *self, NMDeviceStateReason *ou NMConnection *connection; NMSetting8021x *security; const char *setting_name; - NMActStageReturn ret = NM_ACT_STAGE_RETURN_FAILURE; connection = nm_device_get_applied_connection (NM_DEVICE (self)); g_return_val_if_fail (connection, NM_ACT_STAGE_RETURN_FAILURE); @@ -1012,7 +1011,7 @@ supplicant_check_secrets_needed (NMDeviceEthernet *self, NMDeviceStateReason *ou if (!security) { _LOGE (LOGD_DEVICE, "Invalid or missing 802.1X security"); NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_CONFIG_FAILED); - return ret; + return NM_ACT_STAGE_RETURN_FAILURE; } if (!priv->supplicant.mgr) @@ -1027,10 +1026,11 @@ supplicant_check_secrets_needed (NMDeviceEthernet *self, NMDeviceStateReason *ou "Activation: (ethernet) connection '%s' has security, but secrets are required.", nm_connection_get_id (connection)); - ret = handle_auth_or_fail (self, req, FALSE); - if (ret != NM_ACT_STAGE_RETURN_POSTPONE) + if (!handle_auth_or_fail (self, req, FALSE)) { NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_NO_SECRETS); - return ret; + return NM_ACT_STAGE_RETURN_FAILURE; + } + return NM_ACT_STAGE_RETURN_POSTPONE; } _LOGI (LOGD_DEVICE | LOGD_ETHER, @@ -1056,35 +1056,19 @@ carrier_changed (NMSupplicantInterface *iface, NMDeviceStateReason reason; NMActStageReturn ret; - if (nm_device_has_carrier (NM_DEVICE (self))) { - _LOGD (LOGD_DEVICE | LOGD_ETHER, "got carrier, initializing supplicant"); - nm_clear_g_signal_handler (self, &priv->carrier_id); - ret = supplicant_check_secrets_needed (self, &reason); - if (ret == NM_ACT_STAGE_RETURN_FAILURE) { - nm_device_state_changed (NM_DEVICE (self), - NM_DEVICE_STATE_FAILED, - reason); - } + if (!nm_device_has_carrier (NM_DEVICE (self))) + return; + + _LOGD (LOGD_DEVICE | LOGD_ETHER, "got carrier, initializing supplicant"); + nm_clear_g_signal_handler (self, &priv->carrier_id); + ret = supplicant_check_secrets_needed (self, &reason); + if (ret == NM_ACT_STAGE_RETURN_FAILURE) { + nm_device_state_changed (NM_DEVICE (self), + NM_DEVICE_STATE_FAILED, + reason); } } -static NMActStageReturn -nm_8021x_stage2_config (NMDeviceEthernet *self, NMDeviceStateReason *out_failure_reason) -{ - NMDeviceEthernetPrivate *priv = NM_DEVICE_ETHERNET_GET_PRIVATE (self); - - if (!nm_device_has_carrier (NM_DEVICE (self))) { - _LOGD (LOGD_DEVICE | LOGD_ETHER, "delay supplicant initialization until carrier goes up"); - priv->carrier_id = g_signal_connect (self, - "notify::" NM_DEVICE_CARRIER, - G_CALLBACK (carrier_changed), - self); - return NM_ACT_STAGE_RETURN_POSTPONE; - } - - return supplicant_check_secrets_needed (self, out_failure_reason); -} - /*****************************************************************************/ /* PPPoE */ @@ -1388,7 +1372,7 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) NMDeviceEthernetPrivate *priv = NM_DEVICE_ETHERNET_GET_PRIVATE (self); NMSettingConnection *s_con; const char *connection_type; - NMActStageReturn ret = NM_ACT_STAGE_RETURN_SUCCESS; + gboolean do_postpone = FALSE; NMSettingDcb *s_dcb; s_con = nm_device_get_applied_setting (device, NM_TYPE_SETTING_CONNECTION); @@ -1402,14 +1386,23 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) * process opens the port up for normal traffic. */ connection_type = nm_setting_connection_get_connection_type (s_con); - if (!strcmp (connection_type, NM_SETTING_WIRED_SETTING_NAME)) { + if (nm_streq (connection_type, NM_SETTING_WIRED_SETTING_NAME)) { NMSetting8021x *security; security = nm_device_get_applied_setting (device, NM_TYPE_SETTING_802_1X); if (security) { /* FIXME: for now 802.1x is mutually exclusive with DCB */ - return nm_8021x_stage2_config (self, out_failure_reason); + if (!nm_device_has_carrier (NM_DEVICE (self))) { + _LOGD (LOGD_DEVICE | LOGD_ETHER, "delay supplicant initialization until carrier goes up"); + priv->carrier_id = g_signal_connect (self, + "notify::" NM_DEVICE_CARRIER, + G_CALLBACK (carrier_changed), + self); + return NM_ACT_STAGE_RETURN_POSTPONE; + } + + return supplicant_check_secrets_needed (self, out_failure_reason); } } @@ -1431,7 +1424,7 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) } priv->dcb_handle_carrier_changes = TRUE; - ret = NM_ACT_STAGE_RETURN_POSTPONE; + do_postpone = TRUE; } /* PPPoE setup */ @@ -1441,11 +1434,13 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) s_ppp = nm_device_get_applied_setting (device, NM_TYPE_SETTING_PPP); if (s_ppp) { - guint32 mtu = 0, mru = 0, mxu; + guint32 mtu; + guint32 mru; + guint32 mxu; mtu = nm_setting_ppp_get_mtu (s_ppp); mru = nm_setting_ppp_get_mru (s_ppp); - mxu = mru > mtu ? mru : mtu; + mxu = MAX (mru, mtu); if (mxu) { _LOGD (LOGD_PPP, "set MTU to %u (PPP interface MRU %u, MTU %u)", mxu + PPPOE_ENCAP_OVERHEAD, mru, mtu); @@ -1456,7 +1451,9 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) } } - return ret; + return do_postpone + ? NM_ACT_STAGE_RETURN_POSTPONE + : NM_ACT_STAGE_RETURN_SUCCESS; } static NMActStageReturn diff --git a/src/devices/nm-device-macsec.c b/src/devices/nm-device-macsec.c index bf8c6c7898..08a5f8cd30 100644 --- a/src/devices/nm-device-macsec.c +++ b/src/devices/nm-device-macsec.c @@ -511,7 +511,7 @@ supplicant_iface_state_cb (NMSupplicantInterface *iface, supplicant_iface_state_is_completed (self, new_state); } -static NMActStageReturn +static gboolean handle_auth_or_fail (NMDeviceMacsec *self, NMActRequest *req, gboolean new_secrets) @@ -520,7 +520,7 @@ handle_auth_or_fail (NMDeviceMacsec *self, NMConnection *applied_connection; if (!nm_device_auth_retries_try_next (NM_DEVICE (self))) - return NM_ACT_STAGE_RETURN_FAILURE; + return FALSE; nm_device_state_changed (NM_DEVICE (self), NM_DEVICE_STATE_NEED_AUTH, NM_DEVICE_STATE_REASON_NONE); @@ -530,13 +530,13 @@ handle_auth_or_fail (NMDeviceMacsec *self, setting_name = nm_connection_need_secrets (applied_connection, NULL); if (!setting_name) { _LOGI (LOGD_DEVICE, "Cleared secrets, but setting didn't need any secrets."); - return NM_ACT_STAGE_RETURN_FAILURE; + return FALSE; } macsec_secrets_get_secrets (self, setting_name, NM_SECRET_AGENT_GET_SECRETS_FLAG_ALLOW_INTERACTION | (new_secrets ? NM_SECRET_AGENT_GET_SECRETS_FLAG_REQUEST_NEW : 0)); - return NM_ACT_STAGE_RETURN_POSTPONE; + return TRUE; } static gboolean @@ -571,7 +571,7 @@ supplicant_connection_timeout_cb (gpointer user_data) if (nm_settings_connection_get_timestamp (connection, ×tamp)) new_secrets = !timestamp; - if (handle_auth_or_fail (self, req, new_secrets) != NM_ACT_STAGE_RETURN_POSTPONE) { + if (!handle_auth_or_fail (self, req, new_secrets)) { nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_NO_SECRETS); return G_SOURCE_REMOVE; } @@ -646,7 +646,6 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) NMDeviceMacsec *self = NM_DEVICE_MACSEC (device); NMDeviceMacsecPrivate *priv = NM_DEVICE_MACSEC_GET_PRIVATE (self); NMConnection *connection; - NMActStageReturn ret = NM_ACT_STAGE_RETURN_FAILURE; NMDevice *parent; const char *setting_name; int ifindex; @@ -667,10 +666,12 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) "Activation: connection '%s' has security, but secrets are required.", nm_connection_get_id (connection)); - ret = handle_auth_or_fail (self, req, FALSE); - if (ret != NM_ACT_STAGE_RETURN_POSTPONE) + if (!handle_auth_or_fail (self, req, FALSE)) { NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_NO_SECRETS); - return ret; + return NM_ACT_STAGE_RETURN_FAILURE; + } + + return NM_ACT_STAGE_RETURN_POSTPONE; } _LOGI (LOGD_DEVICE | LOGD_ETHER, diff --git a/src/devices/nm-device-ppp.c b/src/devices/nm-device-ppp.c index 7af189962d..52784143a1 100644 --- a/src/devices/nm-device-ppp.c +++ b/src/devices/nm-device-ppp.c @@ -137,11 +137,9 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) GError *error = NULL; req = nm_device_get_act_request (device); - g_return_val_if_fail (req, NM_ACT_STAGE_RETURN_FAILURE); s_pppoe = nm_device_get_applied_setting (device, NM_TYPE_SETTING_PPPOE); - g_return_val_if_fail (s_pppoe, NM_ACT_STAGE_RETURN_FAILURE); g_clear_object (&priv->ip4_config); @@ -157,9 +155,12 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) } if ( !priv->ppp_manager - || !nm_ppp_manager_start (priv->ppp_manager, req, + || !nm_ppp_manager_start (priv->ppp_manager, + req, nm_setting_pppoe_get_username (s_pppoe), - 30, 0, &error)) { + 30, + 0, + &error)) { _LOGW (LOGD_DEVICE | LOGD_PPP, "PPPoE failed to start: %s", error->message); g_error_free (error); @@ -169,16 +170,18 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) return NM_ACT_STAGE_RETURN_FAILURE; } - g_signal_connect (priv->ppp_manager, NM_PPP_MANAGER_SIGNAL_STATE_CHANGED, + g_signal_connect (priv->ppp_manager, + NM_PPP_MANAGER_SIGNAL_STATE_CHANGED, G_CALLBACK (ppp_state_changed), self); - g_signal_connect (priv->ppp_manager, NM_PPP_MANAGER_SIGNAL_IFINDEX_SET, + g_signal_connect (priv->ppp_manager, + NM_PPP_MANAGER_SIGNAL_IFINDEX_SET, G_CALLBACK (ppp_ifindex_set), self); - g_signal_connect (priv->ppp_manager, NM_PPP_MANAGER_SIGNAL_IP4_CONFIG, + g_signal_connect (priv->ppp_manager, + NM_PPP_MANAGER_SIGNAL_IP4_CONFIG, G_CALLBACK (ppp_ip4_config), self); - return NM_ACT_STAGE_RETURN_POSTPONE; } diff --git a/src/devices/nm-device-wireguard.c b/src/devices/nm-device-wireguard.c index f91bf22af7..e4f7122d34 100644 --- a/src/devices/nm-device-wireguard.c +++ b/src/devices/nm-device-wireguard.c @@ -1522,28 +1522,26 @@ act_stage2_config (NMDevice *device, } ret = link_config (NM_DEVICE_WIREGUARD (device), - "configure", - (sys_iface_state == NM_DEVICE_SYS_IFACE_STATE_ASSUME) - ? LINK_CONFIG_MODE_ASSUME - : LINK_CONFIG_MODE_FULL, - &failure_reason); + "configure", + (sys_iface_state == NM_DEVICE_SYS_IFACE_STATE_ASSUME) + ? LINK_CONFIG_MODE_ASSUME + : LINK_CONFIG_MODE_FULL, + &failure_reason); if (sys_iface_state == NM_DEVICE_SYS_IFACE_STATE_ASSUME) { /* this never fails. */ - NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_NONE); return NM_ACT_STAGE_RETURN_SUCCESS; } - if (ret != NM_ACT_STAGE_RETURN_FAILURE) { - NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_NONE); - return ret; + if (ret == NM_ACT_STAGE_RETURN_FAILURE) { + nm_device_state_changed (device, + NM_DEVICE_STATE_FAILED, + failure_reason); + NM_SET_OUT (out_failure_reason, failure_reason); + return NM_ACT_STAGE_RETURN_FAILURE; } - nm_device_state_changed (device, - NM_DEVICE_STATE_FAILED, - failure_reason); - NM_SET_OUT (out_failure_reason, failure_reason); - return NM_ACT_STAGE_RETURN_FAILURE; + return ret; } static NMIPConfig * diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 86c3ad1ded..852aae173e 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -7067,10 +7067,11 @@ activate_stage2_device_config (NMDevice *self) if (!nm_device_sys_iface_state_is_external_or_assume (self)) { if (!nm_device_bring_up (self, FALSE, &no_firmware)) { - if (no_firmware) - nm_device_state_changed (self, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_FIRMWARE_MISSING); - else - nm_device_state_changed (self, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_CONFIG_FAILED); + nm_device_state_changed (self, + NM_DEVICE_STATE_FAILED, + no_firmware + ? NM_DEVICE_STATE_REASON_FIRMWARE_MISSING + : NM_DEVICE_STATE_REASON_CONFIG_FAILED); return; } } diff --git a/src/devices/wifi/nm-device-iwd.c b/src/devices/wifi/nm-device-iwd.c index a6cd817f72..3799d37974 100644 --- a/src/devices/wifi/nm-device-iwd.c +++ b/src/devices/wifi/nm-device-iwd.c @@ -567,7 +567,7 @@ is_connection_known_network (NMConnection *connection) static gboolean is_ap_known_network (NMWifiAP *ap) { - GDBusProxy *network_proxy; + gs_unref_object GDBusProxy *network_proxy = NULL; gs_unref_variant GVariant *known_network = NULL; network_proxy = nm_iwd_manager_get_dbus_interface (nm_iwd_manager_get (), @@ -577,10 +577,7 @@ is_ap_known_network (NMWifiAP *ap) return FALSE; known_network = g_dbus_proxy_get_cached_property (network_proxy, "KnownNetwork"); - g_object_unref (network_proxy); - - return known_network - && g_variant_is_of_type (known_network, G_VARIANT_TYPE_OBJECT_PATH); + return nm_g_variant_is_of_type (known_network, G_VARIANT_TYPE_OBJECT_PATH); } static gboolean @@ -1773,29 +1770,25 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) { NMDeviceIwd *self = NM_DEVICE_IWD (device); NMDeviceIwdPrivate *priv = NM_DEVICE_IWD_GET_PRIVATE (self); - NMActStageReturn ret = NM_ACT_STAGE_RETURN_FAILURE; NMActRequest *req; NMConnection *connection; NMSettingWireless *s_wireless; const char *mode; req = nm_device_get_act_request (device); - g_return_val_if_fail (req, NM_ACT_STAGE_RETURN_FAILURE); - connection = nm_act_request_get_applied_connection (req); - g_assert (connection); - s_wireless = nm_connection_get_setting_wireless (connection); g_return_val_if_fail (s_wireless, NM_ACT_STAGE_RETURN_FAILURE); mode = nm_setting_wireless_get_mode (s_wireless); + if (NM_IN_STRSET (mode, NULL, NM_SETTING_WIRELESS_MODE_INFRA)) { - GDBusProxy *network_proxy; + gs_unref_object GDBusProxy *network_proxy = NULL; NMWifiAP *ap = priv->current_ap; if (!ap) { NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_SUPPLICANT_FAILED); - goto out; + goto out_fail; } /* 802.1x networks that are not IWD Known Networks will definitely @@ -1809,7 +1802,7 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) nm_connection_get_id (connection)); NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_NO_SECRETS); - goto out; + goto out_fail; } network_proxy = nm_iwd_manager_get_dbus_interface (nm_iwd_manager_get (), @@ -1820,7 +1813,7 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) "Activation: (wifi) could not get Network interface proxy for %s", nm_ref_string_get_str (nm_wifi_ap_get_supplicant_path (ap))); NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_SUPPLICANT_FAILED); - goto out; + goto out_fail; } if (!priv->cancellable) @@ -1833,12 +1826,15 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) NULL, G_DBUS_CALL_FLAGS_NONE, G_MAXINT, priv->cancellable, network_connect_cb, self); - g_object_unref (network_proxy); - } else if (NM_IN_STRSET (mode, NM_SETTING_WIRELESS_MODE_AP, NM_SETTING_WIRELESS_MODE_ADHOC)) { + return NM_ACT_STAGE_RETURN_POSTPONE; + } + + if (NM_IN_STRSET (mode, NM_SETTING_WIRELESS_MODE_AP, NM_SETTING_WIRELESS_MODE_ADHOC)) { NMSettingWirelessSecurity *s_wireless_sec; s_wireless_sec = nm_connection_get_setting_wireless_security (connection); - if (s_wireless_sec && !nm_setting_wireless_security_get_psk (s_wireless_sec)) { + if ( s_wireless_sec + && !nm_setting_wireless_security_get_psk (s_wireless_sec)) { /* PSK is missing from the settings, have to request it */ wifi_secrets_cancel (self); @@ -1853,16 +1849,15 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) nm_device_state_changed (device, NM_DEVICE_STATE_NEED_AUTH, NM_DEVICE_STATE_REASON_NONE); } else act_set_mode (self); + + return NM_ACT_STAGE_RETURN_POSTPONE; } - /* We'll get stage3 started when the supplicant connects */ - ret = NM_ACT_STAGE_RETURN_POSTPONE; + return NM_ACT_STAGE_RETURN_POSTPONE; -out: - if (ret == NM_ACT_STAGE_RETURN_FAILURE) - cleanup_association_attempt (self, FALSE); - - return ret; +out_fail: + cleanup_association_attempt (self, FALSE); + return NM_ACT_STAGE_RETURN_FAILURE; } static guint32 diff --git a/src/devices/wifi/nm-device-olpc-mesh.c b/src/devices/wifi/nm-device-olpc-mesh.c index db3802038f..33c3b83007 100644 --- a/src/devices/wifi/nm-device-olpc-mesh.c +++ b/src/devices/wifi/nm-device-olpc-mesh.c @@ -189,10 +189,10 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) gboolean success; s_mesh = nm_device_get_applied_setting (device, NM_TYPE_SETTING_OLPC_MESH); - g_return_val_if_fail (s_mesh, NM_ACT_STAGE_RETURN_FAILURE); ssid = nm_setting_olpc_mesh_get_ssid (s_mesh); + nm_device_take_down (NM_DEVICE (self), TRUE); success = nm_platform_mesh_set_ssid (nm_device_get_platform (device), nm_device_get_ifindex (device), diff --git a/src/devices/wifi/nm-device-wifi-p2p.c b/src/devices/wifi/nm-device-wifi-p2p.c index 08c904dd4c..ae2a741d38 100644 --- a/src/devices/wifi/nm-device-wifi-p2p.c +++ b/src/devices/wifi/nm-device-wifi-p2p.c @@ -429,7 +429,6 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) connection = nm_device_get_applied_connection (device); g_return_val_if_fail (connection, NM_ACT_STAGE_RETURN_FAILURE); - nm_assert (NM_IS_SETTING_WIFI_P2P (nm_connection_get_setting (connection, NM_TYPE_SETTING_WIFI_P2P))); /* The prepare stage ensures that the peer has been found */ @@ -449,7 +448,8 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) /* TODO: Fix "pbc" being hardcoded here! */ nm_supplicant_interface_p2p_connect (priv->mgmt_iface, nm_wifi_p2p_peer_get_supplicant_path (peer), - "pbc", NULL); + "pbc", + NULL); /* Set up a timeout on the connect attempt */ if (priv->sup_timeout_id == 0) { diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 29936e9d33..6b6467119d 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -2780,8 +2780,7 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) { NMDeviceWifi *self = NM_DEVICE_WIFI (device); NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); - NMActStageReturn ret = NM_ACT_STAGE_RETURN_FAILURE; - NMSupplicantConfig *config = NULL; + gs_unref_object NMSupplicantConfig *config = NULL; NM80211Mode ap_mode; NMActRequest *req; NMWifiAP *ap; @@ -2801,15 +2800,14 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) ap = priv->current_ap; if (!ap) { NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_SUPPLICANT_FAILED); - goto out; + goto out_fail; } + ap_mode = nm_wifi_ap_get_mode (ap); connection = nm_act_request_get_applied_connection (req); - g_assert (connection); - s_wireless = nm_connection_get_setting_wireless (connection); - g_assert (s_wireless); + nm_assert (s_wireless); /* If we need secrets, get them */ setting_name = nm_connection_need_secrets (connection, NULL); @@ -2818,13 +2816,12 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) "Activation: (wifi) access point '%s' has security, but secrets are required.", nm_connection_get_id (connection)); - if (handle_auth_or_fail (self, req, FALSE)) - ret = NM_ACT_STAGE_RETURN_POSTPONE; - else { + if (!handle_auth_or_fail (self, req, FALSE)) { NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_NO_SECRETS); - ret = NM_ACT_STAGE_RETURN_FAILURE; + goto out_fail; } - goto out; + + return NM_ACT_STAGE_RETURN_POSTPONE; } if (!wake_on_wlan_enable (self)) @@ -2857,17 +2854,19 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) /* Build up the supplicant configuration */ config = build_supplicant_config (self, connection, nm_wifi_ap_get_freq (ap), &error); - if (config == NULL) { + if (!config) { _LOGE (LOGD_DEVICE | LOGD_WIFI, "Activation: (wifi) couldn't build wireless configuration: %s", error->message); g_clear_error (&error); NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_SUPPLICANT_CONFIG_FAILED); - goto out; + goto out_fail; } - nm_supplicant_interface_assoc (priv->sup_iface, config, - supplicant_iface_assoc_cb, self); + nm_supplicant_interface_assoc (priv->sup_iface, + config, + supplicant_iface_assoc_cb, + self); /* Set up a timeout on the association attempt */ timeout = nm_device_get_supplicant_timeout (NM_DEVICE (self)); @@ -2879,21 +2878,12 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) priv->periodic_source_id = g_timeout_add_seconds (6, periodic_update_cb, self); /* We'll get stage3 started when the supplicant connects */ - ret = NM_ACT_STAGE_RETURN_POSTPONE; + return NM_ACT_STAGE_RETURN_POSTPONE; -out: - if (ret == NM_ACT_STAGE_RETURN_FAILURE) { - cleanup_association_attempt (self, TRUE); - wake_on_wlan_restore (self); - } - - if (config) { - /* Supplicant interface object refs the config; we no longer care about - * it after this function. - */ - g_object_unref (config); - } - return ret; +out_fail: + cleanup_association_attempt (self, TRUE); + wake_on_wlan_restore (self); + return NM_ACT_STAGE_RETURN_FAILURE; } static NMActStageReturn diff --git a/src/devices/wwan/nm-device-modem.c b/src/devices/wwan/nm-device-modem.c index 3de5ae07dd..5b712625bb 100644 --- a/src/devices/wwan/nm-device-modem.c +++ b/src/devices/wwan/nm-device-modem.c @@ -605,7 +605,6 @@ static NMActStageReturn act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) { nm_modem_act_stage2_config (NM_DEVICE_MODEM_GET_PRIVATE (device)->modem); - return NM_ACT_STAGE_RETURN_SUCCESS; } From 5979972e20d167a2ebef69f09f5680efacc00d52 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 28 Feb 2020 12:03:07 +0100 Subject: [PATCH 4/8] device/wifi: don't postpone act_stage2_config() for iwd when nothing to wait --- src/devices/wifi/nm-device-iwd.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/devices/wifi/nm-device-iwd.c b/src/devices/wifi/nm-device-iwd.c index 3799d37974..49fe07fb08 100644 --- a/src/devices/wifi/nm-device-iwd.c +++ b/src/devices/wifi/nm-device-iwd.c @@ -1853,7 +1853,10 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) return NM_ACT_STAGE_RETURN_POSTPONE; } - return NM_ACT_STAGE_RETURN_POSTPONE; + _LOGW (LOGD_DEVICE | LOGD_WIFI, + "Activation: (wifi) iwd cannot handle mode %s", + mode); + NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_SUPPLICANT_FAILED); out_fail: cleanup_association_attempt (self, FALSE); From 3d78740398a8238ee4ba4f02ca14a8812d0d5493 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 28 Feb 2020 13:55:40 +0100 Subject: [PATCH 5/8] device: allow scheduling nm_device_activate_schedule_stage1_prepare() right away There was only API to schedule the stage on an idle handler. Sometimes, we are just in the right situation to schedule the stage right away. It should be possibly to avoid going through the extra hop. For now, none of the caller makes use of this. So, there isn't any actual change in behavior. But by adding this possibility, we may do use in the future. --- src/devices/bluetooth/nm-device-bt.c | 8 ++++---- src/devices/nm-device-ethernet.c | 4 ++-- src/devices/nm-device-macsec.c | 2 +- src/devices/nm-device-private.h | 3 ++- src/devices/nm-device-wireguard.c | 2 +- src/devices/nm-device.c | 26 +++++++++++--------------- src/devices/team/nm-device-team.c | 2 +- src/devices/wifi/nm-device-olpc-mesh.c | 2 +- src/devices/wifi/nm-device-wifi-p2p.c | 2 +- src/devices/wifi/nm-device-wifi.c | 4 ++-- src/devices/wwan/nm-device-modem.c | 4 ++-- 11 files changed, 28 insertions(+), 31 deletions(-) diff --git a/src/devices/bluetooth/nm-device-bt.c b/src/devices/bluetooth/nm-device-bt.c index a6a4bd68c9..ab18897f4e 100644 --- a/src/devices/bluetooth/nm-device-bt.c +++ b/src/devices/bluetooth/nm-device-bt.c @@ -484,7 +484,7 @@ modem_auth_result (NMModem *modem, GError *error, gpointer user_data) } priv->stage1_modem_prepare_state = NM_DEVICE_STAGE_STATE_INIT; - nm_device_activate_schedule_stage1_device_prepare (device); + nm_device_activate_schedule_stage1_device_prepare (device, FALSE); } static void @@ -519,7 +519,7 @@ modem_prepare_result (NMModem *modem, } priv->stage1_modem_prepare_state = NM_DEVICE_STAGE_STATE_COMPLETED; - nm_device_activate_schedule_stage1_device_prepare (NM_DEVICE (self)); + nm_device_activate_schedule_stage1_device_prepare (NM_DEVICE (self), FALSE); } static void @@ -719,7 +719,7 @@ mm_modem_added_cb (NMModemManager *manager, priv = NM_DEVICE_BT_GET_PRIVATE (self); if (priv->stage1_bt_state == NM_DEVICE_STAGE_STATE_COMPLETED) - nm_device_activate_schedule_stage1_device_prepare (NM_DEVICE (self)); + nm_device_activate_schedule_stage1_device_prepare (NM_DEVICE (self), FALSE); } /*****************************************************************************/ @@ -903,7 +903,7 @@ connect_bz_cb (NMBluezManager *bz_mgr, } priv->stage1_bt_state = NM_DEVICE_STAGE_STATE_COMPLETED; - nm_device_activate_schedule_stage1_device_prepare (NM_DEVICE (self)); + nm_device_activate_schedule_stage1_device_prepare (NM_DEVICE (self), FALSE); } static NMActStageReturn diff --git a/src/devices/nm-device-ethernet.c b/src/devices/nm-device-ethernet.c index d54350d24b..dc44c084bd 100644 --- a/src/devices/nm-device-ethernet.c +++ b/src/devices/nm-device-ethernet.c @@ -505,7 +505,7 @@ wired_secrets_cb (NMActRequest *req, } supplicant_interface_release (self); - nm_device_activate_schedule_stage1_device_prepare (device); + nm_device_activate_schedule_stage1_device_prepare (device, FALSE); } static void @@ -954,7 +954,7 @@ pppoe_reconnect_delay (gpointer user_data) priv->pppoe_wait_id = 0; priv->last_pppoe_time = 0; _LOGI (LOGD_DEVICE, "PPPoE reconnect delay complete, resuming connection..."); - nm_device_activate_schedule_stage1_device_prepare (NM_DEVICE (self)); + nm_device_activate_schedule_stage1_device_prepare (NM_DEVICE (self), FALSE); return G_SOURCE_REMOVE; } diff --git a/src/devices/nm-device-macsec.c b/src/devices/nm-device-macsec.c index 08a5f8cd30..4d620831ac 100644 --- a/src/devices/nm-device-macsec.c +++ b/src/devices/nm-device-macsec.c @@ -306,7 +306,7 @@ macsec_secrets_cb (NMActRequest *req, return; } - nm_device_activate_schedule_stage1_device_prepare (device); + nm_device_activate_schedule_stage1_device_prepare (device, FALSE); } static void diff --git a/src/devices/nm-device-private.h b/src/devices/nm-device-private.h index a8e577e645..dc8a33f98e 100644 --- a/src/devices/nm-device-private.h +++ b/src/devices/nm-device-private.h @@ -71,7 +71,8 @@ gboolean nm_device_hw_addr_reset (NMDevice *device, const char *detail); void nm_device_set_firmware_missing (NMDevice *self, gboolean missing); -void nm_device_activate_schedule_stage1_device_prepare (NMDevice *device); +void nm_device_activate_schedule_stage1_device_prepare (NMDevice *device, + gboolean do_sync); void nm_device_activate_schedule_stage2_device_config (NMDevice *device); void nm_device_activate_schedule_ip_config_result (NMDevice *device, diff --git a/src/devices/nm-device-wireguard.c b/src/devices/nm-device-wireguard.c index e4f7122d34..bb7a595060 100644 --- a/src/devices/nm-device-wireguard.c +++ b/src/devices/nm-device-wireguard.c @@ -1266,7 +1266,7 @@ _secrets_cb (NMActRequest *req, return; } - nm_device_activate_schedule_stage1_device_prepare (device); + nm_device_activate_schedule_stage1_device_prepare (device, FALSE); } static void diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 852aae173e..e712d5813b 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -6438,7 +6438,7 @@ master_ready_cb (NMActiveConnection *active, nm_assert (nm_active_connection_get_master_ready (active)); if (priv->state == NM_DEVICE_STATE_PREPARE) - nm_device_activate_schedule_stage1_device_prepare (self); + nm_device_activate_schedule_stage1_device_prepare (self, FALSE); } static void @@ -6586,7 +6586,7 @@ sriov_params_cb (GError *error, gpointer data) priv->stage1_sriov_state = NM_DEVICE_STAGE_STATE_COMPLETED; - nm_device_activate_schedule_stage1_device_prepare (self); + nm_device_activate_schedule_stage1_device_prepare (self, FALSE); } /* @@ -6729,23 +6729,19 @@ activate_stage1_device_prepare (NMDevice *self) activation_source_invoke_sync (self, activate_stage2_device_config, AF_INET); } -/* - * nm_device_activate_schedule_stage1_device_prepare - * - * Prepare a device for activation - * - */ void -nm_device_activate_schedule_stage1_device_prepare (NMDevice *self) +nm_device_activate_schedule_stage1_device_prepare (NMDevice *self, + gboolean do_sync) { - NMDevicePrivate *priv; - g_return_if_fail (NM_IS_DEVICE (self)); + g_return_if_fail (NM_DEVICE_GET_PRIVATE (self)->act_request.obj); - priv = NM_DEVICE_GET_PRIVATE (self); - g_return_if_fail (priv->act_request.obj); + if (!do_sync) { + activation_source_schedule (self, activate_stage1_device_prepare, AF_INET); + return; + } - activation_source_schedule (self, activate_stage1_device_prepare, AF_INET); + activation_source_invoke_sync (self, activate_stage1_device_prepare, AF_INET); } static NMActStageReturn @@ -12600,7 +12596,7 @@ _device_activate (NMDevice *self, NMActRequest *req) act_request_set (self, req); - nm_device_activate_schedule_stage1_device_prepare (self); + nm_device_activate_schedule_stage1_device_prepare (self, FALSE); } static void diff --git a/src/devices/team/nm-device-team.c b/src/devices/team/nm-device-team.c index a0749c21f6..3f7ee48c0e 100644 --- a/src/devices/team/nm-device-team.c +++ b/src/devices/team/nm-device-team.c @@ -416,7 +416,7 @@ teamd_dbus_appeared (GDBusConnection *connection, } priv->stage1_state = NM_DEVICE_STAGE_STATE_COMPLETED; - nm_device_activate_schedule_stage1_device_prepare (device); + nm_device_activate_schedule_stage1_device_prepare (device, FALSE); } static void diff --git a/src/devices/wifi/nm-device-olpc-mesh.c b/src/devices/wifi/nm-device-olpc-mesh.c index 33c3b83007..2850682b00 100644 --- a/src/devices/wifi/nm-device-olpc-mesh.c +++ b/src/devices/wifi/nm-device-olpc-mesh.c @@ -256,7 +256,7 @@ companion_notify_cb (NMDeviceWifi *companion, GParamSpec *pspec, gpointer user_d g_object_get (companion, NM_DEVICE_WIFI_SCANNING, &scanning, NULL); if (!scanning) { priv->stage1_waiting = FALSE; - nm_device_activate_schedule_stage1_device_prepare (NM_DEVICE (self)); + nm_device_activate_schedule_stage1_device_prepare (NM_DEVICE (self), FALSE); } } diff --git a/src/devices/wifi/nm-device-wifi-p2p.c b/src/devices/wifi/nm-device-wifi-p2p.c index ae2a741d38..a655c95025 100644 --- a/src/devices/wifi/nm-device-wifi-p2p.c +++ b/src/devices/wifi/nm-device-wifi-p2p.c @@ -523,7 +523,7 @@ peer_add_remove (NMDeviceWifiP2P *self, if (peer) { /* A peer for the connection was found, cancel the timeout and go to configure state. */ nm_clear_g_source (&priv->find_peer_timeout_id); - nm_device_activate_schedule_stage1_device_prepare (device); + nm_device_activate_schedule_stage1_device_prepare (device, FALSE); } } diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 6b6467119d..ce5cf1a466 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -1743,7 +1743,7 @@ wifi_secrets_cb (NMActRequest *req, return; } - nm_device_activate_schedule_stage1_device_prepare (device); + nm_device_activate_schedule_stage1_device_prepare (device, FALSE); } static void @@ -1810,7 +1810,7 @@ supplicant_iface_wps_credentials_cb (NMSupplicantInterface *iface, } wifi_secrets_cancel (self); - nm_device_activate_schedule_stage1_device_prepare (NM_DEVICE (self)); + nm_device_activate_schedule_stage1_device_prepare (NM_DEVICE (self), FALSE); } static gboolean diff --git a/src/devices/wwan/nm-device-modem.c b/src/devices/wwan/nm-device-modem.c index 5b712625bb..53e4636523 100644 --- a/src/devices/wwan/nm-device-modem.c +++ b/src/devices/wwan/nm-device-modem.c @@ -156,7 +156,7 @@ modem_prepare_result (NMModem *modem, } priv->stage1_state = NM_DEVICE_STAGE_STATE_COMPLETED; - nm_device_activate_schedule_stage1_device_prepare (device); + nm_device_activate_schedule_stage1_device_prepare (device, FALSE); } static void @@ -191,7 +191,7 @@ modem_auth_result (NMModem *modem, GError *error, gpointer user_data) } priv->stage1_state = NM_DEVICE_STAGE_STATE_INIT; - nm_device_activate_schedule_stage1_device_prepare (device); + nm_device_activate_schedule_stage1_device_prepare (device, FALSE); } static void From 99cb791813e416c62010a629939b36a4fc31548d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 28 Feb 2020 13:55:40 +0100 Subject: [PATCH 6/8] device: allow scheduling nm_device_activate_schedule_stage2_device_config() right away We usually want to schedule stage2 when we just completed with the previous stage (or, if we are currently in stage2, and want to re-enter it). In those cases, the conditions are often right to just proceed right away. No need to schedule the stage on an idle handler. Allow to invoke stage2 right away. --- src/devices/nm-device-private.h | 3 ++- src/devices/nm-device.c | 20 +++++++++----------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/devices/nm-device-private.h b/src/devices/nm-device-private.h index dc8a33f98e..8d539026e7 100644 --- a/src/devices/nm-device-private.h +++ b/src/devices/nm-device-private.h @@ -73,7 +73,8 @@ void nm_device_set_firmware_missing (NMDevice *self, gboolean missing); void nm_device_activate_schedule_stage1_device_prepare (NMDevice *device, gboolean do_sync); -void nm_device_activate_schedule_stage2_device_config (NMDevice *device); +void nm_device_activate_schedule_stage2_device_config (NMDevice *device, + gboolean do_sync); void nm_device_activate_schedule_ip_config_result (NMDevice *device, int addr_family, diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index e712d5813b..63cffc6235 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -691,8 +691,6 @@ static void (*const activate_stage4_ip_config_timeout_x[2]) (NMDevice *self) = { static void sriov_op_cb (GError *error, gpointer user_data); -static void activate_stage2_device_config (NMDevice *self); - static void activate_stage5_ip_config_result_4 (NMDevice *self); static void activate_stage5_ip_config_result_6 (NMDevice *self); @@ -6726,7 +6724,7 @@ activate_stage1_device_prepare (NMDevice *self) if (master) master_ready (self, active); - activation_source_invoke_sync (self, activate_stage2_device_config, AF_INET); + nm_device_activate_schedule_stage2_device_config (self, TRUE); } void @@ -7105,18 +7103,18 @@ activate_stage2_device_config (NMDevice *self) nm_device_activate_schedule_stage3_ip_config_start (self); } -/* - * nm_device_activate_schedule_stage2_device_config - * - * Schedule setup of the hardware device - * - */ void -nm_device_activate_schedule_stage2_device_config (NMDevice *self) +nm_device_activate_schedule_stage2_device_config (NMDevice *self, + gboolean do_sync) { g_return_if_fail (NM_IS_DEVICE (self)); - activation_source_schedule (self, activate_stage2_device_config, AF_INET); + if (!do_sync) { + activation_source_schedule (self, activate_stage2_device_config, AF_INET); + return; + } + + activation_source_invoke_sync (self, activate_stage2_device_config, AF_INET); } void From ea3912b70b9a19ab753b1ab7748695d4227e6397 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 28 Feb 2020 13:50:55 +0100 Subject: [PATCH 7/8] device: make device stage2 reentrant for NMDeviceAdsl Configuration stages like act_stage2_config() can postpone progressing to the next stage. Currently, when the condition that we wait for gets satisfied, the code schedules the next stage from there. I think that is wrong, because when we postpone from act_stage2_config(), follow up steps of stage2 get skipped. Thus, when we are ready to progress, the class should enter stage 2 again. This requires that stage2 becomes reentrant and that the code reenters the same stage. --- src/devices/adsl/nm-device-adsl.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/devices/adsl/nm-device-adsl.c b/src/devices/adsl/nm-device-adsl.c index fe4055eb66..8f7cba221a 100644 --- a/src/devices/adsl/nm-device-adsl.c +++ b/src/devices/adsl/nm-device-adsl.c @@ -303,7 +303,7 @@ nas_update_cb (gpointer user_data) return G_SOURCE_REMOVE; } - nm_device_activate_schedule_stage3_ip_config_start (device); + nm_device_activate_schedule_stage2_device_config (device, TRUE); return G_SOURCE_REMOVE; } @@ -362,6 +362,7 @@ static NMActStageReturn act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) { NMDeviceAdsl *self = NM_DEVICE_ADSL (device); + NMDeviceAdslPrivate *priv = NM_DEVICE_ADSL_GET_PRIVATE (self); NMSettingAdsl *s_adsl; const char *protocol; @@ -379,11 +380,16 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) if (nm_streq0 (protocol, NM_SETTING_ADSL_PROTOCOL_PPPOE)) { /* PPPoE needs RFC2684 bridging before we can do PPP over it */ - if (!br2684_create_iface (self)) { - NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_BR2684_FAILED); - return NM_ACT_STAGE_RETURN_FAILURE; + if (priv->nas_ifindex <= 0) { + if (priv->nas_update_id == 0) { + if (!br2684_create_iface (self)) { + NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_BR2684_FAILED); + return NM_ACT_STAGE_RETURN_FAILURE; + } + } + return NM_ACT_STAGE_RETURN_POSTPONE; } - return NM_ACT_STAGE_RETURN_POSTPONE; + return NM_ACT_STAGE_RETURN_SUCCESS; } _LOGW (LOGD_ADSL, "unhandled ADSL protocol '%s'", protocol); From 7af61e2aa00eacd77bf20bcc2766ee7cdac6137b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 28 Feb 2020 16:11:19 +0100 Subject: [PATCH 8/8] device: make device stage2 reentrant for NMDeviceBridge --- src/devices/nm-device-bridge.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/devices/nm-device-bridge.c b/src/devices/nm-device-bridge.c index 5eaf5a98c9..874d4966e3 100644 --- a/src/devices/nm-device-bridge.c +++ b/src/devices/nm-device-bridge.c @@ -532,7 +532,7 @@ _bt_register_bridge_cb (GError *error, return; } - nm_device_activate_schedule_stage3_ip_config_start (NM_DEVICE (self)); + nm_device_activate_schedule_stage2_device_config (NM_DEVICE (self), FALSE); } void @@ -578,6 +578,9 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) if (self->bt_cancellable) return NM_ACT_STAGE_RETURN_POSTPONE; + if (self->bt_registered) + return NM_ACT_STAGE_RETURN_POSTPONE; + self->bt_cancellable = g_cancellable_new (); if (!nm_bt_vtable_network_server->register_bridge (nm_bt_vtable_network_server, nm_setting_bluetooth_get_bdaddr (s_bt),