Re: [PATCH net] net: wwan: qcom_bam_dmux: account network packets

[Stephan Gerhold <stephan.gerhold@linaro.org>]

On Sun, Aug 30, 2026 at 10:54:00AM +0200, Dmitry Sinyavin wrote:
> The BAM-DMUX data path does not update the network device packet and byte
> counters. As a result, userspace sees zero traffic even while packets are
> being transferred.
> 
> Use the standard per-CPU software statistics helpers. Account transmitted
> packets after their DMA completion and received packets after removing the
> BAM-DMUX header and padding.
> 
> Fixes: 21a0ffd9b38c ("net: wwan: Add Qualcomm BAM-DMUX WWAN network driver")
> Signed-off-by: Dmitry Sinyavin <sinyavin@gmail.com>

Thanks for the patch! A few minor comments:

> ---
> Build-tested for ARM with CONFIG_QCOM_BAM_DMUX=m using Clang and W=1.
> 
>  drivers/net/wwan/qcom_bam_dmux.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/net/wwan/qcom_bam_dmux.c b/drivers/net/wwan/qcom_bam_dmux.c
> index cc6ace8d6437..40c40bc5645a 100644
> --- a/drivers/net/wwan/qcom_bam_dmux.c
> +++ b/drivers/net/wwan/qcom_bam_dmux.c
> @@ -177,8 +177,15 @@ static void bam_dmux_tx_callback(void *data)
>  {
>  	struct bam_dmux_skb_dma *skb_dma = data;
>  	struct sk_buff *skb = skb_dma->skb;
> +	struct net_device *netdev = skb->dev;
> +	unsigned int len = 0;
> +
> +	if (netdev)
> +		len = ((struct bam_dmux_hdr *)skb->data)->len;
>  
>  	bam_dmux_tx_done(skb_dma);
> +	if (netdev)
> +		dev_sw_netstats_tx_add(netdev, 1, len);
>  	dev_consume_skb_any(skb);

This is a bit odd, why did you split the two if (netdev) statements?
The skb stays alive until it is freed here, so you should be able to
obtain the length even after bam_dmux_tx_done().

>  }
>  
> @@ -402,6 +409,7 @@ static const struct net_device_ops bam_dmux_ops = {
>  	.ndo_open	= bam_dmux_netdev_open,
>  	.ndo_stop	= bam_dmux_netdev_stop,
>  	.ndo_start_xmit	= bam_dmux_netdev_start_xmit,
> +	.ndo_get_stats64	= dev_get_tstats64,
>  };
>  
>  static const struct device_type wwan_type = {
> @@ -421,6 +429,7 @@ static void bam_dmux_netdev_setup(struct net_device *dev)
>  	dev->needed_headroom = sizeof(struct bam_dmux_hdr);
>  	dev->needed_tailroom = sizeof(u32); /* word-aligned */
>  	dev->tx_queue_len = DEFAULT_TX_QUEUE_LEN;
> +	dev->pcpu_stat_type = NETDEV_PCPU_STAT_TSTATS;
>  
>  	/* This perm addr will be used as interface identifier by IPv6 */
>  	dev->addr_assign_type = NET_ADDR_RANDOM;
> @@ -533,6 +542,7 @@ static void bam_dmux_cmd_data(struct bam_dmux_skb_dma *skb_dma)
>  		break;
>  	}
>  
> +	dev_sw_netstats_rx_add(netdev, skb->len);
>  	netif_receive_skb(skb);

Would it be better to increment the stats after the packet was already
passed to the network subsystem in this call?

I'm not sure if we need to check the return code of netif_receive_skb()
and increment rx_dropped if it fails. This seems to be handled
differently in various drivers. Maybe someone else knows?

Thanks,
Stephan