Guidelines regarding separate directory

Guidelines regarding separate directory

[Larry Finger]

I am planning on adding code to control the LEDs on the RTL8187 devices based on
the latest vendor driver. From the expected size of the code, I think that
separate files will be best.

What number of files in a driver make it advisable to create a separate
directory? Between the rtl8180 and rtl8187 drivers, there are now 15 files and I
will be adding at least 2 more. Is this the time to create a rtl818x directory
under drivers/net/wireless?

Larry

Re: Guidelines regarding separate directory

[Johannes Berg]

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

On Thu, 2008-10-30 at 10:10 -0700, Larry Finger wrote:
> I am planning on adding code to control the LEDs on the RTL8187 devices based on
> the latest vendor driver. From the expected size of the code, I think that
> separate files will be best.
> 
> What number of files in a driver make it advisable to create a separate
> directory? Between the rtl8180 and rtl8187 drivers, there are now 15 files and I
> will be adding at least 2 more. Is this the time to create a rtl818x directory
> under drivers/net/wireless?

I for one think that should have been done already, makes finding
drivers easier imho. But I don't think there are strict guidelines in
any way.

johannes

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

Re: Guidelines regarding separate directory

[Dave]

Johannes Berg wrote:
> On Thu, 2008-10-30 at 10:10 -0700, Larry Finger wrote:
>> I am planning on adding code to control the LEDs on the RTL8187 devices based on
>> the latest vendor driver. From the expected size of the code, I think that
>> separate files will be best.
>>
>> What number of files in a driver make it advisable to create a separate
>> directory? Between the rtl8180 and rtl8187 drivers, there are now 15 files and I
>> will be adding at least 2 more. Is this the time to create a rtl818x directory
>> under drivers/net/wireless?
> 
> I for one think that should have been done already, makes finding
> drivers easier imho. But I don't think there are strict guidelines in
> any way.

On a similar note, would anyone object to collecting the orinoco files
in a subdirectory? I'm looking at doing some refactoring of orinoco.c
which I think can be split into at least 3 files.


Regards,

Dave.

PS the existing files that would move:
airport.c
hermes.c
hermes.h
hermes_dld.c
hermes_dld.h
hermes_rid.h
orinoco.c
orinoco.h
orinoco_cs.c
orinoco_pci.c
orinoco_pci.h
orinoco_plx.c
orinoco_tmd.c
orinoco_nortel.c
spectrum_cs.c

Re: Guidelines regarding separate directory

[Johannes Berg]

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

On Thu, 2008-10-30 at 20:24 +0000, Dave wrote:
> Johannes Berg wrote:
> > On Thu, 2008-10-30 at 10:10 -0700, Larry Finger wrote:
> >> I am planning on adding code to control the LEDs on the RTL8187 devices based on
> >> the latest vendor driver. From the expected size of the code, I think that
> >> separate files will be best.
> >>
> >> What number of files in a driver make it advisable to create a separate
> >> directory? Between the rtl8180 and rtl8187 drivers, there are now 15 files and I
> >> will be adding at least 2 more. Is this the time to create a rtl818x directory
> >> under drivers/net/wireless?
> > 
> > I for one think that should have been done already, makes finding
> > drivers easier imho. But I don't think there are strict guidelines in
> > any way.
> 
> On a similar note, would anyone object to collecting the orinoco files
> in a subdirectory? I'm looking at doing some refactoring of orinoco.c
> which I think can be split into at least 3 files.

Go for it! Well, I'd appreciate if you'd let me repost the ieee80211
removal first :) Will get to that right now.

johannes

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

Re: Guidelines regarding separate directory

[Pavel Roskin]

On Thu, 2008-10-30 at 20:24 +0000, Dave wrote:
> Johannes Berg wrote:

> On a similar note, would anyone object to collecting the orinoco files
> in a subdirectory?

Fine with me.

> I'm looking at doing some refactoring of orinoco.c
> which I think can be split into at least 3 files.

