2013-01-11 09:19:30

by Stanislaw Gruszka

[permalink] [raw]
Subject: [PATCH v3] net, wireless: overwrite default_ethtool_ops

Since:

commit 2c60db037034d27f8c636403355d52872da92f81
Author: Eric Dumazet <[email protected]>
Date: Sun Sep 16 09:17:26 2012 +0000

net: provide a default dev->ethtool_ops

wireless core does not correctly assign ethtool_ops.

After alloc_netdev*() call, some cfg80211 drivers provide they own
ethtool_ops, but some do not. For them, wireless core provide generic
cfg80211_ethtool_ops, which is assigned in NETDEV_REGISTER notify call:

if (!dev->ethtool_ops)
dev->ethtool_ops = &cfg80211_ethtool_ops;

But after Eric's commit, dev->ethtool_ops is no longer NULL (on cfg80211
drivers without custom ethtool_ops), but points to &default_ethtool_ops.

In order to fix the problem, provide function which will overwrite
default_ethtool_ops and use it by wireless core.

Signed-off-by: Stanislaw Gruszka <[email protected]>
---
v1 -> v2: change order of default_ethtool_ops initialization to avoid
the problem. Change the subject accordingly.

v2 -> v3: provide function to overwrite default_ethtool_ops, describe
problem a bit more detailed in the changelog

include/linux/netdevice.h | 3 +++
net/core/dev.c | 8 ++++++++
net/wireless/core.c | 3 +--
3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index c599e47..9ef07d0 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -60,6 +60,9 @@ struct wireless_dev;
#define SET_ETHTOOL_OPS(netdev,ops) \
( (netdev)->ethtool_ops = (ops) )

+extern void netdev_set_default_ethtool_ops(struct net_device *dev,
+ const struct ethtool_ops *ops);
+
/* hardware address assignment types */
#define NET_ADDR_PERM 0 /* address is permanent (default) */
#define NET_ADDR_RANDOM 1 /* address is generated randomly */
diff --git a/net/core/dev.c b/net/core/dev.c
index 515473e..f64e439 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6121,6 +6121,14 @@ struct netdev_queue *dev_ingress_queue_create(struct net_device *dev)

static const struct ethtool_ops default_ethtool_ops;

+void netdev_set_default_ethtool_ops(struct net_device *dev,
+ const struct ethtool_ops *ops)
+{
+ if (dev->ethtool_ops == &default_ethtool_ops)
+ dev->ethtool_ops = ops;
+}
+EXPORT_SYMBOL_GPL(netdev_set_default_ethtool_ops);
+
/**
* alloc_netdev_mqs - allocate network device
* @sizeof_priv: size of private data to allocate space for
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 747dd93..7cbd3bf 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -858,8 +858,7 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
/* allow mac80211 to determine the timeout */
wdev->ps_timeout = -1;

- if (!dev->ethtool_ops)
- dev->ethtool_ops = &cfg80211_ethtool_ops;
+ netdev_set_default_ethtool_ops(dev, &cfg80211_ethtool_ops);

