From 94f42e9bec87fa2b62fc368d2eace8f3323778b4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 29 Aug 2016 14:27:46 +0200 Subject: [PATCH 1/6] device: log changes to ip4_state and ip6_state --- src/devices/nm-device.c | 93 ++++++++++++++++++++++++++++------------- 1 file changed, 63 insertions(+), 30 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 004f869f01..b776a24dc4 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -296,7 +296,10 @@ typedef struct _NMDevicePrivate { /* IP4 configuration info */ NMIP4Config * ip4_config; /* Combined config from VPN, settings, and device */ - IpState ip4_state; + union { + const IpState ip4_state; + IpState ip4_state_; + }; NMIP4Config * con_ip4_config; /* config from the setting */ NMIP4Config * dev_ip4_config; /* Config from DHCP, PPP, LLv4, etc */ NMIP4Config * ext_ip4_config; /* Stuff added outside NM */ @@ -345,7 +348,10 @@ typedef struct _NMDevicePrivate { /* IP6 configuration info */ NMIP6Config * ip6_config; - IpState ip6_state; + union { + const IpState ip6_state; + IpState ip6_state_; + }; NMIP6Config * con_ip6_config; /* config from the setting */ NMIP6Config * wwan_ip6_config; NMIP6Config * ext_ip6_config; /* Stuff added outside NM */ @@ -654,6 +660,34 @@ _get_stable_id (NMConnection *connection, NMUtilsStableType *out_stable_type) /***********************************************************/ +NM_UTILS_LOOKUP_STR_DEFINE_STATIC (_ip_state_to_string, IpState, + NM_UTILS_LOOKUP_DEFAULT_WARN ("unknown"), + NM_UTILS_LOOKUP_STR_ITEM (IP_NONE, "none"), + NM_UTILS_LOOKUP_STR_ITEM (IP_WAIT, "wait"), + NM_UTILS_LOOKUP_STR_ITEM (IP_CONF, "conf"), + NM_UTILS_LOOKUP_STR_ITEM (IP_DONE, "done"), + NM_UTILS_LOOKUP_STR_ITEM (IP_FAIL, "fail"), +); + +static void +_set_ip_state (NMDevice *self, int addr_family, IpState new_state) +{ + IpState *p; + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + + nm_assert (NM_IN_SET (addr_family, AF_INET, AF_INET6)); + + p = addr_family == AF_INET ? &priv->ip4_state_ : &priv->ip6_state_; + + if (*p != new_state) { + _LOGT (LOGD_DEVICE, "ip%c-state: set to %d (%s)", addr_family == AF_INET ? '4' : '6', + (int) new_state, _ip_state_to_string (new_state)); + *p = new_state; + } +} + +/*****************************************************************************/ + const char * nm_device_get_udi (NMDevice *self) { @@ -2892,8 +2926,8 @@ nm_device_slave_notify_enslave (NMDevice *self, gboolean success) } if (activating) { - priv->ip4_state = IP_DONE; - priv->ip6_state = IP_DONE; + _set_ip_state (self, AF_INET, IP_DONE); + _set_ip_state (self, AF_INET6, IP_DONE); if (success) nm_device_queue_state (self, NM_DEVICE_STATE_SECONDARIES, NM_DEVICE_STATE_REASON_NONE); else @@ -3933,7 +3967,8 @@ activate_stage1_device_prepare (NMDevice *self) NMDeviceStateReason reason = NM_DEVICE_STATE_REASON_NONE; NMActiveConnection *active = NM_ACTIVE_CONNECTION (priv->act_request); - priv->ip4_state = priv->ip6_state = IP_NONE; + _set_ip_state (self, AF_INET, IP_NONE); + _set_ip_state (self, AF_INET6, IP_NONE); /* Notify the new ActiveConnection along with the state change */ _notify (self, PROP_ACTIVE_CONNECTION); @@ -4130,7 +4165,8 @@ check_ip_failed (NMDevice *self, gboolean may_fail) if (nm_device_uses_assumed_connection (self)) { /* We have assumed configuration, but couldn't * redo it. No problem, move to check state. */ - priv->ip4_state = priv->ip6_state = IP_DONE; + _set_ip_state (self, AF_INET, IP_DONE); + _set_ip_state (self, AF_INET6, IP_DONE); state = NM_DEVICE_STATE_IP_CHECK; } else if ( may_fail && get_ip_config_may_fail (self, AF_INET) @@ -4155,12 +4191,10 @@ nm_device_ip_method_failed (NMDevice *self, int family, NMDeviceStateReason reas g_return_if_fail (NM_IS_DEVICE (self)); g_return_if_fail (family == AF_INET || family == AF_INET6); + priv = NM_DEVICE_GET_PRIVATE (self); - if (family == AF_INET) - priv->ip4_state = IP_FAIL; - else - priv->ip6_state = IP_FAIL; + _set_ip_state (self, family, IP_FAIL); if (get_ip_config_may_fail (self, family)) check_ip_failed (self, FALSE); @@ -6804,7 +6838,7 @@ nm_device_activate_stage3_ip4_start (NMDevice *self) g_assert (priv->ip4_state == IP_WAIT); - priv->ip4_state = IP_CONF; + _set_ip_state (self, AF_INET, IP_CONF); ret = NM_DEVICE_GET_CLASS (self)->act_stage3_ip4_config_start (self, &ip4_config, &reason); if (ret == NM_ACT_STAGE_RETURN_SUCCESS) { g_assert (ip4_config); @@ -6815,10 +6849,10 @@ nm_device_activate_stage3_ip4_start (NMDevice *self) return FALSE; } else if (ret == NM_ACT_STAGE_RETURN_STOP) { /* Early finish */ - priv->ip4_state = IP_FAIL; + _set_ip_state (self, AF_INET, IP_FAIL); } else if (ret == NM_ACT_STAGE_RETURN_WAIT) { /* Wait for something to try IP config again */ - priv->ip4_state = IP_WAIT; + _set_ip_state (self, AF_INET, IP_WAIT); } else g_assert (ret == NM_ACT_STAGE_RETURN_POSTPONE); @@ -6841,7 +6875,7 @@ nm_device_activate_stage3_ip6_start (NMDevice *self) g_assert (priv->ip6_state == IP_WAIT); - priv->ip6_state = IP_CONF; + _set_ip_state (self, AF_INET6, IP_CONF); ret = NM_DEVICE_GET_CLASS (self)->act_stage3_ip6_config_start (self, &ip6_config, &reason); if (ret == NM_ACT_STAGE_RETURN_SUCCESS) { g_assert (ip6_config); @@ -6856,14 +6890,14 @@ nm_device_activate_stage3_ip6_start (NMDevice *self) return FALSE; } else if (ret == NM_ACT_STAGE_RETURN_STOP) { /* Activation not wanted */ - priv->ip6_state = IP_FAIL; + _set_ip_state (self, AF_INET6, IP_FAIL); } else if (ret == NM_ACT_STAGE_RETURN_FINISH) { /* Early finish, nothing more to do */ - priv->ip6_state = IP_DONE; + _set_ip_state (self, AF_INET6, IP_DONE); check_ip_done (self); } else if (ret == NM_ACT_STAGE_RETURN_WAIT) { /* Wait for something to try IP config again */ - priv->ip6_state = IP_WAIT; + _set_ip_state (self, AF_INET6, IP_WAIT); } else g_assert (ret == NM_ACT_STAGE_RETURN_POSTPONE); @@ -6883,7 +6917,8 @@ activate_stage3_ip_config_start (NMDevice *self) NMActiveConnection *master; NMDevice *master_device; - priv->ip4_state = priv->ip6_state = IP_WAIT; + _set_ip_state (self, AF_INET, IP_WAIT); + _set_ip_state (self, AF_INET6, IP_WAIT); nm_device_state_changed (self, NM_DEVICE_STATE_IP_CONFIG, NM_DEVICE_STATE_REASON_NONE); @@ -7040,7 +7075,6 @@ act_stage4_ip4_config_timeout (NMDevice *self, NMDeviceStateReason *reason) static void activate_stage4_ip4_config_timeout (NMDevice *self) { - NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); NMActStageReturn ret = NM_ACT_STAGE_RETURN_FAILURE; NMDeviceStateReason reason = NM_DEVICE_STATE_REASON_NONE; @@ -7053,7 +7087,7 @@ activate_stage4_ip4_config_timeout (NMDevice *self) } g_assert (ret == NM_ACT_STAGE_RETURN_SUCCESS); - priv->ip4_state = IP_FAIL; + _set_ip_state (self, AF_INET, IP_FAIL); check_ip_failed (self, FALSE); } @@ -7097,7 +7131,6 @@ act_stage4_ip6_config_timeout (NMDevice *self, NMDeviceStateReason *reason) static void activate_stage4_ip6_config_timeout (NMDevice *self) { - NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); NMActStageReturn ret = NM_ACT_STAGE_RETURN_FAILURE; NMDeviceStateReason reason = NM_DEVICE_STATE_REASON_NONE; @@ -7110,7 +7143,7 @@ activate_stage4_ip6_config_timeout (NMDevice *self) } g_assert (ret == NM_ACT_STAGE_RETURN_SUCCESS); - priv->ip6_state = IP_FAIL; + _set_ip_state (self, AF_INET6, IP_FAIL); check_ip_failed (self, FALSE); } @@ -7350,7 +7383,7 @@ activate_stage5_ip4_config_commit (NMDevice *self) nm_device_remove_pending_action (self, PENDING_ACTION_DHCP4, FALSE); /* Enter the IP_CHECK state if this is the first method to complete */ - priv->ip4_state = IP_DONE; + _set_ip_state (self, AF_INET, IP_DONE); check_ip_done (self); } @@ -7492,7 +7525,7 @@ activate_stage5_ip6_config_commit (NMDevice *self) _LOGD (LOGD_DEVICE | LOGD_IP6, "IPv6 DAD: waiting termination"); } else { /* No tentative addresses, proceed right away */ - priv->ip6_state = IP_DONE; + _set_ip_state (self, AF_INET6, IP_DONE); check_ip_done (self); } } @@ -7513,7 +7546,7 @@ nm_device_activate_schedule_ip6_config_result (NMDevice *self) * clearly now have configuration. */ if (priv->ip6_state == IP_FAIL) - priv->ip6_state = IP_CONF; + _set_ip_state (self, AF_INET6, IP_CONF); activation_source_schedule (self, activate_stage5_ip6_config_commit, AF_INET6); } @@ -7682,7 +7715,7 @@ _cleanup_ip4_pre (NMDevice *self, CleanupType cleanup_type) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); - priv->ip4_state = IP_NONE; + _set_ip_state (self, AF_INET, IP_NONE); if (nm_clear_g_source (&priv->queued_ip4_config_id)) _LOGD (LOGD_DEVICE, "clearing queued IP4 config change"); @@ -7698,7 +7731,7 @@ _cleanup_ip6_pre (NMDevice *self, CleanupType cleanup_type) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); - priv->ip6_state = IP_NONE; + _set_ip_state (self, AF_INET6, IP_NONE); if (nm_clear_g_source (&priv->queued_ip6_config_id)) _LOGD (LOGD_DEVICE, "clearing queued IP6 config change"); @@ -7796,7 +7829,7 @@ nm_device_reactivate_ip4_config (NMDevice *self, if (strcmp (nm_setting_ip_config_get_method (s_ip4_new), nm_setting_ip_config_get_method (s_ip4_old))) { _cleanup_ip4_pre (self, CLEANUP_TYPE_DECONFIGURE); - priv->ip4_state = IP_WAIT; + _set_ip_state (self, AF_INET, IP_WAIT); if (!nm_device_activate_stage3_ip4_start (self)) _LOGW (LOGD_IP4, "Failed to apply IPv4 configuration"); } else { @@ -7827,7 +7860,7 @@ nm_device_reactivate_ip6_config (NMDevice *self, if (strcmp (nm_setting_ip_config_get_method (s_ip6_new), nm_setting_ip_config_get_method (s_ip6_old))) { _cleanup_ip6_pre (self, CLEANUP_TYPE_DECONFIGURE); - priv->ip6_state = IP_WAIT; + _set_ip_state (self, AF_INET6, IP_WAIT); if (!nm_device_activate_stage3_ip6_start (self)) _LOGW (LOGD_IP6, "Failed to apply IPv6 configuration"); } else { @@ -9626,7 +9659,7 @@ queued_ip6_config_change (gpointer user_data) priv->dad6_ip6_config)) { _LOGD (LOGD_DEVICE | LOGD_IP6, "IPv6 DAD terminated"); g_clear_object (&priv->dad6_ip6_config); - priv->ip6_state = IP_DONE; + _set_ip_state (self, AF_INET6, IP_DONE); check_ip_done (self); } } From 398e1e8b3c7e63161e7a3c8ac614bb29fd7e3e7c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 29 Aug 2016 15:33:33 +0200 Subject: [PATCH 2/6] device: remove unneeded activation-stage result NM_ACT_STAGE_RETURN_FINISH We can express FINISH by returning SUCCESS and not set out_config in act_stage3_ip4_config_start(). --- src/devices/nm-device-private.h | 1 - src/devices/nm-device.c | 44 ++++++++++++++++++--------------- 2 files changed, 24 insertions(+), 21 deletions(-) diff --git a/src/devices/nm-device-private.h b/src/devices/nm-device-private.h index c85cb969ba..a91bb422f2 100644 --- a/src/devices/nm-device-private.h +++ b/src/devices/nm-device-private.h @@ -32,7 +32,6 @@ enum NMActStageReturn { NM_ACT_STAGE_RETURN_POSTPONE, /* Long-running operation in progress */ NM_ACT_STAGE_RETURN_WAIT, /* Not ready to start stage; wait */ NM_ACT_STAGE_RETURN_STOP, /* Activation not wanted */ - NM_ACT_STAGE_RETURN_FINISH /* Activation stage done; nothing to do */ }; #define NM_DEVICE_CAP_NONSTANDARD_CARRIER 0x80000000 diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index b776a24dc4..790a4a93c7 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -5995,7 +5995,7 @@ dhcp6_start (NMDevice *self, gboolean wait_for_ll, NMDeviceStateReason *reason) } /* success; already have the LL address; kick off DHCP */ - g_assert (ret == NM_ACT_STAGE_RETURN_SUCCESS || ret == NM_ACT_STAGE_RETURN_FINISH); + g_assert (ret == NM_ACT_STAGE_RETURN_SUCCESS); } if (!dhcp6_start_with_link_ready (self, connection)) { @@ -6182,7 +6182,7 @@ linklocal6_start (NMDevice *self) if ( priv->ip6_config && nm_ip6_config_get_address_first_nontentative (priv->ip6_config, TRUE)) - return NM_ACT_STAGE_RETURN_FINISH; + return NM_ACT_STAGE_RETURN_SUCCESS; connection = nm_device_get_applied_connection (self); g_assert (connection); @@ -6507,7 +6507,7 @@ addrconf6_start (NMDevice *self, NMSettingIP6ConfigPrivacy use_tempaddr) } /* success; already have the LL address; kick off router discovery */ - g_assert (ret == NM_ACT_STAGE_RETURN_SUCCESS || ret == NM_ACT_STAGE_RETURN_FINISH); + g_assert (ret == NM_ACT_STAGE_RETURN_SUCCESS); return addrconf6_start_with_link_ready (self); } @@ -6797,8 +6797,6 @@ act_stage3_ip6_config_start (NMDevice *self, } else if (strcmp (method, NM_SETTING_IP6_CONFIG_METHOD_MANUAL) == 0) { /* New blank config */ *out_config = nm_ip6_config_new (nm_device_get_ip_ifindex (self)); - g_assert (*out_config); - ret = NM_ACT_STAGE_RETURN_SUCCESS; } else _LOGW (LOGD_IP6, "unhandled IPv6 config method '%s'; will fail", method); @@ -6841,14 +6839,19 @@ nm_device_activate_stage3_ip4_start (NMDevice *self) _set_ip_state (self, AF_INET, IP_CONF); ret = NM_DEVICE_GET_CLASS (self)->act_stage3_ip4_config_start (self, &ip4_config, &reason); if (ret == NM_ACT_STAGE_RETURN_SUCCESS) { - g_assert (ip4_config); - nm_device_activate_schedule_ip4_config_result (self, ip4_config); - g_object_unref (ip4_config); + if (!ip4_config) { + /* Early finish, nothing more to do */ + _set_ip_state (self, AF_INET, IP_DONE); + check_ip_done (self); + } else { + nm_device_activate_schedule_ip4_config_result (self, ip4_config); + g_object_unref (ip4_config); + } } else if (ret == NM_ACT_STAGE_RETURN_FAILURE) { nm_device_state_changed (self, NM_DEVICE_STATE_FAILED, reason); return FALSE; } else if (ret == NM_ACT_STAGE_RETURN_STOP) { - /* Early finish */ + /* Activation not wanted */ _set_ip_state (self, AF_INET, IP_FAIL); } else if (ret == NM_ACT_STAGE_RETURN_WAIT) { /* Wait for something to try IP config again */ @@ -6878,23 +6881,24 @@ nm_device_activate_stage3_ip6_start (NMDevice *self) _set_ip_state (self, AF_INET6, IP_CONF); ret = NM_DEVICE_GET_CLASS (self)->act_stage3_ip6_config_start (self, &ip6_config, &reason); if (ret == NM_ACT_STAGE_RETURN_SUCCESS) { - g_assert (ip6_config); - /* Here we get a static IPv6 config, like for Shared where it's - * autogenerated or from modems where it comes from ModemManager. - */ - g_warn_if_fail (priv->ac_ip6_config == NULL); - priv->ac_ip6_config = ip6_config; - nm_device_activate_schedule_ip6_config_result (self); + if (!ip6_config) { + /* Early finish, nothing more to do */ + _set_ip_state (self, AF_INET6, IP_DONE); + check_ip_done (self); + } else { + /* Here we get a static IPv6 config, like for Shared where it's + * autogenerated or from modems where it comes from ModemManager. + */ + g_warn_if_fail (priv->ac_ip6_config == NULL); + priv->ac_ip6_config = ip6_config; + nm_device_activate_schedule_ip6_config_result (self); + } } else if (ret == NM_ACT_STAGE_RETURN_FAILURE) { nm_device_state_changed (self, NM_DEVICE_STATE_FAILED, reason); return FALSE; } else if (ret == NM_ACT_STAGE_RETURN_STOP) { /* Activation not wanted */ _set_ip_state (self, AF_INET6, IP_FAIL); - } else if (ret == NM_ACT_STAGE_RETURN_FINISH) { - /* Early finish, nothing more to do */ - _set_ip_state (self, AF_INET6, IP_DONE); - check_ip_done (self); } else if (ret == NM_ACT_STAGE_RETURN_WAIT) { /* Wait for something to try IP config again */ _set_ip_state (self, AF_INET6, IP_WAIT); From 2162a84c5fd191fe6de171412958473c0b6f4487 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 31 Aug 2016 18:51:16 +0200 Subject: [PATCH 3/6] device/trivial: rename NM_ACT_STAGE_RETURN_STOP to NM_ACT_STAGE_RETURN_IP_FAIL and rename NM_ACT_STAGE_RETURN_STOP to NM_ACT_STAGE_RETURN_IP_FAIL. They are only used during IP config stage. Give them a better name. --- src/devices/nm-device-private.h | 6 +++--- src/devices/nm-device.c | 24 ++++++++++++------------ src/devices/wwan/nm-device-modem.c | 2 +- src/devices/wwan/nm-modem.c | 8 ++++---- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/devices/nm-device-private.h b/src/devices/nm-device-private.h index a91bb422f2..0274431bb9 100644 --- a/src/devices/nm-device-private.h +++ b/src/devices/nm-device-private.h @@ -27,11 +27,11 @@ /* This file should only be used by subclasses of NMDevice */ enum NMActStageReturn { - NM_ACT_STAGE_RETURN_FAILURE = 0, + NM_ACT_STAGE_RETURN_FAILURE = 0, /* Hard failure of activation */ NM_ACT_STAGE_RETURN_SUCCESS, /* Activation stage done */ NM_ACT_STAGE_RETURN_POSTPONE, /* Long-running operation in progress */ - NM_ACT_STAGE_RETURN_WAIT, /* Not ready to start stage; wait */ - NM_ACT_STAGE_RETURN_STOP, /* Activation not wanted */ + NM_ACT_STAGE_RETURN_IP_WAIT, /* IP config stage is waiting (state IP_WAIT) */ + NM_ACT_STAGE_RETURN_IP_FAIL, /* IP config stage failed (state IP_FAIL), activation may proceed */ }; #define NM_DEVICE_CAP_NONSTANDARD_CARRIER 0x80000000 diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 790a4a93c7..927d12e76c 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -5388,7 +5388,7 @@ act_stage3_ip4_config_start (NMDevice *self, && !priv->carrier) { _LOGI (LOGD_IP4 | LOGD_DEVICE, "IPv4 config waiting until carrier is on"); - return NM_ACT_STAGE_RETURN_WAIT; + return NM_ACT_STAGE_RETURN_IP_WAIT; } if (priv->is_master && ip4_requires_slaves (connection)) { @@ -5402,7 +5402,7 @@ act_stage3_ip4_config_start (NMDevice *self, if (ready_slaves == FALSE) { _LOGI (LOGD_DEVICE | LOGD_IP4, "IPv4 config waiting until slaves are ready"); - return NM_ACT_STAGE_RETURN_WAIT; + return NM_ACT_STAGE_RETURN_IP_WAIT; } } @@ -5436,7 +5436,7 @@ act_stage3_ip4_config_start (NMDevice *self, } else if (strcmp (method, NM_SETTING_IP4_CONFIG_METHOD_DISABLED) == 0) { apply_mtu_from_config (self); /* Nothing else to do... */ - ret = NM_ACT_STAGE_RETURN_STOP; + ret = NM_ACT_STAGE_RETURN_IP_FAIL; } else _LOGW (LOGD_IP4, "unhandled IPv4 config method '%s'; will fail", method); @@ -6721,7 +6721,7 @@ act_stage3_ip6_config_start (NMDevice *self, && !priv->carrier) { _LOGI (LOGD_IP6 | LOGD_DEVICE, "IPv6 config waiting until carrier is on"); - return NM_ACT_STAGE_RETURN_WAIT; + return NM_ACT_STAGE_RETURN_IP_WAIT; } if (priv->is_master && ip6_requires_slaves (connection)) { @@ -6735,7 +6735,7 @@ act_stage3_ip6_config_start (NMDevice *self, if (ready_slaves == FALSE) { _LOGI (LOGD_DEVICE | LOGD_IP6, "IPv6 config waiting until slaves are ready"); - return NM_ACT_STAGE_RETURN_WAIT; + return NM_ACT_STAGE_RETURN_IP_WAIT; } } @@ -6757,7 +6757,7 @@ act_stage3_ip6_config_start (NMDevice *self, nm_device_ipv6_sysctl_set (self, "disable_ipv6", "1"); restore_ip6_properties (self); } - return NM_ACT_STAGE_RETURN_STOP; + return NM_ACT_STAGE_RETURN_IP_FAIL; } /* Ensure the MTU makes sense. If it was below 1280 the kernel would not @@ -6782,7 +6782,7 @@ act_stage3_ip6_config_start (NMDevice *self, if (strcmp (method, NM_SETTING_IP6_CONFIG_METHOD_AUTO) == 0) { if (!addrconf6_start (self, ip6_privacy)) { /* IPv6 might be disabled; allow IPv4 to proceed */ - ret = NM_ACT_STAGE_RETURN_STOP; + ret = NM_ACT_STAGE_RETURN_IP_FAIL; } else ret = NM_ACT_STAGE_RETURN_POSTPONE; } else if (strcmp (method, NM_SETTING_IP6_CONFIG_METHOD_LINK_LOCAL) == 0) { @@ -6791,7 +6791,7 @@ act_stage3_ip6_config_start (NMDevice *self, priv->dhcp6.mode = NM_RDISC_DHCP_LEVEL_MANAGED; if (!dhcp6_start (self, TRUE, reason)) { /* IPv6 might be disabled; allow IPv4 to proceed */ - ret = NM_ACT_STAGE_RETURN_STOP; + ret = NM_ACT_STAGE_RETURN_IP_FAIL; } else ret = NM_ACT_STAGE_RETURN_POSTPONE; } else if (strcmp (method, NM_SETTING_IP6_CONFIG_METHOD_MANUAL) == 0) { @@ -6850,10 +6850,10 @@ nm_device_activate_stage3_ip4_start (NMDevice *self) } else if (ret == NM_ACT_STAGE_RETURN_FAILURE) { nm_device_state_changed (self, NM_DEVICE_STATE_FAILED, reason); return FALSE; - } else if (ret == NM_ACT_STAGE_RETURN_STOP) { + } else if (ret == NM_ACT_STAGE_RETURN_IP_FAIL) { /* Activation not wanted */ _set_ip_state (self, AF_INET, IP_FAIL); - } else if (ret == NM_ACT_STAGE_RETURN_WAIT) { + } else if (ret == NM_ACT_STAGE_RETURN_IP_WAIT) { /* Wait for something to try IP config again */ _set_ip_state (self, AF_INET, IP_WAIT); } else @@ -6896,10 +6896,10 @@ nm_device_activate_stage3_ip6_start (NMDevice *self) } else if (ret == NM_ACT_STAGE_RETURN_FAILURE) { nm_device_state_changed (self, NM_DEVICE_STATE_FAILED, reason); return FALSE; - } else if (ret == NM_ACT_STAGE_RETURN_STOP) { + } else if (ret == NM_ACT_STAGE_RETURN_IP_FAIL) { /* Activation not wanted */ _set_ip_state (self, AF_INET6, IP_FAIL); - } else if (ret == NM_ACT_STAGE_RETURN_WAIT) { + } else if (ret == NM_ACT_STAGE_RETURN_IP_WAIT) { /* Wait for something to try IP config again */ _set_ip_state (self, AF_INET6, IP_WAIT); } else diff --git a/src/devices/wwan/nm-device-modem.c b/src/devices/wwan/nm-device-modem.c index 0f96dafbfc..f6ca0e9224 100644 --- a/src/devices/wwan/nm-device-modem.c +++ b/src/devices/wwan/nm-device-modem.c @@ -230,7 +230,7 @@ modem_ip6_config_result (NMModem *modem, case NM_ACT_STAGE_RETURN_FAILURE: nm_device_ip_method_failed (device, AF_INET6, reason); break; - case NM_ACT_STAGE_RETURN_STOP: + case NM_ACT_STAGE_RETURN_IP_FAIL: /* all done */ nm_device_activate_schedule_ip6_config_result (device); break; diff --git a/src/devices/wwan/nm-modem.c b/src/devices/wwan/nm-modem.c index 28dab2c157..bf3eac9262 100644 --- a/src/devices/wwan/nm-modem.c +++ b/src/devices/wwan/nm-modem.c @@ -590,7 +590,7 @@ nm_modem_stage3_ip4_config_start (NMModem *self, /* Only Disabled and Auto methods make sense for WWAN */ if (strcmp (method, NM_SETTING_IP4_CONFIG_METHOD_DISABLED) == 0) - return NM_ACT_STAGE_RETURN_STOP; + return NM_ACT_STAGE_RETURN_IP_FAIL; if (strcmp (method, NM_SETTING_IP4_CONFIG_METHOD_AUTO) != 0) { nm_log_warn (LOGD_MB | LOGD_IP4, @@ -615,7 +615,7 @@ nm_modem_stage3_ip4_config_start (NMModem *self, break; default: nm_log_info (LOGD_MB, "(%s): IPv4 configuration disabled", nm_modem_get_uid (self)); - ret = NM_ACT_STAGE_RETURN_STOP; + ret = NM_ACT_STAGE_RETURN_IP_FAIL; break; } @@ -709,7 +709,7 @@ nm_modem_stage3_ip6_config_start (NMModem *self, /* Only Ignore and Auto methods make sense for WWAN */ if (strcmp (method, NM_SETTING_IP6_CONFIG_METHOD_IGNORE) == 0) - return NM_ACT_STAGE_RETURN_STOP; + return NM_ACT_STAGE_RETURN_IP_FAIL; if (strcmp (method, NM_SETTING_IP6_CONFIG_METHOD_AUTO) != 0) { nm_log_warn (LOGD_MB | LOGD_IP6, @@ -734,7 +734,7 @@ nm_modem_stage3_ip6_config_start (NMModem *self, break; default: nm_log_info (LOGD_MB, "(%s): IPv6 configuration disabled", nm_modem_get_uid (self)); - ret = NM_ACT_STAGE_RETURN_STOP; + ret = NM_ACT_STAGE_RETURN_IP_FAIL; break; } From dd484729097642459b15e92dd0d7171796934aba Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 31 Aug 2016 19:11:25 +0200 Subject: [PATCH 4/6] device: only set use_tempaddr sysctl for non-assumed devices and only if the activation stage is not about to fail hard. --- src/devices/nm-device.c | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 927d12e76c..de20e18d14 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -6707,7 +6707,7 @@ act_stage3_ip6_config_start (NMDevice *self, NMConnection *connection; const char *method; NMSettingIP6ConfigPrivacy ip6_privacy = NM_SETTING_IP6_CONFIG_PRIVACY_UNKNOWN; - const char *ip6_privacy_str = "0\n"; + const char *ip6_privacy_str = "0"; GSList *slaves; gboolean ready_slaves; @@ -6803,19 +6803,22 @@ act_stage3_ip6_config_start (NMDevice *self, /* Other methods (shared) aren't implemented yet */ - switch (ip6_privacy) { - case NM_SETTING_IP6_CONFIG_PRIVACY_UNKNOWN: - case NM_SETTING_IP6_CONFIG_PRIVACY_DISABLED: - ip6_privacy_str = "0"; - break; - case NM_SETTING_IP6_CONFIG_PRIVACY_PREFER_PUBLIC_ADDR: - ip6_privacy_str = "1"; - break; - case NM_SETTING_IP6_CONFIG_PRIVACY_PREFER_TEMP_ADDR: - ip6_privacy_str = "2"; - break; + if ( ret != NM_ACT_STAGE_RETURN_FAILURE + && !nm_device_uses_assumed_connection (self)) { + switch (ip6_privacy) { + case NM_SETTING_IP6_CONFIG_PRIVACY_UNKNOWN: + case NM_SETTING_IP6_CONFIG_PRIVACY_DISABLED: + ip6_privacy_str = "0"; + break; + case NM_SETTING_IP6_CONFIG_PRIVACY_PREFER_PUBLIC_ADDR: + ip6_privacy_str = "1"; + break; + case NM_SETTING_IP6_CONFIG_PRIVACY_PREFER_TEMP_ADDR: + ip6_privacy_str = "2"; + break; + } + nm_device_ipv6_sysctl_set (self, "use_tempaddr", ip6_privacy_str); } - nm_device_ipv6_sysctl_set (self, "use_tempaddr", ip6_privacy_str); return ret; } From 067aa5036355fe436e45bd8debcc84e439a92762 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 31 Aug 2016 20:01:12 +0200 Subject: [PATCH 5/6] device: add new result NM_ACT_STAGE_RETURN_IP_DONE for ip config activation This is like NM_ACT_STAGE_RETURN_SUCCESS, except it should only set the IP state without commiting an NMIP[46]Config instance. --- src/devices/nm-device-private.h | 4 +++ src/devices/nm-device.c | 53 ++++++++++++++++----------------- 2 files changed, 30 insertions(+), 27 deletions(-) diff --git a/src/devices/nm-device-private.h b/src/devices/nm-device-private.h index 0274431bb9..927dc8c46c 100644 --- a/src/devices/nm-device-private.h +++ b/src/devices/nm-device-private.h @@ -31,6 +31,10 @@ enum NMActStageReturn { NM_ACT_STAGE_RETURN_SUCCESS, /* Activation stage done */ NM_ACT_STAGE_RETURN_POSTPONE, /* Long-running operation in progress */ NM_ACT_STAGE_RETURN_IP_WAIT, /* IP config stage is waiting (state IP_WAIT) */ + NM_ACT_STAGE_RETURN_IP_DONE, /* IP config stage is done (state IP_DONE), + For the ip-config stage, this is similar to + NM_ACT_STAGE_RETURN_SUCCESS, except that no + IP config should be commited. */ NM_ACT_STAGE_RETURN_IP_FAIL, /* IP config stage failed (state IP_FAIL), activation may proceed */ }; diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index de20e18d14..743202eb90 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -5427,12 +5427,15 @@ act_stage3_ip4_config_start (NMDevice *self, ipv4_dad_start (self, configs, ipv4_manual_method_apply); ret = NM_ACT_STAGE_RETURN_POSTPONE; } else if (strcmp (method, NM_SETTING_IP4_CONFIG_METHOD_SHARED) == 0) { - *out_config = shared4_new_config (self, connection, reason); - if (*out_config) { - priv->dnsmasq_manager = nm_dnsmasq_manager_new (nm_device_get_ip_iface (self)); - ret = NM_ACT_STAGE_RETURN_SUCCESS; + if (out_config) { + *out_config = shared4_new_config (self, connection, reason); + if (*out_config) { + priv->dnsmasq_manager = nm_dnsmasq_manager_new (nm_device_get_ip_iface (self)); + ret = NM_ACT_STAGE_RETURN_SUCCESS; + } else + ret = NM_ACT_STAGE_RETURN_FAILURE; } else - ret = NM_ACT_STAGE_RETURN_FAILURE; + g_return_val_if_reached (NM_ACT_STAGE_RETURN_FAILURE); } else if (strcmp (method, NM_SETTING_IP4_CONFIG_METHOD_DISABLED) == 0) { apply_mtu_from_config (self); /* Nothing else to do... */ @@ -6795,8 +6798,6 @@ act_stage3_ip6_config_start (NMDevice *self, } else ret = NM_ACT_STAGE_RETURN_POSTPONE; } else if (strcmp (method, NM_SETTING_IP6_CONFIG_METHOD_MANUAL) == 0) { - /* New blank config */ - *out_config = nm_ip6_config_new (nm_device_get_ip_ifindex (self)); ret = NM_ACT_STAGE_RETURN_SUCCESS; } else _LOGW (LOGD_IP6, "unhandled IPv6 config method '%s'; will fail", method); @@ -6842,14 +6843,13 @@ nm_device_activate_stage3_ip4_start (NMDevice *self) _set_ip_state (self, AF_INET, IP_CONF); ret = NM_DEVICE_GET_CLASS (self)->act_stage3_ip4_config_start (self, &ip4_config, &reason); if (ret == NM_ACT_STAGE_RETURN_SUCCESS) { - if (!ip4_config) { - /* Early finish, nothing more to do */ - _set_ip_state (self, AF_INET, IP_DONE); - check_ip_done (self); - } else { - nm_device_activate_schedule_ip4_config_result (self, ip4_config); - g_object_unref (ip4_config); - } + if (!ip4_config) + ip4_config = nm_ip4_config_new (nm_device_get_ip_ifindex (self)); + nm_device_activate_schedule_ip4_config_result (self, ip4_config); + g_object_unref (ip4_config); + } else if (ret == NM_ACT_STAGE_RETURN_IP_DONE) { + _set_ip_state (self, AF_INET, IP_DONE); + check_ip_done (self); } else if (ret == NM_ACT_STAGE_RETURN_FAILURE) { nm_device_state_changed (self, NM_DEVICE_STATE_FAILED, reason); return FALSE; @@ -6884,18 +6884,17 @@ nm_device_activate_stage3_ip6_start (NMDevice *self) _set_ip_state (self, AF_INET6, IP_CONF); ret = NM_DEVICE_GET_CLASS (self)->act_stage3_ip6_config_start (self, &ip6_config, &reason); if (ret == NM_ACT_STAGE_RETURN_SUCCESS) { - if (!ip6_config) { - /* Early finish, nothing more to do */ - _set_ip_state (self, AF_INET6, IP_DONE); - check_ip_done (self); - } else { - /* Here we get a static IPv6 config, like for Shared where it's - * autogenerated or from modems where it comes from ModemManager. - */ - g_warn_if_fail (priv->ac_ip6_config == NULL); - priv->ac_ip6_config = ip6_config; - nm_device_activate_schedule_ip6_config_result (self); - } + if (!ip6_config) + ip6_config = nm_ip6_config_new (nm_device_get_ip_ifindex (self)); + /* Here we get a static IPv6 config, like for Shared where it's + * autogenerated or from modems where it comes from ModemManager. + */ + g_warn_if_fail (priv->ac_ip6_config == NULL); + priv->ac_ip6_config = ip6_config; + nm_device_activate_schedule_ip6_config_result (self); + } else if (ret == NM_ACT_STAGE_RETURN_IP_DONE) { + _set_ip_state (self, AF_INET6, IP_DONE); + check_ip_done (self); } else if (ret == NM_ACT_STAGE_RETURN_FAILURE) { nm_device_state_changed (self, NM_DEVICE_STATE_FAILED, reason); return FALSE; From 553717bb1c9ed31be0fab85bc37f6823dc8ab480 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 29 Aug 2016 15:37:55 +0200 Subject: [PATCH 6/6] device: don't set ip4_state=IP_FAIL for ipv4.method=disabled ... and don't set ip6_state=IP_FAIL for ipv6.method=ignore. The disabled state is like having an empty NMIP4Config object. It should not result in %IP_FAIL state. Instead, we just want to proceed and commit an empty NMIP4Config instance. This was introduced by commit 0652d9c5961e4636eab87647af890aaf8c3e3269, which I think was wrong. Likewise, for ipv6.method=ignore we also don't want to mark the IP state as failed. Instead, we want to proceed and set IP_DONE right away -- without commiting anything, which is a difference to the IPv4 case. This is especially important, because an ip4_state/ip6_state of IP_FAIL causes nm_device_can_assume_active_connection() to return FALSE, which means we unmanage devices at shutdown. Ony might say that it doesn't matter so much for a device without IP configuration, but imagine a bond with VLANs on top that only has Layer 2 configuration. This will bring down the entire stack. With this change, devices with IP methods disabled/ignore stay up on exit of NetworkManager (rh#1371126). Of course, that means on restart software devices stay unamanged due to external-down (because since commit e1edcda, devices without IP address are also external-down). So, this really just fixes one scenario, breaking another one. This should be fixed with bgo#746440 by not assuming connections. https://bugzilla.redhat.com/show_bug.cgi?id=1371126 --- src/devices/nm-device.c | 5 ++--- src/devices/wwan/nm-modem.c | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 743202eb90..7fc61509c0 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -5438,8 +5438,7 @@ act_stage3_ip4_config_start (NMDevice *self, g_return_val_if_reached (NM_ACT_STAGE_RETURN_FAILURE); } else if (strcmp (method, NM_SETTING_IP4_CONFIG_METHOD_DISABLED) == 0) { apply_mtu_from_config (self); - /* Nothing else to do... */ - ret = NM_ACT_STAGE_RETURN_IP_FAIL; + ret = NM_ACT_STAGE_RETURN_SUCCESS; } else _LOGW (LOGD_IP4, "unhandled IPv4 config method '%s'; will fail", method); @@ -6760,7 +6759,7 @@ act_stage3_ip6_config_start (NMDevice *self, nm_device_ipv6_sysctl_set (self, "disable_ipv6", "1"); restore_ip6_properties (self); } - return NM_ACT_STAGE_RETURN_IP_FAIL; + return NM_ACT_STAGE_RETURN_IP_DONE; } /* Ensure the MTU makes sense. If it was below 1280 the kernel would not diff --git a/src/devices/wwan/nm-modem.c b/src/devices/wwan/nm-modem.c index bf3eac9262..94430af47a 100644 --- a/src/devices/wwan/nm-modem.c +++ b/src/devices/wwan/nm-modem.c @@ -590,7 +590,7 @@ nm_modem_stage3_ip4_config_start (NMModem *self, /* Only Disabled and Auto methods make sense for WWAN */ if (strcmp (method, NM_SETTING_IP4_CONFIG_METHOD_DISABLED) == 0) - return NM_ACT_STAGE_RETURN_IP_FAIL; + return NM_ACT_STAGE_RETURN_SUCCESS; if (strcmp (method, NM_SETTING_IP4_CONFIG_METHOD_AUTO) != 0) { nm_log_warn (LOGD_MB | LOGD_IP4, @@ -709,7 +709,7 @@ nm_modem_stage3_ip6_config_start (NMModem *self, /* Only Ignore and Auto methods make sense for WWAN */ if (strcmp (method, NM_SETTING_IP6_CONFIG_METHOD_IGNORE) == 0) - return NM_ACT_STAGE_RETURN_IP_FAIL; + return NM_ACT_STAGE_RETURN_IP_DONE; if (strcmp (method, NM_SETTING_IP6_CONFIG_METHOD_AUTO) != 0) { nm_log_warn (LOGD_MB | LOGD_IP6,