2010-02-08 02:58:49

by Andres Salomon

[permalink] [raw]
Subject: [PATCH 2/2] mac80211: give warning if building w/out rate ctrl algorithm


I discovered that if EMBEDDED=y, one can accidentally build a mac80211 stack
w/ no rate control algorithm. When RC_MINISTREL and RC_PID are both disabled,
the RC_DEFAULT string (which rate.c uses as the fallback algorithm) will be
"". That'll cause the rate_control_alloc to fail, which will in turn cause
ieee80211_register_hw to fail. IOW, no driver will load.

This will tell kconfig to provide a warning if no rate control algorithms
have been selected. That'll at least warn the user that they're about to
build a broken wireless stack.

Signed-off-by: Andres Salomon <[email protected]>
---
net/mac80211/Kconfig | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/Kconfig b/net/mac80211/Kconfig
index a10d508..1f0ebab 100644
--- a/net/mac80211/Kconfig
+++ b/net/mac80211/Kconfig
@@ -15,8 +15,12 @@ comment "CFG80211 needs to be enabled for MAC80211"

if MAC80211 != n

+config MAC80211_HAS_RC
+ def_bool n
+
config MAC80211_RC_PID
bool "PID controller based rate control algorithm" if EMBEDDED
+ select MAC80211_HAS_RC
---help---
This option enables a TX rate control algorithm for
mac80211 that uses a PID controller to select the TX
@@ -24,12 +28,14 @@ config MAC80211_RC_PID

config MAC80211_RC_MINSTREL
bool "Minstrel" if EMBEDDED
+ select MAC80211_HAS_RC
default y
---help---
This option enables the 'minstrel' TX rate control algorithm

choice
prompt "Default rate control algorithm"
+ depends on MAC80211_HAS_RC
default MAC80211_RC_DEFAULT_MINSTREL
---help---
This option selects the default rate control algorithm
@@ -62,6 +68,9 @@ config MAC80211_RC_DEFAULT

endif

+comment "mac80211 will not work properly w/out rate control algorithm"
+ depends on MAC80211_HAS_RC=n
+
config MAC80211_MESH
bool "Enable mac80211 mesh networking (pre-802.11s) support"
depends on MAC80211 && EXPERIMENTAL
--
1.5.6.5



2010-02-16 20:05:46

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH 2/2] mac80211: give warning if building w/out rate ctrl algorithm

On Sun, Feb 14, 2010 at 3:18 PM, Andres Salomon
<[email protected]> wrote:
> On Mon, 8 Feb 2010 10:32:50 -0800
> "Luis R. Rodriguez" <[email protected]> wrote:
>
>> On Sun, Feb 7, 2010 at 6:48 PM, Andres Salomon
>> <[email protected]> wrote:
>> >
>> > I discovered that if EMBEDDED=y, one can accidentally build a
>> > mac80211 stack w/ no rate control algorithm.  When RC_MINISTREL and
>> > RC_PID are both disabled, the RC_DEFAULT string (which rate.c uses
>> > as the fallback algorithm) will be "".  That'll cause the
>> > rate_control_alloc to fail, which will in turn cause
>> > ieee80211_register_hw to fail.  IOW, no driver will load.
>> >
>> > This will tell kconfig to provide a warning if no rate control
>> > algorithms have been selected.  That'll at least warn the user that
>> > they're about to build a broken wireless stack.
>>
>>
>> Please Cc: [email protected] here as well, I think we've had this for
>> a while.
>>
>> > Signed-off-by: Andres Salomon <[email protected]>
>>
>>   Luis
>
> Here's an updated patch.  I ended up just changing the wording.
>
>
>
>
> Subject: [PATCH] mac80211: give warning if building w/out rate ctrl algorithm
>
> I discovered that if EMBEDDED=y, one can accidentally build a mac80211 stack
> and drivers w/ no rate control algorithm.  For drivers like RTL8187 that don't
> supply their own RC algorithms, this will cause ieee80211_register_hw to
> fail (making the driver unusable).
>
> This will tell kconfig to provide a warning if no rate control algorithms
> have been selected.  That'll at least warn the user; users that know that
> their drivers supply a rate control algorithm can safely ignore the
> warning, and those who don't know (or who expect to be using multiple
> drivers) can select a default RC algorithm.
>
> Signed-off-by: Andres Salomon <[email protected]>

