Re: [PATCH] wifi: mt76: mt76u: use GRO on the USB RX path

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 4557 bytes --]

> The USB RX path delivers frames to the stack via mt76_rx_complete() with
> a NULL napi pointer, which takes the netif_receive_skb_list() path and
> therefore never benefits from GRO. The DMA-based mt76 drivers pass a real
> napi and get napi_gro_receive(); the USB path does not. For bulk TCP
> traffic this is costly, as every segment traverses the network stack
> individually instead of being coalesced.
> 
> Add a small container NAPI on a dummy netdev that the RX worker drives
> manually: napi_schedule_prep() marks it scheduled, frames are delivered
> through napi_gro_receive(), and napi_complete_done() flushes the coalesced
> list. The poll handler is never invoked by the core.
> 
> On mt7921u at HE-MCS 11 (2x2, 80 MHz) this raises single-stream TCP
> download throughput from ~383 to ~475 Mbit/s (~+24%), averaged over six
> interleaved A/B measurements. The gain only applies while the link is not
> RF-limited, as expected for a host-side optimisation.
> 
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Filip Bakreski <phial@phiality.com>
> ---
>  drivers/net/wireless/mediatek/mt76/mt76.h |  4 +++
>  drivers/net/wireless/mediatek/mt76/usb.c  | 36 ++++++++++++++++++++++-
>  2 files changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
> index 07955555f..f5e52c1f4 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt76.h
> @@ -668,6 +668,10 @@ struct mt76_usb {
>  	struct mt76_worker status_worker;
>  	struct mt76_worker rx_worker;
>  
> +	/* container NAPI used only to batch GRO for the RX worker */
> +	struct net_device *napi_dev;
> +	struct napi_struct napi;

I guess we do not need to add them, we can just reuse napi_dev pointer and
napi[] array available in mt76_dev struct. Agree?

> +
>  	struct work_struct stat_work;
>  
>  	u8 out_ep[__MT_EP_OUT_MAX];
> diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
> index d9638a9b7..f9c48140c 100644
> --- a/drivers/net/wireless/mediatek/mt76/usb.c
> +++ b/drivers/net/wireless/mediatek/mt76/usb.c
> @@ -619,12 +619,31 @@ mt76u_process_rx_queue(struct mt76_dev *dev, struct mt76_queue *q)
>  		mt76u_submit_rx_buf(dev, qid, urb);
>  	}
>  	if (qid == MT_RXQ_MAIN) {
> +		struct napi_struct *napi = &dev->usb.napi;
> +
>  		local_bh_disable();
> -		mt76_rx_poll_complete(dev, MT_RXQ_MAIN, NULL);
> +		/* Drive a container NAPI so the RX path can use
> +		 * napi_gro_receive(): napi_schedule_prep() marks it SCHED and
> +		 * napi_complete_done() flushes the coalesced GRO list. The poll
> +		 * handler is never actually invoked by the core.
> +		 */
> +		if (dev->usb.napi_dev && napi_schedule_prep(napi)) {
> +			mt76_rx_poll_complete(dev, MT_RXQ_MAIN, napi);
> +			napi_complete_done(napi, 1);
> +		} else {
> +			mt76_rx_poll_complete(dev, MT_RXQ_MAIN, NULL);
> +		}
>  		local_bh_enable();
>  	}
>  }
>  
> +/* Never invoked by the core: the RX worker drives GRO via the napi manually. */
> +static int mt76u_napi_poll(struct napi_struct *napi, int budget)
> +{
> +	napi_complete(napi);
> +	return 0;
> +}
> +
>  static void mt76u_rx_worker(struct mt76_worker *w)
>  {
>  	struct mt76_usb *usb = container_of(w, struct mt76_usb, rx_worker);
> @@ -1051,6 +1070,13 @@ void mt76u_queues_deinit(struct mt76_dev *dev)
>  	mt76u_stop_rx(dev);
>  	mt76u_stop_tx(dev);
>  
> +	if (dev->usb.napi_dev) {
> +		napi_disable(&dev->usb.napi);
> +		netif_napi_del(&dev->usb.napi);
> +		free_netdev(dev->usb.napi_dev);
> +		dev->usb.napi_dev = NULL;
> +	}
> +
>  	mt76u_free_rx(dev);
>  	mt76u_free_tx(dev);
>  }
> @@ -1115,6 +1141,14 @@ int __mt76u_init(struct mt76_dev *dev, struct usb_interface *intf,
>  	sched_set_fifo_low(usb->rx_worker.task);
>  	sched_set_fifo_low(usb->status_worker.task);
>  
> +	/* container netdev + NAPI used only to enable GRO on the RX path */
> +	usb->napi_dev = alloc_netdev_dummy(0);
> +	if (!usb->napi_dev)
> +		return -ENOMEM;
> +	strscpy(usb->napi_dev->name, "mt76u-rx", sizeof(usb->napi_dev->name));
> +	netif_napi_add(usb->napi_dev, &usb->napi, mt76u_napi_poll);

I guess it would be interesting verifying if threaded-napi provides better
results.

Regards,
Lorenzo

> +	napi_enable(&usb->napi);
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(__mt76u_init);
> 
> base-commit: 5f6099446d1ddb888e36cdf93b6a0551f05c1267
> -- 
> 2.54.0
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]