questions regarding removing the depencency on CONFIG_IEEE80211

questions regarding removing the depencency on CONFIG_IEEE80211

[Holger Schurig]

Last friday I was asked on IRC about the dependency of libertas 
on CONFIG_IEEE80211. I gave a quick question, but today I had 
time to look into this further.


1. simply things

* remove "select IEEE80211" in wireless/KConfig
* remove "#include <net/ieee80211.h>"


2. MAX_WPA_IE_LEN

This was defined in net/ieee80211.h, but is not defined in 
linux/ieee80211.h. I have two options, which one should I go:

* define it locally in some libertas header file
* define it as IEEE80211_MAX_WPA_IE_LEN in linux/ieee80211.h


3. MAX_NETWORK_COUNT

libertas has a list of found BSSses during scanning. It keeps 
them in an kzalloc()'ed array. However, it doesn't use any of
ieee80211's code for BSS or associating, so I guess I make this a 
private define, e.g. LBS_MAX_BSS_COUNT.


4. DEFAULT_MAX_SCAN_AGE

Again, libertas doesn't use any aging code from ieee80211, so I 
guess I make this a libertas-private define.


5. MFIE_TYPE_xxxx enum

They are easily substituded with the proper WLAN_EID_xxx ones.


6. struct ieee80211_info_element

In scan.c, libertas uses heavily the "struct 
ieee80211_info_element". As this struct might be useful for 
other non-max80211 drivers, I'd suggest putting it in 
include/linux/ieee80211.h.


7. RADIOTAP

Libertas contains lot's of code for it's monitoring mode, which 
uses radiotap. It used "struct net_device *rtap_net_dev" 
and "struct ieee80211_device *ieee". It contains code like

   priv->rtap_net_dev = alloc_ieee80211(0);
   priv->ieee = netdev_priv(priv->rtap_net_dev);

and I don't have any knowledge about radiotap and how to 
substitue the alloc_ieee80211(). All I could currently to is to 
put those sections into #ifdef CONFIG_IEEE80211.

Re: questions regarding removing the depencency on CONFIG_IEEE80211

[Holger Schurig]

7. escape_ssid()

This code from ieee80211 is used in various debug messages.
Should I re-implement this locally in the driver?

Re: questions regarding removing the depencency on CONFIG_IEEE80211

[Dan Williams]

On Tue, 2007-12-04 at 11:53 +0100, Holger Schurig wrote:
> 7. escape_ssid()
> 
> This code from ieee80211 is used in various debug messages.
> Should I re-implement this locally in the driver?

Part of the _point_ of depending on ieee80211 was so that we wouldn't
have a lot of pointless code duplication.  Like this.

Dan

Re: questions regarding removing the depencency on CONFIG_IEEE80211

[John W. Linville]

On Tue, Dec 04, 2007 at 09:56:38AM -0500, Dan Williams wrote:
> On Tue, 2007-12-04 at 11:53 +0100, Holger Schurig wrote:
> > 7. escape_ssid()
> > 
> > This code from ieee80211 is used in various debug messages.
> > Should I re-implement this locally in the driver?
> 
> Part of the _point_ of depending on ieee80211 was so that we wouldn't
> have a lot of pointless code duplication.  Like this.

ACK

We've been talking about a new component for this kind of
shared generic 802.11 code for some time.  In my head I call it
"lib80211". :-)

Maybe it is time to start it?  The header of a thousand bytes start
with a single char... :-)

John
-- 
John W. Linville
linville@tuxdriver.com

Re: questions regarding removing the depencency on CONFIG_IEEE80211

[Dan Williams]

