net: add atomic_long_t to net_device_stats fields

Long standing KCSAN issues are caused by data-race around
some dev->stats changes.

Most performance critical paths already use per-cpu
variables, or per-queue ones.

It is reasonable (and more correct) to use atomic operations
for the slow paths.

This patch adds an union for each field of net_device_stats,
so that we can convert paths that are not yet protected
by a spinlock or a mutex.

netdev_stats_to_stats64() no longer has an #if BITS_PER_LONG==64

Note that the memcpy() we were using on 64bit arches
had no provision to avoid load-tearing,
while atomic_long_read() is providing the needed protection
at no cost.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 02a2318da7c7059ff2479479f4b9777a414fa591..23b3903b067840248dec7a75515125ff1ff577f5 100644 (file)
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -171,31 +171,38 @@ static inline bool dev_xmit_complete(int rc)
  *     (unsigned long) so they can be read and written atomically.
  */
 
+#define NET_DEV_STAT(FIELD)                    \
+       union {                                 \
+               unsigned long FIELD;            \
+               atomic_long_t __##FIELD;        \
+       }
+
 struct net_device_stats {
-       unsigned long   rx_packets;
-       unsigned long   tx_packets;
-       unsigned long   rx_bytes;
-       unsigned long   tx_bytes;
-       unsigned long   rx_errors;
-       unsigned long   tx_errors;
-       unsigned long   rx_dropped;
-       unsigned long   tx_dropped;
-       unsigned long   multicast;
-       unsigned long   collisions;
-       unsigned long   rx_length_errors;
-       unsigned long   rx_over_errors;
-       unsigned long   rx_crc_errors;
-       unsigned long   rx_frame_errors;
-       unsigned long   rx_fifo_errors;
-       unsigned long   rx_missed_errors;
-       unsigned long   tx_aborted_errors;
-       unsigned long   tx_carrier_errors;
-       unsigned long   tx_fifo_errors;
-       unsigned long   tx_heartbeat_errors;
-       unsigned long   tx_window_errors;
-       unsigned long   rx_compressed;
-       unsigned long   tx_compressed;
+       NET_DEV_STAT(rx_packets);
+       NET_DEV_STAT(tx_packets);
+       NET_DEV_STAT(rx_bytes);
+       NET_DEV_STAT(tx_bytes);
+       NET_DEV_STAT(rx_errors);
+       NET_DEV_STAT(tx_errors);
+       NET_DEV_STAT(rx_dropped);
+       NET_DEV_STAT(tx_dropped);
+       NET_DEV_STAT(multicast);
+       NET_DEV_STAT(collisions);
+       NET_DEV_STAT(rx_length_errors);
+       NET_DEV_STAT(rx_over_errors);
+       NET_DEV_STAT(rx_crc_errors);
+       NET_DEV_STAT(rx_frame_errors);
+       NET_DEV_STAT(rx_fifo_errors);
+       NET_DEV_STAT(rx_missed_errors);
+       NET_DEV_STAT(tx_aborted_errors);
+       NET_DEV_STAT(tx_carrier_errors);
+       NET_DEV_STAT(tx_fifo_errors);
+       NET_DEV_STAT(tx_heartbeat_errors);
+       NET_DEV_STAT(tx_window_errors);
+       NET_DEV_STAT(rx_compressed);
+       NET_DEV_STAT(tx_compressed);
 };
+#undef NET_DEV_STAT
 
 /* per-cpu stats, allocated on demand.
  * Try to fit them in a single cache line, for dev_get_stats() sake.
@@ -5171,4 +5178,9 @@ extern struct list_head ptype_base[PTYPE_HASH_SIZE] __read_mostly;
 
 extern struct net_device *blackhole_netdev;
 
+/* Note: Avoid these macros in fast path, prefer per-cpu or per-queue counters. */
+#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)
+
 #endif /* _LINUX_NETDEVICE_H */

diff --git a/include/net/dst.h b/include/net/dst.h
index 00b479ce6b99c9b329a30f8201882a51c9a135a9..d67fda89cd0fabb887c8f95222f531b7fb470a59 100644 (file)
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -356,9 +356,8 @@ static inline void __skb_tunnel_rx(struct sk_buff *skb, struct net_device *dev,
 static inline void skb_tunnel_rx(struct sk_buff *skb, struct net_device *dev,
                                 struct net *net)
 {
-       /* TODO : stats should be SMP safe */
-       dev->stats.rx_packets++;
-       dev->stats.rx_bytes += skb->len;
+       DEV_STATS_INC(dev, rx_packets);
+       DEV_STATS_ADD(dev, rx_bytes, skb->len);
        __skb_tunnel_rx(skb, dev, net);
 }
 

diff --git a/net/core/dev.c b/net/core/dev.c
index fb943dad96513e0f9eb1c3ce30c5bb7170edea5e..d0fb4af9a12611c7f82798a700205604730e4d10 100644 (file)
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10369,24 +10369,16 @@ void netdev_run_todo(void)
 void netdev_stats_to_stats64(struct rtnl_link_stats64 *stats64,
                             const struct net_device_stats *netdev_stats)
 {
-#if BITS_PER_LONG == 64
-       BUILD_BUG_ON(sizeof(*stats64) < sizeof(*netdev_stats));
-       memcpy(stats64, netdev_stats, sizeof(*netdev_stats));
-       /* zero out counters that only exist in rtnl_link_stats64 */
-       memset((char *)stats64 + sizeof(*netdev_stats), 0,
-              sizeof(*stats64) - sizeof(*netdev_stats));
-#else
-       size_t i, n = sizeof(*netdev_stats) / sizeof(unsigned long);
-       const unsigned long *src = (const unsigned long *)netdev_stats;
+       size_t i, n = sizeof(*netdev_stats) / sizeof(atomic_long_t);
+       const atomic_long_t *src = (atomic_long_t *)netdev_stats;
        u64 *dst = (u64 *)stats64;
 
        BUILD_BUG_ON(n > sizeof(*stats64) / sizeof(u64));
        for (i = 0; i < n; i++)
-               dst[i] = src[i];
+               dst[i] = atomic_long_read(&src[i]);
        /* zero out counters that only exist in rtnl_link_stats64 */
        memset((char *)stats64 + n * sizeof(u64), 0,
               sizeof(*stats64) - n * sizeof(u64));
-#endif
 }
 EXPORT_SYMBOL(netdev_stats_to_stats64);