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

["Luka Gejak" <luka.gejak@linux.dev>]

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