From 96cd0ca62f9100bab9952cf18aad0baae4477cee Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 21 Aug 2019 13:10:24 +0200 Subject: [PATCH 01/26] modem/trivial: rename virtual function NMModemClass.act_stage1_prepare() NMDeviceClass already has a function with this name. It's confusing to have multiple virtual functions named the same. Rename. --- src/devices/wwan/nm-modem-broadband.c | 8 ++++---- src/devices/wwan/nm-modem-ofono.c | 8 ++++---- src/devices/wwan/nm-modem.c | 10 +++++----- src/devices/wwan/nm-modem.h | 6 +++--- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/devices/wwan/nm-modem-broadband.c b/src/devices/wwan/nm-modem-broadband.c index 216fedfec6..80a3614e5f 100644 --- a/src/devices/wwan/nm-modem-broadband.c +++ b/src/devices/wwan/nm-modem-broadband.c @@ -586,9 +586,9 @@ connect_context_step (NMModemBroadband *self) } static NMActStageReturn -act_stage1_prepare (NMModem *_self, - NMConnection *connection, - NMDeviceStateReason *out_failure_reason) +modem_act_stage1_prepare (NMModem *_self, + NMConnection *connection, + NMDeviceStateReason *out_failure_reason) { NMModemBroadband *self = NM_MODEM_BROADBAND (_self); @@ -1512,7 +1512,7 @@ nm_modem_broadband_class_init (NMModemBroadbandClass *klass) modem_class->get_user_pass = get_user_pass; modem_class->check_connection_compatible_with_modem = check_connection_compatible_with_modem; modem_class->complete_connection = complete_connection; - modem_class->act_stage1_prepare = act_stage1_prepare; + modem_class->modem_act_stage1_prepare = modem_act_stage1_prepare; modem_class->owns_port = owns_port; obj_properties[PROP_MODEM] = diff --git a/src/devices/wwan/nm-modem-ofono.c b/src/devices/wwan/nm-modem-ofono.c index 31111b62b7..262e57fd84 100644 --- a/src/devices/wwan/nm-modem-ofono.c +++ b/src/devices/wwan/nm-modem-ofono.c @@ -1114,9 +1114,9 @@ create_connect_properties (NMConnection *connection) } static NMActStageReturn -act_stage1_prepare (NMModem *modem, - NMConnection *connection, - NMDeviceStateReason *out_failure_reason) +modem_act_stage1_prepare (NMModem *modem, + NMConnection *connection, + NMDeviceStateReason *out_failure_reason) { NMModemOfono *self = NM_MODEM_OFONO (modem); NMModemOfonoPrivate *priv = NM_MODEM_OFONO_GET_PRIVATE (self); @@ -1308,6 +1308,6 @@ nm_modem_ofono_class_init (NMModemOfonoClass *klass) modem_class->deactivate_cleanup = deactivate_cleanup; modem_class->check_connection_compatible_with_modem = check_connection_compatible_with_modem; - modem_class->act_stage1_prepare = act_stage1_prepare; + modem_class->modem_act_stage1_prepare = modem_act_stage1_prepare; modem_class->static_stage3_ip4_config_start = static_stage3_ip4_config_start; } diff --git a/src/devices/wwan/nm-modem.c b/src/devices/wwan/nm-modem.c index 617096a76a..3057ee8b82 100644 --- a/src/devices/wwan/nm-modem.c +++ b/src/devices/wwan/nm-modem.c @@ -974,9 +974,9 @@ nm_modem_get_secrets (NMModem *self, /*****************************************************************************/ static NMActStageReturn -act_stage1_prepare (NMModem *modem, - NMConnection *connection, - NMDeviceStateReason *out_failure_reason) +modem_act_stage1_prepare (NMModem *modem, + NMConnection *connection, + NMDeviceStateReason *out_failure_reason) { NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_UNKNOWN); return NM_ACT_STAGE_RETURN_FAILURE; @@ -1003,7 +1003,7 @@ nm_modem_act_stage1_prepare (NMModem *self, setting_name = nm_connection_need_secrets (connection, &hints); if (!setting_name) { nm_assert (!hints); - return NM_MODEM_GET_CLASS (self)->act_stage1_prepare (self, connection, out_failure_reason); + return NM_MODEM_GET_CLASS (self)->modem_act_stage1_prepare (self, connection, out_failure_reason); } /* Secrets required... */ @@ -1798,7 +1798,7 @@ nm_modem_class_init (NMModemClass *klass) object_class->dispose = dispose; object_class->finalize = finalize; - klass->act_stage1_prepare = act_stage1_prepare; + klass->modem_act_stage1_prepare = modem_act_stage1_prepare; klass->stage3_ip6_config_request = stage3_ip6_config_request; klass->deactivate_cleanup = deactivate_cleanup; diff --git a/src/devices/wwan/nm-modem.h b/src/devices/wwan/nm-modem.h index f5b386e8b2..3741c64667 100644 --- a/src/devices/wwan/nm-modem.h +++ b/src/devices/wwan/nm-modem.h @@ -136,9 +136,9 @@ typedef struct { NMConnection *const*existing_connections, GError **error); - NMActStageReturn (*act_stage1_prepare) (NMModem *modem, - NMConnection *connection, - NMDeviceStateReason *out_failure_reason); + NMActStageReturn (*modem_act_stage1_prepare) (NMModem *modem, + NMConnection *connection, + NMDeviceStateReason *out_failure_reason); NMActStageReturn (*static_stage3_ip4_config_start) (NMModem *self, NMActRequest *req, From 847f9cbef354958a473c41ed63fd7de4f8489d8e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 22 Aug 2019 08:39:00 +0200 Subject: [PATCH 02/26] device/modem: drop unnecessary cast for NM_DEVICE_MODEM_GET_PRIVATE() macro NM_DEVICE_MODEM_GET_PRIVATE() is based on _NM_GET_PRIVATE(), which has some smarts to check the pointer type, but is fine with well-known parent pointer types like "NMDevice *". --- src/devices/wwan/nm-device-modem.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/devices/wwan/nm-device-modem.c b/src/devices/wwan/nm-device-modem.c index 042a6ca4aa..fc38fec435 100644 --- a/src/devices/wwan/nm-device-modem.c +++ b/src/devices/wwan/nm-device-modem.c @@ -355,7 +355,7 @@ modem_state_cb (NMModem *modem, NMModemState new_state = new_state_i; NMModemState old_state = old_state_i; NMDevice *device = NM_DEVICE (user_data); - NMDeviceModemPrivate *priv = NM_DEVICE_MODEM_GET_PRIVATE ((NMDeviceModem *) device); + NMDeviceModemPrivate *priv = NM_DEVICE_MODEM_GET_PRIVATE (device); NMDeviceState dev_state = nm_device_get_state (device); if (new_state <= NM_MODEM_STATE_DISABLING && @@ -409,7 +409,7 @@ modem_removed_cb (NMModem *modem, gpointer user_data) static gboolean owns_iface (NMDevice *device, const char *iface) { - NMDeviceModemPrivate *priv = NM_DEVICE_MODEM_GET_PRIVATE ((NMDeviceModem *) device); + NMDeviceModemPrivate *priv = NM_DEVICE_MODEM_GET_PRIVATE (device); g_return_val_if_fail (priv->modem, FALSE); @@ -447,7 +447,7 @@ get_generic_capabilities (NMDevice *device) static const char * get_type_description (NMDevice *device) { - NMDeviceModemPrivate *priv = NM_DEVICE_MODEM_GET_PRIVATE ((NMDeviceModem *) device); + NMDeviceModemPrivate *priv = NM_DEVICE_MODEM_GET_PRIVATE (device); if (NM_FLAGS_HAS (priv->current_caps, NM_DEVICE_MODEM_CAPABILITY_GSM_UMTS)) return "gsm"; @@ -464,7 +464,7 @@ check_connection_compatible (NMDevice *device, NMConnection *connection, GError if (!NM_DEVICE_CLASS (nm_device_modem_parent_class)->check_connection_compatible (device, connection, error)) return FALSE; - if (!nm_modem_check_connection_compatible (NM_DEVICE_MODEM_GET_PRIVATE ((NMDeviceModem *) device)->modem, + if (!nm_modem_check_connection_compatible (NM_DEVICE_MODEM_GET_PRIVATE (device)->modem, connection, error ? &local : NULL)) { if (error) { @@ -530,7 +530,7 @@ complete_connection (NMDevice *device, NMConnection *const*existing_connections, GError **error) { - NMDeviceModemPrivate *priv = NM_DEVICE_MODEM_GET_PRIVATE ((NMDeviceModem *) device); + NMDeviceModemPrivate *priv = NM_DEVICE_MODEM_GET_PRIVATE (device); return nm_modem_complete_connection (priv->modem, nm_device_get_iface (device), @@ -542,7 +542,7 @@ complete_connection (NMDevice *device, static void deactivate (NMDevice *device) { - nm_modem_deactivate (NM_DEVICE_MODEM_GET_PRIVATE ((NMDeviceModem *) device)->modem, device); + nm_modem_deactivate (NM_DEVICE_MODEM_GET_PRIVATE (device)->modem, device); } /*****************************************************************************/ @@ -593,7 +593,7 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) req = nm_device_get_act_request (device); g_return_val_if_fail (req, NM_ACT_STAGE_RETURN_FAILURE); - return nm_modem_act_stage1_prepare (NM_DEVICE_MODEM_GET_PRIVATE ((NMDeviceModem *) device)->modem, req, out_failure_reason); + return nm_modem_act_stage1_prepare (NM_DEVICE_MODEM_GET_PRIVATE (device)->modem, req, out_failure_reason); } static NMActStageReturn @@ -604,7 +604,7 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) req = nm_device_get_act_request (device); g_return_val_if_fail (req, NM_ACT_STAGE_RETURN_FAILURE); - return nm_modem_act_stage2_config (NM_DEVICE_MODEM_GET_PRIVATE ((NMDeviceModem *) device)->modem, req, out_failure_reason); + return nm_modem_act_stage2_config (NM_DEVICE_MODEM_GET_PRIVATE (device)->modem, req, out_failure_reason); } static NMActStageReturn @@ -632,7 +632,7 @@ act_stage3_ip_config_start (NMDevice *device, static void ip4_config_pre_commit (NMDevice *device, NMIP4Config *config) { - nm_modem_ip4_pre_commit (NM_DEVICE_MODEM_GET_PRIVATE ((NMDeviceModem *) device)->modem, device, config); + nm_modem_ip4_pre_commit (NM_DEVICE_MODEM_GET_PRIVATE (device)->modem, device, config); } static gboolean @@ -654,7 +654,7 @@ get_ip_iface_identifier (NMDevice *device, NMUtilsIPv6IfaceId *out_iid) static gboolean get_enabled (NMDevice *device) { - NMDeviceModemPrivate *priv = NM_DEVICE_MODEM_GET_PRIVATE ((NMDeviceModem *) device); + NMDeviceModemPrivate *priv = NM_DEVICE_MODEM_GET_PRIVATE (device); NMModemState modem_state = nm_modem_get_state (priv->modem); return priv->rf_enabled && (modem_state >= NM_MODEM_STATE_LOCKED); @@ -745,7 +745,7 @@ static void get_property (GObject *object, guint prop_id, GValue *value, GParamSpec *pspec) { - NMDeviceModemPrivate *priv = NM_DEVICE_MODEM_GET_PRIVATE ((NMDeviceModem *) object); + NMDeviceModemPrivate *priv = NM_DEVICE_MODEM_GET_PRIVATE (object); switch (prop_id) { case PROP_MODEM: @@ -776,7 +776,7 @@ static void set_property (GObject *object, guint prop_id, const GValue *value, GParamSpec *pspec) { - NMDeviceModemPrivate *priv = NM_DEVICE_MODEM_GET_PRIVATE ((NMDeviceModem *) object); + NMDeviceModemPrivate *priv = NM_DEVICE_MODEM_GET_PRIVATE (object); switch (prop_id) { case PROP_MODEM: @@ -834,7 +834,7 @@ nm_device_modem_new (NMModem *modem) static void dispose (GObject *object) { - NMDeviceModemPrivate *priv = NM_DEVICE_MODEM_GET_PRIVATE ((NMDeviceModem *) object); + NMDeviceModemPrivate *priv = NM_DEVICE_MODEM_GET_PRIVATE (object); if (priv->modem) { g_signal_handlers_disconnect_by_data (priv->modem, NM_DEVICE_MODEM (object)); From 0d0d4eaf935877e47b7359468acdac5a38b925da Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 22 Aug 2019 08:39:00 +0200 Subject: [PATCH 03/26] device/team: drop unnecessary cast for NM_DEVICE_TEAM_GET_PRIVATE() macro --- src/devices/team/nm-device-team.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/devices/team/nm-device-team.c b/src/devices/team/nm-device-team.c index 79ac8935bb..10982cfa4f 100644 --- a/src/devices/team/nm-device-team.c +++ b/src/devices/team/nm-device-team.c @@ -70,7 +70,7 @@ struct _NMDeviceTeamClass { G_DEFINE_TYPE (NMDeviceTeam, nm_device_team, NM_TYPE_DEVICE) -#define NM_DEVICE_TEAM_GET_PRIVATE(self) _NM_GET_PRIVATE (self, NMDeviceTeam, NM_IS_DEVICE_TEAM) +#define NM_DEVICE_TEAM_GET_PRIVATE(self) _NM_GET_PRIVATE (self, NMDeviceTeam, NM_IS_DEVICE_TEAM, NMDevice) /*****************************************************************************/ @@ -170,7 +170,7 @@ teamd_read_config (NMDevice *device) static gboolean teamd_read_timeout_cb (gpointer user_data) { - NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE ((NMDeviceTeam *) user_data); + NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (user_data); teamd_read_config ((NMDevice *) user_data); priv->teamd_read_timeout = 0; @@ -297,7 +297,7 @@ teamd_kill_cb (pid_t pid, gboolean success, int child_status, void *user_data) static void teamd_cleanup (NMDevice *device, gboolean free_tdc) { - NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE ((NMDeviceTeam *) device); + NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (device); nm_clear_g_source (&priv->teamd_process_watch); nm_clear_g_source (&priv->teamd_timeout); @@ -871,7 +871,7 @@ static void constructed (GObject *object) { NMDevice *device = NM_DEVICE (object); - NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE ((NMDeviceTeam *) device); + NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (device); char *tmp_str = NULL; G_OBJECT_CLASS (nm_device_team_parent_class)->constructed (object); @@ -904,7 +904,7 @@ static void dispose (GObject *object) { NMDevice *device = NM_DEVICE (object); - NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE ((NMDeviceTeam *) device); + NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (device); if (priv->teamd_dbus_watch) { g_bus_unwatch_name (priv->teamd_dbus_watch); From aef9594fa64803ddcd6a356e952437ad3cc979d4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 21 Aug 2019 16:57:55 +0200 Subject: [PATCH 04/26] device/bond: cleanup act-stage return values in NMDeviceBond's act_stage1_prepare() --- src/devices/nm-device-bond.c | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/src/devices/nm-device-bond.c b/src/devices/nm-device-bond.c index 7626b04a00..e67941d8f3 100644 --- a/src/devices/nm-device-bond.c +++ b/src/devices/nm-device-bond.c @@ -213,10 +213,10 @@ set_simple_option (NMDevice *device, set_bond_attr (device, mode, opt, value); } -static NMActStageReturn -apply_bonding_config (NMDevice *device) +static gboolean +apply_bonding_config (NMDeviceBond *self) { - NMDeviceBond *self = NM_DEVICE_BOND (device); + NMDevice *device = NM_DEVICE (self); NMSettingBond *s_bond; int ifindex = nm_device_get_ifindex (device); const char *mode_str, *value; @@ -239,7 +239,7 @@ apply_bonding_config (NMDevice *device) s_bond = nm_device_get_applied_setting (device, NM_TYPE_SETTING_BOND); - g_return_val_if_fail (s_bond, NM_ACT_STAGE_RETURN_FAILURE); + g_return_val_if_fail (s_bond, FALSE); mode_str = nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_MODE); if (!mode_str) @@ -248,7 +248,7 @@ apply_bonding_config (NMDevice *device) mode = _nm_setting_bond_mode_from_string (mode_str); if (mode == NM_BOND_MODE_UNKNOWN) { _LOGW (LOGD_BOND, "unknown bond mode '%s'", mode_str); - return NM_ACT_STAGE_RETURN_FAILURE; + return FALSE; } /* Set mode first, as some other options (e.g. arp_interval) are valid @@ -334,24 +334,30 @@ apply_bonding_config (NMDevice *device) else set_simple_option (device, mode, s_bond, NM_SETTING_BOND_OPTION_NUM_UNSOL_NA); - return NM_ACT_STAGE_RETURN_SUCCESS; + return TRUE; } static NMActStageReturn -act_stage1_prepare (NMDevice *dev, NMDeviceStateReason *out_failure_reason) +act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) { + NMDeviceBond *self = NM_DEVICE_BOND (device); NMActStageReturn ret = NM_ACT_STAGE_RETURN_SUCCESS; - ret = NM_DEVICE_CLASS (nm_device_bond_parent_class)->act_stage1_prepare (dev, out_failure_reason); + ret = NM_DEVICE_CLASS (nm_device_bond_parent_class)->act_stage1_prepare (device, out_failure_reason); if (ret != NM_ACT_STAGE_RETURN_SUCCESS) return ret; /* Interface must be down to set bond options */ - nm_device_take_down (dev, TRUE); - ret = apply_bonding_config (dev); - if (ret != NM_ACT_STAGE_RETURN_FAILURE) - ret = nm_device_hw_addr_set_cloned (dev, nm_device_get_applied_connection (dev), FALSE); - nm_device_bring_up (dev, TRUE, NULL); + nm_device_take_down (device, TRUE); + if (!apply_bonding_config (self)) + ret = NM_ACT_STAGE_RETURN_FAILURE; + else { + if (!nm_device_hw_addr_set_cloned (device, + nm_device_get_applied_connection (device), + FALSE)) + ret = NM_ACT_STAGE_RETURN_FAILURE; + } + nm_device_bring_up (device, TRUE, NULL); return ret; } From f0775963c2609a837408ba939106ad9e185f85e6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 21 Aug 2019 17:05:20 +0200 Subject: [PATCH 05/26] device/bridge: minor cleanup in NMDeviceBridge's act_stage1_prepare() Only reset "vlan_configured" when deactivating. stage1() should be re-entrant. --- src/devices/nm-device-bridge.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/devices/nm-device-bridge.c b/src/devices/nm-device-bridge.c index 8f3a4045ba..86eaa6e88c 100644 --- a/src/devices/nm-device-bridge.c +++ b/src/devices/nm-device-bridge.c @@ -225,7 +225,7 @@ commit_option (NMDevice *device, NMSetting *setting, const Option *option, gbool GParamSpec *pspec; GValue val = G_VALUE_INIT; guint32 uval = 0; - gs_free char *value = NULL; + char value[100]; g_assert (setting); @@ -258,10 +258,10 @@ commit_option (NMDevice *device, NMSetting *setting, const Option *option, gbool if (option->user_hz_compensate) uval *= 100; } else - g_assert_not_reached (); + nm_assert_not_reached (); g_value_unset (&val); - value = g_strdup_printf ("%u", uval); + nm_sprintf_buf (value, "%u", uval); if (slave) nm_platform_sysctl_slave_set_option (nm_device_get_platform (device), ifindex, option->sysname, value); else @@ -502,14 +502,13 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) NMSetting *s_bridge; const Option *option; - NM_DEVICE_BRIDGE (device)->vlan_configured = FALSE; - ret = NM_DEVICE_CLASS (nm_device_bridge_parent_class)->act_stage1_prepare (device, out_failure_reason); if (ret != NM_ACT_STAGE_RETURN_SUCCESS) return ret; connection = nm_device_get_applied_connection (device); g_return_val_if_fail (connection, NM_ACT_STAGE_RETURN_FAILURE); + s_bridge = (NMSetting *) nm_connection_get_setting_bridge (connection); g_return_val_if_fail (s_bridge, NM_ACT_STAGE_RETURN_FAILURE); @@ -555,6 +554,10 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) static void deactivate (NMDevice *device) { + NMDeviceBridge *self = NM_DEVICE_BRIDGE (device); + + self->vlan_configured = FALSE; + if (nm_bt_vtable_network_server) { /* always call unregister. It does nothing if the device * isn't registered as a hotspot bridge. */ From 2d42c1b102f9bbc797c27fefbd684ae4df622c4f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 21 Aug 2019 17:14:47 +0200 Subject: [PATCH 06/26] device/ethernet: make NMDeviceEthernet.act_stage1_prepare() reentrant and minor cleanups --- src/devices/nm-device-ethernet.c | 43 +++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/src/devices/nm-device-ethernet.c b/src/devices/nm-device-ethernet.c index 3e84847ed1..b51a7dcd69 100644 --- a/src/devices/nm-device-ethernet.c +++ b/src/devices/nm-device-ethernet.c @@ -794,14 +794,19 @@ link_negotiation_set (NMDevice *device) autoneg = nm_setting_wired_get_auto_negotiate (s_wired); speed = nm_setting_wired_get_speed (s_wired); duplex = link_duplex_to_platform (nm_setting_wired_get_duplex (s_wired)); - if (!autoneg && !speed && !duplex) { + if ( !autoneg + && !speed + && !duplex) { _LOGD (LOGD_DEVICE, "set-link: ignore link negotiation"); return; } } - if (!nm_platform_ethtool_get_link_settings (nm_device_get_platform (device), nm_device_get_ifindex (device), - &link_autoneg, &link_speed, &link_duplex)) { + if (!nm_platform_ethtool_get_link_settings (nm_device_get_platform (device), + nm_device_get_ifindex (device), + &link_autoneg, + &link_speed, + &link_duplex)) { _LOGW (LOGD_DEVICE, "set-link: unable to retrieve link negotiation"); return; } @@ -814,16 +819,18 @@ link_negotiation_set (NMDevice *device) return; } - if (autoneg && !speed && !duplex) + if ( autoneg + && !speed + && !duplex) _LOGD (LOGD_DEVICE, "set-link: configure auto-negotiation"); else { _LOGD (LOGD_DEVICE, "set-link: configure %snegotiation (%u Mbit%s - %s duplex%s)", autoneg ? "auto-" : "static ", speed ?: link_speed, speed ? "" : "*", - duplex - ? nm_platform_link_duplex_type_to_string (duplex) - : nm_platform_link_duplex_type_to_string (link_duplex), + duplex + ? nm_platform_link_duplex_type_to_string (duplex) + : nm_platform_link_duplex_type_to_string (link_duplex), duplex ? "" : "*"); } @@ -846,7 +853,7 @@ pppoe_reconnect_delay (gpointer user_data) priv->pppoe_wait_id = 0; _LOGI (LOGD_DEVICE, "PPPoE reconnect delay complete, resuming connection..."); nm_device_activate_schedule_stage2_device_config (NM_DEVICE (self)); - return FALSE; + return G_SOURCE_REMOVE; } static NMActStageReturn @@ -869,20 +876,26 @@ act_stage1_prepare (NMDevice *dev, NMDeviceStateReason *out_failure_reason) * a previous PPPoE connection was torn down, wait a bit to allow the * remote side to handle the disconnection. Otherwise the peer may * get confused and fail to negotiate the new connection. (rh #1023503) + * + * FIXME(shutdown): when exiting, we also need to wait before quiting, + * at least for additional NM_SHUTDOWN_TIMEOUT_MS seconds because + * otherwise after restart the device won't work for the first seconds. */ - if (priv->last_pppoe_time) { + if (priv->last_pppoe_time != 0) { gint32 delay = nm_utils_get_monotonic_timestamp_s () - priv->last_pppoe_time; if ( delay < PPPOE_RECONNECT_DELAY && nm_device_get_applied_setting (dev, NM_TYPE_SETTING_PPPOE)) { - _LOGI (LOGD_DEVICE, "delaying PPPoE reconnect for %d seconds to ensure peer is ready...", - delay); - g_assert (!priv->pppoe_wait_id); - priv->pppoe_wait_id = g_timeout_add_seconds (delay, - pppoe_reconnect_delay, - self); + if (priv->pppoe_wait_id == 0) { + _LOGI (LOGD_DEVICE, "delaying PPPoE reconnect for %d seconds to ensure peer is ready...", + delay); + priv->pppoe_wait_id = g_timeout_add_seconds (delay, + pppoe_reconnect_delay, + self); + } return NM_ACT_STAGE_RETURN_POSTPONE; } + nm_clear_g_source (&priv->pppoe_wait_id); priv->last_pppoe_time = 0; } From cc4d69c1c3629ba3322cae7d4f03cc136d063c8a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 22 Aug 2019 07:41:09 +0200 Subject: [PATCH 07/26] device/wireguard: drop act_stage1_prepare() implementation act_stage1_prepare() should become re-entrant. That means, we should not clear the state there. Instead, we clear it where necessary or on deactivate (which we do already). --- src/devices/nm-device-wireguard.c | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/devices/nm-device-wireguard.c b/src/devices/nm-device-wireguard.c index e3b8e8b4c4..572389b114 100644 --- a/src/devices/nm-device-wireguard.c +++ b/src/devices/nm-device-wireguard.c @@ -1517,17 +1517,6 @@ link_config_delayed_resolver_cb (gpointer user_data) return G_SOURCE_REMOVE; } -static NMActStageReturn -act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) -{ - NMDeviceWireGuardPrivate *priv = NM_DEVICE_WIREGUARD_GET_PRIVATE (device); - - priv->auto_default_route_initialized = FALSE; - priv->auto_default_route_priority_initialized = FALSE; - - return NM_DEVICE_CLASS (nm_device_wireguard_parent_class)->act_stage1_prepare (device, out_failure_reason); -} - static NMActStageReturn act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) @@ -1968,7 +1957,6 @@ nm_device_wireguard_class_init (NMDeviceWireGuardClass *klass) device_class->connection_type_check_compatible = NM_SETTING_WIREGUARD_SETTING_NAME; device_class->link_types = NM_DEVICE_DEFINE_LINK_TYPES (NM_LINK_TYPE_WIREGUARD); - device_class->act_stage1_prepare = act_stage1_prepare; device_class->state_changed = device_state_changed; device_class->create_and_realize = create_and_realize; device_class->act_stage2_config = act_stage2_config; From df086f536616a7e6dfd718ad6278738445ccb9a8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 22 Aug 2019 07:57:38 +0200 Subject: [PATCH 08/26] device/wpan: cleanup act_stage1_prepare() and don't assert with missing hwaddr --- src/devices/nm-device-wpan.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/devices/nm-device-wpan.c b/src/devices/nm-device-wpan.c index 882344121d..89301f3757 100644 --- a/src/devices/nm-device-wpan.c +++ b/src/devices/nm-device-wpan.c @@ -136,7 +136,7 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) return ret; platform = nm_device_get_platform (device); - g_return_val_if_fail (platform, NM_ACT_STAGE_RETURN_FAILURE); + nm_assert (NM_IS_PLATFORM (platform)); ifindex = nm_device_get_ifindex (device); @@ -147,7 +147,11 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) g_return_val_if_fail (s_wpan, NM_ACT_STAGE_RETURN_FAILURE); hwaddr = nm_platform_link_get_address (platform, ifindex, &hwaddr_len); - g_return_val_if_fail (hwaddr, NM_ACT_STAGE_RETURN_FAILURE); + + if (!hwaddr) { + *out_failure_reason = NM_DEVICE_STATE_REASON_CONFIG_FAILED; + return NM_ACT_STAGE_RETURN_FAILURE; + } /* As of kernel 4.16, the 6LoWPAN devices layered on top of WPANs * need to be DOWN as well as the WPAN device itself in order to @@ -156,7 +160,8 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) NM_LINK_TYPE_6LOWPAN, hwaddr, hwaddr_len); - if (lowpan_plink && NM_FLAGS_HAS (lowpan_plink->n_ifi_flags, IFF_UP)) { + if ( lowpan_plink + && NM_FLAGS_HAS (lowpan_plink->n_ifi_flags, IFF_UP)) { lowpan_device = nm_manager_get_device_by_ifindex (nm_manager_get (), lowpan_plink->ifindex); } @@ -192,6 +197,7 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) } ret = NM_ACT_STAGE_RETURN_SUCCESS; + out: nm_device_bring_up (device, TRUE, NULL); From 1f7e0f1d1f642db7a45a90cadbc3f2a2bda47fa2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 22 Aug 2019 08:18:09 +0200 Subject: [PATCH 09/26] device/wifi-p2p: make act_stage1_prepare() re-entrant Don't clear and reschedule finding of p2p peer if called multiple times during (the same) activation. --- src/devices/wifi/nm-device-wifi-p2p.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/src/devices/wifi/nm-device-wifi-p2p.c b/src/devices/wifi/nm-device-wifi-p2p.c index c5826e24ae..4bfd3094fc 100644 --- a/src/devices/wifi/nm-device-wifi-p2p.c +++ b/src/devices/wifi/nm-device-wifi-p2p.c @@ -369,13 +369,10 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) NMDeviceWifiP2P *self = NM_DEVICE_WIFI_P2P (device); NMDeviceWifiP2PPrivate *priv = NM_DEVICE_WIFI_P2P_GET_PRIVATE (self); NMActStageReturn ret; - NMActRequest *req; NMConnection *connection; NMSettingWifiP2P *s_wifi_p2p; NMWifiP2PPeer *peer; - nm_clear_g_source (&priv->sup_timeout_id); - ret = NM_DEVICE_CLASS (nm_device_wifi_p2p_parent_class)->act_stage1_prepare (device, out_failure_reason); if (ret != NM_ACT_STAGE_RETURN_SUCCESS) return ret; @@ -385,10 +382,7 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) return NM_ACT_STAGE_RETURN_FAILURE; } - req = nm_device_get_act_request (NM_DEVICE (self)); - g_return_val_if_fail (req, NM_ACT_STAGE_RETURN_FAILURE); - - connection = nm_act_request_get_applied_connection (req); + connection = nm_device_get_applied_connection (NM_DEVICE (self)); g_return_val_if_fail (connection, NM_ACT_STAGE_RETURN_FAILURE); s_wifi_p2p = NM_SETTING_WIFI_P2P (nm_connection_get_setting (connection, NM_TYPE_SETTING_WIFI_P2P)); @@ -397,12 +391,13 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) peer = nm_wifi_p2p_peers_find_first_compatible (&priv->peers_lst_head, connection); if (!peer) { /* Set up a timeout on the find attempt and run a find for the same period of time */ - priv->sup_timeout_id = g_timeout_add_seconds (10, - supplicant_find_timeout_cb, - self); - - nm_supplicant_interface_p2p_start_find (priv->mgmt_iface, 10); + if (priv->sup_timeout_id == 0) { + priv->sup_timeout_id = g_timeout_add_seconds (10, + supplicant_find_timeout_cb, + self); + nm_supplicant_interface_p2p_start_find (priv->mgmt_iface, 10); + } return NM_ACT_STAGE_RETURN_POSTPONE; } From 81816ebffab800ed7207bf51049c8b9c932c1b44 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 22 Aug 2019 08:25:39 +0200 Subject: [PATCH 10/26] device/wifi: various cleanup in act_stage1_prepare() The only change in behavior is in act_stage1_prepare(). That changes compared to before that we also set the specific object path if it was already set (and we looked up the AP by specific object to start with). Also, for existing APs that we found with nm_wifi_aps_find_first_compatible(), it changes the order of calling set_current_ap() before nm_active_connection_set_specific_object(). That should not make a different though. I anyway wonder why we even bother to set the specific object on the AC. Maybe that should be revisited. --- src/devices/wifi/nm-device-wifi.c | 75 +++++++++++++++---------------- 1 file changed, 37 insertions(+), 38 deletions(-) diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 4fa3f18a45..a587348c89 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -411,7 +411,9 @@ set_current_ap (NMDeviceWifi *self, NMWifiAP *new_ap, gboolean recheck_available NM80211Mode mode = nm_wifi_ap_get_mode (old_ap); /* Remove any AP from the internal list if it was created by NM or isn't known to the supplicant */ - if (mode == NM_802_11_MODE_ADHOC || mode == NM_802_11_MODE_AP || nm_wifi_ap_get_fake (old_ap)) + if ( NM_IN_SET (mode, NM_802_11_MODE_ADHOC, + NM_802_11_MODE_AP) + || nm_wifi_ap_get_fake (old_ap)) ap_add_remove (self, FALSE, old_ap, recheck_available_connections); g_object_unref (old_ap); } @@ -1229,7 +1231,8 @@ scanning_prohibited (NMDeviceWifi *self, gboolean periodic) /* Don't scan when a an AP or Ad-Hoc connection is active as it will * disrupt connected clients or peers. */ - if (priv->mode == NM_802_11_MODE_ADHOC || priv->mode == NM_802_11_MODE_AP) + if (NM_IN_SET (priv->mode, NM_802_11_MODE_ADHOC, + NM_802_11_MODE_AP)) return TRUE; switch (nm_device_get_state (NM_DEVICE (self))) { @@ -2409,9 +2412,9 @@ supplicant_connection_timeout_cb (gpointer user_data) connection = nm_act_request_get_applied_connection (req); g_assert (connection); - if ( priv->mode == NM_802_11_MODE_ADHOC - || priv->mode == NM_802_11_MODE_MESH - || priv->mode == NM_802_11_MODE_AP) { + if (NM_IN_SET (priv->mode, NM_802_11_MODE_ADHOC, + NM_802_11_MODE_MESH, + NM_802_11_MODE_AP)) { /* In Ad-Hoc and AP modes there's nothing to check the encryption key * (if any), so supplicant timeouts here are almost certainly the wifi * driver being really stupid. @@ -2619,6 +2622,7 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); NMActStageReturn ret; NMWifiAP *ap = NULL; + gs_unref_object NMWifiAP *ap_fake = NULL; NMActRequest *req; NMConnection *connection; NMSettingWireless *s_wireless; @@ -2662,44 +2666,39 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) return NM_ACT_STAGE_RETURN_FAILURE; /* AP and Mesh modes never use a specific object or existing scanned AP */ - if (priv->mode != NM_802_11_MODE_AP && priv->mode != NM_802_11_MODE_MESH) { + if (!NM_IN_SET (priv->mode, NM_802_11_MODE_AP, + NM_802_11_MODE_MESH)) { ap_path = nm_active_connection_get_specific_object (NM_ACTIVE_CONNECTION (req)); - ap = ap_path ? nm_wifi_ap_lookup_for_device (NM_DEVICE (self), ap_path) : NULL; - if (ap) - goto done; - + ap = ap_path + ? nm_wifi_ap_lookup_for_device (NM_DEVICE (self), ap_path) + : NULL; + } + if (!ap) ap = nm_wifi_aps_find_first_compatible (&priv->aps_lst_head, connection); + + if (!ap) { + /* If the user is trying to connect to an AP that NM doesn't yet know about + * (hidden network or something), starting a Hotspot or joining a Mesh, + * create a fake APfrom the security settings in the connection. This "fake" + * AP gets used until the real one is found in the scan list (Ad-Hoc or Hidden), + * or until the device is deactivated (Hotspot). + */ + ap_fake = nm_wifi_ap_new_fake_from_connection (connection); + if (!ap_fake) + g_return_val_if_reached (NM_ACT_STAGE_RETURN_FAILURE); + + if (nm_wifi_ap_is_hotspot (ap_fake)) + nm_wifi_ap_set_address (ap_fake, nm_device_get_hw_address (device)); + + g_object_freeze_notify (G_OBJECT (self)); + ap_add_remove (self, TRUE, ap_fake, TRUE); + g_object_thaw_notify (G_OBJECT (self)); + ap = ap_fake; } - if (ap) { - nm_active_connection_set_specific_object (NM_ACTIVE_CONNECTION (req), - nm_dbus_object_get_path (NM_DBUS_OBJECT (ap))); - goto done; - } - - /* If the user is trying to connect to an AP that NM doesn't yet know about - * (hidden network or something), starting a Hotspot or joining a Mesh, - * create a fake APfrom the security settings in the connection. This "fake" - * AP gets used until the real one is found in the scan list (Ad-Hoc or Hidden), - * or until the device is deactivated (Hotspot). - */ - ap = nm_wifi_ap_new_fake_from_connection (connection); - g_return_val_if_fail (ap != NULL, NM_ACT_STAGE_RETURN_FAILURE); - - if (nm_wifi_ap_is_hotspot (ap)) - nm_wifi_ap_set_address (ap, nm_device_get_hw_address (device)); - - g_object_freeze_notify (G_OBJECT (self)); - ap_add_remove (self, TRUE, ap, TRUE); - g_object_thaw_notify (G_OBJECT (self)); set_current_ap (self, ap, FALSE); nm_active_connection_set_specific_object (NM_ACTIVE_CONNECTION (req), nm_dbus_object_get_path (NM_DBUS_OBJECT (ap))); - g_object_unref (ap); - return NM_ACT_STAGE_RETURN_SUCCESS; - -done: - set_current_ap (self, ap, TRUE); return NM_ACT_STAGE_RETURN_SUCCESS; } @@ -2834,8 +2833,8 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) * if the user didn't specify one and we didn't find an AP that matched * the connection, just pick a frequency the device supports. */ - if ( ap_mode == NM_802_11_MODE_ADHOC - || ap_mode == NM_802_11_MODE_MESH + if ( NM_IN_SET (ap_mode, NM_802_11_MODE_ADHOC, + NM_802_11_MODE_MESH) || nm_wifi_ap_is_hotspot (ap)) ensure_hotspot_frequency (self, s_wireless, ap); From 7bf8c45b192547f83adddde83ffcbac9e90fe0b8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 22 Aug 2019 11:02:59 +0200 Subject: [PATCH 11/26] device/wifi: cleanup supplicant_iface_wps_credentials_cb() Restructure code to return early and free resources with nm_auto. --- src/devices/wifi/nm-device-wifi.c | 39 ++++++++++++++++--------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index a587348c89..521d9e3c78 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -1755,10 +1755,11 @@ supplicant_iface_wps_credentials_cb (NMSupplicantInterface *iface, NMDeviceWifi *self) { NMActRequest *req; - GVariant *val, *secrets = NULL; + gs_unref_variant GVariant *val_key = NULL; + gs_unref_variant GVariant *secrets = NULL; + gs_free_error GError *error = NULL; const char *array; gsize psk_len = 0; - GError *error = NULL; if (nm_device_get_state (NM_DEVICE (self)) != NM_DEVICE_STATE_NEED_AUTH) { _LOGI (LOGD_DEVICE | LOGD_WIFI, "WPS: The connection can't be updated with credentials"); @@ -1770,11 +1771,11 @@ supplicant_iface_wps_credentials_cb (NMSupplicantInterface *iface, req = nm_device_get_act_request (NM_DEVICE (self)); g_return_if_fail (NM_IS_ACT_REQUEST (req)); - val = g_variant_lookup_value (credentials, "Key", G_VARIANT_TYPE_BYTESTRING); - if (val) { + val_key = g_variant_lookup_value (credentials, "Key", G_VARIANT_TYPE_BYTESTRING); + if (val_key) { char psk[64]; - array = g_variant_get_fixed_array (val, &psk_len, 1); + array = g_variant_get_fixed_array (val_key, &psk_len, 1); if (psk_len >= 8 && psk_len <= 63) { memcpy (psk, array, psk_len); psk[psk_len] = '\0'; @@ -1787,22 +1788,22 @@ supplicant_iface_wps_credentials_cb (NMSupplicantInterface *iface, } if (!secrets) _LOGW (LOGD_DEVICE | LOGD_WIFI, "WPS: ignore invalid PSK"); - g_variant_unref (val); } - if (secrets) { - if (nm_settings_connection_new_secrets (nm_act_request_get_settings_connection (req), - nm_act_request_get_applied_connection (req), - NM_SETTING_WIRELESS_SECURITY_SETTING_NAME, - secrets, - &error)) { - wifi_secrets_cancel (self); - nm_device_activate_schedule_stage1_device_prepare (NM_DEVICE (self)); - } else { - _LOGW (LOGD_DEVICE | LOGD_WIFI, "WPS: Could not update the connection with credentials: %s", error->message); - g_error_free (error); - } - g_variant_unref (secrets); + + if (!secrets) + return; + + if (!nm_settings_connection_new_secrets (nm_act_request_get_settings_connection (req), + nm_act_request_get_applied_connection (req), + NM_SETTING_WIRELESS_SECURITY_SETTING_NAME, + secrets, + &error)) { + _LOGW (LOGD_DEVICE | LOGD_WIFI, "WPS: Could not update the connection with credentials: %s", error->message); + return; } + + wifi_secrets_cancel (self); + nm_device_activate_schedule_stage1_device_prepare (NM_DEVICE (self)); } static gboolean From 7fd50f2789c35f2a13d0783deb1d7bb9fd6e079e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 22 Aug 2019 07:41:34 +0200 Subject: [PATCH 12/26] device: various minor style cleanup --- src/devices/nm-device-infiniband.c | 2 +- src/devices/nm-device-tun.c | 12 ++++++------ src/devices/nm-device-vlan.c | 5 +++-- src/devices/team/nm-device-team.c | 9 ++++++--- src/devices/wifi/nm-device-iwd.c | 21 ++++++++++++--------- src/devices/wwan/nm-device-modem.c | 4 +++- 6 files changed, 31 insertions(+), 22 deletions(-) diff --git a/src/devices/nm-device-infiniband.c b/src/devices/nm-device-infiniband.c index 89db2af294..18b4d64a3f 100644 --- a/src/devices/nm-device-infiniband.c +++ b/src/devices/nm-device-infiniband.c @@ -94,7 +94,7 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) dirfd = nm_platform_sysctl_open_netdir (nm_device_get_platform (device), nm_device_get_ifindex (device), ifname_verified); if (dirfd < 0) { - if (!strcmp (transport_mode, "datagram")) + if (nm_streq (transport_mode, "datagram")) return NM_ACT_STAGE_RETURN_SUCCESS; else { NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_INFINIBAND_MODE); diff --git a/src/devices/nm-device-tun.c b/src/devices/nm-device-tun.c index afe83f506a..50099248b9 100644 --- a/src/devices/nm-device-tun.c +++ b/src/devices/nm-device-tun.c @@ -357,12 +357,12 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) if (ret != NM_ACT_STAGE_RETURN_SUCCESS) return ret; - /* Nothing to do for TUN devices */ - if (priv->props.type == IFF_TUN) - return NM_ACT_STAGE_RETURN_SUCCESS; - - if (!nm_device_hw_addr_set_cloned (device, nm_device_get_applied_connection (device), FALSE)) - return NM_ACT_STAGE_RETURN_FAILURE; + if (priv->props.type == IFF_TUN) { + /* Nothing to do for TUN devices */ + } else { + if (!nm_device_hw_addr_set_cloned (device, nm_device_get_applied_connection (device), FALSE)) + return NM_ACT_STAGE_RETURN_FAILURE; + } return NM_ACT_STAGE_RETURN_SUCCESS; } diff --git a/src/devices/nm-device-vlan.c b/src/devices/nm-device-vlan.c index 0467a6e450..27611499c9 100644 --- a/src/devices/nm-device-vlan.c +++ b/src/devices/nm-device-vlan.c @@ -497,7 +497,8 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) if (s_vlan) { gs_free NMVlanQosMapping *ingress_map = NULL; gs_free NMVlanQosMapping *egress_map = NULL; - guint n_ingress_map = 0, n_egress_map = 0; + guint n_ingress_map = 0; + guint n_egress_map = 0; _nm_setting_vlan_get_priorities (s_vlan, NM_VLAN_INGRESS_MAP, @@ -520,7 +521,7 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) n_egress_map); } - return ret; + return NM_ACT_STAGE_RETURN_SUCCESS; } static guint32 diff --git a/src/devices/team/nm-device-team.c b/src/devices/team/nm-device-team.c index 10982cfa4f..fb176ed13e 100644 --- a/src/devices/team/nm-device-team.c +++ b/src/devices/team/nm-device-team.c @@ -660,7 +660,8 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) * have a PID, then we must fail. */ cfg = teamdctl_config_get_raw (priv->tdc); - if (cfg && nm_streq0 (cfg, nm_setting_team_get_config (s_team))) { + if ( cfg + && nm_streq0 (cfg, nm_setting_team_get_config (s_team))) { _LOGD (LOGD_TEAM, "using existing matching teamd config"); return NM_ACT_STAGE_RETURN_SUCCESS; } @@ -684,8 +685,10 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) return NM_ACT_STAGE_RETURN_POSTPONE; } - return teamd_start (device, connection) ? - NM_ACT_STAGE_RETURN_POSTPONE : NM_ACT_STAGE_RETURN_FAILURE; + if (!teamd_start (device, connection)) + return NM_ACT_STAGE_RETURN_FAILURE; + + return NM_ACT_STAGE_RETURN_POSTPONE; } static void diff --git a/src/devices/wifi/nm-device-iwd.c b/src/devices/wifi/nm-device-iwd.c index 9cfe5f70a1..b97f439125 100644 --- a/src/devices/wifi/nm-device-iwd.c +++ b/src/devices/wifi/nm-device-iwd.c @@ -1716,6 +1716,7 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) NMDeviceIwdPrivate *priv = NM_DEVICE_IWD_GET_PRIVATE (self); NMActStageReturn ret; NMWifiAP *ap = NULL; + gs_unref_object NMWifiAP *ap_fake = NULL; NMActRequest *req; NMConnection *connection; NMSettingWireless *s_wireless; @@ -1741,7 +1742,9 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) goto add_new; ap_path = nm_active_connection_get_specific_object (NM_ACTIVE_CONNECTION (req)); - ap = ap_path ? nm_wifi_ap_lookup_for_device (NM_DEVICE (self), ap_path) : NULL; + ap = ap_path + ? nm_wifi_ap_lookup_for_device (NM_DEVICE (self), ap_path) + : NULL; if (ap) { set_current_ap (self, ap, TRUE); return NM_ACT_STAGE_RETURN_SUCCESS; @@ -1767,19 +1770,19 @@ add_new: * until the real one is found in the scan list (Ad-Hoc or Hidden), or until * the device is deactivated (Ad-Hoc or Hotspot). */ - ap = nm_wifi_ap_new_fake_from_connection (connection); - g_return_val_if_fail (ap != NULL, NM_ACT_STAGE_RETURN_FAILURE); + ap_fake = nm_wifi_ap_new_fake_from_connection (connection); + if (!ap_fake) + g_return_val_if_reached (NM_ACT_STAGE_RETURN_FAILURE); - if (nm_wifi_ap_is_hotspot (ap)) - nm_wifi_ap_set_address (ap, nm_device_get_hw_address (device)); + if (nm_wifi_ap_is_hotspot (ap_fake)) + nm_wifi_ap_set_address (ap_fake, nm_device_get_hw_address (device)); g_object_freeze_notify (G_OBJECT (self)); - ap_add_remove (self, TRUE, ap, FALSE); + ap_add_remove (self, TRUE, ap_fake, FALSE); g_object_thaw_notify (G_OBJECT (self)); - set_current_ap (self, ap, FALSE); + set_current_ap (self, ap_fake, FALSE); nm_active_connection_set_specific_object (NM_ACTIVE_CONNECTION (req), - nm_dbus_object_get_path (NM_DBUS_OBJECT (ap))); - g_object_unref (ap); + nm_dbus_object_get_path (NM_DBUS_OBJECT (ap_fake))); return NM_ACT_STAGE_RETURN_SUCCESS; } diff --git a/src/devices/wwan/nm-device-modem.c b/src/devices/wwan/nm-device-modem.c index fc38fec435..29c51162d0 100644 --- a/src/devices/wwan/nm-device-modem.c +++ b/src/devices/wwan/nm-device-modem.c @@ -593,7 +593,9 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) req = nm_device_get_act_request (device); g_return_val_if_fail (req, NM_ACT_STAGE_RETURN_FAILURE); - return nm_modem_act_stage1_prepare (NM_DEVICE_MODEM_GET_PRIVATE (device)->modem, req, out_failure_reason); + return nm_modem_act_stage1_prepare (NM_DEVICE_MODEM_GET_PRIVATE (device)->modem, + req, + out_failure_reason); } static NMActStageReturn From f42ced162ff59a3ae58747caaa259a853ce1d03c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 28 Aug 2019 16:03:02 +0200 Subject: [PATCH 13/26] device/trivial: rename local variable for device in "nm-device-{ethernet,macvlan}.c" This variable is commonly called "device", not "dev". Rename. --- src/devices/nm-device-ethernet.c | 26 +++++++++++++------------- src/devices/nm-device-macvlan.c | 9 +++++---- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/src/devices/nm-device-ethernet.c b/src/devices/nm-device-ethernet.c index b51a7dcd69..70531d814d 100644 --- a/src/devices/nm-device-ethernet.c +++ b/src/devices/nm-device-ethernet.c @@ -496,17 +496,17 @@ link_timeout_cb (gpointer user_data) { NMDeviceEthernet *self = NM_DEVICE_ETHERNET (user_data); NMDeviceEthernetPrivate *priv = NM_DEVICE_ETHERNET_GET_PRIVATE (self); - NMDevice *dev = NM_DEVICE (self); + NMDevice *device = NM_DEVICE (self); NMActRequest *req; NMConnection *applied_connection; const char *setting_name; priv->supplicant_timeout_id = 0; - req = nm_device_get_act_request (dev); + req = nm_device_get_act_request (device); - if (nm_device_get_state (dev) == NM_DEVICE_STATE_ACTIVATED) { - nm_device_state_changed (dev, + if (nm_device_get_state (device) == NM_DEVICE_STATE_ACTIVATED) { + nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_SUPPLICANT_TIMEOUT); return FALSE; @@ -516,7 +516,7 @@ link_timeout_cb (gpointer user_data) * ARE checked - we are likely to have wrong key. Ask the user for * another one. */ - if (nm_device_get_state (dev) != NM_DEVICE_STATE_CONFIG) + if (nm_device_get_state (device) != NM_DEVICE_STATE_CONFIG) goto time_out; nm_active_connection_clear_secrets (NM_ACTIVE_CONNECTION (req)); @@ -530,14 +530,14 @@ link_timeout_cb (gpointer user_data) "Activation: (ethernet) disconnected during authentication, asking for new key."); supplicant_interface_release (self); - nm_device_state_changed (dev, NM_DEVICE_STATE_NEED_AUTH, NM_DEVICE_STATE_REASON_SUPPLICANT_DISCONNECT); + nm_device_state_changed (device, NM_DEVICE_STATE_NEED_AUTH, NM_DEVICE_STATE_REASON_SUPPLICANT_DISCONNECT); wired_secrets_get_secrets (self, setting_name, NM_SECRET_AGENT_GET_SECRETS_FLAG_REQUEST_NEW); return FALSE; time_out: _LOGW (LOGD_DEVICE | LOGD_ETHER, "link timed out."); - nm_device_state_changed (dev, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_SUPPLICANT_DISCONNECT); + nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_SUPPLICANT_DISCONNECT); return FALSE; } @@ -857,19 +857,19 @@ pppoe_reconnect_delay (gpointer user_data) } static NMActStageReturn -act_stage1_prepare (NMDevice *dev, NMDeviceStateReason *out_failure_reason) +act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) { - NMDeviceEthernet *self = NM_DEVICE_ETHERNET (dev); + NMDeviceEthernet *self = NM_DEVICE_ETHERNET (device); NMDeviceEthernetPrivate *priv = NM_DEVICE_ETHERNET_GET_PRIVATE (self); NMActStageReturn ret; - ret = NM_DEVICE_CLASS (nm_device_ethernet_parent_class)->act_stage1_prepare (dev, out_failure_reason); + ret = NM_DEVICE_CLASS (nm_device_ethernet_parent_class)->act_stage1_prepare (device, out_failure_reason); if (ret != NM_ACT_STAGE_RETURN_SUCCESS) return ret; - link_negotiation_set (dev); + link_negotiation_set (device); - if (!nm_device_hw_addr_set_cloned (dev, nm_device_get_applied_connection (dev), FALSE)) + if (!nm_device_hw_addr_set_cloned (device, nm_device_get_applied_connection (device), FALSE)) return NM_ACT_STAGE_RETURN_FAILURE; /* If we're re-activating a PPPoE connection a short while after @@ -885,7 +885,7 @@ act_stage1_prepare (NMDevice *dev, NMDeviceStateReason *out_failure_reason) gint32 delay = nm_utils_get_monotonic_timestamp_s () - priv->last_pppoe_time; if ( delay < PPPOE_RECONNECT_DELAY - && nm_device_get_applied_setting (dev, NM_TYPE_SETTING_PPPOE)) { + && nm_device_get_applied_setting (device, NM_TYPE_SETTING_PPPOE)) { if (priv->pppoe_wait_id == 0) { _LOGI (LOGD_DEVICE, "delaying PPPoE reconnect for %d seconds to ensure peer is ready...", delay); diff --git a/src/devices/nm-device-macvlan.c b/src/devices/nm-device-macvlan.c index 709f98dab5..ee8b997460 100644 --- a/src/devices/nm-device-macvlan.c +++ b/src/devices/nm-device-macvlan.c @@ -270,7 +270,7 @@ create_and_realize (NMDevice *device, /*****************************************************************************/ static NMDeviceCapabilities -get_generic_capabilities (NMDevice *dev) +get_generic_capabilities (NMDevice *device) { /* We assume MACVLAN interfaces always support carrier detect */ return NM_DEVICE_CAP_CARRIER_DETECT | NM_DEVICE_CAP_IS_SOFTWARE; @@ -416,16 +416,17 @@ update_connection (NMDevice *device, NMConnection *connection) } static NMActStageReturn -act_stage1_prepare (NMDevice *dev, NMDeviceStateReason *out_failure_reason) +act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) { NMActStageReturn ret; - ret = NM_DEVICE_CLASS (nm_device_macvlan_parent_class)->act_stage1_prepare (dev, out_failure_reason); + ret = NM_DEVICE_CLASS (nm_device_macvlan_parent_class)->act_stage1_prepare (device, out_failure_reason); if (ret != NM_ACT_STAGE_RETURN_SUCCESS) return ret; - if (!nm_device_hw_addr_set_cloned (dev, nm_device_get_applied_connection (dev), FALSE)) + if (!nm_device_hw_addr_set_cloned (device, nm_device_get_applied_connection (device), FALSE)) return NM_ACT_STAGE_RETURN_FAILURE; + return NM_ACT_STAGE_RETURN_SUCCESS; } From c3d41fa4529ec9c9e266d117cc5b8ec4e2d917b8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 21 Aug 2019 11:56:39 +0200 Subject: [PATCH 14/26] device: refactor handling of scheduled activation tasks on idle - use a [2] array for IPv4/IPv6 variants and a IS_IPv4 variable, like we do for other places that have similar implementations for both address families. - drop ActivationHandleData and use the fields directly. Also drop activation_source_get_by_family(). - rename "act_handle*" field to "activation_source_*", to follow the naming of the related accessor functions. - downgrade the severity of some logging messages. --- src/devices/nm-device.c | 176 ++++++++++++++++++---------------------- src/nm-policy.c | 2 +- 2 files changed, 79 insertions(+), 99 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index c08797ca4f..bca194c79b 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -102,11 +102,6 @@ _LOG_DECLARE_SELF (NMDevice); typedef void (*ActivationHandleFunc) (NMDevice *self); -typedef struct { - ActivationHandleFunc func; - guint id; -} ActivationHandleData; - typedef enum { CLEANUP_TYPE_KEEP, CLEANUP_TYPE_REMOVED, @@ -333,8 +328,23 @@ typedef struct _NMDevicePrivate { NMActRequest * queued_act_request; bool queued_act_request_is_waiting_for_carrier:1; NMDBusTrackObjPath act_request; - ActivationHandleData act_handle4; /* for layer2 and IPv4. */ - ActivationHandleData act_handle6; + + union { + struct { + guint activation_source_id_6; + guint activation_source_id_4; /* for layer2 and IPv4. */ + }; + guint activation_source_id_x[2]; + }; + + union { + struct { + ActivationHandleFunc activation_source_func_6; + ActivationHandleFunc activation_source_func_4; /* for layer2 and IPv4. */ + }; + ActivationHandleFunc activation_source_func_x[2]; + }; + guint recheck_assume_id; struct { @@ -633,7 +643,6 @@ static void _carrier_wait_check_queued_act_request (NMDevice *self); static gint64 _get_carrier_wait_ms (NMDevice *self); static const char *_activation_func_to_string (ActivationHandleFunc func); -static void activation_source_handle_cb (NMDevice *self, int addr_family); static void _set_state_full (NMDevice *self, NMDeviceState state, @@ -6094,119 +6103,102 @@ dnsmasq_state_changed_cb (NMDnsMasqManager *manager, guint32 status, gpointer us /*****************************************************************************/ -static gboolean -activation_source_handle_cb4 (gpointer user_data) -{ - activation_source_handle_cb (user_data, AF_INET); - return G_SOURCE_REMOVE; -} - -static gboolean -activation_source_handle_cb6 (gpointer user_data) -{ - activation_source_handle_cb (user_data, AF_INET6); - return G_SOURCE_REMOVE; -} - -static ActivationHandleData * -activation_source_get_by_family (NMDevice *self, - int addr_family, - GSourceFunc *out_idle_func) -{ - NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); - - switch (addr_family) { - case AF_INET6: - NM_SET_OUT (out_idle_func, activation_source_handle_cb6); - return &priv->act_handle6; - case AF_INET: - NM_SET_OUT (out_idle_func, activation_source_handle_cb4); - return &priv->act_handle4; - } - g_return_val_if_reached (NULL); -} - static void activation_source_clear (NMDevice *self, int addr_family) { - ActivationHandleData *act_data; + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + const gboolean IS_IPv4 = (addr_family == AF_INET); - act_data = activation_source_get_by_family (self, addr_family, NULL); - - if (act_data->id) { + if (priv->activation_source_id_x[IS_IPv4] != 0) { _LOGD (LOGD_DEVICE, "activation-stage: clear %s,v%c (id %u)", - _activation_func_to_string (act_data->func), + _activation_func_to_string (priv->activation_source_func_x[IS_IPv4]), nm_utils_addr_family_to_char (addr_family), - act_data->id); - nm_clear_g_source (&act_data->id); - act_data->func = NULL; + priv->activation_source_id_x[IS_IPv4]); + nm_clear_g_source (&priv->activation_source_id_x[IS_IPv4]); + priv->activation_source_func_x[IS_IPv4] = NULL; } } -static void +static gboolean activation_source_handle_cb (NMDevice *self, int addr_family) { - ActivationHandleData *act_data, a; + NMDevicePrivate *priv; + const gboolean IS_IPv4 = (addr_family == AF_INET); + ActivationHandleFunc activation_source_func; + guint activation_source_id; - g_return_if_fail (NM_IS_DEVICE (self)); + g_return_val_if_fail (NM_IS_DEVICE (self), G_SOURCE_REMOVE); - act_data = activation_source_get_by_family (self, addr_family, NULL); + priv = NM_DEVICE_GET_PRIVATE (self); - g_return_if_fail (act_data->id); - g_return_if_fail (act_data->func); + activation_source_func = priv->activation_source_func_x[IS_IPv4]; + activation_source_id = priv->activation_source_id_x[IS_IPv4]; - a = *act_data; + g_return_val_if_fail (activation_source_id != 0, G_SOURCE_REMOVE); + nm_assert (activation_source_func); - act_data->func = NULL; - act_data->id = 0; + priv->activation_source_func_x[IS_IPv4] = NULL; + priv->activation_source_id_x[IS_IPv4] = 0; _LOGD (LOGD_DEVICE, "activation-stage: invoke %s,v%c (id %u)", - _activation_func_to_string (a.func), + _activation_func_to_string (activation_source_func), nm_utils_addr_family_to_char (addr_family), - a.id); + activation_source_id); - a.func (self); + activation_source_func (self); - _LOGD (LOGD_DEVICE, "activation-stage: complete %s,v%c (id %u)", - _activation_func_to_string (a.func), + _LOGT (LOGD_DEVICE, "activation-stage: complete %s,v%c (id %u)", + _activation_func_to_string (activation_source_func), nm_utils_addr_family_to_char (addr_family), - a.id); + activation_source_id); + + return G_SOURCE_REMOVE; +} + +static gboolean +activation_source_handle_cb_4 (gpointer user_data) +{ + return activation_source_handle_cb (user_data, AF_INET); +} + +static gboolean +activation_source_handle_cb_6 (gpointer user_data) +{ + return activation_source_handle_cb (user_data, AF_INET6); } static void activation_source_schedule (NMDevice *self, ActivationHandleFunc func, int addr_family) { - ActivationHandleData *act_data; - GSourceFunc source_func = NULL; + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + const gboolean IS_IPv4 = (addr_family == AF_INET); guint new_id = 0; - act_data = activation_source_get_by_family (self, addr_family, &source_func); - - if (act_data->id && act_data->func == func) { - /* Don't bother rescheduling the same function that's about to - * run anyway. Fixes issues with crappy wireless drivers sending - * streams of associate events before NM has had a chance to process - * the first one. - */ - _LOGD (LOGD_DEVICE, "activation-stage: already scheduled %s,v%c (id %u)", + if ( priv->activation_source_id_x[IS_IPv4] != 0 + && priv->activation_source_func_x[IS_IPv4] == func) { + /* Scheduling the same stage multiple times is fine. */ + _LOGT (LOGD_DEVICE, "activation-stage: already scheduled %s,v%c (id %u)", _activation_func_to_string (func), nm_utils_addr_family_to_char (addr_family), - act_data->id); + priv->activation_source_id_x[IS_IPv4]); return; } - new_id = g_idle_add (source_func, self); + new_id = g_idle_add ( IS_IPv4 + ? activation_source_handle_cb_4 + : activation_source_handle_cb_6, + self); - if (act_data->id) { - _LOGW (LOGD_DEVICE, "activation-stage: schedule %s,v%c which replaces %s,v%c (id %u -> %u)", + if (priv->activation_source_id_x[IS_IPv4] != 0) { + _LOGD (LOGD_DEVICE, "activation-stage: schedule %s,v%c which replaces %s,v%c (id %u -> %u)", _activation_func_to_string (func), nm_utils_addr_family_to_char (addr_family), - _activation_func_to_string (act_data->func), + _activation_func_to_string (priv->activation_source_func_x[IS_IPv4]), nm_utils_addr_family_to_char (addr_family), - act_data->id, new_id); - nm_clear_g_source (&act_data->id); + priv->activation_source_id_x[IS_IPv4], new_id); + nm_clear_g_source (&priv->activation_source_id_x[IS_IPv4]); } else { _LOGD (LOGD_DEVICE, "activation-stage: schedule %s,v%c (id %u)", _activation_func_to_string (func), @@ -6214,19 +6206,8 @@ activation_source_schedule (NMDevice *self, ActivationHandleFunc func, int addr_ new_id); } - act_data->func = func; - act_data->id = new_id; -} - -static gboolean -activation_source_is_scheduled (NMDevice *self, - ActivationHandleFunc func, - int addr_family) -{ - ActivationHandleData *act_data; - - act_data = activation_source_get_by_family (self, addr_family, NULL); - return act_data->func == func; + priv->activation_source_func_x[IS_IPv4] = func; + priv->activation_source_id_x[IS_IPv4] = new_id; } /*****************************************************************************/ @@ -12269,7 +12250,7 @@ nm_device_is_activating (NMDevice *self) * handler is actually run. If there's an activation handler scheduled * we're activating anyway. */ - return priv->act_handle4.id ? TRUE : FALSE; + return priv->activation_source_id_4 != 0; } NMProxyConfig * @@ -13251,9 +13232,8 @@ queued_ip_config_change (NMDevice *self, int addr_family) * it changing IP configurations before they are applied. Postpone the * update in such case. */ - if (activation_source_is_scheduled (self, - activate_stage5_ip_config_result_x[IS_IPv4], - addr_family)) + if ( priv->activation_source_id_x[IS_IPv4] != 0 + && priv->activation_source_func_x[IS_IPv4] == activate_stage5_ip_config_result_x[IS_IPv4]) return G_SOURCE_CONTINUE; priv->queued_ip_config_id_x[IS_IPv4] = 0; diff --git a/src/nm-policy.c b/src/nm-policy.c index 559babed3a..0aace85933 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1886,7 +1886,7 @@ device_state_changed (NMDevice *device, switch (nm_device_state_reason_check (reason)) { case NM_DEVICE_STATE_REASON_USER_REQUESTED: - blocked_reason = NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_USER_REQUEST; + blocked_reason = NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_USER_REQUEST; break; case NM_DEVICE_STATE_REASON_DEPENDENCY_FAILED: blocked_reason = NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED; From cca0c2b56a4c637d8fc222353126b6f46187ba53 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 21 Aug 2019 14:30:34 +0200 Subject: [PATCH 15/26] device: move SR-IOV initialization to activate_stage1_device_prepare() Note that all subclasses of NMDevice that implement act_stage1_prepare(), call the parents implementation as very first thing. Previously, NMDevice's act_stage1_prepare() was setting up SR-IOV. But that is problemantic. Note that it may have returned NM_ACT_STAGE_RETURN_POSTPONE, in which case subclasses would just skip their action and return to the caller. Later, sriov_params_cb() would directly call nm_device_activate_schedule_stage2_device_config(), and thus act_stage1_prepare() was never executed for the subclass. That is wrong. First, I don't think it is good to let subclasses decide whether to call a parents implementation (and when). It just causes ambiguity. In the best case we do it always in the same order, in the worst case we call the parent at the wrong time or never. Instead, we want to initialize SR-IOV always and early in stage1, so we should just do it directly from activate_stage1_device_prepare(). Now NMDevice's act_stage1_prepare() does nothing. The bigger problem is that when a device wants to resume a stage that it previously postponed, that it would schedule the next stage! Instead, it should schedule the same stage again instead. That allows to postpone the completion of a stage for multiple reasons, and each call to a certain stage merely notifies that something changed and we need to check whether we can now complete the stage. For that to work, stages must become re-entrant. That means we need to remember whether an action that we care about needs to be started, is pending or already completed. Compare this to nm_device_activate_schedule_stage3_ip_config_start(), which checks whether firewall is configured. That is likewise the wrong approach. Callers that were in stage2 and postponed stage2, and later would schedule stage3 when they are ready. Then nm_device_activate_schedule_stage3_ip_config_start() would check whether firewall is also ready, and do nothing if that's not the case (relying that when the firewall code completes to call nm_device_activate_schedule_stage3_ip_config_start(). --- src/devices/nm-device-private.h | 6 + src/devices/nm-device.c | 230 ++++++++++++++++++++------------ 2 files changed, 151 insertions(+), 85 deletions(-) diff --git a/src/devices/nm-device-private.h b/src/devices/nm-device-private.h index 0a414af96f..8b2cc47443 100644 --- a/src/devices/nm-device-private.h +++ b/src/devices/nm-device-private.h @@ -25,6 +25,12 @@ /* This file should only be used by subclasses of NMDevice */ +typedef enum { + NM_DEVICE_STAGE_STATE_INIT = 0, + NM_DEVICE_STAGE_STATE_PENDING = 1, + NM_DEVICE_STAGE_STATE_COMPLETED = 2, +} NMDeviceStageState; + typedef enum { NM_DEVICE_IP_STATE_NONE, NM_DEVICE_IP_STATE_WAIT, diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index bca194c79b..cbaeb81a88 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -413,6 +413,8 @@ typedef struct _NMDevicePrivate { bool concheck_rp_filter_checked:1; + NMDeviceStageState stage1_sriov_state:3; + /* Generic DHCP stuff */ char * dhcp_anycast_address; @@ -680,6 +682,8 @@ static void (*const activate_stage4_ip_config_timeout_x[2]) (NMDevice *self) = { activate_stage4_ip_config_timeout_4, }; +static void sriov_op_cb (GError *error, gpointer user_data); + static void activate_stage5_ip_config_result_4 (NMDevice *self); static void activate_stage5_ip_config_result_6 (NMDevice *self); @@ -4243,7 +4247,7 @@ nm_device_update_from_platform_link (NMDevice *self, const NMPlatformLink *plink } } -static void sriov_op_cb (GError *error, gpointer user_data); +/*****************************************************************************/ static void sriov_op_start (NMDevice *self, SriovOp *op) @@ -4276,11 +4280,14 @@ sriov_op_cb (GError *error, gpointer user_data) priv->sriov.pending = NULL; + g_clear_object (&op->cancellable); + if (op->callback) op->callback (error, op->callback_data); - g_clear_object (&op->cancellable); - g_slice_free (SriovOp, op); + nm_assert (!priv->sriov.pending); + + nm_g_slice_free (op); if (priv->sriov.next) { sriov_op_start (self, @@ -4288,6 +4295,44 @@ sriov_op_cb (GError *error, gpointer user_data) } } +static void +sriov_op_queue_op (NMDevice *self, + SriovOp *op) +{ + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + + if (priv->sriov.next) { + SriovOp *op_next = g_steal_pointer (&priv->sriov.next); + + /* Cancel the next operation immediately */ + if (op_next->callback) { + gs_free_error GError *error = NULL; + + nm_utils_error_set_cancelled (&error, FALSE, NULL); + op_next->callback (error, op_next->callback_data); + } + + nm_g_slice_free (op_next); + + if (!priv->sriov.pending) { + /* This (having "next" set but "pending" not) can only happen if we are + * called from inside the callback again. + * + * That means we append the new request as "next" and return. Once + * the callback returns, it will schedule the request. */ + priv->sriov.next = op; + return; + } + } else if (priv->sriov.pending) { + priv->sriov.next = op; + g_cancellable_cancel (priv->sriov.pending->cancellable); + return; + } + + if (op) + sriov_op_start (self, op); +} + static void sriov_op_queue (NMDevice *self, guint num_vfs, @@ -4295,32 +4340,35 @@ sriov_op_queue (NMDevice *self, NMPlatformAsyncCallback callback, gpointer callback_data) { - NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); - GError *error = NULL; SriovOp *op; - op = g_slice_new0 (SriovOp); - op->num_vfs = num_vfs; - op->autoprobe = autoprobe; - op->callback = callback; - op->callback_data = callback_data; + /* We usually never want to cancel an async write operation, unless it's superseded + * by a newer operation (that resets the state). That is, because we need to ensure + * that we never end up doing two concurrent writes (since we write on a background + * thread, that would be unordered/racy). + * Of course, since we queue requests only per-device, when devices get renamed we + * might end up writing the same sysctl concurrently still. But that's really + * unlikely, and don't rename after udev completes! + * + * The "next" operation is not yet even started. It can be replaced/canceled right away + * when a newer request comes. + * The "pending" operation is currently ongoing, and we may cancel it if + * we have a follow-up operation (queued in "next"). Unless we have a such + * a newer request, we cannot cancel it! + * + * FIXME(shutdown): However, during shutdown we don't have a follow-up write request to cancel + * this operation and we have to give it at least some time to complete. The solution is that + * we register a way to abort the last call during shutdown, and after NM_SHUTDOWN_TIMEOUT_MS + * grace period we pull the plug and cancel it. */ - if (priv->sriov.next) { - /* Cancel the next operation immediately */ - if (priv->sriov.next->callback) { - nm_utils_error_set_cancelled (&error, FALSE, NULL); - priv->sriov.next->callback (error, priv->sriov.next->callback_data); - g_clear_error (&error); - } - g_slice_free (SriovOp, priv->sriov.next); - priv->sriov.next = NULL; - } - - if (priv->sriov.pending) { - priv->sriov.next = op; - g_cancellable_cancel (priv->sriov.pending->cancellable); - } else - sriov_op_start (self, op); + op = g_slice_new (SriovOp); + *op = (SriovOp) { + .num_vfs = num_vfs, + .autoprobe = autoprobe, + .callback = callback, + .callback_data = callback_data, + }; + sriov_op_queue_op (self, op); } static void @@ -6396,63 +6444,14 @@ sriov_params_cb (GError *error, gpointer data) return; } - nm_device_activate_schedule_stage2_device_config (self); + priv->stage1_sriov_state = NM_DEVICE_STAGE_STATE_COMPLETED; + + nm_device_activate_schedule_stage1_device_prepare (self); } static NMActStageReturn act_stage1_prepare (NMDevice *self, NMDeviceStateReason *out_failure_reason) { - NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); - NMSettingSriov *s_sriov; - guint i, num; - - if ( priv->ifindex > 0 - && nm_device_has_capability (self, NM_DEVICE_CAP_SRIOV) - && (s_sriov = nm_device_get_applied_setting (self, NM_TYPE_SETTING_SRIOV))) { - nm_auto_freev NMPlatformVF **plat_vfs = NULL; - gs_free_error GError *error = NULL; - NMSriovVF *vf; - NMTernary autoprobe; - gpointer *data; - - autoprobe = nm_setting_sriov_get_autoprobe_drivers (s_sriov); - if (autoprobe == NM_TERNARY_DEFAULT) { - autoprobe = nm_config_data_get_connection_default_int64 (NM_CONFIG_GET_DATA, - NM_CON_DEFAULT ("sriov.autoprobe-drivers"), - self, - NM_TERNARY_FALSE, - NM_TERNARY_TRUE, - NM_TERNARY_TRUE); - } - - num = nm_setting_sriov_get_num_vfs (s_sriov); - plat_vfs = g_new0 (NMPlatformVF *, num + 1); - for (i = 0; i < num; i++) { - vf = nm_setting_sriov_get_vf (s_sriov, i); - plat_vfs[i] = sriov_vf_config_to_platform (self, vf, &error); - if (!plat_vfs[i]) { - _LOGE (LOGD_DEVICE, - "failed to apply SR-IOV VF '%s': %s", - nm_utils_sriov_vf_to_str (vf, FALSE, NULL), - error->message); - NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_SRIOV_CONFIGURATION_FAILED); - return NM_ACT_STAGE_RETURN_FAILURE; - } - } - - /* When changing the number of VFs the kernel can block - * for very long time in the write to sysfs, especially - * if autoprobe-drivers is enabled. Do it asynchronously - * to avoid blocking the entire NM process. - */ - data = nm_utils_user_data_pack (self, g_steal_pointer (&plat_vfs)); - sriov_op_queue (self, - nm_setting_sriov_get_total_vfs (s_sriov), - autoprobe, - sriov_params_cb, - data); - return NM_ACT_STAGE_RETURN_POSTPONE; - } return NM_ACT_STAGE_RETURN_SUCCESS; } @@ -6481,18 +6480,77 @@ activate_stage1_device_prepare (NMDevice *self) nm_device_state_changed (self, NM_DEVICE_STATE_PREPARE, NM_DEVICE_STATE_REASON_NONE); + if (priv->stage1_sriov_state != NM_DEVICE_STAGE_STATE_COMPLETED) { + NMSettingSriov *s_sriov; + + if (nm_device_sys_iface_state_is_external_or_assume (self)) { + /* pass */ + } else if (priv->stage1_sriov_state == NM_DEVICE_STAGE_STATE_PENDING) + return; + else if ( priv->ifindex > 0 + && nm_device_has_capability (self, NM_DEVICE_CAP_SRIOV) + && (s_sriov = nm_device_get_applied_setting (self, NM_TYPE_SETTING_SRIOV))) { + nm_auto_freev NMPlatformVF **plat_vfs = NULL; + gs_free_error GError *error = NULL; + NMSriovVF *vf; + NMTernary autoprobe; + guint i, num; + + autoprobe = nm_setting_sriov_get_autoprobe_drivers (s_sriov); + if (autoprobe == NM_TERNARY_DEFAULT) { + autoprobe = nm_config_data_get_connection_default_int64 (NM_CONFIG_GET_DATA, + NM_CON_DEFAULT ("sriov.autoprobe-drivers"), + self, + NM_TERNARY_FALSE, + NM_TERNARY_TRUE, + NM_TERNARY_TRUE); + } + + num = nm_setting_sriov_get_num_vfs (s_sriov); + plat_vfs = g_new0 (NMPlatformVF *, num + 1); + for (i = 0; i < num; i++) { + vf = nm_setting_sriov_get_vf (s_sriov, i); + plat_vfs[i] = sriov_vf_config_to_platform (self, vf, &error); + if (!plat_vfs[i]) { + _LOGE (LOGD_DEVICE, + "failed to apply SR-IOV VF '%s': %s", + nm_utils_sriov_vf_to_str (vf, FALSE, NULL), + error->message); + nm_device_state_changed (self, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_SRIOV_CONFIGURATION_FAILED); + return; + } + } + + /* When changing the number of VFs the kernel can block + * for very long time in the write to sysfs, especially + * if autoprobe-drivers is enabled. Do it asynchronously + * to avoid blocking the entire NM process. + */ + sriov_op_queue (self, + nm_setting_sriov_get_total_vfs (s_sriov), + autoprobe, + sriov_params_cb, + nm_utils_user_data_pack (self, + g_steal_pointer (&plat_vfs))); + priv->stage1_sriov_state = NM_DEVICE_STAGE_STATE_PENDING; + return; + } + priv->stage1_sriov_state = NM_DEVICE_STAGE_STATE_COMPLETED; + } + /* Assumed connections were already set up outside NetworkManager */ if (!nm_device_sys_iface_state_is_external_or_assume (self)) { NMDeviceStateReason failure_reason = NM_DEVICE_STATE_REASON_NONE; ret = NM_DEVICE_GET_CLASS (self)->act_stage1_prepare (self, &failure_reason); - if (ret == NM_ACT_STAGE_RETURN_POSTPONE) { - return; - } else if (ret == NM_ACT_STAGE_RETURN_FAILURE) { + if (ret == NM_ACT_STAGE_RETURN_FAILURE) { nm_device_state_changed (self, NM_DEVICE_STATE_FAILED, failure_reason); return; } - g_return_if_fail (ret == NM_ACT_STAGE_RETURN_SUCCESS); + if (ret == NM_ACT_STAGE_RETURN_POSTPONE) + return; + + nm_assert (ret == NM_ACT_STAGE_RETURN_SUCCESS); } nm_device_activate_schedule_stage2_device_config (self); @@ -9911,7 +9969,7 @@ _ip6_privacy_get (NMDevice *self) return ip6_privacy; if (!nm_device_get_ip_ifindex (self)) - return NM_SETTING_IP6_CONFIG_PRIVACY_UNKNOWN;; + return NM_SETTING_IP6_CONFIG_PRIVACY_UNKNOWN; /* 3.) No valid default-value configured. Fallback to reading sysctl. * @@ -14572,6 +14630,8 @@ _cleanup_generic_pre (NMDevice *self, CleanupType cleanup_type) _cancel_activation (self); + priv->stage1_sriov_state = NM_DEVICE_STAGE_STATE_INIT; + if (cleanup_type != CLEANUP_TYPE_KEEP) { nm_manager_device_route_metric_clear (nm_manager_get (), nm_device_get_ip_ifindex (self)); @@ -15010,9 +15070,9 @@ deactivate_ready (NMDevice *self, NMDeviceStateReason reason) if (priv->dispatcher.call_id) return; - if (priv->sriov.pending) + if ( priv->sriov.pending + || priv->sriov.next) return; - nm_assert (!priv->sriov.next); nm_device_queue_state (self, NM_DEVICE_STATE_DISCONNECTED, reason); } From dc275121847fcfd02aeeb6360309e54300ebf3d4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 22 Aug 2019 09:28:39 +0200 Subject: [PATCH 16/26] device: don't let subclasses call NMDevice's act_stage1_prepare() NMDevice's act_stage1_prepare() now does nothing. Calling it is not useful and has no effect. In general, when a subclass overwrites a virtual function, it must be defined whether the subclass must, may or must-not call the parents implementation. Likewise, it must be clear when the parents implementation should be chained: first, as last, does it matter? In any case, that very much depends on how the parent is implemented and this can only be solved by documentation and common conventions. It's a forgiving approach to have a parents implementation do nothing, then the subclass may call it at any time (or not call it at all). This is especially useful if classes don't know their parent class well. But in NetworkManager code the relationship between classes are known at compile time, so every of these classes knows it derives directly from NMDevice. This forgingin approach was what NMDevice's act_stage1_prepare() was doing. However, it also adds lines of code resulting in a different kind of complexity. So, it's not clear that this forgiving approach is really better. Note that it also has a (tiny) runtime and code-size overhead. Change the expectation of how NMDevice's act_stage1_prepare() should be called: it is no longer implemented, and subclasses *MUST* not chain up. --- src/devices/nm-device-6lowpan.c | 6 ------ src/devices/nm-device-bond.c | 4 ---- src/devices/nm-device-bridge.c | 5 ----- src/devices/nm-device-dummy.c | 6 ------ src/devices/nm-device-ethernet.c | 5 ----- src/devices/nm-device-infiniband.c | 5 ----- src/devices/nm-device-ip-tunnel.c | 6 ------ src/devices/nm-device-macvlan.c | 6 ------ src/devices/nm-device-tun.c | 5 ----- src/devices/nm-device-vlan.c | 5 ----- src/devices/nm-device-vxlan.c | 6 ------ src/devices/nm-device-wpan.c | 4 ---- src/devices/nm-device.c | 29 ++++++++++++-------------- src/devices/team/nm-device-team.c | 5 ----- src/devices/wifi/nm-device-iwd.c | 5 ----- src/devices/wifi/nm-device-olpc-mesh.c | 5 ----- src/devices/wifi/nm-device-wifi-p2p.c | 5 ----- src/devices/wifi/nm-device-wifi.c | 5 ----- src/devices/wwan/nm-device-modem.c | 5 ----- 19 files changed, 13 insertions(+), 109 deletions(-) diff --git a/src/devices/nm-device-6lowpan.c b/src/devices/nm-device-6lowpan.c index 7a27943139..b63d0745c9 100644 --- a/src/devices/nm-device-6lowpan.c +++ b/src/devices/nm-device-6lowpan.c @@ -233,12 +233,6 @@ update_connection (NMDevice *device, NMConnection *connection) static NMActStageReturn act_stage1_prepare (NMDevice *dev, NMDeviceStateReason *out_failure_reason) { - NMActStageReturn ret; - - ret = NM_DEVICE_CLASS (nm_device_6lowpan_parent_class)->act_stage1_prepare (dev, out_failure_reason); - if (ret != NM_ACT_STAGE_RETURN_SUCCESS) - return ret; - if (!nm_device_hw_addr_set_cloned (dev, nm_device_get_applied_connection (dev), FALSE)) return NM_ACT_STAGE_RETURN_FAILURE; return NM_ACT_STAGE_RETURN_SUCCESS; diff --git a/src/devices/nm-device-bond.c b/src/devices/nm-device-bond.c index e67941d8f3..1a5a430dbe 100644 --- a/src/devices/nm-device-bond.c +++ b/src/devices/nm-device-bond.c @@ -343,10 +343,6 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) NMDeviceBond *self = NM_DEVICE_BOND (device); NMActStageReturn ret = NM_ACT_STAGE_RETURN_SUCCESS; - ret = NM_DEVICE_CLASS (nm_device_bond_parent_class)->act_stage1_prepare (device, out_failure_reason); - if (ret != NM_ACT_STAGE_RETURN_SUCCESS) - return ret; - /* Interface must be down to set bond options */ nm_device_take_down (device, TRUE); if (!apply_bonding_config (self)) diff --git a/src/devices/nm-device-bridge.c b/src/devices/nm-device-bridge.c index 86eaa6e88c..438f850315 100644 --- a/src/devices/nm-device-bridge.c +++ b/src/devices/nm-device-bridge.c @@ -497,15 +497,10 @@ bridge_set_vlan_options (NMDevice *device, NMSettingBridge *s_bridge) static NMActStageReturn act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) { - NMActStageReturn ret; NMConnection *connection; NMSetting *s_bridge; const Option *option; - ret = NM_DEVICE_CLASS (nm_device_bridge_parent_class)->act_stage1_prepare (device, out_failure_reason); - if (ret != NM_ACT_STAGE_RETURN_SUCCESS) - return ret; - connection = nm_device_get_applied_connection (device); g_return_val_if_fail (connection, NM_ACT_STAGE_RETURN_FAILURE); diff --git a/src/devices/nm-device-dummy.c b/src/devices/nm-device-dummy.c index 9c0c30358a..75e8f742b6 100644 --- a/src/devices/nm-device-dummy.c +++ b/src/devices/nm-device-dummy.c @@ -119,12 +119,6 @@ create_and_realize (NMDevice *device, static NMActStageReturn act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) { - NMActStageReturn ret; - - ret = NM_DEVICE_CLASS (nm_device_dummy_parent_class)->act_stage1_prepare (device, out_failure_reason); - if (ret != NM_ACT_STAGE_RETURN_SUCCESS) - return ret; - if (!nm_device_hw_addr_set_cloned (device, nm_device_get_applied_connection (device), FALSE)) return NM_ACT_STAGE_RETURN_FAILURE; diff --git a/src/devices/nm-device-ethernet.c b/src/devices/nm-device-ethernet.c index 70531d814d..d5fd155181 100644 --- a/src/devices/nm-device-ethernet.c +++ b/src/devices/nm-device-ethernet.c @@ -861,11 +861,6 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) { NMDeviceEthernet *self = NM_DEVICE_ETHERNET (device); NMDeviceEthernetPrivate *priv = NM_DEVICE_ETHERNET_GET_PRIVATE (self); - NMActStageReturn ret; - - ret = NM_DEVICE_CLASS (nm_device_ethernet_parent_class)->act_stage1_prepare (device, out_failure_reason); - if (ret != NM_ACT_STAGE_RETURN_SUCCESS) - return ret; link_negotiation_set (device); diff --git a/src/devices/nm-device-infiniband.c b/src/devices/nm-device-infiniband.c index 18b4d64a3f..ca098d2e12 100644 --- a/src/devices/nm-device-infiniband.c +++ b/src/devices/nm-device-infiniband.c @@ -76,16 +76,11 @@ static NMActStageReturn act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) { nm_auto_close int dirfd = -1; - NMActStageReturn ret; NMSettingInfiniband *s_infiniband; char ifname_verified[IFNAMSIZ]; const char *transport_mode; gboolean ok; - ret = NM_DEVICE_CLASS (nm_device_infiniband_parent_class)->act_stage1_prepare (device, out_failure_reason); - if (ret != NM_ACT_STAGE_RETURN_SUCCESS) - return ret; - s_infiniband = nm_device_get_applied_setting (device, NM_TYPE_SETTING_INFINIBAND); g_return_val_if_fail (s_infiniband, NM_ACT_STAGE_RETURN_FAILURE); diff --git a/src/devices/nm-device-ip-tunnel.c b/src/devices/nm-device-ip-tunnel.c index ede4487e3a..a490ea4cd2 100644 --- a/src/devices/nm-device-ip-tunnel.c +++ b/src/devices/nm-device-ip-tunnel.c @@ -968,12 +968,6 @@ set_property (GObject *object, guint prop_id, static NMActStageReturn act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) { - NMActStageReturn ret; - - ret = NM_DEVICE_CLASS (nm_device_ip_tunnel_parent_class)->act_stage1_prepare (device, out_failure_reason); - if (ret != NM_ACT_STAGE_RETURN_SUCCESS) - return ret; - if (!nm_device_hw_addr_set_cloned (device, nm_device_get_applied_connection (device), FALSE)) return NM_ACT_STAGE_RETURN_FAILURE; diff --git a/src/devices/nm-device-macvlan.c b/src/devices/nm-device-macvlan.c index ee8b997460..0a4635a852 100644 --- a/src/devices/nm-device-macvlan.c +++ b/src/devices/nm-device-macvlan.c @@ -418,12 +418,6 @@ update_connection (NMDevice *device, NMConnection *connection) static NMActStageReturn act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) { - NMActStageReturn ret; - - ret = NM_DEVICE_CLASS (nm_device_macvlan_parent_class)->act_stage1_prepare (device, out_failure_reason); - if (ret != NM_ACT_STAGE_RETURN_SUCCESS) - return ret; - if (!nm_device_hw_addr_set_cloned (device, nm_device_get_applied_connection (device), FALSE)) return NM_ACT_STAGE_RETURN_FAILURE; diff --git a/src/devices/nm-device-tun.c b/src/devices/nm-device-tun.c index 50099248b9..6d2a03036a 100644 --- a/src/devices/nm-device-tun.c +++ b/src/devices/nm-device-tun.c @@ -351,11 +351,6 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) { NMDeviceTun *self = NM_DEVICE_TUN (device); NMDeviceTunPrivate *priv = NM_DEVICE_TUN_GET_PRIVATE (self); - NMActStageReturn ret; - - ret = NM_DEVICE_CLASS (nm_device_tun_parent_class)->act_stage1_prepare (device, out_failure_reason); - if (ret != NM_ACT_STAGE_RETURN_SUCCESS) - return ret; if (priv->props.type == IFF_TUN) { /* Nothing to do for TUN devices */ diff --git a/src/devices/nm-device-vlan.c b/src/devices/nm-device-vlan.c index 27611499c9..616565de9c 100644 --- a/src/devices/nm-device-vlan.c +++ b/src/devices/nm-device-vlan.c @@ -477,11 +477,6 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) { NMDevice *parent_device; NMSettingVlan *s_vlan; - NMActStageReturn ret; - - ret = NM_DEVICE_CLASS (nm_device_vlan_parent_class)->act_stage1_prepare (device, out_failure_reason); - if (ret != NM_ACT_STAGE_RETURN_SUCCESS) - return ret; if (!nm_device_hw_addr_set_cloned (device, nm_device_get_applied_connection (device), FALSE)) return NM_ACT_STAGE_RETURN_FAILURE; diff --git a/src/devices/nm-device-vxlan.c b/src/devices/nm-device-vxlan.c index 1a700ba459..b2ccd247c2 100644 --- a/src/devices/nm-device-vxlan.c +++ b/src/devices/nm-device-vxlan.c @@ -484,12 +484,6 @@ update_connection (NMDevice *device, NMConnection *connection) static NMActStageReturn act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) { - NMActStageReturn ret; - - ret = NM_DEVICE_CLASS (nm_device_vxlan_parent_class)->act_stage1_prepare (device, out_failure_reason); - if (ret != NM_ACT_STAGE_RETURN_SUCCESS) - return ret; - if (!nm_device_hw_addr_set_cloned (device, nm_device_get_applied_connection (device), FALSE)) return NM_ACT_STAGE_RETURN_FAILURE; diff --git a/src/devices/nm-device-wpan.c b/src/devices/nm-device-wpan.c index 89301f3757..dc6b0e3b93 100644 --- a/src/devices/nm-device-wpan.c +++ b/src/devices/nm-device-wpan.c @@ -131,10 +131,6 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) NMDevice *lowpan_device = NULL; NMActStageReturn ret = NM_ACT_STAGE_RETURN_FAILURE; - ret = NM_DEVICE_CLASS (nm_device_wpan_parent_class)->act_stage1_prepare (device, out_failure_reason); - if (ret != NM_ACT_STAGE_RETURN_SUCCESS) - return ret; - platform = nm_device_get_platform (device); nm_assert (NM_IS_PLATFORM (platform)); diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index cbaeb81a88..084ba776ed 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -6449,12 +6449,6 @@ sriov_params_cb (GError *error, gpointer data) nm_device_activate_schedule_stage1_device_prepare (self); } -static NMActStageReturn -act_stage1_prepare (NMDevice *self, NMDeviceStateReason *out_failure_reason) -{ - return NM_ACT_STAGE_RETURN_SUCCESS; -} - /* * activate_stage1_device_prepare * @@ -6540,17 +6534,21 @@ activate_stage1_device_prepare (NMDevice *self) /* Assumed connections were already set up outside NetworkManager */ if (!nm_device_sys_iface_state_is_external_or_assume (self)) { - NMDeviceStateReason failure_reason = NM_DEVICE_STATE_REASON_NONE; + NMDeviceClass *klass = NM_DEVICE_GET_CLASS (self); - ret = NM_DEVICE_GET_CLASS (self)->act_stage1_prepare (self, &failure_reason); - if (ret == NM_ACT_STAGE_RETURN_FAILURE) { - nm_device_state_changed (self, NM_DEVICE_STATE_FAILED, failure_reason); - return; + if (klass->act_stage1_prepare) { + NMDeviceStateReason failure_reason = NM_DEVICE_STATE_REASON_NONE; + + ret = klass->act_stage1_prepare (self, &failure_reason); + if (ret == NM_ACT_STAGE_RETURN_FAILURE) { + nm_device_state_changed (self, NM_DEVICE_STATE_FAILED, failure_reason); + return; + } + if (ret == NM_ACT_STAGE_RETURN_POSTPONE) + return; + + nm_assert (ret == NM_ACT_STAGE_RETURN_SUCCESS); } - if (ret == NM_ACT_STAGE_RETURN_POSTPONE) - return; - - nm_assert (ret == NM_ACT_STAGE_RETURN_SUCCESS); } nm_device_activate_schedule_stage2_device_config (self); @@ -17179,7 +17177,6 @@ nm_device_class_init (NMDeviceClass *klass) klass->link_changed = link_changed; klass->is_available = is_available; - klass->act_stage1_prepare = act_stage1_prepare; klass->act_stage2_config = act_stage2_config; klass->act_stage3_ip_config_start = act_stage3_ip_config_start; klass->act_stage4_ip_config_timeout = act_stage4_ip_config_timeout; diff --git a/src/devices/team/nm-device-team.c b/src/devices/team/nm-device-team.c index fb176ed13e..73e288bac5 100644 --- a/src/devices/team/nm-device-team.c +++ b/src/devices/team/nm-device-team.c @@ -637,16 +637,11 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) { NMDeviceTeam *self = NM_DEVICE_TEAM (device); NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (self); - NMActStageReturn ret = NM_ACT_STAGE_RETURN_SUCCESS; gs_free_error GError *error = NULL; NMSettingTeam *s_team; NMConnection *connection; const char *cfg; - ret = NM_DEVICE_CLASS (nm_device_team_parent_class)->act_stage1_prepare (device, out_failure_reason); - if (ret != NM_ACT_STAGE_RETURN_SUCCESS) - return ret; - connection = nm_device_get_applied_connection (device); g_return_val_if_fail (connection, NM_ACT_STAGE_RETURN_FAILURE); diff --git a/src/devices/wifi/nm-device-iwd.c b/src/devices/wifi/nm-device-iwd.c index b97f439125..1b42aa9dd2 100644 --- a/src/devices/wifi/nm-device-iwd.c +++ b/src/devices/wifi/nm-device-iwd.c @@ -1714,7 +1714,6 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) { NMDeviceIwd *self = NM_DEVICE_IWD (device); NMDeviceIwdPrivate *priv = NM_DEVICE_IWD_GET_PRIVATE (self); - NMActStageReturn ret; NMWifiAP *ap = NULL; gs_unref_object NMWifiAP *ap_fake = NULL; NMActRequest *req; @@ -1723,10 +1722,6 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) const char *mode; const char *ap_path; - ret = NM_DEVICE_CLASS (nm_device_iwd_parent_class)->act_stage1_prepare (device, out_failure_reason); - if (ret != NM_ACT_STAGE_RETURN_SUCCESS) - return ret; - req = nm_device_get_act_request (device); g_return_val_if_fail (req, NM_ACT_STAGE_RETURN_FAILURE); diff --git a/src/devices/wifi/nm-device-olpc-mesh.c b/src/devices/wifi/nm-device-olpc-mesh.c index 3c865c5576..0a89e5a671 100644 --- a/src/devices/wifi/nm-device-olpc-mesh.c +++ b/src/devices/wifi/nm-device-olpc-mesh.c @@ -145,13 +145,8 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) { NMDeviceOlpcMesh *self = NM_DEVICE_OLPC_MESH (device); NMDeviceOlpcMeshPrivate *priv = NM_DEVICE_OLPC_MESH_GET_PRIVATE (self); - NMActStageReturn ret; gboolean scanning; - ret = NM_DEVICE_CLASS (nm_device_olpc_mesh_parent_class)->act_stage1_prepare (device, out_failure_reason); - if (ret != NM_ACT_STAGE_RETURN_SUCCESS) - return ret; - /* disconnect companion device, if it is connected */ if (nm_device_get_act_request (NM_DEVICE (priv->companion))) { _LOGI (LOGD_OLPC, "disconnecting companion device %s", diff --git a/src/devices/wifi/nm-device-wifi-p2p.c b/src/devices/wifi/nm-device-wifi-p2p.c index 4bfd3094fc..ef6df10243 100644 --- a/src/devices/wifi/nm-device-wifi-p2p.c +++ b/src/devices/wifi/nm-device-wifi-p2p.c @@ -368,15 +368,10 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) { NMDeviceWifiP2P *self = NM_DEVICE_WIFI_P2P (device); NMDeviceWifiP2PPrivate *priv = NM_DEVICE_WIFI_P2P_GET_PRIVATE (self); - NMActStageReturn ret; NMConnection *connection; NMSettingWifiP2P *s_wifi_p2p; NMWifiP2PPeer *peer; - ret = NM_DEVICE_CLASS (nm_device_wifi_p2p_parent_class)->act_stage1_prepare (device, out_failure_reason); - if (ret != NM_ACT_STAGE_RETURN_SUCCESS) - return ret; - if (!priv->mgmt_iface) { NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_SUPPLICANT_FAILED); return NM_ACT_STAGE_RETURN_FAILURE; diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 521d9e3c78..901cb0a542 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -2621,7 +2621,6 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) { NMDeviceWifi *self = NM_DEVICE_WIFI (device); NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); - NMActStageReturn ret; NMWifiAP *ap = NULL; gs_unref_object NMWifiAP *ap_fake = NULL; NMActRequest *req; @@ -2630,10 +2629,6 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) const char *mode; const char *ap_path; - ret = NM_DEVICE_CLASS (nm_device_wifi_parent_class)->act_stage1_prepare (device, out_failure_reason); - if (ret != NM_ACT_STAGE_RETURN_SUCCESS) - return ret; - req = nm_device_get_act_request (NM_DEVICE (self)); g_return_val_if_fail (req, NM_ACT_STAGE_RETURN_FAILURE); diff --git a/src/devices/wwan/nm-device-modem.c b/src/devices/wwan/nm-device-modem.c index 29c51162d0..be0993916d 100644 --- a/src/devices/wwan/nm-device-modem.c +++ b/src/devices/wwan/nm-device-modem.c @@ -583,13 +583,8 @@ deactivate_async (NMDevice *self, static NMActStageReturn act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) { - NMActStageReturn ret; NMActRequest *req; - ret = NM_DEVICE_CLASS (nm_device_modem_parent_class)->act_stage1_prepare (device, out_failure_reason); - if (ret != NM_ACT_STAGE_RETURN_SUCCESS) - return ret; - req = nm_device_get_act_request (device); g_return_val_if_fail (req, NM_ACT_STAGE_RETURN_FAILURE); From de439148dd5ee890d1879f48875c4624b1ec87d0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 22 Aug 2019 09:40:45 +0200 Subject: [PATCH 17/26] device: move redundant act_stage1_prepare() implementations to set hwaddr to NMDevice This is so common, that NMDevice can handle it for us. --- src/devices/nm-device-6lowpan.c | 10 +--------- src/devices/nm-device-dummy.c | 11 +---------- src/devices/nm-device-ip-tunnel.c | 11 +---------- src/devices/nm-device-macvlan.c | 11 +---------- src/devices/nm-device-vxlan.c | 11 +---------- src/devices/nm-device.c | 9 +++++++++ src/devices/nm-device.h | 3 +++ 7 files changed, 17 insertions(+), 49 deletions(-) diff --git a/src/devices/nm-device-6lowpan.c b/src/devices/nm-device-6lowpan.c index b63d0745c9..e877d30880 100644 --- a/src/devices/nm-device-6lowpan.c +++ b/src/devices/nm-device-6lowpan.c @@ -230,14 +230,6 @@ update_connection (NMDevice *device, NMConnection *connection) NULL); } -static NMActStageReturn -act_stage1_prepare (NMDevice *dev, NMDeviceStateReason *out_failure_reason) -{ - if (!nm_device_hw_addr_set_cloned (dev, nm_device_get_applied_connection (dev), FALSE)) - return NM_ACT_STAGE_RETURN_FAILURE; - return NM_ACT_STAGE_RETURN_SUCCESS; -} - /*****************************************************************************/ static void @@ -267,7 +259,7 @@ nm_device_6lowpan_class_init (NMDevice6LowpanClass *klass) device_class->connection_type_check_compatible = NM_SETTING_6LOWPAN_SETTING_NAME; device_class->link_types = NM_DEVICE_DEFINE_LINK_TYPES (NM_LINK_TYPE_6LOWPAN); - device_class->act_stage1_prepare = act_stage1_prepare; + device_class->act_stage1_prepare_set_hwaddr_ethernet = TRUE; device_class->complete_connection = complete_connection; device_class->create_and_realize = create_and_realize; device_class->get_generic_capabilities = get_generic_capabilities; diff --git a/src/devices/nm-device-dummy.c b/src/devices/nm-device-dummy.c index 75e8f742b6..7b00b615e2 100644 --- a/src/devices/nm-device-dummy.c +++ b/src/devices/nm-device-dummy.c @@ -116,15 +116,6 @@ create_and_realize (NMDevice *device, return TRUE; } -static NMActStageReturn -act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) -{ - if (!nm_device_hw_addr_set_cloned (device, nm_device_get_applied_connection (device), FALSE)) - return NM_ACT_STAGE_RETURN_FAILURE; - - return NM_ACT_STAGE_RETURN_SUCCESS; -} - /*****************************************************************************/ static void @@ -161,7 +152,7 @@ nm_device_dummy_class_init (NMDeviceDummyClass *klass) device_class->create_and_realize = create_and_realize; device_class->get_generic_capabilities = get_generic_capabilities; device_class->update_connection = update_connection; - device_class->act_stage1_prepare = act_stage1_prepare; + device_class->act_stage1_prepare_set_hwaddr_ethernet = TRUE; device_class->get_configured_mtu = nm_device_get_configured_mtu_for_wired; } diff --git a/src/devices/nm-device-ip-tunnel.c b/src/devices/nm-device-ip-tunnel.c index a490ea4cd2..ba73872132 100644 --- a/src/devices/nm-device-ip-tunnel.c +++ b/src/devices/nm-device-ip-tunnel.c @@ -965,15 +965,6 @@ set_property (GObject *object, guint prop_id, } } -static NMActStageReturn -act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) -{ - if (!nm_device_hw_addr_set_cloned (device, nm_device_get_applied_connection (device), FALSE)) - return NM_ACT_STAGE_RETURN_FAILURE; - - return NM_ACT_STAGE_RETURN_SUCCESS; -} - /*****************************************************************************/ static void @@ -1060,7 +1051,7 @@ nm_device_ip_tunnel_class_init (NMDeviceIPTunnelClass *klass) NM_LINK_TYPE_IPIP, NM_LINK_TYPE_SIT); - device_class->act_stage1_prepare = act_stage1_prepare; + device_class->act_stage1_prepare_set_hwaddr_ethernet = TRUE; device_class->link_changed = link_changed; device_class->can_reapply_change = can_reapply_change; device_class->complete_connection = complete_connection; diff --git a/src/devices/nm-device-macvlan.c b/src/devices/nm-device-macvlan.c index 0a4635a852..d20d647d67 100644 --- a/src/devices/nm-device-macvlan.c +++ b/src/devices/nm-device-macvlan.c @@ -415,15 +415,6 @@ update_connection (NMDevice *device, NMConnection *connection) NULL); } -static NMActStageReturn -act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) -{ - if (!nm_device_hw_addr_set_cloned (device, nm_device_get_applied_connection (device), FALSE)) - return NM_ACT_STAGE_RETURN_FAILURE; - - return NM_ACT_STAGE_RETURN_SUCCESS; -} - /*****************************************************************************/ static void @@ -502,7 +493,7 @@ nm_device_macvlan_class_init (NMDeviceMacvlanClass *klass) device_class->connection_type_check_compatible = NM_SETTING_MACVLAN_SETTING_NAME; device_class->link_types = NM_DEVICE_DEFINE_LINK_TYPES (NM_LINK_TYPE_MACVLAN, NM_LINK_TYPE_MACVTAP); - device_class->act_stage1_prepare = act_stage1_prepare; + device_class->act_stage1_prepare_set_hwaddr_ethernet = TRUE; device_class->check_connection_compatible = check_connection_compatible; device_class->complete_connection = complete_connection; device_class->create_and_realize = create_and_realize; diff --git a/src/devices/nm-device-vxlan.c b/src/devices/nm-device-vxlan.c index b2ccd247c2..535ae4a2e6 100644 --- a/src/devices/nm-device-vxlan.c +++ b/src/devices/nm-device-vxlan.c @@ -481,15 +481,6 @@ update_connection (NMDevice *device, NMConnection *connection) } } -static NMActStageReturn -act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) -{ - if (!nm_device_hw_addr_set_cloned (device, nm_device_get_applied_connection (device), FALSE)) - return NM_ACT_STAGE_RETURN_FAILURE; - - return NM_ACT_STAGE_RETURN_SUCCESS; -} - /*****************************************************************************/ static void @@ -614,7 +605,7 @@ nm_device_vxlan_class_init (NMDeviceVxlanClass *klass) device_class->complete_connection = complete_connection; device_class->get_generic_capabilities = get_generic_capabilities; device_class->update_connection = update_connection; - device_class->act_stage1_prepare = act_stage1_prepare; + device_class->act_stage1_prepare_set_hwaddr_ethernet = TRUE; device_class->get_configured_mtu = nm_device_get_configured_mtu_for_wired; obj_properties[PROP_ID] = diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 084ba776ed..1fa23fb05c 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -6536,6 +6536,15 @@ activate_stage1_device_prepare (NMDevice *self) if (!nm_device_sys_iface_state_is_external_or_assume (self)) { NMDeviceClass *klass = NM_DEVICE_GET_CLASS (self); + if (klass->act_stage1_prepare_set_hwaddr_ethernet) { + if (!nm_device_hw_addr_set_cloned (self, + nm_device_get_applied_connection (self), + FALSE)) { + nm_device_state_changed (self, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_CONFIG_FAILED); + return; + } + } + if (klass->act_stage1_prepare) { NMDeviceStateReason failure_reason = NM_DEVICE_STATE_REASON_NONE; diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h index ae6aab392a..bde106235b 100644 --- a/src/devices/nm-device.h +++ b/src/devices/nm-device.h @@ -470,6 +470,9 @@ typedef struct _NMDeviceClass { * a device or for external activations. In this case, act_stage2_config() must * take care not to touch the device's configuration. */ bool act_stage2_config_also_for_external_or_assume:1; + + bool act_stage1_prepare_set_hwaddr_ethernet:1; + } NMDeviceClass; typedef void (*NMDeviceAuthRequestFunc) (NMDevice *device, From 2d40b7ba61590c700eb146f2a07fb14751d82ab8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 22 Aug 2019 09:57:55 +0200 Subject: [PATCH 18/26] device: let NMDevice set hardware address instead of act_stage1_prepare() --- src/devices/nm-device-bridge.c | 6 +----- src/devices/nm-device-vlan.c | 4 +--- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/devices/nm-device-bridge.c b/src/devices/nm-device-bridge.c index 438f850315..4b53f4f325 100644 --- a/src/devices/nm-device-bridge.c +++ b/src/devices/nm-device-bridge.c @@ -507,11 +507,6 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) s_bridge = (NMSetting *) nm_connection_get_setting_bridge (connection); g_return_val_if_fail (s_bridge, NM_ACT_STAGE_RETURN_FAILURE); - if (!nm_device_hw_addr_set_cloned (device, connection, FALSE)) { - NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_CONFIG_FAILED); - return NM_ACT_STAGE_RETURN_FAILURE; - } - for (option = master_options; option->name; option++) commit_option (device, s_bridge, option, FALSE); @@ -758,6 +753,7 @@ nm_device_bridge_class_init (NMDeviceBridgeClass *klass) device_class->master_update_slave_connection = master_update_slave_connection; device_class->create_and_realize = create_and_realize; + device_class->act_stage1_prepare_set_hwaddr_ethernet = TRUE; device_class->act_stage1_prepare = act_stage1_prepare; device_class->act_stage2_config = act_stage2_config; device_class->deactivate = deactivate; diff --git a/src/devices/nm-device-vlan.c b/src/devices/nm-device-vlan.c index 616565de9c..14720b4b44 100644 --- a/src/devices/nm-device-vlan.c +++ b/src/devices/nm-device-vlan.c @@ -478,9 +478,6 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) NMDevice *parent_device; NMSettingVlan *s_vlan; - if (!nm_device_hw_addr_set_cloned (device, nm_device_get_applied_connection (device), FALSE)) - return NM_ACT_STAGE_RETURN_FAILURE; - /* Change MAC address to parent's one if needed */ parent_device = nm_device_parent_get_device (device); if (parent_device) { @@ -599,6 +596,7 @@ nm_device_vlan_class_init (NMDeviceVlanClass *klass) device_class->link_changed = link_changed; device_class->unrealize_notify = unrealize_notify; device_class->get_generic_capabilities = get_generic_capabilities; + device_class->act_stage1_prepare_set_hwaddr_ethernet = TRUE; device_class->act_stage1_prepare = act_stage1_prepare; device_class->get_configured_mtu = get_configured_mtu; device_class->is_available = is_available; From e034cc326470df3b2f192254195bbb753eadd4ac Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 22 Aug 2019 09:57:55 +0200 Subject: [PATCH 19/26] device: let NMDevice set hardware address instead of act_stage1_prepare() for NMDeviceEthernet There is a small change in the order of actions. Now we set the MAC address before calling link_negotiation_set(). That shouldn't make a difference. --- src/devices/nm-device-ethernet.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/devices/nm-device-ethernet.c b/src/devices/nm-device-ethernet.c index d5fd155181..b69669ca6f 100644 --- a/src/devices/nm-device-ethernet.c +++ b/src/devices/nm-device-ethernet.c @@ -864,9 +864,6 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) link_negotiation_set (device); - if (!nm_device_hw_addr_set_cloned (device, nm_device_get_applied_connection (device), FALSE)) - return NM_ACT_STAGE_RETURN_FAILURE; - /* If we're re-activating a PPPoE connection a short while after * a previous PPPoE connection was torn down, wait a bit to allow the * remote side to handle the disconnection. Otherwise the peer may @@ -1771,6 +1768,7 @@ nm_device_ethernet_class_init (NMDeviceEthernetClass *klass) device_class->new_default_connection = new_default_connection; device_class->act_stage1_prepare = act_stage1_prepare; + device_class->act_stage1_prepare_set_hwaddr_ethernet = TRUE; device_class->act_stage2_config = act_stage2_config; device_class->act_stage3_ip_config_start = act_stage3_ip_config_start; device_class->get_configured_mtu = get_configured_mtu; From 34895adcc450fca238818db668b28981c0590d43 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 22 Aug 2019 09:59:53 +0200 Subject: [PATCH 20/26] device: set failure reason when settings hardware address fails --- src/devices/nm-device-tun.c | 6 +++++- src/devices/wifi/nm-device-wifi.c | 4 +++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/devices/nm-device-tun.c b/src/devices/nm-device-tun.c index 6d2a03036a..6087ff65e6 100644 --- a/src/devices/nm-device-tun.c +++ b/src/devices/nm-device-tun.c @@ -355,8 +355,12 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) if (priv->props.type == IFF_TUN) { /* Nothing to do for TUN devices */ } else { - if (!nm_device_hw_addr_set_cloned (device, nm_device_get_applied_connection (device), FALSE)) + if (!nm_device_hw_addr_set_cloned (device, + nm_device_get_applied_connection (device), + FALSE)) { + *out_failure_reason = NM_DEVICE_STATE_REASON_CONFIG_FAILED; return NM_ACT_STAGE_RETURN_FAILURE; + } } return NM_ACT_STAGE_RETURN_SUCCESS; diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 901cb0a542..d9394ffdd5 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -2658,8 +2658,10 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) priv->hw_addr_scan_expire = 0; /* Set spoof MAC to the interface */ - if (!nm_device_hw_addr_set_cloned (device, connection, TRUE)) + if (!nm_device_hw_addr_set_cloned (device, connection, TRUE)) { + *out_failure_reason = NM_DEVICE_STATE_REASON_CONFIG_FAILED; return NM_ACT_STAGE_RETURN_FAILURE; + } /* AP and Mesh modes never use a specific object or existing scanned AP */ if (!NM_IN_SET (priv->mode, NM_802_11_MODE_AP, From efa3b5b44340b65b889f1bd437f3605dfa81f6e0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 22 Aug 2019 11:40:02 +0200 Subject: [PATCH 21/26] device/team: various cleanups --- src/devices/team/nm-device-team.c | 130 +++++++++++++++++------------- 1 file changed, 72 insertions(+), 58 deletions(-) diff --git a/src/devices/team/nm-device-team.c b/src/devices/team/nm-device-team.c index 73e288bac5..aeded7428d 100644 --- a/src/devices/team/nm-device-team.c +++ b/src/devices/team/nm-device-team.c @@ -49,14 +49,14 @@ NM_GOBJECT_PROPERTIES_DEFINE (NMDeviceTeam, typedef struct { struct teamdctl *tdc; + char *config; + NMConnection *connection; GPid teamd_pid; guint teamd_process_watch; guint teamd_timeout; guint teamd_read_timeout; guint teamd_dbus_watch; - char *config; - gboolean kill_in_progress; - NMConnection *connection; + bool kill_in_progress:1; } NMDeviceTeamPrivate; struct _NMDeviceTeam { @@ -74,7 +74,7 @@ G_DEFINE_TYPE (NMDeviceTeam, nm_device_team, NM_TYPE_DEVICE) /*****************************************************************************/ -static gboolean teamd_start (NMDevice *device, NMConnection *connection); +static gboolean teamd_start (NMDeviceTeam *self, NMConnection *connection); /*****************************************************************************/ @@ -141,9 +141,8 @@ _get_config (NMDeviceTeam *self) } static gboolean -teamd_read_config (NMDevice *device) +teamd_read_config (NMDeviceTeam *self) { - NMDeviceTeam *self = NM_DEVICE_TEAM (device); NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (self); const char *config = NULL; int err; @@ -170,11 +169,11 @@ teamd_read_config (NMDevice *device) static gboolean teamd_read_timeout_cb (gpointer user_data) { - NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (user_data); + NMDeviceTeam *self = user_data; + NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (self); - teamd_read_config ((NMDevice *) user_data); priv->teamd_read_timeout = 0; - + teamd_read_config (self); return G_SOURCE_REMOVE; } @@ -192,8 +191,9 @@ update_connection (NMDevice *device, NMConnection *connection) } /* Read the configuration only if not already set */ - if (!priv->config && ensure_teamd_connection (device)) - teamd_read_config (device); + if ( !priv->config + && ensure_teamd_connection (device)) + teamd_read_config (self); /* Restore previous tdc state */ if (priv->tdc && !tdc) { @@ -273,31 +273,33 @@ master_update_slave_connection (NMDevice *self, } /*****************************************************************************/ + static void teamd_kill_cb (pid_t pid, gboolean success, int child_status, void *user_data) { - NMDevice *device = NM_DEVICE (user_data); - NMDeviceTeam *self = (NMDeviceTeam *) device; + gs_unref_object NMDeviceTeam *self = user_data; NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (self); + gs_unref_object NMConnection *connection = NULL; priv->kill_in_progress = FALSE; - if (priv->connection) { - _LOGT (LOGD_TEAM, "kill terminated, starting teamd..."); - if (!teamd_start (device, priv->connection)) { - nm_device_state_changed (device, - NM_DEVICE_STATE_FAILED, - NM_DEVICE_STATE_REASON_TEAMD_CONTROL_FAILED); - } - g_clear_object (&priv->connection); + if (!priv->connection) + return; + + connection = g_steal_pointer (&priv->connection); + + _LOGT (LOGD_TEAM, "kill terminated, starting teamd..."); + if (!teamd_start (self, connection)) { + nm_device_state_changed (NM_DEVICE (self), + NM_DEVICE_STATE_FAILED, + NM_DEVICE_STATE_REASON_TEAMD_CONTROL_FAILED); } - g_object_unref (device); } static void -teamd_cleanup (NMDevice *device, gboolean free_tdc) +teamd_cleanup (NMDeviceTeam *self, gboolean free_tdc) { - NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (device); + NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (self); nm_clear_g_source (&priv->teamd_process_watch); nm_clear_g_source (&priv->teamd_timeout); @@ -305,15 +307,18 @@ teamd_cleanup (NMDevice *device, gboolean free_tdc) if (priv->teamd_pid > 0) { priv->kill_in_progress = TRUE; - nm_utils_kill_child_async (priv->teamd_pid, SIGTERM, - LOGD_TEAM, "teamd", + nm_utils_kill_child_async (priv->teamd_pid, + SIGTERM, + LOGD_TEAM, + "teamd", 2000, teamd_kill_cb, - g_object_ref (device)); + g_object_ref (self)); priv->teamd_pid = 0; } - if (priv->tdc && free_tdc) { + if ( priv->tdc + && free_tdc) { teamdctl_disconnect (priv->tdc); teamdctl_free (priv->tdc); priv->tdc = NULL; @@ -333,7 +338,7 @@ teamd_timeout_cb (gpointer user_data) if (priv->teamd_pid && !priv->tdc) { /* Timed out launching our own teamd process */ _LOGW (LOGD_TEAM, "teamd timed out"); - teamd_cleanup (device, TRUE); + teamd_cleanup (self, TRUE); g_warn_if_fail (nm_device_is_activating (device)); nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_TEAMD_CONTROL_FAILED); @@ -341,7 +346,7 @@ teamd_timeout_cb (gpointer user_data) /* Read again the configuration after the timeout since it might * have changed. */ - if (!teamd_read_config (device)) { + if (!teamd_read_config (self)) { _LOGW (LOGD_TEAM, "failed to read teamd configuration"); nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_TEAMD_CONTROL_FAILED); } @@ -388,7 +393,7 @@ teamd_dbus_appeared (GDBusConnection *connection, if (ret) { g_variant_get (ret, "(u)", &pid); if (pid != priv->teamd_pid) - teamd_cleanup (device, FALSE); + teamd_cleanup (self, FALSE); } else { _LOGW (LOGD_TEAM, "failed to determine D-Bus name owner"); /* If we can't determine the bus name owner, don't kill our @@ -403,16 +408,20 @@ teamd_dbus_appeared (GDBusConnection *connection, * device activation. */ success = ensure_teamd_connection (device); - if (nm_device_get_state (device) == NM_DEVICE_STATE_PREPARE) { - if (success) - success = teamd_read_config (device); - if (success) - nm_device_activate_schedule_stage2_device_config (device); - else if (!nm_device_sys_iface_state_is_external_or_assume (device)) { - teamd_cleanup (device, TRUE); + + if (nm_device_get_state (device) != NM_DEVICE_STATE_PREPARE) + return; + + if (!success) { + if (!nm_device_sys_iface_state_is_external_or_assume (device)) { + teamd_cleanup (self, TRUE); nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_TEAMD_CONTROL_FAILED); } + return; } + + if (teamd_read_config (self)) + nm_device_activate_schedule_stage2_device_config (device); } static void @@ -437,15 +446,19 @@ teamd_dbus_vanished (GDBusConnection *dbus_connection, } _LOGI (LOGD_TEAM, "teamd vanished from D-Bus"); - teamd_cleanup (device, TRUE); + teamd_cleanup (self, TRUE); /* Attempt to respawn teamd */ - if (state >= NM_DEVICE_STATE_PREPARE && state <= NM_DEVICE_STATE_ACTIVATED) { + if ( state >= NM_DEVICE_STATE_PREPARE + && state <= NM_DEVICE_STATE_ACTIVATED) { NMConnection *connection = nm_device_get_applied_connection (device); g_assert (connection); - if (!teamd_start (device, connection)) - nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_TEAMD_CONTROL_FAILED); + if (!teamd_start (self, connection)) { + nm_device_state_changed (device, + NM_DEVICE_STATE_FAILED, + NM_DEVICE_STATE_REASON_TEAMD_CONTROL_FAILED); + } } } @@ -470,7 +483,7 @@ teamd_process_watch_cb (GPid pid, int status, gpointer user_data) (state >= NM_DEVICE_STATE_PREPARE) && (state <= NM_DEVICE_STATE_ACTIVATED)) { _LOGW (LOGD_TEAM, "teamd process %lld quit unexpectedly; failing activation", (long long) pid); - teamd_cleanup (device, TRUE); + teamd_cleanup (self, TRUE); nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_TEAMD_CONTROL_FAILED); } } @@ -525,11 +538,10 @@ teamd_kill (NMDeviceTeam *self, const char *teamd_binary, GError **error) } static gboolean -teamd_start (NMDevice *device, NMConnection *connection) +teamd_start (NMDeviceTeam *self, NMConnection *connection) { - NMDeviceTeam *self = NM_DEVICE_TEAM (device); NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (self); - const char *iface = nm_device_get_ip_iface (device); + const char *iface = nm_device_get_ip_iface (NM_DEVICE (self)); gs_unref_ptrarray GPtrArray *argv = NULL; gs_free_error GError *error = NULL; gs_free char *tmp_str = NULL; @@ -553,7 +565,7 @@ teamd_start (NMDevice *device, NMConnection *connection) g_warn_if_reached (); if (!priv->teamd_pid) teamd_kill (self, teamd_binary, NULL); - teamd_cleanup (device, TRUE); + teamd_cleanup (self, TRUE); } /* Start teamd now */ @@ -568,7 +580,7 @@ teamd_start (NMDevice *device, NMConnection *connection) g_ptr_array_add (argv, (gpointer) iface); config = nm_setting_team_get_config (s_team); - if (!nm_device_hw_addr_get_cloned (device, connection, FALSE, &cloned_mac, NULL, &error)) { + if (!nm_device_hw_addr_get_cloned (NM_DEVICE (self), connection, FALSE, &cloned_mac, NULL, &error)) { _LOGW (LOGD_DEVICE, "set-hw-addr: %s", error->message); return FALSE; } @@ -615,18 +627,18 @@ teamd_start (NMDevice *device, NMConnection *connection) if (!g_spawn_async ("/", (char **) argv->pdata, (char **) envp, G_SPAWN_DO_NOT_REAP_CHILD, teamd_child_setup, NULL, &priv->teamd_pid, &error)) { _LOGW (LOGD_TEAM, "Activation: (team) failed to start teamd: %s", error->message); - teamd_cleanup (device, TRUE); + teamd_cleanup (self, TRUE); return FALSE; } /* Start a timeout for teamd to appear at D-Bus */ if (!priv->teamd_timeout) - priv->teamd_timeout = g_timeout_add_seconds (5, teamd_timeout_cb, device); + priv->teamd_timeout = g_timeout_add_seconds (5, teamd_timeout_cb, self); /* Monitor the child process so we know when it dies */ priv->teamd_process_watch = g_child_watch_add (priv->teamd_pid, teamd_process_watch_cb, - device); + self); _LOGI (LOGD_TEAM, "Activation: (team) started teamd [pid %u]...", (guint) priv->teamd_pid); return TRUE; @@ -671,7 +683,7 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) } _LOGD (LOGD_TEAM, "existing teamd config mismatch; respawning..."); - teamd_cleanup (device, TRUE); + teamd_cleanup (self, TRUE); } if (priv->kill_in_progress) { @@ -680,7 +692,7 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) return NM_ACT_STAGE_RETURN_POSTPONE; } - if (!teamd_start (device, connection)) + if (!teamd_start (self, connection)) return NM_ACT_STAGE_RETURN_FAILURE; return NM_ACT_STAGE_RETURN_POSTPONE; @@ -695,12 +707,14 @@ deactivate (NMDevice *device) if (nm_device_sys_iface_state_is_external (device)) return; - if (priv->teamd_pid || priv->tdc) + if ( priv->teamd_pid + || priv->tdc) _LOGI (LOGD_TEAM, "deactivation: stopping teamd..."); if (!priv->teamd_pid) teamd_kill (self, NULL, NULL); - teamd_cleanup (device, TRUE); + + teamd_cleanup (self, TRUE); g_clear_object (&priv->connection); } @@ -901,15 +915,15 @@ nm_device_team_new (const char *iface) static void dispose (GObject *object) { - NMDevice *device = NM_DEVICE (object); - NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (device); + NMDeviceTeam *self = NM_DEVICE_TEAM (object); + NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (self); if (priv->teamd_dbus_watch) { g_bus_unwatch_name (priv->teamd_dbus_watch); priv->teamd_dbus_watch = 0; } - teamd_cleanup (device, TRUE); + teamd_cleanup (self, TRUE); g_clear_pointer (&priv->config, g_free); G_OBJECT_CLASS (nm_device_team_parent_class)->dispose (object); From 51ddbda5d24eaa95f7e17c803f661d4369e38e2b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 22 Aug 2019 11:31:51 +0200 Subject: [PATCH 22/26] device/team: don't remember connection while killing team We don't need this. The applied-connection is already remembered and suitable. --- src/devices/team/nm-device-team.c | 40 ++++++++++++++----------------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/src/devices/team/nm-device-team.c b/src/devices/team/nm-device-team.c index aeded7428d..c02c308bd8 100644 --- a/src/devices/team/nm-device-team.c +++ b/src/devices/team/nm-device-team.c @@ -50,7 +50,6 @@ NM_GOBJECT_PROPERTIES_DEFINE (NMDeviceTeam, typedef struct { struct teamdctl *tdc; char *config; - NMConnection *connection; GPid teamd_pid; guint teamd_process_watch; guint teamd_timeout; @@ -74,7 +73,7 @@ G_DEFINE_TYPE (NMDeviceTeam, nm_device_team, NM_TYPE_DEVICE) /*****************************************************************************/ -static gboolean teamd_start (NMDeviceTeam *self, NMConnection *connection); +static gboolean teamd_start (NMDeviceTeam *self); /*****************************************************************************/ @@ -279,17 +278,16 @@ teamd_kill_cb (pid_t pid, gboolean success, int child_status, void *user_data) { gs_unref_object NMDeviceTeam *self = user_data; NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (self); - gs_unref_object NMConnection *connection = NULL; priv->kill_in_progress = FALSE; - if (!priv->connection) + if (nm_device_get_state (NM_DEVICE (self)) != NM_DEVICE_STATE_PREPARE) { + _LOGT (LOGD_TEAM, "kill terminated"); return; - - connection = g_steal_pointer (&priv->connection); + } _LOGT (LOGD_TEAM, "kill terminated, starting teamd..."); - if (!teamd_start (self, connection)) { + if (!teamd_start (self)) { nm_device_state_changed (NM_DEVICE (self), NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_TEAMD_CONTROL_FAILED); @@ -451,10 +449,7 @@ teamd_dbus_vanished (GDBusConnection *dbus_connection, /* Attempt to respawn teamd */ if ( state >= NM_DEVICE_STATE_PREPARE && state <= NM_DEVICE_STATE_ACTIVATED) { - NMConnection *connection = nm_device_get_applied_connection (device); - - g_assert (connection); - if (!teamd_start (self, connection)) { + if (!teamd_start (self)) { nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_TEAMD_CONTROL_FAILED); @@ -538,10 +533,11 @@ teamd_kill (NMDeviceTeam *self, const char *teamd_binary, GError **error) } static gboolean -teamd_start (NMDeviceTeam *self, NMConnection *connection) +teamd_start (NMDeviceTeam *self) { NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (self); const char *iface = nm_device_get_ip_iface (NM_DEVICE (self)); + NMConnection *connection; gs_unref_ptrarray GPtrArray *argv = NULL; gs_free_error GError *error = NULL; gs_free char *tmp_str = NULL; @@ -552,8 +548,13 @@ teamd_start (NMDeviceTeam *self, NMConnection *connection) gs_free char *cloned_mac = NULL; gs_free const char **envp = NULL; + connection = nm_device_get_applied_connection (NM_DEVICE (self)); + s_team = nm_connection_get_setting_team (connection); - g_return_val_if_fail (s_team, FALSE); + if (!s_team) + g_return_val_if_reached (FALSE); + + nm_assert (iface); teamd_binary = nm_utils_find_helper ("teamd", NULL, NULL); if (!teamd_binary) { @@ -651,14 +652,11 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (self); gs_free_error GError *error = NULL; NMSettingTeam *s_team; - NMConnection *connection; const char *cfg; - connection = nm_device_get_applied_connection (device); - g_return_val_if_fail (connection, NM_ACT_STAGE_RETURN_FAILURE); - - s_team = nm_connection_get_setting_team (connection); - g_return_val_if_fail (s_team, NM_ACT_STAGE_RETURN_FAILURE); + s_team = nm_device_get_applied_setting (device, NM_TYPE_SETTING_TEAM); + if (!s_team) + g_return_val_if_reached (NM_ACT_STAGE_RETURN_FAILURE); if (priv->tdc) { /* If the existing teamd config is the same as we're about to use, @@ -688,11 +686,10 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) if (priv->kill_in_progress) { _LOGT (LOGD_TEAM, "kill in progress, wait before starting teamd"); - priv->connection = g_object_ref (connection); return NM_ACT_STAGE_RETURN_POSTPONE; } - if (!teamd_start (self, connection)) + if (!teamd_start (self)) return NM_ACT_STAGE_RETURN_FAILURE; return NM_ACT_STAGE_RETURN_POSTPONE; @@ -715,7 +712,6 @@ deactivate (NMDevice *device) teamd_kill (self, NULL, NULL); teamd_cleanup (self, TRUE); - g_clear_object (&priv->connection); } static gboolean From 86f8f5a71c52af36e8e8d73e27612d6588d828c3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 22 Aug 2019 12:51:12 +0200 Subject: [PATCH 23/26] device/wifi-p2p: inline and drop local function cleanup_p2p_connect_attempt() It has only one caller. It's clearer to do the cleanup right there. --- src/devices/wifi/nm-device-wifi-p2p.c | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/src/devices/wifi/nm-device-wifi-p2p.c b/src/devices/wifi/nm-device-wifi-p2p.c index ef6df10243..fe2f7bc4d8 100644 --- a/src/devices/wifi/nm-device-wifi-p2p.c +++ b/src/devices/wifi/nm-device-wifi-p2p.c @@ -399,21 +399,6 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) return NM_ACT_STAGE_RETURN_SUCCESS; } -static void -cleanup_p2p_connect_attempt (NMDeviceWifiP2P *self, gboolean disconnect) -{ - NMDeviceWifiP2PPrivate *priv = NM_DEVICE_WIFI_P2P_GET_PRIVATE (self); - - nm_clear_g_source (&priv->sup_timeout_id); - nm_clear_g_source (&priv->peer_missing_id); - - if (priv->mgmt_iface) - nm_supplicant_interface_p2p_cancel_connect (priv->mgmt_iface); - - if (disconnect && priv->group_iface) - nm_supplicant_interface_p2p_disconnect (priv->group_iface); -} - /* * supplicant_connection_timeout_cb * @@ -609,9 +594,17 @@ static void deactivate (NMDevice *device) { NMDeviceWifiP2P *self = NM_DEVICE_WIFI_P2P (device); + NMDeviceWifiP2PPrivate *priv = NM_DEVICE_WIFI_P2P_GET_PRIVATE (self); int ifindex = nm_device_get_ip_ifindex (device); - cleanup_p2p_connect_attempt (self, TRUE); + nm_clear_g_source (&priv->sup_timeout_id); + nm_clear_g_source (&priv->peer_missing_id); + + if (priv->mgmt_iface) + nm_supplicant_interface_p2p_cancel_connect (priv->mgmt_iface); + + if (priv->group_iface) + nm_supplicant_interface_p2p_disconnect (priv->group_iface); /* Clear any critical protocol notification in the Wi-Fi stack */ if (ifindex > 0) From 29562a9751d97eb56bb5b56f3d36a96345559a5f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 22 Aug 2019 10:43:22 +0200 Subject: [PATCH 24/26] device: let devices call stage1 again after being ready to proceed I am about to change the when stage1 gets postponed, then the way to proceed it is to schedule stage1 again (instead of scheduling stage2). The reason is that stage1 handling should be reentrant and we should keep entering it until there is no more reason to postpone it. If a subclass postpones stage1 and then later progresses it by directly scheduling stage2, then only the subclass is in control over postponing stage 2. Instead, anybody should be able to delay stage2 independently. That can only work if everybody signals readyness to proceed by scheduling stage1 again. --- src/devices/nm-device-ethernet.c | 9 +++-- src/devices/nm-device-macsec.c | 6 ++- src/devices/nm-device-wireguard.c | 6 ++- src/devices/team/nm-device-team.c | 26 +++++++++---- src/devices/wifi/nm-device-olpc-mesh.c | 6 +-- src/devices/wifi/nm-device-wifi-p2p.c | 36 +++++++++++------- src/devices/wifi/nm-device-wifi.c | 6 ++- src/devices/wwan/nm-device-modem.c | 52 ++++++++++++++++++-------- 8 files changed, 99 insertions(+), 48 deletions(-) diff --git a/src/devices/nm-device-ethernet.c b/src/devices/nm-device-ethernet.c index b69669ca6f..360e4d1714 100644 --- a/src/devices/nm-device-ethernet.c +++ b/src/devices/nm-device-ethernet.c @@ -454,8 +454,10 @@ wired_secrets_cb (NMActRequest *req, nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_NO_SECRETS); - } else - nm_device_activate_schedule_stage1_device_prepare (device); + return; + } + + nm_device_activate_schedule_stage1_device_prepare (device); } static void @@ -851,8 +853,9 @@ pppoe_reconnect_delay (gpointer user_data) NMDeviceEthernetPrivate *priv = NM_DEVICE_ETHERNET_GET_PRIVATE (self); priv->pppoe_wait_id = 0; + priv->last_pppoe_time = 0; _LOGI (LOGD_DEVICE, "PPPoE reconnect delay complete, resuming connection..."); - nm_device_activate_schedule_stage2_device_config (NM_DEVICE (self)); + nm_device_activate_schedule_stage1_device_prepare (NM_DEVICE (self)); return G_SOURCE_REMOVE; } diff --git a/src/devices/nm-device-macsec.c b/src/devices/nm-device-macsec.c index e3e3a895b2..c4911eca28 100644 --- a/src/devices/nm-device-macsec.c +++ b/src/devices/nm-device-macsec.c @@ -308,8 +308,10 @@ macsec_secrets_cb (NMActRequest *req, nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_NO_SECRETS); - } else - nm_device_activate_schedule_stage1_device_prepare (device); + return; + } + + nm_device_activate_schedule_stage1_device_prepare (device); } static void diff --git a/src/devices/nm-device-wireguard.c b/src/devices/nm-device-wireguard.c index 572389b114..e64f33b560 100644 --- a/src/devices/nm-device-wireguard.c +++ b/src/devices/nm-device-wireguard.c @@ -1276,8 +1276,10 @@ _secrets_cb (NMActRequest *req, nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_NO_SECRETS); - } else - nm_device_activate_schedule_stage1_device_prepare (device); + return; + } + + nm_device_activate_schedule_stage1_device_prepare (device); } static void diff --git a/src/devices/team/nm-device-team.c b/src/devices/team/nm-device-team.c index c02c308bd8..bea756be35 100644 --- a/src/devices/team/nm-device-team.c +++ b/src/devices/team/nm-device-team.c @@ -56,6 +56,7 @@ typedef struct { guint teamd_read_timeout; guint teamd_dbus_watch; bool kill_in_progress:1; + NMDeviceStageState stage1_state:3; } NMDeviceTeamPrivate; struct _NMDeviceTeam { @@ -407,19 +408,20 @@ teamd_dbus_appeared (GDBusConnection *connection, */ success = ensure_teamd_connection (device); - if (nm_device_get_state (device) != NM_DEVICE_STATE_PREPARE) + if ( nm_device_get_state (device) != NM_DEVICE_STATE_PREPARE + || priv->stage1_state != NM_DEVICE_STAGE_STATE_PENDING) return; + if (success) + success = teamd_read_config (self); + if (!success) { - if (!nm_device_sys_iface_state_is_external_or_assume (device)) { - teamd_cleanup (self, TRUE); - nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_TEAMD_CONTROL_FAILED); - } + nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_TEAMD_CONTROL_FAILED); return; } - if (teamd_read_config (self)) - nm_device_activate_schedule_stage2_device_config (device); + priv->stage1_state = NM_DEVICE_STAGE_STATE_COMPLETED; + nm_device_activate_schedule_stage1_device_prepare (device); } static void @@ -658,6 +660,14 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) if (!s_team) g_return_val_if_reached (NM_ACT_STAGE_RETURN_FAILURE); + if (priv->stage1_state == NM_DEVICE_STAGE_STATE_PENDING) + return NM_ACT_STAGE_RETURN_POSTPONE; + + if (priv->stage1_state == NM_DEVICE_STAGE_STATE_COMPLETED) + return NM_ACT_STAGE_RETURN_SUCCESS; + + priv->stage1_state = NM_DEVICE_STAGE_STATE_PENDING; + if (priv->tdc) { /* If the existing teamd config is the same as we're about to use, * then we can proceed. If it's not the same, and we have a PID, @@ -701,6 +711,8 @@ deactivate (NMDevice *device) NMDeviceTeam *self = NM_DEVICE_TEAM (device); NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (self); + priv->stage1_state = NM_DEVICE_STAGE_STATE_INIT; + if (nm_device_sys_iface_state_is_external (device)) return; diff --git a/src/devices/wifi/nm-device-olpc-mesh.c b/src/devices/wifi/nm-device-olpc-mesh.c index 0a89e5a671..3551a9e486 100644 --- a/src/devices/wifi/nm-device-olpc-mesh.c +++ b/src/devices/wifi/nm-device-olpc-mesh.c @@ -58,7 +58,7 @@ NM_GOBJECT_PROPERTIES_DEFINE (NMDeviceOlpcMesh, typedef struct { NMDevice *companion; NMManager *manager; - gboolean stage1_waiting; + bool stage1_waiting:1; } NMDeviceOlpcMeshPrivate; struct _NMDeviceOlpcMesh { @@ -166,6 +166,7 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) return NM_ACT_STAGE_RETURN_POSTPONE; } + priv->stage1_waiting = FALSE; return NM_ACT_STAGE_RETURN_SUCCESS; } @@ -249,10 +250,9 @@ companion_notify_cb (NMDeviceWifi *companion, GParamSpec *pspec, gpointer user_d return; g_object_get (companion, NM_DEVICE_WIFI_SCANNING, &scanning, NULL); - if (!scanning) { priv->stage1_waiting = FALSE; - nm_device_activate_schedule_stage2_device_config (NM_DEVICE (self)); + nm_device_activate_schedule_stage1_device_prepare (NM_DEVICE (self)); } } diff --git a/src/devices/wifi/nm-device-wifi-p2p.c b/src/devices/wifi/nm-device-wifi-p2p.c index fe2f7bc4d8..5e75e26022 100644 --- a/src/devices/wifi/nm-device-wifi-p2p.c +++ b/src/devices/wifi/nm-device-wifi-p2p.c @@ -63,6 +63,7 @@ typedef struct { CList peers_lst_head; + guint find_peer_timeout_id; guint sup_timeout_id; guint peer_dump_id; guint peer_missing_id; @@ -349,7 +350,7 @@ supplicant_find_timeout_cb (gpointer user_data) NMDeviceWifiP2P *self = NM_DEVICE_WIFI_P2P (user_data); NMDeviceWifiP2PPrivate *priv = NM_DEVICE_WIFI_P2P_GET_PRIVATE (self); - priv->sup_timeout_id = 0; + priv->find_peer_timeout_id = 0; nm_supplicant_interface_p2p_cancel_connect (priv->mgmt_iface); @@ -386,10 +387,10 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) peer = nm_wifi_p2p_peers_find_first_compatible (&priv->peers_lst_head, connection); if (!peer) { /* Set up a timeout on the find attempt and run a find for the same period of time */ - if (priv->sup_timeout_id == 0) { - priv->sup_timeout_id = g_timeout_add_seconds (10, - supplicant_find_timeout_cb, - self); + if (priv->find_peer_timeout_id == 0) { + priv->find_peer_timeout_id = g_timeout_add_seconds (10, + supplicant_find_timeout_cb, + self); nm_supplicant_interface_p2p_start_find (priv->mgmt_iface, 10); } @@ -436,7 +437,8 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) NMWifiP2PPeer *peer; GBytes *wfd_ies; - nm_clear_g_source (&priv->sup_timeout_id); + if (nm_clear_g_source (&priv->find_peer_timeout_id)) + nm_assert_not_reached (); if (!priv->mgmt_iface) { NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_SUPPLICANT_FAILED); @@ -468,9 +470,11 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) "pbc", NULL); /* Set up a timeout on the connect attempt */ - priv->sup_timeout_id = g_timeout_add_seconds (45, - supplicant_connection_timeout_cb, - self); + if (priv->sup_timeout_id == 0) { + priv->sup_timeout_id = g_timeout_add_seconds (45, + supplicant_connection_timeout_cb, + self); + } /* We'll get stage3 started when the P2P group has been started */ return NM_ACT_STAGE_RETURN_POSTPONE; @@ -525,17 +529,19 @@ peer_add_remove (NMDeviceWifiP2P *self, if (is_adding) { /* If we are in prepare state, then we are currently runnign a find * to search for the requested peer. */ - if (nm_device_get_state (device) == NM_DEVICE_STATE_PREPARE) { + if (priv->find_peer_timeout_id != 0) { NMConnection *connection; + nm_assert (nm_device_get_state (device) == NM_DEVICE_STATE_PREPARE); + connection = nm_device_get_applied_connection (device); - g_assert (connection); + nm_assert (NM_IS_CONNECTION (connection)); peer = nm_wifi_p2p_peers_find_first_compatible (&priv->peers_lst_head, connection); if (peer) { /* A peer for the connection was found, cancel the timeout and go to configure state. */ - nm_clear_g_source (&priv->sup_timeout_id); - nm_device_activate_schedule_stage2_device_config (device); + nm_clear_g_source (&priv->find_peer_timeout_id); + nm_device_activate_schedule_stage1_device_prepare (device); } } @@ -594,9 +600,10 @@ static void deactivate (NMDevice *device) { NMDeviceWifiP2P *self = NM_DEVICE_WIFI_P2P (device); - NMDeviceWifiP2PPrivate *priv = NM_DEVICE_WIFI_P2P_GET_PRIVATE (self); int ifindex = nm_device_get_ip_ifindex (device); + NMDeviceWifiP2PPrivate *priv = NM_DEVICE_WIFI_P2P_GET_PRIVATE (self); + nm_clear_g_source (&priv->find_peer_timeout_id); nm_clear_g_source (&priv->sup_timeout_id); nm_clear_g_source (&priv->peer_missing_id); @@ -902,6 +909,7 @@ supplicant_interfaces_release (NMDeviceWifiP2P *self, gboolean set_is_waiting) nm_supplicant_manager_set_wfd_ies (priv->sup_mgr, NULL); g_signal_handlers_disconnect_by_data (priv->mgmt_iface, self); g_clear_object (&priv->mgmt_iface); + nm_clear_g_source (&priv->find_peer_timeout_id); nm_clear_g_source (&priv->sup_timeout_id); } diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index d9394ffdd5..28e8bbefb6 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -1735,8 +1735,10 @@ wifi_secrets_cb (NMActRequest *req, nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_NO_SECRETS); - } else - nm_device_activate_schedule_stage1_device_prepare (device); + return; + } + + nm_device_activate_schedule_stage1_device_prepare (device); } static void diff --git a/src/devices/wwan/nm-device-modem.c b/src/devices/wwan/nm-device-modem.c index be0993916d..7ba664394a 100644 --- a/src/devices/wwan/nm-device-modem.c +++ b/src/devices/wwan/nm-device-modem.c @@ -48,10 +48,11 @@ typedef struct { NMModem *modem; NMDeviceModemCapabilities caps; NMDeviceModemCapabilities current_caps; - gboolean rf_enabled; char *device_id; char *operator_code; char *apn; + bool rf_enabled:1; + NMDeviceStageState stage1_state:3; } NMDeviceModemPrivate; struct _NMDeviceModem { @@ -119,16 +120,17 @@ modem_prepare_result (NMModem *modem, gpointer user_data) { NMDeviceModem *self = NM_DEVICE_MODEM (user_data); + NMDeviceModemPrivate *priv = NM_DEVICE_MODEM_GET_PRIVATE (self); NMDevice *device = NM_DEVICE (self); - NMDeviceState state; NMDeviceStateReason reason = i_reason; - state = nm_device_get_state (device); - g_return_if_fail (state == NM_DEVICE_STATE_PREPARE); + if ( nm_device_get_state (device) != NM_DEVICE_STATE_PREPARE + || priv->stage1_state != NM_DEVICE_STAGE_STATE_PENDING) { + nm_assert_not_reached (); + success = FALSE; + } - if (success) - nm_device_activate_schedule_stage2_device_config (device); - else { + if (!success) { /* There are several reasons to block autoconnection at device level: * * - Wrong SIM-PIN: The device won't autoconnect because it doesn't make sense @@ -164,7 +166,11 @@ modem_prepare_result (NMModem *modem, break; } nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, reason); + return; } + + priv->stage1_state = NM_DEVICE_STAGE_STATE_COMPLETED; + nm_device_activate_schedule_stage1_device_prepare (device); } static void @@ -187,16 +193,19 @@ static void modem_auth_result (NMModem *modem, GError *error, gpointer user_data) { NMDevice *device = NM_DEVICE (user_data); + NMDeviceModemPrivate *priv = NM_DEVICE_MODEM_GET_PRIVATE (device); + + g_return_if_fail (nm_device_get_state (device) == NM_DEVICE_STATE_NEED_AUTH); if (error) { nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_NO_SECRETS); - } else { - /* Otherwise, on success for modem secrets we need to schedule stage1 again */ - g_return_if_fail (nm_device_get_state (device) == NM_DEVICE_STATE_NEED_AUTH); - nm_device_activate_schedule_stage1_device_prepare (device); + return; } + + priv->stage1_state = NM_DEVICE_STAGE_STATE_INIT; + nm_device_activate_schedule_stage1_device_prepare (device); } static void @@ -542,7 +551,10 @@ complete_connection (NMDevice *device, static void deactivate (NMDevice *device) { - nm_modem_deactivate (NM_DEVICE_MODEM_GET_PRIVATE (device)->modem, device); + NMDeviceModemPrivate *priv = NM_DEVICE_MODEM_GET_PRIVATE (device); + + nm_modem_deactivate (priv->modem, device); + priv->stage1_state = NM_DEVICE_STAGE_STATE_INIT; } /*****************************************************************************/ @@ -583,14 +595,24 @@ deactivate_async (NMDevice *self, static NMActStageReturn act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) { + NMDeviceModemPrivate *priv = NM_DEVICE_MODEM_GET_PRIVATE (device); NMActRequest *req; req = nm_device_get_act_request (device); g_return_val_if_fail (req, NM_ACT_STAGE_RETURN_FAILURE); - return nm_modem_act_stage1_prepare (NM_DEVICE_MODEM_GET_PRIVATE (device)->modem, - req, - out_failure_reason); + if (priv->stage1_state == NM_DEVICE_STAGE_STATE_INIT) { + priv->stage1_state = NM_DEVICE_STAGE_STATE_PENDING; + return nm_modem_act_stage1_prepare (NM_DEVICE_MODEM_GET_PRIVATE (device)->modem, + req, + out_failure_reason); + } + + if (priv->stage1_state == NM_DEVICE_STAGE_STATE_PENDING) + return NM_ACT_STAGE_RETURN_POSTPONE; + + nm_assert (priv->stage1_state == NM_DEVICE_STAGE_STATE_COMPLETED); + return NM_ACT_STAGE_RETURN_SUCCESS; } static NMActStageReturn From 5b677d5a3bed37ea89c1f89333d0d8a25ff91fda Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 22 Aug 2019 13:21:38 +0200 Subject: [PATCH 25/26] device: move check for master from nm_device_activate_schedule_stage2_device_config() to end of stage1 Note that by now no callers of nm_device_activate_schedule_stage2_device_config() are left. All previous callers now re-schedule stage1 instead of directly scheduling stage2. Note that if stage2 later also gets re-factored to re-enter itself instead of scheduling stage3 right away, the function will be used again. That means, we can move the check for the master where it belongs: as part (and at the end of) stage1. Also, slightly simplify the code. The handler master_ready_cb() no longer directly calls master_ready(). It's enough to always enter stage1 again. Also drop master_ready_handled. We don't need to remember that this condition was satsified. We can just check it always when we reach the place in activate_stage1_device_prepare(). --- src/devices/nm-device.c | 91 ++++++++++++++++++----------------------- 1 file changed, 39 insertions(+), 52 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 1fa23fb05c..5e6607d9b1 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -404,7 +404,6 @@ typedef struct _NMDevicePrivate { NMDeviceAutoconnectBlockedFlags autoconnect_blocked_flags:5; bool is_enslaved:1; - bool master_ready_handled:1; bool ipv6ll_handle:1; /* TRUE if NM handles the device's IPv6LL address */ bool ipv6ll_has:1; @@ -6268,22 +6267,18 @@ master_ready (NMDevice *self, NMActiveConnection *master_connection; NMDevice *master; - g_return_if_fail (priv->state == NM_DEVICE_STATE_PREPARE); - g_return_if_fail (!priv->master_ready_handled); - /* Notify a master device that it has a new slave */ - g_return_if_fail (nm_active_connection_get_master_ready (active)); - master_connection = nm_active_connection_get_master (active); + nm_assert (nm_active_connection_get_master_ready (active)); - priv->master_ready_handled = TRUE; - nm_clear_g_signal_handler (active, &priv->master_ready_id); + master_connection = nm_active_connection_get_master (active); master = nm_active_connection_get_device (master_connection); _LOGD (LOGD_DEVICE, "master connection ready; master device %s", nm_device_get_iface (master)); - if (priv->master && priv->master != master) + if ( priv->master + && priv->master != master) nm_device_master_release_one_slave (priv->master, self, FALSE, NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED); /* If the master didn't change, add-slave only rechecks whether to assume a connection. */ @@ -6297,8 +6292,12 @@ master_ready_cb (NMActiveConnection *active, GParamSpec *pspec, NMDevice *self) { - master_ready (self, active); - nm_device_activate_schedule_stage2_device_config (self); + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + + nm_assert (nm_active_connection_get_master_ready (active)); + + if (priv->state == NM_DEVICE_STATE_PREPARE) + nm_device_activate_schedule_stage1_device_prepare (self); } static void @@ -6460,6 +6459,8 @@ activate_stage1_device_prepare (NMDevice *self) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); NMActStageReturn ret = NM_ACT_STAGE_RETURN_SUCCESS; + NMActiveConnection *active; + NMActiveConnection *master; priv->v4_route_table_initialized = FALSE; priv->v6_route_table_initialized = FALSE; @@ -6560,6 +6561,30 @@ activate_stage1_device_prepare (NMDevice *self) } } + active = NM_ACTIVE_CONNECTION (priv->act_request.obj); + master = nm_active_connection_get_master (active); + if (master) { + /* If the master connection is ready for slaves, attach ourselves */ + if (!nm_active_connection_get_master_ready (active)) { + if (nm_active_connection_get_state (master) >= NM_ACTIVE_CONNECTION_STATE_DEACTIVATING) { + _LOGD (LOGD_DEVICE, "master connection is deactivating"); + nm_device_state_changed (self, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_DEPENDENCY_FAILED); + return; + } + if (priv->master_ready_id == 0) { + _LOGD (LOGD_DEVICE, "waiting for master connection to become ready"); + priv->master_ready_id = g_signal_connect (active, + "notify::" NM_ACTIVE_CONNECTION_INT_MASTER_READY, + (GCallback) master_ready_cb, + self); + } + return; + } + } + nm_clear_g_signal_handler (priv->act_request.obj, &priv->master_ready_id); + if (master) + master_ready (self, active); + nm_device_activate_schedule_stage2_device_config (self); } @@ -6951,44 +6976,8 @@ activate_stage2_device_config (NMDevice *self) void nm_device_activate_schedule_stage2_device_config (NMDevice *self) { - NMDevicePrivate *priv; - g_return_if_fail (NM_IS_DEVICE (self)); - priv = NM_DEVICE_GET_PRIVATE (self); - g_return_if_fail (priv->act_request.obj); - - if (!priv->master_ready_handled) { - NMActiveConnection *active = NM_ACTIVE_CONNECTION (priv->act_request.obj); - NMActiveConnection *master; - - master = nm_active_connection_get_master (active); - - if (!master) { - g_warn_if_fail (!priv->master_ready_id); - priv->master_ready_handled = TRUE; - } else { - /* If the master connection is ready for slaves, attach ourselves */ - if (nm_active_connection_get_master_ready (active)) - master_ready (self, active); - else if (nm_active_connection_get_state (master) >= NM_ACTIVE_CONNECTION_STATE_DEACTIVATING) { - _LOGD (LOGD_DEVICE, "master connection is deactivating"); - nm_device_state_changed (self, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_DEPENDENCY_FAILED); - } else { - _LOGD (LOGD_DEVICE, "waiting for master connection to become ready"); - - if (priv->master_ready_id == 0) { - priv->master_ready_id = g_signal_connect (active, - "notify::" NM_ACTIVE_CONNECTION_INT_MASTER_READY, - (GCallback) master_ready_cb, - self); - } - /* Postpone */ - return; - } - } - } - activation_source_schedule (self, activate_stage2_device_config, AF_INET); } @@ -14717,10 +14706,7 @@ _cleanup_generic_post (NMDevice *self, CleanupType cleanup_type) if (priv->act_request.obj) { nm_active_connection_set_default (NM_ACTIVE_CONNECTION (priv->act_request.obj), AF_INET, FALSE); - - priv->master_ready_handled = FALSE; nm_clear_g_signal_handler (priv->act_request.obj, &priv->master_ready_id); - act_request_set (self, NULL); } @@ -16663,14 +16649,15 @@ dispose (GObject *object) _cleanup_generic_pre (self, CLEANUP_TYPE_KEEP); - g_warn_if_fail (c_list_is_empty (&priv->slaves)); - g_assert (priv->master_ready_id == 0); + nm_assert (c_list_is_empty (&priv->slaves)); /* Let the kernel manage IPv6LL again */ set_nm_ipv6ll (self, FALSE); _cleanup_generic_post (self, CLEANUP_TYPE_KEEP); + nm_assert (priv->master_ready_id == 0); + g_hash_table_remove_all (priv->ip6_saved_properties); nm_clear_g_source (&priv->recheck_assume_id); From 79952b629636a0b8ff70e88c3ea23824ed2cea15 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 22 Aug 2019 13:36:29 +0200 Subject: [PATCH 26/26] device: after stage1 call stage2 synchronously We know we are ready and in a situation where we can handle state changes. Don't schedule stage2 in an idle handler, just invoke it directly. --- src/devices/nm-device.c | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 5e6607d9b1..5262670886 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -683,6 +683,8 @@ 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); @@ -6257,6 +6259,36 @@ activation_source_schedule (NMDevice *self, ActivationHandleFunc func, int addr_ priv->activation_source_id_x[IS_IPv4] = new_id; } +static void +activation_source_invoke_sync (NMDevice *self, ActivationHandleFunc func, int addr_family) +{ + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + const gboolean IS_IPv4 = (addr_family == AF_INET); + + if (priv->activation_source_id_x[IS_IPv4] == 0) { + _LOGD (LOGD_DEVICE, "activation-stage: synchronously invoke %s,v%c", + _activation_func_to_string (func), + nm_utils_addr_family_to_char (addr_family)); + } else if (priv->activation_source_func_x[IS_IPv4] == func) { + _LOGD (LOGD_DEVICE, "activation-stage: synchronously invoke %s,v%c which was already scheduled (id %u)", + _activation_func_to_string (func), + nm_utils_addr_family_to_char (addr_family), + priv->activation_source_id_x[IS_IPv4]); + } else { + _LOGD (LOGD_DEVICE, "activation-stage: synchronously invoke %s,v%c which replaces %s,v%c (id %u)", + _activation_func_to_string (func), + nm_utils_addr_family_to_char (addr_family), + _activation_func_to_string (priv->activation_source_func_x[IS_IPv4]), + nm_utils_addr_family_to_char (addr_family), + priv->activation_source_id_x[IS_IPv4]); + } + + nm_clear_g_source (&priv->activation_source_id_x[IS_IPv4]); + priv->activation_source_func_x[IS_IPv4] = NULL; + + func (self); +} + /*****************************************************************************/ static void @@ -6585,7 +6617,7 @@ activate_stage1_device_prepare (NMDevice *self) if (master) master_ready (self, active); - nm_device_activate_schedule_stage2_device_config (self); + activation_source_invoke_sync (self, activate_stage2_device_config, AF_INET); } /*