[PATCH - RFC] [1/5] 64-bit network statistics - generic net

[PATCH - RFC] [1/5] 64-bit network statistics - generic net

[Jeff Sipek]

[-- Attachment #1: clearsigned data --]
[-- Type: Text/Plain, Size: 1704 bytes --]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

The variables for network statistics (in struct net_device_stats) are unsigned 
longs. On 32-bit architectures, this makes them overflow every 4GB or 2^32 
packets. The following series of patches [against 2.5.74] makes the 
statistics variable type configurable. The default is to leave everything the 
way it was (unsigned long). However, when NETSTATS64 is set in the config, 
the statistics use 64-bit variables (u_int64_t) - this works only on 32-bit 
architectures.

These patches are *not* ready to be included due to the fact that they add an 
API layer that controls the statistics and all the drivers have to be 
checked. For easier coding, the patch renames the fields in struct 
net_device_stat - this breaks everything (so all the previous code is easily 
visible.)  (The names are prefixed with _.)

This is a RFC, please let me know what you think should be improved.

*NOTE* I am aware that locking is suboptimal - I am looking for other 
solutions.

The patches include:
	- Documentation/networking/64bitstats.txt with description of the API, and 
current status of this project (I'm not sure about some of the arch dependent 
suff)
	- updated drivers to use new API
		- 8139too
		- 8390
		- dummy
		- eepro100
		- loopback
		- ne2k-pci
		- pcnet32
	- update for procfs and sysfs interfaces

Thanks,
Jeff.

- -- 
You measure democracy by the freedom it gives its dissidents, not the
freedom it gives its assimilated conformists.
		- Abbie Hoffman




-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.2 (GNU/Linux)

iD8DBQE/BOcHwFP0+seVj/4RAs8dAJ9UqAyKB3ADE11o6Wo5kP2MYMUrVACeMWKw
OiQPdQD85s9u+G4F/t0GraA=
=aZf/
-----END PGP SIGNATURE-----

[-- Attachment #2: net.diff --]
[-- Type: text/x-diff, Size: 12333 bytes --]

diff -X dontdiff -Naur linux-2.5.74-vanilla/net/Kconfig linux-2.5.74-nx/net/Kconfig
--- linux-2.5.74-vanilla/net/Kconfig	2003-07-02 16:40:31.000000000 -0400
+++ linux-2.5.74-nx/net/Kconfig	2003-07-03 15:11:00.000000000 -0400
@@ -48,6 +48,20 @@
 	  mechanism that results in faster communication.
 
 	  If unsure, say N.
+  
+config NETSTATS64
+	bool "64-bit statistics"
+	depends on NET && (X86 || ARM || PARISC || SPARC32) && !(X86_64)
+	help
+	  If you say Y here, the network statistics will use 64-bit values
+	  no matter what architecture you are on. If you are on a 64-bit
+	  architecture, say N here, the overhead required by this code is
+	  not justifiable. If, however you are on a 32-bit architecture,
+	  you have a choice. You can say:
+	  	Y and statistics will be 64-bit
+	  	N and statistics will remain 32-bit
+	  	
+	  If unsure, say N.
 
 config NETLINK_DEV
 	tristate "Netlink device emulation"
diff -X dontdiff -Naur linux-2.5.74-vanilla/net/core/dev.c linux-2.5.74-nx/net/core/dev.c
--- linux-2.5.74-vanilla/net/core/dev.c	2003-07-02 16:48:08.000000000 -0400
+++ linux-2.5.74-nx/net/core/dev.c	2003-07-03 15:11:00.000000000 -0400
@@ -18,6 +18,7 @@
  *		Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
  *		Adam Sulmicki <adam@cfar.umd.edu>
  *              Pekka Riikonen <priikone@poesidon.pspt.fi>
+ *		Josef "Jeff" Sipek <jeffpc@optonline.net>
  *
  *	Changes:
  *              D.J. Barrow     :       Fixed bug where dev->refcnt gets set
@@ -70,6 +71,7 @@
  *              			indefinitely on dev->refcnt
  * 		J Hadi Salim	:	- Backlog queue sampling
  *				        - netif_rx() feedback
+ *	Josef "Jeff" Sipek	:	- using new API for network statistics
  */
 
 #include <asm/uaccess.h>
@@ -1869,23 +1871,23 @@
 	struct net_device_stats *stats = dev->get_stats ? dev->get_stats(dev) :
 							  NULL;
 	if (stats)
-		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 +
-			     stats->tx_window_errors +
-			     stats->tx_heartbeat_errors,
-			   stats->tx_compressed);
+		seq_printf(seq, "%6s:%8llu %7llu %4llu %4llu %4llu %5llu %10llu %9llu "
+				"%8llu %7llu %4llu %4llu %4llu %5llu %7llu %10llu\n",
+			   dev->name, net_stats_get(rx_bytes,stats), net_stats_get(rx_packets,stats),
+			   net_stats_get(rx_errors,stats),
+			   net_stats_get(rx_dropped,stats) + net_stats_get(rx_missed_errors,stats),
+			   net_stats_get(rx_fifo_errors,stats),
+			   net_stats_get(rx_length_errors,stats) + net_stats_get(rx_over_errors,stats) +
+			     net_stats_get(rx_crc_errors,stats) + net_stats_get(rx_frame_errors,stats),
+			   net_stats_get(rx_compressed,stats), net_stats_get(multicast,stats),
+			   net_stats_get(tx_bytes,stats), net_stats_get(tx_packets,stats),
+			   net_stats_get(tx_errors,stats), net_stats_get(tx_dropped,stats),
+			   net_stats_get(tx_fifo_errors,stats), net_stats_get(collisions,stats),
+			   net_stats_get(tx_carrier_errors,stats) +
+			     net_stats_get(tx_aborted_errors,stats) +
+			     net_stats_get(tx_window_errors,stats) +
+			     net_stats_get(tx_heartbeat_errors,stats),
+			   net_stats_get(tx_compressed,stats));
 	else
 		seq_printf(seq, "%6s: No statistics available.\n", dev->name);
 }
