[PATCH v2 net-next 0/3] net: use DEV_STATS_xxx() helpers in virtio_net and l2tp_eth

[PATCH v2 net-next 0/3] net: use DEV_STATS_xxx() helpers in virtio_net and l2tp_eth

[Eric Dumazet]

Inspired by another (minor) KCSAN syzbot report.
Both virtio_net and l2tp_eth can use DEV_STATS_xxx() helpers.

v2: removed unused @priv variable (Simon, kernel build bot)

Eric Dumazet (3):
  net: add DEV_STATS_READ() helper
  virtio_net: avoid data-races on dev->stats fields
  net: l2tp_eth: use generic dev->stats fields

 drivers/net/macsec.c      |  6 +++---
 drivers/net/virtio_net.c  | 30 +++++++++++++++---------------
 include/linux/netdevice.h |  1 +
 net/l2tp/l2tp_eth.c       | 34 ++++++++++++----------------------
 4 files changed, 31 insertions(+), 40 deletions(-)

-- 
2.42.0.459.ge4e396fd5e-goog

[PATCH v2 net-next 1/3] net: add DEV_STATS_READ() helper

[Eric Dumazet]

Companion of DEV_STATS_INC() & DEV_STATS_ADD().

This is going to be used in the series.

Use it in macsec_get_stats64().

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/net/macsec.c      | 6 +++---
 include/linux/netdevice.h | 1 +
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index b7e151439c4886f851ba87e27a34e33cd834069a..7a44e1cbe30558dafcc2bc1a6ee47faad2e60d15 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -3655,9 +3655,9 @@ static void macsec_get_stats64(struct net_device *dev,
 
 	dev_fetch_sw_netstats(s, dev->tstats);
 
-	s->rx_dropped = atomic_long_read(&dev->stats.__rx_dropped);
-	s->tx_dropped = atomic_long_read(&dev->stats.__tx_dropped);
-	s->rx_errors = atomic_long_read(&dev->stats.__rx_errors);
+	s->rx_dropped = DEV_STATS_READ(dev, rx_dropped);
+	s->tx_dropped = DEV_STATS_READ(dev, tx_dropped);
+	s->rx_errors = DEV_STATS_READ(dev, rx_errors);
 }
 
 static int macsec_get_iflink(const struct net_device *dev)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 7e520c14eb8c68cf2e3821c86c7c3d1f0d8ab226..e070a4540fbaf4a9cf310d5f53c4401840c72776 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -5236,5 +5236,6 @@ extern struct net_device *blackhole_netdev;
 #define DEV_STATS_INC(DEV, FIELD) atomic_long_inc(&(DEV)->stats.__##FIELD)
 #define DEV_STATS_ADD(DEV, FIELD, VAL) 	\
 		atomic_long_add((VAL), &(DEV)->stats.__##FIELD)
+#define DEV_STATS_READ(DEV, FIELD) atomic_long_read(&(DEV)->stats.__##FIELD)
 
 #endif	/* _LINUX_NETDEVICE_H */
-- 
2.42.0.459.ge4e396fd5e-goog

[PATCH v2 net-next 2/3] virtio_net: avoid data-races on dev->stats fields

[Eric Dumazet]

Use DEV_STATS_INC() and DEV_STATS_READ() which provide
atomicity on paths that can be used concurrently.

Reported-by: syzbot <syzkaller@googlegroups.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
---
 drivers/net/virtio_net.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index fe7f314d65c971fcad826fa0b3e44a956ef88940..1ed6bcc4cbb1b5836e2be7ae2909add95f3a231a 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1258,7 +1258,7 @@ static struct sk_buff *receive_small(struct net_device *dev,
 	if (unlikely(len > GOOD_PACKET_LEN)) {
 		pr_debug("%s: rx error: len %u exceeds max size %d\n",
 			 dev->name, len, GOOD_PACKET_LEN);
-		dev->stats.rx_length_errors++;
+		DEV_STATS_INC(dev, rx_length_errors);
 		goto err;
 	}
 
@@ -1323,7 +1323,7 @@ static void mergeable_buf_free(struct receive_queue *rq, int num_buf,
 		if (unlikely(!buf)) {
 			pr_debug("%s: rx error: %d buffers missing\n",
 				 dev->name, num_buf);
-			dev->stats.rx_length_errors++;
+			DEV_STATS_INC(dev, rx_length_errors);
 			break;
 		}
 		stats->bytes += len;
@@ -1432,7 +1432,7 @@ static int virtnet_build_xdp_buff_mrg(struct net_device *dev,
 			pr_debug("%s: rx error: %d buffers out of %d missing\n",
 				 dev->name, *num_buf,
 				 virtio16_to_cpu(vi->vdev, hdr->num_buffers));
-			dev->stats.rx_length_errors++;
+			DEV_STATS_INC(dev, rx_length_errors);
 			goto err;
 		}
 
@@ -1451,7 +1451,7 @@ static int virtnet_build_xdp_buff_mrg(struct net_device *dev,
 			put_page(page);
 			pr_debug("%s: rx error: len %u exceeds truesize %lu\n",
 				 dev->name, len, (unsigned long)(truesize - room));
-			dev->stats.rx_length_errors++;
+			DEV_STATS_INC(dev, rx_length_errors);
 			goto err;
 		}
 
@@ -1630,7 +1630,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
 	if (unlikely(len > truesize - room)) {
 		pr_debug("%s: rx error: len %u exceeds truesize %lu\n",
 			 dev->name, len, (unsigned long)(truesize - room));
-		dev->stats.rx_length_errors++;
+		DEV_STATS_INC(dev, rx_length_errors);
 		goto err_skb;
 	}
 
@@ -1662,7 +1662,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
 				 dev->name, num_buf,
 				 virtio16_to_cpu(vi->vdev,
 						 hdr->num_buffers));
-			dev->stats.rx_length_errors++;
+			DEV_STATS_INC(dev, rx_length_errors);
 			goto err_buf;
 		}
 
@@ -1676,7 +1676,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
 		if (unlikely(len > truesize - room)) {
 			pr_debug("%s: rx error: len %u exceeds truesize %lu\n",
 				 dev->name, len, (unsigned long)(truesize - room));
-			dev->stats.rx_length_errors++;
+			DEV_STATS_INC(dev, rx_length_errors);
 			goto err_skb;
 		}
 
@@ -1763,7 +1763,7 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
 
 	if (unlikely(len < vi->hdr_len + ETH_HLEN)) {
 		pr_debug("%s: short packet %i\n", dev->name, len);
-		dev->stats.rx_length_errors++;
+		DEV_STATS_INC(dev, rx_length_errors);
 		virtnet_rq_free_unused_buf(rq->vq, buf);
 		return;
 	}
@@ -1803,7 +1803,7 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
 	return;
 
 frame_err:
-	dev->stats.rx_frame_errors++;
+	DEV_STATS_INC(dev, rx_frame_errors);
 	dev_kfree_skb(skb);
 }
 
