* Re: [PATCH 2/2] netfilter: xt_condition: change the value from boolean to u32
From: Jan Engelhardt @ 2010-08-05 15:12 UTC (permalink / raw)
To: luciano.coelho; +Cc: netfilter-devel, netdev, kaber, sameo
In-Reply-To: <1281019269-30985-3-git-send-email-luciano.coelho@nokia.com>
On Thursday 2010-08-05 16:41, luciano.coelho@nokia.com wrote:
> struct xt_condition_mtinfo {
>- char name[31];
>+ char name[27];
> __u8 invert;
>+ __u32 value;
Please also bump the .revision field to 2 with this patch so that
testing can always proceed without an ABI clash.
(rev 2 would then remain over the course of the remaining patches
you submit.)
(rev 0 = ipt_condition/pom-ng; rev 1 = xt_condition from Xt-a)
>+ char buf[14];
char buf[sizeof("4294967296")];
seems more intuitive :-)
>+ unsigned long long value;
>+
>+ if (length == 0)
>+ return 0;
>+
>+ if (length > sizeof(buf))
>+ return -EINVAL;
>+
>+ if (copy_from_user(buf, input, length) != 0)
>+ return -EFAULT;
>+
>+ buf[length - 1] = '\0';
>+
>+ if (strict_strtoull(buf, 0, &value) != 0)
>+ return -EINVAL;
>+
>+ if (value > (u32) value)
>+ return -EINVAL;
Is it possible to use just strict_strtoul?
>- return var->enabled ^ info->invert;
>+ return (var->value == info->value) ^ info->invert;
Since the condition value (cdmark) was thought of an nfmark-style thing,
would it perhaps make sense to model it after it
return (var->value & ~info->mask) ^ info->value;
Other opinions?
^ permalink raw reply
* CONFIG_FIXED_PHY breaks fec on arm/mx51
From: Uwe Kleine-König @ 2010-08-05 15:17 UTC (permalink / raw)
To: netdev; +Cc: Bryan Wu, Andy Whitcroft, Amit Kucheria, Stefan Bader
Hello,
when I enable CONFIG_FIXED_PHY my mx51evm board fails to initialize the
fec network device. The relevant kernel log is:
...
Fixed MDIO Bus: probed
FEC Ethernet Driver
------------[ cut here ]------------
WARNING: at /ptx/work/octopus/WORK_2_A/ukl/backup/gsrc/linux-2.6/fs/sysfs/dir.c:451 sysfs_add_one+0x6c/0x8c()
sysfs: cannot create duplicate filename '/class/mdio_bus/0'
Modules linked in:
Backtrace:
[<c0026484>] (dump_backtrace+0x0/0x10c) from [<c02b6158>] (dump_stack+0x18/0x1c)
r7:df839d60 r6:c00db380 r5:c03434df r4:000001c3
[<c02b6140>] (dump_stack+0x0/0x1c) from [<c003750c>] (warn_slowpath_common+0x58/0x88)
[<c00374b4>] (warn_slowpath_common+0x0/0x88) from [<c00375e0>] (warn_slowpath_fmt+0x38/0x40)
r8:df84a480 r7:ffffffef r6:df839db0 r5:df811000 r4:df8dc600
[<c00375a8>] (warn_slowpath_fmt+0x0/0x40) from [<c00db380>] (sysfs_add_one+0x6c/0x8c)
r3:df811000 r2:c0343523
[<c00db314>] (sysfs_add_one+0x0/0x8c) from [<c00dc398>] (sysfs_do_create_link+0xfc/0x1c8)
r7:df8e8648 r6:00000000 r5:df8dc600 r4:df8dc570
[<c00dc29c>] (sysfs_do_create_link+0x0/0x1c8) from [<c00dc490>] (sysfs_create_link+0x14/0x18)
[<c00dc47c>] (sysfs_create_link+0x0/0x18) from [<c01e1918>] (device_add+0x198/0x520)
[<c01e1780>] (device_add+0x0/0x520) from [<c01e1cc0>] (device_register+0x20/0x24)
[<c01e1ca0>] (device_register+0x0/0x24) from [<c01ffbfc>] (mdiobus_register+0x90/0x178)
r4:df8e8600
[<c01ffb6c>] (mdiobus_register+0x0/0x178) from [<c02b5f50>] (fec_probe+0x39c/0x48c)
[<c02b5bb4>] (fec_probe+0x0/0x48c) from [<c01e4fb4>] (platform_drv_probe+0x20/0x24)
[<c01e4f94>] (platform_drv_probe+0x0/0x24) from [<c01e3ec8>] (driver_probe_device+0xb0/0x188)
[<c01e3e18>] (driver_probe_device+0x0/0x188) from [<c01e400c>] (__driver_attach+0x6c/0x90)
r7:df839f00 r6:c03abe0x58/0x98)
r6:c03abe08 r5:c01e3fa0 r4:00000000
[<c01e3668>] (bus_for_each_dev+0x0/0x98) from [<c01e3d24>] (driver_attach+0x20/0x28)
r7:df8db1e0 r6:c03abe08 r5:c03abe08 r4:c001e498
[<c01e3d04>] (driver_attach+0x0/0x28) from [<c01e2f2c>] (bus_add_driver+0xa4/0x228)
[<c01e2e88>] (bus_add_driver+0x0/0x228) from [<c01e4338>] (driver_register+0xc8/0x154)
[<c01e4270>] (driver_register+0x0/0x154) from [<c01e5410>] (platform_driver_register+0x4c/0x60)
r8:00000000 r7:00000000 r6:c00187a8 r5:c001e4f8 r4:c001e498
[<c01e53c4>] (platform_driver_register+0x0/0x60) from [<c00187c4>] (fec_enet_module_init+0x1c/0x28)
[<c00187a8>] (fec_enet_module_init+0x0/0x28) from [<c0022384>] (do_one_initcall+0x64/0x1c4)
[<c0022320>] (do_one_initcall+0x0/0x1c4) from [<c00085f4>] (kernel_init+0xa0/0x154)
r7:00000013 r6:c003a7dc r5:c001e4f8 r4:c001e498
[<c0008554>] (kernel_init+0x0/0x154) from [<c003a7dc>] (do_exit+0x0/0x564)
r5:c0008554 r4:00000000
---[ end trace 1220371fd8e97d24 ]---
mii_bus 0 failed to register
...
As far as I understand the problem this is just that both drivers
(drivers/net/phy/fixed.c and drivers/net/fec.c) try to register a
mdio_bus with the same number. There isn't a mechanism to circumvent
this problem, e.g. some auto-increment logic that just isn't used, is
there?
The people from Ubuntu had the same problem (Cc:d):
http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-lucid.git;a=commitdiff;h=0e5e6e2a981eeab61dcc184d51ab769a33af6589
(I used Linus' tree as of today (3a09b1be53d2) plus a few unrelated
patches with mx51_defconfig.)
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH] platform: Facilitate the creation of pseduo-platform busses
From: Kevin Hilman @ 2010-08-05 15:27 UTC (permalink / raw)
To: Magnus Damm
Cc: Patrick Pannuto, linux-kernel@vger.kernel.org,
linux-arm-msm@vger.kernel.org, linux-omap@vger.kernel.org,
damm@opensource.se, lethal@linux-sh.org, rjw@sisk.pl,
eric.y.miao@gmail.com, netdev@vger.kernel.org, Greg Kroah-Hartman,
alan, zt.tmzt
In-Reply-To: <AANLkTimkMy6_201XNGb41e7LivkvVHMkFZWVoA-7UmBo@mail.gmail.com>
Magnus Damm <magnus.damm@gmail.com> writes:
> On Thu, Aug 5, 2010 at 7:14 AM, Patrick Pannuto <ppannuto@codeaurora.org> wrote:
>> Inspiration for this comes from:
>> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg31161.html
>>
>> RFC: http://lkml.org/lkml/2010/8/3/496
>> Patch is unchanged from the RFC. Reviews seemed generally positive
>> and it seemed this was desired functionality.
>
> Thanks for your patch, it's really nice to see work done in this area!
> I'd like to see something like this merged in the not so distant
> future. At this point I'm not so concerned about the details, so I'll
> restrict myself to this:
>
>> /drivers/my_driver.c
>> static struct platform_driver my_driver = {
>> .driver = {
>> .name = "my-driver",
>> .owner = THIS_MODULE,
>> .bus = &my_bus_type,
>> },
>> };
>
> I would really prefer not to have the bus type in the here. I
> understand it's needed at this point, but I wonder if it's possible to
> adjust the device<->driver matching for platform devices to allow any
> type of pseudo-platform bus_type.
I totally agree here. Keeping the drivers ignorant of the bus (or SoC)
they are on will make them much more portable.
Kevin
^ permalink raw reply
* [PATCH] cfg80211: support sysfs namespaces
From: Johannes Berg @ 2010-08-05 15:45 UTC (permalink / raw)
To: John Linville; +Cc: linux-wireless, netdev, Eric W. Biederman
From: Johannes Berg <johannes.berg@intel.com>
Enable using network namespaces with
wireless devices even when sysfs is
enabled using the same infrastructure
that was built for netdevs.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
NB: This generates a harmless gcc warning, I submitted
a patch to fix it here:
http://thread.gmane.org/gmane.linux.kernel/1019330
include/linux/netdevice.h | 2 ++
net/core/net-sysfs.c | 3 ++-
net/wireless/core.c | 7 ++++++-
net/wireless/sysfs.c | 9 +++++++++
4 files changed, 19 insertions(+), 2 deletions(-)
--- wireless-testing.orig/net/core/net-sysfs.c 2010-08-05 17:33:47.000000000 +0200
+++ wireless-testing/net/core/net-sysfs.c 2010-08-05 17:38:27.000000000 +0200
@@ -789,12 +789,13 @@ static const void *net_netlink_ns(struct
return sock_net(sk);
}
-static struct kobj_ns_type_operations net_ns_type_operations = {
+struct kobj_ns_type_operations net_ns_type_operations = {
.type = KOBJ_NS_TYPE_NET,
.current_ns = net_current_ns,
.netlink_ns = net_netlink_ns,
.initial_ns = net_initial_ns,
};
+EXPORT_SYMBOL_GPL(net_ns_type_operations);
static void net_kobj_ns_exit(struct net *net)
{
--- wireless-testing.orig/net/wireless/sysfs.c 2010-08-05 17:33:47.000000000 +0200
+++ wireless-testing/net/wireless/sysfs.c 2010-08-05 17:39:11.000000000 +0200
@@ -110,6 +110,13 @@ static int wiphy_resume(struct device *d
return ret;
}
+static const void *wiphy_namespace(struct device *d)
+{
+ struct wiphy *wiphy = container_of(d, struct wiphy, dev);
+
+ return wiphy_net(wiphy);
+}
+
struct class ieee80211_class = {
.name = "ieee80211",
.owner = THIS_MODULE,
@@ -120,6 +127,8 @@ struct class ieee80211_class = {
#endif
.suspend = wiphy_suspend,
.resume = wiphy_resume,
+ .ns_type = &net_ns_type_operations,
+ .namespace = wiphy_namespace,
};
int wiphy_sysfs_init(void)
--- wireless-testing.orig/net/wireless/core.c 2010-08-05 17:33:47.000000000 +0200
+++ wireless-testing/net/wireless/core.c 2010-08-05 17:38:27.000000000 +0200
@@ -253,11 +253,16 @@ int cfg80211_switch_netns(struct cfg8021
WARN_ON(err);
wdev->netdev->features |= NETIF_F_NETNS_LOCAL;
}
+
+ return err;
}
wiphy_net_set(&rdev->wiphy, net);
- return err;
+ err = device_rename(&rdev->wiphy.dev, dev_name(&rdev->wiphy.dev));
+ WARN_ON(err);
+
+ return 0;
}
static void cfg80211_rfkill_poll(struct rfkill *rfkill, void *data)
--- wireless-testing.orig/include/linux/netdevice.h 2010-08-05 17:13:50.000000000 +0200
+++ wireless-testing/include/linux/netdevice.h 2010-08-05 17:39:02.000000000 +0200
@@ -2171,6 +2171,8 @@ extern void dev_seq_stop(struct seq_file
extern int netdev_class_create_file(struct class_attribute *class_attr);
extern void netdev_class_remove_file(struct class_attribute *class_attr);
+extern struct kobj_ns_type_operations net_ns_type_operations;
+
extern char *netdev_drivername(const struct net_device *dev, char *buffer, int len);
extern void linkwatch_run_queue(void);
^ permalink raw reply
* Re: [PATCH] platform: Facilitate the creation of pseduo-platform busses
From: Kevin Hilman @ 2010-08-05 15:57 UTC (permalink / raw)
To: Patrick Pannuto
Cc: linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
linux-omap@vger.kernel.org, damm@opensource.se,
lethal@linux-sh.org, rjw@sisk.pl, eric.y.miao@gmail.com,
netdev@vger.kernel.org, Greg Kroah-Hartman, alan, zt.tmzt,
grant.likely, magnus.damm
In-Reply-To: <4C5A0C68.9080500@codeaurora.org>
Patrick Pannuto <ppannuto@codeaurora.org> writes:
> On 08/04/2010 05:16 PM, Kevin Hilman wrote:
>> Patrick Pannuto <ppannuto@codeaurora.org> writes:
>>
>>> Inspiration for this comes from:
>>> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg31161.html
>>
>> Also, later in that thread I also wrote[1] what seems to be the core of
>> what you've done here: namely, allow platform_devices and
>> platform_drivers to to be used on custom busses. Patch is at the end of
>> this mail with a more focused changelog. As Greg suggested in his reply
>> to your first version, this part could be merged today, and the
>> platform_bus_init stuff could be added later, after some more review.
>> Some comments below...
>>
>
> I can split this into 2 patches.
Yes, I think that would be better.
> Was your patch sent to linux-kernel or just linux-omap? I'm not on linux-omap...
That thread was on linux-arm-kernel and linux-omap
>
>>> [snip]
>>>
>>> Which will allow the same driver to easily to used on either
>>> the platform bus or the newly defined bus type.
>>
>> Except it requires a re-compile.
>>
>> Rather than doing this at compile time, it would be better to support
>> legacy devices at runtime. You could handle this by simply registering
>> the driver on the custom bus and the platform_bus and let the bus
>> matching code handle it. Then, the same binary would work on both
>> legacy and updated SoCs.
>>
>
> Can you safely register a driver on more than one bus? I didn't think
> that was safe -- normally it's impossible since you're calling
>
> struct BUS_TYPE_driver mydriver;
> BUS_TYPE_driver_register(&mydriver)
>
> but now we have multiple "bus types" that are all actually platform type; still,
> at a minimum you would need:
> struct platform_driver mydrvier1 = {
> .driver.bus = &sub_bus1,
> };
> struct platform_driver mydrvier2 = {
> .driver.bus = &sub_bus2,
> };
> which would all point to the same driver functions, yet the respective devices
> attached for the "same" driver would be on different buses. I fear this might
> confuse some drivers. I don't think dynamic bus assignment is this easy
>
> In short: I do not believe the same driver can be registered on multiple
> different buses -- if this is wrong, please correct me.
It is possible, and currently done in powerpc land where some
drivers handle devices on the platform_bus and the custom OF bus.
However, as noted by Magnus, what we really need here is a way for
drivers to not care at all what kind of bus they are on. There are an
increasing number of drivers that are re-used not just across different
SoCs in the same family, but across totally different SoCs (e.g. drivers
for hardware shared between TI OMAP and TI DaVinci, or SH and SH-Mobile/ARM)
>>
>> Up to here, this looks exactly what I wrote in thread referenced
>> above.
>>
>
> It is, you just went on vacation :)
>
Ah, OK. The changelog was missing credits to that affect, but I was
more concerned that you hadn't seen my example and didn't want to be
duplicating work.
>>>
>>> if (code != retval)
>>> platform_driver_unregister(drv);
>>> @@ -1017,6 +1019,26 @@ struct bus_type platform_bus_type = {
>>> };
>>> EXPORT_SYMBOL_GPL(platform_bus_type);
>>>
>>> +/** platform_bus_type_init - fill in a pseudo-platform-bus
>>> + * @bus: foriegn bus type
>>> + *
>>> + * This init is basically a selective memcpy that
>>> + * won't overwrite any user-defined attributes and
>>> + * only copies things that platform bus defines anyway
>>> + */
>>
>> minor nit: kernel doc style has wrong indentation
>>
>
> will fix
>
>>> +void platform_bus_type_init(struct bus_type *bus)
>>> +{
>>> + if (!bus->dev_attrs)
>>> + bus->dev_attrs = platform_bus_type.dev_attrs;
>>> + if (!bus->match)
>>> + bus->match = platform_bus_type.match;
>>> + if (!bus->uevent)
>>> + bus->uevent = platform_bus_type.uevent;
>>> + if (!bus->pm)
>>> + bus->pm = platform_bus_type.pm;
>>> +}
>>> +EXPORT_SYMBOL_GPL(platform_bus_type_init);
>>
>> With this approach, you should note in the comments/changelog that
>> any selective customization of the bus PM methods must be done after
>> calling platform_bus_type_init().
>
> No they don't. If you call platform_bus_type_init first then you'll
> just overwrite them with new values;
Yes.
> if you call it second then they will all already be well-defined and
> thus not overwritten.
Right, they will not be overwritten, but you'll be left with a mostly
empty dev_pm_ops on the custom bus.
IOW, Most of these custom busses will only want to customize a small
subset of the dev_pm_ops methods (e.g. only the runtime PM methods.) If
you setup your sparsly populated custom dev_pm_ops and then call
platform_bus_type_init() second, dev_pm_ops on the new buswill have *only*
your custom fields, and none of the defaults from platform_dev_pm_ops.
So, what I was getting at is that it should probably be clearer to the
users of platform_bus_type_init() that any customization of dev_pm_ops
should be done after.
>>
>> Also, You've left out the legacy PM methods here. That implies that
>> moving a driver from the platform_bus to the custom bus is not entirely
>> transparent. If the driver still has legacy PM methods, it would stop
>> working on the custom bus.
>>
>> While this is good motivation for converting a driver to dev_pm_ops, at
>> a minimum it should be clear in the changelog that the derivative busses
>> do not support legacy PM methods. However, since it's quite easy to do,
>> and you want the derivative busses to be *exactly* like the platform bus
>> except where explicitly changed, I'd suggest you also check/copy the
>> legacy PM methods.
>>
>> In addition, you've missed several fields in 'struct bus_type'
>> (bus_attr, drv_attr, p, etc.) Without digging deeper into the driver
>> core, I'm not sure if they are all needed at init time, but it should be
>> clear in the comments why they can be excluded.
>>
>
> I copied everything that was defined for platform_bus_type:
>
> struct bus_type platform_bus_type = {
> .name = "platform",
> .dev_attrs = platform_dev_attrs,
> .match = platform_match,
> .uevent = platform_uevent,
> .pm = &platform_dev_pm_ops,
> };
> EXPORT_SYMBOL_GPL(platform_bus_type);
>
> struct bus_type {
> const char *name;
> struct bus_attribute *bus_attrs;
> struct device_attribute *dev_attrs;
> struct driver_attribute *drv_attrs;
>
> int (*match)(struct device *dev, struct device_driver *drv);
> int (*uevent)(struct device *dev, struct kobj_uevent_env *env);
> int (*probe)(struct device *dev);
> int (*remove)(struct device *dev);
> void (*shutdown)(struct device *dev);
>
> int (*suspend)(struct device *dev, pm_message_t state);
> int (*resume)(struct device *dev);
>
> const struct dev_pm_ops *pm;
>
> struct bus_type_private *p;
> };
>
> It is my understanding that everything that I did not copy *should* remain
> unique to each bus; remaining fields will be filled in by bus_register and
> should not be copied.
>
[...]
>
> If you would like to lead this effort, please do so; I did not mean to step
> on your toes, it's just that this is an issue for me as well.
No worries there, my toes are fine. :)
> You had indicated that you were going on vacation for a month and I
> had not seen any more follow-up on this issue, so I forged ahead.
Great, I'm glad you forged ahead. There is definitely a broader need
for something like this, and I have no personal attachment to the code.
I have no problems with you continuing the work (in fact, I'd prefer it.
I have lots of other things to catch up on after my vacation.)
In the future though, it's common (and kind) to note the original author
in the changelog when basing a patch on previous work. Something like
"originally written by..." or "based on the work of..." etc.
Thanks,
Kevin
^ permalink raw reply
* Re: Receive processing stops when dev->poll returns 1
From: Stephen Hemminger @ 2010-08-05 16:04 UTC (permalink / raw)
To: Usha Srinivasan; +Cc: netdev@vger.kernel.org
In-Reply-To: <7866DA1F8D2D4541B87FEE88E633ABAA2B72081FB6@MNEXMB1.qlogic.org>
On Thu, 5 Aug 2010 09:20:03 -0500
Usha Srinivasan <usha.srinivasan@qlogic.com> wrote:
> Hello,
> I have run into an interesting and frustrating problem which I've not been able to resolve. I am hoping someone can help me.
>
> I have a network driver which sets its dev->weight to 100 (like ipoib) and when it processes 100 received packets, following the rules, it decrements dev->quota and *budget and returns 1 without calling netif_rx_complete. When my driver does that, all processing of incoming packets for all interfaces comes to a halt.
>
> How do I know this? Because, as soon as my driver returns 1 to dev->poll, I lose my putty session and eth0 stops working; eth0 counters show that it stops receiving packets, though it is able to transmit. My own device stops receiving packets. I have scoured the code for ipoib and other network devices and I see no difference in what my driver does. I have tried to lower weight for ipoib & eth0 hoping to reproduce with those device it but no luck.
You maybe looking at old documentation on how NAPI works.
In NAPI <= 2.6.23, the driver changed dev->quota and budget
and returned 0 or 1.
For current kernels, the NAPI poll has changed.
Using your example,
dev->weight = 100
budget would be 100
if your network driver process 100 packets, it should return 100
and call napi_complete().
^ permalink raw reply
* RE: Receive processing stops when dev->poll returns 1
From: Usha Srinivasan @ 2010-08-05 16:11 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev@vger.kernel.org
In-Reply-To: <20100805090447.3f67d990@nehalam>
Thanks for your response. What you said is exactly what my driver is doing:
<= 2.6.23
Calls netif_rx_complete if done < budget; decrements quota & *budget by done; returns 0 if done < budget and 1 otherwise.
When 1 is returned, I encounter the problem I described)
> 2.6.23
Calls napi-complete if done < budget; returns done.
When done==budget, I encounter the problem I described.
Any ideas?
-----Original Message-----
From: Stephen Hemminger [mailto:shemminger@vyatta.com]
Sent: Thursday, August 05, 2010 12:05 PM
To: Usha Srinivasan
Cc: netdev@vger.kernel.org
Subject: Re: Receive processing stops when dev->poll returns 1
On Thu, 5 Aug 2010 09:20:03 -0500
Usha Srinivasan <usha.srinivasan@qlogic.com> wrote:
> Hello,
> I have run into an interesting and frustrating problem which I've not been able to resolve. I am hoping someone can help me.
>
> I have a network driver which sets its dev->weight to 100 (like ipoib) and when it processes 100 received packets, following the rules, it decrements dev->quota and *budget and returns 1 without calling netif_rx_complete. When my driver does that, all processing of incoming packets for all interfaces comes to a halt.
>
> How do I know this? Because, as soon as my driver returns 1 to dev->poll, I lose my putty session and eth0 stops working; eth0 counters show that it stops receiving packets, though it is able to transmit. My own device stops receiving packets. I have scoured the code for ipoib and other network devices and I see no difference in what my driver does. I have tried to lower weight for ipoib & eth0 hoping to reproduce with those device it but no luck.
You maybe looking at old documentation on how NAPI works.
In NAPI <= 2.6.23, the driver changed dev->quota and budget
and returned 0 or 1.
For current kernels, the NAPI poll has changed.
Using your example,
dev->weight = 100
budget would be 100
if your network driver process 100 packets, it should return 100
and call napi_complete().
^ permalink raw reply
* Re: Receive processing stops when dev->poll returns 1
From: Stephen Hemminger @ 2010-08-05 16:16 UTC (permalink / raw)
To: Usha Srinivasan; +Cc: netdev@vger.kernel.org
In-Reply-To: <7866DA1F8D2D4541B87FEE88E633ABAA2B72081FDF@MNEXMB1.qlogic.org>
On Thu, 5 Aug 2010 11:11:51 -0500
Usha Srinivasan <usha.srinivasan@qlogic.com> wrote:
> Thanks for your response. What you said is exactly what my driver is doing:
>
>
> <= 2.6.23
> Calls netif_rx_complete if done < budget; decrements quota & *budget by done; returns 0 if done < budget and 1 otherwise.
>
> When 1 is returned, I encounter the problem I described)
>
> > 2.6.23
> Calls napi-complete if done < budget; returns done.
>
> When done==budget, I encounter the problem I described.
>
Your driver did not call napi_complete (and re-enable interrupts).
^ permalink raw reply
* Re: Receive processing stops when dev->poll returns 1
From: Stephen Hemminger @ 2010-08-05 16:22 UTC (permalink / raw)
To: Usha Srinivasan; +Cc: netdev@vger.kernel.org
In-Reply-To: <7866DA1F8D2D4541B87FEE88E633ABAA2B72081FDF@MNEXMB1.qlogic.org>
On Thu, 5 Aug 2010 11:11:51 -0500
Usha Srinivasan <usha.srinivasan@qlogic.com> wrote:
> Thanks for your response. What you said is exactly what my driver is doing:
>
>
> <= 2.6.23
> Calls netif_rx_complete if done < budget; decrements quota & *budget by done; returns 0 if done < budget and 1 otherwise.
>
> When 1 is returned, I encounter the problem I described)
>
> > 2.6.23
> Calls napi-complete if done < budget; returns done.
>
> When done==budget, I encounter the problem I described.
>
> Any ideas?
Ignore last mail...
If you done == budget, the poll will be recalled (after other drivers).
If quantum exhausts, then it gets called it gets deferred to ksoftirq
thread.
One possibility is that the driver is looking at wrong parameter
for budget and is exceeding the requested value. Please post your code.
^ permalink raw reply
* Re: [RFC PATCH 2/2] igb/ixgbe: add code to trigger function reset if reset_devices is set
From: David Woodhouse @ 2010-08-05 16:27 UTC (permalink / raw)
To: David Miller
Cc: jeffrey.t.kirsher, jbarnes, netdev, linux-pci, alexander.h.duyck
In-Reply-To: <20100801.011516.191407437.davem@davemloft.net>
On Sun, 2010-08-01 at 01:15 -0700, David Miller wrote:
> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Date: Fri, 30 Jul 2010 17:59:12 -0700
>
> > From: Alexander Duyck <alexander.h.duyck@intel.com>
> >
> > This change makes it so that both igb and ixgbe can trigger a full pcie
> > function reset if the reset_devices kernel parameter is defined. The main
> > reason for adding this is that kdump can cause serious issues when the
> > kdump kernel resets the IOMMU while DMA transactions are still occurring.
> >
> > Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>
> I tend to disagree with the essence of this change.
>
> Which is that we should add workaround after workaround for things
> that aren't functioning properly in kdump and kexec.
>
> They should have a pass that shuts devices down properly, so that this
> kind of stuff doesn't need to happen in the kernel we then boot into.
For a normal kexec, arguably true.
But in the kdump case, the original kernel has *crashed* and we really
don't have that option -- we need to jump *straight* to the new kernel
and have it reset the hardware.
The device driver really *ought* to be able to reset the hardware from
whatever state it's in when the new kernel starts up. Anything less is
broken, and reminds me of those crappy drivers that only work after a
soft-reboot from Windows.
Most drivers *do* quite happily initialise their device and reliably get
it into a known state; it's just that this particular hardware goes into
a *particularly* stroppy fit when it gets a DMA master abort (which is
what happens when the IOMMU stops it from scribbling into memory after
the new kernel has taken over).
> What happens on non-PCIE systems then? Do they just lose when this
> happens?
If they have a device that's this broken, and the driver can't get it
into a working state any other way, then yes -- I don't see any way to
*avoid* them losing.
I don't like the reset_devices thing though -- the device driver ought
to cope (and reset the device with a full PCIe reset if that's the only
way to make it stop sulking) *regardless* of that option, if it's
necessary.
--
David Woodhouse Open Source Technology Centre
David.Woodhouse@intel.com Intel Corporation
^ permalink raw reply
* Re: [rfc 10/13] [RFC 10/13] IPVS: management of persistence engine modules
From: Stephen Hemminger @ 2010-08-05 16:29 UTC (permalink / raw)
To: Simon Horman
Cc: lvs-devel, netdev, netfilter, netfilter-devel, Wensong Zhang,
Julian Anastasov, Patrick McHardy
In-Reply-To: <20100805115241.482096330@vergenet.net>
On Thu, 05 Aug 2010 20:48:05 +0900
Simon Horman <horms@verge.net.au> wrote:
> +/* lock for service table */
> +static DEFINE_RWLOCK(__ip_vs_pe_lock);
It is already static so why the __?
Reader/writer locks are slower than spinlocks. Either use
a spinlock, or RCU (if possible)
> +/* Bind a service with a pe */
> +void ip_vs_bind_pe(struct ip_vs_service *svc, struct ip_vs_pe *pe)
> +{
> + svc->pe = pe;
> +}
> +
> +/* Unbind a service from its pe */
> +void ip_vs_unbind_pe(struct ip_vs_service *svc)
> +{
> + svc->pe = NULL;
> +}
What does having these wrappers buy?
^ permalink raw reply
* Re: [PATCH] platform: Facilitate the creation of pseduo-platform busses
From: Patrick Pannuto @ 2010-08-05 16:31 UTC (permalink / raw)
To: Kevin Hilman
Cc: linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
linux-omap@vger.kernel.org, damm@opensource.se,
lethal@linux-sh.org, rjw@sisk.pl, eric.y.miao@gmail.com,
netdev@vger.kernel.org, Greg Kroah-Hartman, alan, zt.tmzt,
grant.likely, magnus.damm
In-Reply-To: <87fwytxdba.fsf@deeprootsystems.com>
>>>> [snip]
>>>>
>>>> Which will allow the same driver to easily to used on either
>>>> the platform bus or the newly defined bus type.
>>>
>>> Except it requires a re-compile.
>>>
>>> Rather than doing this at compile time, it would be better to support
>>> legacy devices at runtime. You could handle this by simply registering
>>> the driver on the custom bus and the platform_bus and let the bus
>>> matching code handle it. Then, the same binary would work on both
>>> legacy and updated SoCs.
>>>
>>
>> Can you safely register a driver on more than one bus? I didn't think
>> that was safe -- normally it's impossible since you're calling
>>
>> struct BUS_TYPE_driver mydriver;
>> BUS_TYPE_driver_register(&mydriver)
>>
>> but now we have multiple "bus types" that are all actually platform type; still,
>> at a minimum you would need:
>> struct platform_driver mydrvier1 = {
>> .driver.bus = &sub_bus1,
>> };
>> struct platform_driver mydrvier2 = {
>> .driver.bus = &sub_bus2,
>> };
>> which would all point to the same driver functions, yet the respective devices
>> attached for the "same" driver would be on different buses. I fear this might
>> confuse some drivers. I don't think dynamic bus assignment is this easy
>>
>> In short: I do not believe the same driver can be registered on multiple
>> different buses -- if this is wrong, please correct me.
>
> It is possible, and currently done in powerpc land where some
> drivers handle devices on the platform_bus and the custom OF bus.
>
> However, as noted by Magnus, what we really need here is a way for
> drivers to not care at all what kind of bus they are on. There are an
> increasing number of drivers that are re-used not just across different
> SoCs in the same family, but across totally different SoCs (e.g. drivers
> for hardware shared between TI OMAP and TI DaVinci, or SH and SH-Mobile/ARM)
>
I will start trying to work on this
>>>
>>> Up to here, this looks exactly what I wrote in thread referenced
>>> above.
>>>
>>
>> It is, you just went on vacation :)
>>
>
> Ah, OK. The changelog was missing credits to that affect, but I was
> more concerned that you hadn't seen my example and didn't want to be
> duplicating work.
>
will fix.
>>>> [snip]
>
>> if you call it second then they will all already be well-defined and
>> thus not overwritten.
>
> Right, they will not be overwritten, but you'll be left with a mostly
> empty dev_pm_ops on the custom bus.
>
> IOW, Most of these custom busses will only want to customize a small
> subset of the dev_pm_ops methods (e.g. only the runtime PM methods.) If
> you setup your sparsly populated custom dev_pm_ops and then call
> platform_bus_type_init() second, dev_pm_ops on the new buswill have *only*
> your custom fields, and none of the defaults from platform_dev_pm_ops.
>
> So, what I was getting at is that it should probably be clearer to the
> users of platform_bus_type_init() that any customization of dev_pm_ops
> should be done after.
>
I understand what you're saying now, and I can fix this as well.
>
>>
>> If you would like to lead this effort, please do so; I did not mean to step
>> on your toes, it's just that this is an issue for me as well.
>
> No worries there, my toes are fine. :)
Good :)
>
>> You had indicated that you were going on vacation for a month and I
>> had not seen any more follow-up on this issue, so I forged ahead.
>
> Great, I'm glad you forged ahead. There is definitely a broader need
> for something like this, and I have no personal attachment to the code.
>
> I have no problems with you continuing the work (in fact, I'd prefer it.
> I have lots of other things to catch up on after my vacation.)
>
> In the future though, it's common (and kind) to note the original author
> in the changelog when basing a patch on previous work. Something like
> "originally written by..." or "based on the work of..." etc.
Ok, I can do that; that was the intention of the "original inspiration from"
line at the beginning. Is there a more formal way of indicating this in the
next version of the patch? Should I add you as a "From:" or an "Author:"?
-Pat
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply
* RE: Receive processing stops when dev->poll returns 1
From: Usha Srinivasan @ 2010-08-05 16:36 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev@vger.kernel.org
In-Reply-To: <20100805092243.41671259@nehalam>
I have compared the code in my driver to code in other drivers and they are quite similar. Here is my code:
int vnic_napi_poll(struct napi_struct *napi, int budget)
{
done = 0;
poll_more:
while (done < budget) {
int max = (budget - done);
t = min(<max-supported-by-driver>, max);
n = get-completions(comp_list);
for (i = 0; i < n; i++, done++)
handle_completions(<complist[i]);
if (n != t)
break;
}
if (done < budget) {
netif_rx_complete(dev, napi);
/* check again just to be sure */
if (more-completions()) {
If netif_rx_reschedule(dev, napi))
goto poll_more;
}
}
return done;
}
***********************
BACKPORTED version:
***********************
int vnic_poll(struct net_device *dev, int *budget)
{
int max = min(*budget, dev->quota);
done = 0;
poll_more:
while (max) {
t = min(<max-supported-by-driver>, max);
n = get-completions(comp_list);
for (i = 0; i < n; i++, --max, done++)
handle_completions(<complist[i]);
if (n != t)
break;
}
if (max) {
netif_rx_complete(dev);
/* check again just to be sure */
if (more-completions()) {
If netif_rx_reschedule(dev, napi))
goto poll_more;
}
ret = 0;
} else
ret = 1;
dev->quota -= done;
*budget -= done;
return ret;
}
***********************
-----Original Message-----
From: Stephen Hemminger [mailto:shemminger@vyatta.com]
Sent: Thursday, August 05, 2010 12:23 PM
To: Usha Srinivasan
Cc: netdev@vger.kernel.org
Subject: Re: Receive processing stops when dev->poll returns 1
On Thu, 5 Aug 2010 11:11:51 -0500
Usha Srinivasan <usha.srinivasan@qlogic.com> wrote:
> Thanks for your response. What you said is exactly what my driver is doing:
>
>
> <= 2.6.23
> Calls netif_rx_complete if done < budget; decrements quota & *budget by done; returns 0 if done < budget and 1 otherwise.
>
> When 1 is returned, I encounter the problem I described)
>
> > 2.6.23
> Calls napi-complete if done < budget; returns done.
>
> When done==budget, I encounter the problem I described.
>
> Any ideas?
Ignore last mail...
If you done == budget, the poll will be recalled (after other drivers).
If quantum exhausts, then it gets called it gets deferred to ksoftirq
thread.
One possibility is that the driver is looking at wrong parameter
for budget and is exceeding the requested value. Please post your code.
^ permalink raw reply
* Re: build failure
From: John Fastabend @ 2010-08-05 16:48 UTC (permalink / raw)
To: divya
Cc: LKML, linux-next@vger.kernel.org, linuxppc-dev@ozlabs.org, netdev,
Kirsher, Jeffrey T
In-Reply-To: <4C5A4CF4.4070203@linux.vnet.ibm.com>
divya wrote:
> Yestersday's linux-next(2.6.35_next_20100802) build fails with the following error on both system p and x.
>
>
> drivers/net/ixgbe/ixgbe_main.c: In function 'ixgbe_select_queue':
> drivers/net/ixgbe/ixgbe_main.c:6159: error: 'struct ixgbe_fcoe' has no member named 'up'
> drivers/net/ixgbe/ixgbe_main.c: In function 'ixgbe_xmit_frame':
> drivers/net/ixgbe/ixgbe_main.c:6221: error: 'struct ixgbe_fcoe' has no member named 'up'
> make[3]: *** [drivers/net/ixgbe/ixgbe_main.o] Error 1
> make[2]: *** [drivers/net/ixgbe] Error 2
> make[2]: *** Waiting for unfinished jobs....
> make[1]: *** [drivers/net] Error 2
> make: *** [drivers] Error 2
>
> Thanks
> Divya
>
>
Hi Divya,
Jeff should have a fix for this in his queue already. This is caused by
building with CONFIG_FCOE and without CONFIG_IXGBE_DCB.
Thanks,
John.
^ permalink raw reply
* Re: NET_NS: unregister_netdevice: waiting for lo to become free (after using openvpn)
From: Ben Greear @ 2010-08-05 16:57 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Michael Leun, Greg KH, netdev, davem, linux-kernel,
Alexey Dobriyan, Patrick McHardy
In-Reply-To: <m1y6clz8ta.fsf_-_@fess.ebiederm.org>
On 08/05/2010 02:51 AM, Eric W. Biederman wrote:
> Michael Leun<lkml20100708@newton.leun.net> writes:
>> Unfortunately looks like indefinitely. Never watched longer so far
>> (rebooted soon), but I'm seeing this message now repeated every 10 secs
>> for ~10 minutes on a idle system.
>
> Ugh. A real bug then. These can be a pain to track down and fix. I
> think the last one of these I tracked down took a couple of weeks. I
> will start digging in when I get back from vacation.
I once spent a similar amount of time putting in debug variants that
printed info for each time a netdev was acquired and released.
Maybe a similar logic could be put into the official kernel (and
disabled by default)?
That should save effort in the long run, I'd think.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: Receive processing stops when dev->poll returns 1
From: Stephen Hemminger @ 2010-08-05 17:37 UTC (permalink / raw)
To: Usha Srinivasan; +Cc: netdev@vger.kernel.org
In-Reply-To: <7866DA1F8D2D4541B87FEE88E633ABAA2B72081FE6@MNEXMB1.qlogic.org>
On Thu, 5 Aug 2010 11:36:26 -0500
Usha Srinivasan <usha.srinivasan@qlogic.com> wrote:
> int max = (budget - done);
> t = min(<max-supported-by-driver>, max);
> n = get-completions(comp_list);
You need to handle all completions pending in the poll, the code will
not call you back. So this min() is the problem.
--
^ permalink raw reply
* Re: [PATCH] platform: Facilitate the creation of pseduo-platform busses
From: Patrick Pannuto @ 2010-08-05 17:43 UTC (permalink / raw)
To: Magnus Damm
Cc: linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
linux-omap@vger.kernel.org, damm@opensource.se,
lethal@linux-sh.org, rjw@sisk.pl, eric.y.miao@gmail.com,
netdev@vger.kernel.org, Greg Kroah-Hartman, alan, zt.tmzt
In-Reply-To: <AANLkTimkMy6_201XNGb41e7LivkvVHMkFZWVoA-7UmBo@mail.gmail.com>
On 08/04/2010 07:32 PM, Magnus Damm wrote:
> On Thu, Aug 5, 2010 at 7:14 AM, Patrick Pannuto <ppannuto@codeaurora.org> wrote:
>> Inspiration for this comes from:
>> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg31161.html
>>
>> RFC: http://lkml.org/lkml/2010/8/3/496
>> Patch is unchanged from the RFC. Reviews seemed generally positive
>> and it seemed this was desired functionality.
>
> Thanks for your patch, it's really nice to see work done in this area!
> I'd like to see something like this merged in the not so distant
> future. At this point I'm not so concerned about the details, so I'll
> restrict myself to this:
>
>> /drivers/my_driver.c
>> static struct platform_driver my_driver = {
>> .driver = {
>> .name = "my-driver",
>> .owner = THIS_MODULE,
>> .bus = &my_bus_type,
>> },
>> };
>
> I would really prefer not to have the bus type in the here. I
> understand it's needed at this point, but I wonder if it's possible to
> adjust the device<->driver matching for platform devices to allow any
> type of pseudo-platform bus_type.
>
> The reason why I'd like to avoid having the bus type in the driver is
> that I'd like to reuse the platform driver across multiple
> architectures and buses. For instance, on the SH architecture and
So would I :). That's where this was all heading eventually, I was just
originally doing it in two passes. I have some ideas for how to do this
and will try to send out a patchset either today or tomorrow.
> SH-Mobile ARM we have SoCs with SCIF hardware blocks driven by the
> sh-sci.c serial driver. The sh-sci.c platform driver supports a wide
> range of different SCI(F)(A)(B) hardware blocks, and on any given SoC
> there is a mix of SCIF blocks spread out on different buses.
>
> At this point our SH platform drivers are unaware where their driver
> instanced are located on the SoC. The I/O address and IRQs are
> assigned via struct resource and clocks are managed through clkdev. I
> believe that adding the bus type in the driver will violate this
> abstraction and make it more difficult to just instantiate a driver
> somewhere on the SoC.
>
>> /somewhere/my_device.c
>> static struct platform_device my_device = {
>> .name = "my-device",
>> .id = -1,
>> .dev.bus = &my_bus_type,
>> .dev.parent = &sub_bus_1.dev,
>> };
>
> This I don't mind at all. Actually, this is the place where the
> topology should be defined IMO.
>
Agreed.
> Cheers,
>
> / magnus
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply
* RE: Receive processing stops when dev->poll returns 1
From: Usha Srinivasan @ 2010-08-05 18:11 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev@vger.kernel.org
In-Reply-To: <20100805103715.7d720cb0@nehalam>
Stephen,
The min is inside a while loop; it is purely used to limit the number of completions that are retrieved at-a-time. The outer while loops ensuring that all the completions are handled until budget is reached or there are no completions left. Please look again at the code I sent you.
Usha
-----Original Message-----
From: Stephen Hemminger [mailto:shemminger@vyatta.com]
Sent: Thursday, August 05, 2010 1:37 PM
To: Usha Srinivasan
Cc: netdev@vger.kernel.org
Subject: Re: Receive processing stops when dev->poll returns 1
On Thu, 5 Aug 2010 11:36:26 -0500
Usha Srinivasan <usha.srinivasan@qlogic.com> wrote:
> int max = (budget - done);
> t = min(<max-supported-by-driver>, max);
> n = get-completions(comp_list);
You need to handle all completions pending in the poll, the code will
not call you back. So this min() is the problem.
--
^ permalink raw reply
* Re: [net 0/2] Clean up netfilter cache on xmit-to-self.
From: Ben Greear @ 2010-08-05 18:43 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20100725.220104.59695038.davem@davemloft.net>
On 07/25/2010 10:01 PM, David Miller wrote:
> From: David Miller<davem@davemloft.net>
> Date: Sun, 25 Jul 2010 21:59:12 -0700 (PDT)
>
>> From: Ben Greear<greearb@candelatech.com>
>> Date: Thu, 22 Jul 2010 12:54:46 -0700
>>
>>> These patches are against 2.6.34.1. The first one may be
>>> worth submitting to stable. The second one is less critical
>>> and should probably just go in the development tree.
>>
>> I'll apply the first one to net-2.6 and queue up for -stable
As far as I can tell, this didn't make it into -stable yet?
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re:await your response
From: Mr Jeryy Ntai @ 2010-08-05 18:47 UTC (permalink / raw)
To: info
Although you might be apprehensive about my email as we have not met before, My name is Mr.Jerry Ntai, Head of Operation Mevas
Bank,Hong Kong.I have a secured business proposal in the tune of $21.5m for you to handle with me from my bank.
---- Nuova grafica e nuove funzionalità! Crea subito Gratis la tua nuova Casella di Posta Katamail
^ permalink raw reply
* Re: [net 0/2] Clean up netfilter cache on xmit-to-self.
From: David Miller @ 2010-08-05 18:58 UTC (permalink / raw)
To: greearb; +Cc: netdev
In-Reply-To: <4C5B0659.7020105@candelatech.com>
From: Ben Greear <greearb@candelatech.com>
Date: Thu, 05 Aug 2010 11:43:37 -0700
> On 07/25/2010 10:01 PM, David Miller wrote:
>> From: David Miller<davem@davemloft.net>
>> Date: Sun, 25 Jul 2010 21:59:12 -0700 (PDT)
>>
>>> From: Ben Greear<greearb@candelatech.com>
>>> Date: Thu, 22 Jul 2010 12:54:46 -0700
>>>
>>>> These patches are against 2.6.34.1. The first one may be
>>>> worth submitting to stable. The second one is less critical
>>>> and should probably just go in the development tree.
>>>
>>> I'll apply the first one to net-2.6 and queue up for -stable
>
> As far as I can tell, this didn't make it into -stable yet?
It's sitting in my queue, I will send it out once 2.6.35 has been out
for about 1 week more just to double make sure there aren't any
unwanted side effects.
I never submit changes to -stable immediately unless the change is
painfully obvious and has no possible unwanted side effects. I always
wait a week or even more sometimes.
^ permalink raw reply
* Re: [PATCH] Fix napi_gro_frags vs netpoll path
From: David Miller @ 2010-08-05 19:01 UTC (permalink / raw)
To: jarkao2; +Cc: herbert, netdev, xiaohui.xin
In-Reply-To: <20100805114944.GA21894@ff.dom.local>
From: Jarek Poplawski <jarkao2@gmail.com>
Date: Thu, 5 Aug 2010 11:49:44 +0000
> On Thu, Aug 05, 2010 at 07:40:59PM +0800, Herbert Xu wrote:
>> On Thu, Aug 05, 2010 at 11:03:55AM +0000, Jarek Poplawski wrote:
>> >
>> > The netpoll_rx_on() check in dev_gro_receive() skips part of the
>> > "common" GRO_NORMAL path, especially "pull:" in dev_gro_receive(),
>> > where at least eth header should be copied for entirely paged skbs.
>> >
>> > Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
>>
>> Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> I hope, David will use it with a nearby v2 (the changelog fixed).
I will :-)
^ permalink raw reply
* [PATCH] ethtool: Provide a default implementation of ethtool_ops::get_drvinfo
From: Ben Hutchings @ 2010-08-05 19:20 UTC (permalink / raw)
To: David Miller; +Cc: rusty, netdev, mst, izumi.taku
In-Reply-To: <20100804.215424.200345652.davem@davemloft.net>
The driver name and bus address for a net_device can normally be found
through the driver model now. Instead of requiring drivers to provide
this information redundantly through the ethtool_ops::get_drvinfo
operation, use the driver model to do so if the driver does not define
the operation. Since ETHTOOL_GDRVINFO no longer requires the driver
to implement any operations, do not require net_device::ethtool_ops to
be set either.
Remove implementations of get_drvinfo and ethtool_ops that provide
only this information.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
On Wed, 2010-08-04 at 21:54 -0700, David Miller wrote:
> From: Ben Hutchings <bhutchings@solarflare.com>
> Date: Thu, 05 Aug 2010 04:47:21 +0100
>
> > On Thu, 2010-08-05 at 13:02 +0930, Rusty Russell wrote:
> >> I often use "ethtool -i" command to check what driver controls the
> >> ehternet device. But because current virtio_net driver doesn't
> >> support "ethtool -i", it becomes the following:
> >>
> >> # ethtool -i eth3
> >> Cannot get driver information: Operation not supported
> >>
> >> This patch simply adds the "ethtool -i" support. The following is the
> >> result when using the virtio_net driver with my patch applied to.
> >>
> >> # ethtool -i eth3
> >> driver: virtio_net
> >> version: N/A
> >> firmware-version: N/A
> >> bus-info: virtio0
> >>
> >> Personally, "-i" is one of the most frequently-used option, and most
> >> network drivers support "ethtool -i", so I think virtio_net also
> >> should do.
> > [...]
> >
> > This information is already available generically through sysfs:
> > basename $(readlink /sys/class/net/eth3/device)
> > basename $(readlink /sys/class/net/eth3/device/driver)
> >
> > Given that, we should either recommend that people use that method
> > instead, or we should add an equivalent default implementation of the
> > get_drvinfo operation.
>
> We've had ethtool for nearly a decade, it's a standard facility and
> it's only wise to have all drivers implement as much of the API as
> possible.
>
> As such I've applied Rusty's patch and I will apply any patch which
> makes a driver more fully provide support for all ethtool facilities.
Here's a patch that should do that for a whole bunch of drivers at once. :-)
This is compile-tested on i386, and I tested that it DTRT if I then
remove the ethtool_ops from sfc. There are a few PowerPC-specific
drivers affected that I have not even compile-tested.
I didn't see the previous patch in your public tree, so I applied it
locally before making this patch.
Ben.
drivers/firewire/net.c | 13 -------------
drivers/ieee1394/eth1394.c | 16 ----------------
drivers/net/bmac.c | 7 -------
drivers/net/fec_mpc52xx.c | 6 ------
drivers/net/pasemi_mac_ethtool.c | 16 ----------------
drivers/net/pcmcia/3c574_cs.c | 13 -------------
drivers/net/pcmcia/axnet_cs.c | 13 -------------
drivers/net/pcmcia/ibmtr_cs.c | 13 -------------
drivers/net/pcmcia/pcnet_cs.c | 16 ----------------
drivers/net/sc92031.c | 11 -----------
drivers/net/tulip/xircom_cb.c | 15 ---------------
drivers/net/usb/hso.c | 9 ---------
drivers/net/usb/kaweth.c | 9 ---------
drivers/net/virtio_net.c | 14 --------------
drivers/net/wireless/ray_cs.c | 16 ----------------
drivers/net/wireless/wl3501_cs.c | 18 ------------------
net/core/ethtool.c | 33 +++++++++++++++++++++++----------
17 files changed, 23 insertions(+), 215 deletions(-)
diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c
index 7142eee..277efb3 100644
--- a/drivers/firewire/net.c
+++ b/drivers/firewire/net.c
@@ -8,7 +8,6 @@
#include <linux/bug.h>
#include <linux/device.h>
-#include <linux/ethtool.h>
#include <linux/firewire.h>
#include <linux/firewire-constants.h>
#include <linux/highmem.h>
@@ -1359,17 +1358,6 @@ static int fwnet_change_mtu(struct net_device *net, int new_mtu)
return 0;
}
-static void fwnet_get_drvinfo(struct net_device *net,
- struct ethtool_drvinfo *info)
-{
- strcpy(info->driver, KBUILD_MODNAME);
- strcpy(info->bus_info, "ieee1394");
-}
-
-static const struct ethtool_ops fwnet_ethtool_ops = {
- .get_drvinfo = fwnet_get_drvinfo,
-};
-
static const struct net_device_ops fwnet_netdev_ops = {
.ndo_open = fwnet_open,
.ndo_stop = fwnet_stop,
@@ -1388,7 +1376,6 @@ static void fwnet_init_dev(struct net_device *net)
net->hard_header_len = FWNET_HLEN;
net->type = ARPHRD_IEEE1394;
net->tx_queue_len = 10;
- SET_ETHTOOL_OPS(net, &fwnet_ethtool_ops);
}
/* caller must hold fwnet_device_mutex */
diff --git a/drivers/ieee1394/eth1394.c b/drivers/ieee1394/eth1394.c
index a4e9dcb..62ab09e 100644
--- a/drivers/ieee1394/eth1394.c
+++ b/drivers/ieee1394/eth1394.c
@@ -58,7 +58,6 @@
#include <linux/tcp.h>
#include <linux/skbuff.h>
#include <linux/bitops.h>
-#include <linux/ethtool.h>
#include <asm/uaccess.h>
#include <asm/delay.h>
#include <asm/unaligned.h>
@@ -173,8 +172,6 @@ static netdev_tx_t ether1394_tx(struct sk_buff *skb,
struct net_device *dev);
static void ether1394_iso(struct hpsb_iso *iso);
-static const struct ethtool_ops ethtool_ops;
-
static int ether1394_write(struct hpsb_host *host, int srcid, int destid,
quadlet_t *data, u64 addr, size_t len, u16 flags);
static void ether1394_add_host(struct hpsb_host *host);
@@ -525,8 +522,6 @@ static void ether1394_init_dev(struct net_device *dev)
dev->header_ops = ðer1394_header_ops;
dev->netdev_ops = ðer1394_netdev_ops;
- SET_ETHTOOL_OPS(dev, ðtool_ops);
-
dev->watchdog_timeo = ETHER1394_TIMEOUT;
dev->flags = IFF_BROADCAST | IFF_MULTICAST;
dev->features = NETIF_F_HIGHDMA;
@@ -1698,17 +1693,6 @@ fail:
return NETDEV_TX_OK;
}
-static void ether1394_get_drvinfo(struct net_device *dev,
- struct ethtool_drvinfo *info)
-{
- strcpy(info->driver, driver_name);
- strcpy(info->bus_info, "ieee1394"); /* FIXME provide more detail? */
-}
-
-static const struct ethtool_ops ethtool_ops = {
- .get_drvinfo = ether1394_get_drvinfo
-};
-
static int __init ether1394_init_module(void)
{
int err;
diff --git a/drivers/net/bmac.c b/drivers/net/bmac.c
index 959add2..9322699 100644
--- a/drivers/net/bmac.c
+++ b/drivers/net/bmac.c
@@ -1233,15 +1233,8 @@ static void bmac_reset_and_enable(struct net_device *dev)
}
spin_unlock_irqrestore(&bp->lock, flags);
}
-static void bmac_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *info)
-{
- struct bmac_data *bp = netdev_priv(dev);
- strcpy(info->driver, "bmac");
- strcpy(info->bus_info, dev_name(&bp->mdev->ofdev.dev));
-}
static const struct ethtool_ops bmac_ethtool_ops = {
- .get_drvinfo = bmac_get_drvinfo,
.get_link = ethtool_op_get_link,
};
diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
index d1a5b17..f04fa98 100644
--- a/drivers/net/fec_mpc52xx.c
+++ b/drivers/net/fec_mpc52xx.c
@@ -771,11 +771,6 @@ static void mpc52xx_fec_reset(struct net_device *dev)
/* ethtool interface */
-static void mpc52xx_fec_get_drvinfo(struct net_device *dev,
- struct ethtool_drvinfo *info)
-{
- strcpy(info->driver, DRIVER_NAME);
-}
static int mpc52xx_fec_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
{
@@ -810,7 +805,6 @@ static void mpc52xx_fec_set_msglevel(struct net_device *dev, u32 level)
}
static const struct ethtool_ops mpc52xx_fec_ethtool_ops = {
- .get_drvinfo = mpc52xx_fec_get_drvinfo,
.get_settings = mpc52xx_fec_get_settings,
.set_settings = mpc52xx_fec_set_settings,
.get_link = ethtool_op_get_link,
diff --git a/drivers/net/pasemi_mac_ethtool.c b/drivers/net/pasemi_mac_ethtool.c
index fefa79e..4825959 100644
--- a/drivers/net/pasemi_mac_ethtool.c
+++ b/drivers/net/pasemi_mac_ethtool.c
@@ -90,21 +90,6 @@ pasemi_mac_ethtool_set_settings(struct net_device *netdev,
return phy_ethtool_sset(phydev, cmd);
}
-static void
-pasemi_mac_ethtool_get_drvinfo(struct net_device *netdev,
- struct ethtool_drvinfo *drvinfo)
-{
- struct pasemi_mac *mac;
- mac = netdev_priv(netdev);
-
- /* clear and fill out info */
- memset(drvinfo, 0, sizeof(struct ethtool_drvinfo));
- strncpy(drvinfo->driver, "pasemi_mac", 12);
- strcpy(drvinfo->version, "N/A");
- strcpy(drvinfo->fw_version, "N/A");
- strncpy(drvinfo->bus_info, pci_name(mac->pdev), 32);
-}
-
static u32
pasemi_mac_ethtool_get_msglevel(struct net_device *netdev)
{
@@ -164,7 +149,6 @@ static void pasemi_mac_get_strings(struct net_device *netdev, u32 stringset,
const struct ethtool_ops pasemi_mac_ethtool_ops = {
.get_settings = pasemi_mac_ethtool_get_settings,
.set_settings = pasemi_mac_ethtool_set_settings,
- .get_drvinfo = pasemi_mac_ethtool_get_drvinfo,
.get_msglevel = pasemi_mac_ethtool_get_msglevel,
.set_msglevel = pasemi_mac_ethtool_set_msglevel,
.get_link = ethtool_op_get_link,
diff --git a/drivers/net/pcmcia/3c574_cs.c b/drivers/net/pcmcia/3c574_cs.c
index 10ee106..3a3fa26 100644
--- a/drivers/net/pcmcia/3c574_cs.c
+++ b/drivers/net/pcmcia/3c574_cs.c
@@ -83,7 +83,6 @@ earlier 3Com products.
#include <linux/skbuff.h>
#include <linux/if_arp.h>
#include <linux/ioport.h>
-#include <linux/ethtool.h>
#include <linux/bitops.h>
#include <linux/mii.h>
@@ -239,7 +238,6 @@ static int el3_rx(struct net_device *dev, int worklimit);
static int el3_close(struct net_device *dev);
static void el3_tx_timeout(struct net_device *dev);
static int el3_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
-static const struct ethtool_ops netdev_ethtool_ops;
static void set_rx_mode(struct net_device *dev);
static void set_multicast_list(struct net_device *dev);
@@ -286,7 +284,6 @@ static int tc574_probe(struct pcmcia_device *link)
link->conf.ConfigIndex = 1;
dev->netdev_ops = &el3_netdev_ops;
- SET_ETHTOOL_OPS(dev, &netdev_ethtool_ops);
dev->watchdog_timeo = TX_TIMEOUT;
return tc574_config(link);
@@ -1065,16 +1062,6 @@ static int el3_rx(struct net_device *dev, int worklimit)
return worklimit;
}
-static void netdev_get_drvinfo(struct net_device *dev,
- struct ethtool_drvinfo *info)
-{
- strcpy(info->driver, "3c574_cs");
-}
-
-static const struct ethtool_ops netdev_ethtool_ops = {
- .get_drvinfo = netdev_get_drvinfo,
-};
-
/* Provide ioctl() calls to examine the MII xcvr state. */
static int el3_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
{
diff --git a/drivers/net/pcmcia/axnet_cs.c b/drivers/net/pcmcia/axnet_cs.c
index 33525bf..3003745 100644
--- a/drivers/net/pcmcia/axnet_cs.c
+++ b/drivers/net/pcmcia/axnet_cs.c
@@ -32,7 +32,6 @@
#include <linux/timer.h>
#include <linux/delay.h>
#include <linux/spinlock.h>
-#include <linux/ethtool.h>
#include <linux/netdevice.h>
#include <linux/etherdevice.h>
#include <linux/crc32.h>
@@ -87,7 +86,6 @@ static netdev_tx_t axnet_start_xmit(struct sk_buff *skb,
static struct net_device_stats *get_stats(struct net_device *dev);
static void set_multicast_list(struct net_device *dev);
static void axnet_tx_timeout(struct net_device *dev);
-static const struct ethtool_ops netdev_ethtool_ops;
static irqreturn_t ei_irq_wrapper(int irq, void *dev_id);
static void ei_watchdog(u_long arg);
static void axnet_reset_8390(struct net_device *dev);
@@ -172,7 +170,6 @@ static int axnet_probe(struct pcmcia_device *link)
dev->netdev_ops = &axnet_netdev_ops;
- SET_ETHTOOL_OPS(dev, &netdev_ethtool_ops);
dev->watchdog_timeo = TX_TIMEOUT;
return axnet_config(link);
@@ -658,16 +655,6 @@ reschedule:
add_timer(&info->watchdog);
}
-static void netdev_get_drvinfo(struct net_device *dev,
- struct ethtool_drvinfo *info)
-{
- strcpy(info->driver, "axnet_cs");
-}
-
-static const struct ethtool_ops netdev_ethtool_ops = {
- .get_drvinfo = netdev_get_drvinfo,
-};
-
/*====================================================================*/
static int axnet_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
diff --git a/drivers/net/pcmcia/ibmtr_cs.c b/drivers/net/pcmcia/ibmtr_cs.c
index 67ee985..c02ae65 100644
--- a/drivers/net/pcmcia/ibmtr_cs.c
+++ b/drivers/net/pcmcia/ibmtr_cs.c
@@ -52,7 +52,6 @@
#include <linux/string.h>
#include <linux/timer.h>
#include <linux/module.h>
-#include <linux/ethtool.h>
#include <linux/netdevice.h>
#include <linux/trdevice.h>
#include <linux/ibmtr.h>
@@ -108,16 +107,6 @@ typedef struct ibmtr_dev_t {
struct tok_info *ti;
} ibmtr_dev_t;
-static void netdev_get_drvinfo(struct net_device *dev,
- struct ethtool_drvinfo *info)
-{
- strcpy(info->driver, "ibmtr_cs");
-}
-
-static const struct ethtool_ops netdev_ethtool_ops = {
- .get_drvinfo = netdev_get_drvinfo,
-};
-
static irqreturn_t ibmtr_interrupt(int irq, void *dev_id) {
ibmtr_dev_t *info = dev_id;
struct net_device *dev = info->dev;
@@ -161,8 +150,6 @@ static int __devinit ibmtr_attach(struct pcmcia_device *link)
info->dev = dev;
- SET_ETHTOOL_OPS(dev, &netdev_ethtool_ops);
-
return ibmtr_config(link);
} /* ibmtr_attach */
diff --git a/drivers/net/pcmcia/pcnet_cs.c b/drivers/net/pcmcia/pcnet_cs.c
index bfdef72..17c7c66 100644
--- a/drivers/net/pcmcia/pcnet_cs.c
+++ b/drivers/net/pcmcia/pcnet_cs.c
@@ -35,7 +35,6 @@
#include <linux/string.h>
#include <linux/timer.h>
#include <linux/delay.h>
-#include <linux/ethtool.h>
#include <linux/netdevice.h>
#include <linux/log2.h>
#include <linux/etherdevice.h>
@@ -101,7 +100,6 @@ static void pcnet_release(struct pcmcia_device *link);
static int pcnet_open(struct net_device *dev);
static int pcnet_close(struct net_device *dev);
static int ei_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
-static const struct ethtool_ops netdev_ethtool_ops;
static irqreturn_t ei_irq_wrapper(int irq, void *dev_id);
static void ei_watchdog(u_long arg);
static void pcnet_reset_8390(struct net_device *dev);
@@ -632,8 +630,6 @@ static int pcnet_config(struct pcmcia_device *link)
ei_status.word16 = 1;
ei_status.reset_8390 = &pcnet_reset_8390;
- SET_ETHTOOL_OPS(dev, &netdev_ethtool_ops);
-
if (info->flags & (IS_DL10019|IS_DL10022))
mii_phy_probe(dev);
@@ -1147,18 +1143,6 @@ reschedule:
/*====================================================================*/
-static void netdev_get_drvinfo(struct net_device *dev,
- struct ethtool_drvinfo *info)
-{
- strcpy(info->driver, "pcnet_cs");
-}
-
-static const struct ethtool_ops netdev_ethtool_ops = {
- .get_drvinfo = netdev_get_drvinfo,
-};
-
-/*====================================================================*/
-
static int ei_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
{
diff --git a/drivers/net/sc92031.c b/drivers/net/sc92031.c
index 8c4067a..31b92f5 100644
--- a/drivers/net/sc92031.c
+++ b/drivers/net/sc92031.c
@@ -1251,16 +1251,6 @@ static int sc92031_ethtool_set_settings(struct net_device *dev,
return 0;
}
-static void sc92031_ethtool_get_drvinfo(struct net_device *dev,
- struct ethtool_drvinfo *drvinfo)
-{
- struct sc92031_priv *priv = netdev_priv(dev);
- struct pci_dev *pdev = priv->pdev;
-
- strcpy(drvinfo->driver, SC92031_NAME);
- strcpy(drvinfo->bus_info, pci_name(pdev));
-}
-
static void sc92031_ethtool_get_wol(struct net_device *dev,
struct ethtool_wolinfo *wolinfo)
{
@@ -1382,7 +1372,6 @@ static void sc92031_ethtool_get_ethtool_stats(struct net_device *dev,
static const struct ethtool_ops sc92031_ethtool_ops = {
.get_settings = sc92031_ethtool_get_settings,
.set_settings = sc92031_ethtool_set_settings,
- .get_drvinfo = sc92031_ethtool_get_drvinfo,
.get_wol = sc92031_ethtool_get_wol,
.set_wol = sc92031_ethtool_set_wol,
.nway_reset = sc92031_ethtool_nway_reset,
diff --git a/drivers/net/tulip/xircom_cb.c b/drivers/net/tulip/xircom_cb.c
index a439e93..5a73752 100644
--- a/drivers/net/tulip/xircom_cb.c
+++ b/drivers/net/tulip/xircom_cb.c
@@ -29,7 +29,6 @@
#include <linux/skbuff.h>
#include <linux/delay.h>
#include <linux/init.h>
-#include <linux/ethtool.h>
#include <linux/bitops.h>
#include <asm/uaccess.h>
@@ -181,19 +180,6 @@ static void print_binary(unsigned int number)
}
#endif
-static void netdev_get_drvinfo(struct net_device *dev,
- struct ethtool_drvinfo *info)
-{
- struct xircom_private *private = netdev_priv(dev);
-
- strcpy(info->driver, "xircom_cb");
- strcpy(info->bus_info, pci_name(private->pdev));
-}
-
-static const struct ethtool_ops netdev_ethtool_ops = {
- .get_drvinfo = netdev_get_drvinfo,
-};
-
static const struct net_device_ops netdev_ops = {
.ndo_open = xircom_open,
.ndo_stop = xircom_close,
@@ -279,7 +265,6 @@ static int __devinit xircom_probe(struct pci_dev *pdev, const struct pci_device_
setup_descriptors(private);
dev->netdev_ops = &netdev_ops;
- SET_ETHTOOL_OPS(dev, &netdev_ethtool_ops);
pci_set_drvdata(pdev, dev);
if (register_netdev(dev)) {
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 6efca66..4f123f8 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -843,16 +843,7 @@ static netdev_tx_t hso_net_start_xmit(struct sk_buff *skb,
return NETDEV_TX_OK;
}
-static void hso_get_drvinfo(struct net_device *net, struct ethtool_drvinfo *info)
-{
- struct hso_net *odev = netdev_priv(net);
-
- strncpy(info->driver, driver_name, ETHTOOL_BUSINFO_LEN);
- usb_make_path(odev->parent->usb, info->bus_info, sizeof info->bus_info);
-}
-
static const struct ethtool_ops ops = {
- .get_drvinfo = hso_get_drvinfo,
.get_link = ethtool_op_get_link
};
diff --git a/drivers/net/usb/kaweth.c b/drivers/net/usb/kaweth.c
index 2b7b39c..5e98643 100644
--- a/drivers/net/usb/kaweth.c
+++ b/drivers/net/usb/kaweth.c
@@ -759,14 +759,6 @@ static int kaweth_close(struct net_device *net)
return 0;
}
-static void kaweth_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *info)
-{
- struct kaweth_device *kaweth = netdev_priv(dev);
-
- strlcpy(info->driver, driver_name, sizeof(info->driver));
- usb_make_path(kaweth->dev, info->bus_info, sizeof (info->bus_info));
-}
-
static u32 kaweth_get_link(struct net_device *dev)
{
struct kaweth_device *kaweth = netdev_priv(dev);
@@ -775,7 +767,6 @@ static u32 kaweth_get_link(struct net_device *dev)
}
static const struct ethtool_ops ops = {
- .get_drvinfo = kaweth_get_drvinfo,
.get_link = kaweth_get_link
};
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 4598e9d..bb6b67f 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -705,19 +705,6 @@ static int virtnet_close(struct net_device *dev)
return 0;
}
-static void virtnet_get_drvinfo(struct net_device *dev,
- struct ethtool_drvinfo *drvinfo)
-{
- struct virtnet_info *vi = netdev_priv(dev);
- struct virtio_device *vdev = vi->vdev;
-
- strncpy(drvinfo->driver, KBUILD_MODNAME, ARRAY_SIZE(drvinfo->driver));
- strncpy(drvinfo->version, "N/A", ARRAY_SIZE(drvinfo->version));
- strncpy(drvinfo->fw_version, "N/A", ARRAY_SIZE(drvinfo->fw_version));
- strncpy(drvinfo->bus_info, dev_name(&vdev->dev),
- ARRAY_SIZE(drvinfo->bus_info));
-}
-
static int virtnet_set_tx_csum(struct net_device *dev, u32 data)
{
struct virtnet_info *vi = netdev_priv(dev);
@@ -830,7 +817,6 @@ static void virtnet_vlan_rx_kill_vid(struct net_device *dev, u16 vid)
}
static const struct ethtool_ops virtnet_ethtool_ops = {
- .get_drvinfo = virtnet_get_drvinfo,
.set_tx_csum = virtnet_set_tx_csum,
.set_sg = ethtool_op_set_sg,
.set_tso = ethtool_op_set_tso,
diff --git a/drivers/net/wireless/ray_cs.c b/drivers/net/wireless/ray_cs.c
index 9c38fc3..0234800 100644
--- a/drivers/net/wireless/ray_cs.c
+++ b/drivers/net/wireless/ray_cs.c
@@ -43,7 +43,6 @@
#include <linux/if_arp.h>
#include <linux/ioport.h>
#include <linux/skbuff.h>
-#include <linux/ethtool.h>
#include <linux/ieee80211.h>
#include <pcmcia/cs_types.h>
@@ -81,8 +80,6 @@ static int ray_dev_config(struct net_device *dev, struct ifmap *map);
static struct net_device_stats *ray_get_stats(struct net_device *dev);
static int ray_dev_init(struct net_device *dev);
-static const struct ethtool_ops netdev_ethtool_ops;
-
static int ray_open(struct net_device *dev);
static netdev_tx_t ray_dev_start_xmit(struct sk_buff *skb,
struct net_device *dev);
@@ -335,7 +332,6 @@ static int ray_probe(struct pcmcia_device *p_dev)
/* Raylink entries in the device structure */
dev->netdev_ops = &ray_netdev_ops;
- SET_ETHTOOL_OPS(dev, &netdev_ethtool_ops);
dev->wireless_handlers = &ray_handler_def;
#ifdef WIRELESS_SPY
local->wireless_data.spy_data = &local->spy_data;
@@ -1079,18 +1075,6 @@ AP to AP 1 1 dest AP src AP dest source
}
} /* end encapsulate_frame */
-/*===========================================================================*/
-
-static void netdev_get_drvinfo(struct net_device *dev,
- struct ethtool_drvinfo *info)
-{
- strcpy(info->driver, "ray_cs");
-}
-
-static const struct ethtool_ops netdev_ethtool_ops = {
- .get_drvinfo = netdev_get_drvinfo,
-};
-
/*====================================================================*/
/*------------------------------------------------------------------*/
diff --git a/drivers/net/wireless/wl3501_cs.c b/drivers/net/wireless/wl3501_cs.c
index 376c6b9..fda932e 100644
--- a/drivers/net/wireless/wl3501_cs.c
+++ b/drivers/net/wireless/wl3501_cs.c
@@ -29,7 +29,6 @@
#include <linux/delay.h>
#include <linux/types.h>
-#include <linux/ethtool.h>
#include <linux/init.h>
#include <linux/interrupt.h>
#include <linux/in.h>
@@ -89,13 +88,6 @@
static int wl3501_config(struct pcmcia_device *link);
static void wl3501_release(struct pcmcia_device *link);
-/*
- * The dev_info variable is the "key" that is used to match up this
- * device driver with appropriate cards, through the card configuration
- * database.
- */
-static dev_info_t wl3501_dev_info = "wl3501_cs";
-
static const struct {
int reg_domain;
int min, max, deflt;
@@ -1419,15 +1411,6 @@ static struct iw_statistics *wl3501_get_wireless_stats(struct net_device *dev)
return wstats;
}
-static void wl3501_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *info)
-{
- strlcpy(info->driver, wl3501_dev_info, sizeof(info->driver));
-}
-
-static const struct ethtool_ops ops = {
- .get_drvinfo = wl3501_get_drvinfo
-};
-
/**
* wl3501_detach - deletes a driver "instance"
* @link - FILL_IN
@@ -1914,7 +1897,6 @@ static int wl3501_probe(struct pcmcia_device *p_dev)
this->p_dev = p_dev;
dev->wireless_data = &this->wireless_data;
dev->wireless_handlers = &wl3501_handler_def;
- SET_ETHTOOL_OPS(dev, &ops);
netif_stop_queue(dev);
p_dev->priv = dev;
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 7a85367..d2c4da5 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -205,18 +205,24 @@ static noinline_for_stack int ethtool_get_drvinfo(struct net_device *dev,
struct ethtool_drvinfo info;
const struct ethtool_ops *ops = dev->ethtool_ops;
- if (!ops->get_drvinfo)
- return -EOPNOTSUPP;
-
memset(&info, 0, sizeof(info));
info.cmd = ETHTOOL_GDRVINFO;
- ops->get_drvinfo(dev, &info);
+ if (ops && ops->get_drvinfo) {
+ ops->get_drvinfo(dev, &info);
+ } else if (dev->dev.parent && dev->dev.parent->driver) {
+ strlcpy(info.bus_info, dev_name(dev->dev.parent),
+ sizeof(info.bus_info));
+ strlcpy(info.driver, dev->dev.parent->driver->name,
+ sizeof(info.driver));
+ } else {
+ return -EOPNOTSUPP;
+ }
/*
* this method of obtaining string set info is deprecated;
* Use ETHTOOL_GSSET_INFO instead.
*/
- if (ops->get_sset_count) {
+ if (ops && ops->get_sset_count) {
int rc;
rc = ops->get_sset_count(dev, ETH_SS_TEST);
@@ -229,9 +235,9 @@ static noinline_for_stack int ethtool_get_drvinfo(struct net_device *dev,
if (rc >= 0)
info.n_priv_flags = rc;
}
- if (ops->get_regs_len)
+ if (ops && ops->get_regs_len)
info.regdump_len = ops->get_regs_len(dev);
- if (ops->get_eeprom_len)
+ if (ops && ops->get_eeprom_len)
info.eedump_len = ops->get_eeprom_len(dev);
if (copy_to_user(useraddr, &info, sizeof(info)))
@@ -1402,12 +1408,19 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
if (!dev || !netif_device_present(dev))
return -ENODEV;
- if (!dev->ethtool_ops)
- return -EOPNOTSUPP;
-
if (copy_from_user(ðcmd, useraddr, sizeof(ethcmd)))
return -EFAULT;
+ if (!dev->ethtool_ops) {
+ /* ETHTOOL_GDRVINFO does not require any driver support.
+ * It is also unprivileged and does not change anything,
+ * so we can take a shortcut to it. */
+ if (ethcmd == ETHTOOL_GDRVINFO)
+ return ethtool_get_drvinfo(dev, useraddr);
+ else
+ return -EOPNOTSUPP;
+ }
+
/* Allow some commands to be done by anyone */
switch (ethcmd) {
case ETHTOOL_GDRVINFO:
--
1.6.2.5
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* null pointer at cls_api.c
From: Franchoze Eric @ 2010-08-05 19:24 UTC (permalink / raw)
To: netdev
Hello, can somebody follow up with approriate fix for that bug? It's reproducible on the latest 2.6.32.17 kernel with Centos5 user land.
Script to reproduce is bellow. I did quick fix for that to show where is the issue.
Thanks.
--- net/sched/cls_api.c_orig 2010-08-05 10:46:01.000000000 -0700
+++ net/sched/cls_api.c 2010-08-05 12:03:01.000000000 -0700
@@ -255,6 +255,11 @@ replay:
kfree(tp);
goto errout;
}
+
+ if (!tp || !tp->ops) {
+ err = -ENOENT;
+ goto errout;
+ }
tp->ops = tp_ops;
tp->protocol = protocol;
tp->prio = nprio ? : TC_H_MAJ(tcf_auto_prio(*back));
#!/bin/bash
DEV=tun1
SPEED="100kbit"
ISPEED="10Mbit"
tc qdisc del dev $DEV root
tc qdisc add dev $DEV root handle 1: htb default 1
tc class add dev $DEV parent 1: classid 1:1 htb rate ${ISPEED}
tc class add dev $DEV parent 1:1 classid 1:2 htb rate ${SPEED} ceil ${SPEED} prio 0
tc qdisc add dev $DEV parent 1:2 handle 2: sfq perturb 10
tc filter add dev $DEV parent 2: protocol ip u32 match ip dport 80 0xffff flowid 2:1
BUG: unable to handle kernel NULL pointer dereference at (null)
IP: [<(null)>] (null)
PGD 81ca27067 PUD 825b8e067 PMD 0
Oops: 0010 [#1] PREEMPT SMP
last sysfs file: /sys/class/misc/watchdog/dev
CPU 7
Modules linked in: wdt_pci sc520_wdt pcwd_pci i6300esb hpwdt cls_u32 sch_sfq sch_htb lockd sunrpc xt_connlimit xt_MARK iptable_mangle ipt_REDIRECT iptable_nat nf_nat tun ipt_REJECT nf_conntrack_ipv4 nf_defrag_ipv4 xt_state nf_conntrack xt_tcpudp iptable_filter ip_tables x_tables dm_mirror dm_multipath scsi_dh sbs sbshc power_meter hwmon battery ac tpm_tis serio_raw rtc_cmos rtc_core rtc_lib tpm tpm_bios button tg3 firmware_class libphy amd64_edac_mod edac_core i2c_piix4 i2c_core dm_region_hash dm_log dm_mod sata_svw libata cciss sd_mod scsi_mod ext3 jbd
Pid: 4807, comm: tc Not tainted 2.6.32.17-af #1 ProLiant DL165 G6
RIP: 0010:[<0000000000000000>] [<(null)>] (null)
RSP: 0018:ffff880823547880 EFLAGS: 00010286
RAX: ffffffffa0292b80 RBX: ffff88082484f700 RCX: ffff880823547898
RDX: 0000000000020001 RSI: 0000000000000000 RDI: ffff88082568e000
RBP: ffff8808235478d8 R08: ffff880427c00040 R09: 0000000000000000
R10: 0000000000000000 R11: ffff88082484f7c0 R12: ffff88082546603c
R13: ffff88081ca8f440 R14: ffff88082484f7c0 R15: ffff880823547908
FS: 00007fb0ca8186e0(0000) GS:ffff880436440000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 00000008259ec000 CR4: 00000000000006e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process tc (pid: 4807, threadinfo ffff880823546000, task ffff880824b68800)
Stack:
ffffffffa0299836 ffff8808235478c8 0000000000000000 0000000000000000
<0> ffff88081ca8f440 ffff88082546603c ffff88081ca8f440 ffff88082546603c
<0> 0000000080000800 ffff88082484f700 0000000000000000 ffff880823547998
Call Trace:
[<ffffffffa0299836>] ? u32_set_parms+0xca/0x14f [cls_u32]
[<ffffffffa0299e49>] u32_change+0x316/0x3bd [cls_u32]
[<ffffffffa0299696>] ? kzalloc+0xf/0x11 [cls_u32]
[<ffffffff8126f9e1>] tc_ctl_tfilter+0x505/0x5f6
[<ffffffff810d4fbd>] ? ____cache_alloc+0x1d/0x223
[<ffffffff81262377>] rtnetlink_rcv_msg+0x1c3/0x1e2
[<ffffffff812621b4>] ? rtnetlink_rcv_msg+0x0/0x1e2
[<ffffffff81273273>] netlink_rcv_skb+0x3e/0x8d
[<ffffffff812620aa>] rtnetlink_rcv+0x20/0x29
[<ffffffff812730e8>] netlink_unicast+0xed/0x154
[<ffffffff8127464b>] netlink_sendmsg+0x245/0x258
[<ffffffff812730f8>] ? netlink_unicast+0xfd/0x154
[<ffffffff81248ffc>] __sock_sendmsg+0x59/0x62
[<ffffffff8124988e>] sock_sendmsg+0xc7/0xe0
[<ffffffff810604bb>] ? autoremove_wake_function+0x0/0x38
[<ffffffff810604bb>] ? autoremove_wake_function+0x0/0x38
[<ffffffff8124a345>] ? move_addr_to_kernel+0x40/0x49
[<ffffffff8125254b>] ? verify_iovec+0x4f/0x94
[<ffffffff81249a85>] sys_sendmsg+0x1de/0x24d
[<ffffffff810ae914>] ? lru_cache_add_lru+0x38/0x3d
[<ffffffff810c6468>] ? page_add_new_anon_rmap+0x3a/0x49
[<ffffffff810bee8f>] ? handle_mm_fault+0x5e8/0x602
[<ffffffff8100bb6b>] system_call_fastpath+0x16/0x1b
Code: Bad RIP value.
RIP [<(null)>] (null)
RSP <ffff880823547880>
CR2: 0000000000000000
---[ end trace 62c007c0b888891e ]---
^ permalink raw reply
* Re: [net 0/2] Clean up netfilter cache on xmit-to-self.
From: Ben Greear @ 2010-08-05 19:29 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20100805.115850.58405719.davem@davemloft.net>
On 08/05/2010 11:58 AM, David Miller wrote:
> From: Ben Greear<greearb@candelatech.com>
> Date: Thu, 05 Aug 2010 11:43:37 -0700
>
>> On 07/25/2010 10:01 PM, David Miller wrote:
>>> From: David Miller<davem@davemloft.net>
>>> Date: Sun, 25 Jul 2010 21:59:12 -0700 (PDT)
>>>
>>>> From: Ben Greear<greearb@candelatech.com>
>>>> Date: Thu, 22 Jul 2010 12:54:46 -0700
>>>>
>>>>> These patches are against 2.6.34.1. The first one may be
>>>>> worth submitting to stable. The second one is less critical
>>>>> and should probably just go in the development tree.
>>>>
>>>> I'll apply the first one to net-2.6 and queue up for -stable
>>
>> As far as I can tell, this didn't make it into -stable yet?
>
> It's sitting in my queue, I will send it out once 2.6.35 has been out
> for about 1 week more just to double make sure there aren't any
> unwanted side effects.
>
> I never submit changes to -stable immediately unless the change is
> painfully obvious and has no possible unwanted side effects. I always
> wait a week or even more sometimes.
Ok, no problem. I just wanted to make sure it hadn't been lost.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox