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

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

[Dmitry Sinyavin]

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>
---
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);
 }
 
@@ -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);
 }
 

base-commit: 2188569e7e1b0bc3f3b557dc97ab7a02befc11c8
-- 
2.53.0

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

[Stephan Gerhold]

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

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

[Dmitry Sinyavin]

Thanks for the review!

31.08.2026 11:23, Stephan Gerhold wrote:
> 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().

Agreed, will join them in v2.

> 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?

My understanding is that rx_packets counts good packets received by the 
host even if they are dropped in the stack downstream. I also checked 
that mac80211 follows the same pattern: first dev_sw_netstats_rx_add(), 
then netif_receive_skb(), ignoring the result (the documentation for the 
latter also states that the result is "usually ignored"). If there are 
no strong objections, I'd leave that as is.

Best regards,
Dmitry

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

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 1 · Medium: 0 (2) · Low: 0

- [High] Use-after-free write of the per-CPU netdev statistics from the
  BAM DMA completion tasklet.

Pre-existing issues:
- [Medium] Registered net_devices are never freed on driver removal.
- [Medium] bam_dmux_cmd_data() trims the RX skb to the modem-supplied
  hdr->len (skb_trim(skb, hdr->len), qcom_bam_dmux.c:529) after only…

--

net: wwan: qcom_bam_dmux: account network packets

The patch makes the BAM-DMUX driver maintain per-CPU software statistics
so that userspace no longer sees zero traffic.  TX bytes/packets are
accounted from the DMA completion callback and RX bytes/packets from the
data command handler, with dev_get_tstats64() wired up as
ndo_get_stats64.

A few questions below about the netdev lifetime in the new accounting
paths.

> 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);
>  }

[Severity: High]
Can this write into freed per-CPU memory during driver removal?

skb->dev here is a borrowed pointer with no reference taken, and
dev_sw_netstats_tx_add() expands to this_cpu_ptr(dev->tstats) plus
u64_stats_add().  Once the netdev is unregistered, netdev_run_todo()
calls netdev_do_free_pcpu_stats(), which for NETDEV_PCPU_STAT_TSTATS
does:

net/core/dev.c:netdev_do_free_pcpu_stats() {
	case NETDEV_PCPU_STAT_TSTATS:
		free_percpu(dev->tstats);
		break;
}

dev->tstats is not set to NULL there, and this runs regardless of
needs_free_netdev.

In bam_dmux_remove() the unregistration completes well before the TX
DMA channel is stopped:

drivers/net/wwan/qcom_bam_dmux.c:bam_dmux_remove() {
	unregister_netdevice_many(&list);
	rtnl_unlock();			/* netdev_run_todo(): tstats freed */
	cancel_work_sync(&dmux->tx_wakeup_work);
	...
	if (!wait_event_timeout(dmux->pc_wait, !dmux->rx, BAM_DMUX_REMOTE_TIMEOUT))
	...
	bam_dmux_power_off(dmux);	/* dmaengine_terminate_sync(dmux->tx) */
	bam_dmux_free_skbs(dmux->tx_skbs, DMA_TO_DEVICE);
}

Is any TX descriptor completing in that window (which includes a
wait_event_timeout() of up to BAM_DMUX_REMOTE_TIMEOUT) going to run
bam_dmux_tx_callback() with skb->dev pointing at the unregistered
netdev, and write through the already freed dev->tstats?

The presence of bam_dmux_free_skbs(dmux->tx_skbs, DMA_TO_DEVICE) at the
end of bam_dmux_remove() suggests outstanding TX skbs are an expected
state at that point.  bam_dmux_netdev_stop() only stops the queue and
sends a CLOSE command, so netdev_wait_allrefs_any() has nothing to wait
on for in-flight descriptors.

There is also the deferred submit path: bam_dmux_netdev_start_xmit()
can hand work to bam_dmux_tx_wakeup_work(), which blocks in
pm_runtime_resume_and_get() (bam_dmux_runtime_resume() can wait
several seconds) and only afterwards calls bam_dmux_skb_dma_submit_tx()
and dma_async_issue_pending().  Can descriptors be submitted there
after the netdevs and their tstats are already gone?

Would accounting TX in bam_dmux_netdev_start_xmit(), where the netdev is
guaranteed live, or terminating the DMA channels and clearing
dmux->netdevs[] before unregistering, avoid this?

[ ... ]

> @@ -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;

[Severity: High]
This is the line that makes register_netdevice() allocate dev->tstats and
netdev_run_todo() free it, so it is the other half of the question above.

[Severity: Medium]
This is a pre-existing issue and not introduced by this patch, but while
looking at bam_dmux_netdev_setup() I noticed it sets neither
dev->needs_free_netdev nor a priv_destructor, and bam_dmux_remove() only
does unregister_netdevice_queue()/unregister_netdevice_many() with no
matching free_netdev().  The only free_netdev() in the driver is the
register_netdev() failure path in bam_dmux_register_netdev_work():

	ret = register_netdev(netdev);
	if (ret) {
		...
		free_netdev(netdev);
		return;
	}

Since netdev_run_todo() only frees the device when dev->needs_free_netdev
is set, does every successfully registered wwan%d interface (up to
BAM_DMUX_NUM_CH) leak its struct net_device on unbind or module unload?

> @@ -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);
>  }

[Severity: High]
On the RX side, netdev comes from dmux->netdevs[hdr->ch], which is never
cleared on unregister, and the only gate is:

	if (!netdev || !netif_running(netdev)) {
		dev_warn(dmux->dev, "Data for inactive channel %u\n", hdr->ch);
		return;
	}

Since this check runs on the DMA tasklet while dev_close() and
unregistration can proceed in parallel, can the check pass and
dev_sw_netstats_rx_add() then write through a dev->tstats that
netdev_run_todo() has already freed?  Before this patch the same stale
pointer was only dereferenced for reads on this path.

[Severity: Medium]
This isn't a bug introduced by this patch, but the length now being
counted as rx_bytes is entirely modem-supplied.  bam_dmux_cmd_data()
only bounds it against BAM_DMUX_MAX_DATA_SIZE and then trims to it:

	if (hdr->len > BAM_DMUX_MAX_DATA_SIZE) {
		...
		return;
	}
	...
	skb_pull(skb, sizeof(*hdr));
	skb_trim(skb, hdr->len);

The RX buffers come from bam_dmux_skb_dma_queue_rx() and are neither
zeroed nor sized to the actual transfer:

	if (!skb_dma->skb) {
		skb_dma->skb = __netdev_alloc_skb(NULL, BAM_DMUX_BUFFER_SIZE, gfp);
		if (!skb_dma->skb)
			return false;
		skb_put(skb_dma->skb, BAM_DMUX_BUFFER_SIZE);
	}

bam_dmux_rx_callback() never consults a transferred length or residue,
and the buffers are reused across receives.  If the modem declares
hdr->len larger than the payload it actually DMA'd, are the trailing
bytes of the reused 2 KB buffer (previous packet contents, or
uninitialized heap on first use) passed to netif_receive_skb() and now
also counted by dev_sw_netstats_rx_add(netdev, skb->len)?

I could not confirm from the BAM DMA driver whether a short transfer is
reportable at all, so this part may not be reachable in practice.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260830085400.2542956-1-sinyavin%40gmail.com