Maybe it's too late at this point, but it would be nice to support
Orinoco USB eventually.  It uses modified orinoco.c.  Please keep it in
mind when splitting.  It may be possible to reuse some of the new parts
in the USB driver.

> PS the existing files that would move:
> airport.c
> hermes.c
> hermes.h
> hermes_dld.c
> hermes_dld.h
> hermes_rid.h
> orinoco.c
> orinoco.h
> orinoco_cs.c
> orinoco_pci.c

orinoco_pci.c is a good candidate for removal.  It only supports the
hardware that is already supported by hostap_pci.

-- 
Regards,
Pavel Roskin

Re: Guidelines regarding separate directory

[Johannes Berg]

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


> > orinoco_pci.c
> 
> orinoco_pci.c is a good candidate for removal.  It only supports the
> hardware that is already supported by hostap_pci.

Wait wait. I think everybody would rather see hostap go than orinoco
since hostap is a huge complex thing and quite buggy.

johannes

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

Re: Guidelines regarding separate directory

[Pavel Roskin]

Quoting Johannes Berg <johannes@sipsolutions.net>:

>
>> > orinoco_pci.c
>>
>> orinoco_pci.c is a good candidate for removal.  It only supports the
>> hardware that is already supported by hostap_pci.
>
> Wait wait. I think everybody would rather see hostap go than orinoco
> since hostap is a huge complex thing and quite buggy.

The WPA support in orinoco is limited to Agere firmware.  Maybe Symbol  
firmware will be supported at some point.  But adding WPA support for  
Intersil firmware would duplicate efforts.

The only thing I would like to change in hostap is that it should  
default to managed mode, like other drivers do.

But since there are objections, let's keep orinoco_pci as long as it  
doesn't stand in the way of further changes.

-- 
Regards,
Pavel Roskin

Re: Guidelines regarding separate directory

[Johannes Berg]

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

On Thu, 2008-10-30 at 23:10 -0400, Pavel Roskin wrote:
> Quoting Johannes Berg <johannes@sipsolutions.net>:

> The WPA support in orinoco is limited to Agere firmware.  Maybe Symbol  
> firmware will be supported at some point.  But adding WPA support for  
> Intersil firmware would duplicate efforts.

Yes, I know, but wouldn't adding that based on the hostap stuff be
fairly simple? I know nobody has time to...

> The only thing I would like to change in hostap is that it should  
> default to managed mode, like other drivers do.

No, that's not true, just see
http://bugzilla.kernel.org/show_bug.cgi?id=11802 for example. The AP
part also duplicates a lot of mac80211 functionality.

johannes

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

Re: Guidelines regarding separate directory

[Pavel Roskin]

On Fri, 2008-10-31 at 11:01 +0100, Johannes Berg wrote:
> On Thu, 2008-10-30 at 23:10 -0400, Pavel Roskin wrote:
> > Quoting Johannes Berg <johannes@sipsolutions.net>:
> 
> > The WPA support in orinoco is limited to Agere firmware.  Maybe Symbol  
> > firmware will be supported at some point.  But adding WPA support for  
> > Intersil firmware would duplicate efforts.
> 
> Yes, I know, but wouldn't adding that based on the hostap stuff be
> fairly simple? I know nobody has time to...

OK, we'll see.  The orinoco driver was written in the assumption that
the three kinds of firmware (and hardware) it supports are very similar.
Once we are trying to do the advanced stuff like WPA, the differences
are everywhere.  There are actually few similarities.  It could be
handled by moving specific routines to separate files and using tables
of functions.

> > The only thing I would like to change in hostap is that it should  
> > default to managed mode, like other drivers do.
> 
> No, that's not true, just see
> http://bugzilla.kernel.org/show_bug.cgi?id=11802 for example. The AP
> part also duplicates a lot of mac80211 functionality.

I thought I was due to my local changes that I never published.  It's
actually good news for me that others are getting it with the unmodified
driver!

-- 
Regards,
Pavel Roskin

Re: Guidelines regarding separate directory

[Dan Williams]

