[RFC] WARNING: at net/mac80211/ieee80211_rate.h:159 rate_lowest_index()

[RFC] WARNING: at net/mac80211/ieee80211_rate.h:159 rate_lowest_index()

[Vladimir Koutny]

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

Hi,
the warning in $subj can pop up in the following scenario:

- iwconfig wlan0 mode ad-hoc; ifconfig wlan0 up
- a frame arrives from an unknown station which triggers
  ieee80211_ibss_sta_add() (called from prepare_for_handlers())

In this case, we create a new sta entry, fill in supp_rates field,
initialize rate_control algo, and insert this sta entry into the list.

The problem is the assignment for supp_rates:

sta->supp_rates[local->hw.conf.channel->band] =
	sdata->u.sta.supp_rates_bits[local->hw.conf.channel->band];

because sdata->u.sta.supp_rates_bits is set only when we actually
join an ibss, which is not the case in this scenario; so we assign
zero here. Then, the code in rate_control_pid_rate_init() calls
rate_lowest_index() for some reason, and since no supported rate is
found, this warning is issued.


The question is how sta->supp_rates should be initialized:

- we could initialize it to our sta's rates, but then we could
  probably transmit to a station at unsupported rate

- add new ibss station only on received beacon, not on a data frame;
  currently, beacons are ignored for this purpose (they just update
  the bss list later on)

- something else (like 1Mbps only)?

As soon as we do sta_join_ibss(), this warning won't happen again.


Note that once we have sta entry (in our ibss), we will update its
supported rates based on beacons it sends (in rx_bss_info()).

Any thoughts?

Regards,
  Vlado


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 370 bytes --]

Re: [RFC] WARNING: at net/mac80211/ieee80211_rate.h:159 rate_lowest_index()

[bruno randolf]

On Tuesday 29 April 2008 18:01:22 Vladimir Koutny wrote:
> Hi,
> the warning in $subj can pop up in the following scenario:
>
> - iwconfig wlan0 mode ad-hoc; ifconfig wlan0 up
> - a frame arrives from an unknown station which triggers
>   ieee80211_ibss_sta_add() (called from prepare_for_handlers())

yep, i have seen this warning too, just didn't get around to look into it...

> In this case, we create a new sta entry, fill in supp_rates field,
> initialize rate_control algo, and insert this sta entry into the list.
>
> The problem is the assignment for supp_rates:
>
> sta->supp_rates[local->hw.conf.channel->band] =
> 	sdata->u.sta.supp_rates_bits[local->hw.conf.channel->band];
>
> because sdata->u.sta.supp_rates_bits is set only when we actually
> join an ibss, which is not the case in this scenario; so we assign
> zero here. Then, the code in rate_control_pid_rate_init() calls
> rate_lowest_index() for some reason, and since no supported rate is
> found, this warning is issued.
>
>
> The question is how sta->supp_rates should be initialized:
>
> - we could initialize it to our sta's rates, but then we could
>   probably transmit to a station at unsupported rate

isn't this what is done right now, and the rateset is zero sometimes and then 
we get the warning?

this might be wrong anyways: as you said it could make us send frames at an 
unsupported rate.

> - add new ibss station only on received beacon, not on a data frame;
>   currently, beacons are ignored for this purpose (they just update
>   the bss list later on)

i think stations should be added on reception of both beacons and data frames.

> - something else (like 1Mbps only)?

what about the rate of the currently received data frame (and maybe any other 
rates we could safely deduce from that)?

bruno

Re: [RFC] WARNING: at net/mac80211/ieee80211_rate.h:159 rate_lowest_index()

[Vladimir Koutny]

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

>> [...]
>>
>> The question is how sta->supp_rates should be initialized:
>>
>> - we could initialize it to our sta's rates, but then we could
>>   probably transmit to a station at unsupported rate
> 
> isn't this what is done right now, and the rateset is zero sometimes and then 
> we get the warning?