diff -X dontdiff -Naur linux-2.5.74-vanilla/net/core/net-sysfs.c linux-2.5.74-nx/net/core/net-sysfs.c
--- linux-2.5.74-vanilla/net/core/net-sysfs.c	2003-07-02 16:55:46.000000000 -0400
+++ linux-2.5.74-nx/net/core/net-sysfs.c	2003-07-03 15:11:00.000000000 -0400
@@ -2,6 +2,9 @@
  * net-sysfs.c - network device class and attributes
  *
  * Copyright (c) 2003 Stephen Hemminber <shemminger@osdl.org>
+ *
+ * Copyright (c) 2003 Josef "Jeff" Sipek <jeffpc@optonline.net>
+ *	- Now using the "New" Networks statistics API
  * 
  */
 
@@ -180,21 +183,21 @@
 
 struct netstat_fs_entry {
 	struct attribute attr;
-	ssize_t (*show)(const struct net_device_stats *, char *);
+	ssize_t (*show)(struct net_device_stats *, char *);
 	ssize_t (*store)(struct net_device_stats *, const char *, size_t);
 };
 
-static ssize_t net_device_stat_show(unsigned long var, char *buf)
+static ssize_t net_device_stat_show(u_int64_t var, char *buf)
 {
-	return sprintf(buf, "%ld\n", var);
+	return sprintf(buf, "%llu\n", var);
 }
 
 /* generate a read-only statistics attribute */
 #define NETDEVICE_STAT(_NAME)						\
-static ssize_t show_stat_##_NAME(const struct net_device_stats *stats,	\
+static ssize_t show_stat_##_NAME(struct net_device_stats *stats,	\
 				 char *buf)				\
 {									\
-	return net_device_stat_show(stats->_NAME, buf);			\
+	return net_device_stat_show(net_stats_get(_NAME,stats), buf);	\
 }									\
 static struct netstat_fs_entry net_stat_##_NAME  = {		   	\
 	.attr = {.name = __stringify(_NAME), .mode = S_IRUGO },		\
diff -X dontdiff -Naur linux-2.5.74-vanilla/include/linux/if_vlan.h linux-2.5.74-nx/include/linux/if_vlan.h
--- linux-2.5.74-vanilla/include/linux/if_vlan.h	2003-07-02 16:42:17.000000000 -0400
+++ linux-2.5.74-nx/include/linux/if_vlan.h	2003-07-03 15:11:00.000000000 -0400
@@ -2,6 +2,7 @@
  * VLAN		An implementation of 802.1Q VLAN tagging.
  *
  * Authors:	Ben Greear <greearb@candelatech.com>
+ *		Josef "Jeff" Sipek <jeffpc@optonline.net>
  *
  *		This program is free software; you can redistribute it and/or
  *		modify it under the terms of the GNU General Public License
@@ -162,8 +163,8 @@
 	skb->dev->last_rx = jiffies;
 
 	stats = vlan_dev_get_stats(skb->dev);
