platform: rework link type detection for better fallback (bgo #743209)dcbw/bgo747628-platform-udev-excise-th-squashed

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

3 files changed, 103 insertions, 136 deletions

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index 95f92217a5..db27fe29ab 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -775,96 +775,115 @@ 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 specificed if it is a direct mapping.  eg, tun/tap
+	 * should not be specified since both tun and tap devices use "tun".
+	 */
+	const char *rtnl_type;
+} LinkDesc;
+
+static const LinkDesc linktypes[] = {
+	{ NM_LINK_TYPE_ETHERNET,      "ethernet",    NULL },
+	{ NM_LINK_TYPE_INFINIBAND,    "infiniband",  NULL },
+	{ NM_LINK_TYPE_OLPC_MESH,     "olpc-mesh",   NULL },
+	{ NM_LINK_TYPE_WIFI,          "wifi",        "wlan" },
+	{ NM_LINK_TYPE_WWAN_ETHERNET, "wwan",        "wwan" },
+	{ NM_LINK_TYPE_WIMAX,         "wimax",       "wimax" },
+	{ NM_LINK_TYPE_LOOPBACK,      "loopback",    NULL },
+	{ NM_LINK_TYPE_OPENVSWITCH,   "openvswitch", "openvswitch" },
+	{ NM_LINK_TYPE_TAP,           "tap",         NULL },
+	{ NM_LINK_TYPE_TUN,           "tun",         NULL },
+	{ NM_LINK_TYPE_DUMMY,         "dummy",       "dummy" },
+	{ NM_LINK_TYPE_GRE,           "gre",         "gre" },
+	{ NM_LINK_TYPE_GRETAP,        "gretap",      "gretap" },
+	{ NM_LINK_TYPE_IFB,           "ifb",         "ifb" },
+	{ NM_LINK_TYPE_MACVLAN,       "macvlan",     "macvlan" },
+	{ NM_LINK_TYPE_MACVTAP,       "macvtap",     "macvtap" },
+	{ NM_LINK_TYPE_VETH,          "veth",        "veth" },
+	{ NM_LINK_TYPE_VLAN,          "vlan",        "vlan" },
+	{ NM_LINK_TYPE_VXLAN,         "vxlan",       "vxlan" },
+	{ NM_LINK_TYPE_BRIDGE,        "bridge",      "bridge" },
+	{ NM_LINK_TYPE_BOND,          "bond",        "bond" },
+	{ NM_LINK_TYPE_TEAM,          "team",        "team" }
+};
+
 static const char *
-type_to_string (NMLinkType type)
+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_warning ("Wrong/unhandled type: %d", type);
+	return NULL;
 }
 
-#define DEVTYPE_PREFIX "DEVTYPE="
-
-static char *
-read_devtype (const char *sysfs_path)
+static const char *
+type_to_string (NMLinkType type)
 {
-	gs_free char *uevent = g_strdup_printf ("%s/uevent", sysfs_path);
-	char *contents = NULL;
-	char *cont, *end;
+	int i;
 
-	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;
-		}
+	for (i = 0; i < G_N_ELEMENTS (linktypes); i++) {
+		if (type == linktypes[i].nm_type)
+			return linktypes[i].type_string;
 	}
-	g_free (contents);
+	g_warning ("Wrong/unhandled type: %d", type);
 	return NULL;
 }
 
 static NMLinkType
 link_extract_type (struct rtnl_link *rtnllink, const char **out_name)
 {
-	const char *type;
+	const char *type, *ifname;
+	int i, arptype;
 
 	if (!rtnllink)
 		return_type (NM_LINK_TYPE_NONE, NULL);
 
 	type = rtnl_link_get_type (rtnllink);
+	if (type) {
+		for (i = 0; i < G_N_ELEMENTS (linktypes); i++) {
+			if (g_strcmp0 (type, linktypes[i].rtnl_type) == 0)
+				return_type (linktypes[i].nm_type, type);
+		}
 
-	if (!type) {
-		int arptype = rtnl_link_get_arptype (rtnllink);
-		const char *driver;
-		const char *ifname;
-		gs_free char *anycast_mask = NULL;
-		gs_free char *sysfs_path = NULL;
-		gs_free char *devtype = NULL;
+		if (!strcmp (type, "tun")) {
+			NMPlatformTunProperties props;
+			guint flags;
 
-		if (arptype == ARPHRD_LOOPBACK)
-			return_type (NM_LINK_TYPE_LOOPBACK, "loopback");
-		else if (arptype == ARPHRD_INFINIBAND)
-			return_type (NM_LINK_TYPE_INFINIBAND, "infiniband");
+			if (nm_platform_tun_get_properties (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);
 
-		ifname = rtnl_link_get_name (rtnllink);
-		if (!ifname)
-			return_type (NM_LINK_TYPE_UNKNOWN, type);
+			/* 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) {
+		gs_free char *sysfs_path = NULL;
+		gs_free char *anycast_mask = NULL;
 
 		if (arptype == 256) {
 			/* Some s390 CTC-type devices report 256 for the encapsulation type
@@ -875,8 +894,8 @@ link_extract_type (struct rtnl_link *rtnllink, const char **out_name)
 				return_type (NM_LINK_TYPE_ETHERNET, "ethernet");
 		}
 
-		driver = ethtool_get_driver (ifname);
-		if (!g_strcmp0 (driver, "openvswitch"))
+		/* Fallback OVS detection for kernel <= 3.16 */
+		if (!g_strcmp0 (ethtool_get_driver (ifname), "openvswitch"))
 			return_type (NM_LINK_TYPE_OPENVSWITCH, "openvswitch");
 
 		sysfs_path = g_strdup_printf ("/sys/class/net/%s", ifname);
@@ -884,65 +903,20 @@ link_extract_type (struct rtnl_link *rtnllink, const char **out_name)
 		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");
-		}
+		/* 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");
 
-		if (arptype == ARPHRD_ETHER)
+		/* 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 && !type)
 			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 (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, type ? type : "unknown");
 }
 
 static gboolean
@@ -2251,7 +2225,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, type_to_rtnl_type_string (type));
 		g_assert (!nle);
 	}
 	return (struct nl_object *) rtnllink;
diff --git a/src/platform/wifi/wifi-utils.c b/src/platform/wifi/wifi-utils.c
index a67a7b05c9..d6dbc36456 100644
--- a/src/platform/wifi/wifi-utils.c
+++ b/src/platform/wifi/wifi-utils.c
@@ -170,20 +170,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 b25e70824a..ad4d141961 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);