if ((wdev->iftype == NL80211_IFTYPE_STATION ||
wdev->iftype == NL80211_IFTYPE_P2P_CLIENT ||
--
1.7.1



2013-01-21 21:47:18

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH v3] net, wireless: overwrite default_ethtool_ops

On Mon, Jan 21, 2013 at 04:04:04PM -0500, David Miller wrote:
>
> It's queued up for -stable already as is clearly seen at:
>
> http://patchwork.ozlabs.org/user/bundle/2566/?state=*

Thanks, I was not aware of this bundle. In this case assuming
this goes into v3.7.5 this is being backported with special handling
between 3.7.0 and 3.7.5 as follows onto compat.

From: "Luis R. Rodriguez" <[email protected]>
Subject: [PATCH] compat: backport netdev_set_default_ethtool_ops()

Stanislaw found that due to commit 2c60db03 by Eric Dumazet
the wireless core was not assigning driver specific ethtool_ops.
This was fixed by Stanislaw's commit d07d7507 which added
netdev_set_default_ethtool_ops(). Since Eric's commit 2c60db03
is on v3.7-rc1 Stanislaw's fix is required down to v3.7 as well.
The d07d7507 commit is currently present on v3.8-rc4 and is on
its way to what we think may be v3.7.5. Because of this kernels
older than v3.7.5 will require the full implementation while
older kernels than v3.7.0 will require just assigning the ops
passed only if netdev has no ops already set just as we used
to have it implemented on cfg80211.

[email protected] ~/linux-stable (git::linux-3.8.y)$ git describe --contains 2c60db
v3.7-rc1~145^2~142

[email protected] ~/linux-stable (git::linux-3.8.y)$ git describe --contains d07d75
v3.8-rc4~29^2~4

ckmake results:

1 2.6.24 [ OK ]
2 2.6.25 [ OK ]
3 2.6.26 [ OK ]
4 2.6.27 [ OK ]
5 2.6.28 [ OK ]
6 2.6.29 [ OK ]
7 2.6.30 [ OK ]
8 2.6.31 [ OK ]
9 2.6.32 [ OK ]
10 2.6.33 [ OK ]
11 2.6.34 [ OK ]
12 2.6.35 [ OK ]
13 2.6.36 [ OK ]
14 2.6.37 [ OK ]
15 2.6.38 [ OK ]
16 2.6.39 [ OK ]
17 3.0.50 [ OK ]
18 3.1.10 [ OK ]
19 3.2.33 [ OK ]
20 3.3.8 [ OK ]
21 3.4.17 [ OK ]
22 3.5.7 [ OK ]
23 3.6.5 [ OK ]
24 3.7.0 [ OK ]

real 0m34.791s
user 11m38.572s
sys 3m56.927s

Signed-off-by: Luis R. Rodriguez <[email protected]>
---
compat/compat-3.8.c | 22 ++++++++++++++++++++++
include/linux/compat-3.8.h | 6 ++++++
2 files changed, 28 insertions(+)

diff --git a/compat/compat-3.8.c b/compat/compat-3.8.c
index 034dd77..1867258 100644
--- a/compat/compat-3.8.c
+++ b/compat/compat-3.8.c
@@ -16,6 +16,28 @@
#include <linux/hid.h>
#include <linux/module.h>
#include "hid-ids.h"
+#include <linux/netdevice.h>
+
+#if (LINUX_VERSION_CODE < KERNEL_VERSION(3,7,5))
+
+#if (LINUX_VERSION_CODE < KERNEL_VERSION(3,7,0))
+void netdev_set_default_ethtool_ops(struct net_device *dev,
+ const struct ethtool_ops *ops)
+{
+ if (!dev->ethtool_ops)
+ dev->ethtool_ops = ops;
+}
+#else /* kernel is between 3.7.0 and 3.7.4 */;
+void netdev_set_default_ethtool_ops(struct net_device *dev,
+ const struct ethtool_ops *ops)
+{
+ if (dev->ethtool_ops == &default_ethtool_ops)
+ dev->ethtool_ops = ops;
+}
+#endif
+
+EXPORT_SYMBOL_GPL(netdev_set_default_ethtool_ops);
+#endif /* (LINUX_VERSION_CODE < KERNEL_VERSION(3,7,5) */

/* a list of devices that shouldn't be handled by HID core at all */
static const struct hid_device_id hid_ignore_list[] = {
diff --git a/include/linux/compat-3.8.h b/include/linux/compat-3.8.h
index 052de95..942b4cb 100644
--- a/include/linux/compat-3.8.h
+++ b/include/linux/compat-3.8.h
@@ -6,6 +6,12 @@
#if (LINUX_VERSION_CODE < KERNEL_VERSION(3,8,0))

#include <linux/hid.h>
+#include <linux/netdevice.h>
+
+#if (LINUX_VERSION_CODE < KERNEL_VERSION(3,7,5))
+extern void netdev_set_default_ethtool_ops(struct net_device *dev,
+ const struct ethtool_ops *ops);
+#endif

#define HID_BUS_ANY 0xffff
#define HID_GROUP_ANY 0x0000
--
1.7.10.4


2013-01-22 10:56:39

by Stanislaw Gruszka

[permalink] [raw]
Subject: Re: [PATCH v3] net, wireless: overwrite default_ethtool_ops

On Mon, Jan 21, 2013 at 04:04:04PM -0500, David Miller wrote:
>
> It's queued up for -stable already as is clearly seen at:
>
> http://patchwork.ozlabs.org/user/bundle/2566/?state=*

Hmm, this link does not work for me (need user/password).

Anyway, I marked cc -stable in v2 patch, but forgot that in v3.
Committed patch does not include the mark, so I'll post patch
to stable just in case.

Thanks
Stanislaw

2013-01-21 21:04:08

by David Miller

[permalink] [raw]

2013-01-11 23:59:37

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v3] net, wireless: overwrite default_ethtool_ops

From: Ben Hutchings <[email protected]>
Date: Fri, 11 Jan 2013 20:00:32 +0000

> On Fri, 2013-01-11 at 10:19 +0100, Stanislaw Gruszka wrote:
>> Since:
>>
>> commit 2c60db037034d27f8c636403355d52872da92f81
>> Author: Eric Dumazet <[email protected]>
>> Date: Sun Sep 16 09:17:26 2012 +0000
>>
>> net: provide a default dev->ethtool_ops
>>
>> wireless core does not correctly assign ethtool_ops.
>>
>> After alloc_netdev*() call, some cfg80211 drivers provide they own
>> ethtool_ops, but some do not. For them, wireless core provide generic
>> cfg80211_ethtool_ops, which is assigned in NETDEV_REGISTER notify call:
>>
>> if (!dev->ethtool_ops)
>> dev->ethtool_ops = &cfg80211_ethtool_ops;
>>
>> But after Eric's commit, dev->ethtool_ops is no longer NULL (on cfg80211
>> drivers without custom ethtool_ops), but points to &default_ethtool_ops.
>>
>> In order to fix the problem, provide function which will overwrite
>> default_ethtool_ops and use it by wireless core.
>>
>> Signed-off-by: Stanislaw Gruszka <[email protected]>
> [...]
>
> Acked-by: Ben Hutchings <[email protected]>