-	stats->rx_packets++;
-	stats->rx_bytes += skb->len;
+	net_stats_inc(rx_packets,stats);
+	net_stats_add(rx_bytes,stats,skb->len);
 
 	skb->priority = vlan_get_ingress_priority(skb->dev, vlan_tag);
 	switch (skb->pkt_type) {
@@ -171,7 +172,7 @@
 		break;
 
 	case PACKET_MULTICAST:
-		stats->multicast++;
+		net_stats_inc(multicast,stats);
 		break;
 
 	case PACKET_OTHERHOST:
diff -X dontdiff -Naur linux-2.5.74-vanilla/include/linux/netdevice.h linux-2.5.74-nx/include/linux/netdevice.h
--- linux-2.5.74-vanilla/include/linux/netdevice.h	2003-07-02 16:56:07.000000000 -0400
+++ linux-2.5.74-nx/include/linux/netdevice.h	2003-07-03 15:11:00.000000000 -0400
@@ -14,6 +14,7 @@
  *		Alan Cox, <Alan.Cox@linux.org>
  *		Bjorn Ekwall. <bj0rn@blox.se>
  *              Pekka Riikonen <priikone@poseidon.pspt.fi>
+ *		Josef "Jeff" Sipek <jeffpc@optonline.net>
  *
  *		This program is free software; you can redistribute it and/or
  *		modify it under the terms of the GNU General Public License
@@ -95,39 +96,75 @@
  *	with byte counters.
  */
  
+#ifdef CONFIG_NETSTATS64
+#define NETSTAT_TYPE u_int64_t
+struct net_device_stats_locks
+{
+	spinlock_t	rx_packets;
+	spinlock_t	tx_packets;
+	spinlock_t	rx_bytes;
+	spinlock_t	tx_bytes;
+	spinlock_t	rx_errors;
+	spinlock_t	tx_errors;
+	spinlock_t	rx_dropped;
+	spinlock_t	tx_dropped;
+	spinlock_t	multicast;
+	spinlock_t	collisions;
+	spinlock_t	rx_length_errors;
+	spinlock_t	rx_over_errors;
+	spinlock_t	rx_crc_errors;
+	spinlock_t	rx_frame_errors;
+	spinlock_t	rx_fifo_errors;
+	spinlock_t	rx_missed_errors;
+	spinlock_t	tx_aborted_errors;
+	spinlock_t	tx_carrier_errors;
+	spinlock_t	tx_fifo_errors;
+	spinlock_t	tx_heartbeat_errors;
+	spinlock_t	tx_window_errors;
+	spinlock_t	rx_compressed;
+	spinlock_t	tx_compressed;
+};
+#else
+#define NETSTAT_TYPE unsigned long
+#endif
+
 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	rx_errors;		/* bad packets received		*/
-	unsigned long	tx_errors;		/* packet transmit problems	*/
-	unsigned long	rx_dropped;		/* no space in linux buffers	*/
-	unsigned long	tx_dropped;		/* no space available in linux	*/
-	unsigned long	multicast;		/* multicast packets received	*/
-	unsigned long	collisions;
+	NETSTAT_TYPE	_rx_packets;		/* total packets received	*/
+	NETSTAT_TYPE	_tx_packets;		/* total packets transmitted	*/
+	NETSTAT_TYPE	_rx_bytes;		/* total bytes received 	*/
+	NETSTAT_TYPE	_tx_bytes;		/* total bytes transmitted	*/
+	NETSTAT_TYPE	_rx_errors;		/* bad packets received		*/
+	NETSTAT_TYPE	_tx_errors;		/* packet transmit problems	*/
+	NETSTAT_TYPE	_rx_dropped;		/* no space in linux buffers	*/
+	NETSTAT_TYPE	_tx_dropped;		/* no space available in linux	*/
+	NETSTAT_TYPE	_multicast;		/* multicast packets received	*/
+	NETSTAT_TYPE	_collisions;
 
 	/* detailed rx_errors: */
-	unsigned long	rx_length_errors;
-	unsigned long	rx_over_errors;		/* receiver ring buff overflow	*/
-	unsigned long	rx_crc_errors;		/* recved pkt with crc error	*/
-	unsigned long	rx_frame_errors;	/* recv'd frame alignment error */
-	unsigned long	rx_fifo_errors;		/* recv'r fifo overrun		*/
-	unsigned long	rx_missed_errors;	/* receiver missed packet	*/
+	NETSTAT_TYPE	_rx_length_errors;
+	NETSTAT_TYPE	_rx_over_errors;	/* receiver ring buff overflow	*/
+	NETSTAT_TYPE	_rx_crc_errors;		/* recved pkt with crc error	*/
+	NETSTAT_TYPE	_rx_frame_errors;	/* recv'd frame alignment error */
+	NETSTAT_TYPE	_rx_fifo_errors;	/* recv'r fifo overrun		*/
+	NETSTAT_TYPE	_rx_missed_errors;	/* receiver missed packet	*/
 
 	/* detailed tx_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;
-	
+	NETSTAT_TYPE	_tx_aborted_errors;
+	NETSTAT_TYPE	_tx_carrier_errors;
+	NETSTAT_TYPE	_tx_fifo_errors;
+	NETSTAT_TYPE	_tx_heartbeat_errors;
+	NETSTAT_TYPE	_tx_window_errors;
+
 	/* for cslip etc */
-	unsigned long	rx_compressed;
-	unsigned long	tx_compressed;
-};
+	NETSTAT_TYPE	_rx_compressed;
+	NETSTAT_TYPE	_tx_compressed;
 
