Re: [GIT] Networking

[BjÃrn Mork]

Ben Hutchings <bhutchings@xxxxxxxxxxxxxx> writes:
> On Thu, 2013-05-02 at 05:17 -0400, David Miller wrote:
>> From: BjÃrn Mork <bjorn@xxxxxxx>
>> Date: Thu, 02 May 2013 11:06:42 +0200
>> 
>> > Adding the new netdev features will make it go from 1 to 2:
>> 
>> We already had more than 31 feature bits before Patrick's
>> changes, and I'm pretty sure this was the case when we added
>> that ethtool API.
>
> It wasn't, but this should be OK.  Userland is supposed to query the
> number of features using ETHTOOL_GSSET_INFO and then work out the number
> of words/blocks using FEATURE_BITS_TO_BLOCKS().  


Looking at
http://cgit.freedesktop.org/NetworkManager/NetworkManager/tree/src/platform/nm-linux-platform.c#n1025
there seems to be a couple of bugs in this area. This is certainly
abusing the exported API, but it does mean that NM breaks if you ever
move NETIF_F_VLAN_CHALLENGED (like the 802.1ad patch did):

----
#define NETIF_F_VLAN_CHALLENGED (1 << 10)

static gboolean
link_supports_vlans (NMPlatform *platform, int ifindex)
{
	auto_nl_object struct rtnl_link *rtnllink = link_get (platform, ifindex);
	const char *name = nm_platform_link_get_name (ifindex);
	struct {
		struct ethtool_gfeatures features;
		struct ethtool_get_features_block features_block;
	} edata = { .features = { .cmd = ETHTOOL_GFEATURES, .size = 1 } };

	/* Only ARPHRD_ETHER links can possibly support VLANs. */
	if (!rtnllink || rtnl_link_get_arptype (rtnllink) != ARPHRD_ETHER)
		return FALSE;

	if (!name || !ethtool_get (name, &edata))
		return FALSE;

	return !(edata.features.features[0].active & NETIF_F_VLAN_CHALLENGED);
}
----


Not that I see how this particular bug matters unless you need VLAN
support in NM.  But there could be similar issues around.  I guess
avoiding unnecessary renumbering of the NETIF_F bits can save us some
trouble.  Although you can certainly argue that those bits never were
intended to be part of the API, and that using them like this is a user
application bug.



BjÃrn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/