On 5/15/23 11:43, Jernej Škrabec wrote:
Dne sobota, 13. maj 2023 ob 23:21:47 CEST je Larry Finger napisal(a):
On 5/13/23 15:13, Jernej Škrabec wrote:
Dne sobota, 13. maj 2023 ob 21:55:51 CEST je Larry Finger napisal(a):
On 5/13/23 05:23, Jernej Škrabec wrote:
Larry,
Dne sreda, 10. maj 2023 ob 23:47:24 CEST je Larry Finger napisal(a):
On 5/10/23 16:07, Martin Blumenstingl wrote:
On Wed, May 10, 2023 at 12:02 AM Larry Finger <Larry.Finger@xxxxxxxxxxxx> wrote:
[...]
I added that patch to the driver. The user reports that he was able to do a ping
and an nslookup before it crashed with the following in the log:
That's some positive news alongside the crash log: it seems that a
part of the driver works! :-)
[ 8.700626] skbuff: skb_over_panic: text:ffff8000011924ac len:3341 put:3341
head:ffff000003b3c000 data:ffff000003b3c040 tail:0xd4d end:0x2c0 dev:<NULL>
[...]
Somehow skb->tail was greater than skb->end. Unfortunately I do not have access
to gdb to tell you what line corresponds to rtw_sdio_rx_skb+0x50 on the MangoPi
MQ Quad.
I need to have a closer look at the pkg_offset and struct
rtw_rx_pkt_stat which we receive.
Recently my own MangoPI MQ-Quad arrived but I did not have the time to
set it up yet. I'll try to do so during the weekend so I can debug
this on my own.
Please ping me next week in case I haven't provided any update until then.
I have some test prints in to check for skb overrun. My initial indication is
that the problem was in the c2h branch of rtw_sdio_rx_skb(), but my next run
should verify that. My changes will do a pr_warn_once() when the problem
happens, and then drop the skb.
My contact reported that he had one run of 3 minutes before the problem
happened, which is good news for most of the driver.
I may have discovered something interesting. rtl8723ds vendor driver has
following checks in RX data parsing code:
https://github.com/lwfinger/rtl8723ds/blob/master/hal/rtl8723d/sdio/rtl8723ds_recv.c#L83-L99
Those checks are absent in rtl8822bs vendor driver, which was my original
development platform for SDIO. This may indicate some kind of bug in FW
and/or HW.
I think that at least second check, which checks for exactly the case your
client experience, can be easily added and tested.
Thanks for this update. I added the following to the start of rtw_sdio_rx_skb():
/* fix Hardware RX data error, drop whole recv_buffer */
if (!(rtwdev->hal.rcr & BIT_ACRC32) && pkt_stat->crc_err) {
kfree_skb(skb);
return;
}
I think that duplicates the code in the vendor driver.
I have not heard from my user as to whether it helps. My communications with him
are at https://github.com/lwfinger/rtl8723ds/issues/37.
I had second part in mind (see attachment), but this is IMO only sanity check
and it will mask the issue. At this point I'm not sure if this is something that
can happen occasionally or is there additional bug in rtw88 code. I'll check
rtl8723ds c2h code in greater detail.
In any case, I would argue that all 3 patches in this thread are valid and
should be submitted upstream.
That patch looks good. I ill apply it to my rtw88 repo.
I see that issue is still there. Next idea would be to check if RX aggregation
is the problem. Just commenting out call to rtw_sdio_enable_rx_aggregation()
is enough to disable it.
Jernej,
With aggregation disabled, we still get "Invalid RX packet size!" messages. I am
changing the statement to log (curr_pkt_len + pkt_desc_sz) > rx_len. I will let
you know when the OP responds.
Larry
