From e43bd9228c46725bf17562e85936acc94603acd0 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Mon, 13 Apr 2015 15:44:10 -0500 Subject: [PATCH 01/12] platform: don't use udev for link type determination This allows us to always announce links when the kernel advertises them, instead of waiting for udev. (cherry picked from commit 2599dadc2859262de784567384ba72ab92204d55) --- data/77-nm-olpc-mesh.rules | 6 --- data/Makefile.am | 4 +- src/platform/nm-linux-platform.c | 83 ++++++++++++++++++-------------- 3 files changed, 47 insertions(+), 46 deletions(-) delete mode 100644 data/77-nm-olpc-mesh.rules diff --git a/data/77-nm-olpc-mesh.rules b/data/77-nm-olpc-mesh.rules deleted file mode 100644 index a1a1554c2b..0000000000 --- a/data/77-nm-olpc-mesh.rules +++ /dev/null @@ -1,6 +0,0 @@ -# do not edit this file, it will be overwritten on update - -# The fact that this device is driven by libertas is not currently exposed -# in the sysfs tree..? -KERNEL=="msh*", SUBSYSTEM=="net", DRIVERS=="usb", ATTRS{idVendor}=="1286", ATTRS{idProduct}=="2001", ENV{ID_NM_OLPC_MESH}="1" - diff --git a/data/Makefile.am b/data/Makefile.am index 5a95ea2a42..64ff337636 100644 --- a/data/Makefile.am +++ b/data/Makefile.am @@ -37,8 +37,7 @@ examples_DATA = server.conf if WITH_UDEV_DIR udevrulesdir = $(UDEV_DIR)/rules.d udevrules_DATA = \ - 85-nm-unmanaged.rules \ - 77-nm-olpc-mesh.rules + 85-nm-unmanaged.rules endif server.conf: server.conf.in @@ -59,7 +58,6 @@ EXTRA_DIST = \ NetworkManager-dispatcher.service.in \ org.freedesktop.NetworkManager.service.in \ 85-nm-unmanaged.rules \ - 77-nm-olpc-mesh.rules \ server.conf.in CLEANFILES = \ diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 97ce2605cb..a51b6f7581 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -496,35 +496,6 @@ udev_get_driver (GUdevDevice *device, int ifindex) return driver; } -static NMLinkType -udev_detect_link_type_from_device (GUdevDevice *udev_device, const char *ifname, int arptype, const char **out_name) -{ - const char *prop, *sysfs_path; - - g_assert (ifname); - - if (!udev_device) - return_type (NM_LINK_TYPE_UNKNOWN, "unknown"); - - if ( g_udev_device_get_property (udev_device, "ID_NM_OLPC_MESH") - || g_udev_device_get_sysfs_attr (udev_device, "anycast_mask")) - return_type (NM_LINK_TYPE_OLPC_MESH, "olpc-mesh"); - - prop = g_udev_device_get_property (udev_device, "DEVTYPE"); - sysfs_path = g_udev_device_get_sysfs_path (udev_device); - if (wifi_utils_is_wifi (ifname, sysfs_path, prop)) - return_type (NM_LINK_TYPE_WIFI, "wifi"); - else if (g_strcmp0 (prop, "wwan") == 0) - return_type (NM_LINK_TYPE_WWAN_ETHERNET, "wwan"); - else if (g_strcmp0 (prop, "wimax") == 0) - return_type (NM_LINK_TYPE_WIMAX, "wimax"); - - if (arptype == ARPHRD_ETHER) - return_type (NM_LINK_TYPE_ETHERNET, "ethernet"); - - return_type (NM_LINK_TYPE_UNKNOWN, "unknown"); -} - /****************************************************************** * NMPlatform types and functions ******************************************************************/ @@ -929,6 +900,31 @@ link_is_announceable (NMPlatform *platform, struct rtnl_link *rtnllink) return FALSE; } +#define DEVTYPE_PREFIX "DEVTYPE=" + +static char * +read_devtype (const char *sysfs_path) +{ + gs_free char *uevent = g_strdup_printf ("%s/uevent", sysfs_path); + char *contents = NULL; + char *cont, *end; + + if (!g_file_get_contents (uevent, &contents, NULL, NULL)) + return NULL; + for (cont = contents; cont; cont = end) { + end = strpbrk (cont, "\r\n"); + if (end) + *end++ = '\0'; + if (strncmp (cont, DEVTYPE_PREFIX, STRLEN (DEVTYPE_PREFIX)) == 0) { + cont += STRLEN (DEVTYPE_PREFIX); + memmove (contents, cont, strlen (cont) + 1); + return contents; + } + } + g_free (contents); + return NULL; +} + static NMLinkType link_extract_type (NMPlatform *platform, struct rtnl_link *rtnllink, const char **out_name) { @@ -943,7 +939,9 @@ link_extract_type (NMPlatform *platform, struct rtnl_link *rtnllink, const char int arptype = rtnl_link_get_arptype (rtnllink); const char *driver; const char *ifname; - GUdevDevice *udev_device = NULL; + gs_free char *anycast_mask = NULL; + gs_free char *sysfs_path = NULL; + gs_free char *devtype = NULL; if (arptype == ARPHRD_LOOPBACK) return_type (NM_LINK_TYPE_LOOPBACK, "loopback"); @@ -966,14 +964,25 @@ link_extract_type (NMPlatform *platform, struct rtnl_link *rtnllink, const char if (!g_strcmp0 (driver, "openvswitch")) return_type (NM_LINK_TYPE_OPENVSWITCH, "openvswitch"); - if (platform) { - udev_device = g_hash_table_lookup (NM_LINUX_PLATFORM_GET_PRIVATE (platform)->udev_devices, - GINT_TO_POINTER (rtnl_link_get_ifindex (rtnllink))); + sysfs_path = g_strdup_printf ("/sys/class/net/%s", ifname); + anycast_mask = g_strdup_printf ("%s/anycast_mask", sysfs_path); + if (g_file_test (anycast_mask, G_FILE_TEST_EXISTS)) + return_type (NM_LINK_TYPE_OLPC_MESH, "olpc-mesh"); + + devtype = read_devtype (sysfs_path); + if (devtype) { + if (wifi_utils_is_wifi (ifname, sysfs_path, devtype)) + return_type (NM_LINK_TYPE_WIFI, "wifi"); + else if (g_strcmp0 (devtype, "wwan") == 0) + return_type (NM_LINK_TYPE_WWAN_ETHERNET, "wwan"); + else if (g_strcmp0 (devtype, "wimax") == 0) + return_type (NM_LINK_TYPE_WIMAX, "wimax"); } - return udev_detect_link_type_from_device (udev_device, - ifname, - arptype, - out_name); + + if (arptype == ARPHRD_ETHER) + return_type (NM_LINK_TYPE_ETHERNET, "ethernet"); + + return_type (NM_LINK_TYPE_UNKNOWN, "unknown"); } else if (!strcmp (type, "dummy")) return_type (NM_LINK_TYPE_DUMMY, "dummy"); else if (!strcmp (type, "gre")) From fab106c912dae592caeb2fd3d5124b8d12be2c93 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Mon, 13 Apr 2015 17:16:55 -0500 Subject: [PATCH 02/12] core: add generic NMDevice function to recheck availability And use it everywhere. (cherry picked from commit 3006df940ca254ed999e527a6e797fd016e7ebc9) --- src/devices/bluetooth/nm-device-bt.c | 64 ++++++-------------------- src/devices/nm-device-private.h | 3 ++ src/devices/nm-device.c | 56 +++++++++++++++++++++- src/devices/wifi/nm-device-olpc-mesh.c | 12 ++--- src/devices/wifi/nm-device-wifi.c | 30 ++++-------- src/devices/wwan/nm-device-modem.c | 16 ++----- 6 files changed, 91 insertions(+), 90 deletions(-) diff --git a/src/devices/bluetooth/nm-device-bt.c b/src/devices/bluetooth/nm-device-bt.c index 852ca39971..88033e2e8e 100644 --- a/src/devices/bluetooth/nm-device-bt.c +++ b/src/devices/bluetooth/nm-device-bt.c @@ -479,6 +479,13 @@ device_state_changed (NMDevice *device, if (priv->modem) nm_modem_device_state_changed (priv->modem, new_state, old_state, reason); + + /* Need to recheck available connections whenever MM appears or disappears, + * since the device could be both DUN and NAP capable and thus may not + * change state (which rechecks available connections) when MM comes and goes. + */ + if (priv->mm_running && (priv->capabilities & NM_BT_CAPABILITY_DUN)) + nm_device_recheck_available_connections (device); } static void @@ -943,61 +950,20 @@ is_available (NMDevice *dev, NMDeviceCheckDevAvailableFlags flags) return priv->mm_running; } -static void -handle_availability_change (NMDeviceBt *self, - gboolean old_available, - NMDeviceStateReason unavailable_reason) -{ - NMDevice *device = NM_DEVICE (self); - NMDeviceState state; - gboolean available; - - state = nm_device_get_state (device); - if (state < NM_DEVICE_STATE_UNAVAILABLE) { - _LOGD (LOGD_BT, "availability blocked by UNMANAGED state"); - return; - } - - available = nm_device_is_available (device, NM_DEVICE_CHECK_DEV_AVAILABLE_NONE); - if (available == old_available) - return; - - if (available) { - if (state != NM_DEVICE_STATE_UNAVAILABLE) - _LOGW (LOGD_CORE | LOGD_BT, "not in expected unavailable state!"); - - nm_device_state_changed (device, - NM_DEVICE_STATE_DISCONNECTED, - NM_DEVICE_STATE_REASON_NONE); - } else { - nm_device_state_changed (device, - NM_DEVICE_STATE_UNAVAILABLE, - unavailable_reason); - } -} - static void set_mm_running (NMDeviceBt *self, gboolean running) { NMDeviceBtPrivate *priv = NM_DEVICE_BT_GET_PRIVATE (self); - gboolean old_available; - if (priv->mm_running == running) - return; + if (priv->mm_running != running) { + _LOGD (LOGD_BT, "ModemManager now %s", + running ? "available" : "unavailable"); - _LOGD (LOGD_BT, "ModemManager now %s", - running ? "available" : "unavailable"); - - old_available = nm_device_is_available (NM_DEVICE (self), NM_DEVICE_CHECK_DEV_AVAILABLE_NONE); - priv->mm_running = running; - handle_availability_change (self, old_available, NM_DEVICE_STATE_REASON_MODEM_MANAGER_UNAVAILABLE); - - /* Need to recheck available connections whenever MM appears or disappears, - * since the device could be both DUN and NAP capable and thus may not - * change state (which rechecks available connections) when MM comes and goes. - */ - if (priv->capabilities & NM_BT_CAPABILITY_DUN) - nm_device_recheck_available_connections (NM_DEVICE (self)); + priv->mm_running = running; + nm_device_queue_recheck_available (NM_DEVICE (self), + NM_DEVICE_STATE_REASON_NONE, + NM_DEVICE_STATE_REASON_MODEM_MANAGER_UNAVAILABLE); + } } static void diff --git a/src/devices/nm-device-private.h b/src/devices/nm-device-private.h index 1f781bbc05..f34969ac3d 100644 --- a/src/devices/nm-device-private.h +++ b/src/devices/nm-device-private.h @@ -97,6 +97,9 @@ void nm_device_set_carrier (NMDevice *self, gboolean carrier); void nm_device_emit_recheck_auto_activate (NMDevice *device); void nm_device_queue_recheck_assume (NMDevice *device); +void nm_device_queue_recheck_available (NMDevice *device, + NMDeviceStateReason available_reason, + NMDeviceStateReason unavailable_reason); void nm_device_set_wwan_ip4_config (NMDevice *device, NMIP4Config *config); void nm_device_set_wwan_ip6_config (NMDevice *device, NMIP6Config *config); diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index d0c5d8a332..63422acf1a 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -219,6 +219,11 @@ typedef struct { guint act_source6_id; gpointer act_source6_func; guint recheck_assume_id; + struct { + guint call_id; + NMDeviceStateReason available_reason; + NMDeviceStateReason unavailable_reason; + } recheck_available; struct { guint call_id; NMDeviceState post_state; @@ -2302,6 +2307,47 @@ nm_device_queue_recheck_assume (NMDevice *self) priv->recheck_assume_id = g_idle_add (nm_device_emit_recheck_assume, self); } +static gboolean +recheck_available (gpointer user_data) +{ + NMDevice *self = NM_DEVICE (user_data); + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + gboolean now_available = nm_device_is_available (self, NM_DEVICE_CHECK_DEV_AVAILABLE_NONE); + NMDeviceState state = nm_device_get_state (self); + NMDeviceState new_state = NM_DEVICE_STATE_UNKNOWN; + + priv->recheck_available.call_id = 0; + + if (state == NM_DEVICE_STATE_UNAVAILABLE && now_available) { + new_state = NM_DEVICE_STATE_DISCONNECTED; + nm_device_queue_state (self, new_state, priv->recheck_available.available_reason); + } else if (state >= NM_DEVICE_STATE_DISCONNECTED && !now_available) { + new_state = NM_DEVICE_STATE_UNAVAILABLE; + nm_device_queue_state (self, new_state, priv->recheck_available.unavailable_reason); + } + _LOGD (LOGD_DEVICE, "device is %savailable, %s %s", + now_available ? "" : "not ", + new_state == NM_DEVICE_STATE_UNAVAILABLE ? "no change required for" : "will transition to", + state_to_string (new_state == NM_DEVICE_STATE_UNAVAILABLE ? state : new_state)); + + priv->recheck_available.available_reason = NM_DEVICE_STATE_REASON_NONE; + priv->recheck_available.unavailable_reason = NM_DEVICE_STATE_REASON_NONE; + return G_SOURCE_REMOVE; +} + +void +nm_device_queue_recheck_available (NMDevice *self, + NMDeviceStateReason available_reason, + NMDeviceStateReason unavailable_reason) +{ + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + + priv->recheck_available.available_reason = available_reason; + priv->recheck_available.unavailable_reason = unavailable_reason; + if (!priv->recheck_available.call_id) + priv->recheck_available.call_id = g_idle_add (recheck_available, self); +} + void nm_device_emit_recheck_auto_activate (NMDevice *self) { @@ -7860,8 +7906,9 @@ _set_state_full (NMDevice *self, * reasons. */ if (nm_device_is_available (self, NM_DEVICE_CHECK_DEV_AVAILABLE_NONE)) { - _LOGD (LOGD_DEVICE, "device is available, will transition to DISCONNECTED"); - nm_device_queue_state (self, NM_DEVICE_STATE_DISCONNECTED, NM_DEVICE_STATE_REASON_NONE); + nm_device_queue_recheck_available (self, + NM_DEVICE_STATE_REASON_NONE, + NM_DEVICE_STATE_REASON_NONE); } else { if (old_state == NM_DEVICE_STATE_UNMANAGED) _LOGD (LOGD_DEVICE, "device not yet available for transition to DISCONNECTED"); @@ -8496,6 +8543,11 @@ dispose (GObject *object) priv->recheck_assume_id = 0; } + if (priv->recheck_available.call_id) { + g_source_remove (priv->recheck_available.call_id); + priv->recheck_available.call_id = 0; + } + link_disconnect_action_cancel (self); if (priv->con_provider) { diff --git a/src/devices/wifi/nm-device-olpc-mesh.c b/src/devices/wifi/nm-device-olpc-mesh.c index f8bf2f7d97..e67d1a20a6 100644 --- a/src/devices/wifi/nm-device-olpc-mesh.c +++ b/src/devices/wifi/nm-device-olpc-mesh.c @@ -369,9 +369,9 @@ device_added_cb (NMManager *manager, NMDevice *other, gpointer user_data) NMDeviceOlpcMeshPrivate *priv = NM_DEVICE_OLPC_MESH_GET_PRIVATE (self); if (!priv->companion && check_companion (self, other)) { - nm_device_state_changed (NM_DEVICE (self), - NM_DEVICE_STATE_DISCONNECTED, - NM_DEVICE_STATE_REASON_NONE); + nm_device_queue_recheck_available (NM_DEVICE (self), + NM_DEVICE_STATE_REASON_NONE, + NM_DEVICE_STATE_REASON_NONE); nm_device_remove_pending_action (NM_DEVICE (self), "waiting for companion", TRUE); } } @@ -399,9 +399,9 @@ find_companion (NMDeviceOlpcMesh *self) /* Try to find the companion if it's already known to the NMManager */ for (list = nm_manager_get_devices (nm_manager_get ()); list ; list = g_slist_next (list)) { if (check_companion (self, NM_DEVICE (list->data))) { - nm_device_queue_state (NM_DEVICE (self), - NM_DEVICE_STATE_DISCONNECTED, - NM_DEVICE_STATE_REASON_NONE); + nm_device_queue_recheck_available (NM_DEVICE (self), + NM_DEVICE_STATE_REASON_NONE, + NM_DEVICE_STATE_REASON_NONE); nm_device_remove_pending_action (NM_DEVICE (self), "waiting for companion", TRUE); break; } diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 3aec4a06d4..bf4702a63d 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -2149,6 +2149,7 @@ supplicant_iface_state_cb (NMSupplicantInterface *iface, NMDevice *device = NM_DEVICE (self); NMDeviceState devstate; gboolean scanning; + gboolean recheck_available = FALSE; if (new_state == old_state) return; @@ -2168,23 +2169,9 @@ supplicant_iface_state_cb (NMSupplicantInterface *iface, switch (new_state) { case NM_SUPPLICANT_INTERFACE_STATE_READY: + _LOGD (LOGD_WIFI_SCAN, "supplicant ready"); + recheck_available = TRUE; priv->scan_interval = SCAN_INTERVAL_MIN; - - /* If the interface can now be activated because the supplicant is now - * available, transition to DISCONNECTED. - */ - if ((devstate == NM_DEVICE_STATE_UNAVAILABLE) && nm_device_is_available (device, NM_DEVICE_CHECK_DEV_AVAILABLE_NONE)) { - nm_device_state_changed (device, - NM_DEVICE_STATE_DISCONNECTED, - NM_DEVICE_STATE_REASON_SUPPLICANT_AVAILABLE); - } - - _LOGD (LOGD_WIFI_SCAN, "supplicant ready, requesting initial scan"); - - /* Request a scan to get latest results */ - cancel_pending_scan (self); - request_wireless_scan (self); - if (old_state < NM_SUPPLICANT_INTERFACE_STATE_READY) nm_device_remove_pending_action (device, "waiting for supplicant", TRUE); break; @@ -2244,6 +2231,7 @@ supplicant_iface_state_cb (NMSupplicantInterface *iface, } break; case NM_SUPPLICANT_INTERFACE_STATE_DOWN: + recheck_available = TRUE; cleanup_association_attempt (self, FALSE); if (old_state < NM_SUPPLICANT_INTERFACE_STATE_READY) @@ -2256,15 +2244,17 @@ supplicant_iface_state_cb (NMSupplicantInterface *iface, */ supplicant_interface_release (self); supplicant_interface_acquire (self); - - nm_device_state_changed (device, - NM_DEVICE_STATE_UNAVAILABLE, - NM_DEVICE_STATE_REASON_SUPPLICANT_FAILED); break; default: break; } + if (recheck_available) { + nm_device_queue_recheck_available (NM_DEVICE (device), + NM_DEVICE_STATE_REASON_SUPPLICANT_AVAILABLE, + NM_DEVICE_STATE_REASON_SUPPLICANT_FAILED); + } + /* Signal scanning state changes */ if ( new_state == NM_SUPPLICANT_INTERFACE_STATE_SCANNING || old_state == NM_SUPPLICANT_INTERFACE_STATE_SCANNING) diff --git a/src/devices/wwan/nm-device-modem.c b/src/devices/wwan/nm-device-modem.c index a429aa0396..c02f5def9b 100644 --- a/src/devices/wwan/nm-device-modem.c +++ b/src/devices/wwan/nm-device-modem.c @@ -300,19 +300,9 @@ modem_state_cb (NMModem *modem, nm_device_recheck_available_connections (device); } - if ((dev_state >= NM_DEVICE_STATE_DISCONNECTED) && !nm_device_is_available (device, NM_DEVICE_CHECK_DEV_AVAILABLE_NONE)) { - nm_device_state_changed (device, - NM_DEVICE_STATE_UNAVAILABLE, - NM_DEVICE_STATE_REASON_MODEM_FAILED); - return; - } - - if ((dev_state == NM_DEVICE_STATE_UNAVAILABLE) && nm_device_is_available (device, NM_DEVICE_CHECK_DEV_AVAILABLE_NONE)) { - nm_device_state_changed (device, - NM_DEVICE_STATE_DISCONNECTED, - NM_DEVICE_STATE_REASON_MODEM_AVAILABLE); - return; - } + nm_device_queue_recheck_available (device, + NM_DEVICE_STATE_REASON_MODEM_AVAILABLE, + NM_DEVICE_STATE_REASON_MODEM_FAILED); } static void From 285ee1fda2052e5b59110a323082c9423bf882e0 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Mon, 13 Apr 2015 16:29:37 -0500 Subject: [PATCH 03/12] platform: don't wait for udev before announcing links (cherry picked from commit 388b7830f322b60960884328ff51f7b4df0ef3d3) --- src/devices/nm-device-ethernet.c | 12 +++ src/devices/nm-device.c | 43 +++++++++- src/devices/nm-device.h | 3 + src/nm-manager.c | 5 +- src/platform/nm-fake-platform.c | 1 + src/platform/nm-linux-platform.c | 132 ++++++++++--------------------- src/platform/nm-platform.c | 11 +++ src/platform/nm-platform.h | 3 + 8 files changed, 114 insertions(+), 96 deletions(-) diff --git a/src/devices/nm-device-ethernet.c b/src/devices/nm-device-ethernet.c index f50da78d64..041eb278e7 100644 --- a/src/devices/nm-device-ethernet.c +++ b/src/devices/nm-device-ethernet.c @@ -1615,6 +1615,17 @@ carrier_changed (NMDevice *device, gboolean carrier) NM_DEVICE_CLASS (nm_device_ethernet_parent_class)->carrier_changed (device, carrier); } +static void +link_changed (NMDevice *device, NMPlatformLink *info) +{ + NMDeviceEthernet *self = NM_DEVICE_ETHERNET (device); + NMDeviceEthernetPrivate *priv = NM_DEVICE_ETHERNET_GET_PRIVATE (self); + + NM_DEVICE_CLASS (nm_device_ethernet_parent_class)->link_changed (device, info); + if (!priv->subchan1 && info->udi) + _update_s390_subchannels (self); +} + static void dispose (GObject *object) { @@ -1714,6 +1725,7 @@ nm_device_ethernet_class_init (NMDeviceEthernetClass *klass) parent_class->spec_match_list = spec_match_list; parent_class->update_connection = update_connection; parent_class->carrier_changed = carrier_changed; + parent_class->link_changed = link_changed; parent_class->state_changed = device_state_changed; diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 63422acf1a..5e5655e0f2 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -175,6 +175,7 @@ typedef struct { typedef struct { gboolean in_state_changed; gboolean initialized; + gboolean platform_link_initialized; NMDeviceState state; NMDeviceStateReason state_reason; @@ -1042,6 +1043,7 @@ void nm_device_finish_init (NMDevice *self) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + gboolean platform_unmanaged = FALSE; g_assert (priv->initialized == FALSE); @@ -1054,6 +1056,20 @@ nm_device_finish_init (NMDevice *self) if (priv->master) nm_device_enslave_slave (priv->master, self, NULL); + if (priv->ifindex > 0) { + if (priv->platform_link_initialized || (priv->is_nm_owned && priv->is_software)) { + nm_platform_link_get_unmanaged (NM_PLATFORM_GET, priv->ifindex, &platform_unmanaged); + nm_device_set_initial_unmanaged_flag (self, NM_UNMANAGED_DEFAULT, platform_unmanaged); + } else { + /* Hardware and externally-created software links stay unmanaged + * until they are fully initialized by the platform. NM created + * links must be available for activation immediately and thus + * do not get the PLATFORM_INIT unmanaged flag set. + */ + nm_device_set_initial_unmanaged_flag (self, NM_UNMANAGED_PLATFORM_INIT, TRUE); + } + } + priv->initialized = TRUE; } @@ -1246,6 +1262,7 @@ device_link_changed (NMDevice *self, NMPlatformLink *info) NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); NMUtilsIPv6IfaceId token_iid; gboolean ip_ifname_changed = FALSE; + gboolean platform_unmanaged = FALSE; if (info->udi && g_strcmp0 (info->udi, priv->udi)) { /* Update UDI to what udev gives us */ @@ -1254,6 +1271,13 @@ device_link_changed (NMDevice *self, NMPlatformLink *info) g_object_notify (G_OBJECT (self), NM_DEVICE_UDI); } + if (g_strcmp0 (info->driver, priv->driver)) { + /* Update driver to what udev gives us */ + g_free (priv->driver); + priv->driver = g_strdup (info->driver); + g_object_notify (G_OBJECT (self), NM_DEVICE_DRIVER); + } + /* Update MTU if it has changed. */ if (priv->mtu != info->mtu) { priv->mtu = info->mtu; @@ -1346,6 +1370,22 @@ device_link_changed (NMDevice *self, NMPlatformLink *info) } } } + + if (priv->ifindex > 0 && !priv->platform_link_initialized && info->initialized) { + priv->platform_link_initialized = TRUE; + + if (nm_platform_link_get_unmanaged (NM_PLATFORM_GET, priv->ifindex, &platform_unmanaged)) { + nm_device_set_unmanaged (self, + NM_UNMANAGED_DEFAULT, + platform_unmanaged, + NM_DEVICE_STATE_REASON_USER_REQUESTED); + } + + nm_device_set_unmanaged (self, + NM_UNMANAGED_PLATFORM_INIT, + FALSE, + NM_DEVICE_STATE_REASON_NOW_MANAGED); + } } static void @@ -6965,7 +7005,7 @@ nm_device_set_unmanaged (NMDevice *self, if (unmanaged) nm_device_state_changed (self, NM_DEVICE_STATE_UNMANAGED, reason); - else + else if (nm_device_get_state (self) == NM_DEVICE_STATE_UNMANAGED) nm_device_state_changed (self, NM_DEVICE_STATE_UNAVAILABLE, reason); } } @@ -8622,6 +8662,7 @@ set_property (GObject *object, guint prop_id, priv->up = platform_device->up; g_free (priv->driver); priv->driver = g_strdup (platform_device->driver); + priv->platform_link_initialized = platform_device->initialized; } break; case PROP_UDI: diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h index 67bc18b0fd..9f30da6026 100644 --- a/src/devices/nm-device.h +++ b/src/devices/nm-device.h @@ -342,6 +342,8 @@ RfKillType nm_device_get_rfkill_type (NMDevice *device); * @NM_UNMANAGED_USER: %TRUE when unmanaged by user decision (via unmanaged-specs) * @NM_UNMANAGED_PARENT: %TRUE when unmanaged due to parent device being unmanaged * @NM_UNMANAGED_EXTERNAL_DOWN: %TRUE when unmanaged because !IFF_UP and not created by NM + * @NM_UNMANAGED_PLATFORM_INIT: %TRUE when unmanaged because platform link not + * yet initialized */ typedef enum { NM_UNMANAGED_NONE = 0x00, @@ -350,6 +352,7 @@ typedef enum { NM_UNMANAGED_USER = 0x04, NM_UNMANAGED_PARENT = 0x08, NM_UNMANAGED_EXTERNAL_DOWN = 0x10, + NM_UNMANAGED_PLATFORM_INIT = 0x20, /* Boundary value */ __NM_UNMANAGED_LAST, diff --git a/src/nm-manager.c b/src/nm-manager.c index 1a94479ae7..21ef828394 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -1797,7 +1797,7 @@ add_device (NMManager *self, NMDevice *device, gboolean try_assume) NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); const char *iface, *driver, *type_desc; const GSList *unmanaged_specs; - gboolean user_unmanaged, sleeping, platform_unmanaged; + gboolean user_unmanaged, sleeping; gboolean enabled = FALSE; RfKillType rtype; GSList *iter, *remove = NULL; @@ -1877,9 +1877,6 @@ add_device (NMManager *self, NMDevice *device, gboolean try_assume) user_unmanaged = nm_device_spec_match_list (device, unmanaged_specs); nm_device_set_initial_unmanaged_flag (device, NM_UNMANAGED_USER, user_unmanaged); - if (nm_platform_link_get_unmanaged (NM_PLATFORM_GET, nm_device_get_ifindex (device), &platform_unmanaged)) - nm_device_set_initial_unmanaged_flag (device, NM_UNMANAGED_DEFAULT, platform_unmanaged); - sleeping = manager_sleeping (self); nm_device_set_initial_unmanaged_flag (device, NM_UNMANAGED_INTERNAL, sleeping); diff --git a/src/platform/nm-fake-platform.c b/src/platform/nm-fake-platform.c index c9785b0035..b49e02d50f 100644 --- a/src/platform/nm-fake-platform.c +++ b/src/platform/nm-fake-platform.c @@ -116,6 +116,7 @@ link_init (NMFakePlatformLink *device, int ifindex, int type, const char *name) device->link.type_name = type_to_type_name (type); device->link.driver = type_to_type_name (type); device->link.udi = device->udi = g_strdup_printf ("fake:%d", ifindex); + device->link.initialized = TRUE; if (name) strcpy (device->link.name, name); switch (device->link.type) { diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index a51b6f7581..d945cf6bac 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -887,19 +887,6 @@ type_to_string (NMLinkType type) } } -static gboolean -link_is_announceable (NMPlatform *platform, struct rtnl_link *rtnllink) -{ - NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform); - - /* Hardware devices must be found by udev so rules get run and tags set */ - if (g_hash_table_lookup (priv->udev_devices, - GINT_TO_POINTER (rtnl_link_get_ifindex (rtnllink)))) - return TRUE; - - return FALSE; -} - #define DEVTYPE_PREFIX "DEVTYPE=" static char * @@ -1058,15 +1045,17 @@ init_link (NMPlatform *platform, NMPlatformLink *info, struct rtnl_link *rtnllin udev_device = g_hash_table_lookup (priv->udev_devices, GINT_TO_POINTER (info->ifindex)); if (udev_device) { info->driver = udev_get_driver (udev_device, info->ifindex); - if (!info->driver) - info->driver = g_intern_string (rtnl_link_get_type (rtnllink)); - if (!info->driver) - info->driver = ethtool_get_driver (info->name); - if (!info->driver) - info->driver = "unknown"; info->udi = g_udev_device_get_sysfs_path (udev_device); + info->initialized = TRUE; } + if (!info->driver) + info->driver = g_intern_string (rtnl_link_get_type (rtnllink)); + if (!info->driver) + info->driver = ethtool_get_driver (info->name); + if (!info->driver) + info->driver = "unknown"; + return TRUE; } @@ -1624,22 +1613,6 @@ announce_object (NMPlatform *platform, const struct nl_object *object, NMPlatfor if (!init_link (platform, &device, rtnl_link)) return; - /* Skip devices not yet discovered by udev. They will be - * announced by udev_device_added(). This doesn't apply to removed - * devices, as those come either from udev_device_removed(), - * event_notification() or link_delete() which block the announcment - * themselves when appropriate. - */ - switch (change_type) { - case NM_PLATFORM_SIGNAL_ADDED: - case NM_PLATFORM_SIGNAL_CHANGED: - if (!device.driver) - return; - break; - default: - break; - } - /* Link deletion or setting down is sometimes accompanied by address * and/or route deletion. * @@ -2067,15 +2040,12 @@ event_notification (struct nl_msg *msg, gpointer user_data) return NL_OK; nl_cache_remove (cached_object); - /* Don't announce removed interfaces that are not recognized by - * udev. They were either not yet discovered or they have been - * already removed and announced. - */ - if (event == RTM_DELLINK) { - if (!link_is_announceable (platform, (struct rtnl_link *) cached_object)) - return NL_OK; - } announce_object (platform, cached_object, NM_PLATFORM_SIGNAL_REMOVED, NM_PLATFORM_REASON_EXTERNAL); + if (event == RTM_DELLINK) { + int ifindex = rtnl_link_get_ifindex ((struct rtnl_link *) cached_object); + + g_hash_table_remove (priv->udev_devices, GINT_TO_POINTER (ifindex)); + } return NL_OK; case RTM_NEWLINK: @@ -2303,12 +2273,8 @@ link_get_all (NMPlatform *platform) struct nl_object *object; for (object = nl_cache_get_first (priv->link_cache); object; object = nl_cache_get_next (object)) { - struct rtnl_link *rtnl_link = (struct rtnl_link *) object; - - if (link_is_announceable (platform, rtnl_link)) { - if (init_link (platform, &device, rtnl_link)) - g_array_append_val (links, device); - } + if (init_link (platform, &device, (struct rtnl_link *) object)) + g_array_append_val (links, device); } return links; @@ -2321,13 +2287,7 @@ _nm_platform_link_get (NMPlatform *platform, int ifindex, NMPlatformLink *l) auto_nl_object struct rtnl_link *rtnllink = NULL; rtnllink = rtnl_link_get (priv->link_cache, ifindex); - if (rtnllink) { - if (link_is_announceable (platform, rtnllink)) { - if (init_link (platform, l, rtnllink)) - return TRUE; - } - } - return FALSE; + return (rtnllink && init_link (platform, l, rtnllink)); } static struct nl_object * @@ -2386,13 +2346,6 @@ link_get (NMPlatform *platform, int ifindex) return NULL; } - /* physical interfaces must be found by udev before they can be used */ - if (!link_is_announceable (platform, rtnllink)) { - platform->error = NM_PLATFORM_ERROR_NOT_FOUND; - rtnl_link_put (rtnllink); - return NULL; - } - return rtnllink; } @@ -4422,7 +4375,6 @@ udev_device_added (NMPlatform *platform, auto_nl_object struct rtnl_link *rtnllink = NULL; const char *ifname; int ifindex; - gboolean was_announceable = FALSE; ifname = g_udev_device_get_name (udev_device); if (!ifname) { @@ -4447,15 +4399,15 @@ udev_device_added (NMPlatform *platform, } rtnllink = rtnl_link_get (priv->link_cache, ifindex); - if (rtnllink) - was_announceable = link_is_announceable (platform, rtnllink); + if (!rtnllink) { + warning ("(%s): udev-add: interface not known via netlink; ignoring...", ifname); + return; + } g_hash_table_insert (priv->udev_devices, GINT_TO_POINTER (ifindex), g_object_ref (udev_device)); - /* Announce devices only if they also have been discovered via Netlink. */ - if (rtnllink && link_is_announceable (platform, rtnllink)) - announce_object (platform, (struct nl_object *) rtnllink, was_announceable ? NM_PLATFORM_SIGNAL_CHANGED : NM_PLATFORM_SIGNAL_ADDED, NM_PLATFORM_REASON_EXTERNAL); + announce_object (platform, (struct nl_object *) rtnllink, NM_PLATFORM_SIGNAL_CHANGED, NM_PLATFORM_REASON_EXTERNAL); } static void @@ -4463,9 +4415,7 @@ udev_device_removed (NMPlatform *platform, GUdevDevice *udev_device) { NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform); - auto_nl_object struct rtnl_link *rtnllink = NULL; int ifindex = 0; - gboolean was_announceable = FALSE; if (g_udev_device_get_property (udev_device, "IFINDEX")) ifindex = g_udev_device_get_property_as_int (udev_device, "IFINDEX"); @@ -4490,15 +4440,7 @@ udev_device_removed (NMPlatform *platform, if (ifindex <= 0) return; - rtnllink = rtnl_link_get (priv->link_cache, ifindex); - if (rtnllink) - was_announceable = link_is_announceable (platform, rtnllink); - g_hash_table_remove (priv->udev_devices, GINT_TO_POINTER (ifindex)); - - /* Announce device removal if it is no longer announceable. */ - if (was_announceable && !link_is_announceable (platform, rtnllink)) - announce_object (platform, (struct nl_object *) rtnllink, NM_PLATFORM_SIGNAL_REMOVED, NM_PLATFORM_REASON_EXTERNAL); } static void @@ -4543,8 +4485,6 @@ constructed (GObject *_object) NMPlatform *platform = NM_PLATFORM (_object); NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform); const char *udev_subsys[] = { "net", NULL }; - GUdevEnumerator *enumerator; - GList *devices, *iter; int channel_flags; gboolean status; int nle; @@ -4604,6 +4544,24 @@ constructed (GObject *_object) g_signal_connect (priv->udev_client, "uevent", G_CALLBACK (handle_udev_event), platform); priv->udev_devices = g_hash_table_new_full (NULL, NULL, NULL, g_object_unref); + /* request all IPv6 addresses (hopeing that there is at least one), to check for + * the IFA_FLAGS attribute. */ + nle = nl_rtgen_request (priv->nlh_event, RTM_GETADDR, AF_INET6, NLM_F_DUMP); + if (nle < 0) + nm_log_warn (LOGD_PLATFORM, "Netlink error: requesting RTM_GETADDR failed with %s", nl_geterror (nle)); + + priv->wifi_data = g_hash_table_new_full (NULL, NULL, NULL, (GDestroyNotify) wifi_utils_deinit); + + G_OBJECT_CLASS (nm_linux_platform_parent_class)->constructed (_object); +} + +static void +setup_devices (NMPlatform *platform) +{ + NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform); + GUdevEnumerator *enumerator; + GList *devices, *iter; + /* And read initial device list */ enumerator = g_udev_enumerator_new (priv->udev_client); g_udev_enumerator_add_match_subsystem (enumerator, "net"); @@ -4621,16 +4579,6 @@ constructed (GObject *_object) } g_list_free (devices); g_object_unref (enumerator); - - /* request all IPv6 addresses (hopeing that there is at least one), to check for - * the IFA_FLAGS attribute. */ - nle = nl_rtgen_request (priv->nlh_event, RTM_GETADDR, AF_INET6, NLM_F_DUMP); - if (nle < 0) - nm_log_warn (LOGD_PLATFORM, "Netlink error: requesting RTM_GETADDR failed with %s", nl_geterror (nle)); - - priv->wifi_data = g_hash_table_new_full (NULL, NULL, NULL, (GDestroyNotify) wifi_utils_deinit); - - G_OBJECT_CLASS (nm_linux_platform_parent_class)->constructed (_object); } static void @@ -4668,6 +4616,8 @@ nm_linux_platform_class_init (NMLinuxPlatformClass *klass) object_class->constructed = constructed; object_class->finalize = nm_linux_platform_finalize; + platform_class->setup_devices = setup_devices; + platform_class->sysctl_set = sysctl_set; platform_class->sysctl_get = sysctl_get; diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index 88bfa5c14f..b5201e37d8 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -407,6 +407,10 @@ nm_platform_query_devices (NMPlatform *self) NM_PLATFORM_REASON_INTERNAL); } g_array_unref (links_array); + + /* Platform specific device setup. */ + if (klass->setup_devices) + klass->setup_devices (self); } /** @@ -2794,6 +2798,12 @@ nm_platform_ip6_route_to_string (const NMPlatformIP6Route *route) return (((a)->field) < ((b)->field)) ? -1 : 1; \ } G_STMT_END +#define _CMP_FIELD_BOOL(a, b, field) \ + G_STMT_START { \ + if ((!((a)->field)) != (!((b)->field))) \ + return ((!((a)->field)) < (!((b)->field))) ? -1 : 1; \ + } G_STMT_END + #define _CMP_FIELD_STR(a, b, field) \ G_STMT_START { \ int c = strcmp ((a)->field, (b)->field); \ @@ -2830,6 +2840,7 @@ nm_platform_link_cmp (const NMPlatformLink *a, const NMPlatformLink *b) _CMP_FIELD (a, b, arp); _CMP_FIELD (a, b, mtu); _CMP_FIELD_STR0 (a, b, type_name); + _CMP_FIELD_BOOL (a, b, initialized); _CMP_FIELD_STR0 (a, b, udi); _CMP_FIELD_STR0 (a, b, driver); return 0; diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h index 2bdc7d79e5..626008a7ed 100644 --- a/src/platform/nm-platform.h +++ b/src/platform/nm-platform.h @@ -88,6 +88,7 @@ struct _NMPlatformLink { const char *type_name; const char *udi; const char *driver; + gboolean initialized; int master; int parent; gboolean up; @@ -358,6 +359,8 @@ struct _NMPlatform { typedef struct { GObjectClass parent; + void (*setup_devices) (NMPlatform *); + gboolean (*sysctl_set) (NMPlatform *, const char *path, const char *value); char * (*sysctl_get) (NMPlatform *, const char *path); From 7c4a657efaad4b9a5159df574ee187503ea18f55 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 4 May 2015 17:18:39 +0200 Subject: [PATCH 04/12] platform: fix root-tests after adding link detection without udev Co-Authored-By: Lubomir Rintel Fixes: 388b7830f322b60960884328ff51f7b4df0ef3d3 Fixes: 285ee1fda2052e5b59110a323082c9423bf882e0 (cherry picked from commit b22bf15c1d4aca4aada52debbe5f4641f5db24a4) --- src/platform/nm-fake-platform.c | 30 ++---------------------------- src/platform/tests/test-address.c | 2 +- src/platform/tests/test-cleanup.c | 2 +- src/platform/tests/test-link.c | 13 ++++++++----- src/platform/tests/test-route.c | 2 +- 5 files changed, 13 insertions(+), 36 deletions(-) diff --git a/src/platform/nm-fake-platform.c b/src/platform/nm-fake-platform.c index b49e02d50f..69776b6be9 100644 --- a/src/platform/nm-fake-platform.c +++ b/src/platform/nm-fake-platform.c @@ -49,11 +49,6 @@ typedef struct { int vlan_id; } NMFakePlatformLink; -typedef struct { - int ifindex; - NMPlatform *platform; -} NMFakePlatformLinkData; - #define NM_FAKE_PLATFORM_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), NM_TYPE_FAKE_PLATFORM, NMFakePlatformPrivate)) G_DEFINE_TYPE (NMFakePlatform, nm_fake_platform, NM_TYPE_PLATFORM) @@ -172,23 +167,6 @@ _nm_platform_link_get (NMPlatform *platform, int ifindex, NMPlatformLink *l) return !!device; } -static gboolean -_link_announce (NMFakePlatformLinkData *data) -{ - NMFakePlatformLink *device = link_get (data->platform, data->ifindex); - - if (device) - g_signal_emit_by_name (data->platform, - NM_PLATFORM_SIGNAL_LINK_CHANGED, - device->link.ifindex, - device, - NM_PLATFORM_SIGNAL_ADDED, - NM_PLATFORM_REASON_INTERNAL); - g_free (data); - - return FALSE; -} - static gboolean link_add (NMPlatform *platform, const char *name, NMLinkType type, const void *address, size_t address_len) { @@ -199,12 +177,8 @@ link_add (NMPlatform *platform, const char *name, NMLinkType type, const void *a g_array_append_val (priv->links, device); - if (device.link.ifindex) { - NMFakePlatformLinkData *data = g_new (NMFakePlatformLinkData, 1); - data->ifindex = device.link.ifindex; - data->platform = platform; - g_idle_add ((GSourceFunc) _link_announce, data); - } + if (device.link.ifindex) + g_signal_emit_by_name (platform, NM_PLATFORM_SIGNAL_LINK_CHANGED, device.link.ifindex, &device, NM_PLATFORM_SIGNAL_ADDED, NM_PLATFORM_REASON_INTERNAL); return TRUE; } diff --git a/src/platform/tests/test-address.c b/src/platform/tests/test-address.c index 0354795d47..bf117163d7 100644 --- a/src/platform/tests/test-address.c +++ b/src/platform/tests/test-address.c @@ -256,7 +256,7 @@ setup_tests (void) nm_platform_link_delete (NM_PLATFORM_GET, nm_platform_link_get_ifindex (NM_PLATFORM_GET, DEVICE_NAME)); g_assert (!nm_platform_link_exists (NM_PLATFORM_GET, DEVICE_NAME)); g_assert (nm_platform_dummy_add (NM_PLATFORM_GET, DEVICE_NAME)); - wait_signal (link_added); + accept_signal (link_added); free_signal (link_added); g_test_add_func ("/address/internal/ip4", test_ip4_address); diff --git a/src/platform/tests/test-cleanup.c b/src/platform/tests/test-cleanup.c index 29892c9c83..473489da81 100644 --- a/src/platform/tests/test-cleanup.c +++ b/src/platform/tests/test-cleanup.c @@ -36,7 +36,7 @@ test_cleanup_internal (void) /* Create and set up device */ g_assert (nm_platform_dummy_add (NM_PLATFORM_GET, DEVICE_NAME)); - wait_signal (link_added); + accept_signal (link_added); free_signal (link_added); g_assert (nm_platform_link_set_up (NM_PLATFORM_GET, nm_platform_link_get_ifindex (NM_PLATFORM_GET, DEVICE_NAME))); ifindex = nm_platform_link_get_ifindex (NM_PLATFORM_GET, DEVICE_NAME); diff --git a/src/platform/tests/test-link.c b/src/platform/tests/test-link.c index 6dddfc0d59..d82d0165e6 100644 --- a/src/platform/tests/test-link.c +++ b/src/platform/tests/test-link.c @@ -115,7 +115,7 @@ software_add (NMLinkType link_type, const char *name) /* Don't call link_callback for the bridge interface */ parent_added = add_signal_ifname (NM_PLATFORM_SIGNAL_LINK_CHANGED, NM_PLATFORM_SIGNAL_ADDED, link_callback, PARENT_NAME); if (nm_platform_bridge_add (NM_PLATFORM_GET, PARENT_NAME, NULL, 0)) - wait_signal (parent_added); + accept_signal (parent_added); free_signal (parent_added); { @@ -148,7 +148,7 @@ test_slave (int master, int type, SignalData *master_changed) g_assert (ifindex > 0); link_changed = add_signal_ifindex (NM_PLATFORM_SIGNAL_LINK_CHANGED, NM_PLATFORM_SIGNAL_CHANGED, link_callback, ifindex); link_removed = add_signal_ifindex (NM_PLATFORM_SIGNAL_LINK_CHANGED, NM_PLATFORM_SIGNAL_REMOVED, link_callback, ifindex); - wait_signal (link_added); + accept_signal (link_added); /* Set the slave up to see whether master's IFF_LOWER_UP is set correctly. * @@ -263,7 +263,7 @@ test_software (NMLinkType link_type, const char *link_typename) link_added = add_signal_ifname (NM_PLATFORM_SIGNAL_LINK_CHANGED, NM_PLATFORM_SIGNAL_ADDED, link_callback, DEVICE_NAME); g_assert (software_add (link_type, DEVICE_NAME)); no_error (); - wait_signal (link_added); + accept_signal (link_added); g_assert (nm_platform_link_exists (NM_PLATFORM_GET, DEVICE_NAME)); ifindex = nm_platform_link_get_ifindex (NM_PLATFORM_GET, DEVICE_NAME); g_assert (ifindex >= 0); @@ -405,7 +405,7 @@ test_internal (void) /* Add device */ g_assert (nm_platform_dummy_add (NM_PLATFORM_GET, DEVICE_NAME)); no_error (); - wait_signal (link_added); + accept_signal (link_added); /* Try to add again */ g_assert (!nm_platform_dummy_add (NM_PLATFORM_GET, DEVICE_NAME)); @@ -485,6 +485,7 @@ test_external (void) run_command ("ip link add %s type %s", DEVICE_NAME, "dummy"); wait_signal (link_added); + g_assert (nm_platform_link_exists (NM_PLATFORM_GET, DEVICE_NAME)); ifindex = nm_platform_link_get_ifindex (NM_PLATFORM_GET, DEVICE_NAME); g_assert (ifindex > 0); @@ -496,7 +497,7 @@ test_external (void) success = nm_platform_link_get (NM_PLATFORM_GET, ifindex, &link); g_assert (success); - if (!link.driver) { + if (!link.initialized) { /* we still lack the notification via UDEV. Expect another link changed signal. */ wait_signal (link_changed); } @@ -505,8 +506,10 @@ test_external (void) g_assert (!nm_platform_link_is_up (NM_PLATFORM_GET, ifindex)); g_assert (!nm_platform_link_is_connected (NM_PLATFORM_GET, ifindex)); g_assert (!nm_platform_link_uses_arp (NM_PLATFORM_GET, ifindex)); + run_command ("ip link set %s up", DEVICE_NAME); wait_signal (link_changed); + g_assert (nm_platform_link_is_up (NM_PLATFORM_GET, ifindex)); g_assert (nm_platform_link_is_connected (NM_PLATFORM_GET, ifindex)); run_command ("ip link set %s down", DEVICE_NAME); diff --git a/src/platform/tests/test-route.c b/src/platform/tests/test-route.c index e644816e11..6296dc89bb 100644 --- a/src/platform/tests/test-route.c +++ b/src/platform/tests/test-route.c @@ -321,7 +321,7 @@ setup_tests (void) nm_platform_link_delete (NM_PLATFORM_GET, nm_platform_link_get_ifindex (NM_PLATFORM_GET, DEVICE_NAME)); g_assert (!nm_platform_link_exists (NM_PLATFORM_GET, DEVICE_NAME)); g_assert (nm_platform_dummy_add (NM_PLATFORM_GET, DEVICE_NAME)); - wait_signal (link_added); + accept_signal (link_added); free_signal (link_added); g_assert (nm_platform_link_set_up (NM_PLATFORM_GET, nm_platform_link_get_ifindex (NM_PLATFORM_GET, DEVICE_NAME))); From 0ab54aeb479c2bb12753360c5b37c681e4bd353e Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Fri, 17 Apr 2015 10:04:21 +0200 Subject: [PATCH 05/12] platform: refactor link-type to string conversion (cherry picked from commit b484b03acf66f99ef59cc688de291f07f1cc5603) --- src/platform/nm-linux-platform.c | 108 ++++++++++++++++++++----------- 1 file changed, 72 insertions(+), 36 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index d945cf6bac..fee5f5b2c1 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -848,43 +848,79 @@ check_support_user_ipv6ll (NMPlatform *platform) /* Object type specific utilities */ +typedef struct { + const NMLinkType nm_type; + const char *type_string; + + /* IFLA_INFO_KIND / rtnl_link_get_type() where applicable; the rtnl type + * should only be specified if the device type can be created without + * additional parameters, and if the device type can be determined from + * the rtnl_type. eg, tun/tap should not be specified since both + * tun and tap devices use "tun", and InfiniBand should not be + * specified because a PKey is required at creation. Drivers set this + * value from their 'struct rtnl_link_ops' structure. + */ + const char *rtnl_type; + + /* uevent DEVTYPE where applicable, from /sys/class/net//uevent; + * drivers set this value from their SET_NETDEV_DEV() call and the + * 'struct device_type' name member. + */ + const char *devtype; +} LinkDesc; + +static const LinkDesc linktypes[] = { + { NM_LINK_TYPE_NONE, "none", NULL, NULL }, + { NM_LINK_TYPE_UNKNOWN, "unknown", NULL, NULL }, + + { NM_LINK_TYPE_ETHERNET, "ethernet", NULL, NULL }, + { NM_LINK_TYPE_INFINIBAND, "infiniband", NULL, NULL }, + { NM_LINK_TYPE_OLPC_MESH, "olpc-mesh", NULL, NULL }, + { NM_LINK_TYPE_WIFI, "wifi", NULL, "wlan" }, + { NM_LINK_TYPE_WWAN_ETHERNET, "wwan", NULL, "wwan" }, + { NM_LINK_TYPE_WIMAX, "wimax", "wimax", "wimax" }, + + { NM_LINK_TYPE_DUMMY, "dummy", "dummy", NULL }, + { NM_LINK_TYPE_GRE, "gre", "gre", NULL }, + { NM_LINK_TYPE_GRETAP, "gretap", "gretap", NULL }, + { NM_LINK_TYPE_IFB, "ifb", "ifb", NULL }, + { NM_LINK_TYPE_LOOPBACK, "loopback", NULL, NULL }, + { NM_LINK_TYPE_MACVLAN, "macvlan", "macvlan", NULL }, + { NM_LINK_TYPE_MACVTAP, "macvtap", "macvtap", NULL }, + { NM_LINK_TYPE_OPENVSWITCH, "openvswitch", "openvswitch", NULL }, + { NM_LINK_TYPE_TAP, "tap", NULL, NULL }, + { NM_LINK_TYPE_TUN, "tun", NULL, NULL }, + { NM_LINK_TYPE_VETH, "veth", "veth", NULL }, + { NM_LINK_TYPE_VLAN, "vlan", "vlan", "vlan" }, + { NM_LINK_TYPE_VXLAN, "vxlan", "vxlan", "vxlan" }, + + { NM_LINK_TYPE_BRIDGE, "bridge", "bridge", "bridge" }, + { NM_LINK_TYPE_BOND, "bond", "bond", "bond" }, + { NM_LINK_TYPE_TEAM, "team", "team", NULL }, +}; + static const char * -type_to_string (NMLinkType type) +nm_link_type_to_rtnl_type_string (NMLinkType type) { - /* Note that this only has to support virtual types */ - switch (type) { - case NM_LINK_TYPE_DUMMY: - return "dummy"; - case NM_LINK_TYPE_GRE: - return "gre"; - case NM_LINK_TYPE_GRETAP: - return "gretap"; - case NM_LINK_TYPE_IFB: - return "ifb"; - case NM_LINK_TYPE_MACVLAN: - return "macvlan"; - case NM_LINK_TYPE_MACVTAP: - return "macvtap"; - case NM_LINK_TYPE_TAP: - return "tap"; - case NM_LINK_TYPE_TUN: - return "tun"; - case NM_LINK_TYPE_VETH: - return "veth"; - case NM_LINK_TYPE_VLAN: - return "vlan"; - case NM_LINK_TYPE_VXLAN: - return "vxlan"; - case NM_LINK_TYPE_BRIDGE: - return "bridge"; - case NM_LINK_TYPE_BOND: - return "bond"; - case NM_LINK_TYPE_TEAM: - return "team"; - default: - g_warning ("Wrong type: %d", type); - return NULL; + int i; + + for (i = 0; i < G_N_ELEMENTS (linktypes); i++) { + if (type == linktypes[i].nm_type) + return linktypes[i].rtnl_type; } + g_return_val_if_reached (NULL); +} + +static const char * +nm_link_type_to_string (NMLinkType type) +{ + int i; + + for (i = 0; i < G_N_ELEMENTS (linktypes); i++) { + if (type == linktypes[i].nm_type) + return linktypes[i].type_string; + } + g_return_val_if_reached (NULL); } #define DEVTYPE_PREFIX "DEVTYPE=" @@ -2298,7 +2334,7 @@ build_rtnl_link (int ifindex, const char *name, NMLinkType type) rtnllink = _nm_rtnl_link_alloc (ifindex, name); if (type) { - nle = rtnl_link_set_type (rtnllink, type_to_string (type)); + nle = rtnl_link_set_type (rtnllink, nm_link_type_to_rtnl_type_string (type)); g_assert (!nle); } return (struct nl_object *) rtnllink; @@ -2322,7 +2358,7 @@ link_add (NMPlatform *platform, const char *name, NMLinkType type, const void *a } debug ("link: add link '%s' of type '%s' (%d)", - name, type_to_string (type), (int) type); + name, nm_link_type_to_string (type), (int) type); l = build_rtnl_link (0, name, type); From 19cdd7d9a16166fdbd802e3966445f3efba334ed Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Tue, 14 Apr 2015 17:18:34 -0500 Subject: [PATCH 06/12] platform: rework link type detection for better fallback (bgo #743209) See "Revert "wireless: Support of IFLA_INFO_KIND rtnl attribute"" http://www.spinics.net/lists/linux-wireless/msg132219.html The reverted kernel patch caused rtnl_link_get_type() to return "wlan" for WiFi devices. Since NM depends on this function returning NULL for WiFi devices so that it goes on to check the sysfs DEVTYPE attribute, the kernel patch caused WiFi devices to show up as Generic ones instead. That's wrong, and NM should be able to more easily handle changes in the kernel drivers from NULL to a more descriptive rtnl_link_get_type() return, since that's the kernel trend. What NM should be doing here is to fall back to other detection schemes if the type is NULL or unrecognized. Make that happen and clean things up to use a table instead of a giant if(strcmp()) block. https://bugzilla.gnome.org/show_bug.cgi?id=743209 (cherry picked from commit 2d527b30ffb57e458f8880e8cf4f5801d648ffeb) --- src/platform/nm-linux-platform.c | 128 ++++++++++++++----------------- src/platform/wifi/wifi-utils.c | 9 +-- src/platform/wifi/wifi-utils.h | 2 +- 3 files changed, 59 insertions(+), 80 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index fee5f5b2c1..7f309cea4f 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -951,31 +951,55 @@ read_devtype (const char *sysfs_path) static NMLinkType link_extract_type (NMPlatform *platform, struct rtnl_link *rtnllink, const char **out_name) { - const char *type; + const char *rtnl_type, *ifname; + int i, arptype; if (!rtnllink) return_type (NM_LINK_TYPE_NONE, NULL); - type = rtnl_link_get_type (rtnllink); + rtnl_type = rtnl_link_get_type (rtnllink); + if (rtnl_type) { + for (i = 0; i < G_N_ELEMENTS (linktypes); i++) { + if (g_strcmp0 (rtnl_type, linktypes[i].rtnl_type) == 0) + return_type (linktypes[i].nm_type, linktypes[i].type_string); + } - if (!type) { - int arptype = rtnl_link_get_arptype (rtnllink); - const char *driver; - const char *ifname; - gs_free char *anycast_mask = NULL; + if (!strcmp (rtnl_type, "tun")) { + NMPlatformTunProperties props; + guint flags; + + if (nm_platform_tun_get_properties (platform, rtnl_link_get_ifindex (rtnllink), &props)) { + if (!g_strcmp0 (props.mode, "tap")) + return_type (NM_LINK_TYPE_TAP, "tap"); + if (!g_strcmp0 (props.mode, "tun")) + return_type (NM_LINK_TYPE_TUN, "tun"); + } + flags = rtnl_link_get_flags (rtnllink); + + nm_log_dbg (LOGD_PLATFORM, "Failed to read tun properties for interface %d (link flags: %X)", + rtnl_link_get_ifindex (rtnllink), flags); + + /* try guessing the type using the link flags instead... */ + if (flags & IFF_POINTOPOINT) + return_type (NM_LINK_TYPE_TUN, "tun"); + return_type (NM_LINK_TYPE_TAP, "tap"); + } + } + + arptype = rtnl_link_get_arptype (rtnllink); + if (arptype == ARPHRD_LOOPBACK) + return_type (NM_LINK_TYPE_LOOPBACK, "loopback"); + else if (arptype == ARPHRD_INFINIBAND) + return_type (NM_LINK_TYPE_INFINIBAND, "infiniband"); + + + ifname = rtnl_link_get_name (rtnllink); + if (ifname) { + const char *driver = ethtool_get_driver (ifname); gs_free char *sysfs_path = NULL; + gs_free char *anycast_mask = NULL; gs_free char *devtype = NULL; - if (arptype == ARPHRD_LOOPBACK) - return_type (NM_LINK_TYPE_LOOPBACK, "loopback"); - else if (arptype == ARPHRD_INFINIBAND) - return_type (NM_LINK_TYPE_INFINIBAND, "infiniband"); - - ifname = rtnl_link_get_name (rtnllink); - if (!ifname) - return_type (NM_LINK_TYPE_UNKNOWN, type); - - driver = ethtool_get_driver (ifname); if (arptype == 256) { /* Some s390 CTC-type devices report 256 for the encapsulation type * for some reason, but we need to call them Ethernet. @@ -984,6 +1008,7 @@ link_extract_type (NMPlatform *platform, struct rtnl_link *rtnllink, const char return_type (NM_LINK_TYPE_ETHERNET, "ethernet"); } + /* Fallback OVS detection for kernel <= 3.16 */ if (!g_strcmp0 (driver, "openvswitch")) return_type (NM_LINK_TYPE_OPENVSWITCH, "openvswitch"); @@ -993,64 +1018,25 @@ link_extract_type (NMPlatform *platform, struct rtnl_link *rtnllink, const char return_type (NM_LINK_TYPE_OLPC_MESH, "olpc-mesh"); devtype = read_devtype (sysfs_path); - if (devtype) { - if (wifi_utils_is_wifi (ifname, sysfs_path, devtype)) - return_type (NM_LINK_TYPE_WIFI, "wifi"); - else if (g_strcmp0 (devtype, "wwan") == 0) - return_type (NM_LINK_TYPE_WWAN_ETHERNET, "wwan"); - else if (g_strcmp0 (devtype, "wimax") == 0) - return_type (NM_LINK_TYPE_WIMAX, "wimax"); + for (i = 0; devtype && i < G_N_ELEMENTS (linktypes); i++) { + if (g_strcmp0 (devtype, linktypes[i].devtype) == 0) + return_type (linktypes[i].nm_type, linktypes[i].type_string); } - if (arptype == ARPHRD_ETHER) + /* Fallback for drivers that don't call SET_NETDEV_DEVTYPE() */ + if (wifi_utils_is_wifi (ifname, sysfs_path)) + return_type (NM_LINK_TYPE_WIFI, "wifi"); + + /* Standard wired ethernet interfaces don't report an rtnl_link_type, so + * only allow fallback to Ethernet if no type is given. This should + * prevent future virtual network drivers from being treated as Ethernet + * when they should be Generic instead. + */ + if (arptype == ARPHRD_ETHER && !rtnl_type && !devtype) return_type (NM_LINK_TYPE_ETHERNET, "ethernet"); + } - return_type (NM_LINK_TYPE_UNKNOWN, "unknown"); - } else if (!strcmp (type, "dummy")) - return_type (NM_LINK_TYPE_DUMMY, "dummy"); - else if (!strcmp (type, "gre")) - return_type (NM_LINK_TYPE_GRE, "gre"); - else if (!strcmp (type, "gretap")) - return_type (NM_LINK_TYPE_GRETAP, "gretap"); - else if (!strcmp (type, "ifb")) - return_type (NM_LINK_TYPE_IFB, "ifb"); - else if (!strcmp (type, "macvlan")) - return_type (NM_LINK_TYPE_MACVLAN, "macvlan"); - else if (!strcmp (type, "macvtap")) - return_type (NM_LINK_TYPE_MACVTAP, "macvtap"); - else if (!strcmp (type, "tun")) { - NMPlatformTunProperties props; - guint flags; - - if (nm_platform_tun_get_properties (platform, rtnl_link_get_ifindex (rtnllink), &props)) { - if (!g_strcmp0 (props.mode, "tap")) - return_type (NM_LINK_TYPE_TAP, "tap"); - if (!g_strcmp0 (props.mode, "tun")) - return_type (NM_LINK_TYPE_TUN, "tun"); - } - flags = rtnl_link_get_flags (rtnllink); - - nm_log_dbg (LOGD_PLATFORM, "Failed to read tun properties for interface %d (link flags: %X)", - rtnl_link_get_ifindex (rtnllink), flags); - - /* try guessing the type using the link flags instead... */ - if (flags & IFF_POINTOPOINT) - return_type (NM_LINK_TYPE_TUN, "tun"); - return_type (NM_LINK_TYPE_TAP, "tap"); - } else if (!strcmp (type, "veth")) - return_type (NM_LINK_TYPE_VETH, "veth"); - else if (!strcmp (type, "vlan")) - return_type (NM_LINK_TYPE_VLAN, "vlan"); - else if (!strcmp (type, "vxlan")) - return_type (NM_LINK_TYPE_VXLAN, "vxlan"); - else if (!strcmp (type, "bridge")) - return_type (NM_LINK_TYPE_BRIDGE, "bridge"); - else if (!strcmp (type, "bond")) - return_type (NM_LINK_TYPE_BOND, "bond"); - else if (!strcmp (type, "team")) - return_type (NM_LINK_TYPE_TEAM, "team"); - - return_type (NM_LINK_TYPE_UNKNOWN, type); + return_type (NM_LINK_TYPE_UNKNOWN, rtnl_type ? rtnl_type : "unknown"); } static gboolean diff --git a/src/platform/wifi/wifi-utils.c b/src/platform/wifi/wifi-utils.c index daef881bf9..068c404e0b 100644 --- a/src/platform/wifi/wifi-utils.c +++ b/src/platform/wifi/wifi-utils.c @@ -162,20 +162,13 @@ wifi_utils_deinit (WifiData *data) } gboolean -wifi_utils_is_wifi (const char *iface, const char *sysfs_path, const char *devtype) +wifi_utils_is_wifi (const char *iface, const char *sysfs_path) { char phy80211_path[255]; struct stat s; g_return_val_if_fail (iface != NULL, FALSE); - if (g_strcmp0 (devtype, "wlan") == 0) { - /* All Wi-Fi drivers should set DEVTYPE=wlan. Since the kernel's - * cfg80211/nl80211 stack does, this check should match any nl80211 - * capable driver (including mac82011-based ones). */ - return TRUE; - } - if (sysfs_path) { /* Check for nl80211 sysfs paths */ g_snprintf (phy80211_path, sizeof (phy80211_path), "%s/phy80211", sysfs_path); diff --git a/src/platform/wifi/wifi-utils.h b/src/platform/wifi/wifi-utils.h index eb37bc22c9..588386ab36 100644 --- a/src/platform/wifi/wifi-utils.h +++ b/src/platform/wifi/wifi-utils.h @@ -29,7 +29,7 @@ typedef struct WifiData WifiData; -gboolean wifi_utils_is_wifi (const char *iface, const char *sysfs_path, const char *devtype); +gboolean wifi_utils_is_wifi (const char *iface, const char *sysfs_path); WifiData *wifi_utils_init (const char *iface, int ifindex, gboolean check_scan); From 251193490a2ec826e0f84500cd51ea0b69bd1a3e Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Thu, 23 Apr 2015 13:39:17 -0500 Subject: [PATCH 07/12] core: change activation failure messages to debug level Otherwise any user with network control privileges can spam the logs. (cherry picked from commit 8a5910c25cd0102d36a9adbb7a2c831e4fa5e046) --- src/nm-manager.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/nm-manager.c b/src/nm-manager.c index 21ef828394..766362f48c 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -2975,9 +2975,9 @@ _internal_activation_failed (NMManager *self, NMActiveConnection *active, const char *error_desc) { - nm_log_warn (LOGD_CORE, "Failed to activate '%s': %s", - nm_connection_get_id (nm_active_connection_get_connection (active)), - error_desc); + nm_log_dbg (LOGD_CORE, "Failed to activate '%s': %s", + nm_connection_get_id (nm_active_connection_get_connection (active)), + error_desc); if (nm_active_connection_get_state (active) <= NM_ACTIVE_CONNECTION_STATE_ACTIVATED) { nm_active_connection_set_state (active, NM_ACTIVE_CONNECTION_STATE_DEACTIVATING); From b4a88db19e19fbda07edf41fa000579d18dff9b0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 23 Apr 2015 23:24:34 +0200 Subject: [PATCH 08/12] core: remove G_GNUC_WARN_UNUSED_RESULT from ASSERT_VALID_PATH_COMPONENT() ASSERT_VALID_PATH_COMPONENT() always returns the input argument -- unless it fails an assertion and terminates the program. No need to require the user to use the return value. (cherry picked from commit 63bb33b5346b431d92e413b8bc6bafc50c8ff9b7) --- src/NetworkManagerUtils.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/NetworkManagerUtils.h b/src/NetworkManagerUtils.h index c2d4948a4d..3bd7252876 100644 --- a/src/NetworkManagerUtils.h +++ b/src/NetworkManagerUtils.h @@ -173,7 +173,7 @@ gint64 nm_utils_get_monotonic_timestamp_us (void); gint64 nm_utils_get_monotonic_timestamp_ms (void); gint32 nm_utils_get_monotonic_timestamp_s (void); -const char *ASSERT_VALID_PATH_COMPONENT (const char *name) G_GNUC_WARN_UNUSED_RESULT; +const char *ASSERT_VALID_PATH_COMPONENT (const char *name); const char *nm_utils_ip6_property_path (const char *ifname, const char *property); const char *nm_utils_ip4_property_path (const char *ifname, const char *property); From bae47558ba31d1e658facd8f249a0e2d3ede1e53 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 23 Apr 2015 23:16:00 +0200 Subject: [PATCH 09/12] platform: detect TUN/TAP device in link_extract_type() independently of platform cache link_extract_type() would call tun_get_properties() to determine whether the link if a TAP or TUN device. The previous implementation would receive the ifindex, and resolve the ifname via lookup in the platform cache. This means, the call on link_extract_type() will only succeed to detect the TUN/TAP properties, if the libnl object is already in the cache. Currently that is always the case and there is no problem. It is desireable, that we can resolve the link type of an object without consulting the platform cache first. (cherry picked from commit 18d611d5d2a613204e7123fda3d3f75271722df6) --- src/platform/nm-linux-platform.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 7f309cea4f..36cbbc637a 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -120,6 +120,8 @@ return t; \ } G_STMT_END +static gboolean tun_get_properties_ifname (NMPlatform *platform, const char *ifname, NMPlatformTunProperties *props); + /****************************************************************** * libnl unility functions and wrappers ******************************************************************/ @@ -968,7 +970,7 @@ link_extract_type (NMPlatform *platform, struct rtnl_link *rtnllink, const char NMPlatformTunProperties props; guint flags; - if (nm_platform_tun_get_properties (platform, rtnl_link_get_ifindex (rtnllink), &props)) { + if (tun_get_properties_ifname (platform, rtnl_link_get_name (rtnllink), &props)) { if (!g_strcmp0 (props.mode, "tap")) return_type (NM_LINK_TYPE_TAP, "tap"); if (!g_strcmp0 (props.mode, "tun")) @@ -3064,9 +3066,8 @@ veth_get_properties (NMPlatform *platform, int ifindex, NMPlatformVethProperties } static gboolean -tun_get_properties (NMPlatform *platform, int ifindex, NMPlatformTunProperties *props) +tun_get_properties_ifname (NMPlatform *platform, const char *ifname, NMPlatformTunProperties *props) { - const char *ifname; char *path, *val; gboolean success = TRUE; @@ -3076,7 +3077,6 @@ tun_get_properties (NMPlatform *platform, int ifindex, NMPlatformTunProperties * props->owner = -1; props->group = -1; - ifname = nm_platform_link_get_name (platform, ifindex); if (!ifname || !nm_utils_iface_valid_name (ifname)) return FALSE; ifname = ASSERT_VALID_PATH_COMPONENT (ifname); @@ -3127,6 +3127,12 @@ tun_get_properties (NMPlatform *platform, int ifindex, NMPlatformTunProperties * return success; } +static gboolean +tun_get_properties (NMPlatform *platform, int ifindex, NMPlatformTunProperties *props) +{ + return tun_get_properties_ifname (platform, nm_platform_link_get_name (platform, ifindex), props); +} + static const struct nla_policy macvlan_info_policy[IFLA_MACVLAN_MAX + 1] = { [IFLA_MACVLAN_MODE] = { .type = NLA_U32 }, #ifdef MACVLAN_FLAG_NOPROMISC From 3d1ab669e3000592aeb73c1a4d47c71a0b68938d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 28 Apr 2015 07:55:30 +0200 Subject: [PATCH 10/12] platform: expose nm_link_type_to_string() function Given the name nm_link_type_to_string(), we would not expect to find it in nm-linux-platform.c. It either should be named nm_platform_link_type_to_string() and be put in a new nm-platform-utils.c file, or it should be named nm_utils_link_type_to_string() and be put in NetworkManagerUtils.h. For now, just leave it here. (cherry picked from commit b538adf12393f67d60f7f5ec74b2a3c1b968f35b) --- src/platform/nm-linux-platform.c | 2 +- src/platform/nm-platform.h | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 36cbbc637a..6ee5f4b053 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -913,7 +913,7 @@ nm_link_type_to_rtnl_type_string (NMLinkType type) g_return_val_if_reached (NULL); } -static const char * +const char * nm_link_type_to_string (NMLinkType type) { int i; diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h index 626008a7ed..871daac0d6 100644 --- a/src/platform/nm-platform.h +++ b/src/platform/nm-platform.h @@ -499,6 +499,8 @@ NMPlatform *nm_platform_try_get (void); /******************************************************************/ +const char *nm_link_type_to_string (NMLinkType link_type); + void nm_platform_set_error (NMPlatform *self, NMPlatformError error); NMPlatformError nm_platform_get_error (NMPlatform *self); const char *nm_platform_get_error_msg (NMPlatform *self); From efec4b42e2b4e0c64136b7ecac04ae28f49c70b5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 28 Apr 2015 10:11:04 +0200 Subject: [PATCH 11/12] platform: refactor extraction of type-name for link link_extract_type() would return the NMLinkType and a @type_name string. If the type was unknown, this string was rtnl_link_get_type() (IFLA_INFO_KIND). Split up this behavior and treat those values independently. link_extract_type() now only detects the NMLinkType. Most users don't care about unknown types and can just use nm_link_type_to_string() to get a string represenation. Only nm_platform_link_get_type_name() (and NMDeviceGeneric:type_description) cared about a more descriptive type. For that, modify link_get_type_name() to return nm_link_type_to_string() if NMLinkType could be detected. As fallback, return rtnl_link_get_type(). Also, rename the field NMPlatformLink:link_type to "kind". For now this field is mostly unused. It will be used later when refactoring platform caching. (cherry picked from commit e2c742c77b7c708d6a5e6f689ff7cc91b23ab445) --- src/nm-manager.c | 2 +- src/platform/nm-fake-platform.c | 2 +- src/platform/nm-linux-platform.c | 70 ++++++++++++++++++-------------- src/platform/nm-platform.c | 26 +++++++++--- src/platform/nm-platform.h | 5 ++- src/platform/tests/dump.c | 2 +- 6 files changed, 66 insertions(+), 41 deletions(-) diff --git a/src/nm-manager.c b/src/nm-manager.c index 766362f48c..8ee7589670 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -2209,7 +2209,7 @@ platform_link_added (NMManager *self, case NM_LINK_TYPE_WIFI: case NM_LINK_TYPE_WIMAX: nm_log_info (LOGD_HW, "(%s): '%s' plugin not available; creating generic device", - plink->name, plink->type_name); + plink->name, nm_link_type_to_string (plink->type)); /* fall through */ default: device = nm_device_generic_new (plink); diff --git a/src/platform/nm-fake-platform.c b/src/platform/nm-fake-platform.c index 69776b6be9..0da6590ffb 100644 --- a/src/platform/nm-fake-platform.c +++ b/src/platform/nm-fake-platform.c @@ -108,7 +108,7 @@ link_init (NMFakePlatformLink *device, int ifindex, int type, const char *name) device->link.ifindex = name ? ifindex : 0; device->link.type = type; - device->link.type_name = type_to_type_name (type); + device->link.kind = type_to_type_name (type); device->link.driver = type_to_type_name (type); device->link.udi = device->udi = g_strdup_printf ("fake:%d", ifindex); device->link.initialized = TRUE; diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 6ee5f4b053..df8456e885 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -111,15 +111,6 @@ #define warning(...) _LOG (LOGL_WARN , _LOG_DOMAIN, NULL, __VA_ARGS__) #define error(...) _LOG (LOGL_ERR , _LOG_DOMAIN, NULL, __VA_ARGS__) -/******************************************************************/ - -#define return_type(t, name) \ - G_STMT_START { \ - if (out_name) \ - *out_name = name; \ - return t; \ - } G_STMT_END - static gboolean tun_get_properties_ifname (NMPlatform *platform, const char *ifname, NMPlatformTunProperties *props); /****************************************************************** @@ -951,19 +942,19 @@ read_devtype (const char *sysfs_path) } static NMLinkType -link_extract_type (NMPlatform *platform, struct rtnl_link *rtnllink, const char **out_name) +link_extract_type (NMPlatform *platform, struct rtnl_link *rtnllink) { const char *rtnl_type, *ifname; int i, arptype; if (!rtnllink) - return_type (NM_LINK_TYPE_NONE, NULL); + return NM_LINK_TYPE_NONE; rtnl_type = rtnl_link_get_type (rtnllink); if (rtnl_type) { for (i = 0; i < G_N_ELEMENTS (linktypes); i++) { if (g_strcmp0 (rtnl_type, linktypes[i].rtnl_type) == 0) - return_type (linktypes[i].nm_type, linktypes[i].type_string); + return linktypes[i].nm_type; } if (!strcmp (rtnl_type, "tun")) { @@ -972,9 +963,9 @@ link_extract_type (NMPlatform *platform, struct rtnl_link *rtnllink, const char if (tun_get_properties_ifname (platform, rtnl_link_get_name (rtnllink), &props)) { if (!g_strcmp0 (props.mode, "tap")) - return_type (NM_LINK_TYPE_TAP, "tap"); + return NM_LINK_TYPE_TAP; if (!g_strcmp0 (props.mode, "tun")) - return_type (NM_LINK_TYPE_TUN, "tun"); + return NM_LINK_TYPE_TUN; } flags = rtnl_link_get_flags (rtnllink); @@ -983,16 +974,16 @@ link_extract_type (NMPlatform *platform, struct rtnl_link *rtnllink, const char /* try guessing the type using the link flags instead... */ if (flags & IFF_POINTOPOINT) - return_type (NM_LINK_TYPE_TUN, "tun"); - return_type (NM_LINK_TYPE_TAP, "tap"); + return NM_LINK_TYPE_TUN; + return NM_LINK_TYPE_TAP; } } arptype = rtnl_link_get_arptype (rtnllink); if (arptype == ARPHRD_LOOPBACK) - return_type (NM_LINK_TYPE_LOOPBACK, "loopback"); + return NM_LINK_TYPE_LOOPBACK; else if (arptype == ARPHRD_INFINIBAND) - return_type (NM_LINK_TYPE_INFINIBAND, "infiniband"); + return NM_LINK_TYPE_INFINIBAND; ifname = rtnl_link_get_name (rtnllink); @@ -1007,27 +998,27 @@ link_extract_type (NMPlatform *platform, struct rtnl_link *rtnllink, const char * for some reason, but we need to call them Ethernet. */ if (!g_strcmp0 (driver, "ctcm")) - return_type (NM_LINK_TYPE_ETHERNET, "ethernet"); + return NM_LINK_TYPE_ETHERNET; } /* Fallback OVS detection for kernel <= 3.16 */ if (!g_strcmp0 (driver, "openvswitch")) - return_type (NM_LINK_TYPE_OPENVSWITCH, "openvswitch"); + return NM_LINK_TYPE_OPENVSWITCH; sysfs_path = g_strdup_printf ("/sys/class/net/%s", ifname); anycast_mask = g_strdup_printf ("%s/anycast_mask", sysfs_path); if (g_file_test (anycast_mask, G_FILE_TEST_EXISTS)) - return_type (NM_LINK_TYPE_OLPC_MESH, "olpc-mesh"); + return NM_LINK_TYPE_OLPC_MESH; devtype = read_devtype (sysfs_path); for (i = 0; devtype && i < G_N_ELEMENTS (linktypes); i++) { if (g_strcmp0 (devtype, linktypes[i].devtype) == 0) - return_type (linktypes[i].nm_type, linktypes[i].type_string); + return linktypes[i].nm_type; } /* Fallback for drivers that don't call SET_NETDEV_DEVTYPE() */ if (wifi_utils_is_wifi (ifname, sysfs_path)) - return_type (NM_LINK_TYPE_WIFI, "wifi"); + return NM_LINK_TYPE_WIFI; /* Standard wired ethernet interfaces don't report an rtnl_link_type, so * only allow fallback to Ethernet if no type is given. This should @@ -1035,10 +1026,10 @@ link_extract_type (NMPlatform *platform, struct rtnl_link *rtnllink, const char * when they should be Generic instead. */ if (arptype == ARPHRD_ETHER && !rtnl_type && !devtype) - return_type (NM_LINK_TYPE_ETHERNET, "ethernet"); + return NM_LINK_TYPE_ETHERNET; } - return_type (NM_LINK_TYPE_UNKNOWN, rtnl_type ? rtnl_type : "unknown"); + return NM_LINK_TYPE_UNKNOWN; } static gboolean @@ -1058,7 +1049,8 @@ init_link (NMPlatform *platform, NMPlatformLink *info, struct rtnl_link *rtnllin g_strlcpy (info->name, name, sizeof (info->name)); else info->name[0] = '\0'; - info->type = link_extract_type (platform, rtnllink, &info->type_name); + info->type = link_extract_type (platform, rtnllink); + info->kind = g_intern_string (rtnl_link_get_type (rtnllink)); info->up = !!(rtnl_link_get_flags (rtnllink) & IFF_UP); info->connected = !!(rtnl_link_get_flags (rtnllink) & IFF_LOWER_UP); info->arp = !(rtnl_link_get_flags (rtnllink) & IFF_NOARP); @@ -1074,7 +1066,7 @@ init_link (NMPlatform *platform, NMPlatformLink *info, struct rtnl_link *rtnllin } if (!info->driver) - info->driver = g_intern_string (rtnl_link_get_type (rtnllink)); + info->driver = info->kind; if (!info->driver) info->driver = ethtool_get_driver (info->name); if (!info->driver) @@ -2446,17 +2438,33 @@ link_get_type (NMPlatform *platform, int ifindex) { auto_nl_object struct rtnl_link *rtnllink = link_get (platform, ifindex); - return link_extract_type (platform, rtnllink, NULL); + return link_extract_type (platform, rtnllink); } static const char * link_get_type_name (NMPlatform *platform, int ifindex) { auto_nl_object struct rtnl_link *rtnllink = link_get (platform, ifindex); - const char *type; + NMLinkType link_type; + const char *l; - link_extract_type (platform, rtnllink, &type); - return type; + if (!rtnllink) + return NULL; + + link_type = link_extract_type (platform, rtnllink); + if (link_type != NM_LINK_TYPE_UNKNOWN) { + /* We could detect the @link_type. In this case the function returns + * our internel module names, which differs from rtnl_link_get_type(): + * - NM_LINK_TYPE_INFINIBAND (gives "infiniband", instead of "ipoib") + * - NM_LINK_TYPE_TAP (gives "tap", instead of "tun"). + * Note that this functions is only used by NMDeviceGeneric to + * set type_description. */ + return nm_link_type_to_string (link_type); + } + + /* Link type not detected. Fallback to rtnl_link_get_type()/IFLA_INFO_KIND. */ + l = rtnl_link_get_type (rtnllink); + return l ? g_intern_string (l) : "unknown"; } static gboolean diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index b5201e37d8..3d00ae4ae4 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -2530,7 +2530,7 @@ nm_platform_link_to_string (const NMPlatformLink *link) { char master[20]; char parent[20]; - char *driver, *udi, *type; + char *driver, *udi; GString *str; if (!link) @@ -2558,16 +2558,20 @@ nm_platform_link_to_string (const NMPlatformLink *link) driver = link->driver ? g_strdup_printf (" driver '%s'", link->driver) : NULL; udi = link->udi ? g_strdup_printf (" udi '%s'", link->udi) : NULL; - type = link->type_name ? NULL : g_strdup_printf ("(%d)", link->type); - g_snprintf (to_string_buffer, sizeof (to_string_buffer), "%d: %s%s <%s> mtu %d%s %s%s%s", + g_snprintf (to_string_buffer, sizeof (to_string_buffer), "%d: %s%s <%s> mtu %d%s " + "%s" /* link->type */ + "%s%s" /* kind */ + "%s%s", link->ifindex, link->name, parent, str->str, - link->mtu, master, link->type_name ? link->type_name : type, + link->mtu, master, + nm_link_type_to_string (link->type), + link->type != NM_LINK_TYPE_UNKNOWN && link->kind ? " kind " : "", + link->type != NM_LINK_TYPE_UNKNOWN && link->kind ? link->kind : "", driver ? driver : "", udi ? udi : ""); g_string_free (str, TRUE); g_free (driver); g_free (udi); - g_free (type); return to_string_buffer; } @@ -2811,6 +2815,16 @@ nm_platform_ip6_route_to_string (const NMPlatformIP6Route *route) return c < 0 ? -1 : 1; \ } G_STMT_END +#define _CMP_FIELD_STR_INTERNED(a, b, field) \ + G_STMT_START { \ + if (((a)->field) != ((b)->field)) { \ + /* just to be sure, also do a strcmp() if the pointers don't match */ \ + int c = g_strcmp0 ((a)->field, (b)->field); \ + if (c != 0) \ + return c < 0 ? -1 : 1; \ + } \ + } G_STMT_END + #define _CMP_FIELD_STR0(a, b, field) \ G_STMT_START { \ int c = g_strcmp0 ((a)->field, (b)->field); \ @@ -2839,8 +2853,8 @@ nm_platform_link_cmp (const NMPlatformLink *a, const NMPlatformLink *b) _CMP_FIELD (a, b, connected); _CMP_FIELD (a, b, arp); _CMP_FIELD (a, b, mtu); - _CMP_FIELD_STR0 (a, b, type_name); _CMP_FIELD_BOOL (a, b, initialized); + _CMP_FIELD_STR_INTERNED (a, b, kind); _CMP_FIELD_STR0 (a, b, udi); _CMP_FIELD_STR0 (a, b, driver); return 0; diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h index 871daac0d6..43b9731f5b 100644 --- a/src/platform/nm-platform.h +++ b/src/platform/nm-platform.h @@ -85,7 +85,10 @@ struct _NMPlatformLink { __NMPlatformObject_COMMON; char name[IFNAMSIZ]; NMLinkType type; - const char *type_name; + + /* rtnl_link_get_type(), IFLA_INFO_KIND */ + const char *kind; + const char *udi; const char *driver; gboolean initialized; diff --git a/src/platform/tests/dump.c b/src/platform/tests/dump.c index e356df0b45..d28d517936 100644 --- a/src/platform/tests/dump.c +++ b/src/platform/tests/dump.c @@ -28,7 +28,7 @@ dump_interface (NMPlatformLink *link) g_assert (link->up || !link->connected); - printf ("%d: %s: %s", link->ifindex, link->name, link->type_name); + printf ("%d: %s: %s", link->ifindex, link->name, nm_link_type_to_string (link->type)); if (link->up) printf (" %s", link->connected ? "CONNECTED" : "DISCONNECTED"); else From 7b6ffb1e5f00f644363cde94d20f679761663fd6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 29 Apr 2015 09:19:15 +0200 Subject: [PATCH 12/12] platform: intern driver string for NMPlatformLink Always intern string from udev_get_driver(). We use the result of udev_get_driver() for setting NMPlatformLink.driver. In all other cases, we already set that value to an interned string, which simplifies memory handling. As it was, the lifetime of that string was tied to the lifetime of the GUdevDevice. This is not a stelar solution, but we assume that the overall numbers of different drivers is limited so we don't leak large amounts of memory. (cherry picked from commit 3171b543dc30090fc50d0c74786e76062e079ed2) --- src/platform/nm-linux-platform.c | 17 ++++++----------- src/platform/nm-platform.c | 2 +- src/platform/nm-platform.h | 8 +++++++- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index df8456e885..126b01f867 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -455,7 +455,7 @@ udev_get_driver (GUdevDevice *device, int ifindex) driver = g_udev_device_get_driver (device); if (driver) - return driver; + goto out; /* Try the parent */ parent = g_udev_device_get_parent (device); @@ -470,23 +470,18 @@ udev_get_driver (GUdevDevice *device, int ifindex) if ( (g_strcmp0 (subsys, "ibmebus") == 0) || (subsys == NULL)) { grandparent = g_udev_device_get_parent (parent); - if (grandparent) { + if (grandparent) driver = g_udev_device_get_driver (grandparent); - } } } } - - /* Intern the string so we don't have to worry about memory - * management in NMPlatformLink. - */ - if (driver) - driver = g_intern_string (driver); - g_clear_object (&parent); g_clear_object (&grandparent); - return driver; +out: + /* Intern the string so we don't have to worry about memory + * management in NMPlatformLink. */ + return g_intern_string (driver); } /****************************************************************** diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index 3d00ae4ae4..9b1d64e7ff 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -2856,7 +2856,7 @@ nm_platform_link_cmp (const NMPlatformLink *a, const NMPlatformLink *b) _CMP_FIELD_BOOL (a, b, initialized); _CMP_FIELD_STR_INTERNED (a, b, kind); _CMP_FIELD_STR0 (a, b, udi); - _CMP_FIELD_STR0 (a, b, driver); + _CMP_FIELD_STR_INTERNED (a, b, driver); return 0; } diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h index 43b9731f5b..8d914c5e3b 100644 --- a/src/platform/nm-platform.h +++ b/src/platform/nm-platform.h @@ -86,11 +86,17 @@ struct _NMPlatformLink { char name[IFNAMSIZ]; NMLinkType type; - /* rtnl_link_get_type(), IFLA_INFO_KIND */ + /* rtnl_link_get_type(), IFLA_INFO_KIND. */ + /* NMPlatform initializes this field with a static string. */ const char *kind; + /* Beware: NMPlatform initializes this string with an allocated string. + * Handle it properly (i.e. don't keep a reference to it). */ const char *udi; + + /* NMPlatform initializes this field with a static string. */ const char *driver; + gboolean initialized; int master; int parent;