netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/9] net:
@ 2022-06-07 23:36 Eric Dumazet
  2022-06-07 23:36 ` [PATCH net-next 1/9] vlan: adopt u64_stats_t Eric Dumazet
                   ` (8 more replies)
  0 siblings, 9 replies; 14+ messages in thread
From: Eric Dumazet @ 2022-06-07 23:36 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: netdev, Jason A . Donenfeld, Eric Dumazet, Eric Dumazet

From: Eric Dumazet <edumazet@google.com>

While KCSAN has not raised any reports yet, we should address the
potential load/store tearing problem happening with per cpu stats.

This series is not exhaustive, but hopefully a step in the right
direction.

Eric Dumazet (9):
  vlan: adopt u64_stats_t
  ipvlan: adopt u64_stats_t
  sit: use dev_sw_netstats_rx_add()
  ip6_tunnel: use dev_sw_netstats_rx_add()
  wireguard: use dev_sw_netstats_rx_add()
  net: adopt u64_stats_t in struct pcpu_sw_netstats
  devlink: adopt u64_stats_t
  drop_monitor: adopt u64_stats_t
  team: adopt u64_stats_t

 drivers/net/ipvlan/ipvlan.h      | 10 ++++-----
 drivers/net/ipvlan/ipvlan_core.c |  6 +++---
 drivers/net/ipvlan/ipvlan_main.c | 18 ++++++++--------
 drivers/net/macsec.c             |  8 +++----
 drivers/net/macvlan.c            | 18 ++++++++--------
 drivers/net/team/team.c          | 26 +++++++++++------------
 drivers/net/usb/usbnet.c         |  8 +++----
 drivers/net/vxlan/vxlan_core.c   |  8 +++----
 drivers/net/wireguard/receive.c  |  9 +-------
 include/linux/if_macvlan.h       |  6 +++---
 include/linux/if_team.h          | 10 ++++-----
 include/linux/if_vlan.h          | 10 ++++-----
 include/linux/netdevice.h        | 16 +++++++-------
 include/net/ip_tunnels.h         |  4 ++--
 net/8021q/vlan_core.c            |  6 +++---
 net/8021q/vlan_dev.c             | 18 ++++++++--------
 net/bridge/br_netlink.c          |  8 +++----
 net/bridge/br_vlan.c             | 36 ++++++++++++++++++--------------
 net/core/dev.c                   | 18 ++++++++--------
 net/core/devlink.c               | 28 ++++++++++++++-----------
 net/core/drop_monitor.c          | 18 ++++++++--------
 net/dsa/slave.c                  |  8 +++----
 net/ipv6/ip6_tunnel.c            |  7 +------
 net/ipv6/sit.c                   |  8 +------
 24 files changed, 151 insertions(+), 161 deletions(-)

-- 
2.36.1.255.ge46751e96f-goog


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH net-next 1/9] vlan: adopt u64_stats_t
  2022-06-07 23:36 [PATCH net-next 0/9] net: Eric Dumazet
@ 2022-06-07 23:36 ` Eric Dumazet
  2022-06-08 10:18   ` David Laight
  2022-06-07 23:36 ` [PATCH net-next 2/9] ipvlan: " Eric Dumazet
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: Eric Dumazet @ 2022-06-07 23:36 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: netdev, Jason A . Donenfeld, Eric Dumazet, Eric Dumazet

From: Eric Dumazet <edumazet@google.com>

As explained in commit 316580b69d0a ("u64_stats: provide u64_stats_t type")
we should use u64_stats_t and related accessors to avoid load/store tearing.

Add READ_ONCE() when reading rx_errors & tx_dropped.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/net/macvlan.c      | 18 +++++++++---------
 include/linux/if_macvlan.h |  6 +++---
 include/linux/if_vlan.h    | 10 +++++-----
 net/8021q/vlan_core.c      |  6 +++---
 net/8021q/vlan_dev.c       | 18 +++++++++---------
 5 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index eff75beb13957b81f8949922d0ffa29b68ebb3f6..0540a53250be2a8d991ef0e2fd18bea481ebf373 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -575,8 +575,8 @@ static netdev_tx_t macvlan_start_xmit(struct sk_buff *skb,
 
 		pcpu_stats = this_cpu_ptr(vlan->pcpu_stats);
 		u64_stats_update_begin(&pcpu_stats->syncp);
-		pcpu_stats->tx_packets++;
-		pcpu_stats->tx_bytes += len;
+		u64_stats_inc(&pcpu_stats->tx_packets);
+		u64_stats_add(&pcpu_stats->tx_bytes, len);
 		u64_stats_update_end(&pcpu_stats->syncp);
 	} else {
 		this_cpu_inc(vlan->pcpu_stats->tx_dropped);
@@ -949,11 +949,11 @@ static void macvlan_dev_get_stats64(struct net_device *dev,
 			p = per_cpu_ptr(vlan->pcpu_stats, i);
 			do {
 				start = u64_stats_fetch_begin_irq(&p->syncp);
-				rx_packets	= p->rx_packets;
-				rx_bytes	= p->rx_bytes;
-				rx_multicast	= p->rx_multicast;
-				tx_packets	= p->tx_packets;
-				tx_bytes	= p->tx_bytes;
+				rx_packets	= u64_stats_read(&p->rx_packets);
+				rx_bytes	= u64_stats_read(&p->rx_bytes);
+				rx_multicast	= u64_stats_read(&p->rx_multicast);
+				tx_packets	= u64_stats_read(&p->tx_packets);
+				tx_bytes	= u64_stats_read(&p->tx_bytes);
 			} while (u64_stats_fetch_retry_irq(&p->syncp, start));
 
 			stats->rx_packets	+= rx_packets;
@@ -964,8 +964,8 @@ static void macvlan_dev_get_stats64(struct net_device *dev,
 			/* rx_errors & tx_dropped are u32, updated
 			 * without syncp protection.
 			 */
-			rx_errors	+= p->rx_errors;
-			tx_dropped	+= p->tx_dropped;
+			rx_errors	+= READ_ONCE(p->rx_errors);
+			tx_dropped	+= READ_ONCE(p->tx_dropped);
 		}
 		stats->rx_errors	= rx_errors;
 		stats->rx_dropped	= rx_errors;
diff --git a/include/linux/if_macvlan.h b/include/linux/if_macvlan.h
index b422947390638a59a9a7e6b23c7d0e0c2eb84b12..523025106a643eebc2d3f948f8fe380bce376bbd 100644
--- a/include/linux/if_macvlan.h
+++ b/include/linux/if_macvlan.h
@@ -46,10 +46,10 @@ static inline void macvlan_count_rx(const struct macvlan_dev *vlan,
 
 		pcpu_stats = get_cpu_ptr(vlan->pcpu_stats);
 		u64_stats_update_begin(&pcpu_stats->syncp);
-		pcpu_stats->rx_packets++;
-		pcpu_stats->rx_bytes += len;
+		u64_stats_inc(&pcpu_stats->rx_packets);
+		u64_stats_add(&pcpu_stats->rx_bytes, len);
 		if (multicast)
-			pcpu_stats->rx_multicast++;
+			u64_stats_inc(&pcpu_stats->rx_multicast);
 		u64_stats_update_end(&pcpu_stats->syncp);
 		put_cpu_ptr(vlan->pcpu_stats);
 	} else {
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 2be4dd7e90a943785fe51d14ffd2d1ff9e0de8ee..e00c4ee81ff7f82e4343fe45c14d8e5d81d80e95 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -118,11 +118,11 @@ static inline void vlan_drop_rx_stag_filter_info(struct net_device *dev)
  *	@tx_dropped: number of tx drops
  */
 struct vlan_pcpu_stats {
-	u64			rx_packets;
-	u64			rx_bytes;
-	u64			rx_multicast;
-	u64			tx_packets;
-	u64			tx_bytes;
+	u64_stats_t		rx_packets;
+	u64_stats_t		rx_bytes;
+	u64_stats_t		rx_multicast;
+	u64_stats_t		tx_packets;
+	u64_stats_t		tx_bytes;
 	struct u64_stats_sync	syncp;
 	u32			rx_errors;
 	u32			tx_dropped;
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index acf8c791f3207bc86fc3d61bfde53e1857d43a28..5aa8144101dc6c82b6251ac9e2b25cc6eeda8fb4 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -63,10 +63,10 @@ bool vlan_do_receive(struct sk_buff **skbp)
 	rx_stats = this_cpu_ptr(vlan_dev_priv(vlan_dev)->vlan_pcpu_stats);
 
 	u64_stats_update_begin(&rx_stats->syncp);
-	rx_stats->rx_packets++;
-	rx_stats->rx_bytes += skb->len;
+	u64_stats_inc(&rx_stats->rx_packets);
+	u64_stats_add(&rx_stats->rx_bytes, skb->len);
 	if (skb->pkt_type == PACKET_MULTICAST)
-		rx_stats->rx_multicast++;
+		u64_stats_inc(&rx_stats->rx_multicast);
 	u64_stats_update_end(&rx_stats->syncp);
 
 	return true;
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 839f2020b015ecaed897bd6672d85b187f982765..968bcfcc16edfab38f45b655402f6543a8db9489 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -128,8 +128,8 @@ static netdev_tx_t vlan_dev_hard_start_xmit(struct sk_buff *skb,
 
 		stats = this_cpu_ptr(vlan->vlan_pcpu_stats);
 		u64_stats_update_begin(&stats->syncp);
-		stats->tx_packets++;
-		stats->tx_bytes += len;
+		u64_stats_inc(&stats->tx_packets);
+		u64_stats_add(&stats->tx_bytes, len);
 		u64_stats_update_end(&stats->syncp);
 	} else {
 		this_cpu_inc(vlan->vlan_pcpu_stats->tx_dropped);
@@ -713,11 +713,11 @@ static void vlan_dev_get_stats64(struct net_device *dev,
 		p = per_cpu_ptr(vlan_dev_priv(dev)->vlan_pcpu_stats, i);
 		do {
 			start = u64_stats_fetch_begin_irq(&p->syncp);
-			rxpackets	= p->rx_packets;
-			rxbytes		= p->rx_bytes;
-			rxmulticast	= p->rx_multicast;
-			txpackets	= p->tx_packets;
-			txbytes		= p->tx_bytes;
+			rxpackets	= u64_stats_read(&p->rx_packets);
+			rxbytes		= u64_stats_read(&p->rx_bytes);
+			rxmulticast	= u64_stats_read(&p->rx_multicast);
+			txpackets	= u64_stats_read(&p->tx_packets);
+			txbytes		= u64_stats_read(&p->tx_bytes);
 		} while (u64_stats_fetch_retry_irq(&p->syncp, start));
 
 		stats->rx_packets	+= rxpackets;
@@ -726,8 +726,8 @@ static void vlan_dev_get_stats64(struct net_device *dev,
 		stats->tx_packets	+= txpackets;
 		stats->tx_bytes		+= txbytes;
 		/* rx_errors & tx_dropped are u32 */
-		rx_errors	+= p->rx_errors;
-		tx_dropped	+= p->tx_dropped;
+		rx_errors	+= READ_ONCE(p->rx_errors);
+		tx_dropped	+= READ_ONCE(p->tx_dropped);
 	}
 	stats->rx_errors  = rx_errors;
 	stats->tx_dropped = tx_dropped;
-- 
2.36.1.255.ge46751e96f-goog


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next 2/9] ipvlan: adopt u64_stats_t
  2022-06-07 23:36 [PATCH net-next 0/9] net: Eric Dumazet
  2022-06-07 23:36 ` [PATCH net-next 1/9] vlan: adopt u64_stats_t Eric Dumazet