On Tue, 2007-12-04 at 10:25 -0500, John W. Linville wrote:
> On Tue, Dec 04, 2007 at 09:56:38AM -0500, Dan Williams wrote:
> > On Tue, 2007-12-04 at 11:53 +0100, Holger Schurig wrote:
> > > 7. escape_ssid()
> > > 
> > > This code from ieee80211 is used in various debug messages.
> > > Should I re-implement this locally in the driver?
> > 
> > Part of the _point_ of depending on ieee80211 was so that we wouldn't
> > have a lot of pointless code duplication.  Like this.
> 
> ACK
> 
> We've been talking about a new component for this kind of
> shared generic 802.11 code for some time.  In my head I call it
> "lib80211". :-)
> 
> Maybe it is time to start it?  The header of a thousand bytes start
> with a single char... :-)

I was thinking about lib80211 this morning too; specifically the code
that everyone has to write to convert local BSS entries into WEXT scan
results.  The problem is that drivers have different ideas of what a BSS
is.  I think the _first_ thing to do is to define a BSS structure (I
used the ipw2x00 bss structure in libertas when doing this) that all the
fullmac drivers can use (airo, atmel, libertas, orinoco, etc).  Then,
each driver can fill out that structure from it's internal scan result,
and hand that off to the lib80211 bss list handling code.  We could have
a help that these driver's get_scan calls to convert the internal list
into WEXT scan results.  The important thing is getting these drivers to
fill out the common bss structure when their firmware returns a scan
result.  We could even make the lib80211 code handle scan result aging
internally.  Could have helper functions to allocate and dispose of the
bss list instead of having to duplicate that code in every driver (it's
already in airo & libertas, and I posted a patch that dupes it again for
orinoco).

It might mean more memory usage for these drivers because not all of
them need all the fields in the common BSS structure.  It may mean
slightly more CPU time used to convert the driver's internal BSS
representation into the common BSS structure.  But that is pretty small
in both cases and I think the benefits outweigh the drawbacks in this
regard.

Each fullmac driver right now is it's own little box, with quite a lot
of duplicated code floating around.  We need to start replacing that
duplicated code/functionality with common stuff for maintainability and
sanity :)

Dan

Re: questions regarding removing the depencency on CONFIG_IEEE80211

[Holger Schurig]

> It might mean more memory usage for these drivers because not
> all of them need all the fields in the common BSS structure. 
> It may mean slightly more CPU time used to convert the
> driver's internal BSS representation into the common BSS
> structure.

I'm mostly doing things for embedded stuff, but actually I like 
the benefits more than the drawbacks.

+1 for your whole idea.

Re: questions regarding removing the depencency on CONFIG_IEEE80211

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1293 bytes --]


> I was thinking about lib80211 this morning too; specifically the code
> that everyone has to write to convert local BSS entries into WEXT scan
> results.  The problem is that drivers have different ideas of what a BSS
> is.  I think the _first_ thing to do is to define a BSS structure (I
> used the ipw2x00 bss structure in libertas when doing this) that all the
> fullmac drivers can use (airo, atmel, libertas, orinoco, etc).  Then,
> each driver can fill out that structure from it's internal scan result,
> and hand that off to the lib80211 bss list handling code.  We could have
> a help that these driver's get_scan calls to convert the internal list
> into WEXT scan results.  The important thing is getting these drivers to
> fill out the common bss structure when their firmware returns a scan
> result.  We could even make the lib80211 code handle scan result aging
> internally.  Could have helper functions to allocate and dispose of the
> bss list instead of having to duplicate that code in every driver (it's
> already in airo & libertas, and I posted a patch that dupes it again for
> orinoco).

Sounds good though I think this in particular is cfg80211 functionality
because over time that will want to export scan results via nl80211 too.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: questions regarding removing the depencency on CONFIG_IEEE80211

[Dan Williams]