@@ -2349,12 +2349,12 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	/* This should not happen! */
 	if (unlikely(err)) {
-		dev->stats.tx_fifo_errors++;
+		DEV_STATS_INC(dev, tx_fifo_errors);
 		if (net_ratelimit())
 			dev_warn(&dev->dev,
 				 "Unexpected TXQ (%d) queue failure: %d\n",
 				 qnum, err);
-		dev->stats.tx_dropped++;
+		DEV_STATS_INC(dev, tx_dropped);
 		dev_kfree_skb_any(skb);
 		return NETDEV_TX_OK;
 	}
@@ -2573,10 +2573,10 @@ static void virtnet_stats(struct net_device *dev,
 		tot->tx_errors  += terrors;
 	}
 
-	tot->tx_dropped = dev->stats.tx_dropped;
-	tot->tx_fifo_errors = dev->stats.tx_fifo_errors;
-	tot->rx_length_errors = dev->stats.rx_length_errors;
-	tot->rx_frame_errors = dev->stats.rx_frame_errors;
+	tot->tx_dropped = DEV_STATS_READ(dev, tx_dropped);
+	tot->tx_fifo_errors = DEV_STATS_READ(dev, tx_fifo_errors);
+	tot->rx_length_errors = DEV_STATS_READ(dev, rx_length_errors);
+	tot->rx_frame_errors = DEV_STATS_READ(dev, rx_frame_errors);
 }
 
 static void virtnet_ack_link_announce(struct virtnet_info *vi)
