diff mbox

[GIT] Networking

Message ID 87d2t9bvj1.fsf@nemi.mork.no
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Bjørn Mork May 2, 2013, 9:06 a.m. UTC 
Patrick McHardy <kaber@trash.net> writes:
> On Thu, May 02, 2013 at 04:16:25AM -0400, David Miller wrote:
>> From: Patrick McHardy <kaber@trash.net>
>> Date: Thu, 2 May 2013 09:03:37 +0200
>> 
>> > I'll also have a look at this.
>> 
>> By the mere existence of /sys/devices/${DEV_PATH}/net/${netdev_name}/flags
>> we have to preserve the bit layout.
>> 
>> So Linus was right.
>> 
>> So network manager is probably reading that flags sysfs file and
>> interpreting it.
>
> Right, that seems plausible. 
>
>> I'll fix the layout to how it was before.
>
> I also found one spot in net/core/dev.c which was using an int for the
> features. Patch attached.

And a couple more attached.

I am also wondering about the consequences of the
ETHTOOL_DEV_FEATURE_WORDS calculation in ethtool.c.  Adding the new
netdev features will make it go from 1 to 2:

#define ETHTOOL_DEV_FEATURE_WORDS	((NETDEV_FEATURE_COUNT + 31) / 32)


But this constant seems to be part of the userspace API AFAICS, so it
cannot just change like that:

static int ethtool_set_features(struct net_device *dev, void __user *useraddr)
{
	struct ethtool_sfeatures cmd;
	struct ethtool_set_features_block features[ETHTOOL_DEV_FEATURE_WORDS];
	netdev_features_t wanted = 0, valid = 0;
	int i, ret = 0;

	if (copy_from_user(&cmd, useraddr, sizeof(cmd)))
		return -EFAULT;
	useraddr += sizeof(cmd);

	if (cmd.size != ETHTOOL_DEV_FEATURE_WORDS)
		return -EINVAL;

..


Is this correctly analyzed?  If so, then I have no clue how to fix
that...



Bjørn

Comments

David Miller May 2, 2013, 9:17 a.m. UTC | #1 
From: Bjørn Mork <bjorn@mork.no>
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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjørn Mork May 2, 2013, 10:19 a.m. UTC | #2 
David Miller <davem@davemloft.net> writes:
> From: Bjørn Mork <bjorn@mork.no>
> 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.

Oh, thanks.  Then I misunderstood the issue.  This is one case where I'm
happy to be wrong :)


Bjørn
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Hutchings May 2, 2013, 10:28 a.m. UTC | #3 
On Thu, 2013-05-02 at 05:17 -0400, David Miller wrote:
> From: Bjørn Mork <bjorn@mork.no>
> 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().  

Ben.
Bjørn Mork May 2, 2013, 11:51 a.m. UTC | #4 
Ben Hutchings <bhutchings@solarflare.com> writes:
> On Thu, 2013-05-02 at 05:17 -0400, David Miller wrote:
>> From: Bjørn Mork <bjorn@mork.no>
>> 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 netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
=?ISO-8859-2?Q?Micha=B3_Miros=B3aw?= May 2, 2013, 4:22 p.m. UTC | #5 
2013/5/2 Bjørn Mork <bjorn@mork.no>:
> Ben Hutchings <bhutchings@solarflare.com> writes:
>> On Thu, 2013-05-02 at 05:17 -0400, David Miller wrote:
>>> From: Bjørn Mork <bjorn@mork.no>
>>> 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.

This is certainly a bug in NM, and a fresh one: commit
b636ea86b1c0a28b77eda311c84d3b2417cad22e from 2013-04-10 14:40:58
(GMT). Userspace is expected to use ETHTOOL_GSTRINGS for
ETH_SS_FEATURES and find a corresponding bit position by feature name
("vlan-challenged" in this case).

Cc: commit's author.

Best Regards,
Michał Mirosław
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dan Williams May 2, 2013, 4:27 p.m. UTC | #6 
On Thu, 2013-05-02 at 13:51 +0200, Bjørn Mork wrote:
> Ben Hutchings <bhutchings@solarflare.com> writes:
> > On Thu, 2013-05-02 at 05:17 -0400, David Miller wrote:
> >> From: Bjørn Mork <bjorn@mork.no>
> >> 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):

NM doesn't actually use any of the code in src/platform/ yet, so it
wouldn't be affecting anything NM does at this time.  However, comments
like this are quite useful so we can fix it before NM does start to
depend on the code :)

Dan