On Tue, 2007-12-04 at 17:59 +0100, Johannes Berg wrote:
> > I was thinking about lib80211 this morning too; specifically the code
> > that everyone has to write to convert local BSS entries into WEXT scan
> > results.  The problem is that drivers have different ideas of what a BSS
> > is.  I think the _first_ thing to do is to define a BSS structure (I
> > used the ipw2x00 bss structure in libertas when doing this) that all the
> > fullmac drivers can use (airo, atmel, libertas, orinoco, etc).  Then,
> > each driver can fill out that structure from it's internal scan result,
> > and hand that off to the lib80211 bss list handling code.  We could have
> > a help that these driver's get_scan calls to convert the internal list
> > into WEXT scan results.  The important thing is getting these drivers to
> > fill out the common bss structure when their firmware returns a scan
> > result.  We could even make the lib80211 code handle scan result aging
> > internally.  Could have helper functions to allocate and dispose of the
> > bss list instead of having to duplicate that code in every driver (it's
> > already in airo & libertas, and I posted a patch that dupes it again for
> > orinoco).
> 
> Sounds good though I think this in particular is cfg80211 functionality
> because over time that will want to export scan results via nl80211 too.

Good point; that might be a great way to move forward and start getting
cfg80211 functionality into the fullmac drivers.

Dan

Re: questions regarding removing the depencency on CONFIG_IEEE80211

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 427 bytes --]


> We've been talking about a new component for this kind of
> shared generic 802.11 code for some time.  In my head I call it
> "lib80211". :-)
> 
> Maybe it is time to start it?  The header of a thousand bytes start
> with a single char... :-)

So far we've stuffed that into cfg80211 like the radiotap stuff. I'm not
against taking the radiotap stuff for example out of cfg80211 into
lib80211 though.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: questions regarding removing the depencency on CONFIG_IEEE80211

[Dan Williams]

On Tue, 2007-12-04 at 11:40 +0100, Holger Schurig wrote:
> Last friday I was asked on IRC about the dependency of libertas 
> on CONFIG_IEEE80211. I gave a quick question, but today I had 
> time to look into this further.

The whole point of ripping all this stuff out of libertas and using the
stuff that was already out there was to avoid code duplication.  If you
can remove the dependency on ieee80211 without adding a whole bunch of
stuff back to libertas, that's great, go for it.  In the end, I think
the only actual symbols used are escape_essid and maybe one more.

> 1. simply things
> 
> * remove "select IEEE80211" in wireless/KConfig
> * remove "#include <net/ieee80211.h>"
> 
> 
> 2. MAX_WPA_IE_LEN
> 
> This was defined in net/ieee80211.h, but is not defined in 
> linux/ieee80211.h. I have two options, which one should I go:
> 
> * define it locally in some libertas header file
> * define it as IEEE80211_MAX_WPA_IE_LEN in linux/ieee80211.h

#2 is better here.  But I'm not actually sure that there _is_ a standard
defined maximum IE length.  I used this define because there was a
similar one in libertas and it was pointless to have two.

> 
> 3. MAX_NETWORK_COUNT
> 
> libertas has a list of found BSSses during scanning. It keeps 
> them in an kzalloc()'ed array. However, it doesn't use any of
> ieee80211's code for BSS or associating, so I guess I make this a 
> private define, e.g. LBS_MAX_BSS_COUNT.

Again, the point of using already defined stuff was to take code out of
libertas.  If this is in a header, it's pointless to move it back into
libertas.  I assume that the reason you want to remove the dep on
ieee80211 is to remove the requirement to load that module, which moving
this define back to libertas wouldn't accomplish.

If you'd like to propose moving this define to some other shared header
that would be better (linux/ieee80211.h?).  Libertas is not the only
driver that uses this define (ipw*, airo, libertas).


> 4. DEFAULT_MAX_SCAN_AGE
> 
> Again, libertas doesn't use any aging code from ieee80211, so I 
> guess I make this a libertas-private define.

No, but it does age the scan results after 15 seconds or something.  Or
at least it should.  It shouldn't be throwing any AP in the scan list
away until it's 15 seconds old.  This define was picked because it was
pointless to create yet another define when there was already a usable
and established one.

> 
> 5. MFIE_TYPE_xxxx enum
> 
> They are easily substituded with the proper WLAN_EID_xxx ones.

Yup; good catch.