@ 2022-06-07 23:36 ` Eric Dumazet
  2022-06-07 23:36 ` [PATCH net-next 3/9] sit: use dev_sw_netstats_rx_add() Eric Dumazet
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Eric Dumazet @ 2022-06-07 23:36 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: netdev, Jason A . Donenfeld, Eric Dumazet, Eric Dumazet

From: Eric Dumazet <edumazet@google.com>

As explained in commit 316580b69d0a ("u64_stats: provide u64_stats_t type")
we should use u64_stats_t and related accessors to avoid load/store tearing.

Add READ_ONCE() when reading rx_errs & tx_drps.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/net/ipvlan/ipvlan.h      | 10 +++++-----
 drivers/net/ipvlan/ipvlan_core.c |  6 +++---
 drivers/net/ipvlan/ipvlan_main.c | 18 +++++++++---------
 3 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ipvlan/ipvlan.h b/drivers/net/ipvlan/ipvlan.h
index 3837c897832eaf6c325f06f86f16edf18775956a..de94921cbef9f2beb2b7e4f5ecde06eb4e5cff9c 100644
--- a/drivers/net/ipvlan/ipvlan.h
+++ b/drivers/net/ipvlan/ipvlan.h
@@ -47,11 +47,11 @@ typedef enum {
 } ipvl_hdr_type;
 
 struct ipvl_pcpu_stats {
-	u64			rx_pkts;
-	u64			rx_bytes;
-	u64			rx_mcast;
-	u64			tx_pkts;
-	u64			tx_bytes;
+	u64_stats_t		rx_pkts;
+	u64_stats_t		rx_bytes;
+	u64_stats_t		rx_mcast;
+	u64_stats_t		tx_pkts;
+	u64_stats_t		tx_bytes;
 	struct u64_stats_sync	syncp;
 	u32			rx_errs;
 	u32			tx_drps;
diff --git a/drivers/net/ipvlan/ipvlan_core.c b/drivers/net/ipvlan/ipvlan_core.c
index 6ffb27419e64b36666d395da924304a943343640..dfeb5b392e642885c3897018337f94fb41d10eab 100644
--- a/drivers/net/ipvlan/ipvlan_core.c
+++ b/drivers/net/ipvlan/ipvlan_core.c
@@ -19,10 +19,10 @@ void ipvlan_count_rx(const struct ipvl_dev *ipvlan,
 
 		pcptr = this_cpu_ptr(ipvlan->pcpu_stats);
 		u64_stats_update_begin(&pcptr->syncp);
-		pcptr->rx_pkts++;
-		pcptr->rx_bytes += len;
+		u64_stats_inc(&pcptr->rx_pkts);
+		u64_stats_add(&pcptr->rx_bytes, len);
 		if (mcast)
-			pcptr->rx_mcast++;
+			u64_stats_inc(&pcptr->rx_mcast);
 		u64_stats_update_end(&pcptr->syncp);
 	} else {
 		this_cpu_inc(ipvlan->pcpu_stats->rx_errs);
diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index aa28a29e228c0f3337b6c32d89d525e55a2c2021..49ba8a50dfb1e169beb137296fb3d3c1cfde4c19 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -224,8 +224,8 @@ static netdev_tx_t ipvlan_start_xmit(struct sk_buff *skb,
 		pcptr = this_cpu_ptr(ipvlan->pcpu_stats);
 
 		u64_stats_update_begin(&pcptr->syncp);
-		pcptr->tx_pkts++;
-		pcptr->tx_bytes += skblen;
+		u64_stats_inc(&pcptr->tx_pkts);
+		u64_stats_add(&pcptr->tx_bytes, skblen);
 		u64_stats_update_end(&pcptr->syncp);
 	} else {
 		this_cpu_inc(ipvlan->pcpu_stats->tx_drps);
@@ -300,11 +300,11 @@ static void ipvlan_get_stats64(struct net_device *dev,
 			pcptr = per_cpu_ptr(ipvlan->pcpu_stats, idx);
 			do {
 				strt= u64_stats_fetch_begin_irq(&pcptr->syncp);
-				rx_pkts = pcptr->rx_pkts;
-				rx_bytes = pcptr->rx_bytes;
-				rx_mcast = pcptr->rx_mcast;
-				tx_pkts = pcptr->tx_pkts;
-				tx_bytes = pcptr->tx_bytes;
+				rx_pkts = u64_stats_read(&pcptr->rx_pkts);
+				rx_bytes = u64_stats_read(&pcptr->rx_bytes);
+				rx_mcast = u64_stats_read(&pcptr->rx_mcast);
+				tx_pkts = u64_stats_read(&pcptr->tx_pkts);
+				tx_bytes = u64_stats_read(&pcptr->tx_bytes);
 			} while (u64_stats_fetch_retry_irq(&pcptr->syncp,
 							   strt));
 
@@ -315,8 +315,8 @@ static void ipvlan_get_stats64(struct net_device *dev,
 			s->tx_bytes += tx_bytes;
 
 			/* u32 values are updated without syncp protection. */
-			rx_errs += pcptr->rx_errs;
-			tx_drps += pcptr->tx_drps;
+			rx_errs += READ_ONCE(pcptr->rx_errs);
+			tx_drps += READ_ONCE(pcptr->tx_drps);
 		}
 		s->rx_errors = rx_errs;
 		s->rx_dropped = rx_errs;
-- 
2.36.1.255.ge46751e96f-goog


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next 3/9] sit: use dev_sw_netstats_rx_add()
  2022-06-07 23:36 [PATCH net-next 0/9] net: Eric Dumazet
  2022-06-07 23:36 ` [PATCH net-next 1/9] vlan: adopt u64_stats_t Eric Dumazet
  2022-06-07 23:36 ` [PATCH net-next 2/9] ipvlan: " Eric Dumazet
@ 2022-06-07 23:36 ` Eric Dumazet
  2022-06-07 23:36 ` [PATCH net-next 4/9] ip6_tunnel: " Eric Dumazet
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Eric Dumazet @ 2022-06-07 23:36 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: netdev, Jason A . Donenfeld, Eric Dumazet, Eric Dumazet

From: Eric Dumazet <edumazet@google.com>

We have a convenient helper, let's use it.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv6/sit.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index c0b138c2099256bdcb041caa72407f234963c601..d3254aa80a5251bea50dd061cc437dbf793f96fd 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -686,8 +686,6 @@ static int ipip6_rcv(struct sk_buff *skb)
 	tunnel = ipip6_tunnel_lookup(dev_net(skb->dev), skb->dev,
 				     iph->saddr, iph->daddr, sifindex);
 	if (tunnel) {
-		struct pcpu_sw_netstats *tstats;
-
 		if (tunnel->parms.iph.protocol != IPPROTO_IPV6 &&
 		    tunnel->parms.iph.protocol != 0)
 			goto out;
@@ -724,11 +722,7 @@ static int ipip6_rcv(struct sk_buff *skb)
 			}
 		}
 
-		tstats = this_cpu_ptr(tunnel->dev->tstats);
-		u64_stats_update_begin(&tstats->syncp);
-		tstats->rx_packets++;
-		tstats->rx_bytes += skb->len;
-		u64_stats_update_end(&tstats->syncp);
+		dev_sw_netstats_rx_add(tunnel->dev, skb->len);
 
 		netif_rx(skb);
 
-- 
2.36.1.255.ge46751e96f-goog


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next 4/9] ip6_tunnel: use dev_sw_netstats_rx_add()
  2022-06-07 23:36 [PATCH net-next 0/9] net: Eric Dumazet
                   ` (2 preceding siblings ...)
  2022-06-07 23:36 ` [PATCH net-next 3/9] sit: use dev_sw_netstats_rx_add() Eric Dumazet
@ 2022-06-07 23:36 ` Eric Dumazet
  2022-06-07 23:36 ` [PATCH net-next 5/9] wireguard: " Eric Dumazet
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Eric Dumazet @ 2022-06-07 23:36 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: netdev, Jason A . Donenfeld, Eric Dumazet, Eric Dumazet

From: Eric Dumazet <edumazet@google.com>

We have a convenient helper, let's use it.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv6/ip6_tunnel.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 19325b7600bba3a8c8bd5a41be9d2340186d44aa..ded5b813e21fa28ea3c9c9c10f6b89de893990c2 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -796,7 +796,6 @@ static int __ip6_tnl_rcv(struct ip6_tnl *tunnel, struct sk_buff *skb,
 						struct sk_buff *skb),
 			 bool log_ecn_err)
 {
-	struct pcpu_sw_netstats *tstats;
 	const struct ipv6hdr *ipv6h = ipv6_hdr(skb);
 	int err;
 
@@ -856,11 +855,7 @@ static int __ip6_tnl_rcv(struct ip6_tnl *tunnel, struct sk_buff *skb,
 		}
 	}
 
