Re: [PATCH]snmp6 64-bit counter support in proc.c

Re: [PATCH]snmp6 64-bit counter support in proc.c

[Krishna Kumar]

[-- Attachment #1.1: Type: text/plain, Size: 3921 bytes --]





Alexei,

That is a good point you raised, we don't want to read the counter while
the writer
might change and overflow of one word can result in a really corrupt value.

If 64bit counters is a good idea to implement, what I find OK to do is to
penalize the
readers (proc filesystem interface or netlink) but make sure that the
writers don't get
penalized by being forced to serialize, in effect writers must run as fast
as if there were
no other readers. I could think of a hack to make that happen (hopefully
not too ugly :-) :

#if 64_bit_system
the old code is OK here.
#else
__u64 get_sync_data(void *mib[], int nr)
{
      __u64 res1, res2;
      __u64 res3;

      res1 = *((__u64 *) (((void *) per_cpu_ptr(mib[0], i)) + sizeof
(__u64) * nr)));
      synchronize_kernel();
      res2 = *((__u64 *) (((void *) per_cpu_ptr(mib[0], i)) + sizeof
(__u64) * nr)));
      if (res2 < res1) {
            / * Overflow, sync and re-read, the next read is guaranteed to
be greater */
            synchronize_kernel();
            res2 = *((__u64 *) (((void *) per_cpu_ptr(mib[0], i)) + sizeof
(__u64) * nr)));
      }

      /* similar code for mib[1], add both into res3

      return res3;
}
#endif

static __u64
 fold_field(void *mib[], int nr)
{
      ...
      res += get_sync_data(mib, nr);
      ...
}

The value can reduce only once every 4gig increments, which means that
reading res2 after the first
sync_kernel will be less than res1 very rarely (once a few days on a fast
ethernet card). In case res2
is less than res1, doing another sync_kernel and rereading of res2 is
guaranteed to return a value
greater than res1 because another 4Gig iterations of increments couldn't
happen in the time for one
context switch of all cpus. The sync_kernel is needed so that we don't read
the value faster than the
writer is updating the two words.

Does that sound realistic for implementing 64 bit counters ? Or do you have
better or simpler
suggestions ?

Thanks,

- KK



|---------+---------------------------->
|         |           kuznet@ms2.inr.ac|
|         |           .ru              |
|         |           Sent by:         |
|         |           netdev-bounce@oss|
|         |           .sgi.com         |
|         |                            |
|         |                            |
|         |           01/22/2004 10:26 |
|         |           AM               |
|         |                            |
|---------+---------------------------->
  >-----------------------------------------------------------------------------------------------------------------|
  |                                                                                                                 |
  |       To:       Shirley Ma/Beaverton/IBM@IBMUS                                                                  |
  |       cc:       davem@redhat.com (David S. Miller), mashirle@us.ltcfwd.linux.ibm.com, kuznet@ms2.inr.ac.ru,     |
  |        netdev@oss.sgi.com                                                                                       |
  |       Subject:  Re: [PATCH]snmp6 64-bit counter support in proc.c                                               |
  |                                                                                                                 |
  >-----------------------------------------------------------------------------------------------------------------|




Hello!

> Did you hear different voices?

Here is a little warning. It will give corrupt values on 32 bit archs
when update with 32 bit overflow happens while value is folded.

To do 64 bit arithmetics you need either to serialize reader wrt writer
or to do some funny tricks with detecting overflows while reading and
special sequence of operations at update with proper barriers, which
will be reflected in performance anyway. Essentially, this haemorhoids
is the reason why they stayed 32 bit.

Alexey



[-- Attachment #1.2: Type: text/html, Size: 3992 bytes --]

[-- Attachment #2: pic20320.gif --]
[-- Type: image/gif, Size: 1255 bytes --]

Re: [PATCH]snmp6 64-bit counter support in proc.c

[David S. Miller]

The most portable and simple algorithm to solve this on the reader
side is (and I recommend we don't special case this on 64-bit platforms
just to get wider testing):

	u32 high, low1, low2;

	do {
		low1 = stat & 0xffffffff;
		rmb();
		high = stat >> 32;
		rmb();
		low2 = stat & 0xffffffff;
	} while (low2 < low1);

Something like that.  The idea is to sample the lower 32-bit twice
and if it overflows resample both high and low halfs.

Re: [PATCH]snmp6 64-bit counter support in proc.c

[Shirley Ma]

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





This solution doesn't favor writers also. How about do some udelay in the
reader to avoid modifing the writers?

Shirley Ma
IBM Linux Technology Center
15300 SW Koll Parkway
Beaverton, OR 97006-6063
Phone: (503) 578-7638
FAX:      (503) 578-3228

[-- Attachment #2: Type: text/html, Size: 308 bytes --]

Re: [PATCH]snmp6 64-bit counter support in proc.c

[Shirley Ma]

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





> Perhaps there are some better ideas?

Yes. Dave, could we create a new interface to avoid the spin_lock?

Example 1:
modify write_seqlock(set_lock *s) to write_seqlock(seq_lock *s, int lock),
it will modify all calling routines.

Example 2:
write a new interface write_seqlock_percpu(), which gets rid of the
spin_lock, we can call by a different name, since there is no lock.

Thanks
Shirley Ma
IBM Linux Technology Center
15300 SW Koll Parkway
Beaverton, OR 97006-6063
Phone: (503) 578-7638
FAX:      (503) 578-3228

[-- Attachment #2: Type: text/html, Size: 655 bytes --]

Re: [PATCH]snmp6 64-bit counter support in proc.c

[Krishna Kumar]

[-- Attachment #1.1: Type: text/plain, Size: 3011 bytes --]





Hi Dave,

> We can overflow the lower-half TWICE in the res1=...res2=... sequence
> you do, and this res2 reread in the if branch can overflow itself again,

The snippet assumed that the number will not overflow 4G times twice within
two reads, which I thought was quite a reasonable assumption.

I think a fast and clean solution is using Seq locks, which is how
jiffies_64
is implemented on 32 bit systems. The writers always get the spin lock (can
be per cpu with zero contention/thrashing) and increment the 'seq' number
of
seqlock data struct (all inside write_seqlock), then modify the 64 bit
counter,
and release the lock which increases the 'seq' again. The readers don't get
a lock at all, they just read the 'seq' before and after the reading of the
64 bit
counter, and if it changed, it needs to repeat this cycle of reads. In this
case,
no comparison of high/low order words are also needed. Does that sound
better ?

thanks,

- KK



|---------+---------------------------->
|         |           "David S. Miller"|
|         |           <davem@redhat.com|
|         |           >                |
|         |                            |
|         |           01/28/2004 11:09 |
|         |           AM               |
|         |                            |
|---------+---------------------------->
  >-----------------------------------------------------------------------------------------------------------------|
  |                                                                                                                 |
  |       To:       Krishna Kumar/Beaverton/IBM@IBMUS                                                               |
  |       cc:       kuznet@ms2.inr.ac.ru, mashirle@us.ltcfwd.linux.ibm.com, netdev@oss.sgi.com, Shirley             |
  |        Ma/Beaverton/IBM@IBMUS                                                                                   |
  |       Subject:  Re: [PATCH]snmp6 64-bit counter support in proc.c                                               |
  |                                                                                                                 |
  >-----------------------------------------------------------------------------------------------------------------|




On Thu, 22 Jan 2004 18:45:18 -0800
Krishna Kumar <kumarkr@us.ibm.com> wrote:

>             / * Overflow, sync and re-read, the next read is guaranteed
to
> be greater
>              * than res1.
>              */
>             synchronize_kernel();
>             res2 = *((__u64 *) (((void *) per_cpu_ptr(mib[0], i)) +
sizeof
> (__u64) * nr)));

