RE: wifi: rtw88: question about SDIO RX aggregation limiting

[Ping-Ke Shih]

> -----Original Message-----
> From: Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx>
> Sent: Tuesday, July 4, 2023 5:25 AM
> To: Ping-Ke Shih <pkshih@xxxxxxxxxxx>
> Cc: linux-wireless@xxxxxxxxxxxxxxx; Lukas F. Hartmann <lukas@xxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx;
> tony0620emma@xxxxxxxxx; jernej.skrabec@xxxxxxxxx
> Subject: Re: wifi: rtw88: question about SDIO RX aggregation limiting
> 
> Hello Ping-Ke,
> 
> sorry again for the long waiting time. I'll be quicker next time.
> 
> On Tue, Jun 20, 2023 at 7:26 AM Ping-Ke Shih <pkshih@xxxxxxxxxxx> wrote:
> [...]
> > > > The unit of BIT_RXDMA_AGG_PG_TH is 1k bytes, so I think you can
> > > > set mmc_host->max_req_size/1024.
> > > I tried this but I got a result that I don't understand.
> > > I've been testing with three BIT_RXDMA_AGG_PG_TH values on a SoC that
> > > can handle 255 * 1024 bytes. Each time I connected to the same AP and
> > > downloaded a bigger file over http(s).
> > > BIT_RXDMA_AGG_PG_TH: biggest observed rx_len in rtw_sdio_rxfifo_recv()
> > > 255: 20968
> > > 6: 5122
> > > 1: 1602
> >
> > Please also print out number of packets you receive, and then we can see how
> > many packets aggregate.
> sure - here are the results:
> BIT_RXDMA_AGG_PG_TH: biggest observed rx_len in rtw_sdio_rxfifo_recv()
> / number of (aggregated) packets
> 255: 20824 / 12
> 6: 5128 / 4
> 1: 3132 / 1 (these were a few exceptions and I'm not able to reliably
> reproduce it, 1602 / 1 is what I can easily reproduce)

These results are expected. 

The minimum number of received packet is one, so it is possible that packet size
is larger than 1k.

> 
> > > The biggest rx_len I have observed for BIT_RXDMA_AGG_PG_TH 1 looks suspicious:
> > > My understanding is that I shouldn't be seeing rx_len larger than
> > > BIT_RXDMA_AGG_PG_TH * 1024.
> > > BIT_RXDMA_AGG_PG_TH = 6 is within this limit but BIT_RXDMA_AGG_PG_TH =
> > > 1 isn't (I'm seeing 578 extra bytes in addition to the 1024 bytes that
> > > I was expecting).
> >
> > Assume threshold is 1k, and single one packet is larger than 1k. Hardware
> > will not split it into two. Also, please make sure 0x280[29] BIT_EN_PRE_CALC
> > is 1. Otherwise, it will possibly aggregate additional one packet to over
> > the threshold.
> From the numbers above it seems most likely that we're hitting the
> "one packet is larger than 1k" case.
> 
> Also I'm seeing:
>   wlan0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue
> state UP group default qlen 1000
> My interface's MTU is 1500 bytes. Seeing 1602 bytes rx_len with one
> packet is already odd (that would mean 102 bytes for overhead like RX
> descriptor and other headers/metadata). But 3132 bytes rx_len is very
> odd.

I think MTU only affects transmitting packets. Would you mind to try to
set peer's MTU to 1000 or lower?

For received packets, driver is the first entry, so maybe net stack can
shrink packet size to fit MTU, not sure though.

The large 3132-byte packet is also very odd to me. Try to dump raw data
and check the content that packet size is larger than 1500.


An idea from my colleague. Is it possible to multiple read_port for a certain
receiving? Like below pseudo code: (I don't define '* host')

@@ -502,17 +502,26 @@ static int rtw_sdio_read_port(struct rtw_dev *rtwdev, u8 *buf, size_t count)
        struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
        bool bus_claim = rtw_sdio_bus_claim_needed(rtwsdio);
        u32 rxaddr = rtwsdio->rx_addr++;
+       size_t n;
        int ret;

        if (bus_claim)
                sdio_claim_host(rtwsdio->sdio_func);

-       ret = sdio_memcpy_fromio(rtwsdio->sdio_func, buf,
-                                RTW_SDIO_ADDR_RX_RX0FF_GEN(rxaddr), count);
-       if (ret)
-               rtw_warn(rtwdev,
-                        "Failed to read %zu byte(s) from SDIO port 0x%08x",
-                        count, rxaddr);
+       while (count > 0) {
+               n = min(host->max_req_size, count);
+
+               ret = sdio_memcpy_fromio(rtwsdio->sdio_func, buf,
+                                        RTW_SDIO_ADDR_RX_RX0FF_GEN(rxaddr), n);
+               if (ret)
+                       rtw_warn(rtwdev,
+                                "Failed to read %zu byte(s) from SDIO port 0x%08x",
+                                n, rxaddr);
+
+               count -= n;
+               buf += n;
+       }
+

        if (bus_claim)
                sdio_release_host(rtwsdio->sdio_func);


Ping-Ke