On Fri, 2008-10-31 at 11:01 +0100, Johannes Berg wrote:
> On Thu, 2008-10-30 at 23:10 -0400, Pavel Roskin wrote:
> > Quoting Johannes Berg <johannes@sipsolutions.net>:
> 
> > The WPA support in orinoco is limited to Agere firmware.  Maybe Symbol  
> > firmware will be supported at some point.  But adding WPA support for  
> > Intersil firmware would duplicate efforts.
> 
> Yes, I know, but wouldn't adding that based on the hostap stuff be
> fairly simple? I know nobody has time to...
> 
> > The only thing I would like to change in hostap is that it should  
> > default to managed mode, like other drivers do.
> 
> No, that's not true, just see
> http://bugzilla.kernel.org/show_bug.cgi?id=11802 for example. The AP
> part also duplicates a lot of mac80211 functionality.

Orinoco is a mostly-fullmac driver, while hostap is quasi-softmac,
right?

Dan

Re: Guidelines regarding separate directory

[Johannes Berg]

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

On Fri, 2008-10-31 at 12:13 -0400, Dan Williams wrote:

> > No, that's not true, just see
> > http://bugzilla.kernel.org/show_bug.cgi?id=11802 for example. The AP
> > part also duplicates a lot of mac80211 functionality.
> 
> Orinoco is a mostly-fullmac driver, while hostap is quasi-softmac,
> right?

I thought hostap was mostly fullmac for STA mode, and softmac for AP
mode? Not sure though.

johannes

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

Re: Guidelines regarding separate directory

[Richard Farina]

Johannes Berg wrote:
>>> orinoco_pci.c
>>>       
>> orinoco_pci.c is a good candidate for removal.  It only supports the
>> hardware that is already supported by hostap_pci.
>>     
>
> Wait wait. I think everybody would rather see hostap go than orinoco
> since hostap is a huge complex thing and quite buggy.
>
>   
I really disagree with this statement.  Until kernel version 2.6.21 the 
hostap driver was used very widely by the wifi security community 
because of it's great abilities for monitor mode and frame injection.  
In kernel 2.6.21 there was a change that made the injection patches not 
work and my ability is insufficient to fix this regression, however, the 
orinoco driver has always had morbid issues with simple things such as 
monitor mode.

I understand that hostap needs some work, and it would be nice if 
someone volunteered to help fix the packet injection, but removing this 
driver in favor of orinoco_cs would be a serious regression.

thanks,
Rick Farina
> johannes
>   

Re: Guidelines regarding separate directory

[Pavel Roskin]

On Fri, 2008-10-31 at 13:04 -0400, Richard Farina wrote:
> Johannes Berg wrote:
> >>> orinoco_pci.c
> >>>       
> >> orinoco_pci.c is a good candidate for removal.  It only supports the
> >> hardware that is already supported by hostap_pci.
> >>     
> >
> > Wait wait. I think everybody would rather see hostap go than orinoco
> > since hostap is a huge complex thing and quite buggy.
> >
> >   
> I really disagree with this statement.  Until kernel version 2.6.21 the 
> hostap driver was used very widely by the wifi security community 
> because of it's great abilities for monitor mode and frame injection.  
> In kernel 2.6.21 there was a change that made the injection patches not 
> work and my ability is insufficient to fix this regression, however, the 
> orinoco driver has always had morbid issues with simple things such as 
> monitor mode.

The only issue I know in the monitor mode in orinoco is lack of radiotap
headers.  That should not be hard to add.  Issues with Agere firmware
are irrelevant since hostap doesn't support it.  Are there any other
issues you have in mind?

> I understand that hostap needs some work, and it would be nice if 
> someone volunteered to help fix the packet injection, but removing this 
> driver in favor of orinoco_cs would be a serious regression.

At this point, I agree.  It's better that we concentrate on fixing known
issues rather than on choosing which driver to remove.

-- 
Regards,
Pavel Roskin

Re: Guidelines regarding separate directory

[Richard Farina]

