* [PATCH] can: vcan: use per-CPU device stats
@ 2026-10-07 11:36 Yogesh Gaur
2026-10-07 13:20 ` Marc Kleine-Budde
2026-10-07 17:10 ` Oliver Hartkopp
0 siblings, 2 replies; 6+ messages in thread
From: Yogesh Gaur @ 2026-10-07 11:36 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol
Cc: linux-can, linux-kernel, Yogesh Gaur, syzbot+937a3a1fbfbc99d6fcaf
vcan_tx() and vcan_rx() update dev->stats with plain increments and
rely on the tx queue lock to serialise them. That only holds for a
single tx queue. vcan does not implement get_num_tx_queues(), so
rtnl_create_link() honours IFLA_NUM_TX_QUEUES and a vcan device can be
created with several queues, each with its own xmit lock. Two CPUs
transmitting on different queues then update the same counters
concurrently and lose increments:
BUG: KCSAN: data-race in vcan_tx / vcan_tx
read-write to 0xffff88811aad8228 of 8 bytes by interrupt on cpu 1:
vcan_tx+0x325/0x5d0 drivers/net/can/vcan.c:110
dev_hard_start_xmit+0x10c/0x380 net/core/dev.c:3969
__dev_queue_xmit+0xbfd/0x1ec0 net/core/dev.c:4958
can_send+0x584/0x720 net/can/af_can.c:279
bcm_can_tx+0x3ba/0x5b0 net/can/bcm.c:367
read-write to 0xffff88811aad8228 of 8 bytes by interrupt on cpu 0:
vcan_tx+0x325/0x5d0 drivers/net/can/vcan.c:110
value changed: 0x00000000000025ef -> 0x00000000000025f0
Switch the packet and byte counters to the core's per-CPU dstats.
The core allocates them for NETDEV_PCPU_STAT_DSTATS and folds them
together with dev->stats in dev_get_stats(), so tx_dropped from
can_dropped_invalid_skb() is still reported.
Reported-by: syzbot+937a3a1fbfbc99d6fcaf@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=937a3a1fbfbc99d6fcaf
Assisted-by: LLM
Signed-off-by: Yogesh Gaur <yogeshgaur.83@gmail.com>
---
drivers/net/can/vcan.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/net/can/vcan.c b/drivers/net/can/vcan.c
index 76e6b7b5c6a1..2ff90cb7eee9 100644
--- a/drivers/net/can/vcan.c
+++ b/drivers/net/can/vcan.c
@@ -71,10 +71,7 @@ MODULE_PARM_DESC(echo, "Echo sent frames (for testing). Default: 0 (Off)");
static void vcan_rx(struct sk_buff *skb, struct net_device *dev)
{
- struct net_device_stats *stats = &dev->stats;
-
- stats->rx_packets++;
- stats->rx_bytes += can_skb_get_data_len(skb);
+ dev_dstats_rx_add(dev, can_skb_get_data_len(skb));
skb->pkt_type = PACKET_BROADCAST;
skb->dev = dev;
@@ -85,7 +82,6 @@ static void vcan_rx(struct sk_buff *skb, struct net_device *dev)
static netdev_tx_t vcan_tx(struct sk_buff *skb, struct net_device *dev)
{
- struct net_device_stats *stats = &dev->stats;
unsigned int len;
int loop;
@@ -93,8 +89,7 @@ static netdev_tx_t vcan_tx(struct sk_buff *skb, struct net_device *dev)
return NETDEV_TX_OK;
len = can_skb_get_data_len(skb);
- stats->tx_packets++;
- stats->tx_bytes += len;
+ dev_dstats_tx_add(dev, len);
/* set flag whether this packet has to be looped back */
loop = skb->pkt_type == PACKET_LOOPBACK;
@@ -107,8 +102,7 @@ static netdev_tx_t vcan_tx(struct sk_buff *skb, struct net_device *dev)
/* only count the packets here, because the
* CAN core already did the echo for us
*/
- stats->rx_packets++;
- stats->rx_bytes += len;
+ dev_dstats_rx_add(dev, len);
}
consume_skb(skb);
return NETDEV_TX_OK;
@@ -185,6 +179,7 @@ static void vcan_setup(struct net_device *dev)
dev->netdev_ops = &vcan_netdev_ops;
dev->ethtool_ops = &vcan_ethtool_ops;
dev->needs_free_netdev = true;
+ dev->pcpu_stat_type = NETDEV_PCPU_STAT_DSTATS;
}
static struct rtnl_link_ops vcan_link_ops __read_mostly = {
--
2.55.0.windows.5
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] can: vcan: use per-CPU device stats
2026-10-07 11:36 [PATCH] can: vcan: use per-CPU device stats Yogesh Gaur
@ 2026-10-07 13:20 ` Marc Kleine-Budde
2026-10-08 4:15 ` Yogesh Gaur
2026-10-07 17:10 ` Oliver Hartkopp
1 sibling, 1 reply; 6+ messages in thread
From: Marc Kleine-Budde @ 2026-10-07 13:20 UTC (permalink / raw)
To: Yogesh Gaur
Cc: Vincent Mailhol, linux-can, linux-kernel, syzbot+937a3a1fbfbc99d6fcaf
[-- Attachment #1: Type: text/plain, Size: 2646 bytes --]
On 07.10.2026 17:06:11, Yogesh Gaur wrote:
> vcan_tx() and vcan_rx() update dev->stats with plain increments and
> rely on the tx queue lock to serialise them. That only holds for a
> single tx queue. vcan does not implement get_num_tx_queues(), so
> rtnl_create_link() honours IFLA_NUM_TX_QUEUES and a vcan device can be
> created with several queues, each with its own xmit lock. Two CPUs
> transmitting on different queues then update the same counters
> concurrently and lose increments:
>
> BUG: KCSAN: data-race in vcan_tx / vcan_tx
> read-write to 0xffff88811aad8228 of 8 bytes by interrupt on cpu 1:
> vcan_tx+0x325/0x5d0 drivers/net/can/vcan.c:110
> dev_hard_start_xmit+0x10c/0x380 net/core/dev.c:3969
> __dev_queue_xmit+0xbfd/0x1ec0 net/core/dev.c:4958
> can_send+0x584/0x720 net/can/af_can.c:279
> bcm_can_tx+0x3ba/0x5b0 net/can/bcm.c:367
> read-write to 0xffff88811aad8228 of 8 bytes by interrupt on cpu 0:
> vcan_tx+0x325/0x5d0 drivers/net/can/vcan.c:110
> value changed: 0x00000000000025ef -> 0x00000000000025f0
>
> Switch the packet and byte counters to the core's per-CPU dstats.
> The core allocates them for NETDEV_PCPU_STAT_DSTATS and folds them
> together with dev->stats in dev_get_stats(), so tx_dropped from
> can_dropped_invalid_skb() is still reported.
>
> Reported-by: syzbot+937a3a1fbfbc99d6fcaf@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=937a3a1fbfbc99d6fcaf
> Assisted-by: LLM
> Signed-off-by: Yogesh Gaur <yogeshgaur.83@gmail.com>
So far all drivers use the classic stats, and the CAN dev helper code
(in drivers/net/can/dev) also modifies these fields. A quick search
shows these places:
drivers/net/can/dev/rx-offload.c:48: struct net_device_stats *stats = &dev->stats;
drivers/net/can/dev/rx-offload.c:162: offload->dev->stats.rx_dropped++;
drivers/net/can/dev/rx-offload.c:163: offload->dev->stats.rx_fifo_errors++;
drivers/net/can/dev/rx-offload.c:249: struct net_device_stats *stats = &dev->stats;
drivers/net/can/dev/rx-offload.c:289: struct net_device_stats *stats = &dev->stats;
drivers/net/can/dev/skb.c:29: struct net_device_stats *stats = &dev->stats;
drivers/net/can/dev/skb.c:402: dev->stats.tx_dropped++;
Can you modify the code to update the dstats if they are active?
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung Nürnberg | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] can: vcan: use per-CPU device stats
2026-10-07 11:36 [PATCH] can: vcan: use per-CPU device stats Yogesh Gaur
2026-10-07 13:20 ` Marc Kleine-Budde
@ 2026-10-07 17:10 ` Oliver Hartkopp
2026-10-08 4:13 ` Yogesh Gaur
1 sibling, 1 reply; 6+ messages in thread
From: Oliver Hartkopp @ 2026-10-07 17:10 UTC (permalink / raw)
To: Yogesh Gaur, Marc Kleine-Budde, Vincent Mailhol
Cc: linux-can, linux-kernel, syzbot+937a3a1fbfbc99d6fcaf
On 07.10.26 13:36, Yogesh Gaur wrote:
> vcan_tx() and vcan_rx() update dev->stats with plain increments and
> rely on the tx queue lock to serialise them. That only holds for a
> single tx queue. vcan does not implement get_num_tx_queues(), so
> rtnl_create_link() honours IFLA_NUM_TX_QUEUES and a vcan device can be
> created with several queues
dev_dstats_tx_add() has been introduced in Linux 6.14 so it is probably
some can-next material for Linux 7.4+.
Btw. why don't we implement get_num_tx_queues() and return 1 as a
potential stable fix? This would add a correct locking via the existing
tx queue lock for all vcan statistics right?
Best regards,
Oliver
ps. vxcan should have a similar problem.
, each with its own xmit lock. Two CPUs
> transmitting on different queues then update the same counters
> concurrently and lose increments:
>
> BUG: KCSAN: data-race in vcan_tx / vcan_tx
> read-write to 0xffff88811aad8228 of 8 bytes by interrupt on cpu 1:
> vcan_tx+0x325/0x5d0 drivers/net/can/vcan.c:110
> dev_hard_start_xmit+0x10c/0x380 net/core/dev.c:3969
> __dev_queue_xmit+0xbfd/0x1ec0 net/core/dev.c:4958
> can_send+0x584/0x720 net/can/af_can.c:279
> bcm_can_tx+0x3ba/0x5b0 net/can/bcm.c:367
> read-write to 0xffff88811aad8228 of 8 bytes by interrupt on cpu 0:
> vcan_tx+0x325/0x5d0 drivers/net/can/vcan.c:110
> value changed: 0x00000000000025ef -> 0x00000000000025f0
>
> Switch the packet and byte counters to the core's per-CPU dstats.
> The core allocates them for NETDEV_PCPU_STAT_DSTATS and folds them
> together with dev->stats in dev_get_stats(), so tx_dropped from
> can_dropped_invalid_skb() is still reported.
>
> Reported-by: syzbot+937a3a1fbfbc99d6fcaf@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=937a3a1fbfbc99d6fcaf
> Assisted-by: LLM
> Signed-off-by: Yogesh Gaur <yogeshgaur.83@gmail.com>
> ---
> drivers/net/can/vcan.c | 13 ++++---------
> 1 file changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/can/vcan.c b/drivers/net/can/vcan.c
> index 76e6b7b5c6a1..2ff90cb7eee9 100644
> --- a/drivers/net/can/vcan.c
> +++ b/drivers/net/can/vcan.c
> @@ -71,10 +71,7 @@ MODULE_PARM_DESC(echo, "Echo sent frames (for testing). Default: 0 (Off)");
>
> static void vcan_rx(struct sk_buff *skb, struct net_device *dev)
> {
> - struct net_device_stats *stats = &dev->stats;
> -
> - stats->rx_packets++;
> - stats->rx_bytes += can_skb_get_data_len(skb);
> + dev_dstats_rx_add(dev, can_skb_get_data_len(skb));
>
> skb->pkt_type = PACKET_BROADCAST;
> skb->dev = dev;
> @@ -85,7 +82,6 @@ static void vcan_rx(struct sk_buff *skb, struct net_device *dev)
>
> static netdev_tx_t vcan_tx(struct sk_buff *skb, struct net_device *dev)
> {
> - struct net_device_stats *stats = &dev->stats;
> unsigned int len;
> int loop;
>
> @@ -93,8 +89,7 @@ static netdev_tx_t vcan_tx(struct sk_buff *skb, struct net_device *dev)
> return NETDEV_TX_OK;
>
> len = can_skb_get_data_len(skb);
> - stats->tx_packets++;
> - stats->tx_bytes += len;
> + dev_dstats_tx_add(dev, len);
>
> /* set flag whether this packet has to be looped back */
> loop = skb->pkt_type == PACKET_LOOPBACK;
> @@ -107,8 +102,7 @@ static netdev_tx_t vcan_tx(struct sk_buff *skb, struct net_device *dev)
> /* only count the packets here, because the
> * CAN core already did the echo for us
> */
> - stats->rx_packets++;
> - stats->rx_bytes += len;
> + dev_dstats_rx_add(dev, len);
> }
> consume_skb(skb);
> return NETDEV_TX_OK;
> @@ -185,6 +179,7 @@ static void vcan_setup(struct net_device *dev)
> dev->netdev_ops = &vcan_netdev_ops;
> dev->ethtool_ops = &vcan_ethtool_ops;
> dev->needs_free_netdev = true;
> + dev->pcpu_stat_type = NETDEV_PCPU_STAT_DSTATS;
> }
>
> static struct rtnl_link_ops vcan_link_ops __read_mostly = {
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] can: vcan: use per-CPU device stats
2026-10-07 17:10 ` Oliver Hartkopp
@ 2026-10-08 4:13 ` Yogesh Gaur
2026-10-08 6:12 ` Oliver Hartkopp
0 siblings, 1 reply; 6+ messages in thread
From: Yogesh Gaur @ 2026-10-08 4:13 UTC (permalink / raw)
To: socketcan
Cc: Marc Kleine-Budde, Vincent Mailhol, linux-can, linux-kernel,
syzbot+937a3a1fbfbc99d6fcaf
On Wed, Oct 7, 2026 at 10:41 PM Oliver Hartkopp <socketcan@hartkopp.net> wrote:
>
>
>
> On 07.10.26 13:36, Yogesh Gaur wrote:
> > vcan_tx() and vcan_rx() update dev->stats with plain increments and
> > rely on the tx queue lock to serialise them. That only holds for a
> > single tx queue. vcan does not implement get_num_tx_queues(), so
> > rtnl_create_link() honours IFLA_NUM_TX_QUEUES and a vcan device can be
> > created with several queues
>
> dev_dstats_tx_add() has been introduced in Linux 6.14 so it is probably
> some can-next material for Linux 7.4+.
Agreed.
>
> Btw. why don't we implement get_num_tx_queues() and return 1 as a
> potential stable fix? This would add a correct locking via the existing
> tx queue lock for all vcan statistics right?
The locking argument holds: vcan is not LLTX, and every counter update
(tx, the echo-mode rx in vcan_rx(), and tx_dropped from
can_dropped_invalid_skb()) happens in vcan_tx() under the xmit locak.
But get_num_tx_queues() alone does not stop the multi-queue case, as
rtnl_create_link() only consults it when userspace did not ask.
>
> Best regards,
> Oliver
>
> ps. vxcan should have a similar problem.
Yes - same pattern and the same single-queue argument works there.
Plan for v2: limiting vcan and vxcan to one TX queue, then the dstats
conversion for can-next.
Yogesh
>
> , each with its own xmit lock. Two CPUs
> > transmitting on different queues then update the same counters
> > concurrently and lose increments:
> >
> > BUG: KCSAN: data-race in vcan_tx / vcan_tx
> > read-write to 0xffff88811aad8228 of 8 bytes by interrupt on cpu 1:
> > vcan_tx+0x325/0x5d0 drivers/net/can/vcan.c:110
> > dev_hard_start_xmit+0x10c/0x380 net/core/dev.c:3969
> > __dev_queue_xmit+0xbfd/0x1ec0 net/core/dev.c:4958
> > can_send+0x584/0x720 net/can/af_can.c:279
> > bcm_can_tx+0x3ba/0x5b0 net/can/bcm.c:367
> > read-write to 0xffff88811aad8228 of 8 bytes by interrupt on cpu 0:
> > vcan_tx+0x325/0x5d0 drivers/net/can/vcan.c:110
> > value changed: 0x00000000000025ef -> 0x00000000000025f0
> >
> > Switch the packet and byte counters to the core's per-CPU dstats.
> > The core allocates them for NETDEV_PCPU_STAT_DSTATS and folds them
> > together with dev->stats in dev_get_stats(), so tx_dropped from
> > can_dropped_invalid_skb() is still reported.
> >
> > Reported-by: syzbot+937a3a1fbfbc99d6fcaf@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=937a3a1fbfbc99d6fcaf
> > Assisted-by: LLM
> > Signed-off-by: Yogesh Gaur <yogeshgaur.83@gmail.com>
> > ---
> > drivers/net/can/vcan.c | 13 ++++---------
> > 1 file changed, 4 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/net/can/vcan.c b/drivers/net/can/vcan.c
> > index 76e6b7b5c6a1..2ff90cb7eee9 100644
> > --- a/drivers/net/can/vcan.c
> > +++ b/drivers/net/can/vcan.c
> > @@ -71,10 +71,7 @@ MODULE_PARM_DESC(echo, "Echo sent frames (for testing). Default: 0 (Off)");
> >
> > static void vcan_rx(struct sk_buff *skb, struct net_device *dev)
> > {
> > - struct net_device_stats *stats = &dev->stats;
> > -
> > - stats->rx_packets++;
> > - stats->rx_bytes += can_skb_get_data_len(skb);
> > + dev_dstats_rx_add(dev, can_skb_get_data_len(skb));
> >
> > skb->pkt_type = PACKET_BROADCAST;
> > skb->dev = dev;
> > @@ -85,7 +82,6 @@ static void vcan_rx(struct sk_buff *skb, struct net_device *dev)
> >
> > static netdev_tx_t vcan_tx(struct sk_buff *skb, struct net_device *dev)
> > {
> > - struct net_device_stats *stats = &dev->stats;
> > unsigned int len;
> > int loop;
> >
> > @@ -93,8 +89,7 @@ static netdev_tx_t vcan_tx(struct sk_buff *skb, struct net_device *dev)
> > return NETDEV_TX_OK;
> >
> > len = can_skb_get_data_len(skb);
> > - stats->tx_packets++;
> > - stats->tx_bytes += len;
> > + dev_dstats_tx_add(dev, len);
> >
> > /* set flag whether this packet has to be looped back */
> > loop = skb->pkt_type == PACKET_LOOPBACK;
> > @@ -107,8 +102,7 @@ static netdev_tx_t vcan_tx(struct sk_buff *skb, struct net_device *dev)
> > /* only count the packets here, because the
> > * CAN core already did the echo for us
> > */
> > - stats->rx_packets++;
> > - stats->rx_bytes += len;
> > + dev_dstats_rx_add(dev, len);
> > }
> > consume_skb(skb);
> > return NETDEV_TX_OK;
> > @@ -185,6 +179,7 @@ static void vcan_setup(struct net_device *dev)
> > dev->netdev_ops = &vcan_netdev_ops;
> > dev->ethtool_ops = &vcan_ethtool_ops;
> > dev->needs_free_netdev = true;
> > + dev->pcpu_stat_type = NETDEV_PCPU_STAT_DSTATS;
> > }
> >
> > static struct rtnl_link_ops vcan_link_ops __read_mostly = {
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] can: vcan: use per-CPU device stats
2026-10-07 13:20 ` Marc Kleine-Budde
@ 2026-10-08 4:15 ` Yogesh Gaur
0 siblings, 0 replies; 6+ messages in thread
From: Yogesh Gaur @ 2026-10-08 4:15 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: Vincent Mailhol, linux-can, linux-kernel, syzbot+937a3a1fbfbc99d6fcaf
On Wed, Oct 7, 2026 at 6:50 PM Marc Kleine-Budde <mkl@pengutronix.de> wrote:
>
> On 07.10.2026 17:06:11, Yogesh Gaur wrote:
> > vcan_tx() and vcan_rx() update dev->stats with plain increments and
> > rely on the tx queue lock to serialise them. That only holds for a
> > single tx queue. vcan does not implement get_num_tx_queues(), so
> > rtnl_create_link() honours IFLA_NUM_TX_QUEUES and a vcan device can be
> > created with several queues, each with its own xmit lock. Two CPUs
> > transmitting on different queues then update the same counters
> > concurrently and lose increments:
> >
> > BUG: KCSAN: data-race in vcan_tx / vcan_tx
> > read-write to 0xffff88811aad8228 of 8 bytes by interrupt on cpu 1:
> > vcan_tx+0x325/0x5d0 drivers/net/can/vcan.c:110
> > dev_hard_start_xmit+0x10c/0x380 net/core/dev.c:3969
> > __dev_queue_xmit+0xbfd/0x1ec0 net/core/dev.c:4958
> > can_send+0x584/0x720 net/can/af_can.c:279
> > bcm_can_tx+0x3ba/0x5b0 net/can/bcm.c:367
> > read-write to 0xffff88811aad8228 of 8 bytes by interrupt on cpu 0:
> > vcan_tx+0x325/0x5d0 drivers/net/can/vcan.c:110
> > value changed: 0x00000000000025ef -> 0x00000000000025f0
> >
> > Switch the packet and byte counters to the core's per-CPU dstats.
> > The core allocates them for NETDEV_PCPU_STAT_DSTATS and folds them
> > together with dev->stats in dev_get_stats(), so tx_dropped from
> > can_dropped_invalid_skb() is still reported.
> >
> > Reported-by: syzbot+937a3a1fbfbc99d6fcaf@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=937a3a1fbfbc99d6fcaf
> > Assisted-by: LLM
> > Signed-off-by: Yogesh Gaur <yogeshgaur.83@gmail.com>
>
> So far all drivers use the classic stats, and the CAN dev helper code
> (in drivers/net/can/dev) also modifies these fields. A quick search
> shows these places:
>
> drivers/net/can/dev/rx-offload.c:48: struct net_device_stats *stats = &dev->stats;
> drivers/net/can/dev/rx-offload.c:162: offload->dev->stats.rx_dropped++;
> drivers/net/can/dev/rx-offload.c:163: offload->dev->stats.rx_fifo_errors++;
> drivers/net/can/dev/rx-offload.c:249: struct net_device_stats *stats = &dev->stats;
> drivers/net/can/dev/rx-offload.c:289: struct net_device_stats *stats = &dev->stats;
> drivers/net/can/dev/skb.c:29: struct net_device_stats *stats = &dev->stats;
> drivers/net/can/dev/skb.c:402: dev->stats.tx_dropped++;
>
> Can you modify the code to update the dstats if they are active?
Yes and as per Oliver's suggestion send race fix along with dstats
work on top for can-next.
Yogesh
>
> regards,
> Marc
>
> --
> Pengutronix e.K. | Marc Kleine-Budde |
> Embedded Linux | https://www.pengutronix.de |
> Vertretung Nürnberg | Phone: +49-5121-206917-129 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] can: vcan: use per-CPU device stats
2026-10-08 4:13 ` Yogesh Gaur
@ 2026-10-08 6:12 ` Oliver Hartkopp
0 siblings, 0 replies; 6+ messages in thread
From: Oliver Hartkopp @ 2026-10-08 6:12 UTC (permalink / raw)
To: Yogesh Gaur
Cc: Marc Kleine-Budde, Vincent Mailhol, linux-can, linux-kernel,
syzbot+937a3a1fbfbc99d6fcaf
On 08.10.26 06:13, Yogesh Gaur wrote:
> On Wed, Oct 7, 2026 at 10:41 PM Oliver Hartkopp <socketcan@hartkopp.net> wrote:
>>
>>
>>
>> On 07.10.26 13:36, Yogesh Gaur wrote:
>>> vcan_tx() and vcan_rx() update dev->stats with plain increments and
>>> rely on the tx queue lock to serialise them. That only holds for a
>>> single tx queue. vcan does not implement get_num_tx_queues(), so
>>> rtnl_create_link() honours IFLA_NUM_TX_QUEUES and a vcan device can be
>>> created with several queues
>>
>> dev_dstats_tx_add() has been introduced in Linux 6.14 so it is probably
>> some can-next material for Linux 7.4+.
>
> Agreed.
>
>>
>> Btw. why don't we implement get_num_tx_queues() and return 1 as a
>> potential stable fix? This would add a correct locking via the existing
>> tx queue lock for all vcan statistics right?
>
> The locking argument holds: vcan is not LLTX, and every counter update
> (tx, the echo-mode rx in vcan_rx(), and tx_dropped from
> can_dropped_invalid_skb()) happens in vcan_tx() under the xmit locak.
>
> But get_num_tx_queues() alone does not stop the multi-queue case, as
> rtnl_create_link() only consults it when userspace did not ask.
Right, but there is no reason why userspace should set this value -
except if you want to race in the stats now ;-)
>>
>> Best regards,
>> Oliver
>>
>> ps. vxcan should have a similar problem.
>
> Yes - same pattern and the same single-queue argument works there.
>
> Plan for v2: limiting vcan and vxcan to one TX queue, then the dstats
> conversion for can-next.
Great! This might then also cover the dstats for "real" CAN devices as
Marc already pointed out.
Many thanks!
Oliver
>
> Yogesh
>>
>> , each with its own xmit lock. Two CPUs
>>> transmitting on different queues then update the same counters
>>> concurrently and lose increments:
>>>
>>> BUG: KCSAN: data-race in vcan_tx / vcan_tx
>>> read-write to 0xffff88811aad8228 of 8 bytes by interrupt on cpu 1:
>>> vcan_tx+0x325/0x5d0 drivers/net/can/vcan.c:110
>>> dev_hard_start_xmit+0x10c/0x380 net/core/dev.c:3969
>>> __dev_queue_xmit+0xbfd/0x1ec0 net/core/dev.c:4958
>>> can_send+0x584/0x720 net/can/af_can.c:279
>>> bcm_can_tx+0x3ba/0x5b0 net/can/bcm.c:367
>>> read-write to 0xffff88811aad8228 of 8 bytes by interrupt on cpu 0:
>>> vcan_tx+0x325/0x5d0 drivers/net/can/vcan.c:110
>>> value changed: 0x00000000000025ef -> 0x00000000000025f0
>>>
>>> Switch the packet and byte counters to the core's per-CPU dstats.
>>> The core allocates them for NETDEV_PCPU_STAT_DSTATS and folds them
>>> together with dev->stats in dev_get_stats(), so tx_dropped from
>>> can_dropped_invalid_skb() is still reported.
>>>
>>> Reported-by: syzbot+937a3a1fbfbc99d6fcaf@syzkaller.appspotmail.com
>>> Closes: https://syzkaller.appspot.com/bug?extid=937a3a1fbfbc99d6fcaf
>>> Assisted-by: LLM
>>> Signed-off-by: Yogesh Gaur <yogeshgaur.83@gmail.com>
>>> ---
>>> drivers/net/can/vcan.c | 13 ++++---------
>>> 1 file changed, 4 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/net/can/vcan.c b/drivers/net/can/vcan.c
>>> index 76e6b7b5c6a1..2ff90cb7eee9 100644
>>> --- a/drivers/net/can/vcan.c
>>> +++ b/drivers/net/can/vcan.c
>>> @@ -71,10 +71,7 @@ MODULE_PARM_DESC(echo, "Echo sent frames (for testing). Default: 0 (Off)");
>>>
>>> static void vcan_rx(struct sk_buff *skb, struct net_device *dev)
>>> {
>>> - struct net_device_stats *stats = &dev->stats;
>>> -
>>> - stats->rx_packets++;
>>> - stats->rx_bytes += can_skb_get_data_len(skb);
>>> + dev_dstats_rx_add(dev, can_skb_get_data_len(skb));
>>>
>>> skb->pkt_type = PACKET_BROADCAST;
>>> skb->dev = dev;
>>> @@ -85,7 +82,6 @@ static void vcan_rx(struct sk_buff *skb, struct net_device *dev)
>>>
>>> static netdev_tx_t vcan_tx(struct sk_buff *skb, struct net_device *dev)
>>> {
>>> - struct net_device_stats *stats = &dev->stats;
>>> unsigned int len;
>>> int loop;
>>>
>>> @@ -93,8 +89,7 @@ static netdev_tx_t vcan_tx(struct sk_buff *skb, struct net_device *dev)
>>> return NETDEV_TX_OK;
>>>
>>> len = can_skb_get_data_len(skb);
>>> - stats->tx_packets++;
>>> - stats->tx_bytes += len;
>>> + dev_dstats_tx_add(dev, len);
>>>
>>> /* set flag whether this packet has to be looped back */
>>> loop = skb->pkt_type == PACKET_LOOPBACK;
>>> @@ -107,8 +102,7 @@ static netdev_tx_t vcan_tx(struct sk_buff *skb, struct net_device *dev)
>>> /* only count the packets here, because the
>>> * CAN core already did the echo for us
>>> */
>>> - stats->rx_packets++;
>>> - stats->rx_bytes += len;
>>> + dev_dstats_rx_add(dev, len);
>>> }
>>> consume_skb(skb);
>>> return NETDEV_TX_OK;
>>> @@ -185,6 +179,7 @@ static void vcan_setup(struct net_device *dev)
>>> dev->netdev_ops = &vcan_netdev_ops;
>>> dev->ethtool_ops = &vcan_ethtool_ops;
>>> dev->needs_free_netdev = true;
>>> + dev->pcpu_stat_type = NETDEV_PCPU_STAT_DSTATS;
>>> }
>>>
>>> static struct rtnl_link_ops vcan_link_ops __read_mostly = {
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-10-08 6:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-07 11:36 [PATCH] can: vcan: use per-CPU device stats Yogesh Gaur
2026-10-07 13:20 ` Marc Kleine-Budde
2026-10-08 4:15 ` Yogesh Gaur
2026-10-07 17:10 ` Oliver Hartkopp
2026-10-08 4:13 ` Yogesh Gaur
2026-10-08 6:12 ` Oliver Hartkopp
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®