> ----
> #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 netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller May 2, 2013, 6:01 p.m. UTC | #7 
From: Bjørn Mork <bjorn@mork.no>
Date: Thu, 02 May 2013 11:06:42 +0200

> From d957cf339bf625869c39d852ac6733ef597ecef9 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Bj=C3=B8rn=20Mork?= <bjorn@mork.no>
> Date: Thu, 2 May 2013 10:37:05 +0200
> Subject: [PATCH] net: vlan,ethtool: netdev_features_t is more than 32 bit
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> Signed-off-by: Bjørn Mork <bjorn@mork.no>

Also applied and queued up for -stable.

These changes show me that this special type isn't providing type
safety in the way that we actually need it.

Something like how we do the MM page table types would work better:

typedef struct { u64 val; } netdev_features_t;

#define __netdev_feature(X)	((netdev_features_t) { X } )

and also with the appropriate set of accessors.

Then you can't get it wrong without a compile error.

But this is net-next material of course.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
John Stoffel May 2, 2013, 6:53 p.m. UTC | #8 
>>>>> "David" == David Miller <davem@davemloft.net> writes:

David> From: Bjørn Mork <bjorn@mork.no>
David> Date: Thu, 02 May 2013 11:06:42 +0200

>> From d957cf339bf625869c39d852ac6733ef597ecef9 Mon Sep 17 00:00:00 2001
>> From: Bjørn Mork <bjorn@mork.no>
>> Date: Thu, 2 May 2013 10:37:05 +0200
>> Subject: [PATCH] net: vlan,ethtool: netdev_features_t is more than 32 bit
>> MIME-Version: 1.0
>> Content-Type: text/plain; charset=UTF-8
>> Content-Transfer-Encoding: 8bit
>> 
>> Signed-off-by: Bjørn Mork <bjorn@mork.no>

David> Also applied and queued up for -stable.

David> These changes show me that this special type isn't providing type
David> safety in the way that we actually need it.

David> Something like how we do the MM page table types would work better:

David> typedef struct { u64 val; } netdev_features_t;

David> #define __netdev_feature(X)	((netdev_features_t) { X } )

David> and also with the appropriate set of accessors.

David> Then you can't get it wrong without a compile error.

Isn't part of the problem that you're exporting it into /sys in a
binary format?  Why not just have each flag as it's own file and
value?  Sure, it's a waste in some ways, but then it makes it simpler
to just do an 'opendir()' to see if the flag exists, much less what
it's set to.

John
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Hutchings May 2, 2013, 8:18 p.m. UTC | #9 
On Thu, 2013-05-02 at 14:53 -0400, John Stoffel wrote:
> >>>>> "David" == David Miller <davem@davemloft.net> writes:
> 
> David> From: Bjørn Mork <bjorn@mork.no>
> David> Date: Thu, 02 May 2013 11:06:42 +0200
> 
> >> From d957cf339bf625869c39d852ac6733ef597ecef9 Mon Sep 17 00:00:00 2001
> >> From: Bjørn Mork <bjorn@mork.no>
> >> Date: Thu, 2 May 2013 10:37:05 +0200
> >> Subject: [PATCH] net: vlan,ethtool: netdev_features_t is more than 32 bit
> >> MIME-Version: 1.0
> >> Content-Type: text/plain; charset=UTF-8
> >> Content-Transfer-Encoding: 8bit
> >> 
> >> Signed-off-by: Bjørn Mork <bjorn@mork.no>
> 
> David> Also applied and queued up for -stable.
> 
> David> These changes show me that this special type isn't providing type
> David> safety in the way that we actually need it.
> 
> David> Something like how we do the MM page table types would work better:
> 
> David> typedef struct { u64 val; } netdev_features_t;
> 
> David> #define __netdev_feature(X)	((netdev_features_t) { X } )
> 
> David> and also with the appropriate set of accessors.
> 
> David> Then you can't get it wrong without a compile error.
> 
> Isn't part of the problem that you're exporting it into /sys in a
> binary format?
[...]

Features are exported through SIOCETHTOOL, not sysfs (though they *used*
to be there).

The 'flags' attribue in sysfs is something different.

Ben.
John Stoffel May 2, 2013, 8:40 p.m. UTC | #10 
>>>>> "Ben" == Ben Hutchings <bhutchings@solarflare.com> writes:

Ben> On Thu, 2013-05-02 at 14:53 -0400, John Stoffel wrote:
>> >>>>> "David" == David Miller <davem@davemloft.net> writes:
>> 
David> From: Bjørn Mork <bjorn@mork.no>
David> Date: Thu, 02 May 2013 11:06:42 +0200
>> 
>> >> From d957cf339bf625869c39d852ac6733ef597ecef9 Mon Sep 17 00:00:00 2001
>> >> From: Bjørn Mork <bjorn@mork.no>
>> >> Date: Thu, 2 May 2013 10:37:05 +0200
>> >> Subject: [PATCH] net: vlan,ethtool: netdev_features_t is more than 32 bit
>> >> MIME-Version: 1.0
>> >> Content-Type: text/plain; charset=UTF-8
>> >> Content-Transfer-Encoding: 8bit
>> >> 
>> >> Signed-off-by: Bjørn Mork <bjorn@mork.no>
>> 
David> Also applied and queued up for -stable.
>> 
David> These changes show me that this special type isn't providing type
David> safety in the way that we actually need it.
>> 
David> Something like how we do the MM page table types would work better:
>> 
David> typedef struct { u64 val; } netdev_features_t;
>> 
David> #define __netdev_feature(X)	((netdev_features_t) { X } )
>> 
David> and also with the appropriate set of accessors.
>> 
David> Then you can't get it wrong without a compile error.
>> 
>> Isn't part of the problem that you're exporting it into /sys in a
>> binary format?
Ben> [...]

Ben> Features are exported through SIOCETHTOOL, not sysfs (though they *used*
Ben> to be there).

Ben> The 'flags' attribue in sysfs is something different.

THanks for the clarification.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pavel Simerda May 3, 2013, 11:35 p.m. UTC | #11 
----- Original Message -----
> From: "Michał Mirosław" <mirqus@gmail.com>
> To: "Bjørn Mork" <bjorn@mork.no>, "Pavel Šimerda" <psimerda@redhat.com>
> Cc: "Ben Hutchings" <bhutchings@solarflare.com>, "David Miller" <davem@davemloft.net>, kaber@trash.net,
> torvalds@linux-foundation.org, hayeswang@realtek.com, akpm@linux-foundation.org, netdev@vger.kernel.org,
> linux-kernel@vger.kernel.org
> Sent: Thursday, May 2, 2013 6:22:47 PM
> Subject: Re: [GIT] Networking
> 
> 2013/5/2 Bjørn Mork <bjorn@mork.no>:
> > Ben Hutchings <bhutchings@solarflare.com> writes:
> >> On Thu, 2013-05-02 at 05:17 -0400, David Miller wrote:
> >>> From: Bjørn Mork <bjorn@mork.no>
> >>> 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.
> 
> This is certainly a bug in NM, and a fresh one: commit
> b636ea86b1c0a28b77eda311c84d3b2417cad22e from 2013-04-10 14:40:58
> (GMT). Userspace is expected to use ETHTOOL_GSTRINGS for
> ETH_SS_FEATURES and find a corresponding bit position by feature name
> ("vlan-challenged" in this case).
> 
> Cc: commit's author.

Recorded the bug with NetworkManager bugzilla blocking the upcoming release:

https://bugzilla.gnome.org/show_bug.cgi?id=699649

The code is experimental and is not used by NetworkManager even in the 'master' branch except by nm-platform's automated tests. Once we fix it, you don't need to worry about it.

Thanks!

Pavel
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

From d957cf339bf625869c39d852ac6733ef597ecef9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B8rn=20Mork?= <bjorn@mork.no>
Date: Thu, 2 May 2013 10:37:05 +0200
Subject: [PATCH] net: vlan,ethtool: netdev_features_t is more than 32 bit
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 net/8021q/vlan_dev.c |    2 +-
 net/core/ethtool.c   |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 8af5085..3a8c8fd 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -628,7 +628,7 @@  static netdev_features_t vlan_dev_fix_features(struct net_device *dev,
 	netdev_features_t features)
 {
 	struct net_device *real_dev = vlan_dev_priv(dev)->real_dev;
-	u32 old_features = features;
+	netdev_features_t old_features = features;
 
 	features &= real_dev->vlan_features;
 	features |= NETIF_F_RXCSUM;
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 5a934ef..22efdaa 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -1421,7 +1421,7 @@  int dev_ethtool(struct net *net, struct ifreq *ifr)
 	void __user *useraddr = ifr->ifr_data;
 	u32 ethcmd;
 	int rc;
-	u32 old_features;
+	netdev_features_t old_features;
 
 	if (!dev || !netif_device_present(dev))
 		return -ENODEV;
-- 
1.7.10.4