You want to add Cc: [email protected] on the commit log, not on the e-mail.

Luis

2010-02-14 23:18:33

by Andres Salomon

[permalink] [raw]
Subject: Re: [PATCH 2/2] mac80211: give warning if building w/out rate ctrl algorithm

On Mon, 8 Feb 2010 10:32:50 -0800
"Luis R. Rodriguez" <[email protected]> wrote:

> On Sun, Feb 7, 2010 at 6:48 PM, Andres Salomon
> <[email protected]> wrote:
> >
> > I discovered that if EMBEDDED=y, one can accidentally build a
> > mac80211 stack w/ no rate control algorithm.  When RC_MINISTREL and
> > RC_PID are both disabled, the RC_DEFAULT string (which rate.c uses
> > as the fallback algorithm) will be "".  That'll cause the
> > rate_control_alloc to fail, which will in turn cause
> > ieee80211_register_hw to fail.  IOW, no driver will load.
> >
> > This will tell kconfig to provide a warning if no rate control
> > algorithms have been selected.  That'll at least warn the user that
> > they're about to build a broken wireless stack.
>
>
> Please Cc: [email protected] here as well, I think we've had this for
> a while.
>
> > Signed-off-by: Andres Salomon <[email protected]>
>
> Luis

Here's an updated patch. I ended up just changing the wording.




Subject: [PATCH] mac80211: give warning if building w/out rate ctrl algorithm

I discovered that if EMBEDDED=y, one can accidentally build a mac80211 stack
and drivers w/ no rate control algorithm. For drivers like RTL8187 that don't
supply their own RC algorithms, this will cause ieee80211_register_hw to
fail (making the driver unusable).

This will tell kconfig to provide a warning if no rate control algorithms
have been selected. That'll at least warn the user; users that know that
their drivers supply a rate control algorithm can safely ignore the
warning, and those who don't know (or who expect to be using multiple
drivers) can select a default RC algorithm.

Signed-off-by: Andres Salomon <[email protected]>
---
net/mac80211/Kconfig | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/Kconfig b/net/mac80211/Kconfig
index a10d508..2a51b0b 100644
--- a/net/mac80211/Kconfig
+++ b/net/mac80211/Kconfig
@@ -15,8 +15,12 @@ comment "CFG80211 needs to be enabled for MAC80211"

if MAC80211 != n

+config MAC80211_HAS_RC
+ def_bool n
+
config MAC80211_RC_PID
bool "PID controller based rate control algorithm" if EMBEDDED
+ select MAC80211_HAS_RC
---help---
This option enables a TX rate control algorithm for
mac80211 that uses a PID controller to select the TX
@@ -24,12 +28,14 @@ config MAC80211_RC_PID

config MAC80211_RC_MINSTREL
bool "Minstrel" if EMBEDDED
+ select MAC80211_HAS_RC
default y
---help---
This option enables the 'minstrel' TX rate control algorithm

choice
prompt "Default rate control algorithm"
+ depends on MAC80211_HAS_RC
default MAC80211_RC_DEFAULT_MINSTREL
---help---
This option selects the default rate control algorithm
@@ -62,6 +68,9 @@ config MAC80211_RC_DEFAULT

endif

+comment "some wireless drivers require a rate control algorithm"
+ depends on MAC80211_HAS_RC=n
+
config MAC80211_MESH
bool "Enable mac80211 mesh networking (pre-802.11s) support"
depends on MAC80211 && EXPERIMENTAL
--
1.5.6.5


2010-02-08 13:46:16

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 2/2] mac80211: give warning if building w/out rate ctrl algorithm

On Sun, 2010-02-07 at 21:48 -0500, Andres Salomon wrote:
> I discovered that if EMBEDDED=y, one can accidentally build a mac80211 stack
> w/ no rate control algorithm. When RC_MINISTREL and RC_PID are both disabled,
> the RC_DEFAULT string (which rate.c uses as the fallback algorithm) will be
> "". That'll cause the rate_control_alloc to fail, which will in turn cause
> ieee80211_register_hw to fail. IOW, no driver will load.
>
> This will tell kconfig to provide a warning if no rate control algorithms
> have been selected. That'll at least warn the user that they're about to
> build a broken wireless stack.


