Re: [wireless-next PATCH 3/5] wifi: Allow overriding some HT information.

[Johannes Berg <johannes@sipsolutions.net>]

On Wed, 2011-11-02 at 09:59 -0700, Ben Greear wrote:

> > No. Neither of these can ever be increased so it's not that simple. And
> > making it smaller is always possible since it's just advertising.
> > Presumably you do understand the reasons for this advertising/these
> > restrictions?
> 
> It seems that a driver might default to a mid-range value for the MPDU values
> because it is somehow 'better' for whatever reason, and yet it might still
> support larger values if the user desires, perhaps because in specific
> scenarios larger values are better.  Ath9k does set to the max value,
> so if we do a per-driver capabilities flag as I did in v2 then we
> are safe.

Then the proper way to do that would be to not have a flag, but a max it
can be set to. However, I see no reason it should default to a mid-range
value?! iwlwifi for example needs to allocate enough space but ... I
don't get it. What's wrong with simply not allowing to increase, only
decrease?

> >>          /* add attributes here, update the policy in nl80211.c */
> 
> I copied some of that code from nl80211_set_station, which appears to
> also forget to check the length for the NL80211_ATTR_HT_CAPABILITY
> object.  Is there some reason why it doesn't need to check, or does
> that code need fixing as well?

NL80211_ATTR_HT_CAPABILITY in particular *has* a policy entry.

> Well, it's more obvious now.  Do you also need to check the length when
> doing code like this code from nl80211_set_station:
> 
> 	if (info->attrs[NL80211_ATTR_STA_LISTEN_INTERVAL])
> 		params.listen_interval =
> 		    nla_get_u16(info->attrs[NL80211_ATTR_STA_LISTEN_INTERVAL]);
> 
> In other words, is it safe to assume you have 16 bits, or could a broken
> user-space pass in a single byte and screw this up?

There's the policy that validates it.


>  From 20.1.1 of the 802.11n spec:
> 
> "An HT non-AP STA shall support all equal modulation (EQM) rates for one spatial stream (MCSs 0 through
> 7) using 20 MHz channel width. An HT AP shall support all EQM rates for one and two spatial streams
> (MCSs 0 through 15) using 20 MHz channel width."
> 
> That is why I wrote that code as I did, but perhaps I misunderstand that section of
> the docs.

No, that makes some sense, I wasn't aware of that restriction.

johannes