Applied.

2013-01-21 20:53:05

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH v3] net, wireless: overwrite default_ethtool_ops

On Fri, Jan 11, 2013 at 3:59 PM, David Miller <[email protected]> wrote:
> From: Ben Hutchings <[email protected]>
> Date: Fri, 11 Jan 2013 20:00:32 +0000
>
>> On Fri, 2013-01-11 at 10:19 +0100, Stanislaw Gruszka wrote:
>>> Since:
>>>
>>> commit 2c60db037034d27f8c636403355d52872da92f81
>>> Author: Eric Dumazet <[email protected]>
>>> Date: Sun Sep 16 09:17:26 2012 +0000
>>>
>>> net: provide a default dev->ethtool_ops
>>>
>>> wireless core does not correctly assign ethtool_ops.
>>>
>>> After alloc_netdev*() call, some cfg80211 drivers provide they own
>>> ethtool_ops, but some do not. For them, wireless core provide generic
>>> cfg80211_ethtool_ops, which is assigned in NETDEV_REGISTER notify call:
>>>
>>> if (!dev->ethtool_ops)
>>> dev->ethtool_ops = &cfg80211_ethtool_ops;
>>>
>>> But after Eric's commit, dev->ethtool_ops is no longer NULL (on cfg80211
>>> drivers without custom ethtool_ops), but points to &default_ethtool_ops.
>>>
>>> In order to fix the problem, provide function which will overwrite
>>> default_ethtool_ops and use it by wireless core.
>>>
>>> Signed-off-by: Stanislaw Gruszka <[email protected]>
>> [...]
>>
>> Acked-by: Ben Hutchings <[email protected]>
>
> Applied.

