| Message ID | 20201111183359.43528-2-nbd@nbd.name (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | [5.10,1/3] mac80211: fix memory leak on filtered powersave frames | expand |
Hi,
I tracked down an issue with using the Kernel 4.9.277 and 5.4.131 back
to this patch.
In 802.11bg mode the effective transmit rate drops to 3MBit/s.
With that patch and perfect SNR (> 45dB) I get and rc_stats:
*********************
best __________rate_________ ________statistics________
________last_______ ______sum-of________
rate [name idx airtime max_tp] [avg(tp) avg(prob) sd(prob)]
[prob.|retry|suc|att] [#success | #attempts]
1 0 9738 0.9 0.9 93.7 0.5 75.0
1 3 4 562 572
2 1 4922 1.6 1.6 97.0 1.5 100.0
1 2 2 559 576
5.5 2 1858 4.7 4.7 99.9 0.0 100.0
2 2 2 542 565
11 3 982 9.1 9.1 94.9 1.2 100.0
4 3 3 560 578
6 4 1648 5.3 5.3 95.3 1.3 100.0
3 3 3 538 595
9 5 1112 8.0 8.0 97.5 1.8 100.0
4 2 2 560 596
12 6 844 10.5 10.5 92.4 2.0 75.0
5 3 4 572 615
18 7 576 15.5 15.4 89.0 1.4 66.6
5 2 3 559 609
D 24 8 440 20.4 20.4 99.4 1.1 100.0
6 2 2 560 604
C 36 9 308 29.1 29.1 98.7 1.6 100.0
6 2 2 565 600
B 48 10 240 37.3 36.3 87.3 1.4 66.6
6 2 3 608 671
A P 54 11 216 41.6 41.6 97.3 1.7 100.0
6 3 3 565 620
Total packet count:: ideal 6111 lookaround 652
*********************
After reverting that patch the transmit rate is back to 20MBit/s:
*********************
best __________rate_________ ________statistics________
________last_______ ______sum-of________
rate [name idx airtime max_tp] [avg(tp) avg(prob) sd(prob)]
[prob.|retry|suc|att] [#success | #attempts]
1 0 9738 0.9 0.9 99.8 0.6 100.0
1 0 0 41 44
2 1 4922 1.6 1.6 100.0 0.0 100.0
1 0 0 45 45
5.5 2 1858 4.7 4.7 99.9 0.0 100.0
2 0 0 43 44
11 3 982 9.1 9.1 97.0 1.4 100.0
4 0 0 44 45
6 4 1648 5.3 5.3 99.9 0.0 100.0
3 0 0 43 44
9 5 1112 8.0 8.0 99.9 0.0 100.0
4 0 0 45 47
12 6 844 10.5 8.8 74.9 0.7 0.0
5 0 0 43 47
18 7 576 15.5 15.5 100.0 0.0 100.0
5 0 0 42 42
D 24 8 440 20.4 20.4 99.9 0.0 100.0
6 0 0 60 62
C P 36 9 308 29.1 29.1 99.9 0.0 100.0
6 0 0 44 46
A 48 10 240 37.3 37.3 93.8 1.2 100.0
6 1 1 587 618
B 54 11 216 41.6 32.3 69.8 2.1 33.3
6 0 0 24506 25849
Total packet count:: ideal 4962 lookaround 552
*********************
I'm using the ath9k driver with a sparklan WPEA-121N card.
For my case reverting that patch is enough.
What I see is that IEEE80211_TX_CTL_RATE_CTRL_PROBE is set but not used
anywhere in the code. Maybe there is an easy and better fix than just
reverting.
Regards,
Thomas
Am 11.11.2020 um 19:33 schrieb Felix Fietkau:
> Deferring sampling attempts to the second stage has some bad interactions
> with drivers that process the rate table in hardware and use the probe flag
> to indicate probing packets (e.g. most mt76 drivers). On affected drivers
> it can lead to probing not working at all.
>
> If the link conditions turn worse, it might not be such a good idea to
> do a lot of sampling for lower rates in this case.
>
> Fix this by simply skipping the sample attempt instead of deferring it,
> but keep the checks that would allow it to be sampled if it was skipped
> too often, but only if it has less than 95% success probability.
>
> Also ensure that IEEE80211_TX_CTL_RATE_CTRL_PROBE is set for all probing
> packets.
>
> Cc: stable@vger.kernel.org
> Fixes: cccf129f820e ("mac80211: add the 'minstrel' rate control algorithm")
> Signed-off-by: Felix Fietkau <nbd@nbd.name>
> ---
> net/mac80211/rc80211_minstrel.c | 25 ++++---------------------
> net/mac80211/rc80211_minstrel.h | 1 -
> 2 files changed, 4 insertions(+), 22 deletions(-)
>
> diff --git a/net/mac80211/rc80211_minstrel.c b/net/mac80211/rc80211_minstrel.c
> index 86bc469a28bc..a8a940e97a9a 100644
> --- a/net/mac80211/rc80211_minstrel.c
> +++ b/net/mac80211/rc80211_minstrel.c
> @@ -287,12 +287,6 @@ minstrel_tx_status(void *priv, struct ieee80211_supported_band *sband,
> mi->r[ndx].stats.success += success;
> }
>
> - if ((info->flags & IEEE80211_TX_CTL_RATE_CTRL_PROBE) && (i >= 0))
> - mi->sample_packets++;
> -
> - if (mi->sample_deferred > 0)
> - mi->sample_deferred--;
> -
> if (time_after(jiffies, mi->last_stats_update +
> mp->update_interval / (mp->new_avg ? 2 : 1)))
> minstrel_update_stats(mp, mi);
> @@ -367,7 +361,7 @@ minstrel_get_rate(void *priv, struct ieee80211_sta *sta,
> return;
>
> delta = (mi->total_packets * sampling_ratio / 100) -
> - (mi->sample_packets + mi->sample_deferred / 2);
> + mi->sample_packets;
>
> /* delta < 0: no sampling required */
> prev_sample = mi->prev_sample;
> @@ -376,7 +370,6 @@ minstrel_get_rate(void *priv, struct ieee80211_sta *sta,
> return;
>
> if (mi->total_packets >= 10000) {
> - mi->sample_deferred = 0;
> mi->sample_packets = 0;
> mi->total_packets = 0;
> } else if (delta > mi->n_rates * 2) {
> @@ -401,19 +394,8 @@ minstrel_get_rate(void *priv, struct ieee80211_sta *sta,
> * rate sampling method should be used.
> * Respect such rates that are not sampled for 20 interations.
> */
> - if (mrr_capable &&
> - msr->perfect_tx_time > mr->perfect_tx_time &&
> - msr->stats.sample_skipped < 20) {
> - /* Only use IEEE80211_TX_CTL_RATE_CTRL_PROBE to mark
> - * packets that have the sampling rate deferred to the
> - * second MRR stage. Increase the sample counter only
> - * if the deferred sample rate was actually used.
> - * Use the sample_deferred counter to make sure that
> - * the sampling is not done in large bursts */
> - info->flags |= IEEE80211_TX_CTL_RATE_CTRL_PROBE;
> - rate++;
> - mi->sample_deferred++;
> - } else {
> + if (msr->perfect_tx_time < mr->perfect_tx_time ||
> + msr->stats.sample_skipped >= 20) {
> if (!msr->sample_limit)
> return;
>
> @@ -433,6 +415,7 @@ minstrel_get_rate(void *priv, struct ieee80211_sta *sta,
>
> rate->idx = mi->r[ndx].rix;
> rate->count = minstrel_get_retry_count(&mi->r[ndx], info);
> + info->flags |= IEEE80211_TX_CTL_RATE_CTRL_PROBE;
> }
>
>
> diff --git a/net/mac80211/rc80211_minstrel.h b/net/mac80211/rc80211_minstrel.h
> index dbb43bcd3c45..86cd80b3ffde 100644
> --- a/net/mac80211/rc80211_minstrel.h
> +++ b/net/mac80211/rc80211_minstrel.h
> @@ -126,7 +126,6 @@ struct minstrel_sta_info {
> u8 max_prob_rate;
> unsigned int total_packets;
> unsigned int sample_packets;
> - int sample_deferred;
>
> unsigned int sample_row;
> unsigned int sample_column;
>
Thomas Graf <post@thomas-graf.de> writes: > I tracked down an issue with using the Kernel 4.9.277 and 5.4.131 back > to this patch. > > In 802.11bg mode the effective transmit rate drops to 3MBit/s. > With that patch and perfect SNR (> 45dB) I get and rc_stats: > > ********************* > best __________rate_________ ________statistics________ > ________last_______ ______sum-of________ > rate [name idx airtime max_tp] [avg(tp) avg(prob) sd(prob)] > [prob.|retry|suc|att] [#success | #attempts] > 1 0 9738 0.9 0.9 93.7 0.5 75.0 > 1 3 4 562 572 > 2 1 4922 1.6 1.6 97.0 1.5 100.0 > 1 2 2 559 576 > 5.5 2 1858 4.7 4.7 99.9 0.0 100.0 > 2 2 2 542 565 > 11 3 982 9.1 9.1 94.9 1.2 100.0 > 4 3 3 560 578 > 6 4 1648 5.3 5.3 95.3 1.3 100.0 > 3 3 3 538 595 > 9 5 1112 8.0 8.0 97.5 1.8 100.0 > 4 2 2 560 596 > 12 6 844 10.5 10.5 92.4 2.0 75.0 > 5 3 4 572 615 > 18 7 576 15.5 15.4 89.0 1.4 66.6 > 5 2 3 559 609 > D 24 8 440 20.4 20.4 99.4 1.1 100.0 > 6 2 2 560 604 > C 36 9 308 29.1 29.1 98.7 1.6 100.0 > 6 2 2 565 600 > B 48 10 240 37.3 36.3 87.3 1.4 66.6 > 6 2 3 608 671 > A P 54 11 216 41.6 41.6 97.3 1.7 100.0 > 6 3 3 565 620 > > Total packet count:: ideal 6111 lookaround 652 > ********************* > > After reverting that patch the transmit rate is back to 20MBit/s: > ********************* > best __________rate_________ ________statistics________ > ________last_______ ______sum-of________ > rate [name idx airtime max_tp] [avg(tp) avg(prob) sd(prob)] > [prob.|retry|suc|att] [#success | #attempts] > 1 0 9738 0.9 0.9 99.8 0.6 100.0 > 1 0 0 41 44 > 2 1 4922 1.6 1.6 100.0 0.0 100.0 > 1 0 0 45 45 > 5.5 2 1858 4.7 4.7 99.9 0.0 100.0 > 2 0 0 43 44 > 11 3 982 9.1 9.1 97.0 1.4 100.0 > 4 0 0 44 45 > 6 4 1648 5.3 5.3 99.9 0.0 100.0 > 3 0 0 43 44 > 9 5 1112 8.0 8.0 99.9 0.0 100.0 > 4 0 0 45 47 > 12 6 844 10.5 8.8 74.9 0.7 0.0 > 5 0 0 43 47 > 18 7 576 15.5 15.5 100.0 0.0 100.0 > 5 0 0 42 42 > D 24 8 440 20.4 20.4 99.9 0.0 100.0 > 6 0 0 60 62 > C P 36 9 308 29.1 29.1 99.9 0.0 100.0 > 6 0 0 44 46 > A 48 10 240 37.3 37.3 93.8 1.2 100.0 > 6 1 1 587 618 > B 54 11 216 41.6 32.3 69.8 2.1 33.3 > 6 0 0 24506 25849 > > Total packet count:: ideal 4962 lookaround 552 > ********************* > > I'm using the ath9k driver with a sparklan WPEA-121N card. > > For my case reverting that patch is enough. > What I see is that IEEE80211_TX_CTL_RATE_CTRL_PROBE is set but not > used anywhere in the code. Maybe there is an easy and better fix than > just reverting. I lost track, is this regression resolved now or is it still open?
diff --git a/net/mac80211/rc80211_minstrel.c b/net/mac80211/rc80211_minstrel.c index 86bc469a28bc..a8a940e97a9a 100644 --- a/net/mac80211/rc80211_minstrel.c +++ b/net/mac80211/rc80211_minstrel.c @@ -287,12 +287,6 @@ minstrel_tx_status(void *priv, struct ieee80211_supported_band *sband, mi->r[ndx].stats.success += success; } - if ((info->flags & IEEE80211_TX_CTL_RATE_CTRL_PROBE) && (i >= 0)) - mi->sample_packets++; - - if (mi->sample_deferred > 0) - mi->sample_deferred--; - if (time_after(jiffies, mi->last_stats_update + mp->update_interval / (mp->new_avg ? 2 : 1))) minstrel_update_stats(mp, mi); @@ -367,7 +361,7 @@ minstrel_get_rate(void *priv, struct ieee80211_sta *sta, return; delta = (mi->total_packets * sampling_ratio / 100) - - (mi->sample_packets + mi->sample_deferred / 2); + mi->sample_packets; /* delta < 0: no sampling required */ prev_sample = mi->prev_sample; @@ -376,7 +370,6 @@ minstrel_get_rate(void *priv, struct ieee80211_sta *sta, return; if (mi->total_packets >= 10000) { - mi->sample_deferred = 0; mi->sample_packets = 0; mi->total_packets = 0; } else if (delta > mi->n_rates * 2) { @@ -401,19 +394,8 @@ minstrel_get_rate(void *priv, struct ieee80211_sta *sta, * rate sampling method should be used. * Respect such rates that are not sampled for 20 interations. */ - if (mrr_capable && - msr->perfect_tx_time > mr->perfect_tx_time && - msr->stats.sample_skipped < 20) { - /* Only use IEEE80211_TX_CTL_RATE_CTRL_PROBE to mark - * packets that have the sampling rate deferred to the - * second MRR stage. Increase the sample counter only - * if the deferred sample rate was actually used. - * Use the sample_deferred counter to make sure that - * the sampling is not done in large bursts */ - info->flags |= IEEE80211_TX_CTL_RATE_CTRL_PROBE; - rate++; - mi->sample_deferred++; - } else { + if (msr->perfect_tx_time < mr->perfect_tx_time || + msr->stats.sample_skipped >= 20) { if (!msr->sample_limit) return; @@ -433,6 +415,7 @@ minstrel_get_rate(void *priv, struct ieee80211_sta *sta, rate->idx = mi->r[ndx].rix; rate->count = minstrel_get_retry_count(&mi->r[ndx], info); + info->flags |= IEEE80211_TX_CTL_RATE_CTRL_PROBE; } diff --git a/net/mac80211/rc80211_minstrel.h b/net/mac80211/rc80211_minstrel.h index dbb43bcd3c45..86cd80b3ffde 100644 --- a/net/mac80211/rc80211_minstrel.h +++ b/net/mac80211/rc80211_minstrel.h @@ -126,7 +126,6 @@ struct minstrel_sta_info { u8 max_prob_rate; unsigned int total_packets; unsigned int sample_packets; - int sample_deferred; unsigned int sample_row; unsigned int sample_column;
Deferring sampling attempts to the second stage has some bad interactions with drivers that process the rate table in hardware and use the probe flag to indicate probing packets (e.g. most mt76 drivers). On affected drivers it can lead to probing not working at all. If the link conditions turn worse, it might not be such a good idea to do a lot of sampling for lower rates in this case. Fix this by simply skipping the sample attempt instead of deferring it, but keep the checks that would allow it to be sampled if it was skipped too often, but only if it has less than 95% success probability. Also ensure that IEEE80211_TX_CTL_RATE_CTRL_PROBE is set for all probing packets. Cc: stable@vger.kernel.org Fixes: cccf129f820e ("mac80211: add the 'minstrel' rate control algorithm") Signed-off-by: Felix Fietkau <nbd@nbd.name> --- net/mac80211/rc80211_minstrel.c | 25 ++++--------------------- net/mac80211/rc80211_minstrel.h | 1 - 2 files changed, 4 insertions(+), 22 deletions(-)