* Re: oops with recent wireless-dev tree [not found] ` <20070829223752.GA6969-52ZyvxujusuELgA04lAiVw@public.gmane.org> @ 2007-08-30 12:05 ` Johannes Berg 2007-08-30 14:49 ` Stephen Hemminger 0 siblings, 1 reply; 9+ messages in thread From: Johannes Berg @ 2007-08-30 12:05 UTC (permalink / raw) To: Jochen Voss Cc: linux wireless list, linville-2XuSBdqkA4R54TAoqtyWWQ, netdev, shemminger-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b [-- Attachment #1: Type: text/plain, Size: 785 bytes --] Hi Jochen, [added CCs since it affects bridge code] > If I read this correctly, the EIP in the last line corresponds to > net/bridge/br_if.c, line 36: > > static int port_cost(struct net_device *dev) > { > if (dev->ethtool_ops->get_settings) { > ^^^^ > > As far as I can figure out, dev->ethtool_ops is NULL and the crash > happens while trying to derefernce ...->get_settings. > > Is dev->ethtool_ops allowed to be NULL? In this case the appended > patch might be the correct fix. At least it makes the oops disappear > for me. Another possible fix would be to add an ethtool_ops structure > to the device created by b43. I don't think adding ethtool_ops in mac80211 should be necessary. Stephen? johannes [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 190 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: oops with recent wireless-dev tree 2007-08-30 12:05 ` oops with recent wireless-dev tree Johannes Berg @ 2007-08-30 14:49 ` Stephen Hemminger [not found] ` <20070830074949.7cd25b04-s08KbqtN0aBORcJjwVk88+YtYHrY8QCHmKZK+fsXvFM@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: Stephen Hemminger @ 2007-08-30 14:49 UTC (permalink / raw) To: Johannes Berg Cc: bridge, Matthew Wilcox, netdev, Jochen Voss, linux wireless list On Thu, 30 Aug 2007 14:05:30 +0200 Johannes Berg <johannes@sipsolutions.net> wrote: > Hi Jochen, > > [added CCs since it affects bridge code] > > > If I read this correctly, the EIP in the last line corresponds to > > net/bridge/br_if.c, line 36: > > > > static int port_cost(struct net_device *dev) > > { > > if (dev->ethtool_ops->get_settings) { > > ^^^^ > > > > As far as I can figure out, dev->ethtool_ops is NULL and the crash > > happens while trying to derefernce ...->get_settings. > > > > Is dev->ethtool_ops allowed to be NULL? In this case the appended > > patch might be the correct fix. At least it makes the oops disappear > > for me. Another possible fix would be to add an ethtool_ops structure > > to the device created by b43. > > I don't think adding ethtool_ops in mac80211 should be necessary. > Stephen? Devices aren't required to have ethtool_ops. The code there used to call ethtool directly, and it would handle the error cases. I'll rollup a fix this morning. The bug was introduced by this: commit 61a44b9c4b20d40c41fd1b70a4ceb13b75ea79a4 Author: Matthew Wilcox <matthew@wil.cx> Date: Tue Jul 31 14:00:02 2007 -0700 [NET]: ethtool ops are the only way During the transition to the ethtool_ops way of doing things, we supported calling the device's ->do_ioctl method to allow unconverted drivers to continue working. Those days are long behind us, all in-tree drivers use the ethtool_ops way, and so we no longer need to support this. The bonding driver is the biggest beneficiary of this; it no longer needs to call ioctl() as a fallback if ethtool_ops aren't supported. Also put a proper copyright statement on ethtool.c. Signed-off-by: Matthew Wilcox <matthew@wil.cx> Signed-off-by: David S. Miller <davem@davemloft.net> -- Stephen Hemminger <shemminger@linux-foundation.org> ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <20070830074949.7cd25b04-s08KbqtN0aBORcJjwVk88+YtYHrY8QCHmKZK+fsXvFM@public.gmane.org>]
* Re: oops with recent wireless-dev tree [not found] ` <20070830074949.7cd25b04-s08KbqtN0aBORcJjwVk88+YtYHrY8QCHmKZK+fsXvFM@public.gmane.org> @ 2007-08-30 14:58 ` Matthew Wilcox [not found] ` <20070830145840.GU14130-6jwH94ZQLHl74goWV3ctuw@public.gmane.org> 2007-08-30 14:58 ` oops with recent wireless-dev tree Johannes Berg 2007-08-30 15:01 ` Johannes Berg 2 siblings, 1 reply; 9+ messages in thread From: Matthew Wilcox @ 2007-08-30 14:58 UTC (permalink / raw) To: Stephen Hemminger Cc: Johannes Berg, Jochen Voss, linux wireless list, linville-2XuSBdqkA4R54TAoqtyWWQ, netdev, bridge-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b On Thu, Aug 30, 2007 at 07:49:49AM -0700, Stephen Hemminger wrote: > > > static int port_cost(struct net_device *dev) > > > { > > > if (dev->ethtool_ops->get_settings) { > > > ^^^^ > > > > > > As far as I can figure out, dev->ethtool_ops is NULL and the crash > > > happens while trying to derefernce ...->get_settings. > > Devices aren't required to have ethtool_ops. The code there used to > call ethtool directly, and it would handle the error cases. I'll rollup > a fix this morning. Yep, clearly my fault; that should read: if (dev->ethtool_ops && dev->ethtool_ops->get_settings) { Since Stephen's already bagged fixing this, I shan't send a patch. But if it includes something like the line above please add: Acked-by: Matthew Wilcox <matthew-Ztpu424NOJ8@public.gmane.org> -- Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step." ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <20070830145840.GU14130-6jwH94ZQLHl74goWV3ctuw@public.gmane.org>]
* [PATCH] bridge: fix OOPS when bridging device without ethtool [not found] ` <20070830145840.GU14130-6jwH94ZQLHl74goWV3ctuw@public.gmane.org> @ 2007-08-30 15:29 ` Stephen Hemminger 2007-08-30 16:48 ` Matthew Wilcox 0 siblings, 1 reply; 9+ messages in thread From: Stephen Hemminger @ 2007-08-30 15:29 UTC (permalink / raw) To: David S. Miller Cc: Matthew Wilcox, Johannes Berg, Jochen Voss, linux wireless list, linville-2XuSBdqkA4R54TAoqtyWWQ, netdev, bridge-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b Bridge code calls ethtool to get speed. The conversion to using only ethtool_ops broke the case of devices without ethtool_ops. This is a new regression in 2.6.23. Rearranged the switch to a logical order, and use gcc initializer. Ps: speed should have been part of the network device structure from the start rather than burying it in ethtool. Signed-off-by: Stephen Hemminger <shemminger-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> --- a/net/bridge/br_if.c 2007-08-30 07:49:01.000000000 -0700 +++ b/net/bridge/br_if.c 2007-08-30 07:48:16.000000000 -0700 @@ -33,17 +33,17 @@ */ static int port_cost(struct net_device *dev) { - if (dev->ethtool_ops->get_settings) { - struct ethtool_cmd ecmd = { ETHTOOL_GSET }; - int err = dev->ethtool_ops->get_settings(dev, &ecmd); - if (!err) { + if (dev->ethtool_ops && dev->ethtool_ops->get_settings) { + struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET, }; + + if (!dev->ethtool_ops->get_settings(dev, &ecmd)) { switch(ecmd.speed) { - case SPEED_100: - return 19; - case SPEED_1000: - return 4; case SPEED_10000: return 2; + case SPEED_1000: + return 4; + case SPEED_100: + return 19; case SPEED_10: return 100; } ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] bridge: fix OOPS when bridging device without ethtool 2007-08-30 15:29 ` [PATCH] bridge: fix OOPS when bridging device without ethtool Stephen Hemminger @ 2007-08-30 16:48 ` Matthew Wilcox 2007-08-31 5:16 ` David Miller 0 siblings, 1 reply; 9+ messages in thread From: Matthew Wilcox @ 2007-08-30 16:48 UTC (permalink / raw) To: Stephen Hemminger Cc: David S. Miller, Johannes Berg, Jochen Voss, linux wireless list, linville, netdev, bridge On Thu, Aug 30, 2007 at 08:29:32AM -0700, Stephen Hemminger wrote: > Bridge code calls ethtool to get speed. The conversion to using > only ethtool_ops broke the case of devices without ethtool_ops. > This is a new regression in 2.6.23. > > Rearranged the switch to a logical order, and use gcc initializer. > > Ps: speed should have been part of the network device structure from > the start rather than burying it in ethtool. Feel free to do the conversion ;-) One of the things I like about the ethtool framework is it gives us a way to take stuff out of the drivers and put it in the midlayer without disturbing userspace. > Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org> Acked-by: Matthew Wilcox <matthew@wil.cx> -- Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step." ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] bridge: fix OOPS when bridging device without ethtool 2007-08-30 16:48 ` Matthew Wilcox @ 2007-08-31 5:16 ` David Miller 0 siblings, 0 replies; 9+ messages in thread From: David Miller @ 2007-08-31 5:16 UTC (permalink / raw) To: matthew Cc: shemminger, johannes, voss, linux-wireless, linville, netdev, bridge From: Matthew Wilcox <matthew@wil.cx> Date: Thu, 30 Aug 2007 10:48:13 -0600 > On Thu, Aug 30, 2007 at 08:29:32AM -0700, Stephen Hemminger wrote: > > Bridge code calls ethtool to get speed. The conversion to using > > only ethtool_ops broke the case of devices without ethtool_ops. > > This is a new regression in 2.6.23. > > > > Rearranged the switch to a logical order, and use gcc initializer. > > > > Ps: speed should have been part of the network device structure from > > the start rather than burying it in ethtool. > > Feel free to do the conversion ;-) One of the things I like about the > ethtool framework is it gives us a way to take stuff out of the drivers > and put it in the midlayer without disturbing userspace. > > > Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org> > > Acked-by: Matthew Wilcox <matthew@wil.cx> Applied, thanks! ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: oops with recent wireless-dev tree [not found] ` <20070830074949.7cd25b04-s08KbqtN0aBORcJjwVk88+YtYHrY8QCHmKZK+fsXvFM@public.gmane.org> 2007-08-30 14:58 ` Matthew Wilcox @ 2007-08-30 14:58 ` Johannes Berg 2007-08-30 15:01 ` Johannes Berg 2 siblings, 0 replies; 9+ messages in thread From: Johannes Berg @ 2007-08-30 14:58 UTC (permalink / raw) To: Stephen Hemminger Cc: Jochen Voss, linux wireless list, linville-2XuSBdqkA4R54TAoqtyWWQ, netdev, Matthew Wilcox, bridge-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b [-- Attachment #1: Type: text/plain, Size: 262 bytes --] On Thu, 2007-08-30 at 07:49 -0700, Stephen Hemminger wrote: > Devices aren't required to have ethtool_ops. The code there used to > call ethtool directly, and it would handle the error cases. I'll rollup > a fix this morning. Great, thanks. johannes [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 190 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: oops with recent wireless-dev tree [not found] ` <20070830074949.7cd25b04-s08KbqtN0aBORcJjwVk88+YtYHrY8QCHmKZK+fsXvFM@public.gmane.org> 2007-08-30 14:58 ` Matthew Wilcox 2007-08-30 14:58 ` oops with recent wireless-dev tree Johannes Berg @ 2007-08-30 15:01 ` Johannes Berg [not found] ` <1188486091.3978.32.camel-YfaajirXv214zXjbi5bjpg@public.gmane.org> 2 siblings, 1 reply; 9+ messages in thread From: Johannes Berg @ 2007-08-30 15:01 UTC (permalink / raw) To: Stephen Hemminger Cc: Jochen Voss, linux wireless list, linville-2XuSBdqkA4R54TAoqtyWWQ, netdev, Matthew Wilcox, bridge-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b [-- Attachment #1: Type: text/plain, Size: 342 bytes --] On Thu, 2007-08-30 at 07:49 -0700, Stephen Hemminger wrote: > Devices aren't required to have ethtool_ops. The code there used to > call ethtool directly, and it would handle the error cases. I'll rollup > a fix this morning. Great, thanks. Jochen had a patch: http://marc.info/?l=linux-wireless&m=118842715026614&w=2 johannes [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 190 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <1188486091.3978.32.camel-YfaajirXv214zXjbi5bjpg@public.gmane.org>]
* Re: oops with recent wireless-dev tree [not found] ` <1188486091.3978.32.camel-YfaajirXv214zXjbi5bjpg@public.gmane.org> @ 2007-08-30 15:45 ` Matthew Wilcox 0 siblings, 0 replies; 9+ messages in thread From: Matthew Wilcox @ 2007-08-30 15:45 UTC (permalink / raw) To: Johannes Berg Cc: Stephen Hemminger, Jochen Voss, linux wireless list, linville-2XuSBdqkA4R54TAoqtyWWQ, netdev, bridge-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b On Thu, Aug 30, 2007 at 05:01:31PM +0200, Johannes Berg wrote: > Jochen had a patch: http://marc.info/?l=linux-wireless&m=118842715026614&w=2 That's exactly the right patch, please add Acked-by: Matthew Wilcox <matthew-Ztpu424NOJ8@public.gmane.org> I just checked over the commit that introduced the bug, and I didn't make the same mistake anywhere else. -- Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step." ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2007-08-31 5:16 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20070829223752.GA6969@seehuhn.de>
[not found] ` <20070829223752.GA6969-52ZyvxujusuELgA04lAiVw@public.gmane.org>
2007-08-30 12:05 ` oops with recent wireless-dev tree Johannes Berg
2007-08-30 14:49 ` Stephen Hemminger
[not found] ` <20070830074949.7cd25b04-s08KbqtN0aBORcJjwVk88+YtYHrY8QCHmKZK+fsXvFM@public.gmane.org>
2007-08-30 14:58 ` Matthew Wilcox
[not found] ` <20070830145840.GU14130-6jwH94ZQLHl74goWV3ctuw@public.gmane.org>
2007-08-30 15:29 ` [PATCH] bridge: fix OOPS when bridging device without ethtool Stephen Hemminger
2007-08-30 16:48 ` Matthew Wilcox
2007-08-31 5:16 ` David Miller
2007-08-30 14:58 ` oops with recent wireless-dev tree Johannes Berg
2007-08-30 15:01 ` Johannes Berg
[not found] ` <1188486091.3978.32.camel-YfaajirXv214zXjbi5bjpg@public.gmane.org>
2007-08-30 15:45 ` Matthew Wilcox
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).