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

[Jérôme Pouiller]

On Wednesday 4 October 2023 14:57:19 CEST Kalle Valo wrote:
> Jérôme Pouiller <jerome.pouiller@xxxxxxxxxx> writes:
> 
> > From: Felipe Negrelli Wolter <felipe.negrelliwolter@xxxxxxxxxx>
> >
> > 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@xxxxxxxxxx>
> > 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@xxxxxxxxxx>
> > Signed-off-by: Jérôme Pouiller <jerome.pouiller@xxxxxxxxxx>
> > Signed-off-by: Felipe Negrelli Wolter <felipe.negrelliwolter@xxxxxxxxxx>
> > ---
> > 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.

I am on top of v6.6-rc3. I get:

    $ ./scripts/checkpatch.pl -g HEAD^..HEAD
    WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
    #26:
                      best    ____________rate__________    ____statistics___    _____last____    ______sum-of________

    WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
    #57:
    Reported-by: Alexandr Suslenko <suslenko.o@ajax.systems>
    Co-developed-by: Jérôme Pouiller <jerome.pouiller@xxxxxxxxxx>
    [...]

-- 
Jérôme Pouiller