> +comment "mac80211 will not work properly w/out rate control algorithm"
> + depends on MAC80211_HAS_RC=n

Except that isn't true for hardware that doesn't require an algorithm,
or drivers that provide it themselves, like iwlagn or ath9k. I would
prefer it to be worded accordingly.

johannes


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part

2010-02-08 18:33:11

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH 2/2] mac80211: give warning if building w/out rate ctrl algorithm

On Sun, Feb 7, 2010 at 6:48 PM, Andres Salomon <[email protected]> wrote:
>
> I discovered that if EMBEDDED=y, one can accidentally build a mac80211 stack
> w/ no rate control algorithm.  When RC_MINISTREL and RC_PID are both disabled,
> the RC_DEFAULT string (which rate.c uses as the fallback algorithm) will be
> "".  That'll cause the rate_control_alloc to fail, which will in turn cause
> ieee80211_register_hw to fail.  IOW, no driver will load.
>
> This will tell kconfig to provide a warning if no rate control algorithms
> have been selected.  That'll at least warn the user that they're about to
> build a broken wireless stack.


Please Cc: [email protected] here as well, I think we've had this for a while.

> Signed-off-by: Andres Salomon <[email protected]>

Luis

2010-02-08 09:23:52

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 2/2] mac80211: give warning if building w/out rate ctrl algorithm

Andres Salomon <[email protected]> writes:

> I discovered that if EMBEDDED=y, one can accidentally build a
> mac80211 stack w/ no rate control algorithm. When RC_MINISTREL and
> RC_PID are both disabled, the RC_DEFAULT string (which rate.c uses
> as the fallback algorithm) will be "". That'll cause the
> rate_control_alloc to fail, which will in turn cause
> ieee80211_register_hw to fail. IOW, no driver will load.

There are devices that don't need rate control algorithm, for example
wl1271 (see IEEE80211_HW_HAS_RATE_CONTROL). A better solution is
instead to fix rate_control_alloc() to handle this, or something
similar.

--
Kalle Valo

2010-02-09 08:00:53

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 2/2] mac80211: give warning if building w/out rate ctrl algorithm

On Mon, 2010-02-08 at 12:07 -0500, Andres Salomon wrote:

> > > +comment "mac80211 will not work properly w/out rate control
> > > algorithm"
> > > + depends on MAC80211_HAS_RC=n
> >
> > Except that isn't true for hardware that doesn't require an algorithm,
> > or drivers that provide it themselves, like iwlagn or ath9k. I would
> > prefer it to be worded accordingly.
> >
>
> Noted. I was wondering if there were any.
>
> How about having drivers that need them depend upon MAC80211_HAS_RC or
> some such, and having the text say something like "most drivers require
> rate control"?

I guess that might make sense in that if you have any driver that needs
it, it'd remind you to select one. I'd be happy with that if you want to
do it, but that means touching all the driver Kconfig. Alternatively,
I'd just reword the warning to include that possibility.

johannes


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part

2010-02-08 09:27:58

by Gábor Stefanik

[permalink] [raw]
Subject: Re: [PATCH 2/2] mac80211: give warning if building w/out rate ctrl algorithm

On Mon, Feb 8, 2010 at 10:23 AM, Kalle Valo <[email protected]> wrote:
> Andres Salomon <[email protected]> writes:
>
>> I discovered that if EMBEDDED=y, one can accidentally build a
>> mac80211 stack w/ no rate control algorithm. When RC_MINISTREL and
>> RC_PID are both disabled, the RC_DEFAULT string (which rate.c uses
>> as the fallback algorithm) will be "". That'll cause the
>> rate_control_alloc to fail, which will in turn cause
>> ieee80211_register_hw to fail. IOW, no driver will load.
>
> There are devices that don't need rate control algorithm, for example
> wl1271 (see IEEE80211_HW_HAS_RATE_CONTROL). A better solution is
> instead to fix rate_control_alloc() to handle this, or something
> similar.
>
> --
> Kalle Valo
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>

