Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <CAFBinCBaXtebixKbjkWKW_WXc5k=NdGNaGUjVE8NCPNxOhsb2g@mail.gmail.com>
 <9ab8cc85d4d440bfa63dcade4e4f9ecf@realtek.com> <CAFBinCBsg8jPhpqSOr9w2JhwN5YjPeME1Uye7meSY8h=b_N4Qg@mail.gmail.com>
 <e87abbe35f4945cba3440232880424b1@realtek.com>
In-Reply-To: <e87abbe35f4945cba3440232880424b1@realtek.com>
From:   Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Date:   Mon, 3 Jul 2023 23:25:22 +0200
Message-ID: <CAFBinCB6Q67nG3Z33i=nDLcYGENCSHFKNq=ViJks0j9aXbcYpA@mail.gmail.com>
Subject: Re: wifi: rtw88: question about SDIO RX aggregation limiting
To:     Ping-Ke Shih <pkshih@realtek.com>
Cc:     "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
        "Lukas F. Hartmann" <lukas@mntre.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "tony0620emma@gmail.com" <tony0620emma@gmail.com>,
        "jernej.skrabec@gmail.com" <jernej.skrabec@gmail.com>
Content-Type: multipart/mixed; boundary="000000000000824ae805ff9bcecc"
Precedence: bulk

--000000000000824ae805ff9bcecc
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hello Ping-Ke,

sorry again for the long waiting time. I'll be quicker next time.

On Tue, Jun 20, 2023 at 7:26=E2=80=AFAM Ping-Ke Shih <pkshih@realtek.com> w=
rote:
[...]
> > > 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)

> > The biggest rx_len I have observed for BIT_RXDMA_AGG_PG_TH 1 looks susp=
icious:
> > My understanding is that I shouldn't be seeing rx_len larger than
> > BIT_RXDMA_AGG_PG_TH * 1024.
> > BIT_RXDMA_AGG_PG_TH =3D 6 is within this limit but BIT_RXDMA_AGG_PG_TH =
=3D
> > 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_C=
ALC
> 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.

BIT_EN_PRE_CALC is set, see also the attached diff (it's not meant to
be applied anywhere - it's just so you understand what I've been
testing with).

> 0x280[15:8] is timeout time in unit of 1us for SDIO interface. When set
> threshold to 255, you can enlarge this to see if it can aggregate more as
> expected.
I did not experiment with this yet as I'd like to understand the above
findings first.


Best regards,
Martin

--000000000000824ae805ff9bcecc
Content-Type: text/x-patch; charset="US-ASCII"; name="rtw88-rx-agg-test.diff"
Content-Disposition: attachment; filename="rtw88-rx-agg-test.diff"
Content-Transfer-Encoding: base64
Content-ID: <f_ljncq4dc0>
X-Attachment-Id: f_ljncq4dc0

