rt2800, minstrel_ht & mac80211

rt2800, minstrel_ht & mac80211

[Helmut Schaa]

Hi,

While working on throughput problems with some mcs rates on rt2800 cards I've
noticed a strange issue between rt2800, minstrel_ht and mac80211.

Despite the fact that rt2800 always reported MCS15 as _not_ successful (and
it was really never successful), minstrel_ht's rc_stats always claimed that
the attempt was successful.

rt2800 has a global rate fallback table (something like MCS15 -> MCS14 ->
MCS13 ...), so it is capable of trying a frame at different rates but we
cannot specify the rates to be used for every single frame. Hence, rt2800
does not set hw->max_rates and thus ends up with max_rates = 1 but since
rt2800 will try different rates it will fill the tx status with all the rates
it really tried.

Unfortunately, mac80211 (status.c) decides to drop all reported rates >=
max_rates and thus we end up with minstrel_ht thinking that MCS15 was
successful while rt2800 reported that it tried MCS15,14,13 and 12 was
finally successful.

This meant that minstrel most of the time selected MCS15 as tx rate which
resulted in a huge number of retries and thus a very slow throughput.

status.c:

178                 /* the HW cannot have attempted that rate */
179                 if (i >= hw->max_rates) {
180                         info->status.rates[i].idx = -1;
181                         info->status.rates[i].count = 0;

Johannes, is there a valid reason for this check in status.c. Can we remove
it without breaking anything else?

What other options would we have to fix this issue? Just use max_rates = 4 
in rt2800 even if we will only use the first rate provided by minstrel and
then fall back to the global fallback setup (I tried that already and it
works just fine but seems not correct to me)?

Thanks,
Helmut

Re: rt2800, minstrel_ht & mac80211

[Johannes Berg]

On Fri, 2010-06-04 at 12:22 +0200, Helmut Schaa wrote:

> This meant that minstrel most of the time selected MCS15 as tx rate which
> resulted in a huge number of retries and thus a very slow throughput.
> 
> status.c:
> 
> 178                 /* the HW cannot have attempted that rate */
> 179                 if (i >= hw->max_rates) {
> 180                         info->status.rates[i].idx = -1;
> 181                         info->status.rates[i].count = 0;
> 
> Johannes, is there a valid reason for this check in status.c. Can we remove
> it without breaking anything else?

IIRC there were/are some drivers that were not terminating the rates[]
array correctly with a -1, thus this code.

> What other options would we have to fix this issue? Just use max_rates = 4 
> in rt2800 even if we will only use the first rate provided by minstrel and
> then fall back to the global fallback setup (I tried that already and it
> works just fine but seems not correct to me)?

p54 kinda does some trickery in this area too, not really sure what to
do though.

johannes

Re: rt2800, minstrel_ht & mac80211

[Helmut Schaa]

Am Freitag 04 Juni 2010 schrieb Johannes Berg:
> On Fri, 2010-06-04 at 12:22 +0200, Helmut Schaa wrote:
> 
> > This meant that minstrel most of the time selected MCS15 as tx rate which
> > resulted in a huge number of retries and thus a very slow throughput.
> > 
> > status.c:
> > 
> > 178                 /* the HW cannot have attempted that rate */
> > 179                 if (i >= hw->max_rates) {
> > 180                         info->status.rates[i].idx = -1;
> > 181                         info->status.rates[i].count = 0;
> > 
> > Johannes, is there a valid reason for this check in status.c. Can we remove
> > it without breaking anything else?
> 
> IIRC there were/are some drivers that were not terminating the rates[]
> array correctly with a -1, thus this code.

Understood, so it's a driver workaround ;)

Maybe we should check which driver doesn't set it and fix these?

> > What other options would we have to fix this issue? Just use max_rates = 4 
> > in rt2800 even if we will only use the first rate provided by minstrel and
> > then fall back to the global fallback setup (I tried that already and it
> > works just fine but seems not correct to me)?
> 
> p54 kinda does some trickery in this area too, not really sure what to
> do though.

Thanks for the pointer, p54 just sets max_rates=4 even if it cannot support
it.

So, if you would like to keep the check I'll just use max_rates=4 in rt2800.

Helmut