I don't understand how your scheme can work.

We're trying to detect if the upper 32-bit half of the 64-bit value
belongs with the lower-half.  We can overflow the lower-half TWICE
in the res1=...res2=... sequence you do, and this res2 reread in the if
branch can overflow itself again, combining a lower and upper half which
do not belong to each other.

This is not so trivial to fix, see? :)


[-- Attachment #1.2: Type: text/html, Size: 3248 bytes --]

[-- Attachment #2: pic14735.gif --]
[-- Type: image/gif, Size: 1255 bytes --]

Re: [PATCH]snmp6 64-bit counter support in proc.c

[David S. Miller]

On Wed, 28 Jan 2004 11:19:36 -0800
Krishna Kumar <kumarkr@us.ibm.com> wrote:

[ ... idea to use seq locks ]
> Does that sound better ?

Well, I thought the goal was to move the expensive part of doing
this out of the writers, which we assume will exceed readers.
Seq locks favor readers, and assume that the write is the less
common operation.

Putting seq locks around every stat counter bump is going to plump
up the code a lot.

Maybe your idea and original assumption are fine, in essence we live
with this now, don't we? :-)

Perhaps there are some better ideas?

Re: [PATCH]snmp6 64-bit counter support in proc.c

[Shirley Ma]

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





> correctness > performance

OK, I am fixing the reader and will resubmit it when next release is
available.

Thanks
Shirley Ma
IBM Linux Technology Center
15300 SW Koll Parkway
Beaverton, OR 97006-6063
Phone: (503) 578-7638
FAX:      (503) 578-3228

[-- Attachment #2: Type: text/html, Size: 330 bytes --]

Re: [PATCH]snmp6 64-bit counter support in proc.c

[Krishna Kumar]

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






> correctness > performance

I agree with this statement, if every bug is explained as a "very tiny
race"
leads to one large defective product :-)

and performance also need not be hit much if writers are not penalized as I
had suggested earlier...

- KK

[-- Attachment #2: Type: text/html, Size: 535 bytes --]

Re: [PATCH]snmp6 64-bit counter support in proc.c

[Krishna Kumar]

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





Hi dave,

> > Do you really care about this situation? It only happens as a race
within
> > one instruction in 4 billion. It will slow everytime if we do this way.

> correctness > performance

> If MRTG hits this case and my net usage graphs have weird spikes
> in them as a result, can I assign the bugzilla entry to you? :-)

If so, do you think the solution that I proposed earlier would work ? No
doubt it is quite ugly to behold :-)
The one issue with it is one extra cache loading in 99.999999999% of cases
(2 instead of 1) and two
extra loading the remaining % of cases, but this is executed rarely and the
user can always wait for
data instead of penalizing the kernel writers. (synchronize_kernel is also
a little heavy, so maybe there
is a more lightweight mechanism to make sure that writer is finished).

thanks,

- KK

__u64 get_sync_data(void *mib[], int nr)
{
      __u64 res1, res2;
      __u64 res3;

      res1 = *((__u64 *) (((void *) per_cpu_ptr(mib[0], i)) + sizeof
(__u64) * nr)));
      synchronize_kernel();
      res2 = *((__u64 *) (((void *) per_cpu_ptr(mib[0], i)) + sizeof
(__u64) * nr)));
      if (res2 < res1) {
            / * Overflow, sync and re-read, the next read is guaranteed to
be greater
             * than res1.
             */
            synchronize_kernel();
            res2 = *((__u64 *) (((void *) per_cpu_ptr(mib[0], i)) + sizeof
(__u64) * nr)));
      }

      /* similar code for mib[1], add both into res3

      return res3;
}
#endif

static __u64
 fold_field(void *mib[], int nr)
{
      ...
      res += get_sync_data(mib, nr);
      ...
}

[-- Attachment #2: Type: text/html, Size: 1867 bytes --]

Re: [PATCH]snmp6 64-bit counter support in proc.c

[David S. Miller]

On Thu, 22 Jan 2004 18:45:18 -0800
Krishna Kumar <kumarkr@us.ibm.com> wrote:

>             / * Overflow, sync and re-read, the next read is guaranteed to
> be greater
>              * than res1.
>              */
>             synchronize_kernel();
>             res2 = *((__u64 *) (((void *) per_cpu_ptr(mib[0], i)) + sizeof
> (__u64) * nr)));

I don't understand how your scheme can work.

We're trying to detect if the upper 32-bit half of the 64-bit value
belongs with the lower-half.  We can overflow the lower-half TWICE
in the res1=...res2=... sequence you do, and this res2 reread in the if
branch can overflow itself again, combining a lower and upper half which
do not belong to each other.

This is not so trivial to fix, see? :)

Re: [PATCH]snmp6 64-bit counter support in proc.c

[Shirley Ma]

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





Do you really care about this situation? It only happens as a race within
one instruction in 4 billion. It will slow everytime if we do this way.

Thanks
Shirley Ma
IBM Linux Technology Center
15300 SW Koll Parkway
Beaverton, OR 97006-6063
Phone: (503) 578-7638
FAX:      (503) 578-3228

[-- Attachment #2: Type: text/html, Size: 359 bytes --]

Re: [PATCH]snmp6 64-bit counter support in proc.c

[David S. Miller]

   From: Shirley Ma <xma@us.ibm.com>
   Date: Thu, 22 Jan 2004 17:08:04 -0800
   
   Do you really care about this situation? It only happens as a race within
   one instruction in 4 billion. It will slow everytime if we do this way.

correctness > performance

If MRTG hits this case and my net usage graphs have weird spikes
in them as a result, can I assign the bugzilla entry to you? :-)

Re: [PATCH]snmp6 64-bit counter support in proc.c

[Krishna Kumar]