Stanislaw, I see Eric's patch went in on v3.7-rc1 as such I suspect
this needs to be submitted as a stable fix for v3.7.5. Its already on
v3.8-rc4.

[email protected] ~/linux-stable (git::linux-3.7.y)$ git describe
--contains 2c60db037034d27f8c636403355d52872da92f81
v3.7-rc1~145^2~142

Luis

2013-01-22 19:06:26

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v3] net, wireless: overwrite default_ethtool_ops

From: Stanislaw Gruszka <[email protected]>
Date: Tue, 22 Jan 2013 11:56:19 +0100

> On Mon, Jan 21, 2013 at 04:04:04PM -0500, David Miller wrote:
>>
>> It's queued up for -stable already as is clearly seen at:
>>
>> http://patchwork.ozlabs.org/user/bundle/2566/?state=*
>
> Hmm, this link does not work for me (need user/password).

You just need to make a patchwork login, it's visible to any
registered user.

> Anyway, I marked cc -stable in v2 patch, but forgot that in v3.
> Committed patch does not include the mark, so I'll post patch
> to stable just in case.

Do NOT DO THIS, I do the submissions myself, by hand.

2013-01-11 10:51:28

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v3] net, wireless: overwrite default_ethtool_ops

On Fri, 2013-01-11 at 10:19 +0100, Stanislaw Gruszka wrote:
> Since:
>
> commit 2c60db037034d27f8c636403355d52872da92f81
> Author: Eric Dumazet <[email protected]>
> Date: Sun Sep 16 09:17:26 2012 +0000
>
> net: provide a default dev->ethtool_ops
>
> wireless core does not correctly assign ethtool_ops.
>
> After alloc_netdev*() call, some cfg80211 drivers provide they own
> ethtool_ops, but some do not. For them, wireless core provide generic
> cfg80211_ethtool_ops, which is assigned in NETDEV_REGISTER notify call:
>
> if (!dev->ethtool_ops)
> dev->ethtool_ops = &cfg80211_ethtool_ops;
>
> But after Eric's commit, dev->ethtool_ops is no longer NULL (on cfg80211
> drivers without custom ethtool_ops), but points to &default_ethtool_ops.
>
> In order to fix the problem, provide function which will overwrite
> default_ethtool_ops and use it by wireless core.
>
> Signed-off-by: Stanislaw Gruszka <[email protected]>

Acked-by: Johannes Berg <[email protected]>

This will work nicely, clearly I didn't understand davem's suggestion :)

johannes


2013-01-11 20:00:36

by Ben Hutchings

[permalink] [raw]
Subject: Re: [PATCH v3] net, wireless: overwrite default_ethtool_ops

On Fri, 2013-01-11 at 10:19 +0100, Stanislaw Gruszka wrote:
> Since:
>
> commit 2c60db037034d27f8c636403355d52872da92f81
> Author: Eric Dumazet <[email protected]>
> Date: Sun Sep 16 09:17:26 2012 +0000
>
> net: provide a default dev->ethtool_ops
>
> wireless core does not correctly assign ethtool_ops.
>
> After alloc_netdev*() call, some cfg80211 drivers provide they own
> ethtool_ops, but some do not. For them, wireless core provide generic
> cfg80211_ethtool_ops, which is assigned in NETDEV_REGISTER notify call:
>
> if (!dev->ethtool_ops)
> dev->ethtool_ops = &cfg80211_ethtool_ops;
>
> But after Eric's commit, dev->ethtool_ops is no longer NULL (on cfg80211
> drivers without custom ethtool_ops), but points to &default_ethtool_ops.
>
> In order to fix the problem, provide function which will overwrite
> default_ethtool_ops and use it by wireless core.
>
> Signed-off-by: Stanislaw Gruszka <[email protected]>
[...]

Acked-by: Ben Hutchings <[email protected]>

--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.