Pavel Roskin wrote:
> On Fri, 2008-10-31 at 13:04 -0400, Richard Farina wrote:
>   
>> Johannes Berg wrote:
>>     
>>>>> orinoco_pci.c
>>>>>       
>>>>>           
>>>> orinoco_pci.c is a good candidate for removal.  It only supports the
>>>> hardware that is already supported by hostap_pci.
>>>>     
>>>>         
>>> Wait wait. I think everybody would rather see hostap go than orinoco
>>> since hostap is a huge complex thing and quite buggy.
>>>
>>>   
>>>       
>> I really disagree with this statement.  Until kernel version 2.6.21 the 
>> hostap driver was used very widely by the wifi security community 
>> because of it's great abilities for monitor mode and frame injection.  
>> In kernel 2.6.21 there was a change that made the injection patches not 
>> work and my ability is insufficient to fix this regression, however, the 
>> orinoco driver has always had morbid issues with simple things such as 
>> monitor mode.
>>     
>
> The only issue I know in the monitor mode in orinoco is lack of radiotap
> headers.  That should not be hard to add.  Issues with Agere firmware
> are irrelevant since hostap doesn't support it.  Are there any other
> issues you have in mind?
>
>   
Last I checked, the orinoco_cs driver couldn't even put a prism2 card in 
monitor mode.  I just tried it again and that is working, but the 
channel hopping control doesn't seem to work properly at all. As an 
example, airodump-ng cannot hop channels (which I'm reporting to the 
aircrack-ng team next).  I know that the idea of having one driver for 
the two devices may seem like a good idea, but I've always been against 
it because in fact the orinoco cards were a poor implementation of a 
modified prism2 reference implementation.  I mean no disrespect to the 
orinoco driver team (yes Pavel, I see your name on it) but the driver 
simply is not nearly as mature as the hostap drivers.  The additional 
functionality of the hostap drivers such as master mode, monitor mode 
headers, and firmware loading (both flash and ram) make is an excellent 
choice to be maintained at least in the foreseeable future.

That said, if there is anyone with some interest in fixing the monitor 
mode inject which was broken in kernel 2.6.21 I would be happy to 
provide you with everything I know if you can help (I have a lot of 
documentation and the existing patch sets).  Sadly, my coding skills 
only let me work on very minor things.

Thanks,
Rick Farina

>> I understand that hostap needs some work, and it would be nice if 
>> someone volunteered to help fix the packet injection, but removing this 
>> driver in favor of orinoco_cs would be a serious regression.
>>     
>
> At this point, I agree.  It's better that we concentrate on fixing known
> issues rather than on choosing which driver to remove.
>
>   

Re: Guidelines regarding separate directory

[Pavel Roskin]

On Fri, 2008-10-31 at 15:06 -0400, Richard Farina wrote:

> Last I checked, the orinoco_cs driver couldn't even put a prism2 card in 
> monitor mode.

It must have been some really obsolete driver.  I cannot reproduce this
problem with the current code.

>   I just tried it again and that is working, but the 
> channel hopping control doesn't seem to work properly at all. As an 
> example, airodump-ng cannot hop channels (which I'm reporting to the 
> aircrack-ng team next).

I'm not going to install that software, but I have no problem changing
channel in monitor mode with iwconfig.

>   I know that the idea of having one driver for 
> the two devices may seem like a good idea, but I've always been against 
> it because in fact the orinoco cards were a poor implementation of a 
> modified prism2 reference implementation.  I mean no disrespect to the 
> orinoco driver team (yes Pavel, I see your name on it) but the driver 
> simply is not nearly as mature as the hostap drivers.  The additional 
> functionality of the hostap drivers such as master mode, monitor mode 
> headers, and firmware loading (both flash and ram) make is an excellent 
> choice to be maintained at least in the foreseeable future.

I don't expect master mode to be implemented in orinoco, but the
radiotap headers should not be hard to add.  In fact, the hard part
would be not coding but negative feedback from users with various badly
written hacking tools that don't expect the radiotap header to be
present.