Yes, kind-of. It is initialized to the rateset of an ibss we are part of,
which is the one of ibss we are joining to, or our supported rateset if we
create a new one. However, it is 0 before one of those 2 actions happen, and
then we get the warning.

> this might be wrong anyways: as you said it could make us send frames at an 
> unsupported rate.

Actually, this can happen even now:
- we join an g-ibss, thus setting the rateset to 1-54
- we receive a data frame from a b-only station -> we assign all 1-54 rates to it
- we send something to this station
Probably not a big issue, as a) we will update the sta-rateset on its beacon,
and b) I would expect that such an ibss would adapt to 1-11 only in the
presence of b-only station..

> 
>> - add new ibss station only on received beacon, not on a data frame;
>>   currently, beacons are ignored for this purpose (they just update
>>   the bss list later on)
> 
> i think stations should be added on reception of both beacons and data frames.

Or, do we need this information _before_ joining/creating a new ibss?
Couldn't we just ignore all STAs around until we have a reason not to?

> 
>> - something else (like 1Mbps only)?
> 
> what about the rate of the currently received data frame (and maybe any other 
> rates we could safely deduce from that)?

Yes, that would be a reasonable option - providing that we have the rate
(which we should - there is a WARN_ON(1) in rx-path if it is not set correctly)

As I think about that, I would suggest:
- no sta info is being collected prior to join/create
- sta entry is added/updated on each beacon with reported rateset
- sta entry is added/updated on data frames with just the received-rate

Vlado

> 
> bruno


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 370 bytes --]

Re: [RFC] WARNING: at net/mac80211/ieee80211_rate.h:159 rate_lowest_index()

[Johannes Berg]

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


> Yes, that would be a reasonable option - providing that we have the rate
> (which we should - there is a WARN_ON(1) in rx-path if it is not set correctly)

the packet is also dropped in that case so you don't see it.

> As I think about that, I would suggest:
> - no sta info is being collected prior to join/create
> - sta entry is added/updated on each beacon with reported rateset
> - sta entry is added/updated on data frames with just the received-rate

Want to do the patch?

johannes

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

Re: [RFC] WARNING: at net/mac80211/ieee80211_rate.h:159 rate_lowest_index()

[Vladimir Koutny]

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

Johannes Berg wrote:
>> Yes, that would be a reasonable option - providing that we have the rate
>> (which we should - there is a WARN_ON(1) in rx-path if it is not set correctly)
> 
> the packet is also dropped in that case so you don't see it.
Oh yes. Should have read one more line of code :)

> 
>> As I think about that, I would suggest:
>> - no sta info is being collected prior to join/create
>> - sta entry is added/updated on each beacon with reported rateset
>> - sta entry is added/updated on data frames with just the received-rate
> 
> Want to do the patch?

I'll try. Hope we have rx-rate on hand where needed :)

Vlado


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 370 bytes --]

Re: [RFC] WARNING: at net/mac80211/ieee80211_rate.h:159 rate_lowest_index()

[Johannes Berg]

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


> because sdata->u.sta.supp_rates_bits is set only when we actually
> join an ibss, which is not the case in this scenario; so we assign
> zero here. Then, the code in rate_control_pid_rate_init() calls
> rate_lowest_index() for some reason, and since no supported rate is
> found, this warning is issued.
> 
> 
> The question is how sta->supp_rates should be initialized:
> 
> - we could initialize it to our sta's rates, but then we could
>   probably transmit to a station at unsupported rate
> 
> - add new ibss station only on received beacon, not on a data frame;
>   currently, beacons are ignored for this purpose (they just update
>   the bss list later on)
> 
> - something else (like 1Mbps only)?
> 
> As soon as we do sta_join_ibss(), this warning won't happen again.
> 
> 
> Note that once we have sta entry (in our ibss), we will update its
> supported rates based on beacons it sends (in rx_bss_info()).
> 
> Any thoughts?

I don't know. I don't use IBSS nor am I intimately familiar with the
specs for it.

johannes

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