Fw: iproute2 showing wrong number of bytes on 64bit architectures.

Fw: iproute2 showing wrong number of bytes on 64bit architectures.

[Stephen Hemminger]

Looks like net_device_stats should have always used u32?
Too late to change it now.

Begin forwarded message:

Date: Tue, 10 Jul 2007 13:58:51 +0200
From: Andreas Henriksson <andreas@fatal.se>
To: netdev@vger.kernel.org
Cc: 199054@bugs.debian.org
Subject: iproute2 showing wrong number of bytes on 64bit architectures.


Hello!

While investigating the problems reported in Debian bug #199054
(http://bugs.debian.org/199054), stating that the RX/TX bytes differ between
ifconfig and ip(route2) I came across this:

include/linux/if_link.h (used in kernel and iproute2 source):
/* The struct should be in sync with struct net_device_stats */
struct rtnl_link_stats
{
	__u32 ...;
	__u32 ...;
	__u32 ...;
	__u32 ...;
...


include/linux/netdevice.h:
struct net_device_stats
{
	unsigned long   ...;
	unsigned long   ...;
	unsigned long   ...;
	unsigned long   ...;
	unsigned long   ...;
....


This will be a problem when unsigned long isn't 32bits, but I guess the
rtnetlink message size is static for good reason. 
Can someone please advise on how to fix this?

-- 
Regards,
Andreas Henriksson
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
Stephen Hemminger <shemminger@linux-foundation.org>

Re: iproute2 showing wrong number of bytes on 64bit architectures.

[David Miller]

From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Tue, 10 Jul 2007 08:56:40 -0700

> Looks like net_device_stats should have always used u32?

It used unsigned long ages ago, and ifconfig gets the bits
exported from /proc/net/dev output whereas we have to used
fixed data types in whatever we use over netlink so u32
was choosen.

Re: iproute2 showing wrong number of bytes on 64bit architectures.

[Chris Friesen]

David Miller wrote:

> It used unsigned long ages ago, and ifconfig gets the bits
> exported from /proc/net/dev output whereas we have to used
> fixed data types in whatever we use over netlink so u32
> was choosen.

It's rather ironic that the "new-and-improved" way of doing things is 
subject to rollover while the "old" way is not.

Chris

Re: iproute2 showing wrong number of bytes on 64bit architectures.

[David Miller]

From: "Chris Friesen" <cfriesen@nortel.com>
Date: Tue, 10 Jul 2007 12:17:12 -0600

> It's rather ironic that the "new-and-improved" way of doing things is 
> subject to rollover while the "old" way is not.

Text is always more flexible, but is hard to extend.

We can trivially add "u64" statistics to netlink.

Re: iproute2 showing wrong number of bytes on 64bit architectures.

[Andreas Henriksson]

(Sorry for breaking the thread, please CC me on future replies...)

If I understand the discussion correctly, the problem in itself isn't greater
then that different methods of gathering the statistics have different
rollovers and thus confuse people who aren't aware of which method each tool
uses. The the main issue with fixing this is that it needs to be handled
gently to not break the userspace API/ABI.

I see three different paths...

a) just ignore the issue and let everybody who run into this stay confused
and cry about linux being crap unless they dig deeper and gets informed of
the different interfaces and their rollovers.

b) be lazy and fix only the confusion by making sure that the statistics in
/proc/net/dev also rolls over at 32bits so that all exported statistics show
the same number (while ignoring possible problems with 32bit rollovers, like
fast networks might roll over too quickly and make the stats useless).

c) take the more painful route of switching over to 64bit statistics in
netlink. Add 64bit interface, port userspace tools, deprecate the old 32bit
interface. (I guess exporting as u64 even on 32bit architectures wouldn't
hurt them, even if they still will rollover at (unsigned long) 32 bits.)


I'd prefer not to do (a). Would (b) be acceptable? If not, could someone who
is more familiar with how to do netlink please help me out with a patch for
adding the required kernel part of (c)?
(I'm going away for 2 weeks on friday, so I'll pick up whatever needs doing
when I'm back...)


Here's a completely untested patch for (b):


--- linux-2.6.22/net/core/dev.c	2007-07-09 01:32:17.000000000 +0200
+++ linux-2.6.22/net/core/dev.c.new	2007-07-11 13:56:21.000000000 +0200
@@ -2184,23 +2184,24 @@
 {
 	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",
-		   dev->name, stats->rx_bytes, stats->rx_packets,
-		   stats->rx_errors,
-		   stats->rx_dropped + stats->rx_missed_errors,
-		   stats->rx_fifo_errors,
-		   stats->rx_length_errors + stats->rx_over_errors +
-		    stats->rx_crc_errors + stats->rx_frame_errors,
-		   stats->rx_compressed, stats->multicast,
-		   stats->tx_bytes, stats->tx_packets,
-		   stats->tx_errors, stats->tx_dropped,
-		   stats->tx_fifo_errors, stats->collisions,
-		   stats->tx_carrier_errors +
-		    stats->tx_aborted_errors +
+	/* Always rollover stats at 32bits to match netlink interface */
+	seq_printf(seq, "%6s:%8u %7u %4u %4u %4u %5u %10u %9u "
+		   "%8u %7u %4u %4u %4u %5u %7u %10u\n",
+		   dev->name, (u32)stats->rx_bytes, (u32)stats->rx_packets,
+		   (u32)stats->rx_errors,
+		   (u32)(stats->rx_dropped + stats->rx_missed_errors),
+		   (u32)stats->rx_fifo_errors,
+		   (u32)(stats->rx_length_errors + stats->rx_over_errors +
+		    stats->rx_crc_errors + stats->rx_frame_errors),
+		   (u32)stats->rx_compressed, (u32)stats->multicast,
+		   (u32)stats->tx_bytes, (u32)stats->tx_packets,
+		   (u32)stats->tx_errors, (u32)stats->tx_dropped,
+		   (u32)stats->tx_fifo_errors, (u32)stats->collisions,
+		   (u32)(stats->tx_carrier_errors +
+		    ustats->tx_aborted_errors +
 		    stats->tx_window_errors +
-		    stats->tx_heartbeat_errors,
-		   stats->tx_compressed);
+		    stats->tx_heartbeat_errors),
+		   (u32)stats->tx_compressed);
 }
 
 /*


-- 
Regards,
Andreas Henriksson

Re: iproute2 showing wrong number of bytes on 64bit architectures.

[Norman Rasmussen]

re: http://marc.info/?l=linux-netdev&m=118415534518953
(please cc me on replies)

On 2007-07-11 Andreas Henriksson wrote:
> c) take the more painful route of switching over to 64bit statistics in
> netlink. Add 64bit interface, port userspace tools, deprecate the old 32bit
> interface. (I guess exporting as u64 even on 32bit architectures wouldn't
> hurt them, even if they still will rollover at (unsigned long) 32 bits.)

I youthfully opened a ticket here:
http://bugzilla.kernel.org/show_bug.cgi?id=12425 to track the
implementation for this feature request.

Is there someone more qualified than me to add support for 64 bit
device counters, or is this something I should try and hack together
in a UML and submit a patch?

--
- Norman Rasmussen
- Email: norman@rasmussen.co.za
- Home page: http://norman.rasmussen.co.za/