+#ifdef CONFIG_NETSTATS64
+	/* locks */
+	struct net_device_stats_locks locks;
+#endif
+};
 
 /* Media selection options. */
 enum {
@@ -865,6 +902,46 @@
 extern void		dev_clear_fastroute(struct net_device *dev);
 #endif
 
+static inline void net_stats_init(struct net_device_stats* stats)
+{
+#ifdef CONFIG_NETSTATS64
+	spin_lock_init(&stats->locks.rx_packets);
+	spin_lock_init(&stats->locks.tx_packets);
+	spin_lock_init(&stats->locks.rx_bytes);
+	spin_lock_init(&stats->locks.tx_bytes);
+	spin_lock_init(&stats->locks.rx_errors);
+	spin_lock_init(&stats->locks.tx_errors);
+	spin_lock_init(&stats->locks.rx_dropped);
+	spin_lock_init(&stats->locks.tx_dropped);
+	spin_lock_init(&stats->locks.multicast);
+	spin_lock_init(&stats->locks.collisions);
+	spin_lock_init(&stats->locks.rx_length_errors);
+	spin_lock_init(&stats->locks.rx_over_errors);
+	spin_lock_init(&stats->locks.rx_crc_errors);
+	spin_lock_init(&stats->locks.rx_frame_errors);
+	spin_lock_init(&stats->locks.rx_fifo_errors);
+	spin_lock_init(&stats->locks.rx_missed_errors);
+	spin_lock_init(&stats->locks.tx_aborted_errors);
+	spin_lock_init(&stats->locks.tx_carrier_errors);
+	spin_lock_init(&stats->locks.tx_fifo_errors);
+	spin_lock_init(&stats->locks.tx_heartbeat_errors);
+	spin_lock_init(&stats->locks.tx_window_errors);
+	spin_lock_init(&stats->locks.rx_compressed);
+	spin_lock_init(&stats->locks.tx_compressed);
+#endif
+}
+
+#ifdef CONFIG_NETSTATS64
+#define net_stats_add(what,stats,num)		locked_add64(&(stats)->_##what,(u_int64_t) num,&(stats)->locks.what)
+#define net_stats_inc(what,stats)		locked_add64(&(stats)->_##what,1,&(stats)->locks.what)
+#define net_stats_set(what,stats,num)		locked_set64(&(stats)->_##what,(u_int64_t) num,&(stats)->locks.what)
+#define net_stats_get(what,stats)		locked_get64(&(stats)->_##what,&(stats)->locks.what)
+#else
+#define net_stats_add(what,stats,num)		(stats)->_##what += (NETSTAT_TYPE) num
+#define net_stats_inc(what,stats)		((stats)->_##what++)
+#define net_stats_set(what,stats,num)		(stats)->_##what = (NETSTAT_TYPE) num
+#define net_stats_get(what,stats)		((u_int64_t) (stats)->_##what)
+#endif
 
 #endif /* __KERNEL__ */
 

Re: [PATCH - RFC] [1/5] 64-bit network statistics - generic net

[Jeff Garzik]

Jeff Sipek wrote:
> +	spinlock_t	rx_packets;
> +	spinlock_t	tx_packets;
> +	spinlock_t	rx_bytes;
> +	spinlock_t	tx_bytes;
> +	spinlock_t	rx_errors;
> +	spinlock_t	tx_errors;
> +	spinlock_t	rx_dropped;
> +	spinlock_t	tx_dropped;
> +	spinlock_t	multicast;
> +	spinlock_t	collisions;
> +	spinlock_t	rx_length_errors;
> +	spinlock_t	rx_over_errors;
> +	spinlock_t	rx_crc_errors;
> +	spinlock_t	rx_frame_errors;
> +	spinlock_t	rx_fifo_errors;
> +	spinlock_t	rx_missed_errors;
> +	spinlock_t	tx_aborted_errors;
> +	spinlock_t	tx_carrier_errors;
> +	spinlock_t	tx_fifo_errors;
> +	spinlock_t	tx_heartbeat_errors;
> +	spinlock_t	tx_window_errors;
> +	spinlock_t	rx_compressed;
> +	spinlock_t	tx_compressed;

That's a fat daddy list of locks you got there.


> +	NETSTAT_TYPE	_rx_packets;		/* total packets received	*/
> +	NETSTAT_TYPE	_tx_packets;		/* total packets transmitted	*/
> +	NETSTAT_TYPE	_rx_bytes;		/* total bytes received 	*/
> +	NETSTAT_TYPE	_tx_bytes;		/* total bytes transmitted	*/
> +	NETSTAT_TYPE	_rx_errors;		/* bad packets received		*/
> +	NETSTAT_TYPE	_tx_errors;		/* packet transmit problems	*/
> +	NETSTAT_TYPE	_rx_dropped;		/* no space in linux buffers	*/
> +	NETSTAT_TYPE	_tx_dropped;		/* no space available in linux	*/
> +	NETSTAT_TYPE	_multicast;		/* multicast packets received	*/
> +	NETSTAT_TYPE	_collisions;

Increasing user-visible sizes arbitrarily breaks stuff.  Having 
config-dependent types like this increases complexity.

Short term, just sample the stats more rapidly.

Long term, I suppose with 10GbE we should start thinking about this. 
Personally, I would prefer to make the standard net device stats 
available in the format already exported by ETHTOOL_GSTATS -- which I 
note uses u64's for its counters, and it's easily extensible.  I 
received a request for this just today, even.

	Jeff


P.S.  Please cc netdev@oss.sgi.com for networking discussions.

Re: [PATCH - RFC] [1/5] 64-bit network statistics - generic net

[Linus Torvalds]

On Thu, 3 Jul 2003, Jeff Sipek wrote:
> 
> The variables for network statistics (in struct net_device_stats) are unsigned 
> longs. On 32-bit architectures, this makes them overflow every 4GB or 2^32 
> packets. The following series of patches [against 2.5.74] makes the 
> statistics variable type configurable. The default is to leave everything the 
> way it was (unsigned long). However, when NETSTATS64 is set in the config, 
> the statistics use 64-bit variables (u_int64_t) - this works only on 32-bit 
> architectures.

Please do this in user space. The "overflow every 2^32 packets" thing is 
_not_ a problem, if you just gather the statistics at any kind of 
reasonable interval.

I'd hate to penalise performance for something like this. We have
generally avoided locking _entirely_ for statistics, exactly because
people felt that there are major performance issues wrt network packet
handling, and that "perfect statistics" aren't important enough to
penalize performance over.

Remember: "perfect is the enemy of good". 

		Linus

Re: [PATCH - RFC] [1/5] 64-bit network statistics - generic net

[Jeff Sipek]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Thursday 03 July 2003 23:08, Linus Torvalds wrote:
> Please do this in user space. The "overflow every 2^32 packets" thing is
> _not_ a problem, if you just gather the statistics at any kind of
> reasonable interval.

The packet counters are fine (for now, that is), but the tx_bytes and rx_bytes 
counters need those 64-bits. 4GB (= 2^32 bytes) is not enough. For example:

- - gigabit ethernet will cause 32-bit counters to overflow about every 34 
seconds (at full speed.)
- - 10Gb/s ethernet will only take about 3.4 seconds
- - a user like me, who has 5Mbit/s connection to the net can cause the counter 
to overflow in 1 hour 54 minutes

(Most of the time, the devices are not maxed out, but we have to check the 
worst case scenario.) Now, how often should the user space 
statistics-gathering program should run? Well, at least every 30 seconds, for 
now that should be good, but the rein of 10Gb/s is approaching...

> I'd hate to penalise performance for something like this. We have
> generally avoided locking _entirely_ for statistics, exactly because
> people felt that there are major performance issues wrt network packet
> handling, and that "perfect statistics" aren't important enough to
> penalize performance over.

I agree with you, that is why I made it optional so the user may choose to 
sacrifice performace for statistics when needed.

Additionally, I am sure there is a way of optimizing the patch I wrote (i.e. 
actual transmition is locked with a lock from struct net_device.) I am aware 
that this patch is a major undertaking, but it is only a matter of time 
before someone will have to do it anyway.

> Remember: "perfect is the enemy of good".

Very true.


Jeff.

- -- 
Only two things are infinite, the universe and human stupidity, and I'm 
not sure about the former.
		- Albert Einstein
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.2 (GNU/Linux)

iD8DBQE/BRBjwFP0+seVj/4RAnDSAJ90uOIpgtk0O7YLSsdj97kNbhr/jgCgrmlS
GYbA4luLnY7bli1jYVuZD3s=
=7zXz
-----END PGP SIGNATURE-----

Re: [PATCH - RFC] [1/5] 64-bit network statistics - generic net

[Jeff Sipek]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Thursday 03 July 2003 22:46, Jeff Garzik wrote:
> Jeff Sipek wrote:
> > +	spinlock_t	rx_packets;
<snip>
> > +	spinlock_t	tx_compressed;
>
> That's a fat daddy list of locks you got there.

Yeah, I know, I am sure there is a way of getting rid of some of those.. (i.e.
the tx functions are inside a spinlock from struct net_device.)

> > +	NETSTAT_TYPE	_rx_packets;		/* total packets received	*/
> > +	NETSTAT_TYPE	_tx_packets;		/* total packets transmitted	*/
> > +	NETSTAT_TYPE	_rx_bytes;		/* total bytes received 	*/
> > +	NETSTAT_TYPE	_tx_bytes;		/* total bytes transmitted	*/
> > +	NETSTAT_TYPE	_rx_errors;		/* bad packets received		*/
> > +	NETSTAT_TYPE	_tx_errors;		/* packet transmit problems	*/
> > +	NETSTAT_TYPE	_rx_dropped;		/* no space in linux buffers	*/
> > +	NETSTAT_TYPE	_tx_dropped;		/* no space available in linux	*/
> > +	NETSTAT_TYPE	_multicast;		/* multicast packets received	*/
> > +	NETSTAT_TYPE	_collisions;
>
> Increasing user-visible sizes arbitrarily breaks stuff.  Having
> config-dependent types like this increases complexity.

Not really, those macros used to change the variables hide everything from the
driver programmer. Besides those changes in procfs and sysfs which always
return 64-bits, everything else is type casted (if needed) by those macros -
depending on CONFIG_NETSTATS64.

> Short term, just sample the stats more rapidly.

That's what Linus said. But it is only a temporary fix.

> Long term, I suppose with 10GbE we should start thinking about this.
> Personally, I would prefer to make the standard net device stats
> available in the format already exported by ETHTOOL_GSTATS -- which I
> note uses u64's for its counters, and it's easily extensible.  I
> received a request for this just today, even.

I was thinking about making the 64-bit stats mandatory, but then I opted to
make in an option in the config. (As Linus pointed out, some people want
performance, not statistics.)

Jeff.

- --
I'm somewhere between geek and normal.
		- Linus Torvalds
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.2 (GNU/Linux)

iD8DBQE/BRhfwFP0+seVj/4RAtBbAJ4nmbs8ZQLgFagfb4KrJGZ55AYTmwCgzkcs
1uPma124BorLUdrcsbF2Txs=
=EIag
-----END PGP SIGNATURE-----

Re: [PATCH - RFC] [1/5] 64-bit network statistics - generic net

[bert hubert]

On Thu, Jul 03, 2003 at 08:08:03PM -0700, Linus Torvalds wrote:

> Please do this in user space. The "overflow every 2^32 packets" thing is 
> _not_ a problem, if you just gather the statistics at any kind of 
> reasonable interval.

At 114 megabits/second, we pass the mrtg threshold of an overflow within 5
minutes. A 1 gigabit link will do this once every 34 seconds. There are 10
gigabit adaptors out there which may need to be polled once every 3 seconds
then.

> Remember: "perfect is the enemy of good". 

Pretty good is not however. Can't we do what we do for jiffies where we only
do a 64 bit operation very seldomly?

Regards,

bert

-- 
http://www.PowerDNS.com      Open source, database driven DNS Software 
http://lartc.org           Linux Advanced Routing & Traffic Control HOWTO

Re: [PATCH - RFC] [1/5] 64-bit network statistics - generic net

[Jan-Benedict Glaw]

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

On Thu, 2003-07-03 22:31:27 -0400, Jeff Sipek <jeffpc@optonline.net>
wrote in message <200307032231.39842.jeffpc@optonline.net>:
Content-Description: clearsigned data
> The variables for network statistics (in struct net_device_stats) are unsigned 
> longs. On 32-bit architectures, this makes them overflow every 4GB or 2^32 
> packets. The following series of patches [against 2.5.74] makes the 
> statistics variable type configurable. The default is to leave everything the 
> way it was (unsigned long). However, when NETSTATS64 is set in the config, 
> the statistics use 64-bit variables (u_int64_t) - this works only on 32-bit 
> architectures.

Well... I don't really like to break userspace, but why don't we simply
make packet/traffic counters long long / u_int64_t? This way, we'd
simply keep almost all drivers untouched and only need to fiddle with
some sprints()/printk() statements?

Really, how many programs use the current statistics? I'd prefer to
modify them over adding strange patches like this one to the kernel...

MfG, JBG

-- 
   Jan-Benedict Glaw       jbglaw@lug-owl.de    . +49-172-7608481
   "Eine Freie Meinung in  einem Freien Kopf    | Gegen Zensur | Gegen Krieg
    fuer einen Freien Staat voll Freier Bürger" | im Internet! |   im Irak!
      ret = do_actions((curr | FREE_SPEECH) & ~(IRAQ_WAR_2 | DRM | TCPA));

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH - RFC] [1/5] 64-bit network statistics - generic net

