[PATCH] netdev: stats on multiqueue possible bug

[PATCH] netdev: stats on multiqueue possible bug

[Stephen Hemminger]

Dave, I think you were trying to optimize something here that
doesn't need optimization.

If transmit stats (all) wrap to zero, then the stats would not
be set correctly. Move local variable into loop as well.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

--- a/net/core/dev.c	2009-09-19 15:19:13.902495560 -0700
+++ b/net/core/dev.c	2009-09-19 15:48:21.126458728 -0700
@@ -5079,19 +5079,19 @@ const struct net_device_stats *dev_get_s
 		unsigned long tx_bytes = 0, tx_packets = 0, tx_dropped = 0;
 		struct net_device_stats *stats = &dev->stats;
 		unsigned int i;
-		struct netdev_queue *txq;
 
 		for (i = 0; i < dev->num_tx_queues; i++) {
-			txq = netdev_get_tx_queue(dev, i);
+			const struct netdev_queue *txq
+				 = netdev_get_tx_queue(dev, i);
 			tx_bytes   += txq->tx_bytes;
 			tx_packets += txq->tx_packets;
 			tx_dropped += txq->tx_dropped;
 		}
-		if (tx_bytes || tx_packets || tx_dropped) {
-			stats->tx_bytes   = tx_bytes;
-			stats->tx_packets = tx_packets;
-			stats->tx_dropped = tx_dropped;
-		}
+
+		stats->tx_bytes   = tx_bytes;
+		stats->tx_packets = tx_packets;
+		stats->tx_dropped = tx_dropped;
+
 		return stats;
 	}
 }

Re: [PATCH] netdev: stats on multiqueue possible bug

[Eric Dumazet]

Stephen Hemminger a écrit :
> Dave, I think you were trying to optimize something here that
> doesn't need optimization.
> 
> If transmit stats (all) wrap to zero, then the stats would not
> be set correctly. Move local variable into loop as well.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> 
> --- a/net/core/dev.c	2009-09-19 15:19:13.902495560 -0700
> +++ b/net/core/dev.c	2009-09-19 15:48:21.126458728 -0700
> @@ -5079,19 +5079,19 @@ const struct net_device_stats *dev_get_s
>  		unsigned long tx_bytes = 0, tx_packets = 0, tx_dropped = 0;
>  		struct net_device_stats *stats = &dev->stats;
>  		unsigned int i;
> -		struct netdev_queue *txq;
>  
>  		for (i = 0; i < dev->num_tx_queues; i++) {
> -			txq = netdev_get_tx_queue(dev, i);
> +			const struct netdev_queue *txq
> +				 = netdev_get_tx_queue(dev, i);
>  			tx_bytes   += txq->tx_bytes;
>  			tx_packets += txq->tx_packets;
>  			tx_dropped += txq->tx_dropped;
>  		}
> -		if (tx_bytes || tx_packets || tx_dropped) {
> -			stats->tx_bytes   = tx_bytes;
> -			stats->tx_packets = tx_packets;
> -			stats->tx_dropped = tx_dropped;
> -		}
> +
> +		stats->tx_bytes   = tx_bytes;
> +		stats->tx_packets = tx_packets;
> +		stats->tx_dropped = tx_dropped;
> +
>  		return stats;
>  	}
>  }

Most devices dont update txq->tx_bytes/tx_packets/tx_dropped yet, but
still update their device->stats

Your patch makes these devices clearing their stats.

In the case all stats wrap to zero, we'll give old values. If you think this is 
a bug, you should find another way to fix it :)