[PATCH rtw-next 0/2] wifi: rtw88: usb: keep bmc traffic from exhausting the TX page pool

[PATCH rtw-next 0/2] wifi: rtw88: usb: keep bmc traffic from exhausting the TX page pool

[Mehmet Fide]

From: Mehmet Fide <mehmet.fide@screeningeagle.com>

Hi Ping-Ke, Bitterblue,

this follows up on the discussion under the v3 routing patch [1]: with
that patch applied, a single dozing station is still enough to route all
broadcast and multicast traffic through the high queue, and since the
chip only drains that queue in the ATIM window after DTIM beacons while
the frames occupy the shared TX page pool, ordinary chatter empties the
pool and the AP stops accepting stations. Numbers and the ATIM window
experiment are in the thread [2]; the short version: at the default 2 TU
window the queue drains ~3 frames per DTIM, ~40 frames/s of mDNS takes
the pool from 1803 to 16 pages in about 100 s, and enlarging the window
to 4 TU or more makes the same storm harmless.

The series follows the order Ping-Ke suggested (bound first, then
filter):

Patch 1 caps how many frames the driver hands to the high queue with a
small token bucket set below the measured drain rate; the excess goes
out on the access category queue right away. This is the guarantee: the
pool stays healthy under any storm, including ARP/DHCP bursts.

Patch 2 admits only ARP, EAPOL and DHCP to the after-DTIM path, matching
the vendor driver's default high queue filter, so ordinary chatter never
reaches the beacon-paced queue and the budget is left for the frames a
sleeping station actually needs. At runtime the filter is evaluated
before a token is taken, so chatter does not consume the budget.

Not included: raising REG_ATIMWND. It would add drain capacity, but the
queue stays unbounded and a wider window costs every PS station awake
time after each DTIM; if the firmware is fine with a larger window on
the USB chips I can add it as a separate patch. The mac80211-side
buffering (IEEE80211_HW_HOST_BROADCAST_PS_BUFFERING) needs a beacon-time
event; the firmware seems to have one (C2H_BCN_EARLY_RPT, used by the
vendor driver for TDLS), that is a follow-up once we know it works in AP
mode.

Tested on a Verdin AM62 AP with RTL8822BU (USB2, 20 MHz, WPA2, one
Windows client in power save), free page count read at 0x240, on top of
rtw-next with c710c7c2e038:
  - baseline (rtw-next as is), one client in power save, ~80 bmc frames/s
    generated on the AP: free pages 1803 -> 16 within 30 s and pinned for
    the whole storm, 18 "error beacon valid" / rsvd page failures, and
    after the storm three reconnect attempts failed (no association
    until the client left and the pool recovered ~90 s later)
  - patch 1 alone: 1803 pages throughout the same 180 s storm and a
    120 s DHCP-port storm, no beacon errors, ping over the existing link
    6/6, reconnects 3/3
  - patches 1+2: 1803 pages throughout both storms as well, no
    beacon errors, ping 6/6, reconnects 3/3; the filter keeps the
    budget for ARP/EAPOL/DHCP, so the DHCP-port storm is the case where
    the token bucket actually engages
  - join/ping cycling without power save (10 cycles): unchanged
  - with an iPhone (screen off) as the only associated station instead
    of the Windows client: baseline 1803 -> 17 in 30 s and three join
    attempts from a second device fail; with both patches 1803
    throughout and the second device joins 3/3
  - the same series on an RTL8821CU AP (i.MX8MP, PREEMPT_RT kernel,
    411 free pages when idle): baseline drops to 16 pages within 30 s
    and stays there, ping and three join attempts all fail; with the
    series the count stays at 411 through the multicast storm, dips to
    357 and recovers during the DHCP-port storm (the budget doing its
    job), ping 6/6 and joins 3/3. No atomic-context or lockdep
    complaints on the RT kernel.

[1] https://lore.kernel.org/linux-wireless/20260814053426.2473247-1-mehmet.fide@gmail.com/
[2] https://lore.kernel.org/linux-wireless/20260901091830.2506562-1-mehmet.fide@gmail.com/

Mehmet Fide (2):
  wifi: rtw88: usb: bound what the driver feeds the after-DTIM queue
  wifi: rtw88: usb: only let the frames a dozing station needs use the
    after-DTIM queue

 drivers/net/wireless/realtek/rtw88/usb.c | 82 +++++++++++++++++++++++-
 drivers/net/wireless/realtek/rtw88/usb.h |  5 ++
 2 files changed, 84 insertions(+), 3 deletions(-)