-- 
2.42.0.459.ge4e396fd5e-goog

[PATCH v2 net-next 3/3] net: l2tp_eth: use generic dev->stats fields

[Eric Dumazet]

Core networking has opt-in atomic variant of dev->stats,
simply use DEV_STATS_INC(), DEV_STATS_ADD() and DEV_STATS_READ().

v2: removed @priv local var in l2tp_eth_dev_recv() (Simon)

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Simon Horman <horms@kernel.org>
---
 net/l2tp/l2tp_eth.c | 34 ++++++++++++----------------------
 1 file changed, 12 insertions(+), 22 deletions(-)

diff --git a/net/l2tp/l2tp_eth.c b/net/l2tp/l2tp_eth.c
index f2ae03c404736d826fd7dc327b1567eac1c8651a..25ca89f804145a0ed9b407011bb3013a745e50f3 100644
--- a/net/l2tp/l2tp_eth.c
+++ b/net/l2tp/l2tp_eth.c
@@ -37,12 +37,6 @@
 /* via netdev_priv() */
 struct l2tp_eth {
 	struct l2tp_session	*session;
-	atomic_long_t		tx_bytes;
-	atomic_long_t		tx_packets;
-	atomic_long_t		tx_dropped;
-	atomic_long_t		rx_bytes;
-	atomic_long_t		rx_packets;
-	atomic_long_t		rx_errors;
 };
 
 /* via l2tp_session_priv() */
@@ -79,10 +73,10 @@ static netdev_tx_t l2tp_eth_dev_xmit(struct sk_buff *skb, struct net_device *dev
 	int ret = l2tp_xmit_skb(session, skb);
 
 	if (likely(ret == NET_XMIT_SUCCESS)) {
-		atomic_long_add(len, &priv->tx_bytes);
-		atomic_long_inc(&priv->tx_packets);
+		DEV_STATS_ADD(dev, tx_bytes, len);
+		DEV_STATS_INC(dev, tx_packets);
 	} else {
-		atomic_long_inc(&priv->tx_dropped);
+		DEV_STATS_INC(dev, tx_dropped);
 	}
 	return NETDEV_TX_OK;
 }