[Jeff Sipek]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Friday 04 July 2003 05:47, Jan-Benedict Glaw wrote:
<snip>
> Well... I don't really like to break userspace, but why don't we simply
> make packet/traffic counters long long / u_int64_t? This way, we'd
> simply keep almost all drivers untouched and only need to fiddle with
> some sprints()/printk() statements?

I'm no hardware expert, however, that approach contains potential race 
condition - not a system critical one, but something we should be concerned 
about. If one cpu tries to read a u_int64_t variable while another tries to 
update it, the worst case scenario is that the reader will get the high 
32-bits before the write, and low 32-bit after the write, now if the counter 
overflow, the number would be off by 4GB! (This only applies to 32-bit 
architectures.) True, there are cache coherency algorithms, etc...

> Really, how many programs use the current statistics? I'd prefer to
> modify them over adding strange patches like this one to the kernel...

I believe that on any kind of router some at some point in time would like to 
know the data transfered.

Jeff.

- -- 
Keyboard not found!
Press F1 to enter Setup
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.2 (GNU/Linux)

iD8DBQE/BcADwFP0+seVj/4RAq2TAKDS0oAnj0/PrCuPoxdQF0euBiy6LACeMHqk
gWJhwub4y0VtQmC/hcevJB4=
=RCSe
-----END PGP SIGNATURE-----

