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

[PATCH] 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
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>
Cc: stable@kernel.org
---
 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] mac80211: give warning if building w/out rate ctrl algorithm

[Marcel Holtmann]

Hi Andres,

> 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.

this is not an acceptable policy. You are generating false positives now
for drivers that have been fine before.

Regards

Marcel

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

[Andres Salomon]

On Tue, 16 Feb 2010 13:21:19 -0800
Marcel Holtmann <marcel@holtmann.org> wrote:

> Hi Andres,
> 
> > 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.
> 
> this is not an acceptable policy. You are generating false positives
> now for drivers that have been fine before.
> 

Unacceptable how?  It's a warning, not a requirement.  It's
telling the user to double-check their config.

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

[Marcel Holtmann]

Hi Andres,

> > > 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.
> > 
> > this is not an acceptable policy. You are generating false positives
> > now for drivers that have been fine before.
> > 
> 
> Unacceptable how?  It's a warning, not a requirement.  It's
> telling the user to double-check their config.

and why not either whitelist drivers with their own rate control or have
the ones without rate control depend on a selection. You can't really
expect all users to know which driver comes with rate control and which
doesn't.

Regards

Marcel

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

[Andres Salomon]

On Tue, 16 Feb 2010 15:01:39 -0800
Marcel Holtmann <marcel@holtmann.org> wrote:

> Hi Andres,
> 
> > > > 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.
> > > 
> > > this is not an acceptable policy. You are generating false
> > > positives now for drivers that have been fine before.
> > > 
> > 
> > Unacceptable how?  It's a warning, not a requirement.  It's
> > telling the user to double-check their config.
> 
> and why not either whitelist drivers with their own rate control or
> have the ones without rate control depend on a selection. You can't
> really expect all users to know which driver comes with rate control
> and which doesn't.
> 

Two reasons:

1) It's not clear which drivers should be whitelisted.  *No* driver sets
IEEE80211_HW_HAS_RATE_CONTROL. Some drivers (ath9k, iwlagn, iwl3945)
call ieee80211_rate_control_register().  That's a pretty incomplete
list.

2) Here's a scenario to consider: via menuconfig, a user configures
net/mac80211 (that section comes before drivers/net/wireless); there's
no warning, because iwlagn is already selected in drivers/net/wireless.
They don't select any RC algorithms, and then proceed to
drivers/net/wireless, where they select rtl8187 (maybe they use both,
or maybe iwlagn was just selected as the default, or maybe from an
old .config; who knows?). Rtl8187 requires an RC algorithm, but there's
no warning displayed to the user that RC algorithms aren't being built,
so the user then builds a broken rtl8187 driver.  Maybe the rtl8187
'help' section should be warning the user, or maybe the current warning
should be moved over to drivers/net/wireless/Kconfig, or maybe the
rtl8187 driver should just forcefully select an RC algorithm if none is
selected (which is frowned upon).  This kind of problem is not fixed by
a simplistic whitelist, though.


I'm more than happy to come up w/ a Kconfig framework for drivers to
opt into, stating that they don't require RC, but that's something that
needs wider discussion, feedback from driver authors, and potentially
changes to drivers as well.  In the meantime, a simplistic warning
(suggesting to the user a *safe* choice) is better than nothing, and
hurts no one.

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

[Holger Schurig]

> I discovered that if EMBEDDED=y, one can accidentally build a
> mac80211 stack and drivers w/ no rate control algorithm.

I think this is fine. I'm working with embedded devices (AVR32 &
ARM) as well. And I usually turn on CONFIG_EMBEDDED=Y because I
want to really fine-tune my kernel to my device.

And therefore I know my hardware.

And because I know my hardware, I can decide on my own if I need 
rate ctrl or not.

-- 
http://www.holgerschurig.de