2021-10-28 22:33:12

by Benjamin Li

[permalink] [raw]
Subject: [PATCH 1/2] wcn36xx: populate band before determining rate on RX

status.band is used in determination of status.rate -- for 5GHz on legacy
rates there is a linear shift between the BD descriptor's rate field and
the wcn36xx driver's rate table (wcn_5ghz_rates).

We have a special clause to populate status.band for hardware scan offload
frames. However, this block occurs after status.rate is already populated.
Correctly handle this dependency by moving the band block before the rate
block.

This patch addresses kernel warnings & missing scan results for 5GHz APs
that send their probe responses at the higher four legacy rates (24-54
Mbps), when using hardware scan offload:

------------[ cut here ]------------
WARNING: CPU: 0 PID: 0 at net/mac80211/rx.c:4532 ieee80211_rx_napi+0x744/0x8d8
Modules linked in: wcn36xx [...]
CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 4.19.107-g73909fa #1
Hardware name: Square, Inc. T2 (all variants) (DT)
Call trace:
dump_backtrace+0x0/0x148
show_stack+0x14/0x1c
dump_stack+0xb8/0xf0
__warn+0x2ac/0x2d8
warn_slowpath_null+0x44/0x54
ieee80211_rx_napi+0x744/0x8d8
ieee80211_tasklet_handler+0xa4/0xe0
tasklet_action_common+0xe0/0x118
tasklet_action+0x20/0x28
__do_softirq+0x108/0x1ec
irq_exit+0xd4/0xd8
__handle_domain_irq+0x84/0xbc
gic_handle_irq+0x4c/0xb8
el1_irq+0xe8/0x190
lpm_cpuidle_enter+0x220/0x260
cpuidle_enter_state+0x114/0x1c0
cpuidle_enter+0x34/0x48
do_idle+0x150/0x268
cpu_startup_entry+0x20/0x24
rest_init+0xd4/0xe0
start_kernel+0x398/0x430
---[ end trace ae28cb759352b403 ]---

Fixes: 8a27ca394782 ("wcn36xx: Correct band/freq reporting on RX")
Signed-off-by: Benjamin Li <[email protected]>
---
drivers/net/wireless/ath/wcn36xx/txrx.c | 37 +++++++++++++------------
1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/txrx.c b/drivers/net/wireless/ath/wcn36xx/txrx.c
index 75951ccbc840e..f0a9f069a92a9 100644
--- a/drivers/net/wireless/ath/wcn36xx/txrx.c
+++ b/drivers/net/wireless/ath/wcn36xx/txrx.c
@@ -314,8 +314,6 @@ int wcn36xx_rx_skb(struct wcn36xx *wcn, struct sk_buff *skb)
fc = __le16_to_cpu(hdr->frame_control);
sn = IEEE80211_SEQ_TO_SN(__le16_to_cpu(hdr->seq_ctrl));

- status.freq = WCN36XX_CENTER_FREQ(wcn);
- status.band = WCN36XX_BAND(wcn);
status.mactime = 10;
status.signal = -get_rssi0(bd);
status.antenna = 1;
@@ -327,6 +325,25 @@ int wcn36xx_rx_skb(struct wcn36xx *wcn, struct sk_buff *skb)

wcn36xx_dbg(WCN36XX_DBG_RX, "status.flags=%x\n", status.flag);

+ if (bd->scan_learn) {
+ /* If packet originate from hardware scanning, extract the
+ * band/channel from bd descriptor.
+ */
+ u8 hwch = (bd->reserved0 << 4) + bd->rx_ch;
+
+ if (bd->rf_band != 1 && hwch <= sizeof(ab_rx_ch_map) && hwch >= 1) {
+ status.band = NL80211_BAND_5GHZ;
+ status.freq = ieee80211_channel_to_frequency(ab_rx_ch_map[hwch - 1],
+ status.band);
+ } else {
+ status.band = NL80211_BAND_2GHZ;
+ status.freq = ieee80211_channel_to_frequency(hwch, status.band);
+ }
+ } else {
+ status.band = WCN36XX_BAND(wcn);
+ status.freq = WCN36XX_CENTER_FREQ(wcn);
+ }
+
if (bd->rate_id < ARRAY_SIZE(wcn36xx_rate_table)) {
rate = &wcn36xx_rate_table[bd->rate_id];
status.encoding = rate->encoding;
@@ -353,22 +370,6 @@ int wcn36xx_rx_skb(struct wcn36xx *wcn, struct sk_buff *skb)
ieee80211_is_probe_resp(hdr->frame_control))
status.boottime_ns = ktime_get_boottime_ns();