-	tstats = this_cpu_ptr(tunnel->dev->tstats);
-	u64_stats_update_begin(&tstats->syncp);
-	tstats->rx_packets++;
-	tstats->rx_bytes += skb->len;
-	u64_stats_update_end(&tstats->syncp);
+	dev_sw_netstats_rx_add(tunnel->dev, skb->len);
 
 	skb_scrub_packet(skb, !net_eq(tunnel->net, dev_net(tunnel->dev)));
 
-- 
2.36.1.255.ge46751e96f-goog


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next 5/9] wireguard: use dev_sw_netstats_rx_add()
  2022-06-07 23:36 [PATCH net-next 0/9] net: Eric Dumazet
                   ` (3 preceding siblings ...)
  2022-06-07 23:36 ` [PATCH net-next 4/9] ip6_tunnel: " Eric Dumazet
@ 2022-06-07 23:36 ` Eric Dumazet
  2022-06-08  7:36   ` Jason A. Donenfeld
  2022-06-07 23:36 ` [PATCH net-next 6/9] net: adopt u64_stats_t in struct pcpu_sw_netstats Eric Dumazet
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: Eric Dumazet @ 2022-06-07 23:36 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: netdev, Jason A . Donenfeld, Eric Dumazet, Eric Dumazet

From: Eric Dumazet <edumazet@google.com>

We have a convenient helper, let's use it.
This will make the following patch easier to review and smaller.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Jason A. Donenfeld <Jason@zx2c4.com>
---
 drivers/net/wireguard/receive.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/net/wireguard/receive.c b/drivers/net/wireguard/receive.c
index 7b8df406c7737398f0270361afcb196af4b6a76e..7135d51d2d872edb66c856379ce2923b214289e9 100644
--- a/drivers/net/wireguard/receive.c
+++ b/drivers/net/wireguard/receive.c
@@ -19,15 +19,8 @@
 /* Must be called with bh disabled. */
 static void update_rx_stats(struct wg_peer *peer, size_t len)
 {
-	struct pcpu_sw_netstats *tstats =
-		get_cpu_ptr(peer->device->dev->tstats);
-
-	u64_stats_update_begin(&tstats->syncp);
-	++tstats->rx_packets;
-	tstats->rx_bytes += len;
+	dev_sw_netstats_rx_add(peer->device->dev, len);
 	peer->rx_bytes += len;
-	u64_stats_update_end(&tstats->syncp);
-	put_cpu_ptr(tstats);
 }
 
 #define SKB_TYPE_LE32(skb) (((struct message_header *)(skb)->data)->type)
-- 
2.36.1.255.ge46751e96f-goog


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next 6/9] net: adopt u64_stats_t in struct pcpu_sw_netstats
  2022-06-07 23:36 [PATCH net-next 0/9] net: Eric Dumazet
                   ` (4 preceding siblings ...)
  2022-06-07 23:36 ` [PATCH net-next 5/9] wireguard: " Eric Dumazet
@ 2022-06-07 23:36 ` Eric Dumazet
  2022-06-07 23:36 ` [PATCH net-next 7/9] devlink: adopt u64_stats_t Eric Dumazet
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Eric Dumazet @ 2022-06-07 23:36 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: netdev, Jason A . Donenfeld, Eric Dumazet, Eric Dumazet

From: Eric Dumazet <edumazet@google.com>

As explained in commit 316580b69d0a ("u64_stats: provide u64_stats_t type")
we should use u64_stats_t and related accessors to avoid load/store tearing.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/net/macsec.c           |  8 ++++----
 drivers/net/usb/usbnet.c       |  8 ++++----
 drivers/net/vxlan/vxlan_core.c |  8 ++++----
 include/linux/netdevice.h      | 16 +++++++--------
 include/net/ip_tunnels.h       |  4 ++--
 net/bridge/br_netlink.c        |  8 ++++----
 net/bridge/br_vlan.c           | 36 +++++++++++++++++++---------------
 net/core/dev.c                 | 18 ++++++++---------
 net/dsa/slave.c                |  8 ++++----
 9 files changed, 59 insertions(+), 55 deletions(-)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 817577e713d709fb2961b3bdf195879234d08183..96e8d5fd90b2dba92b833de8b2dc7e682ef47a76 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -523,8 +523,8 @@ static void count_tx(struct net_device *dev, int ret, int len)
 		struct pcpu_sw_netstats *stats = this_cpu_ptr(dev->tstats);
 
 		u64_stats_update_begin(&stats->syncp);
-		stats->tx_packets++;
-		stats->tx_bytes += len;
+		u64_stats_inc(&stats->tx_packets);
+		u64_stats_add(&stats->tx_bytes, len);
 		u64_stats_update_end(&stats->syncp);
 	}
 }
@@ -825,8 +825,8 @@ static void count_rx(struct net_device *dev, int len)
 	struct pcpu_sw_netstats *stats = this_cpu_ptr(dev->tstats);
 
 	u64_stats_update_begin(&stats->syncp);
-	stats->rx_packets++;
-	stats->rx_bytes += len;
+	u64_stats_inc(&stats->rx_packets);
+	u64_stats_add(&stats->rx_bytes, len);
 	u64_stats_update_end(&stats->syncp);
 }
 
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 36b24ec1165043def00bfd151eb1b60929995deb..4409d6b24101075cb2c3a2eaaefe260b50cb36ad 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -337,8 +337,8 @@ void usbnet_skb_return (struct usbnet *dev, struct sk_buff *skb)
 		skb->protocol = eth_type_trans (skb, dev->net);
 
 	flags = u64_stats_update_begin_irqsave(&stats64->syncp);
-	stats64->rx_packets++;
-	stats64->rx_bytes += skb->len;
+	u64_stats_inc(&stats64->rx_packets);
+	u64_stats_add(&stats64->rx_bytes, skb->len);
 	u64_stats_update_end_irqrestore(&stats64->syncp, flags);
 
 	netif_dbg(dev, rx_status, dev->net, "< rx, len %zu, type 0x%x\n",
@@ -1258,8 +1258,8 @@ static void tx_complete (struct urb *urb)
 		unsigned long flags;
 
 		flags = u64_stats_update_begin_irqsave(&stats64->syncp);
-		stats64->tx_packets += entry->packets;
-		stats64->tx_bytes += entry->length;
+		u64_stats_add(&stats64->tx_packets, entry->packets);
+		u64_stats_add(&stats64->tx_bytes, entry->length);
 		u64_stats_update_end_irqrestore(&stats64->syncp, flags);
 	} else {
 		dev->net->stats.tx_errors++;
diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
index 265d4a0245e7fcb31f469061eaaf46baa47ebbab..8b0710b576c2109c135e86515ded416360383535 100644
--- a/drivers/net/vxlan/vxlan_core.c
+++ b/drivers/net/vxlan/vxlan_core.c
@@ -2385,15 +2385,15 @@ static void vxlan_encap_bypass(struct sk_buff *skb, struct vxlan_dev *src_vxlan,
 		vxlan_snoop(dev, &loopback, eth_hdr(skb)->h_source, 0, vni);
 
 	u64_stats_update_begin(&tx_stats->syncp);
-	tx_stats->tx_packets++;
-	tx_stats->tx_bytes += len;
+	u64_stats_inc(&tx_stats->tx_packets);
+	u64_stats_add(&tx_stats->tx_bytes, len);
 	u64_stats_update_end(&tx_stats->syncp);
 	vxlan_vnifilter_count(src_vxlan, vni, NULL, VXLAN_VNI_STATS_TX, len);
 
 	if (__netif_rx(skb) == NET_RX_SUCCESS) {
 		u64_stats_update_begin(&rx_stats->syncp);
-		rx_stats->rx_packets++;
-		rx_stats->rx_bytes += len;
+		u64_stats_inc(&rx_stats->rx_packets);
+		u64_stats_add(&rx_stats->rx_bytes, len);
 		u64_stats_update_end(&rx_stats->syncp);
 		vxlan_vnifilter_count(dst_vxlan, vni, NULL, VXLAN_VNI_STATS_RX,
 				      len);
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index f615a66c89e98b5d58e1b23d6674fa142106fb6e..a07fae3ef10817c77bdec59660fa0feb7cdeb406 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2636,10 +2636,10 @@ struct packet_offload {
 
 /* often modified stats are per-CPU, other are shared (netdev->stats) */
 struct pcpu_sw_netstats {
-	u64     rx_packets;
-	u64     rx_bytes;
-	u64     tx_packets;
-	u64     tx_bytes;
+	u64_stats_t		rx_packets;
+	u64_stats_t		rx_bytes;
+	u64_stats_t		tx_packets;
+	u64_stats_t		tx_bytes;
 	struct u64_stats_sync   syncp;
 } __aligned(4 * sizeof(u64));
 
@@ -2656,8 +2656,8 @@ static inline void dev_sw_netstats_rx_add(struct net_device *dev, unsigned int l
 	struct pcpu_sw_netstats *tstats = this_cpu_ptr(dev->tstats);
 
 	u64_stats_update_begin(&tstats->syncp);
-	tstats->rx_bytes += len;
-	tstats->rx_packets++;
+	u64_stats_add(&tstats->rx_bytes, len);
+	u64_stats_inc(&tstats->rx_packets);
 	u64_stats_update_end(&tstats->syncp);
 }
 
@@ -2668,8 +2668,8 @@ static inline void dev_sw_netstats_tx_add(struct net_device *dev,
 	struct pcpu_sw_netstats *tstats = this_cpu_ptr(dev->tstats);
 
 	u64_stats_update_begin(&tstats->syncp);
-	tstats->tx_bytes += len;
-	tstats->tx_packets += packets;
+	u64_stats_add(&tstats->tx_bytes, len);
+	u64_stats_add(&tstats->tx_packets, packets);
 	u64_stats_update_end(&tstats->syncp);
 }
 
diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
index c24fa934221dde1c59ae6519cee783233d19af48..70cbc4a726691de160e89e92dfb0700c77d3097b 100644
--- a/include/net/ip_tunnels.h
+++ b/include/net/ip_tunnels.h
@@ -456,8 +456,8 @@ static inline void iptunnel_xmit_stats(struct net_device *dev, int pkt_len)
 		struct pcpu_sw_netstats *tstats = get_cpu_ptr(dev->tstats);
 
 		u64_stats_update_begin(&tstats->syncp);
-		tstats->tx_bytes += pkt_len;
-		tstats->tx_packets++;
+		u64_stats_add(&tstats->tx_bytes, pkt_len);
+		u64_stats_inc(&tstats->tx_packets);
 		u64_stats_update_end(&tstats->syncp);
 		put_cpu_ptr(tstats);
 	} else {
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index bb01776d2d88c46ac29ba94c2647fe26ff3468df..1ef14a099c6b023dcf2b54e8ef8c9eeb6820e0e4 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -1770,10 +1770,10 @@ static int br_fill_linkxstats(struct sk_buff *skb,
 			if (v->vid == pvid)
 				vxi.flags |= BRIDGE_VLAN_INFO_PVID;
 			br_vlan_get_stats(v, &stats);
-			vxi.rx_bytes = stats.rx_bytes;
-			vxi.rx_packets = stats.rx_packets;
-			vxi.tx_bytes = stats.tx_bytes;
-			vxi.tx_packets = stats.tx_packets;
+			vxi.rx_bytes = u64_stats_read(&stats.rx_bytes);
+			vxi.rx_packets = u64_stats_read(&stats.rx_packets);
+			vxi.tx_bytes = u64_stats_read(&stats.tx_bytes);
+			vxi.tx_packets = u64_stats_read(&stats.tx_packets);
 
 			if (nla_put(skb, BRIDGE_XSTATS_VLAN, sizeof(vxi), &vxi))
 				goto nla_put_failure;
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 0f5e75ccac7957184b18acb8dc53876c12663dc0..6e53dc991409429f26316d2c407e01c50c47c664 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -505,8 +505,8 @@ struct sk_buff *br_handle_vlan(struct net_bridge *br,
 	if (br_opt_get(br, BROPT_VLAN_STATS_ENABLED)) {
 		stats = this_cpu_ptr(v->stats);
 		u64_stats_update_begin(&stats->syncp);
-		stats->tx_bytes += skb->len;
-		stats->tx_packets++;
+		u64_stats_add(&stats->tx_bytes, skb->len);
+		u64_stats_inc(&stats->tx_packets);
 		u64_stats_update_end(&stats->syncp);
 	}
 
@@ -624,8 +624,8 @@ static bool __allowed_ingress(const struct net_bridge *br,
 	if (br_opt_get(br, BROPT_VLAN_STATS_ENABLED)) {
 		stats = this_cpu_ptr(v->stats);
 		u64_stats_update_begin(&stats->syncp);
-		stats->rx_bytes += skb->len;
-		stats->rx_packets++;
+		u64_stats_add(&stats->rx_bytes, skb->len);
+		u64_stats_inc(&stats->rx_packets);
 		u64_stats_update_end(&stats->syncp);
 	}
 
@@ -1379,16 +1379,16 @@ void br_vlan_get_stats(const struct net_bridge_vlan *v,
 		cpu_stats = per_cpu_ptr(v->stats, i);
 		do {
 			start = u64_stats_fetch_begin_irq(&cpu_stats->syncp);
-			rxpackets = cpu_stats->rx_packets;
-			rxbytes = cpu_stats->rx_bytes;
-			txbytes = cpu_stats->tx_bytes;
-			txpackets = cpu_stats->tx_packets;
+			rxpackets = u64_stats_read(&cpu_stats->rx_packets);
+			rxbytes = u64_stats_read(&cpu_stats->rx_bytes);
+			txbytes = u64_stats_read(&cpu_stats->tx_bytes);
+			txpackets = u64_stats_read(&cpu_stats->tx_packets);
 		} while (u64_stats_fetch_retry_irq(&cpu_stats->syncp, start));
 
-		stats->rx_packets += rxpackets;
-		stats->rx_bytes += rxbytes;
-		stats->tx_bytes += txbytes;
-		stats->tx_packets += txpackets;
+		u64_stats_add(&stats->rx_packets, rxpackets);
+		u64_stats_add(&stats->rx_bytes, rxbytes);
+		u64_stats_add(&stats->tx_bytes, txbytes);
+		u64_stats_add(&stats->tx_packets, txpackets);
 	}
 }
 
@@ -1779,14 +1779,18 @@ static bool br_vlan_stats_fill(struct sk_buff *skb,
 		return false;
 
 	br_vlan_get_stats(v, &stats);
-	if (nla_put_u64_64bit(skb, BRIDGE_VLANDB_STATS_RX_BYTES, stats.rx_bytes,
+	if (nla_put_u64_64bit(skb, BRIDGE_VLANDB_STATS_RX_BYTES,
+			      u64_stats_read(&stats.rx_bytes),
 			      BRIDGE_VLANDB_STATS_PAD) ||
 	    nla_put_u64_64bit(skb, BRIDGE_VLANDB_STATS_RX_PACKETS,
-			      stats.rx_packets, BRIDGE_VLANDB_STATS_PAD) ||
-	    nla_put_u64_64bit(skb, BRIDGE_VLANDB_STATS_TX_BYTES, stats.tx_bytes,
+			      u64_stats_read(&stats.rx_packets),
+			      BRIDGE_VLANDB_STATS_PAD) ||
+	    nla_put_u64_64bit(skb, BRIDGE_VLANDB_STATS_TX_BYTES,
+			      u64_stats_read(&stats.tx_bytes),
 			      BRIDGE_VLANDB_STATS_PAD) ||
 	    nla_put_u64_64bit(skb, BRIDGE_VLANDB_STATS_TX_PACKETS,
-			      stats.tx_packets, BRIDGE_VLANDB_STATS_PAD))
+			      u64_stats_read(&stats.tx_packets),
+			      BRIDGE_VLANDB_STATS_PAD))
 		goto out_err;
 
 	nla_nest_end(skb, nest);
diff --git a/net/core/dev.c b/net/core/dev.c
index 08ce317fcec89609f6f8e9335b3d9f57e813024d..6ed775459f45e8df1d14233aed92b90d1a84cca2 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10459,23 +10459,23 @@ void dev_fetch_sw_netstats(struct rtnl_link_stats64 *s,
 	int cpu;
 
 	for_each_possible_cpu(cpu) {
+		u64 rx_packets, rx_bytes, tx_packets, tx_bytes;
 		const struct pcpu_sw_netstats *stats;
-		struct pcpu_sw_netstats tmp;
 		unsigned int start;
 
 		stats = per_cpu_ptr(netstats, cpu);
 		do {
 			start = u64_stats_fetch_begin_irq(&stats->syncp);
-			tmp.rx_packets = stats->rx_packets;
-			tmp.rx_bytes   = stats->rx_bytes;
-			tmp.tx_packets = stats->tx_packets;
-			tmp.tx_bytes   = stats->tx_bytes;
+			rx_packets = u64_stats_read(&stats->rx_packets);
+			rx_bytes   = u64_stats_read(&stats->rx_bytes);
+			tx_packets = u64_stats_read(&stats->tx_packets);
+			tx_bytes   = u64_stats_read(&stats->tx_bytes);
 		} while (u64_stats_fetch_retry_irq(&stats->syncp, start));
 
-		s->rx_packets += tmp.rx_packets;
-		s->rx_bytes   += tmp.rx_bytes;
-		s->tx_packets += tmp.tx_packets;
-		s->tx_bytes   += tmp.tx_bytes;
+		s->rx_packets += rx_packets;
+		s->rx_bytes   += rx_bytes;
+		s->tx_packets += tx_packets;
+		s->tx_bytes   += tx_bytes;
 	}
 }
 EXPORT_SYMBOL_GPL(dev_fetch_sw_netstats);
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 801a5d445833c0f6b0328fc1adb9508d79244e90..2e1ac638d135e8b83cd80f83a67736e31f387afa 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -935,10 +935,10 @@ static void dsa_slave_get_ethtool_stats(struct net_device *dev,
 		s = per_cpu_ptr(dev->tstats, i);
 		do {
 			start = u64_stats_fetch_begin_irq(&s->syncp);
-			tx_packets = s->tx_packets;
-			tx_bytes = s->tx_bytes;
-			rx_packets = s->rx_packets;
-			rx_bytes = s->rx_bytes;
+			tx_packets = u64_stats_read(&s->tx_packets);
+			tx_bytes = u64_stats_read(&s->tx_bytes);
+			rx_packets = u64_stats_read(&s->rx_packets);
+			rx_bytes = u64_stats_read(&s->rx_bytes);
 		} while (u64_stats_fetch_retry_irq(&s->syncp, start));
 		data[0] += tx_packets;
 		data[1] += tx_bytes;
-- 
2.36.1.255.ge46751e96f-goog


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next 7/9] devlink: adopt u64_stats_t
  2022-06-07 23:36 [PATCH net-next 0/9] net: Eric Dumazet
                   ` (5 preceding siblings ...)
  2022-06-07 23:36 ` [PATCH net-next 6/9] net: adopt u64_stats_t in struct pcpu_sw_netstats Eric Dumazet
@ 2022-06-07 23:36 ` Eric Dumazet
  2022-06-07 23:36 ` [PATCH net-next 8/9] drop_monitor: " Eric Dumazet
  2022-06-07 23:36 ` [PATCH net-next 9/9] team: " Eric Dumazet
  8 siblings, 0 replies; 14+ messages in thread
