From 5912b2f9a170893002b789fe37a7784eefac4e34 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 13 Oct 2016 15:56:13 +0200 Subject: [PATCH] core: persist the fake permanent hardware address to the device's statefile On devices that have no real permanent hardware address (as returned by ethtool), we take the current MAC address of the device. Currently, NM is a bit flaky about whether to accept such fake permanent addresses for settings like keyfile.unmanaged-devices or the per- connection property ethernet.mac-address. Probably, we should allow using fake addresses there in general. However, that leads to problems because NetworkManager itself changes the current MAC address of such devices. For example when configuing keyfile.unmanaged-device=22:33:44:55:66:77 and later activating a connection with ethernet.cloned-mac-address=22:33:44:55:66:77 we have a strange situation after restart and the device becomes unmanaged. We are going to avoid that, by remembering the fake permanent address in the device state file. This only matters: - for devices that don't have a real permanent address (veth) - if the user or NetworkManager itself changed the MAC address of the device - after a restart of NetworkManager, without reboot. A reboot clears the device state for /var/run/NetworkManager. --- src/devices/nm-device.c | 57 ++++++++++++++++++++++++++++++++------- src/devices/nm-device.h | 2 ++ src/nm-config.c | 60 ++++++++++++++++++++++++++++++++--------- src/nm-config.h | 3 +++ src/nm-manager.c | 7 +++++ 5 files changed, 107 insertions(+), 22 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index d7868f6502..6cc2b4db07 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -11697,24 +11697,49 @@ nm_device_update_permanent_hw_address (NMDevice *self) success_read = nm_platform_link_get_permanent_address (NM_PLATFORM_GET, priv->ifindex, buf, &len); if (!success_read || len != priv->hw_addr_len) { - /* Fall back to current address. We use the fake address and keep it - * until the device unrealizes. + gs_free NMConfigDeviceStateData *dev_state = NULL; + + /* we failed to read a permanent MAC address, thus we use a fake address, + * that is the current MAC address of the device. * - * In some cases it might be necessary to know whether this is a "real" or - * a temporary address (fake). */ + * Note that the permanet MAC address of a NMDevice instance does not change + * after being set once. Thus, we use now a fake address and stick to that + * (until we unrealize the device). */ + priv->hw_addr_perm_fake = TRUE; + + /* We also persist our choice of the fake address to the device state + * file to use the same address on restart of NetworkManager. + * First, try to reload the address from the state file. */ + dev_state = nm_config_device_state_load (nm_config_get (), priv->ifindex); + if ( dev_state + && dev_state->perm_hw_addr_fake + && nm_utils_hwaddr_aton (dev_state->perm_hw_addr_fake, buf, priv->hw_addr_len) + && !nm_utils_hwaddr_matches (buf, priv->hw_addr_len, priv->hw_addr, -1)) { + _LOGD (LOGD_PLATFORM | LOGD_ETHER, "hw-addr: %s (use from statefile: %s, current: %s)", + success_read + ? "read HW addr length of permanent MAC address differs" + : "unable to read permanent MAC address", + dev_state->perm_hw_addr_fake, + priv->hw_addr); + priv->hw_addr_perm = nm_utils_hwaddr_ntoa (buf, priv->hw_addr_len); + goto out; + } + _LOGD (LOGD_PLATFORM | LOGD_ETHER, "hw-addr: %s (use current: %s)", success_read ? "read HW addr length of permanent MAC address differs" : "unable to read permanent MAC address", priv->hw_addr); - priv->hw_addr_perm_fake = TRUE; priv->hw_addr_perm = g_strdup (priv->hw_addr); - } else { - priv->hw_addr_perm_fake = FALSE; - priv->hw_addr_perm = nm_utils_hwaddr_ntoa (buf, len); - _LOGD (LOGD_DEVICE, "hw-addr: read permanent MAC address '%s'", - priv->hw_addr_perm); + goto out; } + + priv->hw_addr_perm_fake = FALSE; + priv->hw_addr_perm = nm_utils_hwaddr_ntoa (buf, len); + _LOGD (LOGD_DEVICE, "hw-addr: read permanent MAC address '%s'", + priv->hw_addr_perm); + +out: _notify (self, PROP_PERM_HW_ADDRESS); } @@ -12050,6 +12075,18 @@ nm_device_hw_addr_reset (NMDevice *self, const char *detail) return _hw_addr_set (self, addr, "reset", detail); } +const char * +nm_device_get_permanent_hw_address_full (NMDevice *self, gboolean *out_is_fake) +{ + NMDevicePrivate *priv; + + g_return_val_if_fail (NM_IS_DEVICE (self), NULL); + + priv = NM_DEVICE_GET_PRIVATE (self); + NM_SET_OUT (out_is_fake, priv->hw_addr_perm && priv->hw_addr_perm_fake); + return priv->hw_addr_perm; +} + const char * nm_device_get_permanent_hw_address (NMDevice *self, gboolean fallback_fake) { diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h index ceee6c0b83..9977c7c55e 100644 --- a/src/devices/nm-device.h +++ b/src/devices/nm-device.h @@ -364,6 +364,8 @@ guint32 nm_device_get_ip6_route_metric (NMDevice *dev); const char * nm_device_get_hw_address (NMDevice *dev); const char * nm_device_get_permanent_hw_address (NMDevice *dev, gboolean fallback_fake); +const char * nm_device_get_permanent_hw_address_full (NMDevice *self, + gboolean *out_is_fake); const char * nm_device_get_initial_hw_address (NMDevice *dev); NMProxyConfig * nm_device_get_proxy_config (NMDevice *dev); diff --git a/src/nm-config.c b/src/nm-config.c index 92ef0468cd..7f22b54f74 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -1879,6 +1879,7 @@ _nm_config_state_set (NMConfig *self, #define DEVICE_RUN_STATE_KEYFILE_GROUP_DEVICE "device" #define DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_MANAGED "managed" +#define DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_PERM_HW_ADDR_FAKE "perm-hw-addr-fake" #define DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_CONNECTION_UUID "connection-uuid" static NMConfigDeviceStateData * @@ -1887,7 +1888,10 @@ _config_device_state_data_new (int ifindex, GKeyFile *kf) NMConfigDeviceStateData *device_state; NMConfigDeviceStateManagedType managed_type = NM_CONFIG_DEVICE_STATE_MANAGED_TYPE_UNKNOWN; gs_free char *connection_uuid = NULL; - gsize len_plus_1; + gs_free char *perm_hw_addr_fake = NULL; + gsize connection_uuid_len; + gsize perm_hw_addr_fake_len; + char *p; nm_assert (ifindex > 0); @@ -1908,21 +1912,42 @@ _config_device_state_data_new (int ifindex, GKeyFile *kf) DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_CONNECTION_UUID, NM_CONFIG_GET_VALUE_STRIP | NM_CONFIG_GET_VALUE_NO_EMPTY); } + + perm_hw_addr_fake = nm_config_keyfile_get_value (kf, + DEVICE_RUN_STATE_KEYFILE_GROUP_DEVICE, + DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_PERM_HW_ADDR_FAKE, + NM_CONFIG_GET_VALUE_STRIP | NM_CONFIG_GET_VALUE_NO_EMPTY); + if (perm_hw_addr_fake) { + char *normalized; + + normalized = nm_utils_hwaddr_canonical (perm_hw_addr_fake, -1); + g_free (perm_hw_addr_fake); + perm_hw_addr_fake = normalized; + } } - len_plus_1 = connection_uuid ? strlen (connection_uuid) + 1 : 0; + connection_uuid_len = connection_uuid ? strlen (connection_uuid) + 1 : 0; + perm_hw_addr_fake_len = perm_hw_addr_fake ? strlen (perm_hw_addr_fake) + 1 : 0; - device_state = g_malloc (sizeof (NMConfigDeviceStateData) + len_plus_1); + device_state = g_malloc (sizeof (NMConfigDeviceStateData) + + connection_uuid_len + + perm_hw_addr_fake_len); device_state->ifindex = ifindex; device_state->managed = managed_type; device_state->connection_uuid = NULL; - if (connection_uuid) { - char *device_state_data; + device_state->perm_hw_addr_fake = NULL; - device_state_data = (char *) (&device_state[1]); - memcpy (device_state_data, connection_uuid, len_plus_1); - device_state->connection_uuid = device_state_data; + p = (char *) (&device_state[1]); + if (connection_uuid) { + memcpy (p, connection_uuid, connection_uuid_len); + device_state->connection_uuid = p; + p += connection_uuid_len; + } + if (perm_hw_addr_fake) { + memcpy (p, perm_hw_addr_fake, perm_hw_addr_fake_len); + device_state->perm_hw_addr_fake = p; + p += perm_hw_addr_fake_len; } return device_state; @@ -1955,10 +1980,11 @@ nm_config_device_state_load (NMConfig *self, device_state = _config_device_state_data_new (ifindex, kf); if (kf) { - _LOGT ("device-state: read #%d (%s); managed=%d, connection-uuid=%s%s%s", + _LOGT ("device-state: read #%d (%s); managed=%d%s%s%s%s%s%s", ifindex, path, device_state->managed, - NM_PRINT_FMT_QUOTE_STRING (device_state->connection_uuid)); + NM_PRINT_FMT_QUOTED (device_state->connection_uuid, ", connection-uuid=", device_state->connection_uuid, "", ""), + NM_PRINT_FMT_QUOTED (device_state->perm_hw_addr_fake, ", perm-hw-addr-fake=", device_state->perm_hw_addr_fake, "", "")); } else { _LOGT ("device-state: read #%d (%s); no persistent state", ifindex, path); @@ -1971,6 +1997,7 @@ gboolean nm_config_device_state_write (NMConfig *self, int ifindex, gboolean managed, + const char *perm_hw_addr_fake, const char *connection_uuid) { char path[NM_STRLEN (NM_CONFIG_DEVICE_STATE_DIR) + 60]; @@ -1982,6 +2009,8 @@ nm_config_device_state_write (NMConfig *self, g_return_val_if_fail (!connection_uuid || *connection_uuid, FALSE); g_return_val_if_fail (managed || !connection_uuid, FALSE); + nm_assert (!perm_hw_addr_fake || nm_utils_hwaddr_valid (perm_hw_addr_fake, -1)); + nm_sprintf_buf (path, "%s/%d", NM_CONFIG_DEVICE_STATE_DIR, ifindex); kf = nm_config_create_keyfile (); @@ -1989,6 +2018,12 @@ nm_config_device_state_write (NMConfig *self, DEVICE_RUN_STATE_KEYFILE_GROUP_DEVICE, DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_MANAGED, !!managed); + if (perm_hw_addr_fake) { + g_key_file_set_string (kf, + DEVICE_RUN_STATE_KEYFILE_GROUP_DEVICE, + DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_PERM_HW_ADDR_FAKE, + perm_hw_addr_fake); + } if (connection_uuid) { g_key_file_set_string (kf, DEVICE_RUN_STATE_KEYFILE_GROUP_DEVICE, @@ -2001,10 +2036,11 @@ nm_config_device_state_write (NMConfig *self, g_error_free (local); return FALSE; } - _LOGT ("device-state: write #%d (%s); managed=%d, connection-uuid=%s%s%s", + _LOGT ("device-state: write #%d (%s); managed=%d%s%s%s%s%s%s", ifindex, path, (bool) managed, - NM_PRINT_FMT_QUOTE_STRING (connection_uuid)); + NM_PRINT_FMT_QUOTED (connection_uuid, ", connection-uuid=", connection_uuid, "", ""), + NM_PRINT_FMT_QUOTED (perm_hw_addr_fake, ", perm-hw-addr-fake=", perm_hw_addr_fake, "", "")); return TRUE; } diff --git a/src/nm-config.h b/src/nm-config.h index 0d3faa8832..326661e9f2 100644 --- a/src/nm-config.h +++ b/src/nm-config.h @@ -199,6 +199,8 @@ struct _NMConfigDeviceStateData { /* the UUID of the last settings-connection active * on the device. */ const char *connection_uuid; + + const char *perm_hw_addr_fake; }; NMConfigDeviceStateData *nm_config_device_state_load (NMConfig *self, @@ -206,6 +208,7 @@ NMConfigDeviceStateData *nm_config_device_state_load (NMConfig *self, gboolean nm_config_device_state_write (NMConfig *self, int ifindex, gboolean managed, + const char *perm_hw_addr_fake, const char *connection_uuid); void nm_config_device_state_prune_unseen (NMConfig *self, GHashTable *seen_ifindexes); diff --git a/src/nm-manager.c b/src/nm-manager.c index ed63b58605..dbc8083f64 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -4674,6 +4674,8 @@ nm_manager_write_device_state (NMManager *self) gboolean managed; NMConnection *settings_connection; const char *uuid = NULL; + const char *perm_hw_addr_fake = NULL; + gboolean perm_hw_addr_is_fake; ifindex = nm_device_get_ip_ifindex (device); if (ifindex <= 0) @@ -4693,9 +4695,14 @@ nm_manager_write_device_state (NMManager *self) uuid = nm_connection_get_uuid (settings_connection); } + perm_hw_addr_fake = nm_device_get_permanent_hw_address_full (device, &perm_hw_addr_is_fake); + if (perm_hw_addr_fake && !perm_hw_addr_is_fake) + perm_hw_addr_fake = NULL; + if (nm_config_device_state_write (priv->config, ifindex, managed, + perm_hw_addr_fake, uuid)) g_hash_table_add (seen_ifindexes, GINT_TO_POINTER (ifindex)); }