ZGlmZiAtLWdpdCBhL2RyaXZlcnMvbmV0L3dpcmVsZXNzL3JlYWx0ZWsvcnR3ODgvc2Rpby5jIGIv
ZHJpdmVycy9uZXQvd2lyZWxlc3MvcmVhbHRlay9ydHc4OC9zZGlvLmMKaW5kZXggMmMxZmIyZGFi
ZDQwLi4wYmI4NzJlZjEyOTYgMTAwNjQ0Ci0tLSBhL2RyaXZlcnMvbmV0L3dpcmVsZXNzL3JlYWx0
ZWsvcnR3ODgvc2Rpby5jCisrKyBiL2RyaXZlcnMvbmV0L3dpcmVsZXNzL3JlYWx0ZWsvcnR3ODgv
c2Rpby5jCkBAIC02NTQsMjMgKzY1NCwzNSBAQCBzdGF0aWMgdm9pZCBydHdfc2Rpb19pbml0KHN0
cnVjdCBydHdfZGV2ICpydHdkZXYpCiAKIHN0YXRpYyB2b2lkIHJ0d19zZGlvX2VuYWJsZV9yeF9h
Z2dyZWdhdGlvbihzdHJ1Y3QgcnR3X2RldiAqcnR3ZGV2KQogewotCXU4IHNpemUsIHRpbWVvdXQ7
CisJc3RydWN0IHJ0d19zZGlvICpydHdzZGlvID0gKHN0cnVjdCBydHdfc2RpbyAqKXJ0d2Rldi0+
cHJpdjsKKwlzdHJ1Y3QgbW1jX2hvc3QgKmhvc3QgPSBydHdzZGlvLT5zZGlvX2Z1bmMtPmNhcmQt
Pmhvc3Q7CisJdW5zaWduZWQgaW50IGhvc3RfbWF4X3BhZ2VzOworCXU4IGNoaXBfbWF4X3BhZ2Vz
LCB0aW1lb3V0OwogCiAJaWYgKHJ0d19jaGlwX3djcHVfMTFuKHJ0d2RldikpIHsKLQkJc2l6ZSA9
IDB4NjsKKwkJY2hpcF9tYXhfcGFnZXMgPSAweDY7CiAJCXRpbWVvdXQgPSAweDY7CiAJfSBlbHNl
IHsKLQkJc2l6ZSA9IDB4ZmY7CisJCWNoaXBfbWF4X3BhZ2VzID0gMHhmZjsKIAkJdGltZW91dCA9
IDB4MTsKIAl9CiAKKwlob3N0X21heF9wYWdlcyA9IGhvc3QtPm1heF9yZXFfc2l6ZSAvIFNaXzFL
OworCisJcnR3X2VycihydHdkZXYsICJNYXggUlggcGFnZXMgLSBjaGlwIGFnZ3JlZ2F0aW9uIGxp
bWl0OiAldSwgaG9zdCBjb250cm9sbGVyIGxpbWl0OiAldVxuIiwgY2hpcF9tYXhfcGFnZXMsIGhv
c3RfbWF4X3BhZ2VzKTsvLyBIQUNLCisJcnR3X2RiZyhydHdkZXYsIFJUV19EQkdfU0RJTywKKwkJ
Ik1heCBSWCBwYWdlcyAtIGNoaXAgYWdncmVnYXRpb24gbGltaXQ6ICV1LCBob3N0IGNvbnRyb2xs
ZXIgbGltaXQ6ICV1XG4iLAorCQljaGlwX21heF9wYWdlcywgaG9zdF9tYXhfcGFnZXMpOworCiAJ
LyogTWFrZSB0aGUgZmlybXdhcmUgaG9ub3IgdGhlIHNpemUgbGltaXQgY29uZmlndXJlZCBiZWxv
dyAqLwogCXJ0d193cml0ZTMyX3NldChydHdkZXYsIFJFR19SWERNQV9BR0dfUEdfVEgsIEJJVF9F
Tl9QUkVfQ0FMQyk7CiAKIAlydHdfd3JpdGU4X3NldChydHdkZXYsIFJFR19UWERNQV9QUV9NQVAs
IEJJVF9SWERNQV9BR0dfRU4pOwogCiAJcnR3X3dyaXRlMTYocnR3ZGV2LCBSRUdfUlhETUFfQUdH
X1BHX1RILAotCQkgICAgRklFTERfUFJFUChCSVRfUlhETUFfQUdHX1BHX1RILCBzaXplKSB8CisJ
CSAgICBGSUVMRF9QUkVQKEJJVF9SWERNQV9BR0dfUEdfVEgsIG1pbl90KHVuc2lnbmVkIGludCwK
KwkJCQkJCQkgIGNoaXBfbWF4X3BhZ2VzLAorCQkJCQkJCSAgaG9zdF9tYXhfcGFnZXMpKSB8CiAJ
CSAgICBGSUVMRF9QUkVQKEJJVF9ETUFfQUdHX1RPX1YxLCB0aW1lb3V0KSk7CiAKIAlydHdfd3Jp
dGU4X3NldChydHdkZXYsIFJFR19SWERNQV9NT0RFLCBCSVRfRE1BX01PREUpOwpAQCAtOTM2LDYg
Kzk0OCw3IEBAIHN0YXRpYyB2b2lkIHJ0d19zZGlvX3J4Zmlmb19yZWN2KHN0cnVjdCBydHdfZGV2
ICpydHdkZXYsIHUzMiByeF9sZW4pCiB7CiAJc3RydWN0IHJ0d19zZGlvICpydHdzZGlvID0gKHN0
cnVjdCBydHdfc2RpbyAqKXJ0d2Rldi0+cHJpdjsKIAljb25zdCBzdHJ1Y3QgcnR3X2NoaXBfaW5m
byAqY2hpcCA9IHJ0d2Rldi0+Y2hpcDsKKwl1bnNpZ25lZCBpbnQgbnVtX3BrdCA9IDAsIG9yaWdf
cnhfbGVuID0gcnhfbGVuOwogCXUzMiBwa3RfZGVzY19zeiA9IGNoaXAtPnJ4X3BrdF9kZXNjX3N6
OwogCXN0cnVjdCBpZWVlODAyMTFfcnhfc3RhdHVzIHJ4X3N0YXR1czsKIAlzdHJ1Y3QgcnR3X3J4
X3BrdF9zdGF0IHBrdF9zdGF0OwpAQCAtOTc0LDYgKzk4Nyw3IEBAIHN0YXRpYyB2b2lkIHJ0d19z
ZGlvX3J4Zmlmb19yZWN2KHN0cnVjdCBydHdfZGV2ICpydHdkZXYsIHUzMiByeF9sZW4pCiAJCQkg
Ki8KIAkJCXJ0d19zZGlvX3J4X3NrYihydHdkZXYsIHNrYiwgcGt0X29mZnNldCwgJnBrdF9zdGF0
LAogCQkJCQkmcnhfc3RhdHVzKTsKKwkJCW51bV9wa3QrKzsKIAkJCWJyZWFrOwogCQl9CiAKQEAg
LTk4MSw2ICs5OTUsNyBAQCBzdGF0aWMgdm9pZCBydHdfc2Rpb19yeGZpZm9fcmVjdihzdHJ1Y3Qg
cnR3X2RldiAqcnR3ZGV2LCB1MzIgcnhfbGVuKQogCQlpZiAoIXNwbGl0X3NrYikgewogCQkJcnR3
X3NkaW9fcnhfc2tiKHJ0d2Rldiwgc2tiLCBwa3Rfb2Zmc2V0LCAmcGt0X3N0YXQsCiAJCQkJCSZy
eF9zdGF0dXMpOworCQkJbnVtX3BrdCsrOwogCQkJYnJlYWs7CiAJCX0KIApAQCAtOTg5LDExICsx
MDA0LDE0IEBAIHN0YXRpYyB2b2lkIHJ0d19zZGlvX3J4Zmlmb19yZWN2KHN0cnVjdCBydHdfZGV2
ICpydHdkZXYsIHUzMiByeF9sZW4pCiAKIAkJcnR3X3NkaW9fcnhfc2tiKHJ0d2Rldiwgc3BsaXRf
c2tiLCBwa3Rfb2Zmc2V0LCAmcGt0X3N0YXQsCiAJCQkJJnJ4X3N0YXR1cyk7CisJCW51bV9wa3Qr
KzsKIAogCQkvKiBNb3ZlIHRvIHRoZSBzdGFydCBvZiB0aGUgbmV4dCBSWCBkZXNjcmlwdG9yICov
CiAJCXNrYl9yZXNlcnZlKHNrYiwgY3Vycl9wa3RfbGVuKTsKIAkJcnhfbGVuIC09IGN1cnJfcGt0
X2xlbjsKIAl9CisKKwlwcl9lcnIoIiVzKCV1KSAtIG51bWJlciBvZiBwYWNrZXRzOiAldVxuIiwg
X19mdW5jX18sIG9yaWdfcnhfbGVuLCBudW1fcGt0KTsvLyBIQUNLCiB9CiAKIHN0YXRpYyB2b2lk
IHJ0d19zZGlvX3J4X2lzcihzdHJ1Y3QgcnR3X2RldiAqcnR3ZGV2KQo=
--000000000000824ae805ff9bcecc--