- if (bd->scan_learn) {
- /* If packet originates from hardware scanning, extract the
- * band/channel from bd descriptor.
- */
- u8 hwch = (bd->reserved0 << 4) + bd->rx_ch;
-
- if (bd->rf_band != 1 && hwch <= sizeof(ab_rx_ch_map) && hwch >= 1) {
- status.band = NL80211_BAND_5GHZ;
- status.freq = ieee80211_channel_to_frequency(ab_rx_ch_map[hwch - 1],
- status.band);
- } else {
- status.band = NL80211_BAND_2GHZ;
- status.freq = ieee80211_channel_to_frequency(hwch, status.band);
- }
- }
-
memcpy(IEEE80211_SKB_RXCB(skb), &status, sizeof(status));

if (ieee80211_is_beacon(hdr->frame_control)) {
--
2.25.1


2021-10-28 22:33:24

by Benjamin Li

[permalink] [raw]
Subject: [PATCH 2/2] wcn36xx: fix RX BD rate mapping for 5GHz legacy rates

The linear mapping between the BD rate field and the driver's 5GHz
legacy rates table (wcn_5ghz_rates) does not only apply for the latter
four rates -- it applies to all eight rates.

Fixes: 6ea131acea98 ("wcn36xx: Fix warning due to bad rate_idx")
Signed-off-by: Benjamin Li <[email protected]>
---
drivers/net/wireless/ath/wcn36xx/txrx.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/txrx.c b/drivers/net/wireless/ath/wcn36xx/txrx.c
index f0a9f069a92a9..b4a36acdaca74 100644
--- a/drivers/net/wireless/ath/wcn36xx/txrx.c
+++ b/drivers/net/wireless/ath/wcn36xx/txrx.c
@@ -354,8 +354,7 @@ int wcn36xx_rx_skb(struct wcn36xx *wcn, struct sk_buff *skb)
status.nss = 1;

if (status.band == NL80211_BAND_5GHZ &&
- status.encoding == RX_ENC_LEGACY &&
- status.rate_idx >= sband->n_bitrates) {
+ status.encoding == RX_ENC_LEGACY) {
/* no dsss rates in 5Ghz rates table */
status.rate_idx -= 4;
}
--
2.25.1

2021-10-29 00:28:47

by Bryan O'Donoghue

[permalink] [raw]
Subject: Re: [PATCH 2/2] wcn36xx: fix RX BD rate mapping for 5GHz legacy rates

On 28/10/2021 23:31, Benjamin Li wrote:
> - status.rate_idx >= sband->n_bitrates) {
This fix was applied because we were getting a negative index

If you want to remove that, you'll need to do something about this

status.rate_idx -= 4;

---
bod

2021-10-29 00:41:38

by Benjamin Li

[permalink] [raw]
Subject: Re: [PATCH 2/2] wcn36xx: fix RX BD rate mapping for 5GHz legacy rates

On 10/28/21 5:30 PM, Bryan O'Donoghue wrote:
> On 28/10/2021 23:31, Benjamin Li wrote:
>> -            status.rate_idx >= sband->n_bitrates) {
> This fix was applied because we were getting a negative index
>
> If you want to remove that, you'll need to do something about this
>
> status.rate_idx -= 4;

Hmm... so you're saying there's a FW bug where sometimes we get
bd->rate_id = 0-7 (leading to status.rate_idx = 0-3) on a 5GHz
channel?

static const struct wcn36xx_rate wcn36xx_rate_table[] = {
/* 11b rates */
{ 10, 0, RX_ENC_LEGACY, 0, RATE_INFO_BW_20 },
{ 20, 1, RX_ENC_LEGACY, 0, RATE_INFO_BW_20 },
{ 55, 2, RX_ENC_LEGACY, 0, RATE_INFO_BW_20 },
{ 110, 3, RX_ENC_LEGACY, 0, RATE_INFO_BW_20 },

/* 11b SP (short preamble) */
{ 10, 0, RX_ENC_LEGACY, RX_ENC_FLAG_SHORTPRE, RATE_INFO_BW_20 },
{ 20, 1, RX_ENC_LEGACY, RX_ENC_FLAG_SHORTPRE, RATE_INFO_BW_20 },
{ 55, 2, RX_ENC_LEGACY, RX_ENC_FLAG_SHORTPRE, RATE_INFO_BW_20 },
{ 110, 3, RX_ENC_LEGACY, RX_ENC_FLAG_SHORTPRE, RATE_INFO_BW_20 },

It sounds like we should WARN and drop the frame in that case. If
you agree I'll send a v2.

>
> ---
> bod

2021-10-29 01:12:24

by Bryan O'Donoghue

[permalink] [raw]
Subject: Re: [PATCH 2/2] wcn36xx: fix RX BD rate mapping for 5GHz legacy rates

On 29/10/2021 01:39, Benjamin Li wrote:
> On 10/28/21 5:30 PM, Bryan O'Donoghue wrote:
>> On 28/10/2021 23:31, Benjamin Li wrote:
>>> -            status.rate_idx >= sband->n_bitrates) {
>> This fix was applied because we were getting a negative index
>>
>> If you want to remove that, you'll need to do something about this
>>
>> status.rate_idx -= 4;
>
> Hmm... so you're saying there's a FW bug where sometimes we get
> bd->rate_id = 0-7 (leading to status.rate_idx = 0-3) on a 5GHz
> channel?

My memory is I saw a negative index as a result of the -4 offset but, it
is quite some time ago and we have made all sorts of changes since.

> static const struct wcn36xx_rate wcn36xx_rate_table[] = {
> /* 11b rates */
> { 10, 0, RX_ENC_LEGACY, 0, RATE_INFO_BW_20 },
> { 20, 1, RX_ENC_LEGACY, 0, RATE_INFO_BW_20 },
> { 55, 2, RX_ENC_LEGACY, 0, RATE_INFO_BW_20 },
> { 110, 3, RX_ENC_LEGACY, 0, RATE_INFO_BW_20 },
>
> /* 11b SP (short preamble) */
> { 10, 0, RX_ENC_LEGACY, RX_ENC_FLAG_SHORTPRE, RATE_INFO_BW_20 },
> { 20, 1, RX_ENC_LEGACY, RX_ENC_FLAG_SHORTPRE, RATE_INFO_BW_20 },
> { 55, 2, RX_ENC_LEGACY, RX_ENC_FLAG_SHORTPRE, RATE_INFO_BW_20 },
> { 110, 3, RX_ENC_LEGACY, RX_ENC_FLAG_SHORTPRE, RATE_INFO_BW_20 },
>
> It sounds like we should WARN and drop the frame in that case. If
> you agree I'll send a v2.

So,

Let me see if I can replicate the previous bug - tomorrow - later this
morning in fact - in this timezone, and I'll get back to you.

---
bod

2021-10-29 18:31:48

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 2/2] wcn36xx: fix RX BD rate mapping for 5GHz legacy rates

Hi Benjamin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on kvalo-wireless-drivers-next/master]
[also build test WARNING on kvalo-ath/ath-next kvalo-wireless-drivers/master v5.15-rc7 next-20211029]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Benjamin-Li/wcn36xx-populate-band-before-determining-rate-on-RX/20211029-064020
base: https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/4713a80ea03fc60eaa4de959a3ec73154493f35a
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Benjamin-Li/wcn36xx-populate-band-before-determining-rate-on-RX/20211029-064020
git checkout 4713a80ea03fc60eaa4de959a3ec73154493f35a
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=arm64

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

drivers/net/wireless/ath/wcn36xx/txrx.c: In function 'wcn36xx_rx_skb':
>> drivers/net/wireless/ath/wcn36xx/txrx.c:275:42: warning: variable 'sband' set but not used [-Wunused-but-set-variable]
275 | struct ieee80211_supported_band *sband;
| ^~~~~


vim +/sband +275 drivers/net/wireless/ath/wcn36xx/txrx.c

a224b47ab36d7d Loic Poulain 2021-10-25 268
8e84c25821698b Eugene Krasnikov 2013-10-08 269 int wcn36xx_rx_skb(struct wcn36xx *wcn, struct sk_buff *skb)
8e84c25821698b Eugene Krasnikov 2013-10-08 270 {
8e84c25821698b Eugene Krasnikov 2013-10-08 271 struct ieee80211_rx_status status;
0aa90483f23e79 Loic Poulain 2020-06-15 272 const struct wcn36xx_rate *rate;
8e84c25821698b Eugene Krasnikov 2013-10-08 273 struct ieee80211_hdr *hdr;
8e84c25821698b Eugene Krasnikov 2013-10-08 274 struct wcn36xx_rx_bd *bd;
6ea131acea9802 Loic Poulain 2020-08-29 @275 struct ieee80211_supported_band *sband;
8e84c25821698b Eugene Krasnikov 2013-10-08 276 u16 fc, sn;
8e84c25821698b Eugene Krasnikov 2013-10-08 277
8e84c25821698b Eugene Krasnikov 2013-10-08 278 /*
8e84c25821698b Eugene Krasnikov 2013-10-08 279 * All fields must be 0, otherwise it can lead to
8e84c25821698b Eugene Krasnikov 2013-10-08 280 * unexpected consequences.
8e84c25821698b Eugene Krasnikov 2013-10-08 281 */
8e84c25821698b Eugene Krasnikov 2013-10-08 282 memset(&status, 0, sizeof(status));
8e84c25821698b Eugene Krasnikov 2013-10-08 283
8e84c25821698b Eugene Krasnikov 2013-10-08 284 bd = (struct wcn36xx_rx_bd *)skb->data;
8e84c25821698b Eugene Krasnikov 2013-10-08 285 buff_to_be((u32 *)bd, sizeof(*bd)/sizeof(u32));
8e84c25821698b Eugene Krasnikov 2013-10-08 286 wcn36xx_dbg_dump(WCN36XX_DBG_RX_DUMP,
8e84c25821698b Eugene Krasnikov 2013-10-08 287 "BD <<< ", (char *)bd,
8e84c25821698b Eugene Krasnikov 2013-10-08 288 sizeof(struct wcn36xx_rx_bd));
8e84c25821698b Eugene Krasnikov 2013-10-08 289
a224b47ab36d7d Loic Poulain 2021-10-25 290 if (bd->pdu.mpdu_data_off <= bd->pdu.mpdu_header_off ||
a224b47ab36d7d Loic Poulain 2021-10-25 291 bd->pdu.mpdu_len < bd->pdu.mpdu_header_len)
a224b47ab36d7d Loic Poulain 2021-10-25 292 goto drop;
a224b47ab36d7d Loic Poulain 2021-10-25 293
a224b47ab36d7d Loic Poulain 2021-10-25 294 if (bd->asf && !bd->esf) { /* chained A-MSDU chunks */
a224b47ab36d7d Loic Poulain 2021-10-25 295 /* Sanity check */
a224b47ab36d7d Loic Poulain 2021-10-25 296 if (bd->pdu.mpdu_data_off + bd->pdu.mpdu_len > WCN36XX_PKT_SIZE)
a224b47ab36d7d Loic Poulain 2021-10-25 297 goto drop;
a224b47ab36d7d Loic Poulain 2021-10-25 298
a224b47ab36d7d Loic Poulain 2021-10-25 299 skb_put(skb, bd->pdu.mpdu_data_off + bd->pdu.mpdu_len);
a224b47ab36d7d Loic Poulain 2021-10-25 300 skb_pull(skb, bd->pdu.mpdu_data_off);
a224b47ab36d7d Loic Poulain 2021-10-25 301
a224b47ab36d7d Loic Poulain 2021-10-25 302 /* Only set status for first chained BD (with mac header) */
a224b47ab36d7d Loic Poulain 2021-10-25 303 goto done;
a224b47ab36d7d Loic Poulain 2021-10-25 304 }
a224b47ab36d7d Loic Poulain 2021-10-25 305
a224b47ab36d7d Loic Poulain 2021-10-25 306 if (bd->pdu.mpdu_header_off < sizeof(*bd) ||
a224b47ab36d7d Loic Poulain 2021-10-25 307 bd->pdu.mpdu_header_off + bd->pdu.mpdu_len > WCN36XX_PKT_SIZE)
a224b47ab36d7d Loic Poulain 2021-10-25 308 goto drop;
a224b47ab36d7d Loic Poulain 2021-10-25 309
8e84c25821698b Eugene Krasnikov 2013-10-08 310 skb_put(skb, bd->pdu.mpdu_header_off + bd->pdu.mpdu_len);
8e84c25821698b Eugene Krasnikov 2013-10-08 311 skb_pull(skb, bd->pdu.mpdu_header_off);
8e84c25821698b Eugene Krasnikov 2013-10-08 312
886039036c2004 Bjorn Andersson 2017-01-11 313 hdr = (struct ieee80211_hdr *) skb->data;
886039036c2004 Bjorn Andersson 2017-01-11 314 fc = __le16_to_cpu(hdr->frame_control);
886039036c2004 Bjorn Andersson 2017-01-11 315 sn = IEEE80211_SEQ_TO_SN(__le16_to_cpu(hdr->seq_ctrl));
886039036c2004 Bjorn Andersson 2017-01-11 316
886039036c2004 Bjorn Andersson 2017-01-11 317 status.mactime = 10;
8e84c25821698b Eugene Krasnikov 2013-10-08 318 status.signal = -get_rssi0(bd);
8e84c25821698b Eugene Krasnikov 2013-10-08 319 status.antenna = 1;
8e84c25821698b Eugene Krasnikov 2013-10-08 320 status.flag = 0;
8e84c25821698b Eugene Krasnikov 2013-10-08 321 status.rx_flags = 0;
8e84c25821698b Eugene Krasnikov 2013-10-08 322 status.flag |= RX_FLAG_IV_STRIPPED |
8e84c25821698b Eugene Krasnikov 2013-10-08 323 RX_FLAG_MMIC_STRIPPED |
8e84c25821698b Eugene Krasnikov 2013-10-08 324 RX_FLAG_DECRYPTED;
8e84c25821698b Eugene Krasnikov 2013-10-08 325
7fdd69c5af2160 Johannes Berg 2017-04-26 326 wcn36xx_dbg(WCN36XX_DBG_RX, "status.flags=%x\n", status.flag);
8e84c25821698b Eugene Krasnikov 2013-10-08 327
cec59cdeb543bd Benjamin Li 2021-10-28 328 if (bd->scan_learn) {
cec59cdeb543bd Benjamin Li 2021-10-28 329 /* If packet originate from hardware scanning, extract the
cec59cdeb543bd Benjamin Li 2021-10-28 330 * band/channel from bd descriptor.
cec59cdeb543bd Benjamin Li 2021-10-28 331 */
cec59cdeb543bd Benjamin Li 2021-10-28 332 u8 hwch = (bd->reserved0 << 4) + bd->rx_ch;
cec59cdeb543bd Benjamin Li 2021-10-28 333
cec59cdeb543bd Benjamin Li 2021-10-28 334 if (bd->rf_band != 1 && hwch <= sizeof(ab_rx_ch_map) && hwch >= 1) {
cec59cdeb543bd Benjamin Li 2021-10-28 335 status.band = NL80211_BAND_5GHZ;
cec59cdeb543bd Benjamin Li 2021-10-28 336 status.freq = ieee80211_channel_to_frequency(ab_rx_ch_map[hwch - 1],
cec59cdeb543bd Benjamin Li 2021-10-28 337 status.band);
cec59cdeb543bd Benjamin Li 2021-10-28 338 } else {
cec59cdeb543bd Benjamin Li 2021-10-28 339 status.band = NL80211_BAND_2GHZ;
cec59cdeb543bd Benjamin Li 2021-10-28 340 status.freq = ieee80211_channel_to_frequency(hwch, status.band);
cec59cdeb543bd Benjamin Li 2021-10-28 341 }
cec59cdeb543bd Benjamin Li 2021-10-28 342 } else {
cec59cdeb543bd Benjamin Li 2021-10-28 343 status.band = WCN36XX_BAND(wcn);
cec59cdeb543bd Benjamin Li 2021-10-28 344 status.freq = WCN36XX_CENTER_FREQ(wcn);
cec59cdeb543bd Benjamin Li 2021-10-28 345 }
cec59cdeb543bd Benjamin Li 2021-10-28 346
0aa90483f23e79 Loic Poulain 2020-06-15 347 if (bd->rate_id < ARRAY_SIZE(wcn36xx_rate_table)) {
0aa90483f23e79 Loic Poulain 2020-06-15 348 rate = &wcn36xx_rate_table[bd->rate_id];
0aa90483f23e79 Loic Poulain 2020-06-15 349 status.encoding = rate->encoding;
0aa90483f23e79 Loic Poulain 2020-06-15 350 status.enc_flags = rate->encoding_flags;
0aa90483f23e79 Loic Poulain 2020-06-15 351 status.bw = rate->bw;
0aa90483f23e79 Loic Poulain 2020-06-15 352 status.rate_idx = rate->mcs_or_legacy_index;
6ea131acea9802 Loic Poulain 2020-08-29 353 sband = wcn->hw->wiphy->bands[status.band];
1af05d43b9bef4 Bryan O'Donoghue 2020-08-29 354 status.nss = 1;
6ea131acea9802 Loic Poulain 2020-08-29 355
6ea131acea9802 Loic Poulain 2020-08-29 356 if (status.band == NL80211_BAND_5GHZ &&
4713a80ea03fc6 Benjamin Li 2021-10-28 357 status.encoding == RX_ENC_LEGACY) {
6ea131acea9802 Loic Poulain 2020-08-29 358 /* no dsss rates in 5Ghz rates table */
6ea131acea9802 Loic Poulain 2020-08-29 359 status.rate_idx -= 4;
6ea131acea9802 Loic Poulain 2020-08-29 360 }
0aa90483f23e79 Loic Poulain 2020-06-15 361 } else {
0aa90483f23e79 Loic Poulain 2020-06-15 362 status.encoding = 0;
0aa90483f23e79 Loic Poulain 2020-06-15 363 status.bw = 0;
0aa90483f23e79 Loic Poulain 2020-06-15 364 status.enc_flags = 0;
0aa90483f23e79 Loic Poulain 2020-06-15 365 status.rate_idx = 0;
0aa90483f23e79 Loic Poulain 2020-06-15 366 }
0aa90483f23e79 Loic Poulain 2020-06-15 367
8678fd31f2d3eb Loic Poulain 2021-08-26 368 if (ieee80211_is_beacon(hdr->frame_control) ||
8678fd31f2d3eb Loic Poulain 2021-08-26 369 ieee80211_is_probe_resp(hdr->frame_control))
8678fd31f2d3eb Loic Poulain 2021-08-26 370 status.boottime_ns = ktime_get_boottime_ns();
8678fd31f2d3eb Loic Poulain 2021-08-26 371
8e84c25821698b Eugene Krasnikov 2013-10-08 372 memcpy(IEEE80211_SKB_RXCB(skb), &status, sizeof(status));
8e84c25821698b Eugene Krasnikov 2013-10-08 373
8e84c25821698b Eugene Krasnikov 2013-10-08 374 if (ieee80211_is_beacon(hdr->frame_control)) {
8e84c25821698b Eugene Krasnikov 2013-10-08 375 wcn36xx_dbg(WCN36XX_DBG_BEACON, "beacon skb %p len %d fc %04x sn %d\n",
8e84c25821698b Eugene Krasnikov 2013-10-08 376 skb, skb->len, fc, sn);
8e84c25821698b Eugene Krasnikov 2013-10-08 377 wcn36xx_dbg_dump(WCN36XX_DBG_BEACON_DUMP, "SKB <<< ",
8e84c25821698b Eugene Krasnikov 2013-10-08 378 (char *)skb->data, skb->len);
8e84c25821698b Eugene Krasnikov 2013-10-08 379 } else {
8e84c25821698b Eugene Krasnikov 2013-10-08 380 wcn36xx_dbg(WCN36XX_DBG_RX, "rx skb %p len %d fc %04x sn %d\n",
8e84c25821698b Eugene Krasnikov 2013-10-08 381 skb, skb->len, fc, sn);
8e84c25821698b Eugene Krasnikov 2013-10-08 382 wcn36xx_dbg_dump(WCN36XX_DBG_RX_DUMP, "SKB <<< ",
8e84c25821698b Eugene Krasnikov 2013-10-08 383 (char *)skb->data, skb->len);
8e84c25821698b Eugene Krasnikov 2013-10-08 384 }
8e84c25821698b Eugene Krasnikov 2013-10-08 385
a224b47ab36d7d Loic Poulain 2021-10-25 386 done:
a224b47ab36d7d Loic Poulain 2021-10-25 387 /* Chained AMSDU ? slow path */
a224b47ab36d7d Loic Poulain 2021-10-25 388 if (unlikely(bd->asf && !(bd->lsf && bd->esf))) {
a224b47ab36d7d Loic Poulain 2021-10-25 389 if (bd->esf && !skb_queue_empty(&wcn->amsdu)) {
a224b47ab36d7d Loic Poulain 2021-10-25 390 wcn36xx_err("Discarding non complete chain");
a224b47ab36d7d Loic Poulain 2021-10-25 391 __skb_queue_purge_irq(&wcn->amsdu);
a224b47ab36d7d Loic Poulain 2021-10-25 392 }
a224b47ab36d7d Loic Poulain 2021-10-25 393
a224b47ab36d7d Loic Poulain 2021-10-25 394 __skb_queue_tail(&wcn->amsdu, skb);
a224b47ab36d7d Loic Poulain 2021-10-25 395
a224b47ab36d7d Loic Poulain 2021-10-25 396 if (!bd->lsf)
a224b47ab36d7d Loic Poulain 2021-10-25 397 return 0; /* Not the last AMSDU, wait for more */
a224b47ab36d7d Loic Poulain 2021-10-25 398
a224b47ab36d7d Loic Poulain 2021-10-25 399 skb = wcn36xx_unchain_msdu(&wcn->amsdu);
a224b47ab36d7d Loic Poulain 2021-10-25 400 if (!skb)
a224b47ab36d7d Loic Poulain 2021-10-25 401 goto drop;
a224b47ab36d7d Loic Poulain 2021-10-25 402 }
a224b47ab36d7d Loic Poulain 2021-10-25 403
8e84c25821698b Eugene Krasnikov 2013-10-08 404 ieee80211_rx_irqsafe(wcn->hw, skb);
8e84c25821698b Eugene Krasnikov 2013-10-08 405
8e84c25821698b Eugene Krasnikov 2013-10-08 406 return 0;
a224b47ab36d7d Loic Poulain 2021-10-25 407

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (12.71 kB)
.config.gz (55.10 kB)
Download all attachments

2021-11-01 13:03:25

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 2/2] wcn36xx: fix RX BD rate mapping for 5GHz legacy rates

Benjamin Li <[email protected]> writes:

> On 10/28/21 5:30 PM, Bryan O'Donoghue wrote:
>> On 28/10/2021 23:31, Benjamin Li wrote:
>>> -            status.rate_idx >= sband->n_bitrates) {
>> This fix was applied because we were getting a negative index
>>
>> If you want to remove that, you'll need to do something about this
>>
>> status.rate_idx -= 4;
>
> Hmm... so you're saying there's a FW bug where sometimes we get
> bd->rate_id = 0-7 (leading to status.rate_idx = 0-3) on a 5GHz
> channel?
>
> static const struct wcn36xx_rate wcn36xx_rate_table[] = {
> /* 11b rates */
> { 10, 0, RX_ENC_LEGACY, 0, RATE_INFO_BW_20 },
> { 20, 1, RX_ENC_LEGACY, 0, RATE_INFO_BW_20 },
> { 55, 2, RX_ENC_LEGACY, 0, RATE_INFO_BW_20 },
> { 110, 3, RX_ENC_LEGACY, 0, RATE_INFO_BW_20 },
>
> /* 11b SP (short preamble) */
> { 10, 0, RX_ENC_LEGACY, RX_ENC_FLAG_SHORTPRE, RATE_INFO_BW_20 },
> { 20, 1, RX_ENC_LEGACY, RX_ENC_FLAG_SHORTPRE, RATE_INFO_BW_20 },
> { 55, 2, RX_ENC_LEGACY, RX_ENC_FLAG_SHORTPRE, RATE_INFO_BW_20 },
> { 110, 3, RX_ENC_LEGACY, RX_ENC_FLAG_SHORTPRE, RATE_INFO_BW_20 },
>
> It sounds like we should WARN and drop the frame in that case. If
> you agree I'll send a v2.

BTW, please avoid using WARN() family of functions in the data path as
that can cause host crashes due to too much spamming in the logs. A some
kind of ratelimited version of an error message is much safer. For
example ath11k_warn() is ratelimited, maybe wcn36xx_warn() should be as
well?

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

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