> 6. struct ieee80211_info_element
> 
> In scan.c, libertas uses heavily the "struct 
> ieee80211_info_element". As this struct might be useful for 
> other non-max80211 drivers, I'd suggest putting it in 
> include/linux/ieee80211.h.

Probably good; again this was picked because it was already around and
contributed to code removal from libertas.  It was pointless to create
yet another structure for each BSS in libertas when there was already a
usable one sitting there.

> 7. RADIOTAP
> 
> Libertas contains lot's of code for it's monitoring mode, which 
> uses radiotap. It used "struct net_device *rtap_net_dev" 
> and "struct ieee80211_device *ieee". It contains code like
> 
>    priv->rtap_net_dev = alloc_ieee80211(0);
>    priv->ieee = netdev_priv(priv->rtap_net_dev);
> 
> and I don't have any knowledge about radiotap and how to 
> substitue the alloc_ieee80211(). All I could currently to is to 
> put those sections into #ifdef CONFIG_IEEE80211.

Javier might have more comments on this.

dan

Re: questions regarding removing the depencency on CONFIG_IEEE80211

[Holger Schurig]

>> > 3. MAX_NETWORK_COUNT
> Again, the point of using already defined stuff was to take
> code out of libertas.  If this is in a header, it's pointless
> to move it back into libertas.

I haven't found something similar in linux/ieee80211.h, but I 
don't know if something with this meaning is in some other 
header file which I don't know. I just doubt it.

> I assume that the reason you 
> want to remove the dep on ieee80211 is to remove the
> requirement to load that module, which moving this define back
> to libertas wouldn't accomplish.

And the reason that some people want to remove the old ieee80211 
code from the kernel source "really soon now" :-)

> If you'd like to propose moving this define to some other
> shared header that would be better (linux/ieee80211.h?). 
> Libertas is not the only driver that uses this define (ipw*,
> airo, libertas).

Okay, if other's don't object I can do that.


> > 4. DEFAULT_MAX_SCAN_AGE
> No, but it does age the scan results after 15 seconds or
> something.  Or at least it should.  It shouldn't be throwing
> any AP in the scan list away until it's 15 seconds old.  This
> define was picked because it was pointless to create yet
> another define when there was already a usable and established
> one.

If I understand you correctly, then this constant should end up 
in linux/ieee80211.h as well?


> > 7. RADIOTAP
> >
> > Libertas contains lot's of code for it's monitoring mode,
> > which uses radiotap. It used "struct net_device
> > *rtap_net_dev" and "struct ieee80211_device *ieee". It
> > contains code like
> >
> >    priv->rtap_net_dev = alloc_ieee80211(0);
> >    priv->ieee = netdev_priv(priv->rtap_net_dev);
> >
> > and I don't have any knowledge about radiotap and how to
> > substitue the alloc_ieee80211(). All I could currently to is
> > to put those sections into #ifdef CONFIG_IEEE80211.
>
> Javier might have more comments on this.

I actually have a WIP patch that removes the ieee80211 
dependency, it just clashes with my WIP 
patch "new-scan.patch" :-)

The patch applies, compiles sparse-clean and is checkpatch clean, 
so all is well ---- except that it completely disables monitor 
mode via radiotap. Maybe I add the bunches you said and the ask 
Javier to take things over, if he wants & has time. Or I split 
the patch into things that can be committed today and only 
remove ieee80211-dependency halfway.

But then again, ieee80211 is still in the kernel, so there's no 
haste in making libertas ready for the post-ieee80211 time. So I 
think I'll wait until David's patches settled.

Re: questions regarding removing the depencency on CONFIG_IEEE80211

[Dan Williams]

On Tue, 2007-12-04 at 16:47 +0100, Holger Schurig wrote:
> >> > 3. MAX_NETWORK_COUNT
> > Again, the point of using already defined stuff was to take
> > code out of libertas.  If this is in a header, it's pointless
> > to move it back into libertas.
> 
> I haven't found something similar in linux/ieee80211.h, but I 
> don't know if something with this meaning is in some other 
> header file which I don't know. I just doubt it.