Re: [PATCH - RFC] [1/5] 64-bit network statistics - generic net

[Jeff Sipek]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Thursday 03 July 2003 23:08, you wrote:
> Please do this in user space. The "overflow every 2^32 packets" thing is
> _not_ a problem, if you just gather the statistics at any kind of
> reasonable interval.
<snip>

While discussing this patch on IRC, an interesting idea came up: why not make 
the counters count something different from bytes? "Less granular stats are 
every bit (bad pun intended) as useful." This would break userspace, but 
that's what everyone has to expect during odd releases (i.e. the modules in 
2.5.)

To avoid having to change all the code, we could have (in addition to what we 
currently have) something like tx_kbytes or tx_mbytes which would be updated 
via a timer every x milliseconds (I'd say maybe 350-500). The sysfs and 
procfs interfaces would have to be modified, however those are just couple of 
lines of code.

Using KB would give us additional 10 bits (making the overflow at 4 TB.) I 
don't really like the idea of using MB, but the underlying idea is the same - 
20 more bits, making the limit 4 PB.

What is the consensus on this way of solving the problem?

Jeff.

- -- 
Failure is not an option,
It comes bundled with your Microsoft product.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.2 (GNU/Linux)

iD4DBQE/Bx23wFP0+seVj/4RApsVAJUaKZG6px09U87j6tCakrQQebj6AKC52f55
xSuyYxe62N8kefAoxposfg==
=As/F
-----END PGP SIGNATURE-----

Re: [PATCH - RFC] [1/5] 64-bit network statistics - generic net

[Bernd Eckenfels]

In article <200307041357.32871.jeffpc@optonline.net> you wrote:
> If one cpu tries to read a u_int64_t variable while another tries to 
> update it, the worst case scenario is that the reader will get the high 
> 32-bits before the write, and low 32-bit after the write, now if the counter 
> overflow, the number would be off by 4GB! (This only applies to 32-bit 
> architectures.) True, there are cache coherency algorithms, etc...

a reader like ifconfig can easyly work around this with multiple tries, but
incremeting those variables wont work that easy, and therefore needs a lock,
which will be a major pita.

64bit counters should be a result of lockless per-cpu network counters
(32bit) with some kind of async merging.

Or we wait till 64bit hardware is more common :)