From: Eric Dumazet @ 2022-06-07 23:36 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: netdev, Jason A . Donenfeld, Eric Dumazet, Eric Dumazet

From: Eric Dumazet <edumazet@google.com>

As explained in commit 316580b69d0a ("u64_stats: provide u64_stats_t type")
we should use u64_stats_t and related accessors to avoid load/store tearing.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/core/devlink.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index 5cc88490f18fd24329df0a83a425ed680b7a10fb..db61f3a341cb24b4de79198db7ae11b5e3132d42 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -7946,8 +7946,8 @@ static int devlink_nl_cmd_health_reporter_test_doit(struct sk_buff *skb,
 }
 
 struct devlink_stats {
-	u64 rx_bytes;
-	u64 rx_packets;
+	u64_stats_t rx_bytes;
+	u64_stats_t rx_packets;
 	struct u64_stats_sync syncp;
 };
 
@@ -8104,12 +8104,12 @@ static void devlink_trap_stats_read(struct devlink_stats __percpu *trap_stats,
 		cpu_stats = per_cpu_ptr(trap_stats, i);
 		do {
 			start = u64_stats_fetch_begin_irq(&cpu_stats->syncp);
-			rx_packets = cpu_stats->rx_packets;
-			rx_bytes = cpu_stats->rx_bytes;
+			rx_packets = u64_stats_read(&cpu_stats->rx_packets);
+			rx_bytes = u64_stats_read(&cpu_stats->rx_bytes);
 		} while (u64_stats_fetch_retry_irq(&cpu_stats->syncp, start));
 
-		stats->rx_packets += rx_packets;
-		stats->rx_bytes += rx_bytes;
+		u64_stats_add(&stats->rx_packets, rx_packets);
+		u64_stats_add(&stats->rx_bytes, rx_bytes);
 	}
 }
 
