* [PATCH 0/11] convert d80211 to a proper protocol @ 2006-11-05 15:39 Johannes Berg 2006-11-05 16:54 ` Ivo van Doorn 2006-11-06 20:01 ` Jiri Benc 0 siblings, 2 replies; 16+ messages in thread From: Johannes Berg @ 2006-11-05 15:39 UTC (permalink / raw) To: netdev Cc: John W. Linville, Jiri Benc, Jouni Malinen, Simon Barber, Hong Liu, Ivo van Doorn, Michael Wu, Michael Buesch, David Kimdon, James Ketrenos [-- Attachment #1: Type: text/plain, Size: 2476 bytes --] Hi, I figured I'd look how hard it actually was to convert d80211 to a proper protocol. And contrary to my own expectations I succeeded in doing that in just over a day. Just like 8021q, it has virtual devices. The patchset is huge and can be found at http://johannes.sipsolutions.net/files/d80211-proto/ John, please apply patches 001 and 002 out of the patchset as soon as you can -- they fix issues with cfg80211 that came up on IRC and in private mail to me. Very important, and not related to converting it to a protocol. Each patch comes with a subject and description but since they're large let me discuss some bits here. 001-cfg80211-fix-Makefile.patch cfg80211: fix Makefile brokenness 002-cfg80211-wext-compat.patch cfg80211: fix WE compat code Absolutely required to fix two issues with cfg80211 that make the current tree usable only in the strange configuration WE_NETLINK on and CFG80211 off. 003-d80211-cookie.patch d80211: change the cookie to be opaque This changes the 'cookie' that d80211 returns from alloc_hw to be an opaque value to the driver. Turned out that it wasn't such a great idea but since it was generally a clean up I kept this patch to base my other patches on. 004-bcm43xx-adjust.patch: bcm43xx: adjust for the d80211 changes This adjusts bcm43xx for above changes with the cookie pointer. 005-d80211-reduce-mdev-1.patch 006-d80211-reduce-mdev-2.patch d80211: reduce mdev usage These two patches reduce mdev madness and change a lot of functions to take a struct ieee80211_local * instead of the master netdev 007-d80211-cleanup-rxmgmt.patch d80211: reduce mdev usage, fix ieee80211_rx_mgmt Cleans up the ieee80211_rx_mgmt and related code 008-d80211-scan-sanity.patch d80211: reduce master ieee80211_ptr deref in scan routines Similar to the reduce mdev patches, just for the scan routines 009-d80211-convert-spaces.patch d80211: convert leading spaces to tabs I hated working on the code, so I did this. The next patch breaks everything anyway. 010-d80211-proto.patch d80211: convert to an 802.11 protocol Converts d80211 to be a protocol together with tons of cleanups and more. Hard to describe in two lines. 011-bcm43xx-adjust-proto.patch bcm43xx: adjust to register a real struct net_device makes bcm43xx function after the previous patch Have fun! johannes [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 828 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/11] convert d80211 to a proper protocol 2006-11-05 15:39 [PATCH 0/11] convert d80211 to a proper protocol Johannes Berg @ 2006-11-05 16:54 ` Ivo van Doorn 2006-11-05 17:05 ` Johannes Berg 2006-11-05 17:06 ` Johannes Berg 2006-11-06 20:01 ` Jiri Benc 1 sibling, 2 replies; 16+ messages in thread From: Ivo van Doorn @ 2006-11-05 16:54 UTC (permalink / raw) To: Johannes Berg Cc: netdev, John W. Linville, Jiri Benc, Jouni Malinen, Simon Barber, Hong Liu, Michael Wu, Michael Buesch, David Kimdon, James Ketrenos Hi, > I figured I'd look how hard it actually was to convert d80211 to a > proper protocol. And contrary to my own expectations I succeeded in > doing that in just over a day. Just like 8021q, it has virtual devices. > > The patchset is huge and can be found at > http://johannes.sipsolutions.net/files/d80211-proto/ I have a question regarding the enabling and disabling the radio in this new layout. Previously the open() and stop() methods in ieee80211_hw had been deprecated and the driver should enable or disable the radio based on the add_interface and remove_interface() functions. Now that the driver should provide open() and stop() for the netdevice structure does this mean that these 2 methods are back in control for enabling and disabling the radio? And if so what should the add_interface and remove_interface, should they be only in charge of triggering some configuration changes (like mode and mac address)? Ivo ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/11] convert d80211 to a proper protocol 2006-11-05 16:54 ` Ivo van Doorn @ 2006-11-05 17:05 ` Johannes Berg 2006-11-05 17:09 ` Johannes Berg 2006-11-05 17:06 ` Johannes Berg 1 sibling, 1 reply; 16+ messages in thread From: Johannes Berg @ 2006-11-05 17:05 UTC (permalink / raw) To: Ivo van Doorn Cc: netdev, John W. Linville, Jiri Benc, Jouni Malinen, Simon Barber, Hong Liu, Michael Wu, Michael Buesch, David Kimdon, James Ketrenos [-- Attachment #1: Type: text/plain, Size: 1035 bytes --] Hi, > Previously the open() and stop() methods in ieee80211_hw had been > deprecated and the driver should enable or disable the radio > based on the add_interface and remove_interface() functions. > Now that the driver should provide open() and stop() for the netdevice > structure does this mean that these 2 methods are back in control > for enabling and disabling the radio? Yeah, it'll have to, otherwise the device it created itself will be totally useless unless some other device is also open. > And if so what should the > add_interface and remove_interface, should they be only in charge of > triggering some configuration changes (like mode and mac address)? Mostly notably it's probably going to be used for the drivers to have veto powers, if they can't handle some specific combinations etc. Though for 802.11h the radio does need to be turned off in some cases regardless of the up/down state, so we'll have to think about that. Or we just force all interfaces down? Not sure yet. johannes [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 828 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/11] convert d80211 to a proper protocol 2006-11-05 17:05 ` Johannes Berg @ 2006-11-05 17:09 ` Johannes Berg 0 siblings, 0 replies; 16+ messages in thread From: Johannes Berg @ 2006-11-05 17:09 UTC (permalink / raw) To: Ivo van Doorn Cc: netdev, John W. Linville, Jiri Benc, Jouni Malinen, Simon Barber, Hong Liu, Michael Wu, Michael Buesch, David Kimdon, James Ketrenos [-- Attachment #1: Type: text/plain, Size: 197 bytes --] On Sun, 2006-11-05 at 18:05 +0100, Johannes Berg wrote: hey, I thought I caught this one before it went out to add the last paragraph :/ sorry. the other one is the intended copy. johannes [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 828 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/11] convert d80211 to a proper protocol 2006-11-05 16:54 ` Ivo van Doorn 2006-11-05 17:05 ` Johannes Berg @ 2006-11-05 17:06 ` Johannes Berg 1 sibling, 0 replies; 16+ messages in thread From: Johannes Berg @ 2006-11-05 17:06 UTC (permalink / raw) To: Ivo van Doorn Cc: netdev, John W. Linville, Jiri Benc, Jouni Malinen, Simon Barber, Hong Liu, Michael Wu, Michael Buesch, David Kimdon, James Ketrenos [-- Attachment #1: Type: text/plain, Size: 1265 bytes --] Hi, > Previously the open() and stop() methods in ieee80211_hw had been > deprecated and the driver should enable or disable the radio > based on the add_interface and remove_interface() functions. > Now that the driver should provide open() and stop() for the netdevice > structure does this mean that these 2 methods are back in control > for enabling and disabling the radio? Yeah, it'll have to, otherwise the device it created itself will be totally useless unless some other device is also open. > And if so what should the > add_interface and remove_interface, should they be only in charge of > triggering some configuration changes (like mode and mac address)? Mostly notably it's probably going to be used for the drivers to have veto powers, if they can't handle some specific combinations etc. Though for 802.11h the radio does need to be turned off in some cases regardless of the up/down state, so we'll have to think about that. Or we just force all interfaces down? Not sure yet. I confident we can work out these issues if we agree that converting it to a protocol is a good thing. After doing it, I'm personally pretty convinced that it is, some things just fell into place nicely and feel less hacky. johannes [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 828 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/11] convert d80211 to a proper protocol 2006-11-05 15:39 [PATCH 0/11] convert d80211 to a proper protocol Johannes Berg 2006-11-05 16:54 ` Ivo van Doorn @ 2006-11-06 20:01 ` Jiri Benc 2006-11-06 21:09 ` Johannes Berg ` (2 more replies) 1 sibling, 3 replies; 16+ messages in thread From: Jiri Benc @ 2006-11-06 20:01 UTC (permalink / raw) To: Johannes Berg Cc: netdev, John W. Linville, Jouni Malinen, Simon Barber, Hong Liu, Ivo van Doorn, Michael Wu, Michael Buesch, David Kimdon, James Ketrenos On Sun, 05 Nov 2006 16:39:34 +0100, Johannes Berg wrote: > 003-d80211-cookie.patch > d80211: change the cookie to be opaque > > This changes the 'cookie' that d80211 returns from alloc_hw > to be an opaque value to the driver. Turned out that it wasn't > such a great idea but since it was generally a clean up I kept > this patch to base my other patches on. ACK. > 005-d80211-reduce-mdev-1.patch > 006-d80211-reduce-mdev-2.patch > d80211: reduce mdev usage > > These two patches reduce mdev madness and change a lot of functions > to take a struct ieee80211_local * instead of the master netdev ACK. > 007-d80211-cleanup-rxmgmt.patch > d80211: reduce mdev usage, fix ieee80211_rx_mgmt > > Cleans up the ieee80211_rx_mgmt and related code Looks good after a quick look. Need to review it more deeply. > 008-d80211-scan-sanity.patch > d80211: reduce master ieee80211_ptr deref in scan routines > > Similar to the reduce mdev patches, just for the scan routines ACK. > 009-d80211-convert-spaces.patch > d80211: convert leading spaces to tabs > > I hated working on the code, so I did this. The next patch > breaks everything anyway. NAK. There are too many patches pending. Let's do this just before merging. > 010-d80211-proto.patch > d80211: convert to an 802.11 protocol > > Converts d80211 to be a protocol together with tons of > cleanups and more. Hard to describe in two lines. NAK. This is too big patch for a review, it does too much things and I fundamentally disagree with some parts of the patch. Split it into individual patches. Just some things which are broken with the patch (the list is probably not complete): > * The mdev no longer has a sub_if_data attached (why ever did it??) > It's private area is for the driver since we don't create it but > the driver does. I did keep the notation of mdev/master all through, > but it's no longer the stacks device. Keep that in mind. This definitely breaks AP mode. In the code, there is heavily (ab)used the fact that the master device is in fact an AP device. I tried to fix that but it was so difficult I gave up. It is needed to rewrite the whole RX path (and even that is probably not enough). As this will be fixed for free when we have native 802.11 devices, I don't think we need to do anything about it now. > * sysfs layout changed. There is no wiphy or an ieee80211 class any more, > the attributes that used to be there are now in the net_device that > the driver registered, and our attributes are below the devices we created. You want an ieee80211 class. Once you get rid of a master interface you need something with per-hardware information, statistics etc. > * sysfs layout changed. There is no wiphy or an ieee80211 class any more, > the attributes that used to be there are now in the net_device that > the driver registered, and our attributes are below the devices we created. Doesn't belong to this patch. > And probably lots more. ??? What did happen with d80211: add a function to get the wiphy index d80211: add a perm_addr hardware property d80211: add a struct device* hardware property d80211: add a ethtool_ops hardware property patches? Thanks, Jiri -- Jiri Benc SUSE Labs ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/11] convert d80211 to a proper protocol 2006-11-06 20:01 ` Jiri Benc @ 2006-11-06 21:09 ` Johannes Berg 2006-11-08 11:58 ` Jiri Benc 2006-11-06 23:06 ` Johannes Berg 2006-11-07 18:52 ` Johannes Berg 2 siblings, 1 reply; 16+ messages in thread From: Johannes Berg @ 2006-11-06 21:09 UTC (permalink / raw) To: Jiri Benc Cc: netdev, John W. Linville, Jouni Malinen, Simon Barber, Hong Liu, Ivo van Doorn, Michael Wu, Michael Buesch, David Kimdon, James Ketrenos [-- Attachment #1: Type: text/plain, Size: 3354 bytes --] [reordering a bit] > > This changes the 'cookie' that d80211 returns from alloc_hw > > to be an opaque value to the driver. Turned out that it wasn't > > such a great idea but since it was generally a clean up I kept > > this patch to base my other patches on. > > ACK. > What did happen with > d80211: add a function to get the wiphy index > d80211: add a perm_addr hardware property > d80211: add a struct device* hardware property > d80211: add a ethtool_ops hardware property > patches? Well after some chat with a few people I decided that it was stupid and not very maintainable to copy all the fields in net_device to a new structure. > > 009-d80211-convert-spaces.patch > > d80211: convert leading spaces to tabs > > > > I hated working on the code, so I did this. The next patch > > breaks everything anyway. > > NAK. There are too many patches pending. Let's do this just before > merging. Oh come off it! It's really stupid to have to check all the tabs/spaces all the time. The patch changes 451 lines. And wiggle can handle that just fine. Besides, if you do s/^\+ /+\t/ s/^- /-\t/ s/ /\t/ on your patches, they'll be fine too. > This is too big patch for a review, Yeah. It's pretty bad actually, but I couldn't really find a good way to split it into logical chunks. > > * The mdev no longer has a sub_if_data attached (why ever did it??) > > It's private area is for the driver since we don't create it but > > the driver does. I did keep the notation of mdev/master all through, > > but it's no longer the stacks device. Keep that in mind. > > This definitely breaks AP mode. In the code, there is heavily (ab)used > the fact that the master device is in fact an AP device. I tried to fix > that but it was so difficult I gave up. It is needed to rewrite the > whole RX path (and even that is probably not enough). Bugger. I didn't notice that. I'll have a look. That is indeed a showstopper. > As this will > be fixed for free when we have native 802.11 devices, I don't think we > need to do anything about it now. I don't think I understand this. I mean, my patch actually gives us native 802.11 devices by making the drivers register those and then handling them virtually similar to how 8021q handles ethernet devices. I honestly thought that this was the plan for said "native 802.11 devices". > > * sysfs layout changed. There is no wiphy or an ieee80211 class any more, > > the attributes that used to be there are now in the net_device that > > the driver registered, and our attributes are below the devices we created. > > You want an ieee80211 class. Once you get rid of a master interface you > need something with per-hardware information, statistics etc. Yeah, I gave up trying to get rid of the master interface in favour of having a native 802.11 device which is registered by the phy driver instead. > > * sysfs layout changed. There is no wiphy or an ieee80211 class any more, > > the attributes that used to be there are now in the net_device that > > the driver registered, and our attributes are below the devices we created. > > Doesn't belong to this patch. Had to be here initially due to the way I did things, but ok, probably changeable. johannes [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 828 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/11] convert d80211 to a proper protocol 2006-11-06 21:09 ` Johannes Berg @ 2006-11-08 11:58 ` Jiri Benc 2006-11-08 12:33 ` Johannes Berg 2006-11-08 13:36 ` Jeff Garzik 0 siblings, 2 replies; 16+ messages in thread From: Jiri Benc @ 2006-11-08 11:58 UTC (permalink / raw) To: Johannes Berg Cc: netdev, John W. Linville, Jouni Malinen, Simon Barber, Hong Liu, Ivo van Doorn, Michael Wu, Michael Buesch, David Kimdon, James Ketrenos On Mon, 06 Nov 2006 22:09:54 +0100, Johannes Berg wrote: > > What did happen with > > d80211: add a function to get the wiphy index > > d80211: add a perm_addr hardware property > > d80211: add a struct device* hardware property > > d80211: add a ethtool_ops hardware property > > patches? > > Well after some chat with a few people I decided that it was stupid and > not very maintainable to copy all the fields in net_device to a new > structure. Ok. Personally, I don't care if we pass net_device or ieee80211_local to drivers. I see pros and cons of both solutions. > Oh come off it! It's really stupid to have to check all the tabs/spaces > all the time. The patch changes 451 lines. And wiggle can handle that > just fine. Besides, if you do > s/^\+ /+\t/ > s/^- /-\t/ > s/ /\t/ > on your patches, they'll be fine too. I understand that. But the patch isn't small - it's 71 kiB and I'm concerned mainly about David's bitfields conversion patches. So, what about this: let's clean up those spaces (and perhaps do more coding style cleanups?) after David's patches are merged. Is it feasible for you? > Yeah. It's pretty bad actually, but I couldn't really find a good way to > split it into logical chunks. Unfortunately, in the current form it's unreviewable for me, Individual patches may be laid as follows (take it as a guideline only): - introduce IEEE80211_IF_TYPE_MASTER and convert mdev to be of this type - get rid of mdev's sdata (you'll probably find out that you need to preserve it at least in some form but let's see) - do whatever you feel appropriate with driver's private data (I'm not saying I'll be happy with any such solution, though) - do other changes of memory layout - remove ieee80211_dev.c and replace it by cfg80211 index - do some changes to the sysfs layout (again, I'm not promising to be happy with it) - introduce ieee80211_tx.c and ieee80211_rx.c files (I'll probably NAK such patch for the same reasons as the white space cleanup, though) - do .h files cleanup (I'd really like to see this) > I don't think I understand this. I mean, my patch actually gives us > native 802.11 devices by making the drivers register those and then > handling them virtually similar to how 8021q handles ethernet devices. I > honestly thought that this was the plan for said "native 802.11 > devices". What I understand as a "native 802.11 device": 1. it uses native 802.11 protocol, ie. no conversions from/to Ethernet frames 2. better qdiscs support (1) implies some interesting things: a. drivers can call netif_rx directly b. tx/rx path cleanup happens, this will lead to solving of some currently hardly solvable issues c. overall speedup of the frame processing (1) _doesn't_ particularly mean the following things: a. requiring drivers to register net_device by themselves (I'd like to preserve the concept of add_interface/remove_interface callbacks) b. setting hard_start_xmit of mdev by drivers instead of using ieee80211_hw->tx callback (but this isn't disallowed either) c. removing of master net_device (again, this is possible to happen) The main goal for me is to make things as easy as possible for driver developers. Currently, we're going the way of mixed layer/library approach and I really like it. I'm afraid that a pure library way would put unnecessary requirements to drivers. Also, please note that making a patch to use native 802.11 protocol is easy. Actually, I wrote it long time ago (it doesn't apply now and I'd need to rewrite it completly, but anyway). However, it's not possible to apply such patch now - the bridging needs to be solved first. > > > * sysfs layout changed. There is no wiphy or an ieee80211 class any more, > > > the attributes that used to be there are now in the net_device that > > > the driver registered, and our attributes are below the devices we created. > > > > Doesn't belong to this patch. > > Had to be here initially due to the way I did things, but ok, probably > changeable. Sorry, cut&paste error. My comment belonged to: > * memory layout. struct ieee80211_local is allocated by itself now. > > * saner source layout. rx/tx handlers in their own files, one header > for each C file that exports things. But it's valid for the sysfs part as well anyway :-) Thanks, Jiri -- Jiri Benc SUSE Labs ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/11] convert d80211 to a proper protocol 2006-11-08 11:58 ` Jiri Benc @ 2006-11-08 12:33 ` Johannes Berg 2006-11-08 13:36 ` Jeff Garzik 1 sibling, 0 replies; 16+ messages in thread From: Johannes Berg @ 2006-11-08 12:33 UTC (permalink / raw) To: Jiri Benc Cc: netdev, John W. Linville, Jouni Malinen, Simon Barber, Hong Liu, Ivo van Doorn, Michael Wu, Michael Buesch, David Kimdon, James Ketrenos On Wed, 2006-11-08 at 12:58 +0100, Jiri Benc wrote: > Ok. Personally, I don't care if we pass net_device or ieee80211_local > to drivers. I see pros and cons of both solutions. As I said in the other patchset, I'd prefer to not change the API for these cleanups unless we have to, so dropping this. > I understand that. But the patch isn't small - it's 71 kiB and I'm > concerned mainly about David's bitfields conversion patches. So, what > about this: let's clean up those spaces (and perhaps do more coding > style cleanups?) after David's patches are merged. Is it feasible for > you? Sure. > Unfortunately, in the current form it's unreviewable for me, I know that. Actually, I didn't really expect these to be applied. Just more of a proof-of-concept of how it would look like when converted to a protocol. And you've proven me wrong with the subif issue already :) > Individual patches may be laid as follows (take it as a guideline only): > - introduce IEEE80211_IF_TYPE_MASTER and convert mdev to be of this type > - get rid of mdev's sdata (you'll probably find out that you need to > preserve it at least in some form but let's see) Nah, I don't see a reason why that couldn't be completely in local. In fact, the only things we ought to be using the master device for is sending packets to the driver and attaching the qdisc. Hence, we can completely layer the virtual interfaces atop it instead of making it a peer of them in some ways. I plan to put a magic constant into sub_if_data as the first entry and assign some magic to it for all the non-master interfaces and make the DEV_TO_SUBIF macro BUG_ON() magic != correct. That'll help me get rid of the sdata ;) > - introduce ieee80211_tx.c and ieee80211_rx.c files (I'll probably NAK > such patch for the same reasons as the white space cleanup, though) I think it's a pain to work on the code if we don't do this. > - do .h files cleanup (I'd really like to see this) So far I only did the internal headers... I can redo that patch though. > What I understand as a "native 802.11 device": > 1. it uses native 802.11 protocol, ie. no conversions from/to Ethernet > frames > 2. better qdiscs support Right. That'd be a native 802.11 device. But you haven't specified where you want it, and that's the key issue. For starters, I want it on the master device, sort of like we have now except that it's pretty useless. But you also want the virtual devices to be native 802.11 devices. These two, however, are completely orthogonal! > (1) implies some interesting things: > a. drivers can call netif_rx directly > b. tx/rx path cleanup happens, this will lead to solving of some > currently hardly solvable issues > c. overall speedup of the frame processing Correct, and desperately needed. > (1) _doesn't_ particularly mean the following things: > a. requiring drivers to register net_device by themselves (I'd like to > preserve the concept of add_interface/remove_interface callbacks) Why? > b. setting hard_start_xmit of mdev by drivers instead of using > ieee80211_hw->tx callback (but this isn't disallowed either) Again, you can't at the same time allow removing the mdev and allow drivers to set the hard_start_xmit call :) > c. removing of master net_device (again, this is possible to happen) Yes, but only if you preserve the semantics of having the stack have full control over the hardware. I've come to think that this is wrong. > The main goal for me is to make things as easy as possible for driver > developers. Currently, we're going the way of mixed layer/library > approach and I really like it. I'm afraid that a pure library way would > put unnecessary requirements to drivers. Actually, I was thinking that it would be a lot easier for driver authors if they could just register their net_device along with some 802.11 help functions (ieee80211_hw) and otherwise behave just like a real net_dev. Here's my proposal in text-form instead of code, vaguely listed in order of the work to do: a) we clean up the rx path to get rid of master sub_if, and mostly of the master dev as well. b) instead of having __ieee80211_rx() we register a packet_type for 802.11 frames, and a netdevice notifier to see when new devices show up These are the basic items we have to do. We gain: * drivers must now register a proper net_device with 802.11 arphrd, and when they receive skbs they simply netif_rx them, we get rid of __ieee80211_rx * drivers just assign hard_start_xmit * drivers fully control their net_device so ethtool things make sense again etc. * we don't need the notion of a wiphy since like vlan, the sta/ap/wds/... devices are logically stacked on top of the driver's 802.11 device * we can get rid of virtual monitor interfaces (the driver's 802.11 device gets the frames anyway), we do need a few new config options to configure hard monitor mode for example. * 802.11 finally becomes a proper protocol instead of being a stack Hey, I think the last point is pretty good. :) johannes ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/11] convert d80211 to a proper protocol 2006-11-08 11:58 ` Jiri Benc 2006-11-08 12:33 ` Johannes Berg @ 2006-11-08 13:36 ` Jeff Garzik 1 sibling, 0 replies; 16+ messages in thread From: Jeff Garzik @ 2006-11-08 13:36 UTC (permalink / raw) To: Jiri Benc Cc: Johannes Berg, netdev, John W. Linville, Jouni Malinen, Simon Barber, Hong Liu, Ivo van Doorn, Michael Wu, Michael Buesch, David Kimdon, James Ketrenos Jiri Benc wrote: > On Mon, 06 Nov 2006 22:09:54 +0100, Johannes Berg wrote: >>> What did happen with >>> d80211: add a function to get the wiphy index >>> d80211: add a perm_addr hardware property >>> d80211: add a struct device* hardware property >>> d80211: add a ethtool_ops hardware property >>> patches? >> Well after some chat with a few people I decided that it was stupid and >> not very maintainable to copy all the fields in net_device to a new >> structure. > > Ok. Personally, I don't care if we pass net_device or ieee80211_local > to drivers. I see pros and cons of both solutions. Agreed. Though if using ieee80211_local, please remove the "_local". A better name is needed. Jeff ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/11] convert d80211 to a proper protocol 2006-11-06 20:01 ` Jiri Benc 2006-11-06 21:09 ` Johannes Berg @ 2006-11-06 23:06 ` Johannes Berg 2006-11-08 12:09 ` Jiri Benc 2006-11-07 18:52 ` Johannes Berg 2 siblings, 1 reply; 16+ messages in thread From: Johannes Berg @ 2006-11-06 23:06 UTC (permalink / raw) To: Jiri Benc Cc: netdev, John W. Linville, Jouni Malinen, Simon Barber, Hong Liu, Ivo van Doorn, Michael Wu, Michael Buesch, David Kimdon, James Ketrenos [-- Attachment #1: Type: text/plain, Size: 1597 bytes --] On Mon, 2006-11-06 at 21:01 +0100, Jiri Benc wrote: > This definitely breaks AP mode. In the code, there is heavily (ab)used > the fact that the master device is in fact an AP device. I tried to fix > that but it was so difficult I gave up. It is needed to rewrite the > whole RX path (and even that is probably not enough). Alright, I see the point. Let's fix it up good while at it. Here's a braindump. Keeping with the protocol theme, we need to rewrite the rx path to do the following in the order listed: 1) if any monitor interfaces are up, copy the skb, add the prism/radiotap/whatever header and then clone it to all monitor interfaces (don't actually copy for each as we do now) 2) if it is a unicast frame, determine which interface the frame should go to now, it can't be going to more than one afaik 2) decrypt the frame 3) defragment the frame 4) send the frame off to each sub_if that might care about it 5) do further device specific processing I need to think more about this though. If we're careful, we can probably get away with a lot less copying than we do now by pushing the copying down into those handlers that really need different data. This would probably only be the crypto handlers, the others can live with just pskb_copy instead. Then again, we don't handle non-linear skbs anyway. Actually, we should be calling skb_linearize at least, no? And if the frame is not encrypted at all, then we can even save the skb_copy for the monitor and just use skb_clone and later pskb_copy... Time for bed, johannes [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 828 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/11] convert d80211 to a proper protocol 2006-11-06 23:06 ` Johannes Berg @ 2006-11-08 12:09 ` Jiri Benc 2006-11-08 12:53 ` Johannes Berg 0 siblings, 1 reply; 16+ messages in thread From: Jiri Benc @ 2006-11-08 12:09 UTC (permalink / raw) To: Johannes Berg Cc: netdev, John W. Linville, Jouni Malinen, Simon Barber, Hong Liu, Ivo van Doorn, Michael Wu, Michael Buesch, David Kimdon, James Ketrenos On Tue, 07 Nov 2006 00:06:34 +0100, Johannes Berg wrote: > Keeping with the protocol theme, we need to rewrite the rx path to do > the following in the order listed: > > 1) if any monitor interfaces are up, copy the skb, add the > prism/radiotap/whatever header and then clone it to all monitor > interfaces (don't actually copy for each as we do now) > 2) if it is a unicast frame, determine which interface the frame should > go to now, it can't be going to more than one afaik > 2) decrypt the frame > 3) defragment the frame > 4) send the frame off to each sub_if that might care about it > 5) do further device specific processing Multicast and broadcast frames complicates this a bit but other than that you're right. See below. > If we're careful, we can probably get away with a lot less copying than > we do now by pushing the copying down into those handlers that really > need different data. Correct. > This would probably only be the crypto handlers, > the others can live with just pskb_copy instead. Unfortunately, in case of broadcast and multicast frames, we probably need to go through decryption handlers for each interface individually. At least I wasn't able to find a solution which allows to perform decryption only once for each frame. But theoretically it should be possible but I'm afraid it would be less effective than the current situation in the end. Unfortunately, when you perform decryption in each interface separately, you are tended to perform defragmentation separately as well. This is something that could be possibly improved. > Then again, we don't > handle non-linear skbs anyway. Actually, we should be calling > skb_linearize at least, no? > > And if the frame is not encrypted at all, then we can even save the > skb_copy for the monitor and just use skb_clone and later pskb_copy... Jiri -- Jiri Benc SUSE Labs ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/11] convert d80211 to a proper protocol 2006-11-08 12:09 ` Jiri Benc @ 2006-11-08 12:53 ` Johannes Berg 0 siblings, 0 replies; 16+ messages in thread From: Johannes Berg @ 2006-11-08 12:53 UTC (permalink / raw) To: Jiri Benc Cc: netdev, John W. Linville, Jouni Malinen, Simon Barber, Hong Liu, Ivo van Doorn, Michael Wu, Michael Buesch, David Kimdon, James Ketrenos On Wed, 2006-11-08 at 13:09 +0100, Jiri Benc wrote: > Unfortunately, in case of broadcast and multicast frames, we probably > need to go through decryption handlers for each interface individually. > At least I wasn't able to find a solution which allows to perform > decryption only once for each frame. But theoretically it should be > possible but I'm afraid it would be less effective than the current > situation in the end. Yeah, I can see the point, even though it seems that we can only decrypt a frame with a single key, everything else will be invalid. Maybe we can filter somehow and find which key to use instead of filtering to the interface first... will need to think about it. johannes ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/11] convert d80211 to a proper protocol 2006-11-06 20:01 ` Jiri Benc 2006-11-06 21:09 ` Johannes Berg 2006-11-06 23:06 ` Johannes Berg @ 2006-11-07 18:52 ` Johannes Berg 2006-11-07 19:02 ` Ivo van Doorn 2 siblings, 1 reply; 16+ messages in thread From: Johannes Berg @ 2006-11-07 18:52 UTC (permalink / raw) To: Jiri Benc Cc: netdev, John W. Linville, Jouni Malinen, Simon Barber, Hong Liu, Ivo van Doorn, Michael Wu, Michael Buesch, David Kimdon, James Ketrenos [-- Attachment #1: Type: text/plain, Size: 725 bytes --] I've put a new patchset up at http://johannes.sipsolutions.net/files/d80211-cleanup/ It now contains: 001-cfg80211-fix-Makefile.patch 002-cfg80211-wext-compat.patch as before. 003-d80211-reduce-mdev-1.patch 004-d80211-reduce-mdev-2.patch 005-d80211-cleanup-rxmgmt.patch 006-d80211-scan-sanity.patch similar to before, but modified to apply without the previous cookie patch. I decided to drop the cookie patch because it unnecessarily breaks drivers. We obviously haven't figured out what we want, so let's just go for the lowest common denominator. I think these are mostly cleanups and it all compiles fine after each one. No API changes. Haven't gotten around to testing it yet. johannes [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 828 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/11] convert d80211 to a proper protocol 2006-11-07 18:52 ` Johannes Berg @ 2006-11-07 19:02 ` Ivo van Doorn 2006-11-07 20:33 ` Johannes Berg 0 siblings, 1 reply; 16+ messages in thread From: Ivo van Doorn @ 2006-11-07 19:02 UTC (permalink / raw) To: Johannes Berg Cc: Jiri Benc, netdev, John W. Linville, Jouni Malinen, Simon Barber, Hong Liu, Michael Wu, Michael Buesch, David Kimdon, James Ketrenos Hi, > http://johannes.sipsolutions.net/files/d80211-cleanup/ You might want to fix the rights to the folder again ;) Ivo ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/11] convert d80211 to a proper protocol 2006-11-07 19:02 ` Ivo van Doorn @ 2006-11-07 20:33 ` Johannes Berg 0 siblings, 0 replies; 16+ messages in thread From: Johannes Berg @ 2006-11-07 20:33 UTC (permalink / raw) To: Ivo van Doorn Cc: Jiri Benc, netdev, John W. Linville, Jouni Malinen, Simon Barber, Hong Liu, Michael Wu, Michael Buesch, David Kimdon, James Ketrenos [-- Attachment #1: Type: text/plain, Size: 300 bytes --] On Tue, 2006-11-07 at 20:02 +0100, Ivo van Doorn wrote: > > http://johannes.sipsolutions.net/files/d80211-cleanup/ > > You might want to fix the rights to the folder again ;) Ahrg, sorry. I should make http://johannes.sipsolutions.net/patches/ and leave that open all the way. johannes [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 828 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2006-11-08 13:36 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-11-05 15:39 [PATCH 0/11] convert d80211 to a proper protocol Johannes Berg 2006-11-05 16:54 ` Ivo van Doorn 2006-11-05 17:05 ` Johannes Berg 2006-11-05 17:09 ` Johannes Berg 2006-11-05 17:06 ` Johannes Berg 2006-11-06 20:01 ` Jiri Benc 2006-11-06 21:09 ` Johannes Berg 2006-11-08 11:58 ` Jiri Benc 2006-11-08 12:33 ` Johannes Berg 2006-11-08 13:36 ` Jeff Garzik 2006-11-06 23:06 ` Johannes Berg 2006-11-08 12:09 ` Jiri Benc 2006-11-08 12:53 ` Johannes Berg 2006-11-07 18:52 ` Johannes Berg 2006-11-07 19:02 ` Ivo van Doorn 2006-11-07 20:33 ` Johannes Berg
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).