There is also iwlwifi (and ath9k) which use their own RC algorithms.
AFAIK they need no external RC algo either.

--
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)

2010-02-08 17:08:04

by Andres Salomon

[permalink] [raw]
Subject: Re: [PATCH 2/2] mac80211: give warning if building w/out rate ctrl algorithm

On Mon, 08 Feb 2010 10:06:33 +0100
Johannes Berg <[email protected]> wrote:

> On Sun, 2010-02-07 at 21:48 -0500, Andres Salomon wrote:
> > I discovered that if EMBEDDED=y, one can accidentally build a
> > mac80211 stack w/ no rate control algorithm. When RC_MINISTREL and
> > RC_PID are both disabled, the RC_DEFAULT string (which rate.c uses
> > as the fallback algorithm) will be "". That'll cause the
> > rate_control_alloc to fail, which will in turn cause
> > ieee80211_register_hw to fail. IOW, no driver will load.
> >
> > This will tell kconfig to provide a warning if no rate control
> > algorithms have been selected. That'll at least warn the user that
> > they're about to build a broken wireless stack.
>
>
> > +comment "mac80211 will not work properly w/out rate control
> > algorithm"
> > + depends on MAC80211_HAS_RC=n
>
> Except that isn't true for hardware that doesn't require an algorithm,
> or drivers that provide it themselves, like iwlagn or ath9k. I would
> prefer it to be worded accordingly.
>

Noted. I was wondering if there were any.

How about having drivers that need them depend upon MAC80211_HAS_RC or
some such, and having the text say something like "most drivers require
rate control"?


Attachments:
signature.asc (198.00 B)

2010-02-16 20:15:51

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH 2/2] mac80211: give warning if building w/out rate ctrl algorithm

On Tue, Feb 16, 2010 at 12:05:25PM -0800, Luis R. Rodriguez wrote:
> On Sun, Feb 14, 2010 at 3:18 PM, Andres Salomon
> <[email protected]> wrote:
> > On Mon, 8 Feb 2010 10:32:50 -0800
> > "Luis R. Rodriguez" <[email protected]> wrote:
> >
> >> On Sun, Feb 7, 2010 at 6:48 PM, Andres Salomon
> >> <[email protected]> wrote:
> >> >
> >> > I discovered that if EMBEDDED=y, one can accidentally build a
> >> > mac80211 stack w/ no rate control algorithm. ?When RC_MINISTREL and
> >> > RC_PID are both disabled, the RC_DEFAULT string (which rate.c uses
> >> > as the fallback algorithm) will be "". ?That'll cause the
> >> > rate_control_alloc to fail, which will in turn cause
> >> > ieee80211_register_hw to fail. ?IOW, no driver will load.
> >> >
> >> > This will tell kconfig to provide a warning if no rate control
> >> > algorithms have been selected. ?That'll at least warn the user that
> >> > they're about to build a broken wireless stack.
> >>
> >>
> >> Please Cc: [email protected] here as well, I think we've had this for
> >> a while.
> >>
> >> > Signed-off-by: Andres Salomon <[email protected]>
> >>
> >> ? Luis
> >
> > Here's an updated patch. ?I ended up just changing the wording.
> >
> >
> >
> >
> > Subject: [PATCH] mac80211: give warning if building w/out rate ctrl algorithm
> >
> > I discovered that if EMBEDDED=y, one can accidentally build a mac80211 stack
> > and drivers w/ no rate control algorithm. ?For drivers like RTL8187 that don't
> > supply their own RC algorithms, this will cause ieee80211_register_hw to
> > fail (making the driver unusable).
> >
> > This will tell kconfig to provide a warning if no rate control algorithms
> > have been selected. ?That'll at least warn the user; users that know that
> > their drivers supply a rate control algorithm can safely ignore the
> > warning, and those who don't know (or who expect to be using multiple
> > drivers) can select a default RC algorithm.
> >
> > Signed-off-by: Andres Salomon <[email protected]>
>
> You want to add Cc: [email protected] on the commit log, not on the e-mail.

And repost the patch in a separate email, not in a reply that I have
to munge before I can feed it to git...

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.