Re: [PATCH v2] wifi: wfx: fix case where rates are out of order

[Kalle Valo <kvalo@kernel.org>]

Jérôme Pouiller <jerome.pouiller@silabs.com> writes:

> From: Felipe Negrelli Wolter <felipe.negrelliwolter@silabs.com>
>
> When frames are sent over the air, the device always applies the data
> rates in descending order. The driver assumed Minstrel also provided
> rate in descending order.
>
> However, in some cases, Minstrel can a choose a fallback rate greater
> than the primary rate. In this case, the two rates was inverted, the
> device try highest rate first and we get many retries.
>
> Since the device always applies rates in descending order, the
> workaround is to drop the rate when it higher than its predecessor in
> the rate list. Thus [ 4, 5, 3 ] becomes [ 4, 3 ].
>
> This patch has been tested in isolated room with a series of
> attenuators. Here are the Minstrel statistics with 80dBm of attenuation:
>
>   Without the fix:
>
>                   best    ____________rate__________    ____statistics___    _____last____    ______sum-of________
>     mode guard #  rate   [name   idx airtime  max_tp]  [avg(tp) avg(prob)]  [retry|suc|att]  [#success | #attempts]
>     HT20  LGI  1       S  MCS0     0    1477     5.6       5.2      82.7       3     0 0             3   4
>     HT20  LGI  1          MCS1     1     738    10.6       0.0       0.0       0     0 0             0   1
>     HT20  LGI  1     D    MCS2     2     492    14.9      13.5      81.5       5     0 0             5   9
>     HT20  LGI  1    C     MCS3     3     369    18.8      17.6      84.3       5     0 0            76   96
>     HT20  LGI  1  A   P   MCS4     4     246    25.4      22.4      79.5       5     0 0         11268   14026
>     HT20  LGI  1   B   S  MCS5     5     185    30.7      19.7      57.7       5     8 9          3918   9793
>     HT20  LGI  1          MCS6     6     164    33.0       0.0       0.0       5     0 0             6   102
>     HT20  LGI  1          MCS7     7     148    35.1       0.0       0.0       0     0 0             0   44
>
>   With the fix:
>
>                   best    ____________rate__________    ____statistics___    _____last____    ______sum-of________
>     mode guard #  rate   [name   idx airtime  max_tp]  [avg(tp) avg(prob)]  [retry|suc|att]  [#success | #attempts]
>     HT20  LGI  1       S  MCS0     0    1477     5.6       1.8      28.6       1     0 0             1   5
>     HT20  LGI  1     DP   MCS1     1     738    10.6       9.7      82.6       4     0 0            14   34
>     HT20  LGI  1          MCS2     2     492    14.9       9.2      55.4       5     0 0            52   77
>     HT20  LGI  1   B   S  MCS3     3     369    18.8      15.6      74.9       5     1 1           417   554
>     HT20  LGI  1  A       MCS4     4     246    25.4      16.7      59.2       5     1 1         13812   17951
>     HT20  LGI  1    C  S  MCS5     5     185    30.7      14.0      41.0       5     1 5            57   640
>     HT20  LGI  1          MCS6     6     164    33.0       0.0       0.0       0     0 1             0   48
>     HT20  LGI  1       S  MCS7     7     148    35.1       0.0       0.0       0     0 0             0   36
>
> We can notice the device try now to send with lower rates (and high
> success rates). At the end, we measured 20-25% better throughput with
> this patch.
>
> Fixes: 9bca45f3d692 ("staging: wfx: allow to send 802.11 frames")
> Tested-by: Olivier Souloumiac <olivier.souloumiac@silabs.com>
> Tested-by: Alexandr Suslenko <suslenko.o@ajax.systems>
> Reported-by: Alexandr Suslenko <suslenko.o@ajax.systems>
> Co-developed-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
> Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
> Signed-off-by: Felipe Negrelli Wolter <felipe.negrelliwolter@silabs.com>
> ---
> v2:
>   - Fix malformed tags in commit body. (checkpatch still complains about
>     missing Close tag, but the bug tracker is not public and I don't have
>     the exact URL)

Just out of curiosity why does the checkpatch complain about a missing
Close tag? I don't get it why there should be one.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches