Re: [PATCH] Revert ipw2200: select CFG80211_WEXT
On Mon, 2015-01-05 at 23:13 +0100, Arend van Spriel wrote: On 01/05/15 23:05, Paul Bolle wrote: On Mon, 2015-01-05 at 19:57 +0100, Johannes Berg wrote: Multiple other groups of ioctls could be converted in similar patches, until at the end you can completely remove ipw_wx_handlers and rely entirely on cfg80211's wext compatibility. So far the theory - in practice nobody cared enough to start working on any of these drivers, let alone actually has the hardware today. So my suggestion to make ipw2200 no longer use cfg80211_wext_giwname() would actually be backwards. What's actually needed, in theory, is to use more of what's provided under CFG80211_WEXT (and, I guess, less of what's provided under WIRELESS_EXT). Did I get that right? Yes, but as Johannes indicated it needs consideration what to group in the patches. Oh, that's not strictly necessary - that was just so it would actually work after each single patch. If you wanted to, you could do it all in a single huge patch as well :-) I was explaining though why we did the cfg80211-wext code the way it is - which enables making smaller changes that don't break the driver. johannes -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Revert ipw2200: select CFG80211_WEXT
On Mon, 2015-01-05 at 23:05 +0100, Paul Bolle wrote: On Mon, 2015-01-05 at 19:57 +0100, Johannes Berg wrote: Multiple other groups of ioctls could be converted in similar patches, until at the end you can completely remove ipw_wx_handlers and rely entirely on cfg80211's wext compatibility. So far the theory - in practice nobody cared enough to start working on any of these drivers, let alone actually has the hardware today. So my suggestion to make ipw2200 no longer use cfg80211_wext_giwname() would actually be backwards. What's actually needed, in theory, is to use more of what's provided under CFG80211_WEXT (and, I guess, less of what's provided under WIRELESS_EXT). Did I get that right? Yes, though I'd argue there are multiple levels of truth here. Yours is the theoretical, hopefully-forward-looking one where we still expect the driver to actually be modified to take advantage of the new frameworks (which is independent of wext support towards userspace). In that scenario, yes, it should use more until it uses all, and then it can stop concerning itself with wext (which would be a win because driver/wext interaction is always finicky) (*) Then there's the other level that you were looking at earlier - simply removing all of this again from this driver because nobody is going to work on it. That'd actually make sense and shrink the driver footprint (no need to load cfg80211 for using almost nothing of it) but this is no longer feasible since it would again break userspace API - as I said you can today discover the presence of this device/driver with nl80211 - that would be broken by removing cfg80211 from here. And then there's the practical third version that's likely to happen - i.e. nothing :-) johannes (*) FWIW, if done to all drivers it would allow shrinking cfg80211-wext by a lot because all the EXPORT_SYMBOL would no longer be needed - but come to think of it we can fix that differently. -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Revert ipw2200: select CFG80211_WEXT
On Tue, 2015-01-06 at 09:23 +0100, Johannes Berg wrote: Yours is the theoretical, hopefully-forward-looking one where we still expect the driver to actually be modified to take advantage of the new frameworks (which is independent of wext support towards userspace). In that scenario, yes, it should use more until it uses all, and then it can stop concerning itself with wext (which would be a win because driver/wext interaction is always finicky) (*) I just realized that in order to actually do this you'd have to revert 04b0c5c6995103eef56391c163e970ab1f03b59f first, but that would be pretty simple. johannes -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Revert ipw2200: select CFG80211_WEXT
On Sat, 2015-01-03 at 23:28 +0100, Paul Bolle wrote: On Sat, 2015-01-03 at 10:07 -0800, Linus Torvalds wrote: On Sat, Jan 3, 2015 at 10:02 AM, Marcel Holtmann mar...@holtmann.org wrote: why would you revert this? It is obviously the correct change to actually select CFG80211_WEXT. I don't know about obvious, but yeah, I think the select in this case is actually the better idea anyway. Obviously it wasn't obvious to me! My reasoning was that the ipw2200: select CFG80211_WEXT commit was _solely_ a workaround for the breakage introduced by that other patch. And since that one is now reverted the workaround wasn't needed anymore. Well, you thought of it only as a workaround - but it makes sense. You shouldn't have to Besied, I thought we try to avoid select-ing symbols that can also be set manually. As that makes it more likely to trigger circular dependency problems in the kconfig tools, doesn't it? I don't think that has much to do with whether or not they can be set manually - it's more a question of what the dependencies of the selected symbol are. In this case it's a bool leaf symbol with the only dependency being something ipw already needs, so it's pretty much guaranteed to be safe. Side note: am I correct in thinking that there's some successor to CFG80211_WEXT and that the ipw2200 driver could, at least in theory, be ported to that successor? (ipw2200 hardware appears to be a bit old, so probably no one would care enough to actually do that.) net/wireless/kconfig doesn't mention anything like that, so probably I'm just confused. Well, see the big thread over there with the revert that I'm tempted to not even read ... The real successor, for various reasons (like simply always being able to connect to a single AP, no matter how many others can be found by scanning!), is nl80211. Many drivers have been converted to the new framework by using mac80211, but some drivers cannot use mac80211 because they do more in the device/firmware. Among those are many of the old 11b/11g ones that still exist in the tree, but also a few new ones (like mwifiex) that were already written to cfg80211 APIs. It should definitely be possible to convert all the drivers (*) to pure cfg80211 instead of having direct userspace wext to driver calls - but nobody has gone and done it. johannes (*) caveat - at least the playstation3 driver will require some new nl80211 API -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Revert ipw2200: select CFG80211_WEXT
On Mon, 5 Jan 2015, Johannes Berg wrote: Well, see the big thread over there with the revert that I'm tempted to not even read ... I'd actually like to hear from you whether you share Emmanuel's point of view that my revert of your patch was inappropriate; I was really surprised that there might be even questions about that, given the obvious userspace wreckage. Again, the only thing I really care for is the compatibility layer to userspace. I don't question at all the fact nl80211 provides much better functionality and its usage definitely should be encouraged. Thanks, -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Revert ipw2200: select CFG80211_WEXT
On Mon, 2015-01-05 at 11:12 +0100, Jiri Kosina wrote: On Mon, 5 Jan 2015, Johannes Berg wrote: Well, see the big thread over there with the revert that I'm tempted to not even read ... I'd actually like to hear from you whether you share Emmanuel's point of view that my revert of your patch was inappropriate; I was really surprised that there might be even questions about that, given the obvious userspace wreckage. Let's not pollute this thread with that though ... :-) I agree that the revert was the right thing to do, but from what I saw when I briefly glimpsed at part of the thread during my vacation (and then deciding I didn't want to deal with it then) a lot has been said in the thread that is completely wrong. I may not care enough to correct it though. johannes -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Revert ipw2200: select CFG80211_WEXT
On Mon, 2015-01-05 at 11:14 +0100, Arend van Spriel wrote: On 01/03/15 23:28, Paul Bolle wrote: Side note: am I correct in thinking that there's some successor to CFG80211_WEXT and that the ipw2200 driver could, at least in theory, be ported to that successor? (ipw2200 hardware appears to be a bit old, so probably no one would care enough to actually do that.) net/wireless/kconfig doesn't mention anything like that, so probably I'm just confused. ipw2200 is a WEXT driver using some wext functionality (and struct wiphy) provided by cfg80211 hence it needs CFG80211_WEXT. I guess that is what makes it confusing. It doesn't help that I hardly know anything about mac80211, cfg80211 and nl80211 (and lib80211 for that matter). To me these are mostly just names that end in 80211. Anyhow, concerning, CFG80211_WEXT: it seems the only functionality provided by that symbol that ipw2200 uses directly is cfg80211_wext_giwname(). Perhaps ipw2200 could have a private version of that function, something like ipw2100's ipw2100_wx_get_name(). Should be trivial to implement (ie, it could take _me_ a day or two). But perhaps ipw2200 uses CFG80211_WEXT _indirectly_ too. Ie, in net/wireless/core.c I stumbled on #ifdef CONFIG_CFG80211_WEXT rdev-wiphy.wext = cfg80211_wext_handler; #endif But I net/wireless/wext-core.c I then found #ifdef CONFIG_CFG80211_WEXT if (dev-ieee80211_ptr dev-ieee80211_ptr-wiphy) handlers = dev-ieee80211_ptr-wiphy-wext; #endif #ifdef CONFIG_WIRELESS_EXT if (dev-wireless_handlers) handlers = dev-wireless_handlers; #endif (There's much more to discover about WEXT, of course.) Anyhow, IPW2200 uses both CFG80211_WEXT and WIRELESS_EXT and cfg80211_wext_handler and ipw2200's wireless_handlers appear to cover the same set of IOCTLS (one exception: SIOCSIWPMKSA). So by now I'm really puzzled how this all fits together. Thanks, Paul Bolle -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Revert ipw2200: select CFG80211_WEXT
On Mon, 2015-01-05 at 19:57 +0100, Johannes Berg wrote: Multiple other groups of ioctls could be converted in similar patches, until at the end you can completely remove ipw_wx_handlers and rely entirely on cfg80211's wext compatibility. So far the theory - in practice nobody cared enough to start working on any of these drivers, let alone actually has the hardware today. So my suggestion to make ipw2200 no longer use cfg80211_wext_giwname() would actually be backwards. What's actually needed, in theory, is to use more of what's provided under CFG80211_WEXT (and, I guess, less of what's provided under WIRELESS_EXT). Did I get that right? Thanks, Paul Bolle -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Revert ipw2200: select CFG80211_WEXT
On 01/05/15 23:05, Paul Bolle wrote: On Mon, 2015-01-05 at 19:57 +0100, Johannes Berg wrote: Multiple other groups of ioctls could be converted in similar patches, until at the end you can completely remove ipw_wx_handlers and rely entirely on cfg80211's wext compatibility. So far the theory - in practice nobody cared enough to start working on any of these drivers, let alone actually has the hardware today. So my suggestion to make ipw2200 no longer use cfg80211_wext_giwname() would actually be backwards. What's actually needed, in theory, is to use more of what's provided under CFG80211_WEXT (and, I guess, less of what's provided under WIRELESS_EXT). Did I get that right? Yes, but as Johannes indicated it needs consideration what to group in the patches. Regards, Arend Thanks, Paul Bolle -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Revert ipw2200: select CFG80211_WEXT
On 01/05/15 18:38, Paul Bolle wrote: On Mon, 2015-01-05 at 11:14 +0100, Arend van Spriel wrote: On 01/03/15 23:28, Paul Bolle wrote: Side note: am I correct in thinking that there's some successor to CFG80211_WEXT and that the ipw2200 driver could, at least in theory, be ported to that successor? (ipw2200 hardware appears to be a bit old, so probably no one would care enough to actually do that.) net/wireless/kconfig doesn't mention anything like that, so probably I'm just confused. ipw2200 is a WEXT driver using some wext functionality (and struct wiphy) provided by cfg80211 hence it needs CFG80211_WEXT. I guess that is what makes it confusing. It doesn't help that I hardly know anything about mac80211, cfg80211 and nl80211 (and lib80211 for that matter). To me these are mostly just names that end in 80211. Grapjas ;-) cfg80211 provides thin-layer API for fullmac drivers (running 802.11 stack on the device) and mac80211-based drivers (running 802.11 stack in kernel). Anyhow, concerning, CFG80211_WEXT: it seems the only functionality provided by that symbol that ipw2200 uses directly is cfg80211_wext_giwname(). Perhaps ipw2200 could have a private version of that function, something like ipw2100's ipw2100_wx_get_name(). Should be trivial to implement (ie, it could take _me_ a day or two). Indeed or even an hour or two. But perhaps ipw2200 uses CFG80211_WEXT _indirectly_ too. Ie, in net/wireless/core.c I stumbled on #ifdef CONFIG_CFG80211_WEXT rdev-wiphy.wext =cfg80211_wext_handler; #endif This is the wext compatibility being enabled for any cfg80211 or mac80211 based driver. But I net/wireless/wext-core.c I then found #ifdef CONFIG_CFG80211_WEXT if (dev-ieee80211_ptr dev-ieee80211_ptr-wiphy) handlers = dev-ieee80211_ptr-wiphy-wext; #endif wext-core is the WEXT framework and here it extracts WEXT handlers from a cfg80211/mac80211-based driver that are store in wiphy structure. #ifdef CONFIG_WIRELESS_EXT if (dev-wireless_handlers) handlers = dev-wireless_handlers; #endif Here wext-core extracts WEXT handlers from a WEXT driver. struct net_device::wireless_handlers is only defined for CONFIG_WIRELESS_EXT. (There's much more to discover about WEXT, of course.) Anyhow, IPW2200 uses both CFG80211_WEXT and WIRELESS_EXT and cfg80211_wext_handler and ipw2200's wireless_handlers appear to cover the same set of IOCTLS (one exception: SIOCSIWPMKSA). So by now I'm really puzzled how this all fits together. I think ipw2200 is a bit of both worlds indeed adopting the use of struct wiphy and wiphy_register() call. That seems to suggest it is a cfg80211 driver, but it does not register any cfg80211 driver callbacks (see libipw_config_ops in libipw_module.c). So it overrides the WEXT ioctls because it needs that to interact with the device. Regards, Arend Thanks, Paul Bolle -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Revert ipw2200: select CFG80211_WEXT
On Mon, 2015-01-05 at 18:38 +0100, Paul Bolle wrote:
ipw2200 is a WEXT driver using some wext functionality (and struct
wiphy) provided by cfg80211 hence it needs CFG80211_WEXT. I guess that
is what makes it confusing.
It doesn't help that I hardly know anything about mac80211, cfg80211 and
nl80211 (and lib80211 for that matter). To me these are mostly just
names that end in 80211.
:-)
There isn't really all that much that ipw2x00 is using from cfg80211
though - of note is that it has completely empty ops:
static struct cfg80211_ops libipw_config_ops = { };
IOW - all it does is register with the framework - courtesy of
a3caa99e6c68f. It's practically useless.
Anyhow, concerning, CFG80211_WEXT: it seems the only functionality
provided by that symbol that ipw2200 uses directly is
cfg80211_wext_giwname(). Perhaps ipw2200 could have a private version of
that function, something like ipw2100's ipw2100_wx_get_name(). Should be
trivial to implement (ie, it could take _me_ a day or two).
We could just revert that part of the commit above - or even completely.
However, in theory at least doing *that* would now be a userspace
regression - today you can at least discover the presence of ipw2200
devices with nl80211, even if you can't do anything with them that way.
But perhaps ipw2200 uses CFG80211_WEXT _indirectly_ too. Ie, in
net/wireless/core.c I stumbled on
#ifdef CONFIG_CFG80211_WEXT
rdev-wiphy.wext = cfg80211_wext_handler;
#endif
I don't think it does - see the note about the ops above. If it did,
it'd have to implement the ops.
But I net/wireless/wext-core.c I then found
#ifdef CONFIG_CFG80211_WEXT
if (dev-ieee80211_ptr dev-ieee80211_ptr-wiphy)
handlers = dev-ieee80211_ptr-wiphy-wext;
#endif
#ifdef CONFIG_WIRELESS_EXT
if (dev-wireless_handlers)
handlers = dev-wireless_handlers;
#endif
(There's much more to discover about WEXT, of course.) Anyhow, IPW2200
uses both CFG80211_WEXT and WIRELESS_EXT and cfg80211_wext_handler and
ipw2200's wireless_handlers appear to cover the same set of IOCTLS (one
exception: SIOCSIWPMKSA). So by now I'm really puzzled how this all fits
together.
Well, this was meant as a transition mechanism for drivers. Ultimately,
the way we thought how you'd convert a driver (and how we converted
mac80211) would be to have the wext handlers like for example the scan
ones:
static iw_handler ipw_wx_handlers[] = {
...
IW_HANDLER(SIOCSIWSCAN, ipw_wx_set_scan),
IW_HANDLER(SIOCGIWSCAN, ipw_wx_get_scan),
...
};
Then you could make a patch that uses the cfg80211 APIs for scanning in
the driver -- i.e. implement the cfg80211_ops.scan method, report frames
to the cfg80211 scanning and remove all the ieee-network_list stuff
etc. using the related cfg80211 API (e.g. cfg80211_get_bss() and friends
for getting a network, etc.) And then you'd change the handlers to be
static iw_handler ipw_wx_handlers[] = {
...
IW_HANDLER(SIOCSIWSCAN, cfg80211_wext_siwscan),
IW_HANDLER(SIOCGIWSCAN, cfg80211_wext_giwscan),
...
};
This part would have to be done in a single patch.
Multiple other groups of ioctls could be converted in similar patches,
until at the end you can completely remove ipw_wx_handlers and rely
entirely on cfg80211's wext compatibility.
So far the theory - in practice nobody cared enough to start working on
any of these drivers, let alone actually has the hardware today.
johannes
--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Revert ipw2200: select CFG80211_WEXT
On Sat, 2015-01-03 at 10:07 -0800, Linus Torvalds wrote: On Sat, Jan 3, 2015 at 10:02 AM, Marcel Holtmann mar...@holtmann.org wrote: why would you revert this? It is obviously the correct change to actually select CFG80211_WEXT. I don't know about obvious, but yeah, I think the select in this case is actually the better idea anyway. Obviously it wasn't obvious to me! My reasoning was that the ipw2200: select CFG80211_WEXT commit was _solely_ a workaround for the breakage introduced by that other patch. And since that one is now reverted the workaround wasn't needed anymore. Besied, I thought we try to avoid select-ing symbols that can also be set manually. As that makes it more likely to trigger circular dependency problems in the kconfig tools, doesn't it? We could make the CFG80211_WEXT help message be very negative so that people aren't encouraged to select it even if they can, but then if they need the ipw driver it gets selected because of that. Because the ipw driver is probably the more important of the two if you just happen to have old hardware but are upgrading yout software (and anybody who recompiles their own kernel is obviously doing the latter). Side note: am I correct in thinking that there's some successor to CFG80211_WEXT and that the ipw2200 driver could, at least in theory, be ported to that successor? (ipw2200 hardware appears to be a bit old, so probably no one would care enough to actually do that.) net/wireless/kconfig doesn't mention anything like that, so probably I'm just confused. Paul Bolle -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Revert ipw2200: select CFG80211_WEXT
This reverts commit 60220f41775e634258efd1b54c6fa81ce706. The raison d'être of commit 60220f41 (ipw2200: select CFG80211_WEXT) was reverted in commit 2d36e008739e (Revert cfg80211: make WEXT compatibility unselectable). So revert this commit too. Signed-off-by: Paul Bolle pebo...@tiscali.nl --- drivers/net/wireless/ipw2x00/Kconfig | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/net/wireless/ipw2x00/Kconfig b/drivers/net/wireless/ipw2x00/Kconfig index 21de4fe6cf2d..91c0cb3c368e 100644 --- a/drivers/net/wireless/ipw2x00/Kconfig +++ b/drivers/net/wireless/ipw2x00/Kconfig @@ -65,8 +65,7 @@ config IPW2100_DEBUG config IPW2200 tristate Intel PRO/Wireless 2200BG and 2915ABG Network Connection - depends on PCI CFG80211 - select CFG80211_WEXT + depends on PCI CFG80211 CFG80211_WEXT select WIRELESS_EXT select WEXT_SPY select WEXT_PRIV -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
