[PATCH rtw-next] wifi: rtw88: sdio: limit the high queue to DTIM delivery

[PATCH rtw-next] wifi: rtw88: sdio: limit the high queue to DTIM delivery

[Luka Gejak]

In AP mode every broadcast and multicast (bmc) frame is sent through the
high queue, which the hardware only drains after a DTIM beacon. Only a
few frames leave per DTIM, so steady multicast traffic builds a backlog
that holds the TX pages the other queues need. Unicast traffic then
stalls for seconds and clients can no longer renew their DHCP lease.

USB had the same problem and was fixed by commit c710c7c2e038 ("wifi:
rtw88: usb: route bmc frames via the high queue only for DTIM delivery")
and the two commits that followed it. Use the same rule for SDIO. A bmc
frame goes to the high queue only when mac80211 asks for DTIM delivery
because a station is dozing, the frame is ARP, EAPOL or DHCP, and the
budget allows it. All other bmc frames go out on their access category
queue.

Signed-off-by: Luka Gejak <luka.gejak@linux.dev>
---
  Tested on rtl8723bs with atk19 client. No regression in station mode.
  Note: this adds support for AP mode for rtl8723bs, as I states AP
  mode was left for future work.
- Broadcast ping round trip was 3.6 ms, it was ~104 ms before the
    patch.
  - During a 50 frames/s broadcast flood, the TX page pool didn't
    move and unicast was unaffected.
  - A DHCP renewal under that load got a lease on the first DISCOVER.
  - A dozing client received 18 of 20 broadcast ARP requests.

 drivers/net/wireless/realtek/rtw88/sdio.c | 69 +++++++++++++++++++++++
 drivers/net/wireless/realtek/rtw88/sdio.h |  5 ++
 2 files changed, 74 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
index dc2fd0f8f9ff..3cdffac5b5b0 100644
--- a/drivers/net/wireless/realtek/rtw88/sdio.c
+++ b/drivers/net/wireless/realtek/rtw88/sdio.c
@@ -6,9 +6,11 @@
  *   Copyright(c) 2018-2019  Realtek Corporation
  */
 
+#include <linux/ip.h>
 #include <linux/module.h>
 #include <linux/mmc/host.h>
 #include <linux/mmc/sdio_func.h>
+#include <linux/udp.h>
 #include "main.h"
 #include "mac.h"
 #include "debug.h"
@@ -26,6 +28,8 @@
 #define RTW_SDIO_TX_FIFO_HIWATER			16
 #define RTW_SDIO_TX_FIFO_LOWATER			8
 #define RTW_SDIO_TX_RETRY_DELAY			msecs_to_jiffies(1)
+#define RTW_SDIO_HIQ_REFILL_INTERVAL			(HZ / 10)
+#define RTW_SDIO_HIQ_BUDGET_MAX				16
 
 static bool rtw_sdio_is_bus_addr(u32 addr)
 {
@@ -1299,6 +1303,64 @@ static void rtw_sdio_8723bs_wake_tx_queue(struct rtw_dev *rtwdev,
 	ieee80211_wake_queue(rtwdev->hw, q_map);
 }
 
+static bool rtw_sdio_bmc_needs_dtim(struct sk_buff *skb)
+{
+	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+	struct udphdr *udphdr;
+
+	if (info->control.flags & IEEE80211_TX_CTRL_PORT_CTRL_PROTO)
+		return true;
+
+	if (skb->protocol == htons(ETH_P_ARP))
+		return true;
+
+	if (skb->protocol == htons(ETH_P_IP) &&
+	    ip_hdr(skb)->protocol == IPPROTO_UDP) {
+		udphdr = udp_hdr(skb);
+
+		return udphdr->dest == htons(67) || udphdr->dest == htons(68);
+	}
+
+	return false;
+}
+
+static bool rtw_sdio_hiq_take_budget(struct rtw_sdio *rtwsdio)
+{
+	unsigned long flags, elapsed, add;
+	bool ok;
+
+	spin_lock_irqsave(&rtwsdio->hiq_lock, flags);
+
+	elapsed = jiffies - rtwsdio->hiq_refill;
+	add = elapsed / RTW_SDIO_HIQ_REFILL_INTERVAL;
+	if (add) {
+		rtwsdio->hiq_budget = min_t(unsigned long,
+					    rtwsdio->hiq_budget + add,
+					    RTW_SDIO_HIQ_BUDGET_MAX);
+
+		/* keep the unfinished part of the interval for the next unit */
+		rtwsdio->hiq_refill = jiffies -
+				      elapsed % RTW_SDIO_HIQ_REFILL_INTERVAL;
+	}
+
+	ok = rtwsdio->hiq_budget > 0;
+	if (ok)
+		rtwsdio->hiq_budget--;
+
+	spin_unlock_irqrestore(&rtwsdio->hiq_lock, flags);
+
+	return ok;
+}
+
+static bool rtw_sdio_bmc_use_hiq(struct rtw_sdio *rtwsdio, struct sk_buff *skb)
+{
+	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+
+	return (info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM) &&
+	       rtw_sdio_bmc_needs_dtim(skb) &&
+	       rtw_sdio_hiq_take_budget(rtwsdio);
+}
+
 static int rtw_sdio_tx_write(struct rtw_dev *rtwdev,
 			     struct rtw_tx_pkt_info *pkt_info,
 			     struct sk_buff *skb)
@@ -1308,6 +1370,9 @@ static int rtw_sdio_tx_write(struct rtw_dev *rtwdev,
 	u16 q_map = skb_get_queue_mapping(skb);
 	struct rtw_sdio_tx_data *tx_data;
 
+	if (queue == RTW_TX_QUEUE_HI0 && !rtw_sdio_bmc_use_hiq(rtwsdio, skb))
+		queue = rtw_tx_ac_to_hwq(q_map);
+
 	rtw_sdio_tx_skb_prepare(rtwdev, pkt_info, skb, queue);
 
 	tx_data = rtw_sdio_get_tx_data(skb);
@@ -1730,6 +1795,10 @@ static int rtw_sdio_init_tx(struct rtw_dev *rtwdev)
 	struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
 	int i;
 
+	spin_lock_init(&rtwsdio->hiq_lock);
+	rtwsdio->hiq_budget = RTW_SDIO_HIQ_BUDGET_MAX;
+	rtwsdio->hiq_refill = jiffies;
+
 	rtwsdio->txwq = create_singlethread_workqueue("rtw88_sdio: tx wq");
 	if (!rtwsdio->txwq) {
 		rtw_err(rtwdev, "failed to create TX work queue\n");
diff --git a/drivers/net/wireless/realtek/rtw88/sdio.h b/drivers/net/wireless/realtek/rtw88/sdio.h
index 6e7e6009744b..2083e6fb3fa9 100644
--- a/drivers/net/wireless/realtek/rtw88/sdio.h
+++ b/drivers/net/wireless/realtek/rtw88/sdio.h
@@ -174,6 +174,11 @@ struct rtw_sdio {
 	struct sk_buff_head tx_queue[RTK_MAX_TX_QUEUE_NUM];
 	bool tx_queue_stopped[RTK_MAX_TX_QUEUE_NUM];
 
+	/* protects hiq_budget and hiq_refill */
+	spinlock_t hiq_lock;
+	u32 hiq_budget;
+	unsigned long hiq_refill;
+
 	atomic_t free_pg_high;
 	atomic_t free_pg_normal;
 	atomic_t free_pg_low;
-- 
2.53.0

RE: [PATCH rtw-next] wifi: rtw88: sdio: limit the high queue to DTIM delivery

[Ping-Ke Shih]

Luka Gejak <luka.gejak@linux.dev> wrote:
> In AP mode every broadcast and multicast (bmc) frame is sent through the
> high queue, which the hardware only drains after a DTIM beacon. Only a
> few frames leave per DTIM, so steady multicast traffic builds a backlog
> that holds the TX pages the other queues need. Unicast traffic then
> stalls for seconds and clients can no longer renew their DHCP lease.
> 
> USB had the same problem and was fixed by commit c710c7c2e038 ("wifi:
> rtw88: usb: route bmc frames via the high queue only for DTIM delivery")
> and the two commits that followed it. Use the same rule for SDIO. A bmc
> frame goes to the high queue only when mac80211 asks for DTIM delivery
> because a station is dozing, the frame is ARP, EAPOL or DHCP, and the
> budget allows it. All other bmc frames go out on their access category
> queue.

I think it'd be good to add:

Tested on rtl8723bs with atk19 client.

> 
> Signed-off-by: Luka Gejak <luka.gejak@linux.dev>
> ---
>   Tested on rtl8723bs with atk19 client. 

(mention this in commit message)

> diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
> index dc2fd0f8f9ff..3cdffac5b5b0 100644
> --- a/drivers/net/wireless/realtek/rtw88/sdio.c
> +++ b/drivers/net/wireless/realtek/rtw88/sdio.c
> @@ -6,9 +6,11 @@
>   *   Copyright(c) 2018-2019  Realtek Corporation
>   */
> 
> +#include <linux/ip.h>

When moving rtw_sdio_bmc_needs_dtim() to tx.h, no need to include this, 
nor does usb.c.

>  #include <linux/module.h>
>  #include <linux/mmc/host.h>
>  #include <linux/mmc/sdio_func.h>
> +#include <linux/udp.h>
>  #include "main.h"
>  #include "mac.h"
>  #include "debug.h"
> @@ -26,6 +28,8 @@
>  #define RTW_SDIO_TX_FIFO_HIWATER                       16
>  #define RTW_SDIO_TX_FIFO_LOWATER                       8
>  #define RTW_SDIO_TX_RETRY_DELAY                        msecs_to_jiffies(1)
> +#define RTW_SDIO_HIQ_REFILL_INTERVAL                   (HZ / 10)
> +#define RTW_SDIO_HIQ_BUDGET_MAX                                16
> 
>  static bool rtw_sdio_is_bus_addr(u32 addr)
>  {
> @@ -1299,6 +1303,64 @@ static void rtw_sdio_8723bs_wake_tx_queue(struct rtw_dev *rtwdev,
>         ieee80211_wake_queue(rtwdev->hw, q_map);
>  }
> 
> +static bool rtw_sdio_bmc_needs_dtim(struct sk_buff *skb)
> +{
> +       struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
> +       struct udphdr *udphdr;
> +
> +       if (info->control.flags & IEEE80211_TX_CTRL_PORT_CTRL_PROTO)
> +               return true;
> +
> +       if (skb->protocol == htons(ETH_P_ARP))
> +               return true;
> +
> +       if (skb->protocol == htons(ETH_P_IP) &&
> +           ip_hdr(skb)->protocol == IPPROTO_UDP) {
> +               udphdr = udp_hdr(skb);
> +
> +               return udphdr->dest == htons(67) || udphdr->dest == htons(68);
> +       }
> +
> +       return false;
> +}

This is totally the same as USB. To share with USB, move it to tx.c 
as an export function or to tx.h as an inline function.

> +
> +static bool rtw_sdio_hiq_take_budget(struct rtw_sdio *rtwsdio)
> +{
> +       unsigned long flags, elapsed, add;
> +       bool ok;
> +
> +       spin_lock_irqsave(&rtwsdio->hiq_lock, flags);
> +
> +       elapsed = jiffies - rtwsdio->hiq_refill;
> +       add = elapsed / RTW_SDIO_HIQ_REFILL_INTERVAL;
> +       if (add) {
> +               rtwsdio->hiq_budget = min_t(unsigned long,
> +                                           rtwsdio->hiq_budget + add,
> +                                           RTW_SDIO_HIQ_BUDGET_MAX);
> +
> +               /* keep the unfinished part of the interval for the next unit */
> +               rtwsdio->hiq_refill = jiffies -
> +                                     elapsed % RTW_SDIO_HIQ_REFILL_INTERVAL;
> +       }
> +
> +       ok = rtwsdio->hiq_budget > 0;
> +       if (ok)
> +               rtwsdio->hiq_budget--;
> +
> +       spin_unlock_irqrestore(&rtwsdio->hiq_lock, flags);
> +
> +       return ok;
> +}

This is also the same as USB. Please consider how to share the logic.

Re: [PATCH rtw-next] wifi: rtw88: sdio: limit the high queue to DTIM delivery

[Luka Gejak]

October 21, 2026 at 3:34 AM, "Ping-Ke Shih" <pkshih@realtek.com> wrote:


> 
> Luka Gejak <luka.gejak@linux.dev> wrote:
> 
> > 
> > In AP mode every broadcast and multicast (bmc) frame is sent through the
> >  high queue, which the hardware only drains after a DTIM beacon. Only a
> >  few frames leave per DTIM, so steady multicast traffic builds a backlog
> >  that holds the TX pages the other queues need. Unicast traffic then
> >  stalls for seconds and clients can no longer renew their DHCP lease.
> >  
> >  USB had the same problem and was fixed by commit c710c7c2e038 ("wifi:
> >  rtw88: usb: route bmc frames via the high queue only for DTIM delivery")
> >  and the two commits that followed it. Use the same rule for SDIO. A bmc
> >  frame goes to the high queue only when mac80211 asks for DTIM delivery
> >  because a station is dozing, the frame is ARP, EAPOL or DHCP, and the
> >  budget allows it. All other bmc frames go out on their access category
> >  queue.
> > 
> I think it'd be good to add:
> 
> Tested on rtl8723bs with atk19 client.
> 

Ok, will add.

> > 
> > Signed-off-by: Luka Gejak <luka.gejak@linux.dev>
> >  ---
> >  Tested on rtl8723bs with atk19 client.
> > 
> (mention this in commit message)
> 
> > 
> > diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
> >  index dc2fd0f8f9ff..3cdffac5b5b0 100644
> >  --- a/drivers/net/wireless/realtek/rtw88/sdio.c
> >  +++ b/drivers/net/wireless/realtek/rtw88/sdio.c
> >  @@ -6,9 +6,11 @@
> >  * Copyright(c) 2018-2019 Realtek Corporation
> >  */
> >  
> >  +#include <linux/ip.h>
> > 
> When moving rtw_sdio_bmc_needs_dtim() to tx.h, no need to include this, 
> nor does usb.c.
> 
> > 
> > #include <linux/module.h>
> >  #include <linux/mmc/host.h>
> >  #include <linux/mmc/sdio_func.h>
> >  +#include <linux/udp.h>
> >  #include "main.h"
> >  #include "mac.h"
> >  #include "debug.h"
> >  @@ -26,6 +28,8 @@
> >  #define RTW_SDIO_TX_FIFO_HIWATER 16
> >  #define RTW_SDIO_TX_FIFO_LOWATER 8
> >  #define RTW_SDIO_TX_RETRY_DELAY msecs_to_jiffies(1)
> >  +#define RTW_SDIO_HIQ_REFILL_INTERVAL (HZ / 10)
> >  +#define RTW_SDIO_HIQ_BUDGET_MAX 16
> >  
> >  static bool rtw_sdio_is_bus_addr(u32 addr)
> >  {
> >  @@ -1299,6 +1303,64 @@ static void rtw_sdio_8723bs_wake_tx_queue(struct rtw_dev *rtwdev,
> >  ieee80211_wake_queue(rtwdev->hw, q_map);
> >  }
> >  
> >  +static bool rtw_sdio_bmc_needs_dtim(struct sk_buff *skb)
> >  +{
> >  + struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
> >  + struct udphdr *udphdr;
> >  +
> >  + if (info->control.flags & IEEE80211_TX_CTRL_PORT_CTRL_PROTO)
> >  + return true;
> >  +
> >  + if (skb->protocol == htons(ETH_P_ARP))
> >  + return true;
> >  +
> >  + if (skb->protocol == htons(ETH_P_IP) &&
> >  + ip_hdr(skb)->protocol == IPPROTO_UDP) {
> >  + udphdr = udp_hdr(skb);
> >  +
> >  + return udphdr->dest == htons(67) || udphdr->dest == htons(68);
> >  + }
> >  +
> >  + return false;
> >  +}
> > 
> This is totally the same as USB. To share with USB, move it to tx.c 
> as an export function or to tx.h as an inline function.
> 

I am aware of that, but ai wasn't sure what is preferred approach so I
resorted to changing as little shared code as possible to mitigate the
risk of regression on other drivers. I will make it shared between USB
and SDIO.

> > 
> > +
> >  +static bool rtw_sdio_hiq_take_budget(struct rtw_sdio *rtwsdio)
> >  +{
> >  + unsigned long flags, elapsed, add;
> >  + bool ok;
> >  +
> >  + spin_lock_irqsave(&rtwsdio->hiq_lock, flags);
> >  +
> >  + elapsed = jiffies - rtwsdio->hiq_refill;
> >  + add = elapsed / RTW_SDIO_HIQ_REFILL_INTERVAL;
> >  + if (add) {
> >  + rtwsdio->hiq_budget = min_t(unsigned long,
> >  + rtwsdio->hiq_budget + add,
> >  + RTW_SDIO_HIQ_BUDGET_MAX);
> >  +
> >  + /* keep the unfinished part of the interval for the next unit */
> >  + rtwsdio->hiq_refill = jiffies -
> >  + elapsed % RTW_SDIO_HIQ_REFILL_INTERVAL;
> >  + }
> >  +
> >  + ok = rtwsdio->hiq_budget > 0;
> >  + if (ok)
> >  + rtwsdio->hiq_budget--;
> >  +
> >  + spin_unlock_irqrestore(&rtwsdio->hiq_lock, flags);
> >  +
> >  + return ok;
> >  +}
> > 
> This is also the same as USB. Please consider how to share the logic.
>

Same as above.

Best regards,
Luka Gejak

Re: [PATCH rtw-next] wifi: rtw88: sdio: limit the high queue to DTIM delivery

[Luka Gejak]

October 21, 2026 at 6:23 AM, "Luka Gejak" <luka.gejak@linux.dev> wrote:


> 
> October 21, 2026 at 3:34 AM, "Ping-Ke Shih" <pkshih@realtek.com> wrote:
> 
> > 
> > Luka Gejak <luka.gejak@linux.dev> wrote:
> >  
> >  
[...]
> >  This is totally the same as USB. To share with USB, move it to tx.c 
> >  as an export function or to tx.h as an inline function.
> > 
> I am aware of that, but ai wasn't sure

                          *I wasn't sure

 what is preferred approach so I
> resorted to changing as little shared code as possible to mitigate the
> risk of regression on other drivers. I will make it shared between USB
> and SDIO.
> 
> > 
> > +
> >  +static bool rtw_sdio_hiq_take_budget(struct rtw_sdio *rtwsdio)
> >  +{
> >  + unsigned long flags, elapsed, add;
> >  + bool ok;
> >  +
> >  + spin_lock_irqsave(&rtwsdio->hiq_lock, flags);
> >  +
> >  + elapsed = jiffies - rtwsdio->hiq_refill;
> >  + add = elapsed / RTW_SDIO_HIQ_REFILL_INTERVAL;
> >  + if (add) {
> >  + rtwsdio->hiq_budget = min_t(unsigned long,
> >  + rtwsdio->hiq_budget + add,
> >  + RTW_SDIO_HIQ_BUDGET_MAX);
> >  +
> >  + /* keep the unfinished part of the interval for the next unit */
> >  + rtwsdio->hiq_refill = jiffies -
> >  + elapsed % RTW_SDIO_HIQ_REFILL_INTERVAL;
> >  + }
> >  +
> >  + ok = rtwsdio->hiq_budget > 0;
> >  + if (ok)
> >  + rtwsdio->hiq_budget--;
> >  +
> >  + spin_unlock_irqrestore(&rtwsdio->hiq_lock, flags);
> >  +
> >  + return ok;
> >  +}
> >  
> >  This is also the same as USB. Please consider how to share the logic.
> > 
> Same as above.
> 
> Best regards,
> Luka Gejak
>