> That said, if there is anyone with some interest in fixing the monitor 
> mode inject which was broken in kernel 2.6.21 I would be happy to 
> provide you with everything I know if you can help (I have a lot of 
> documentation and the existing patch sets).  Sadly, my coding skills 
> only let me work on very minor things.

I'm not interested in coding it myself, but I'm would not object if
somebody provides a patch.

-- 
Regards,
Pavel Roskin

Re: Guidelines regarding separate directory

[Dave]

Pavel Roskin wrote:
> On Thu, 2008-10-30 at 20:24 +0000, Dave wrote:
>> On a similar note, would anyone object to collecting the orinoco files
>> in a subdirectory?
> 
> Fine with me.
> 
>> I'm looking at doing some refactoring of orinoco.c
>> which I think can be split into at least 3 files.
> 
> Maybe it's too late at this point, but it would be nice to support
> Orinoco USB eventually.  It uses modified orinoco.c.  Please keep it in
> mind when splitting.  It may be possible to reuse some of the new parts
> in the USB driver.

That's partly why I'm refactorring. Other things I'm thinking of:

cfg80211 (i.e. disentangling the wext handlers from hermes calls)
hermes II
radiotap

Just don't hold your breath.

>> PS the existing files that would move:
>> orinoco_pci.c
> 
> orinoco_pci.c is a good candidate for removal.  It only supports the
> hardware that is already supported by hostap_pci.

ACK. I'll include that as part of the same series.


Dave.

Re: Guidelines regarding separate directory

[John W. Linville]

On Thu, Oct 30, 2008 at 10:10:16AM -0700, Larry Finger wrote:
> I am planning on adding code to control the LEDs on the RTL8187 devices based on
> the latest vendor driver. From the expected size of the code, I think that
> separate files will be best.
> 
> What number of files in a driver make it advisable to create a separate
> directory? Between the rtl8180 and rtl8187 drivers, there are now 15 files and I
> will be adding at least 2 more. Is this the time to create a rtl818x directory
> under drivers/net/wireless?

Yes.  It should have been done long ago -- I had them all in
drivers/net/wireless due to my own misinterpretation of the policy
in the past.

John
-- 
John W. Linville		Linux should be at the core
linville@tuxdriver.com			of your literate lifestyle.

Re: Guidelines regarding separate directory

[Larry Finger]

John W. Linville wrote:
> 
> Yes.  It should have been done long ago -- I had them all in
> drivers/net/wireless due to my own misinterpretation of the policy
> in the past.

I will do it, but not until some timing patches have been reviewed and
submitted. At least there is some time before the 2.6.29 merge window.

Larry

Re: Guidelines regarding separate directory

[Johannes Berg]

On Thu, 2008-10-30 at 16:47 -0700, Larry Finger wrote:
> John W. Linville wrote:
> > 
> > Yes.  It should have been done long ago -- I had them all in
> > drivers/net/wireless due to my own misinterpretation of the policy
> > in the past.
> 
> I will do it, but not until some timing patches have been reviewed and
> submitted. At least there is some time before the 2.6.29 merge window.

Btw, for the actual execution you may want to send a git patch rather
than a normal patch, that would contain the renames rather than huge
adds/removes. To do that, use "git format-patch -M", "git am" can take
such a special patch, and it looks like this (example done on iw):

diff --git a/Makefile b/Makefile
index 3fa0a83..74b2e16 100644
--- a/Makefile
+++ b/Makefile
@@ -11,7 +11,7 @@ CFLAGS += -O2 -g
 LDFLAGS += `pkg-config --libs libnl-1`
 NLVERSION = 1.0
 
-OBJS = iw.o info.o phy.o interface.o station.o util.o mpath.o reg.o mesh.o genl
.o
+OBJS = iw.o info-test.o phy.o interface.o station.o util.o mpath.o reg.o mesh.o
 genl.o
 ALL = iw
 
 ifeq ($(V),1)
diff --git a/info.c b/info-test.c
similarity index 100%
rename from info.c
rename to info-test.c


johannes