-- 
2.54.0

[PATCH rtw-next 1/2] wifi: rtw88: usb: bound what the driver feeds the after-DTIM queue

[Mehmet Fide]

From: Mehmet Fide <mehmet.fide@screeningeagle.com>

Frames routed to the high queue are transmitted right after DTIM beacons
only, inside the ATIM window, while they wait in the shared TX page pool.
The driver puts no limit on how many it hands over, so as long as one
station dozes, any sustained broadcast or multicast traffic outruns the
drain and empties the pool: measured on an RTL8822BU AP with a single
client in power save and ~40 frames/s of mDNS chatter, the free page
count at 0x240 goes from 1803 to 16 in about 100 seconds and stays there
for as long as the traffic lasts. From that point every other transmit
queues behind the backlog, authentication responses arrive too late for
anyone to join, and the reserved page download fails ("error beacon
valid"). The AP keeps beaconing and only a reboot recovers.

Feed the high queue through a small token bucket set below the measured
drain rate (about 3 frames per DTIM, ~15/s at dtim_period 2 on the
default 2 TU ATIM window); whatever exceeds the budget leaves on its
access category queue right away. The high queue backlog is now bounded
by the burst size under any load, so the page pool cannot run dry, at
the price that a dozing station may miss part of a broadcast storm.

Signed-off-by: Mehmet Fide <mehmet.fide@screeningeagle.com>
---
 drivers/net/wireless/realtek/rtw88/usb.c | 42 ++++++++++++++++++++++--
 drivers/net/wireless/realtek/rtw88/usb.h |  5 +++
 2 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
index c90802919473..80965e5ea778 100644
--- a/drivers/net/wireless/realtek/rtw88/usb.c
+++ b/drivers/net/wireless/realtek/rtw88/usb.c
@@ -562,7 +562,38 @@ static int rtw_usb_write_data_h2c(struct rtw_dev *rtwdev, u8 *buf, u32 size)
 	return rtw_usb_write_data(rtwdev, &pkt_info, buf);
 }
 