[-- Attachment #1.1: Type: text/plain, Size: 2814 bytes --]





Dave,

Isn't memory barrier used to stop re-ordering of instructions and needs to
be present
in both reader as well as writer to have synchronization since mb is for
the CPU on
which it is executing ? In this case : suppose stat is getting incremented
from
00000000 FFFFFFFF to 00000001 00000000, and stat was read after the low
word was
incremented to 0 (with overflow set), then low1 = 0 and low2 can get
executed before the
low2 is incremented on the other processor, so low2 is still zero. We
return zero, when
the value should be 4G. That why I felt that we need to read second after
making sure
the writer is over, that doesn't assume that writer is faster than reader
and works in all
cases.

Am I wrong here ?

thanks,

- KK



|---------+---------------------------->
|         |           "David S. Miller"|
|         |           <davem@redhat.com|
|         |           >                |
|         |           Sent by:         |
|         |           netdev-bounce@oss|
|         |           .sgi.com         |
|         |                            |
|         |                            |
|         |           01/22/2004 02:10 |
|         |           PM               |
|         |                            |
|---------+---------------------------->
  >-----------------------------------------------------------------------------------------------------------------|
  |                                                                                                                 |
  |       To:       Krishna Kumar/Beaverton/IBM@IBMUS                                                               |
  |       cc:       kuznet@ms2.inr.ac.ru, mashirle@us.ltcfwd.linux.ibm.com, netdev@oss.sgi.com,                     |
  |        netdev-bounce@oss.sgi.com, Shirley Ma/Beaverton/IBM@IBMUS                                                |
  |       Subject:  Re: [PATCH]snmp6 64-bit counter support in proc.c                                               |
  |                                                                                                                 |
  >-----------------------------------------------------------------------------------------------------------------|





The most portable and simple algorithm to solve this on the reader
side is (and I recommend we don't special case this on 64-bit platforms
just to get wider testing):

             u32 high, low1, low2;

             do {
                         low1 = stat & 0xffffffff;
                         rmb();
                         high = stat >> 32;
                         rmb();
                         low2 = stat & 0xffffffff;
             } while (low2 < low1);

Something like that.  The idea is to sample the lower 32-bit twice
and if it overflows resample both high and low halfs.



[-- Attachment #1.2: Type: text/html, Size: 2808 bytes --]

[-- Attachment #2: pic24167.gif --]
[-- Type: image/gif, Size: 1255 bytes --]

Re: [PATCH]snmp6 64-bit counter support in proc.c

[David S. Miller]

   From: Krishna Kumar <kumarkr@us.ibm.com>
   Date: Thu, 22 Jan 2004 14:50:15 -0800
   
   Isn't memory barrier used to stop re-ordering of instructions and needs to
   be present in both reader as well as writer to have synchronization
   since mb is for the CPU on which it is executing ?

You are correct.

Good thing I delayed this for a release, now we can work on
making sure we get this right. :-)

Re: [PATCH]snmp6 64-bit counter support in proc.c

[Shirley Ma]

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





Thanks for the input. I will fix it.

Thanks
Shirley Ma
IBM Linux Technology Center
15300 SW Koll Parkway
Beaverton, OR 97006-6063
Phone: (503) 578-7638
FAX:      (503) 578-3228

[-- Attachment #2: Type: text/html, Size: 244 bytes --]

Re: [PATCH]snmp6 64-bit counter support in proc.c

[Shirley Ma]

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





Hi, Dave,

> So I'm going to let this sit for another day or two so people can voice
any
objections they may have.

Did you hear different voices? If not could you please check in this patch?
I am going to submit another patch about new IPv6 MIBs system & interface
statistics counters for your review, which depends on this one.

Thanks
Shirley Ma
IBM Linux Technology Center
15300 SW Koll Parkway
Beaverton, OR 97006-6063
Phone: (503) 578-7638
FAX:      (503) 578-3228

[-- Attachment #2: Type: text/html, Size: 597 bytes --]

Re: [PATCH]snmp6 64-bit counter support in proc.c

[YOSHIFUJI Hideaki / 吉藤英明]

In article <OF1837C6DA.A9CB8829-ON87256E22.006C142B@us.ibm.com> (at Wed, 21 Jan 2004 11:45:30 -0800), Shirley Ma <xma@us.ibm.com> says:

> > So I'm going to let this sit for another day or two so people can voice
> any
> objections they may have.
> 
> Did you hear different voices? If not could you please check in this patch?

I'm not against holding in u64, 
but I rebember that Linus did not liked that.
Is it okay?

BTW,

diff -urN linux-2.6.1/net/ipv6/proc.c linux-2.6.1-ipv6mib2-64/net/ipv6/proc.c
:
 struct snmp6_item
 {
        char *name;
+       int   size;
        int   offset;
 };
:
+               if (size == 4) {
+                       res += *((__u32 *)
+                               (((void *)per_cpu_ptr(mib[0], i)) + offt));
+                       res += *((__u32 *)
+                               (((void *)per_cpu_ptr(mib[1], i)) + offt));
+               } else if (size == 8) {
+                       res += *((__u64 *)
+                               (((void *)per_cpu_ptr(mib[0], i)) + offt));
+                       res += *((__u32 *)
                                  ~~~~~__u64?
+                               (((void *)per_cpu_ptr(mib[1], i)) + offt));
+               }

I don't understand who really requires the "size" field.
The size is always 8, isn't it?
Am I missing sonething?

I'd prefer:
		if (size == sizeof(u32)) {
			:
                } else if (size == sizeof(u64) {
		        :
		}

> I am going to submit another patch about new IPv6 MIBs system & interface
> statistics counters for your review, which depends on this one.

Please be sure not to change the interface when you submit the next patch.

--yoshfuji

Re: [PATCH]snmp6 64-bit counter support in proc.c

[David S. Miller]

On Wed, 21 Jan 2004 11:45:30 -0800
Shirley Ma <xma@us.ibm.com> wrote:

> Did you hear different voices? If not could you please check in this patch?
> I am going to submit another patch about new IPv6 MIBs system & interface
> statistics counters for your review, which depends on this one.

No objections, and I am fine with the change as well.

However, I think I will defer it as 2.6.x has a lot of changes in it already.
Definitely next release.

Re: [PATCH]snmp6 64-bit counter support in proc.c

[kuznet]

Hello!

> Did you hear different voices?

Here is a little warning. It will give corrupt values on 32 bit archs
when update with 32 bit overflow happens while value is folded.

To do 64 bit arithmetics you need either to serialize reader wrt writer
or to do some funny tricks with detecting overflows while reading and
special sequence of operations at update with proper barriers, which
will be reflected in performance anyway. Essentially, this haemorhoids
is the reason why they stayed 32 bit.

Alexey

Re: [PATCH]snmp6 64-bit counter support in proc.c

[Shirley Ma]

Thanks, Dave. When I created the patch, I was concern about this.
I will change the MIBs counters to 'u32' or 'u64'.

Thanks
Shirley Ma
IBM Linux Technology Center
15300 SW Koll Parkway
Beaverton, OR 97006-6063
Phone: (503) 578-7638
FAX:      (503) 578-3228



"David S. Miller" <davem@redhat.com> on 12/05/2003 12:31:33 PM

To:    mashirle@us.ltcfwd.linux.ibm.com
cc:    kuznet@ms2.inr.ac.ru, netdev@oss.sgi.com, Shirley
       Ma/Beaverton/IBM@IBMUS
Subject:    Re: [PATCH]snmp6 64-bit counter support in proc.c


On Fri, 5 Dec 2003 12:14:47 -0800
Shirley Ma <mashirle@us.ibm.com> wrote:

> I add this because there are some 64-bit counters in the new IPv6 MIBs.
> This patch has been tested agaist linux-2.6.0-test9, and cleanly applied
to
> linux-2.6.0-test11.

"sizeof(unsigned long)" evaluates to 8 on 64-bit systems,
yet you assume it always evaluated to 4 as on 32-bit systems.

Maybe it would be wiser to explicitly use 'u32' and 'u64' for
the types of the snmp counters?

This has always been a sore area.

Re: IPv6 MIB:ipv6PrefixTable implementation

[kuznet]

Hello!

> One implementation detail question, do you think I need to save all the o=
> ther=20
> Prefix Objects: Type, Origin(addrconf, manually, dhcp, others),

These two things should be stored right now. Existing implementation is
quite a mess, but we definitely want to remember origin of each route
in "protocol" and another flags, they are of common interest.


> AutonomoueFlag, AdvPreferredLiftTime and ValidLifeTime

ValidLifeTime is "expires" on this route.

What's about AdvPreferredLiftTime I am puzzled a little,
preferred time is not an attribute of a prefix at all,
it is attribute of address, is not it? Unless the prefix is used
to install local address it does not make sense, right?

Alexey

[PATCH]snmp6 64-bit counter support in proc.c

[Shirley Ma]

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

I add this because there are some 64-bit counters in the new IPv6 MIBs. 
This patch has been tested agaist linux-2.6.0-test9, and cleanly applied to 
linux-2.6.0-test11.

Thanks
Shirley Ma
IBM Linux Technology Center

[-- Attachment #2: linux-2.6.0-test11-ipv6mib2-64.patch --]
[-- Type: text/x-diff, Size: 3156 bytes --]

diff -urN linux-2.6.0-test11/net/ipv6/proc.c linux-2.6.0-test11-ipv6mib2-64/net/ipv6/proc.c
--- linux-2.6.0-test11/net/ipv6/proc.c	2003-11-26 12:43:38.000000000 -0800
+++ linux-2.6.0-test11-ipv6mib2-64/net/ipv6/proc.c	2003-12-05 12:04:19.000000000 -0800
@@ -60,13 +60,14 @@
 struct snmp6_item
 {
 	char *name;
+	int   size;
 	int   offset;
 };
-#define SNMP6_SENTINEL	{ .name = NULL, .offset = 0 }
+#define SNMP6_SENTINEL	{ .name = NULL, .size = 0, .offset = 0 }
 
 static struct snmp6_item snmp6_ipv6_list[] = {
 /* ipv6 mib according to RFC 2465 */
-#define SNMP6_GEN(x) { .name = #x , .offset = offsetof(struct ipv6_mib, x) }
+#define SNMP6_GEN(x) { .name = #x , .size = sizeof(((struct ipv6_mib *)0)->x), .offset = offsetof(struct ipv6_mib, x) }
 	SNMP6_GEN(Ip6InReceives),
 	SNMP6_GEN(Ip6InHdrErrors),
 	SNMP6_GEN(Ip6InTooBigErrors),
@@ -104,7 +105,7 @@
 		OutRouterAdvertisements too.
 		OutGroupMembQueries too.
  */
-#define SNMP6_GEN(x) { .name = #x , .offset = offsetof(struct icmpv6_mib, x) }
+#define SNMP6_GEN(x) { .name = #x , .size = sizeof(((struct icmpv6_mib *)0)->x), .offset = offsetof(struct icmpv6_mib, x) }
 	SNMP6_GEN(Icmp6InMsgs),
 	SNMP6_GEN(Icmp6InErrors),
 	SNMP6_GEN(Icmp6InDestUnreachs),
@@ -138,7 +139,7 @@
 };
 
 static struct snmp6_item snmp6_udp6_list[] = {
-#define SNMP6_GEN(x) { .name = "Udp6" #x , .offset = offsetof(struct udp_mib, Udp##x) }
+#define SNMP6_GEN(x) { .name = "Udp6" #x , .size = sizeof(((struct udp_mib *)0)->Udp##x), .offset = offsetof(struct udp_mib, Udp##x) }
 	SNMP6_GEN(InDatagrams),
 	SNMP6_GEN(NoPorts),
 	SNMP6_GEN(InErrors),
@@ -147,22 +148,27 @@
 	SNMP6_SENTINEL
 };
 
-static unsigned long
-fold_field(void *mib[], int offt)
+static unsigned long long
+fold_field(void *mib[], int size, int offt)
 {
-        unsigned long res = 0;
+        unsigned long long res = 0;
         int i;
  
         for (i = 0; i < NR_CPUS; i++) {
                 if (!cpu_possible(i))
                         continue;
-                res +=
-                    *((unsigned long *) (((void *)per_cpu_ptr(mib[0], i)) +
-                                         offt));
-                res +=
-                    *((unsigned long *) (((void *)per_cpu_ptr(mib[1], i)) +
-                                         offt));
-        }
+		if (size == 4) {
+			res += *((unsigned long *) 
+				(((void *)per_cpu_ptr(mib[0], i)) + offt));
+			res += *((unsigned long *)
+				(((void *)per_cpu_ptr(mib[1], i)) + offt));
+		} else if (size == 8) {
+			res += *((unsigned long long *) 
+				(((void *)per_cpu_ptr(mib[0], i)) + offt));
+			res += *((unsigned long long *)
+				(((void *)per_cpu_ptr(mib[1], i)) + offt));
+		}
+	}
         return res;
 }
 
@@ -170,9 +176,9 @@
 snmp6_seq_show_item(struct seq_file *seq, void **mib, struct snmp6_item *itemlist)
 {
 	int i;
-	for (i=0; itemlist[i].name; i++)
-		seq_printf(seq, "%-32s\t%lu\n", itemlist[i].name, 
-				fold_field(mib, itemlist[i].offset));
+	for (i=0; itemlist[i].name; i++) 
+		seq_printf(seq, "%-32s\t%llu\n", itemlist[i].name, 
+			fold_field(mib, itemlist[i].size, itemlist[i].offset));
 }
 
 static int snmp6_seq_show(struct seq_file *seq, void *v)

Re: [PATCH]snmp6 64-bit counter support in proc.c

[David S. Miller]

On Fri, 5 Dec 2003 12:14:47 -0800
Shirley Ma <mashirle@us.ibm.com> wrote:

> I add this because there are some 64-bit counters in the new IPv6 MIBs. 
> This patch has been tested agaist linux-2.6.0-test9, and cleanly applied to 
> linux-2.6.0-test11.

"sizeof(unsigned long)" evaluates to 8 on 64-bit systems,
yet you assume it always evaluated to 4 as on 32-bit systems.

Maybe it would be wiser to explicitly use 'u32' and 'u64' for
the types of the snmp counters?

This has always been a sore area.

Re: [PATCH]snmp6 64-bit counter support in proc.c

[Shirley Ma]

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

> "sizeof(unsigned long)" evaluates to 8 on 64-bit systems,
> yet you assume it always evaluated to 4 as on 32-bit systems.
>
> Maybe it would be wiser to explicitly use 'u32' and 'u64' for
> the types of the snmp counters?
>
> This has always been a sore area.

This is the new patch against 2.6.1 kernel.

Thanks
Shirley Ma
IBM Linux Technology Center

[-- Attachment #2: linux-2.6.1-ipv6mib2-64.patch --]
[-- Type: text/x-diff, Size: 20376 bytes --]

diff -urN linux-2.6.1/include/net/snmp.h linux-2.6.1-ipv6mib2-64/include/net/snmp.h
--- linux-2.6.1/include/net/snmp.h	2004-01-08 22:59:26.000000000 -0800
+++ linux-2.6.1-ipv6mib2-64/include/net/snmp.h	2004-01-13 13:45:23.000000000 -0800
@@ -49,24 +49,24 @@
  */
 struct ip_mib
 {
- 	unsigned long	IpInReceives;
- 	unsigned long	IpInHdrErrors;
- 	unsigned long	IpInAddrErrors;
- 	unsigned long	IpForwDatagrams;
- 	unsigned long	IpInUnknownProtos;
- 	unsigned long	IpInDiscards;
- 	unsigned long	IpInDelivers;
- 	unsigned long	IpOutRequests;
- 	unsigned long	IpOutDiscards;
- 	unsigned long	IpOutNoRoutes;
- 	unsigned long	IpReasmTimeout;
- 	unsigned long	IpReasmReqds;
- 	unsigned long	IpReasmOKs;
- 	unsigned long	IpReasmFails;
- 	unsigned long	IpFragOKs;
- 	unsigned long	IpFragFails;
- 	unsigned long	IpFragCreates;
-	unsigned long   __pad[0]; 
+ 	__u64	IpInReceives;
+ 	__u64	IpInHdrErrors;
+ 	__u64	IpInAddrErrors;
+ 	__u64	IpForwDatagrams;
+ 	__u64	IpInUnknownProtos;
+ 	__u64	IpInDiscards;
+ 	__u64	IpInDelivers;
+ 	__u64	IpOutRequests;
+ 	__u64	IpOutDiscards;
+ 	__u64	IpOutNoRoutes;
+ 	__u64	IpReasmTimeout;
+ 	__u64	IpReasmReqds;
+ 	__u64	IpReasmOKs;
+ 	__u64	IpReasmFails;
+ 	__u64	IpFragOKs;
+ 	__u64	IpFragFails;
+ 	__u64	IpFragCreates;
+	__u64   __pad[0]; 
 };
  
 /*
@@ -74,29 +74,29 @@
  */
 struct ipv6_mib
 {
-	unsigned long	Ip6InReceives;
- 	unsigned long	Ip6InHdrErrors;
- 	unsigned long	Ip6InTooBigErrors;
- 	unsigned long	Ip6InNoRoutes;
- 	unsigned long	Ip6InAddrErrors;
- 	unsigned long	Ip6InUnknownProtos;
- 	unsigned long	Ip6InTruncatedPkts;
- 	unsigned long	Ip6InDiscards;
- 	unsigned long	Ip6InDelivers;
- 	unsigned long	Ip6OutForwDatagrams;
- 	unsigned long	Ip6OutRequests;
- 	unsigned long	Ip6OutDiscards;
- 	unsigned long	Ip6OutNoRoutes;
- 	unsigned long	Ip6ReasmTimeout;
- 	unsigned long	Ip6ReasmReqds;
- 	unsigned long	Ip6ReasmOKs;
- 	unsigned long	Ip6ReasmFails;
- 	unsigned long	Ip6FragOKs;
- 	unsigned long	Ip6FragFails;
- 	unsigned long	Ip6FragCreates;
- 	unsigned long	Ip6InMcastPkts;
- 	unsigned long	Ip6OutMcastPkts;
-	unsigned long   __pad[0]; 
+	__u64	Ip6InReceives;
+ 	__u64	Ip6InHdrErrors;
+ 	__u64	Ip6InTooBigErrors;
+ 	__u64	Ip6InNoRoutes;
+ 	__u64	Ip6InAddrErrors;
+ 	__u64	Ip6InUnknownProtos;
+ 	__u64	Ip6InTruncatedPkts;
+ 	__u64	Ip6InDiscards;
+ 	__u64	Ip6InDelivers;
+ 	__u64	Ip6OutForwDatagrams;
+ 	__u64	Ip6OutRequests;
+ 	__u64	Ip6OutDiscards;
+ 	__u64	Ip6OutNoRoutes;
+ 	__u64	Ip6ReasmTimeout;
+ 	__u64	Ip6ReasmReqds;
+ 	__u64	Ip6ReasmOKs;
+ 	__u64	Ip6ReasmFails;
+ 	__u64	Ip6FragOKs;
+ 	__u64	Ip6FragFails;
+ 	__u64	Ip6FragCreates;
+ 	__u64	Ip6InMcastPkts;
+ 	__u64	Ip6OutMcastPkts;
+	__u64   __pad[0]; 
 };
  
 /*
@@ -105,34 +105,34 @@
  */
 struct icmp_mib
 {
- 	unsigned long	IcmpInMsgs;
- 	unsigned long	IcmpInErrors;
-  	unsigned long	IcmpInDestUnreachs;
- 	unsigned long	IcmpInTimeExcds;
- 	unsigned long	IcmpInParmProbs;
- 	unsigned long	IcmpInSrcQuenchs;
- 	unsigned long	IcmpInRedirects;
- 	unsigned long	IcmpInEchos;
- 	unsigned long	IcmpInEchoReps;
- 	unsigned long	IcmpInTimestamps;
- 	unsigned long	IcmpInTimestampReps;
- 	unsigned long	IcmpInAddrMasks;
- 	unsigned long	IcmpInAddrMaskReps;
- 	unsigned long	IcmpOutMsgs;
- 	unsigned long	IcmpOutErrors;
- 	unsigned long	IcmpOutDestUnreachs;
- 	unsigned long	IcmpOutTimeExcds;
- 	unsigned long	IcmpOutParmProbs;
- 	unsigned long	IcmpOutSrcQuenchs;
- 	unsigned long	IcmpOutRedirects;
- 	unsigned long	IcmpOutEchos;
- 	unsigned long	IcmpOutEchoReps;
- 	unsigned long	IcmpOutTimestamps;
- 	unsigned long	IcmpOutTimestampReps;
- 	unsigned long	IcmpOutAddrMasks;
- 	unsigned long	IcmpOutAddrMaskReps;
-	unsigned long	dummy;
-	unsigned long   __pad[0]; 
+ 	__u64	IcmpInMsgs;
+ 	__u64	IcmpInErrors;
+  	__u64	IcmpInDestUnreachs;
+ 	__u64	IcmpInTimeExcds;
+ 	__u64	IcmpInParmProbs;
+ 	__u64	IcmpInSrcQuenchs;
+ 	__u64	IcmpInRedirects;
+ 	__u64	IcmpInEchos;
+ 	__u64	IcmpInEchoReps;
+ 	__u64	IcmpInTimestamps;
+ 	__u64	IcmpInTimestampReps;
+ 	__u64	IcmpInAddrMasks;
+ 	__u64	IcmpInAddrMaskReps;
+ 	__u64	IcmpOutMsgs;
+ 	__u64	IcmpOutErrors;
+ 	__u64	IcmpOutDestUnreachs;
+ 	__u64	IcmpOutTimeExcds;
+ 	__u64	IcmpOutParmProbs;
+ 	__u64	IcmpOutSrcQuenchs;
+ 	__u64	IcmpOutRedirects;
+ 	__u64	IcmpOutEchos;
+ 	__u64	IcmpOutEchoReps;
+ 	__u64	IcmpOutTimestamps;
+ 	__u64	IcmpOutTimestampReps;
+ 	__u64	IcmpOutAddrMasks;
+ 	__u64	IcmpOutAddrMaskReps;
+	__u64	dummy;
+	__u64	__pad[0]; 
 };
 
 /*
@@ -140,40 +140,35 @@
  */
 struct icmpv6_mib
 {
-	unsigned long	Icmp6InMsgs;
-	unsigned long	Icmp6InErrors;
-
-	unsigned long	Icmp6InDestUnreachs;
-	unsigned long	Icmp6InPktTooBigs;
-	unsigned long	Icmp6InTimeExcds;
-	unsigned long	Icmp6InParmProblems;
-
-	unsigned long	Icmp6InEchos;
-	unsigned long	Icmp6InEchoReplies;
-	unsigned long	Icmp6InGroupMembQueries;
-	unsigned long	Icmp6InGroupMembResponses;
-	unsigned long	Icmp6InGroupMembReductions;
-	unsigned long	Icmp6InRouterSolicits;
-	unsigned long	Icmp6InRouterAdvertisements;
-	unsigned long	Icmp6InNeighborSolicits;
-	unsigned long	Icmp6InNeighborAdvertisements;
-	unsigned long	Icmp6InRedirects;
-
-	unsigned long	Icmp6OutMsgs;
-
-	unsigned long	Icmp6OutDestUnreachs;
-	unsigned long	Icmp6OutPktTooBigs;
-	unsigned long	Icmp6OutTimeExcds;
-	unsigned long	Icmp6OutParmProblems;
-
-	unsigned long	Icmp6OutEchoReplies;
-	unsigned long	Icmp6OutRouterSolicits;
-	unsigned long	Icmp6OutNeighborSolicits;
-	unsigned long	Icmp6OutNeighborAdvertisements;
-	unsigned long	Icmp6OutRedirects;
-	unsigned long	Icmp6OutGroupMembResponses;
-	unsigned long	Icmp6OutGroupMembReductions;
-	unsigned long   __pad[0]; 
+	__u64	Icmp6InMsgs;
+	__u64	Icmp6InErrors;
+	__u64	Icmp6InDestUnreachs;
+	__u64	Icmp6InPktTooBigs;
+	__u64	Icmp6InTimeExcds;
+	__u64	Icmp6InParmProblems;
+	__u64	Icmp6InEchos;
+	__u64	Icmp6InEchoReplies;
+	__u64	Icmp6InGroupMembQueries;
+	__u64	Icmp6InGroupMembResponses;
+	__u64	Icmp6InGroupMembReductions;
+	__u64	Icmp6InRouterSolicits;
+	__u64	Icmp6InRouterAdvertisements;
+	__u64	Icmp6InNeighborSolicits;
+	__u64	Icmp6InNeighborAdvertisements;
+	__u64	Icmp6InRedirects;
+	__u64	Icmp6OutMsgs;
+	__u64	Icmp6OutDestUnreachs;
+	__u64	Icmp6OutPktTooBigs;
+	__u64	Icmp6OutTimeExcds;
+	__u64	Icmp6OutParmProblems;
+	__u64	Icmp6OutEchoReplies;
+	__u64	Icmp6OutRouterSolicits;
+	__u64	Icmp6OutNeighborSolicits;
+	__u64	Icmp6OutNeighborAdvertisements;
+	__u64	Icmp6OutRedirects;
+	__u64	Icmp6OutGroupMembResponses;
+	__u64	Icmp6OutGroupMembReductions;
+	__u64   __pad[0]; 
 };
  
 /*
@@ -182,21 +177,21 @@
  */
 struct tcp_mib
 {
- 	unsigned long	TcpRtoAlgorithm;
- 	unsigned long	TcpRtoMin;
- 	unsigned long	TcpRtoMax;
- 	unsigned long	TcpMaxConn;
- 	unsigned long	TcpActiveOpens;
- 	unsigned long	TcpPassiveOpens;
- 	unsigned long	TcpAttemptFails;
- 	unsigned long	TcpEstabResets;
- 	unsigned long	TcpCurrEstab;
- 	unsigned long	TcpInSegs;
- 	unsigned long	TcpOutSegs;
- 	unsigned long	TcpRetransSegs;
- 	unsigned long	TcpInErrs;
- 	unsigned long	TcpOutRsts;
-	unsigned long   __pad[0]; 
+ 	__u64	TcpRtoAlgorithm;
+ 	__u64	TcpRtoMin;
+ 	__u64	TcpRtoMax;
+ 	__u64	TcpMaxConn;
+ 	__u64	TcpActiveOpens;
+ 	__u64	TcpPassiveOpens;
+ 	__u64	TcpAttemptFails;
+ 	__u64	TcpEstabResets;
+ 	__u64	TcpCurrEstab;
+ 	__u64	TcpInSegs;
+ 	__u64	TcpOutSegs;
+ 	__u64	TcpRetransSegs;
+ 	__u64	TcpInErrs;
+ 	__u64	TcpOutRsts;
+	__u64   __pad[0]; 
 };
  
 /*
@@ -205,110 +200,110 @@
  */
 struct udp_mib
 {
- 	unsigned long	UdpInDatagrams;
- 	unsigned long	UdpNoPorts;
- 	unsigned long	UdpInErrors;
- 	unsigned long	UdpOutDatagrams;
-	unsigned long   __pad[0];
+ 	__u64	UdpInDatagrams;
+ 	__u64	UdpNoPorts;
+ 	__u64	UdpInErrors;
+ 	__u64	UdpOutDatagrams;
+	__u64   __pad[0];
 }; 
 
 /* draft-ietf-sigtran-sctp-mib-07.txt */
 struct sctp_mib
 {
-	unsigned long   SctpCurrEstab;
-	unsigned long   SctpActiveEstabs;
-	unsigned long   SctpPassiveEstabs;
-	unsigned long   SctpAborteds;
-	unsigned long   SctpShutdowns;
-	unsigned long   SctpOutOfBlues;
-	unsigned long   SctpChecksumErrors;
-	unsigned long   SctpOutCtrlChunks;
-	unsigned long   SctpOutOrderChunks;
-	unsigned long   SctpOutUnorderChunks;
-	unsigned long   SctpInCtrlChunks;
-	unsigned long   SctpInOrderChunks;
-	unsigned long   SctpInUnorderChunks;
-	unsigned long   SctpFragUsrMsgs;
-	unsigned long   SctpReasmUsrMsgs;
-	unsigned long   SctpOutSCTPPacks;
-	unsigned long   SctpInSCTPPacks;
-	unsigned long   SctpRtoAlgorithm;
-	unsigned long   SctpRtoMin;
-	unsigned long   SctpRtoMax;
-	unsigned long   SctpRtoInitial;
-	unsigned long   SctpValCookieLife;
-	unsigned long   SctpMaxInitRetr;
-	unsigned long   __pad[0];
+	__u64   SctpCurrEstab;
+	__u64   SctpActiveEstabs;
+	__u64   SctpPassiveEstabs;
+	__u64   SctpAborteds;
+	__u64   SctpShutdowns;
+	__u64   SctpOutOfBlues;
+	__u64   SctpChecksumErrors;
+	__u64   SctpOutCtrlChunks;
+	__u64   SctpOutOrderChunks;
+	__u64   SctpOutUnorderChunks;
+	__u64   SctpInCtrlChunks;
+	__u64   SctpInOrderChunks;
+	__u64   SctpInUnorderChunks;
+	__u64   SctpFragUsrMsgs;
+	__u64   SctpReasmUsrMsgs;
+	__u64   SctpOutSCTPPacks;
+	__u64   SctpInSCTPPacks;
+	__u64   SctpRtoAlgorithm;
+	__u64   SctpRtoMin;
+	__u64   SctpRtoMax;
+	__u64   SctpRtoInitial;
+	__u64   SctpValCookieLife;
+	__u64   SctpMaxInitRetr;
+	__u64   __pad[0];
 };
 
 struct linux_mib 
 {
-	unsigned long	SyncookiesSent;
-	unsigned long	SyncookiesRecv;
-	unsigned long	SyncookiesFailed;
-	unsigned long	EmbryonicRsts;
-	unsigned long	PruneCalled; 
-	unsigned long	RcvPruned;
-	unsigned long	OfoPruned;
-	unsigned long	OutOfWindowIcmps; 
-	unsigned long	LockDroppedIcmps; 
-        unsigned long   ArpFilter;
-	unsigned long	TimeWaited; 
-	unsigned long	TimeWaitRecycled; 
-	unsigned long	TimeWaitKilled; 
-	unsigned long	PAWSPassiveRejected; 
-	unsigned long	PAWSActiveRejected; 
-	unsigned long	PAWSEstabRejected; 
-	unsigned long	DelayedACKs;
-	unsigned long	DelayedACKLocked;
-	unsigned long	DelayedACKLost;
-	unsigned long	ListenOverflows;
-	unsigned long	ListenDrops;
-	unsigned long	TCPPrequeued;
-	unsigned long	TCPDirectCopyFromBacklog;
-	unsigned long	TCPDirectCopyFromPrequeue;
-	unsigned long	TCPPrequeueDropped;
-	unsigned long	TCPHPHits;
-	unsigned long	TCPHPHitsToUser;
-	unsigned long	TCPPureAcks;
-	unsigned long	TCPHPAcks;
-	unsigned long	TCPRenoRecovery;
-	unsigned long	TCPSackRecovery;
-	unsigned long	TCPSACKReneging;
-	unsigned long	TCPFACKReorder;
-	unsigned long	TCPSACKReorder;
-	unsigned long	TCPRenoReorder;
-	unsigned long	TCPTSReorder;
-	unsigned long	TCPFullUndo;
-	unsigned long	TCPPartialUndo;
-	unsigned long	TCPDSACKUndo;
-	unsigned long	TCPLossUndo;
-	unsigned long	TCPLoss;
-	unsigned long	TCPLostRetransmit;
-	unsigned long	TCPRenoFailures;
-	unsigned long	TCPSackFailures;
-	unsigned long	TCPLossFailures;
-	unsigned long	TCPFastRetrans;
-	unsigned long	TCPForwardRetrans;
-	unsigned long	TCPSlowStartRetrans;
-	unsigned long	TCPTimeouts;
-	unsigned long	TCPRenoRecoveryFail;
-	unsigned long	TCPSackRecoveryFail;
-	unsigned long	TCPSchedulerFailed;
-	unsigned long	TCPRcvCollapsed;
-	unsigned long	TCPDSACKOldSent;
-	unsigned long	TCPDSACKOfoSent;
-	unsigned long	TCPDSACKRecv;
-	unsigned long	TCPDSACKOfoRecv;
-	unsigned long	TCPAbortOnSyn;
-	unsigned long	TCPAbortOnData;
-	unsigned long	TCPAbortOnClose;
-	unsigned long	TCPAbortOnMemory;
-	unsigned long	TCPAbortOnTimeout;
-	unsigned long	TCPAbortOnLinger;
-	unsigned long	TCPAbortFailed;
-	unsigned long	TCPMemoryPressures;
-	unsigned long   __pad[0];
+	__u64	SyncookiesSent;
+	__u64	SyncookiesRecv;
+	__u64	SyncookiesFailed;
+	__u64	EmbryonicRsts;
+	__u64	PruneCalled; 
+	__u64	RcvPruned;
+	__u64	OfoPruned;
+	__u64	OutOfWindowIcmps; 
+	__u64	LockDroppedIcmps; 
+	__u64   ArpFilter;
+	__u64	TimeWaited; 
+	__u64	TimeWaitRecycled; 
+	__u64	TimeWaitKilled; 
+	__u64	PAWSPassiveRejected; 
+	__u64	PAWSActiveRejected; 
+	__u64	PAWSEstabRejected; 
+	__u64	DelayedACKs;
+	__u64	DelayedACKLocked;
+	__u64	DelayedACKLost;
+	__u64	ListenOverflows;
+	__u64	ListenDrops;
+	__u64	TCPPrequeued;
+	__u64	TCPDirectCopyFromBacklog;
+	__u64	TCPDirectCopyFromPrequeue;
+	__u64	TCPPrequeueDropped;
+	__u64	TCPHPHits;
+	__u64	TCPHPHitsToUser;
+	__u64	TCPPureAcks;
+	__u64	TCPHPAcks;
+	__u64	TCPRenoRecovery;
+	__u64	TCPSackRecovery;
+	__u64	TCPSACKReneging;
+	__u64	TCPFACKReorder;
+	__u64	TCPSACKReorder;
+	__u64	TCPRenoReorder;
+	__u64	TCPTSReorder;
+	__u64	TCPFullUndo;
+	__u64	TCPPartialUndo;
+	__u64	TCPDSACKUndo;
+	__u64	TCPLossUndo;
+	__u64	TCPLoss;
+	__u64	TCPLostRetransmit;
+	__u64	TCPRenoFailures;
+	__u64	TCPSackFailures;
+	__u64	TCPLossFailures;
+	__u64	TCPFastRetrans;
+	__u64	TCPForwardRetrans;
+	__u64	TCPSlowStartRetrans;
+	__u64	TCPTimeouts;
+	__u64	TCPRenoRecoveryFail;
+	__u64	TCPSackRecoveryFail;
+	__u64	TCPSchedulerFailed;
+	__u64	TCPRcvCollapsed;
+	__u64	TCPDSACKOldSent;
+	__u64	TCPDSACKOfoSent;
+	__u64	TCPDSACKRecv;
+	__u64	TCPDSACKOfoRecv;
+	__u64	TCPAbortOnSyn;
+	__u64	TCPAbortOnData;
+	__u64	TCPAbortOnClose;
+	__u64	TCPAbortOnMemory;
+	__u64	TCPAbortOnTimeout;
+	__u64	TCPAbortOnLinger;
+	__u64	TCPAbortFailed;
+	__u64	TCPMemoryPressures;
+	__u64   __pad[0];
 };
 
 
diff -urN linux-2.6.1/net/ipv4/proc.c linux-2.6.1-ipv6mib2-64/net/ipv4/proc.c
--- linux-2.6.1/net/ipv4/proc.c	2004-01-08 22:59:05.000000000 -0800
+++ linux-2.6.1-ipv6mib2-64/net/ipv4/proc.c	2004-01-13 14:06:51.000000000 -0800
@@ -87,21 +87,21 @@
 	.release = single_release,
 };
 
-static unsigned long
+static __u64 
 fold_field(void *mib[], int nr)
 {
-	unsigned long res = 0;
+	__u64 res = 0;
 	int i;
 
 	for (i = 0; i < NR_CPUS; i++) {
 		if (!cpu_possible(i))
 			continue;
 		res +=
-		    *((unsigned long *) (((void *) per_cpu_ptr(mib[0], i)) +
-					 sizeof (unsigned long) * nr));
+		    *((__u64 *) (((void *) per_cpu_ptr(mib[0], i)) +
+					 sizeof (__u64) * nr));
 		res +=
-		    *((unsigned long *) (((void *) per_cpu_ptr(mib[1], i)) +
-					 sizeof (unsigned long) * nr));
+		    *((__u64 *) (((void *) per_cpu_ptr(mib[1], i)) +
+					 sizeof (__u64) * nr));
 	}
 	return res;
 }
@@ -121,8 +121,8 @@
 			ipv4_devconf.forwarding ? 1 : 2, sysctl_ip_default_ttl);
 
 	for (i = 0;
-	     i < offsetof(struct ip_mib, __pad) / sizeof(unsigned long); i++)
-		seq_printf(seq, " %lu",
+	     i < offsetof(struct ip_mib, __pad) / sizeof(__u64); i++)
+		seq_printf(seq, " %llu",
 			   fold_field((void **) ip_statistics, i));
 
 	seq_printf(seq, "\nIcmp: InMsgs InErrors InDestUnreachs InTimeExcds "
@@ -134,8 +134,8 @@
 			"OutAddrMasks OutAddrMaskReps\nIcmp:");
 
 	for (i = 0;
-	     i < offsetof(struct icmp_mib, dummy) / sizeof(unsigned long); i++)
-		seq_printf(seq, " %lu",
+	     i < offsetof(struct icmp_mib, dummy) / sizeof(__u64); i++)
+		seq_printf(seq, " %llu",
 			   fold_field((void **) icmp_statistics, i)); 
 
 	seq_printf(seq, "\nTcp: RtoAlgorithm RtoMin RtoMax MaxConn ActiveOpens "
@@ -143,13 +143,13 @@
 			"InSegs OutSegs RetransSegs InErrs OutRsts\nTcp:");
 
 	for (i = 0;
-	     i < offsetof(struct tcp_mib, __pad) / sizeof(unsigned long); i++) {
-		if (i == (offsetof(struct tcp_mib, TcpMaxConn) / sizeof(unsigned long)))
+	     i < offsetof(struct tcp_mib, __pad) / sizeof(__u64); i++) {
+		if (i == (offsetof(struct tcp_mib, TcpMaxConn) / sizeof(__u64)))
 			/* MaxConn field is negative, RFC 2012 */
 			seq_printf(seq, " %ld", 
-				   fold_field((void **) tcp_statistics, i));
+				  (long int)fold_field((void **) tcp_statistics, i));
 		else
-			seq_printf(seq, " %lu", 
+			seq_printf(seq, " %llu", 
 				   fold_field((void **) tcp_statistics, i));
 	}
 
@@ -157,8 +157,8 @@
 			"Udp:");
 
 	for (i = 0;
-	     i < offsetof(struct udp_mib, __pad) / sizeof(unsigned long); i++)
-		seq_printf(seq, " %lu", 
+	     i < offsetof(struct udp_mib, __pad) / sizeof(__u64); i++)
+		seq_printf(seq, " %llu", 
 				fold_field((void **) udp_statistics, i));
 
 	seq_putc(seq, '\n');
@@ -214,9 +214,9 @@
 		      " TCPAbortFailed TCPMemoryPressures\n"
 		      "TcpExt:");
 	for (i = 0;
-	     i < offsetof(struct linux_mib, __pad) / sizeof(unsigned long); 
+	     i < offsetof(struct linux_mib, __pad) / sizeof(__u64); 
 	     i++)
-		seq_printf(seq, " %lu", 
+		seq_printf(seq, " %llu", 
 		 	   fold_field((void **) net_statistics, i)); 
 	seq_putc(seq, '\n');
 	return 0;
diff -urN linux-2.6.1/net/ipv6/proc.c linux-2.6.1-ipv6mib2-64/net/ipv6/proc.c
--- linux-2.6.1/net/ipv6/proc.c	2004-01-08 22:59:26.000000000 -0800
+++ linux-2.6.1-ipv6mib2-64/net/ipv6/proc.c	2004-01-12 16:21:43.000000000 -0800
@@ -60,13 +60,14 @@
 struct snmp6_item
 {
 	char *name;
+	int   size;
 	int   offset;
 };
-#define SNMP6_SENTINEL	{ .name = NULL, .offset = 0 }
+#define SNMP6_SENTINEL	{ .name = NULL, .size = 0, .offset = 0 }
 
 static struct snmp6_item snmp6_ipv6_list[] = {
 /* ipv6 mib according to RFC 2465 */
-#define SNMP6_GEN(x) { .name = #x , .offset = offsetof(struct ipv6_mib, x) }
+#define SNMP6_GEN(x) { .name = #x , .size = sizeof(((struct ipv6_mib *)0)->x), .offset = offsetof(struct ipv6_mib, x) }
 	SNMP6_GEN(Ip6InReceives),
 	SNMP6_GEN(Ip6InHdrErrors),
 	SNMP6_GEN(Ip6InTooBigErrors),
@@ -104,7 +105,7 @@
 		OutRouterAdvertisements too.
 		OutGroupMembQueries too.
  */
-#define SNMP6_GEN(x) { .name = #x , .offset = offsetof(struct icmpv6_mib, x) }
+#define SNMP6_GEN(x) { .name = #x , .size = sizeof(((struct icmpv6_mib *)0)->x), .offset = offsetof(struct icmpv6_mib, x) }
 	SNMP6_GEN(Icmp6InMsgs),
 	SNMP6_GEN(Icmp6InErrors),
 	SNMP6_GEN(Icmp6InDestUnreachs),
@@ -138,7 +139,7 @@
 };
 
 static struct snmp6_item snmp6_udp6_list[] = {
-#define SNMP6_GEN(x) { .name = "Udp6" #x , .offset = offsetof(struct udp_mib, Udp##x) }
+#define SNMP6_GEN(x) { .name = "Udp6" #x , .size = sizeof(((struct udp_mib *)0)->Udp##x), .offset = offsetof(struct udp_mib, Udp##x) }
 	SNMP6_GEN(InDatagrams),
 	SNMP6_GEN(NoPorts),
 	SNMP6_GEN(InErrors),
@@ -147,22 +148,27 @@
 	SNMP6_SENTINEL
 };
 
-static unsigned long
-fold_field(void *mib[], int offt)
+static __u64 
+fold_field(void *mib[], int size, int offt)
 {
-        unsigned long res = 0;
+        __u64 res = 0;
         int i;
  
         for (i = 0; i < NR_CPUS; i++) {
                 if (!cpu_possible(i))
                         continue;
-                res +=
-                    *((unsigned long *) (((void *)per_cpu_ptr(mib[0], i)) +
-                                         offt));
-                res +=
-                    *((unsigned long *) (((void *)per_cpu_ptr(mib[1], i)) +
-                                         offt));
-        }
+		if (size == 4) {
+			res += *((__u32 *) 
+				(((void *)per_cpu_ptr(mib[0], i)) + offt));
+			res += *((__u32 *)
+				(((void *)per_cpu_ptr(mib[1], i)) + offt));
+		} else if (size == 8) {
+			res += *((__u64 *) 
+				(((void *)per_cpu_ptr(mib[0], i)) + offt));
+			res += *((__u32 *)
+				(((void *)per_cpu_ptr(mib[1], i)) + offt));
+		}
+	}
         return res;
 }
 
@@ -170,9 +176,9 @@
 snmp6_seq_show_item(struct seq_file *seq, void **mib, struct snmp6_item *itemlist)
 {
 	int i;
-	for (i=0; itemlist[i].name; i++)
-		seq_printf(seq, "%-32s\t%lu\n", itemlist[i].name, 
-				fold_field(mib, itemlist[i].offset));
+	for (i=0; itemlist[i].name; i++) 
+		seq_printf(seq, "%-32s\t%llu\n", itemlist[i].name, 
+			fold_field(mib, itemlist[i].size, itemlist[i].offset));
 }
 
 static int snmp6_seq_show(struct seq_file *seq, void *v)
diff -urN linux-2.6.1/net/sctp/proc.c linux-2.6.1-ipv6mib2-64/net/sctp/proc.c
--- linux-2.6.1/net/sctp/proc.c	2004-01-08 22:59:26.000000000 -0800
+++ linux-2.6.1-ipv6mib2-64/net/sctp/proc.c	2004-01-13 13:57:12.000000000 -0800
@@ -64,21 +64,21 @@
 /* Return the current value of a particular entry in the mib by adding its
  * per cpu counters.
  */ 
-static unsigned long
+static __u64
 fold_field(void *mib[], int nr)
 {
-	unsigned long res = 0;
+	__u64 res = 0;
 	int i;
 
 	for (i = 0; i < NR_CPUS; i++) {
 		if (!cpu_possible(i))
 			continue;
 		res +=
-		    *((unsigned long *) (((void *) per_cpu_ptr(mib[0], i)) +
-					 sizeof (unsigned long) * nr));
+		    *((__u64 *) (((void *) per_cpu_ptr(mib[0], i)) +
+					 sizeof (__u64) * nr));
 		res +=
-		    *((unsigned long *) (((void *) per_cpu_ptr(mib[1], i)) +
-					 sizeof (unsigned long) * nr));
+		    *((__u64 *) (((void *) per_cpu_ptr(mib[1], i)) +
+					 sizeof (__u64) * nr));
 	}
 	return res;
 }
@@ -89,7 +89,7 @@
 	int i;
 
 	for (i = 0; i < sizeof(sctp_snmp_list) / sizeof(char *); i++)
-		seq_printf(seq, "%-32s\t%ld\n", sctp_snmp_list[i],
+		seq_printf(seq, "%-32s\t%llu\n", sctp_snmp_list[i],
 			   fold_field((void **)sctp_statistics, i));
 
 	return 0;

Re: [PATCH]snmp6 64-bit counter support in proc.c

[David S. Miller]

On Wed, 14 Jan 2004 14:52:51 -0800
Shirley Ma <mashirle@us.ibm.com> wrote:

> > "sizeof(unsigned long)" evaluates to 8 on 64-bit systems,
> > yet you assume it always evaluated to 4 as on 32-bit systems.
> >
> > Maybe it would be wiser to explicitly use 'u32' and 'u64' for
> > the types of the snmp counters?
> >
> > This has always been a sore area.
> 
> This is the new patch against 2.6.1 kernel.

I am personally fine with this patch, but I do believe some folks might find
it controversial (for whatever reason) to move all of these stats over to 64-bits.

So I'm going to let this sit for another day or two so people can voice any
objections they may have.