We should  move something like this to lib80211 in the end anyway, but
for now make it local to libertas.

> > I assume that the reason you 
> > want to remove the dep on ieee80211 is to remove the
> > requirement to load that module, which moving this define back
> > to libertas wouldn't accomplish.
> 
> And the reason that some people want to remove the old ieee80211 
> code from the kernel source "really soon now" :-)

Yeah; I assume this means folding the ieee80211 stack back to being
private to the ipw* drivers too...

> > If you'd like to propose moving this define to some other
> > shared header that would be better (linux/ieee80211.h?). 
> > Libertas is not the only driver that uses this define (ipw*,
> > airo, libertas).
> 
> Okay, if other's don't object I can do that.

Another lib80211 candidate; but if you really want you can move it back
to libertas and we'll extract it for lib80211 later.

> 
> > > 4. DEFAULT_MAX_SCAN_AGE
> > No, but it does age the scan results after 15 seconds or
> > something.  Or at least it should.  It shouldn't be throwing
> > any AP in the scan list away until it's 15 seconds old.  This
> > define was picked because it was pointless to create yet
> > another define when there was already a usable and established
> > one.
> 
> If I understand you correctly, then this constant should end up 
> in linux/ieee80211.h as well?

When we do lib80211 it should really be private to lib80211 because that
would handle all the scan aging; but if this define isn't appropriate
for other drivers that use ieee80211.h then maybe we keep it private for
now and move it to lib80211 later.

> 
> > > 7. RADIOTAP
> > >
> > > Libertas contains lot's of code for it's monitoring mode,
> > > which uses radiotap. It used "struct net_device
> > > *rtap_net_dev" and "struct ieee80211_device *ieee". It
> > > contains code like
> > >
> > >    priv->rtap_net_dev = alloc_ieee80211(0);
> > >    priv->ieee = netdev_priv(priv->rtap_net_dev);
> > >
> > > and I don't have any knowledge about radiotap and how to
> > > substitue the alloc_ieee80211(). All I could currently to is
> > > to put those sections into #ifdef CONFIG_IEEE80211.
> >
> > Javier might have more comments on this.
> 
> I actually have a WIP patch that removes the ieee80211 
> dependency, it just clashes with my WIP 
> patch "new-scan.patch" :-)
> 
> The patch applies, compiles sparse-clean and is checkpatch clean, 
> so all is well ---- except that it completely disables monitor 
> mode via radiotap. Maybe I add the bunches you said and the ask 
> Javier to take things over, if he wants & has time. Or I split 
> the patch into things that can be committed today and only 
> remove ieee80211-dependency halfway.

That's probably best for the _moment_, need to investigate more what
exact functionality from ieee80211 Javier used for radiotap.

> But then again, ieee80211 is still in the kernel, so there's no 
> haste in making libertas ready for the post-ieee80211 time. So I 
> think I'll wait until David's patches settled.

Yeah, sounds like a plan.

Dan

Re: questions regarding removing the depencency on CONFIG_IEEE80211

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 723 bytes --]


> 7. RADIOTAP
> 
> Libertas contains lot's of code for it's monitoring mode, which 
> uses radiotap. It used "struct net_device *rtap_net_dev" 
> and "struct ieee80211_device *ieee". It contains code like
> 
>    priv->rtap_net_dev = alloc_ieee80211(0);
>    priv->ieee = netdev_priv(priv->rtap_net_dev);
> 
> and I don't have any knowledge about radiotap and how to 
> substitue the alloc_ieee80211(). All I could currently to is to 
> put those sections into #ifdef CONFIG_IEEE80211.

You don't really need a ieee80211 interface for radiotap, look how
mac80211 does it, it's a simple netdev with the right arp header. You
don't need to provide anything like wext on it if you don't wish to.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]