@@ -8127,11 +8127,13 @@ devlink_trap_group_stats_put(struct sk_buff *msg,
 		return -EMSGSIZE;
 
 	if (nla_put_u64_64bit(msg, DEVLINK_ATTR_STATS_RX_PACKETS,
-			      stats.rx_packets, DEVLINK_ATTR_PAD))
+			      u64_stats_read(&stats.rx_packets),
+			      DEVLINK_ATTR_PAD))
 		goto nla_put_failure;
 
 	if (nla_put_u64_64bit(msg, DEVLINK_ATTR_STATS_RX_BYTES,
-			      stats.rx_bytes, DEVLINK_ATTR_PAD))
+			      u64_stats_read(&stats.rx_bytes),
+			      DEVLINK_ATTR_PAD))
 		goto nla_put_failure;
 
 	nla_nest_end(msg, attr);
@@ -8171,11 +8173,13 @@ static int devlink_trap_stats_put(struct sk_buff *msg, struct devlink *devlink,
 		goto nla_put_failure;
 
 	if (nla_put_u64_64bit(msg, DEVLINK_ATTR_STATS_RX_PACKETS,
-			      stats.rx_packets, DEVLINK_ATTR_PAD))
+			      u64_stats_read(&stats.rx_packets),
+			      DEVLINK_ATTR_PAD))
 		goto nla_put_failure;
 
 	if (nla_put_u64_64bit(msg, DEVLINK_ATTR_STATS_RX_BYTES,
-			      stats.rx_bytes, DEVLINK_ATTR_PAD))
+			      u64_stats_read(&stats.rx_bytes),
+			      DEVLINK_ATTR_PAD))
 		goto nla_put_failure;
 
 	nla_nest_end(msg, attr);
