overflows in /proc/net/dev

overflows in /proc/net/dev

[Sebastian Claßen]

[-- Attachment #1: Type: text/plain, Size: 851 bytes --]

Hi all...

in struct net_device_stats all members are defined as unsgined long. In
time of gigabit ethernet this takes not long to overflow. Are there any
plans to change these coutners to unsigned long long? I saw in ifconfig
source code the byte and packet counters are already defined as unsigned
long long.

I've tried the attached little patch which seems to work well. Are there
any good reasons against this changes?

-- 
Mit freundlichen Grüßen / Yours sincerely

Sebastian Claßen, Postmaster
freenet.de AG, Willstätterstraße 13, D-40549 Düsseldorf
Phone: +49 (0) 211 / 53087-522
----------------------------------------------------------------------
Vorsitzender des Aufsichtsrates: Prof. Dr. Helmut Thoma
Vorstand: Eckhard Spoerr (Vors.), Axel Krieger,
          Stephan Esch, Eric Berger
Amtsgericht Hamburg HRB 74048

[-- Attachment #2: proc_net_dev.diff --]
[-- Type: text/x-patch, Size: 1496 bytes --]

--- include/linux/netdevice.h.orig	2005-08-17 15:28:05.000000000 +0200
+++ include/linux/netdevice.h	2005-08-17 15:27:02.000000000 +0200
@@ -107,10 +107,10 @@ struct netpoll;
  
 struct net_device_stats
 {
-	unsigned long rx_packets;		/* total packets received	*/
-	unsigned long tx_packets;		/* total packets transmitted	*/
-	unsigned long rx_bytes;		/* total bytes received 	*/
-	unsigned long tx_bytes;		/* total bytes transmitted	*/
+	unsigned long long rx_packets;		/* total packets received	*/
+	unsigned long long tx_packets;		/* total packets transmitted	*/
+	unsigned long long rx_bytes;		/* total bytes received 	*/
+	unsigned long long tx_bytes;		/* total bytes transmitted	*/
 	unsigned long rx_errors;		/* bad packets received		*/
 	unsigned long tx_errors;		/* packet transmit problems	*/
 	unsigned long rx_dropped;		/* no space in linux buffers	*/
--- net/core/dev.c.orig	2005-08-17 15:29:40.000000000 +0200
+++ net/core/dev.c	2005-08-17 15:30:53.000000000 +0200
@@ -1984,8 +1984,8 @@ static void dev_seq_printf_stats(struct 
 	if (dev->get_stats) {
 		struct net_device_stats *stats = dev->get_stats(dev);
 
-		seq_printf(seq, "%6s:%8lu %7lu %4lu %4lu %4lu %5lu %10lu %9lu "
-				"%8lu %7lu %4lu %4lu %4lu %5lu %7lu %10lu\n",
+		seq_printf(seq, "%6s:%8llu %7llu %4lu %4lu %4lu %5lu %10lu %9lu "
+				"%8llu %7llu %4lu %4lu %4lu %5lu %7lu %10lu\n",
 			   dev->name, stats->rx_bytes, stats->rx_packets,
 			   stats->rx_errors,
 			   stats->rx_dropped + stats->rx_missed_errors,

Re: overflows in /proc/net/dev

[Chris Wedgwood]

On Thu, Aug 18, 2005 at 09:28:10AM +0200, Sebastian Cla?en wrote:

> in struct net_device_stats all members are defined as unsgined
> long. In time of gigabit ethernet this takes not long to overflow.

It should still take an appreciable amount of time surely?  We can
detect those wraps in userspace and deal with it as needed.

> Are there any plans to change these coutners to unsigned long long?

It comes up from time to time (see below).

> I saw in ifconfig source code the byte and packet counters are
> already defined as unsigned long long.

ifconfig is userspace.


[...]

>  struct net_device_stats
>  {
> -	unsigned long rx_packets;		/* total packets received	*/
> -	unsigned long tx_packets;		/* total packets transmitted	*/
> -	unsigned long rx_bytes;		/* total bytes received 	*/
> -	unsigned long tx_bytes;		/* total bytes transmitted	*/
> +	unsigned long long rx_packets;		/* total packets received	*/
> +	unsigned long long tx_packets;		/* total packets transmitted	*/
> +	unsigned long long rx_bytes;		/* total bytes received 	*/
> +	unsigned long long tx_bytes;		/* total bytes transmitted	*/

I thought the concensurs here was that because doing reliable atomic
updates of 64-bit values isn't possible on some (most?) 32-bit
architectures so we need additional locking to make this work which is
undesirable?  (It might even be a FAQ by now as this comes up fairly
often).

Re: overflows in /proc/net/dev

[Mario 'BitKoenig' Holbe]

Chris Wedgwood <cw@f00f.org> wrote:
> On Thu, Aug 18, 2005 at 09:28:10AM +0200, Sebastian Cla?en wrote:
>> in struct net_device_stats all members are defined as unsgined
>> long. In time of gigabit ethernet this takes not long to overflow.
> It should still take an appreciable amount of time surely?  We can

With 100MBit it took about 5 minutes to overflow the 32bit byte counters
(too less for typical mrtg and Nyquist-Shannon sampling theorem in
mind), with 1GBit it takes about 30 seconds.
If you consider this appreciable... well :)

> detect those wraps in userspace and deal with it as needed.

Of course, it's usually enough time for a typical userspace application
to get scheduled at least twice.


regards
   Mario
-- 
User sind wie ideale Gase - sie verteilen sich gleichmaessig ueber alle Platten

Re: overflows in /proc/net/dev

[David S. Miller]

From: Chris Wedgwood <cw@f00f.org>
Date: Thu, 18 Aug 2005 09:33:58 -0700

> I thought the concensurs here was that because doing reliable atomic
> updates of 64-bit values isn't possible on some (most?) 32-bit
> architectures so we need additional locking to make this work which is
> undesirable?  (It might even be a FAQ by now as this comes up fairly
> often).

That's correct.

Re: overflows in /proc/net/dev

[Stephen Hemminger]

On Thu, 18 Aug 2005 14:32:48 -0700 (PDT)
"David S. Miller" <davem@davemloft.net> wrote:

> From: Chris Wedgwood <cw@f00f.org>
> Date: Thu, 18 Aug 2005 09:33:58 -0700
> 
> > I thought the concensurs here was that because doing reliable atomic
> > updates of 64-bit values isn't possible on some (most?) 32-bit
> > architectures so we need additional locking to make this work which is
> > undesirable?  (It might even be a FAQ by now as this comes up fairly
> > often).
> 
> That's correct.

Also width of fields in /proc/net/dev can't change without potentially
breaking ABI of applications.