Greetings
Bernd
-- 
eckes privat - http://www.eckes.org/
Project Freefire - http://www.freefire.org/

Re: [PATCH - RFC] [1/5] 64-bit network statistics - generic net

[Jeff Sipek]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Saturday 05 July 2003 15:58, Bernd Eckenfels wrote:
> a reader like ifconfig can easyly work around this with multiple tries, but
> incremeting those variables wont work that easy, and therefore needs a
> lock, which will be a major pita.
>
> 64bit counters should be a result of lockless per-cpu network counters
> (32bit) with some kind of async merging.

This is going to make the structure huge - not only you have the 32-bit 
variables for every CPU, but you have one global set of 64-bit variables 
(possibly you will need a lock for the 64-bit vars.)

Also another thing to consider is portability across architectures - we don't 
need all this code on 64-bit arches.

On the other hand, per-cpu stats may possibly make up for the extra code - no 
cache bouncing, etc.

> Or we wait till 64bit hardware is more common :)

Hehe, the thing is, that when 64bits beecome more common you will have this 
huge number of unused x86 computers that people will:

- - throw out
- - donate
- - convert to all sorts of "embedded" systems which need stable OS (read: 
Linux) (these include routers)

So, x86 is here to stay for some time.

Jeff.

- -- 
The Moon is Waxing Crescent (36% of Full)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.2 (GNU/Linux)

iD8DBQE/BzcbwFP0+seVj/4RAuMHAJ9sN0E4OgsPeM09D6hbgM3boECLDwCbBDTP
6u8SSobW0+Y0oWq3H4koHd0=
=Z89A
-----END PGP SIGNATURE-----

Re: [PATCH - RFC] [1/5] 64-bit network statistics - generic net

[Jeff Garzik]

The net stats are already unsigned long internally.

64-bit case is handled quite nicely today, thanks :)

I'm such a 64-bit bigot that "buy a 64-bit computer" is a solution I 
commonly suggest, and it seems to fit well here, too.

	Jeff, wondering if Intel will bother to compete w/ Athlon64

Re: [PATCH - RFC] [1/5] 64-bit network statistics - generic net

[Jeff Sipek]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Saturday 05 July 2003 16:40, Jeff Garzik wrote:
> The net stats are already unsigned long internally.
>
> 64-bit case is handled quite nicely today, thanks :)

I wonder why jiffies are not unsigned long...the 64-bit case would have been 
handled quite nicely too... :-)

> I'm such a 64-bit bigot that "buy a 64-bit computer" is a solution I
> commonly suggest, and it seems to fit well here, too.

I wish I had the resources to get one... :-(

The thing is that x86 is here to stay for quite some time. Even if 64-bit 
processors take over the market, you will have so many "old" computers that 
can:

- - be thrown out
- - donated to some institution
- - converted to routers, and other "embedded" systems

Plus, they will be dirt cheap.

> 	Jeff, wondering if Intel will bother to compete w/ Athlon64

Intel screwed up with Itanium, if it had "legacy" support, they would have 
been better off...

Jeff.

- -- 
Defenestration n. (formal or joc.):
  The act of removing Windows from your computer in disgust, usually followed
  by the installation of Linux or some other Unix-like operating system.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.2 (GNU/Linux)

iD8DBQE/BzwdwFP0+seVj/4RAoUZAJ0dptf9cB60dDUQHU61zg6lO5CXSQCgqHWU
8VvyepQwEdTsNFYyozGedAY=
=OsNH
-----END PGP SIGNATURE-----

Re: [PATCH - RFC] [1/5] 64-bit network statistics - generic net

[Ben Greear]

Jeff Garzik wrote:
> The net stats are already unsigned long internally.
> 
> 64-bit case is handled quite nicely today, thanks :)
> 
> I'm such a 64-bit bigot that "buy a 64-bit computer" is a solution I 
> commonly suggest, and it seems to fit well here, too.
> 
>     Jeff, wondering if Intel will bother to compete w/ Athlon64

Untill the net-stats are 64-bit on 32-bit systems, we will need some
way to know if they have wrapped or not when reading from nettool
and getting 64-bit numbers.