@@ -11641,8 +11645,8 @@ devlink_trap_stats_update(struct devlink_stats __percpu *trap_stats,
 
 	stats = this_cpu_ptr(trap_stats);
 	u64_stats_update_begin(&stats->syncp);
-	stats->rx_bytes += skb_len;
-	stats->rx_packets++;
+	u64_stats_add(&stats->rx_bytes, skb_len);
+	u64_stats_inc(&stats->rx_packets);
 	u64_stats_update_end(&stats->syncp);
 }
 
-- 
2.36.1.255.ge46751e96f-goog


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next 8/9] drop_monitor: adopt u64_stats_t
  2022-06-07 23:36 [PATCH net-next 0/9] net: Eric Dumazet
                   ` (6 preceding siblings ...)
  2022-06-07 23:36 ` [PATCH net-next 7/9] devlink: adopt u64_stats_t Eric Dumazet
@ 2022-06-07 23:36 ` Eric Dumazet
  2022-06-07 23:36 ` [PATCH net-next 9/9] team: " Eric Dumazet
  8 siblings, 0 replies; 14+ messages in thread
From: Eric Dumazet @ 2022-06-07 23:36 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: netdev, Jason A . Donenfeld, Eric Dumazet, Eric Dumazet

From: Eric Dumazet <edumazet@google.com>

As explained in commit 316580b69d0a ("u64_stats: provide u64_stats_t type")
we should use u64_stats_t and related accessors to avoid load/store tearing.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/core/drop_monitor.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index 4ad1decce72418818aa4c2415b202e4311061a8c..98952ffcee452fe8437b0d020ce834496cd26570 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -55,7 +55,7 @@ static bool monitor_hw;
 static DEFINE_MUTEX(net_dm_mutex);
 
 struct net_dm_stats {
-	u64 dropped;
+	u64_stats_t dropped;
 	struct u64_stats_sync syncp;
 };
 
@@ -530,7 +530,7 @@ static void net_dm_packet_trace_kfree_skb_hit(void *ignore,
 unlock_free:
 	spin_unlock_irqrestore(&data->drop_queue.lock, flags);
 	u64_stats_update_begin(&data->stats.syncp);
-	data->stats.dropped++;
+	u64_stats_inc(&data->stats.dropped);
 	u64_stats_update_end(&data->stats.syncp);
 	consume_skb(nskb);
 }
@@ -985,7 +985,7 @@ net_dm_hw_trap_packet_probe(void *ignore, const struct devlink *devlink,
 unlock_free:
 	spin_unlock_irqrestore(&hw_data->drop_queue.lock, flags);
 	u64_stats_update_begin(&hw_data->stats.syncp);
-	hw_data->stats.dropped++;
+	u64_stats_inc(&hw_data->stats.dropped);
 	u64_stats_update_end(&hw_data->stats.syncp);
 	net_dm_hw_metadata_free(n_hw_metadata);
 free:
@@ -1432,10 +1432,10 @@ static void net_dm_stats_read(struct net_dm_stats *stats)
 
 		do {
 			start = u64_stats_fetch_begin_irq(&cpu_stats->syncp);
-			dropped = cpu_stats->dropped;
+			dropped = u64_stats_read(&cpu_stats->dropped);
 		} while (u64_stats_fetch_retry_irq(&cpu_stats->syncp, start));
 
-		stats->dropped += dropped;
+		u64_stats_add(&stats->dropped, dropped);
 	}
 }
 