@@ -90,14 +84,12 @@ static netdev_tx_t l2tp_eth_dev_xmit(struct sk_buff *skb, struct net_device *dev
 static void l2tp_eth_get_stats64(struct net_device *dev,
 				 struct rtnl_link_stats64 *stats)
 {
-	struct l2tp_eth *priv = netdev_priv(dev);
-
-	stats->tx_bytes   = (unsigned long)atomic_long_read(&priv->tx_bytes);
-	stats->tx_packets = (unsigned long)atomic_long_read(&priv->tx_packets);
-	stats->tx_dropped = (unsigned long)atomic_long_read(&priv->tx_dropped);
-	stats->rx_bytes   = (unsigned long)atomic_long_read(&priv->rx_bytes);
-	stats->rx_packets = (unsigned long)atomic_long_read(&priv->rx_packets);
-	stats->rx_errors  = (unsigned long)atomic_long_read(&priv->rx_errors);
+	stats->tx_bytes   = DEV_STATS_READ(dev, tx_bytes);
+	stats->tx_packets = DEV_STATS_READ(dev, tx_packets);
+	stats->tx_dropped = DEV_STATS_READ(dev, tx_dropped);
+	stats->rx_bytes   = DEV_STATS_READ(dev, rx_bytes);
+	stats->rx_packets = DEV_STATS_READ(dev, rx_packets);
+	stats->rx_errors  = DEV_STATS_READ(dev, rx_errors);
 }
 
 static const struct net_device_ops l2tp_eth_netdev_ops = {
@@ -126,7 +118,6 @@ static void l2tp_eth_dev_recv(struct l2tp_session *session, struct sk_buff *skb,
 {
 	struct l2tp_eth_sess *spriv = l2tp_session_priv(session);
 	struct net_device *dev;
-	struct l2tp_eth *priv;
 
 	if (!pskb_may_pull(skb, ETH_HLEN))
 		goto error;
@@ -144,12 +135,11 @@ static void l2tp_eth_dev_recv(struct l2tp_session *session, struct sk_buff *skb,
 	if (!dev)
 		goto error_rcu;
 
-	priv = netdev_priv(dev);
 	if (dev_forward_skb(dev, skb) == NET_RX_SUCCESS) {
-		atomic_long_inc(&priv->rx_packets);
-		atomic_long_add(data_len, &priv->rx_bytes);
+		DEV_STATS_INC(dev, rx_packets);
+		DEV_STATS_ADD(dev, rx_bytes, data_len);
 	} else {
-		atomic_long_inc(&priv->rx_errors);
+		DEV_STATS_INC(dev, rx_errors);
 	}
 	rcu_read_unlock();
 
-- 
2.42.0.459.ge4e396fd5e-goog

Re: [PATCH v2 net-next 0/3] net: use DEV_STATS_xxx() helpers in virtio_net and l2tp_eth

[Simon Horman]

On Thu, Sep 21, 2023 at 08:52:15AM +0000, Eric Dumazet wrote:
> Inspired by another (minor) KCSAN syzbot report.
> Both virtio_net and l2tp_eth can use DEV_STATS_xxx() helpers.
> 
> v2: removed unused @priv variable (Simon, kernel build bot)
> 
> Eric Dumazet (3):
>   net: add DEV_STATS_READ() helper
>   virtio_net: avoid data-races on dev->stats fields
>   net: l2tp_eth: use generic dev->stats fields

For series,

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH v2 net-next 0/3] net: use DEV_STATS_xxx() helpers in virtio_net and l2tp_eth

[Przemek Kitszel]

On 9/21/23 10:52, Eric Dumazet wrote:
> Inspired by another (minor) KCSAN syzbot report.
> Both virtio_net and l2tp_eth can use DEV_STATS_xxx() helpers.
> 
> v2: removed unused @priv variable (Simon, kernel build bot)
> 
> Eric Dumazet (3):
>    net: add DEV_STATS_READ() helper
>    virtio_net: avoid data-races on dev->stats fields
>    net: l2tp_eth: use generic dev->stats fields
> 
>   drivers/net/macsec.c      |  6 +++---
>   drivers/net/virtio_net.c  | 30 +++++++++++++++---------------
>   include/linux/netdevice.h |  1 +
>   net/l2tp/l2tp_eth.c       | 34 ++++++++++++----------------------
>   4 files changed, 31 insertions(+), 40 deletions(-)
> 

Nice read,
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>

Re: [PATCH v2 net-next 0/3] net: use DEV_STATS_xxx() helpers in virtio_net and l2tp_eth

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Thu, 21 Sep 2023 08:52:15 +0000 you wrote:
> Inspired by another (minor) KCSAN syzbot report.
> Both virtio_net and l2tp_eth can use DEV_STATS_xxx() helpers.
> 
> v2: removed unused @priv variable (Simon, kernel build bot)
> 
> Eric Dumazet (3):
>   net: add DEV_STATS_READ() helper
>   virtio_net: avoid data-races on dev->stats fields
>   net: l2tp_eth: use generic dev->stats fields
> 
> [...]

Here is the summary with links:
  - [v2,net-next,1/3] net: add DEV_STATS_READ() helper
    https://git.kernel.org/netdev/net-next/c/0b068c714ca9
  - [v2,net-next,2/3] virtio_net: avoid data-races on dev->stats fields
    https://git.kernel.org/netdev/net-next/c/d12a26b74fb7
  - [v2,net-next,3/3] net: l2tp_eth: use generic dev->stats fields
    https://git.kernel.org/netdev/net-next/c/a56d9390bd60

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html