I guess what I really mean to say is that, if nettool is returning 64-bit
values, we need to know which ones are obtained from 32-bit counters.
32 -> 64 bit mapping will require wrap handling on low 32-bits, but
64 -> 64 bit mapping will require wrapping about 4-billion times less often :)

Perhaps a precision field is also needed for backwards/forwards compatability,
and perhaps a nettool version field as well to also help with backwards/forwards
compat.

Ben

> 
> 
> 


-- 
Ben Greear <greearb@candelatech.com>       <Ben_Greear AT excite.com>
President of Candela Technologies Inc      http://www.candelatech.com
ScryMUD:  http://scry.wanfear.com     http://scry.wanfear.com/~greear

Re: [PATCH - RFC] [1/5] 64-bit network statistics - generic net

[Ben Greear]

Jeff Sipek wrote:

> Using KB would give us additional 10 bits (making the overflow at 4 TB.) I 
> don't really like the idea of using MB, but the underlying idea is the same - 
> 20 more bits, making the limit 4 PB.
> 
> What is the consensus on this way of solving the problem?

I guess it could be useful for something like ifconfig, but serious
applications will need more precision and should deal with wraps anyway
(even on 64-bits, in my opinion..why have to fix bugs in 10 years because
we were too lazy to take the 10 minutes to make it right now).

Ben


-- 
Ben Greear <greearb@candelatech.com>       <Ben_Greear AT excite.com>
President of Candela Technologies Inc      http://www.candelatech.com
ScryMUD:  http://scry.wanfear.com     http://scry.wanfear.com/~greear

Re: [PATCH - RFC] [1/5] 64-bit network statistics - generic net

[Francois Romieu]

Jeff Sipek <jeffpc@optonline.net> :
[network counter overflow on 32 bits systems]
> The thing is that x86 is here to stay for quite some time. Even if 64-bit 
> processors take over the market, you will have so many "old" computers that 
> can:
> 
> - - be thrown out
> - - donated to some institution
> - - converted to routers, and other "embedded" systems
> 
> Plus, they will be dirt cheap.

- the PCI bus don't/won't/can't handle multiple 10 Gb/s adapters;
- nobody sane would recycle x86 systems as core routers after having bought
  a few Gb/s link.

--
Ueimor

Re: [PATCH - RFC] [1/5] 64-bit network statistics - generic net

[Jeff Sipek]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Saturday 05 July 2003 17:51, Francois Romieu wrote:
> Jeff Sipek <jeffpc@optonline.net> :
> > The thing is that x86 is here to stay for quite some time. Even if 64-bit
> > processors take over the market, you will have so many "old" computers
> > that can:
> >
> > - - be thrown out
> > - - donated to some institution
> > - - converted to routers, and other "embedded" systems
> >
> > Plus, they will be dirt cheap.
>
> - the PCI bus don't/won't/can't handle multiple 10 Gb/s adapters;

Ok, so let's stay in the range of gigabit ethernet...

> - nobody sane would recycle x86 systems as core routers after having bought
>   a few Gb/s link.

When you have "a few Gb/s links" you would not use your beloved Pentium 100 
MHz to do the job, instead you would go for something like 1.5 GHz P4 or 
Athlon, both of which would be cheaper than the new 64-bit architecture.

Jeff.

P.S. I just looked up the cheapest gigabit copper I could find in 10 seconds, 
and I found: D-Link DGE-500T for $36.27 this is just 4 times the price of the 
cheapest fast ethernet I found on the same site (cdw.com - they are not the 
cheapest, but I like them)

- -- 
A computer without Microsoft is like chocolate cake without mustard.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.2 (GNU/Linux)

iD8DBQE/B1OxwFP0+seVj/4RAkWfAJ9lYLk9zwpR2LpVLgVIDLovQewZKwCeLivr
bRCwwzVIj29rmxiT5tpmkaM=
=HXK9
-----END PGP SIGNATURE-----

Re: [PATCH - RFC] [1/5] 64-bit network statistics - generic net

[Roland Dreier]

    Francois> - the PCI bus don't/won't/can't handle multiple 10 Gb/s adapters

In a year, there will be 32-bit x86 systems with multiple 8X PCI
Express slots (16 Gb/sec full duplex to each slot).  In five years,
those systems will sell for $10 on Ebay.

 - Roland

Re: [PATCH - RFC] [1/5] 64-bit network statistics - generic net

[Francois Romieu]

Jeff Sipek <jeffpc@optonline.net> :
[...]
> P.S. I just looked up the cheapest gigabit copper I could find in 10 seconds, 
> and I found: D-Link DGE-500T for $36.27 this is just 4 times the price of the 
> cheapest fast ethernet I found on the same site (cdw.com - they are not the 
> cheapest, but I like them)

Please google around on the topic "nanog/gigabit/routing/linux" and read
netdev archive again.

It isn't _that_ simple.

--
Ueimor

Re: [PATCH - RFC] [1/5] 64-bit network statistics - generic net

[Alan Cox]

On Sad, 2003-07-05 at 22:41, Ben Greear wrote:
> Untill the net-stats are 64-bit on 32-bit systems, we will need some
> way to know if they have wrapped or not when reading from nettool
> and getting 64-bit numbers.

iptables

Collecting the data on a need to know basis 8)