@@ -1451,7 +1451,7 @@ static int net_dm_stats_put(struct sk_buff *msg)
 		return -EMSGSIZE;
 
 	if (nla_put_u64_64bit(msg, NET_DM_ATTR_STATS_DROPPED,
-			      stats.dropped, NET_DM_ATTR_PAD))
+			      u64_stats_read(&stats.dropped), NET_DM_ATTR_PAD))
 		goto nla_put_failure;
 
 	nla_nest_end(msg, attr);
@@ -1476,10 +1476,10 @@ static void net_dm_hw_stats_read(struct net_dm_stats *stats)
 
 		do {
 			start = u64_stats_fetch_begin_irq(&cpu_stats->syncp);
-			dropped = cpu_stats->dropped;
+			dropped = u64_stats_read(&cpu_stats->dropped);
 		} while (u64_stats_fetch_retry_irq(&cpu_stats->syncp, start));
 
-		stats->dropped += dropped;
+		u64_stats_add(&stats->dropped, dropped);
 	}
 }
 
@@ -1495,7 +1495,7 @@ static int net_dm_hw_stats_put(struct sk_buff *msg)
 		return -EMSGSIZE;
 
 	if (nla_put_u64_64bit(msg, NET_DM_ATTR_STATS_DROPPED,
-			      stats.dropped, NET_DM_ATTR_PAD))
+			      u64_stats_read(&stats.dropped), NET_DM_ATTR_PAD))
 		goto nla_put_failure;
 
 	nla_nest_end(msg, attr);
-- 
2.36.1.255.ge46751e96f-goog


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next 9/9] team: adopt u64_stats_t
  2022-06-07 23:36 [PATCH net-next 0/9] net: Eric Dumazet
                   ` (7 preceding siblings ...)
  2022-06-07 23:36 ` [PATCH net-next 8/9] drop_monitor: " Eric Dumazet
@ 2022-06-07 23:36 ` Eric Dumazet
  8 siblings, 0 replies; 14+ messages in thread
From: Eric Dumazet @ 2022-06-07 23:36 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: netdev, Jason A . Donenfeld, Eric Dumazet, Eric Dumazet

From: Eric Dumazet <edumazet@google.com>

As explained in commit 316580b69d0a ("u64_stats: provide u64_stats_t type")
we should use u64_stats_t and related accessors to avoid load/store tearing.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/net/team/team.c | 26 +++++++++++++-------------
 include/linux/if_team.h | 10 +++++-----
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index b07dde6f0abf273195ff0f60217bd7158535b153..aac133a1e27a5f64fe8f83a456aa0598fad6824c 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -749,10 +749,10 @@ static rx_handler_result_t team_handle_frame(struct sk_buff **pskb)
 
 		pcpu_stats = this_cpu_ptr(team->pcpu_stats);
 		u64_stats_update_begin(&pcpu_stats->syncp);
-		pcpu_stats->rx_packets++;
-		pcpu_stats->rx_bytes += skb->len;
+		u64_stats_inc(&pcpu_stats->rx_packets);
+		u64_stats_add(&pcpu_stats->rx_bytes, skb->len);
 		if (skb->pkt_type == PACKET_MULTICAST)
-			pcpu_stats->rx_multicast++;
+			u64_stats_inc(&pcpu_stats->rx_multicast);
 		u64_stats_update_end(&pcpu_stats->syncp);
 
 		skb->dev = team->dev;
@@ -1720,8 +1720,8 @@ static netdev_tx_t team_xmit(struct sk_buff *skb, struct net_device *dev)
 
 		pcpu_stats = this_cpu_ptr(team->pcpu_stats);
 		u64_stats_update_begin(&pcpu_stats->syncp);
-		pcpu_stats->tx_packets++;
-		pcpu_stats->tx_bytes += len;
+		u64_stats_inc(&pcpu_stats->tx_packets);
+		u64_stats_add(&pcpu_stats->tx_bytes, len);
 		u64_stats_update_end(&pcpu_stats->syncp);
 	} else {
 		this_cpu_inc(team->pcpu_stats->tx_dropped);
@@ -1854,11 +1854,11 @@ team_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats)
 		p = per_cpu_ptr(team->pcpu_stats, i);
 		do {
 			start = u64_stats_fetch_begin_irq(&p->syncp);
-			rx_packets	= p->rx_packets;
-			rx_bytes	= p->rx_bytes;
-			rx_multicast	= p->rx_multicast;
-			tx_packets	= p->tx_packets;
-			tx_bytes	= p->tx_bytes;
+			rx_packets	= u64_stats_read(&p->rx_packets);
+			rx_bytes	= u64_stats_read(&p->rx_bytes);
+			rx_multicast	= u64_stats_read(&p->rx_multicast);
+			tx_packets	= u64_stats_read(&p->tx_packets);
+			tx_bytes	= u64_stats_read(&p->tx_bytes);
 		} while (u64_stats_fetch_retry_irq(&p->syncp, start));
 
 		stats->rx_packets	+= rx_packets;
@@ -1870,9 +1870,9 @@ team_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats)
 		 * rx_dropped, tx_dropped & rx_nohandler are u32,
 		 * updated without syncp protection.
 		 */
-		rx_dropped	+= p->rx_dropped;
-		tx_dropped	+= p->tx_dropped;
-		rx_nohandler	+= p->rx_nohandler;
+		rx_dropped	+= READ_ONCE(p->rx_dropped);
+		tx_dropped	+= READ_ONCE(p->tx_dropped);
+		rx_nohandler	+= READ_ONCE(p->rx_nohandler);
 	}
 	stats->rx_dropped	= rx_dropped;
 	stats->tx_dropped	= tx_dropped;
diff --git a/include/linux/if_team.h b/include/linux/if_team.h
index add607943c9564365e8d72d7522291d7a3d899d2..fc985e5c739d434148e8ff19d30ebc3ee8abf1d8 100644
--- a/include/linux/if_team.h
+++ b/include/linux/if_team.h
@@ -12,11 +12,11 @@
 #include <uapi/linux/if_team.h>
 
 struct team_pcpu_stats {
-	u64			rx_packets;
-	u64			rx_bytes;
-	u64			rx_multicast;
-	u64			tx_packets;
-	u64			tx_bytes;
+	u64_stats_t		rx_packets;
+	u64_stats_t		rx_bytes;
+	u64_stats_t		rx_multicast;
+	u64_stats_t		tx_packets;
+	u64_stats_t		tx_bytes;
 	struct u64_stats_sync	syncp;
 	u32			rx_dropped;
 	u32			tx_dropped;
-- 
2.36.1.255.ge46751e96f-goog


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next 5/9] wireguard: use dev_sw_netstats_rx_add()
  2022-06-07 23:36 ` [PATCH net-next 5/9] wireguard: " Eric Dumazet
@ 2022-06-08  7:36   ` Jason A. Donenfeld
  2022-06-08 15:41     ` Eric Dumazet
  0 siblings, 1 reply; 14+ messages in thread
From: Jason A. Donenfeld @ 2022-06-08  7:36 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, netdev, Eric Dumazet

Hi Eric,

On Tue, Jun 07, 2022 at 04:36:10PM -0700, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> We have a convenient helper, let's use it.
> This will make the following patch easier to review and smaller.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Jason A. Donenfeld <Jason@zx2c4.com>

The subject line should be:

    wireguard: receive: use dev_sw_netstats_rx_add()

Please don't commit it before changing that. With that addressed,

    Acked-by: Jason A. Donenfeld <Jason@zx2c4.com>

Regards,
Jason

