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

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

[Andres Salomon]

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 <dilinger@collabora.co.uk>
---
 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

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

[Johannes Berg]

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

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

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

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

[Andres Salomon]

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

On Mon, 08 Feb 2010 10:06:33 +0100
Johannes Berg <johannes@sipsolutions.net> 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"?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

[Johannes Berg]

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

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

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

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

[Kalle Valo]

Andres Salomon <dilinger@collabora.co.uk> 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

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

[Gábor Stefanik]

On Mon, Feb 8, 2010 at 10:23 AM, Kalle Valo <kalle.valo@iki.fi> wrote:
> Andres Salomon <dilinger@collabora.co.uk> 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 majordomo@vger.kernel.org
> 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. :-)

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

[Luis R. Rodriguez]

On Sun, Feb 7, 2010 at 6:48 PM, Andres Salomon <dilinger@collabora.co.uk> 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: stable@kernel.org here as well, I think we've had this for a while.

> Signed-off-by: Andres Salomon <dilinger@collabora.co.uk>

  Luis

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

[Andres Salomon]

On Mon, 8 Feb 2010 10:32:50 -0800
"Luis R. Rodriguez" <mcgrof@gmail.com> wrote:

> On Sun, Feb 7, 2010 at 6:48 PM, Andres Salomon
> <dilinger@collabora.co.uk> 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: stable@kernel.org here as well, I think we've had this for
> a while.
> 
> > Signed-off-by: Andres Salomon <dilinger@collabora.co.uk>
> 
>   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 <dilinger@collabora.co.uk>
---
 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

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

[Luis R. Rodriguez]

On Sun, Feb 14, 2010 at 3:18 PM, Andres Salomon
<dilinger@collabora.co.uk> wrote:
> On Mon, 8 Feb 2010 10:32:50 -0800
> "Luis R. Rodriguez" <mcgrof@gmail.com> wrote:
>
>> On Sun, Feb 7, 2010 at 6:48 PM, Andres Salomon
>> <dilinger@collabora.co.uk> 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: stable@kernel.org here as well, I think we've had this for
>> a while.
>>
>> > Signed-off-by: Andres Salomon <dilinger@collabora.co.uk>
>>
>>   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 <dilinger@collabora.co.uk>

You want to add Cc: stable@kernel.org on the commit log, not on the e-mail.

  Luis

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

[John W. Linville]

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
> <dilinger@collabora.co.uk> wrote:
> > On Mon, 8 Feb 2010 10:32:50 -0800
> > "Luis R. Rodriguez" <mcgrof@gmail.com> wrote:
> >
> >> On Sun, Feb 7, 2010 at 6:48 PM, Andres Salomon
> >> <dilinger@collabora.co.uk> 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: stable@kernel.org here as well, I think we've had this for
> >> a while.
> >>
> >> > Signed-off-by: Andres Salomon <dilinger@collabora.co.uk>
> >>
> >>   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 <dilinger@collabora.co.uk>
> 
> You want to add Cc: stable@kernel.org 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
linville@tuxdriver.com			might be all we have.  Be ready.