[PATCH 0/11] convert d80211 to a proper protocol

[PATCH 0/11] convert d80211 to a proper protocol

[Johannes Berg]

[-- 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 --]

Re: [PATCH 0/11] convert d80211 to a proper protocol

[Ivo van Doorn]

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

Re: [PATCH 0/11] convert d80211 to a proper protocol

[Johannes Berg]

[-- 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 --]

Re: [PATCH 0/11] convert d80211 to a proper protocol

[Johannes Berg]

[-- 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 --]

Re: [PATCH 0/11] convert d80211 to a proper protocol

[Johannes Berg]

[-- 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 --]

Re: [PATCH 0/11] convert d80211 to a proper protocol

[Jiri Benc]

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

Re: [PATCH 0/11] convert d80211 to a proper protocol

[Johannes Berg]

[-- 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 --]

Re: [PATCH 0/11] convert d80211 to a proper protocol

[Johannes Berg]

[-- 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 --]

Re: [PATCH 0/11] convert d80211 to a proper protocol

[Johannes Berg]

[-- 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 --]

Re: [PATCH 0/11] convert d80211 to a proper protocol

[Ivo van Doorn]

Hi,

> http://johannes.sipsolutions.net/files/d80211-cleanup/

You might want to fix the rights to the folder again ;)

Ivo

Re: [PATCH 0/11] convert d80211 to a proper protocol

[Johannes Berg]

[-- 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 --]

Re: [PATCH 0/11] convert d80211 to a proper protocol

[Jiri Benc]

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

Re: [PATCH 0/11] convert d80211 to a proper protocol

[Jiri Benc]

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

Re: [PATCH 0/11] convert d80211 to a proper protocol

[Johannes Berg]

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

Re: [PATCH 0/11] convert d80211 to a proper protocol

[Johannes Berg]

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

Re: [PATCH 0/11] convert d80211 to a proper protocol

[Jeff Garzik]

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