>  drivers/net/wireguard/receive.c | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/drivers/net/wireguard/receive.c b/drivers/net/wireguard/receive.c
> index 7b8df406c7737398f0270361afcb196af4b6a76e..7135d51d2d872edb66c856379ce2923b214289e9 100644
> --- a/drivers/net/wireguard/receive.c
> +++ b/drivers/net/wireguard/receive.c
> @@ -19,15 +19,8 @@
>  /* Must be called with bh disabled. */
>  static void update_rx_stats(struct wg_peer *peer, size_t len)
>  {
> -	struct pcpu_sw_netstats *tstats =
> -		get_cpu_ptr(peer->device->dev->tstats);
> -
> -	u64_stats_update_begin(&tstats->syncp);
> -	++tstats->rx_packets;
> -	tstats->rx_bytes += len;
> +	dev_sw_netstats_rx_add(peer->device->dev, len);
>  	peer->rx_bytes += len;
> -	u64_stats_update_end(&tstats->syncp);
> -	put_cpu_ptr(tstats);
>  }
>  
>  #define SKB_TYPE_LE32(skb) (((struct message_header *)(skb)->data)->type)
> -- 
> 2.36.1.255.ge46751e96f-goog
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH net-next 1/9] vlan: adopt u64_stats_t
  2022-06-07 23:36 ` [PATCH net-next 1/9] vlan: adopt u64_stats_t Eric Dumazet
@ 2022-06-08 10:18   ` David Laight
  2022-06-08 10:37     ` Eric Dumazet
  0 siblings, 1 reply; 14+ messages in thread
From: David Laight @ 2022-06-08 10:18 UTC (permalink / raw)
  To: 'Eric Dumazet', David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: netdev, Jason A . Donenfeld, Eric Dumazet

From: Eric Dumazet
> Sent: 08 June 2022 00:36
> 
> As explained in commit 316580b69d0a ("u64_stats: provide u64_stats_t type")
> we should use u64_stats_t and related accessors to avoid load/store tearing.
> 
> Add READ_ONCE() when reading rx_errors & tx_dropped.

Isn't this all getting entirely stupid?

AFAICT nearly every 'memory' access in the kernel is going
to get wrapped in READ/WRITE_ONCE() to avoid something
that really never actually happens?

It might be better to just mark everything 'volatile'.
Although perhaps that ought to be a compiler option.

OTOH I've seen gcc generate extra instructions for 'volatile'
accesses - to the point where I used 'barrier()' to optimise
code.
I think the volatile casts in READ_ONCE() can generate worse
code than volatile variables.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next 1/9] vlan: adopt u64_stats_t
  2022-06-08 10:18   ` David Laight
@ 2022-06-08 10:37     ` Eric Dumazet
  0 siblings, 0 replies; 14+ messages in thread
From: Eric Dumazet @ 2022-06-08 10:37 UTC (permalink / raw)
  To: David Laight
  Cc: Eric Dumazet, David S . Miller, Jakub Kicinski, Paolo Abeni,
	netdev, Jason A . Donenfeld

On Wed, Jun 8, 2022 at 3:18 AM David Laight <David.Laight@aculab.com> wrote:
>
> From: Eric Dumazet
> > Sent: 08 June 2022 00:36
> >
> > As explained in commit 316580b69d0a ("u64_stats: provide u64_stats_t type")
> > we should use u64_stats_t and related accessors to avoid load/store tearing.
> >
> > Add READ_ONCE() when reading rx_errors & tx_dropped.
>
> Isn't this all getting entirely stupid?

Be careful about using these kinds of words, some people get offended
quite easily.

>
> AFAICT nearly every 'memory' access in the kernel is going
> to get wrapped in READ/WRITE_ONCE() to avoid something
> that really never actually happens?
>

When needed, yes. KCSAN can catch real bugs, thank you.

> It might be better to just mark everything 'volatile'.
> Although perhaps that ought to be a compiler option.
>
> OTOH I've seen gcc generate extra instructions for 'volatile'
> accesses - to the point where I used 'barrier()' to optimise
> code.
> I think the volatile casts in READ_ONCE() can generate worse
> code than volatile variables.

For these patches, code generation on x86 is actually exactly the same.

 Read https://lwn.net/Articles/793253

Then if you really think Jade Alglave, Will Deacon, Boqun Feng, David Howells,
 Daniel Lustig, Luc Maranget, Paul E. McKenney, Andrea Parri, Nicholas Piggin,
Alan Stern, Akira Yokosawa, and Peter Zijlstra were stupid, please
start a conversation
with them.

Thank you.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next 5/9] wireguard: use dev_sw_netstats_rx_add()
  2022-06-08  7:36   ` Jason A. Donenfeld
@ 2022-06-08 15:41     ` Eric Dumazet
  0 siblings, 0 replies; 14+ messages in thread
From: Eric Dumazet @ 2022-06-08 15:41 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: Eric Dumazet, David S . Miller, Jakub Kicinski, Paolo Abeni, netdev

On Wed, Jun 8, 2022 at 12:36 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> Hi Eric,
>
> On Tue, Jun 07, 2022 at 04:36:10PM -0700, Eric Dumazet wrote:
> > From: Eric Dumazet <edumazet@google.com>
> >
> > We have a convenient helper, let's use it.
> > This will make the following patch easier to review and smaller.
> >
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > Cc: Jason A. Donenfeld <Jason@zx2c4.com>
>
> The subject line should be:
>
>     wireguard: receive: use dev_sw_netstats_rx_add()
>
> Please don't commit it before changing that. With that addressed,
>
>     Acked-by: Jason A. Donenfeld <Jason@zx2c4.com>
>

Got it, thanks.


> Regards,
> Jason
>
> >  drivers/net/wireguard/receive.c | 9 +--------
> >  1 file changed, 1 insertion(+), 8 deletions(-)
> >
> > diff --git a/drivers/net/wireguard/receive.c b/drivers/net/wireguard/receive.c
> > index 7b8df406c7737398f0270361afcb196af4b6a76e..7135d51d2d872edb66c856379ce2923b214289e9 100644
> > --- a/drivers/net/wireguard/receive.c
> > +++ b/drivers/net/wireguard/receive.c
> > @@ -19,15 +19,8 @@
> >  /* Must be called with bh disabled. */
> >  static void update_rx_stats(struct wg_peer *peer, size_t len)
> >  {
> > -     struct pcpu_sw_netstats *tstats =
> > -             get_cpu_ptr(peer->device->dev->tstats);
> > -
> > -     u64_stats_update_begin(&tstats->syncp);
> > -     ++tstats->rx_packets;
> > -     tstats->rx_bytes += len;
> > +     dev_sw_netstats_rx_add(peer->device->dev, len);
> >       peer->rx_bytes += len;
> > -     u64_stats_update_end(&tstats->syncp);
> > -     put_cpu_ptr(tstats);
> >  }
> >
> >  #define SKB_TYPE_LE32(skb) (((struct message_header *)(skb)->data)->type)
> > --
> > 2.36.1.255.ge46751e96f-goog
> >

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-06-08 15:41 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-07 23:36 [PATCH net-next 0/9] net: Eric Dumazet
2022-06-07 23:36 ` [PATCH net-next 1/9] vlan: adopt u64_stats_t Eric Dumazet
2022-06-08 10:18   ` David Laight
2022-06-08 10:37     ` Eric Dumazet
2022-06-07 23:36 ` [PATCH net-next 2/9] ipvlan: " Eric Dumazet
2022-06-07 23:36 ` [PATCH net-next 3/9] sit: use dev_sw_netstats_rx_add() Eric Dumazet
2022-06-07 23:36 ` [PATCH net-next 4/9] ip6_tunnel: " Eric Dumazet
2022-06-07 23:36 ` [PATCH net-next 5/9] wireguard: " Eric Dumazet
2022-06-08  7:36   ` Jason A. Donenfeld
2022-06-08 15:41     ` Eric Dumazet
2022-06-07 23:36 ` [PATCH net-next 6/9] net: adopt u64_stats_t in struct pcpu_sw_netstats Eric Dumazet
2022-06-07 23:36 ` [PATCH net-next 7/9] devlink: adopt u64_stats_t Eric Dumazet
2022-06-07 23:36 ` [PATCH net-next 8/9] drop_monitor: " Eric Dumazet
2022-06-07 23:36 ` [PATCH net-next 9/9] team: " Eric Dumazet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).