From e2c742c77b7c708d6a5e6f689ff7cc91b23ab445 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 28 Apr 2015 10:11:04 +0200 Subject: [PATCH] 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. --- 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 0bf66ab76..0bb3b0efd 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -2203,7 +2203,7 @@ platform_link_added (NMManager *self, case NM_LINK_TYPE_TEAM: case NM_LINK_TYPE_WIFI: 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)); nm_plugin_missing = TRUE; /* fall through */ default: diff --git a/src/platform/nm-fake-platform.c b/src/platform/nm-fake-platform.c index b49e02d50..530812198 100644 --- a/src/platform/nm-fake-platform.c +++ b/src/platform/nm-fake-platform.c @@ -113,7 +113,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 c51c5fe9b..2e55c5daf 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 37a1a6f04..d8322a729 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -2344,7 +2344,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) @@ -2372,16 +2372,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; } @@ -2625,6 +2629,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); \ @@ -2652,8 +2666,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 abc1dad56..d24f56e2f 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 e356df0b4..d28d51793 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