-static u8 rtw_usb_tx_queue_mapping_to_qsel(struct sk_buff *skb)
+#define RTW_USB_HIQ_RATE	10
+#define RTW_USB_HIQ_BURST	16
+
+static bool rtw_usb_hiq_take(struct rtw_usb *rtwusb)
+{
+	unsigned long flags, delta;
+	bool ok;
+	u32 add;
+
+	spin_lock_irqsave(&rtwusb->hiq_lock, flags);
+	delta = jiffies - rtwusb->hiq_refill;
+	if (delta >= HZ / RTW_USB_HIQ_RATE) {
+		add = delta / (HZ / RTW_USB_HIQ_RATE);
+		if (add >= RTW_USB_HIQ_BURST) {
+			rtwusb->hiq_tokens = RTW_USB_HIQ_BURST;
+			rtwusb->hiq_refill = jiffies;
+		} else {
+			rtwusb->hiq_tokens = min_t(u32, rtwusb->hiq_tokens + add,
+						   RTW_USB_HIQ_BURST);
+			rtwusb->hiq_refill += add * (HZ / RTW_USB_HIQ_RATE);
+		}
+	}
+	ok = rtwusb->hiq_tokens > 0;
+	if (ok)
+		rtwusb->hiq_tokens--;
+	spin_unlock_irqrestore(&rtwusb->hiq_lock, flags);
+
+	return ok;
+}
+
+static u8 rtw_usb_tx_queue_mapping_to_qsel(struct rtw_usb *rtwusb,
+					   struct sk_buff *skb)
 {
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
@@ -573,7 +604,8 @@ static u8 rtw_usb_tx_queue_mapping_to_qsel(struct sk_buff *skb)
 		qsel = TX_DESC_QSEL_MGMT;
 	else if (is_broadcast_ether_addr(hdr->addr1) ||
 		 is_multicast_ether_addr(hdr->addr1))
-		qsel = (info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM) ?
+		qsel = (info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM) &&
+		       rtw_usb_hiq_take(rtwusb) ?
 		       TX_DESC_QSEL_HIGH : skb->priority;
 	else if (skb_get_queue_mapping(skb) <= IEEE80211_AC_BK)
 		qsel = skb->priority;
@@ -593,7 +625,7 @@ static int rtw_usb_tx_write(struct rtw_dev *rtwdev,
 	u8 *pkt_desc;
 	int ep;
 
-	pkt_info->qsel = rtw_usb_tx_queue_mapping_to_qsel(skb);
+	pkt_info->qsel = rtw_usb_tx_queue_mapping_to_qsel(rtwusb, skb);
 	pkt_desc = skb_push(skb, chip->tx_pkt_desc_sz);
 	memset(pkt_desc, 0, chip->tx_pkt_desc_sz);
 	ep = qsel_to_ep(rtwusb, pkt_info->qsel);
@@ -1034,6 +1066,10 @@ static int rtw_usb_init_tx(struct rtw_dev *rtwdev)
 	struct rtw_usb *rtwusb = rtw_get_usb_priv(rtwdev);
 	int i;
 
+	spin_lock_init(&rtwusb->hiq_lock);
+	rtwusb->hiq_tokens = RTW_USB_HIQ_BURST;
+	rtwusb->hiq_refill = jiffies;
+
 	rtwusb->txwq = create_singlethread_workqueue("rtw88_usb: tx wq");
 	if (!rtwusb->txwq) {
 		rtw_err(rtwdev, "failed to create TX work queue\n");
diff --git a/drivers/net/wireless/realtek/rtw88/usb.h b/drivers/net/wireless/realtek/rtw88/usb.h
index 9b695b688b24..67463a4d2f90 100644
--- a/drivers/net/wireless/realtek/rtw88/usb.h
+++ b/drivers/net/wireless/realtek/rtw88/usb.h
@@ -75,6 +75,11 @@ struct rtw_usb {
 	u8 out_ep[RTW_USB_EP_MAX];
 	int qsel_to_ep[TX_DESC_QSEL_MAX];
 
+	/* protects hiq_tokens and hiq_refill */
+	spinlock_t hiq_lock;
+	u32 hiq_tokens;
+	unsigned long hiq_refill;
+
 	struct workqueue_struct *txwq, *rxwq;
 
 	struct sk_buff_head tx_queue[RTW_USB_EP_MAX];
-- 
2.54.0

[PATCH rtw-next 2/2] wifi: rtw88: usb: only let the frames a dozing station needs use the after-DTIM queue

[Mehmet Fide]

From: Mehmet Fide <mehmet.fide@screeningeagle.com>

With the budget in place the high queue can no longer take the chip
down, but ordinary chatter still competes with the frames a sleeping
station actually needs. mac80211 marks every broadcast and multicast
frame with IEEE80211_TX_CTL_SEND_AFTER_DTIM while a station dozes,
mDNS and SSDP included, so under normal traffic the budget is spent on
frames nobody waits for.

Do what the vendor driver does with its default "allow special" high
queue filter: admit only ARP, EAPOL and DHCP to the after-DTIM path,
the frames a station coming out of power save has to see; everything
else goes out on its access category queue at line rate. Encrypted
group frames carry the IV between the header and the SNAP header, so
the parser accounts for hw_key->iv_len. With the filter in place the
page pool stays at 1803 through the same 180 second storm, a DHCP
flood still takes the after-DTIM path (and is then held by the budget),
and join/ping cycling without power save is unchanged (10/10).

Signed-off-by: Mehmet Fide <mehmet.fide@screeningeagle.com>
---
 drivers/net/wireless/realtek/rtw88/usb.c | 42 +++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
index 80965e5ea778..dd65b304b2c7 100644
--- a/drivers/net/wireless/realtek/rtw88/usb.c
+++ b/drivers/net/wireless/realtek/rtw88/usb.c
@@ -5,6 +5,9 @@
 #include <linux/module.h>
 #include <linux/usb.h>
 #include <linux/mutex.h>
+#include <linux/ip.h>
+#include <linux/udp.h>
+#include <linux/unaligned.h>
 #include "main.h"
 #include "debug.h"
 #include "mac.h"
@@ -562,6 +565,43 @@ static int rtw_usb_write_data_h2c(struct rtw_dev *rtwdev, u8 *buf, u32 size)
 	return rtw_usb_write_data(rtwdev, &pkt_info, buf);
 }
 
+static bool rtw_usb_bmc_needs_dtim(struct sk_buff *skb)
+{
+	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
+	unsigned int hdrlen = ieee80211_hdrlen(hdr->frame_control);
+	unsigned int paylen = sizeof(rfc1042_header) + sizeof(__be16);
+	const struct udphdr *udp;
+	const struct iphdr *ip;
+	const u8 *snap;
+	__be16 proto;
+
+	if (info->control.hw_key)
+		hdrlen += info->control.hw_key->iv_len;
+
+	if (skb->len < hdrlen + paylen)
+		return false;
+
+	snap = skb->data + hdrlen;
+	proto = get_unaligned((__be16 *)(snap + sizeof(rfc1042_header)));
+
+	if (proto == htons(ETH_P_ARP) || proto == htons(ETH_P_PAE))
+		return true;
+
+	if (proto != htons(ETH_P_IP) || skb->len < hdrlen + paylen + sizeof(*ip))
+		return false;
+
+	ip = (const struct iphdr *)(snap + paylen);
+	if (ip->protocol != IPPROTO_UDP)
+		return false;
+
+	udp = (const struct udphdr *)((const u8 *)ip + ip->ihl * 4);
+	if (skb->len < (unsigned int)((const u8 *)udp - skb->data) + sizeof(*udp))
+		return false;
+
+	return udp->dest == htons(67) || udp->dest == htons(68);
+}
+
 #define RTW_USB_HIQ_RATE	10
 #define RTW_USB_HIQ_BURST	16
 
@@ -605,7 +645,7 @@ static u8 rtw_usb_tx_queue_mapping_to_qsel(struct rtw_usb *rtwusb,
 	else if (is_broadcast_ether_addr(hdr->addr1) ||
 		 is_multicast_ether_addr(hdr->addr1))
 		qsel = (info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM) &&
-		       rtw_usb_hiq_take(rtwusb) ?
+		       rtw_usb_bmc_needs_dtim(skb) && rtw_usb_hiq_take(rtwusb) ?
 		       TX_DESC_QSEL_HIGH : skb->priority;
 	else if (skb_get_queue_mapping(skb) <= IEEE80211_AC_BK)
 		qsel = skb->priority;
-- 
2.54.0

RE: [PATCH rtw-next 1/2] wifi: rtw88: usb: bound what the driver feeds the after-DTIM queue

[Ping-Ke Shih]

Mehmet Fide <mehmet.fide@gmail.com> wrote:

[...]

> --- a/drivers/net/wireless/realtek/rtw88/usb.c
> +++ b/drivers/net/wireless/realtek/rtw88/usb.c
> @@ -562,7 +562,38 @@ static int rtw_usb_write_data_h2c(struct rtw_dev *rtwdev, u8 *buf, u32 size)
>         return rtw_usb_write_data(rtwdev, &pkt_info, buf);
>  }
> 
> -static u8 rtw_usb_tx_queue_mapping_to_qsel(struct sk_buff *skb)
> +#define RTW_USB_HIQ_RATE       10
> +#define RTW_USB_HIQ_BURST      16
> +
> +static bool rtw_usb_hiq_take(struct rtw_usb *rtwusb)
> +{
> +       unsigned long flags, delta;
> +       bool ok;
> +       u32 add;
> +
> +       spin_lock_irqsave(&rtwusb->hiq_lock, flags);

nit: a blank line

> +       delta = jiffies - rtwusb->hiq_refill;
> +       if (delta >= HZ / RTW_USB_HIQ_RATE) {
> +               add = delta / (HZ / RTW_USB_HIQ_RATE);

The pattern ' HZ / RTW_USB_HIQ_RATE' repeat many times. Can you just define
the it as a part of macro? (The macro name should change as well.)

> +               if (add >= RTW_USB_HIQ_BURST) {
> +                       rtwusb->hiq_tokens = RTW_USB_HIQ_BURST;
> +                       rtwusb->hiq_refill = jiffies;
> +               } else {
> +                       rtwusb->hiq_tokens = min_t(u32, rtwusb->hiq_tokens + add,
> +                                                  RTW_USB_HIQ_BURST);

Will 'quota' or 'budget' be clearer than 'token'?

> +                       rtwusb->hiq_refill += add * (HZ / RTW_USB_HIQ_RATE);

It looks like you shift refill jiffies according to the tokens you are adding.
How can I understand '+= add * (HZ / RTW_USB_HIQ_RATE)'?

> +               }
> +       }
> +       ok = rtwusb->hiq_tokens > 0;
> +       if (ok)
> +               rtwusb->hiq_tokens--;

nit: a blank line

> +       spin_unlock_irqrestore(&rtwusb->hiq_lock, flags);
> +
> +       return ok;
> +}
> +
> +static u8 rtw_usb_tx_queue_mapping_to_qsel(struct rtw_usb *rtwusb,
> +                                          struct sk_buff *skb)
>  {
>         struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
>         struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);

RE: [PATCH rtw-next 2/2] wifi: rtw88: usb: only let the frames a dozing station needs use the after-DTIM queue

[Ping-Ke Shih]

Mehmet Fide <mehmet.fide@gmail.com> wrote:
> From: Mehmet Fide <mehmet.fide@screeningeagle.com>
> 
> With the budget in place the high queue can no longer take the chip
> down, but ordinary chatter still competes with the frames a sleeping
> station actually needs. mac80211 marks every broadcast and multicast
> frame with IEEE80211_TX_CTL_SEND_AFTER_DTIM while a station dozes,
> mDNS and SSDP included, so under normal traffic the budget is spent on
> frames nobody waits for.
> 
> Do what the vendor driver does with its default "allow special" high
> queue filter: admit only ARP, EAPOL and DHCP to the after-DTIM path,
> the frames a station coming out of power save has to see; everything
> else goes out on its access category queue at line rate. Encrypted
> group frames carry the IV between the header and the SNAP header, so
> the parser accounts for hw_key->iv_len. With the filter in place the
> page pool stays at 1803 through the same 180 second storm, a DHCP
> flood still takes the after-DTIM path (and is then held by the budget),
> and join/ping cycling without power save is unchanged (10/10).
> 
> Signed-off-by: Mehmet Fide <mehmet.fide@screeningeagle.com>
> ---
>  drivers/net/wireless/realtek/rtw88/usb.c | 42 +++++++++++++++++++++++-
>  1 file changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
> index 80965e5ea778..dd65b304b2c7 100644
> --- a/drivers/net/wireless/realtek/rtw88/usb.c
> +++ b/drivers/net/wireless/realtek/rtw88/usb.c
> @@ -5,6 +5,9 @@
>  #include <linux/module.h>
>  #include <linux/usb.h>
>  #include <linux/mutex.h>
> +#include <linux/ip.h>
> +#include <linux/udp.h>
> +#include <linux/unaligned.h>

The existing is messed already. But could please put them in increasing
alphabet order? (only apply the part of '#include <linux/*')

>  #include "main.h"
>  #include "debug.h"
>  #include "mac.h"
> @@ -562,6 +565,43 @@ static int rtw_usb_write_data_h2c(struct rtw_dev *rtwdev, u8 *buf, u32 size)
>         return rtw_usb_write_data(rtwdev, &pkt_info, buf);
>  }
> 
> +static bool rtw_usb_bmc_needs_dtim(struct sk_buff *skb)
> +{
> +       struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
> +       struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
> +       unsigned int hdrlen = ieee80211_hdrlen(hdr->frame_control);
> +       unsigned int paylen = sizeof(rfc1042_header) + sizeof(__be16);

In reverse X'mas tree order. If you want keep 'struct' and 'unsigned int'
together individually. We can have

	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
	unsigned int paylen = sizeof(rfc1042_header) + sizeof(__be16);
	unsigned int hdrlen = ieee80211_hdrlen(hdr->frame_control);


> +       const struct udphdr *udp;
> +       const struct iphdr *ip;
> +       const u8 *snap;
> +       __be16 proto;
> +
> +       if (info->control.hw_key)
> +               hdrlen += info->control.hw_key->iv_len;
> +
> +       if (skb->len < hdrlen + paylen)
> +               return false;
> +
> +       snap = skb->data + hdrlen;
> +       proto = get_unaligned((__be16 *)(snap + sizeof(rfc1042_header)));
> +
> +       if (proto == htons(ETH_P_ARP) || proto == htons(ETH_P_PAE))

Can we just 'skb->protocol == htons(ETH_P_ARP)' ?

> +               return true;
> +
> +       if (proto != htons(ETH_P_IP) || skb->len < hdrlen + paylen + sizeof(*ip))
> +               return false;
> +
> +       ip = (const struct iphdr *)(snap + paylen);
> +       if (ip->protocol != IPPROTO_UDP)
> +               return false;
> +
> +       udp = (const struct udphdr *)((const u8 *)ip + ip->ihl * 4);
> +       if (skb->len < (unsigned int)((const u8 *)udp - skb->data) + sizeof(*udp))
> +               return false;

udphdr = udp_hdr(skb); ?

reference to rtw89_core_tx_btc_spec_pkt_notify().

> +
> +       return udp->dest == htons(67) || udp->dest == htons(68);
> +}
> +
>  #define RTW_USB_HIQ_RATE       10
>  #define RTW_USB_HIQ_BURST      16
> 
> @@ -605,7 +645,7 @@ static u8 rtw_usb_tx_queue_mapping_to_qsel(struct rtw_usb *rtwusb,
>         else if (is_broadcast_ether_addr(hdr->addr1) ||
>                  is_multicast_ether_addr(hdr->addr1))
>                 qsel = (info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM) &&
> -                      rtw_usb_hiq_take(rtwusb) ?
> +                      rtw_usb_bmc_needs_dtim(skb) && rtw_usb_hiq_take(rtwusb) ?
>                        TX_DESC_QSEL_HIGH : skb->priority;
>         else if (skb_get_queue_mapping(skb) <= IEEE80211_AC_BK)
>                 qsel = skb->priority;
> --
> 2.54.0

Re: [PATCH rtw-next 1/2] wifi: rtw88: usb: bound what the driver feeds the after-DTIM queue

[Mehmet Fide]

Hi Ping-Ke,

thanks for the review.

> nit: a blank line

Will add both.

> The pattern ' HZ / RTW_USB_HIQ_RATE' repeat many times. Can you just define
> the it as a part of macro? (The macro name should change as well.)

Yes. v2 will have

	#define RTW_USB_HIQ_REFILL_INTERVAL	(HZ / 10)	/* jiffies per unit of budget */
	#define RTW_USB_HIQ_BUDGET_MAX		16

and use RTW_USB_HIQ_REFILL_INTERVAL everywhere.

> Will 'quota' or 'budget' be clearer than 'token'?

Budget it is: hiq_budget, hiq_budget_refill, rtw_usb_hiq_take_budget().

> It looks like you shift refill jiffies according to the tokens you are adding.
> How can I understand '+= add * (HZ / RTW_USB_HIQ_RATE)'?

hiq_refill is the point in time up to which budget has already been granted.
When 'add' whole intervals have passed I move it forward by exactly those
intervals, not to 'jiffies', so the part of the current interval that has
not completed yet keeps counting toward the next unit instead of being
thrown away (the usual token bucket bookkeeping). When more than a full
burst has elapsed the exact position no longer matters, which is why that
branch simply resets it to jiffies.

I agree it reads badly. v2 will say the same thing as

	elapsed = jiffies - rtwusb->hiq_refill;
	add = elapsed / RTW_USB_HIQ_REFILL_INTERVAL;
	if (add) {
		rtwusb->hiq_budget = min_t(u32, rtwusb->hiq_budget + add,
					   RTW_USB_HIQ_BUDGET_MAX);
		rtwusb->hiq_refill = jiffies - elapsed % RTW_USB_HIQ_REFILL_INTERVAL;
	}

with a comment on the remainder, so the intent is visible without the
multiplication.

I will send v2 early next week together with the changes to 2/2.

Mehmet

Re: [PATCH rtw-next 2/2] wifi: rtw88: usb: only let the frames a dozing station needs use the after-DTIM queue

[Mehmet Fide]

Hi Ping-Ke,

> The existing is messed already. But could please put them in increasing
> alphabet order? (only apply the part of '#include <linux/*')

Will do.

> In reverse X'mas tree order. If you want keep 'struct' and 'unsigned int'
> together individually. We can have
> 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
> 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
> 	unsigned int paylen = sizeof(rfc1042_header) + sizeof(__be16);
> 	unsigned int hdrlen = ieee80211_hdrlen(hdr->frame_control);

Will use exactly that order (fewer of them after the change below).

> Can we just 'skb->protocol == htons(ETH_P_ARP)' ?

Yes. mac80211 keeps skb->protocol and the network header offset from the
netdev path (ieee80211_build_hdr() only pushes the 802.11 and SNAP headers
in front, the offsets are relative to skb->head), so the SNAP parsing, the
hw_key->iv_len handling and the unaligned.h include can go entirely. For
EAPOL I will test IEEE80211_TX_CTRL_PORT_CTRL_PROTO like
rtw89_core_tx_btc_spec_pkt_notify() does, which also covers a non-default
control port protocol.

> udphdr = udp_hdr(skb); ?
> reference to rtw89_core_tx_btc_spec_pkt_notify().

Will do: skb->protocol == htons(ETH_P_IP), ip_hdr(skb)->protocol ==
IPPROTO_UDP, then udp_hdr(skb) for the 67/68 ports, same shape as the rtw89
helper. The transport header is set by the IP stack for the frames this
matters for (the AP's own DHCP replies).

v2 early next week, with the changes to 1/2 and, if you agree with the
proposal in the other thread, a third patch for REG_ATIMWND.

Mehmet