* [PATCH] netdev: re-expose net_device_stats to user headers
@ 2010-08-23 15:32 Stephen Hemminger
2010-08-23 15:43 ` Ben Hutchings
0 siblings, 1 reply; 9+ messages in thread
From: Stephen Hemminger @ 2010-08-23 15:32 UTC (permalink / raw)
To: David Miller; +Cc: Ben Hutchings, netdev
This commit broke iproute2 build:
commit be1f3c2c027cc5ad735df6a45a542ed1db7ec48b
Author: Ben Hutchings <bhutchings@solarflare.com>
Date: Tue Jun 8 07:19:54 2010 +0000
net: Enable 64-bit net device statistics on 32-bit architectures
Use struct rtnl_link_stats64 as the statistics structure.
Iproute2 uses a the kernel exported headers, and the structure
net_device_stats which is part of the netlink IFLA_STATS message
was no longer exposed.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
---
This patch needs to be applied to net-2.6 (2.6.36)
--- a/include/linux/netdevice.h 2010-08-23 08:22:17.801906612 -0700
+++ b/include/linux/netdevice.h 2010-08-23 08:25:23.502078638 -0700
@@ -165,6 +165,8 @@ static inline bool dev_xmit_complete(int
#define MAX_HEADER (LL_MAX_HEADER + 48)
#endif
+#endif /* __KERNEL__ */
+
/*
* Old network device statistics. Fields are native words
* (unsigned long) so they can be read and written atomically.
@@ -196,9 +198,6 @@ struct net_device_stats {
unsigned long tx_compressed;
};
-#endif /* __KERNEL__ */
-
-
/* Media selection options. */
enum {
IF_PORT_UNKNOWN = 0,
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] netdev: re-expose net_device_stats to user headers
2010-08-23 15:32 [PATCH] netdev: re-expose net_device_stats to user headers Stephen Hemminger
@ 2010-08-23 15:43 ` Ben Hutchings
2010-08-23 16:17 ` Stephen Hemminger
0 siblings, 1 reply; 9+ messages in thread
From: Ben Hutchings @ 2010-08-23 15:43 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, netdev
On Mon, 2010-08-23 at 08:32 -0700, Stephen Hemminger wrote:
> This commit broke iproute2 build:
>
> commit be1f3c2c027cc5ad735df6a45a542ed1db7ec48b
> Author: Ben Hutchings <bhutchings@solarflare.com>
> Date: Tue Jun 8 07:19:54 2010 +0000
>
> net: Enable 64-bit net device statistics on 32-bit architectures
>
> Use struct rtnl_link_stats64 as the statistics structure.
>
> Iproute2 uses a the kernel exported headers, and the structure
> net_device_stats which is part of the netlink IFLA_STATS message
> was no longer exposed.
[...]
This is not true; the kernel uses struct rtnl_link_stats for IFLA_STATS.
AFAICS the only reference to struct net_device_stats in iproute2 is:
misc/ifstat.c:51:#define MAXS (sizeof(struct net_device_stats)/sizeof(unsigned long))
This should be changed to refer to the structure that is actually used.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] netdev: re-expose net_device_stats to user headers
2010-08-23 15:43 ` Ben Hutchings
@ 2010-08-23 16:17 ` Stephen Hemminger
2010-08-23 16:54 ` [PATCH] iproute2: add 64bit support to ifstat Eric Dumazet
0 siblings, 1 reply; 9+ messages in thread
From: Stephen Hemminger @ 2010-08-23 16:17 UTC (permalink / raw)
To: Ben Hutchings; +Cc: David Miller, netdev
On Mon, 23 Aug 2010 16:43:23 +0100
Ben Hutchings <bhutchings@solarflare.com> wrote:
> On Mon, 2010-08-23 at 08:32 -0700, Stephen Hemminger wrote:
> > This commit broke iproute2 build:
> >
> > commit be1f3c2c027cc5ad735df6a45a542ed1db7ec48b
> > Author: Ben Hutchings <bhutchings@solarflare.com>
> > Date: Tue Jun 8 07:19:54 2010 +0000
> >
> > net: Enable 64-bit net device statistics on 32-bit architectures
> >
> > Use struct rtnl_link_stats64 as the statistics structure.
> >
> > Iproute2 uses a the kernel exported headers, and the structure
> > net_device_stats which is part of the netlink IFLA_STATS message
> > was no longer exposed.
> [...]
>
> This is not true; the kernel uses struct rtnl_link_stats for IFLA_STATS.
> AFAICS the only reference to struct net_device_stats in iproute2 is:
>
> misc/ifstat.c:51:#define MAXS (sizeof(struct net_device_stats)/sizeof(unsigned long))
>
> This should be changed to refer to the structure that is actually used.
Ok.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] iproute2: add 64bit support to ifstat
2010-08-23 16:17 ` Stephen Hemminger
@ 2010-08-23 16:54 ` Eric Dumazet
2010-08-23 17:33 ` Stephen Hemminger
0 siblings, 1 reply; 9+ messages in thread
From: Eric Dumazet @ 2010-08-23 16:54 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Ben Hutchings, David Miller, netdev
Le lundi 23 août 2010 à 09:17 -0700, Stephen Hemminger a écrit :
> On Mon, 23 Aug 2010 16:43:23 +0100
> Ben Hutchings <bhutchings@solarflare.com> wrote:
>
> > On Mon, 2010-08-23 at 08:32 -0700, Stephen Hemminger wrote:
> > > This commit broke iproute2 build:
> > >
> > > commit be1f3c2c027cc5ad735df6a45a542ed1db7ec48b
> > > Author: Ben Hutchings <bhutchings@solarflare.com>
> > > Date: Tue Jun 8 07:19:54 2010 +0000
> > >
> > > net: Enable 64-bit net device statistics on 32-bit architectures
> > >
> > > Use struct rtnl_link_stats64 as the statistics structure.
> > >
> > > Iproute2 uses a the kernel exported headers, and the structure
> > > net_device_stats which is part of the netlink IFLA_STATS message
> > > was no longer exposed.
> > [...]
> >
> > This is not true; the kernel uses struct rtnl_link_stats for IFLA_STATS.
> > AFAICS the only reference to struct net_device_stats in iproute2 is:
> >
> > misc/ifstat.c:51:#define MAXS (sizeof(struct net_device_stats)/sizeof(unsigned long))
> >
> > This should be changed to refer to the structure that is actually used.
>
> Ok.
Stephen,
Here is a patch to ifstat on top of your last one.
I guess nobody tried it on a 64bit platform...
Thanks
iproute2: add 64bit support to ifstat
ifstat assumes IFLA_STATS fields are "unsigned long"
ifstat can use 64bit stats if available (IFLA_STATS64)
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/misc/ifstat.c b/misc/ifstat.c
index 5cf2e01..c649ee2 100644
--- a/misc/ifstat.c
+++ b/misc/ifstat.c
@@ -49,7 +49,7 @@ int npatterns;
char info_source[128];
int source_mismatch;
-#define MAXS (sizeof(struct rtnl_link_stats)/sizeof(unsigned long))
+#define MAXS (sizeof(struct rtnl_link_stats)/sizeof(__u32))
struct ifstat_ent
{
@@ -58,7 +58,7 @@ struct ifstat_ent
int ifindex;
unsigned long long val[MAXS];
double rate[MAXS];
- unsigned long ival[MAXS];
+ __u32 ival[MAXS];
};
struct ifstat_ent *kern_db;
@@ -108,8 +108,12 @@ static int get_nlmsg(const struct sockaddr_nl *who,
n->name = strdup(RTA_DATA(tb[IFLA_IFNAME]));
memcpy(&n->ival, RTA_DATA(tb[IFLA_STATS]), sizeof(n->ival));
memset(&n->rate, 0, sizeof(n->rate));
- for (i=0; i<MAXS; i++)
- n->val[i] = n->ival[i];
+ if (tb[IFLA_STATS64])
+ memcpy(&n->val, RTA_DATA(tb[IFLA_STATS64]), sizeof(n->val));
+ else
+ for (i = 0; i < MAXS; i++)
+ n->val[i] = n->ival[i];
+
n->next = kern_db;
kern_db = n;
return 0;
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] iproute2: add 64bit support to ifstat
2010-08-23 16:54 ` [PATCH] iproute2: add 64bit support to ifstat Eric Dumazet
@ 2010-08-23 17:33 ` Stephen Hemminger
2010-08-23 19:44 ` Eric Dumazet
0 siblings, 1 reply; 9+ messages in thread
From: Stephen Hemminger @ 2010-08-23 17:33 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Ben Hutchings, David Miller, netdev
On Mon, 23 Aug 2010 18:54:24 +0200
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le lundi 23 août 2010 à 09:17 -0700, Stephen Hemminger a écrit :
> > On Mon, 23 Aug 2010 16:43:23 +0100
> > Ben Hutchings <bhutchings@solarflare.com> wrote:
> >
> > > On Mon, 2010-08-23 at 08:32 -0700, Stephen Hemminger wrote:
> > > > This commit broke iproute2 build:
> > > >
> > > > commit be1f3c2c027cc5ad735df6a45a542ed1db7ec48b
> > > > Author: Ben Hutchings <bhutchings@solarflare.com>
> > > > Date: Tue Jun 8 07:19:54 2010 +0000
> > > >
> > > > net: Enable 64-bit net device statistics on 32-bit architectures
> > > >
> > > > Use struct rtnl_link_stats64 as the statistics structure.
> > > >
> > > > Iproute2 uses a the kernel exported headers, and the structure
> > > > net_device_stats which is part of the netlink IFLA_STATS message
> > > > was no longer exposed.
> > > [...]
> > >
> > > This is not true; the kernel uses struct rtnl_link_stats for IFLA_STATS.
> > > AFAICS the only reference to struct net_device_stats in iproute2 is:
> > >
> > > misc/ifstat.c:51:#define MAXS (sizeof(struct net_device_stats)/sizeof(unsigned long))
> > >
> > > This should be changed to refer to the structure that is actually used.
> >
> > Ok.
>
> Stephen,
>
> Here is a patch to ifstat on top of your last one.
>
> I guess nobody tried it on a 64bit platform...
>
> Thanks
>
> iproute2: add 64bit support to ifstat
>
> ifstat assumes IFLA_STATS fields are "unsigned long"
>
> ifstat can use 64bit stats if available (IFLA_STATS64)
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
>
> diff --git a/misc/ifstat.c b/misc/ifstat.c
> index 5cf2e01..c649ee2 100644
> --- a/misc/ifstat.c
> +++ b/misc/ifstat.c
> @@ -49,7 +49,7 @@ int npatterns;
> char info_source[128];
> int source_mismatch;
>
> -#define MAXS (sizeof(struct rtnl_link_stats)/sizeof(unsigned long))
> +#define MAXS (sizeof(struct rtnl_link_stats)/sizeof(__u32))
>
> struct ifstat_ent
> {
> @@ -58,7 +58,7 @@ struct ifstat_ent
> int ifindex;
> unsigned long long val[MAXS];
> double rate[MAXS];
> - unsigned long ival[MAXS];
> + __u32 ival[MAXS];
> };
>
> struct ifstat_ent *kern_db;
> @@ -108,8 +108,12 @@ static int get_nlmsg(const struct sockaddr_nl *who,
> n->name = strdup(RTA_DATA(tb[IFLA_IFNAME]));
> memcpy(&n->ival, RTA_DATA(tb[IFLA_STATS]), sizeof(n->ival));
> memset(&n->rate, 0, sizeof(n->rate));
> - for (i=0; i<MAXS; i++)
> - n->val[i] = n->ival[i];
> + if (tb[IFLA_STATS64])
> + memcpy(&n->val, RTA_DATA(tb[IFLA_STATS64]), sizeof(n->val));
> + else
> + for (i = 0; i < MAXS; i++)
> + n->val[i] = n->ival[i];
> +
> n->next = kern_db;
> kern_db = n;
> return 0;
>
>
I think this breaks the wraparound detection code in this command.
--
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] iproute2: add 64bit support to ifstat
2010-08-23 17:33 ` Stephen Hemminger
@ 2010-08-23 19:44 ` Eric Dumazet
2010-08-23 20:06 ` Stephen Hemminger
0 siblings, 1 reply; 9+ messages in thread
From: Eric Dumazet @ 2010-08-23 19:44 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Ben Hutchings, David Miller, netdev
Le lundi 23 août 2010 à 10:33 -0700, Stephen Hemminger a écrit :
> I think this breaks the wraparound detection code in this command.
>
>
OK lets fix the bug only, before adding 64bit counters capabilities.
Thanks
[PATCH] iproute2: add 64bit arches support to ifstat
ifstat assumes IFLA_STATS fields are "unsigned long", but they are
__u32. This fix is needed to let ifstat run on 64bit arches.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/misc/ifstat.c b/misc/ifstat.c
index 5cf2e01..5b229e7 100644
--- a/misc/ifstat.c
+++ b/misc/ifstat.c
@@ -49,7 +49,7 @@ int npatterns;
char info_source[128];
int source_mismatch;
-#define MAXS (sizeof(struct rtnl_link_stats)/sizeof(unsigned long))
+#define MAXS (sizeof(struct rtnl_link_stats)/sizeof(__u32))
struct ifstat_ent
{
@@ -58,7 +58,7 @@ struct ifstat_ent
int ifindex;
unsigned long long val[MAXS];
double rate[MAXS];
- unsigned long ival[MAXS];
+ __u32 ival[MAXS];
};
struct ifstat_ent *kern_db;
@@ -187,7 +187,7 @@ void load_raw_table(FILE *fp)
*next++ = 0;
if (sscanf(p, "%llu", n->val+i) != 1)
abort();
- n->ival[i] = (unsigned long)n->val[i];
+ n->ival[i] = (__u32)n->val[i];
p = next;
if (!(next = strchr(p, ' ')))
abort();
@@ -563,8 +563,6 @@ static void usage(void)
" -s, --noupdate don;t update history\n"
" -t, --interval=SECS report average over the last SECS\n"
" -V, --version output version information\n"
-" -z, --zeros show entries with zero activity\n"
-" -e, --errors show errors\n"
" -z, --zeros show entries with zero activity\n");
exit(-1);
@@ -581,8 +579,6 @@ static const struct option longopts[] = {
{ "interval", 1, 0, 't' },
{ "version", 0, 0, 'V' },
{ "zeros", 0, 0, 'z' },
- { "errors", 0, 0, 'e' },
- { "zeros", 0, 0, 'z' },
{ 0 }
};
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] iproute2: add 64bit support to ifstat
2010-08-23 19:44 ` Eric Dumazet
@ 2010-08-23 20:06 ` Stephen Hemminger
2010-08-23 20:28 ` Eric Dumazet
0 siblings, 1 reply; 9+ messages in thread
From: Stephen Hemminger @ 2010-08-23 20:06 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Ben Hutchings, David Miller, netdev
> @@ -563,8 +563,6 @@ static void usage(void)
> " -s, --noupdate don;t update history\n"
> " -t, --interval=SECS report average over the last SECS\n"
> " -V, --version output version information\n"
> -" -z, --zeros show entries with zero activity\n"
> -" -e, --errors show errors\n"
> " -z, --zeros show entries with zero activity\n");
>
> exit(-1);
> @@ -581,8 +579,6 @@ static const struct option longopts[] = {
> { "interval", 1, 0, 't' },
> { "version", 0, 0, 'V' },
> { "zeros", 0, 0, 'z' },
> - { "errors", 0, 0, 'e' },
> - { "zeros", 0, 0, 'z' },
> { 0 }
> };
>
>
>
Did you mean to break --errors?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] iproute2: add 64bit support to ifstat
2010-08-23 20:06 ` Stephen Hemminger
@ 2010-08-23 20:28 ` Eric Dumazet
2010-08-23 20:39 ` Stephen Hemminger
0 siblings, 1 reply; 9+ messages in thread
From: Eric Dumazet @ 2010-08-23 20:28 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Ben Hutchings, David Miller, netdev
Le lundi 23 août 2010 à 13:06 -0700, Stephen Hemminger a écrit :
> > @@ -563,8 +563,6 @@ static void usage(void)
> > " -s, --noupdate don;t update history\n"
> > " -t, --interval=SECS report average over the last SECS\n"
> > " -V, --version output version information\n"
> > -" -z, --zeros show entries with zero activity\n"
> > -" -e, --errors show errors\n"
> > " -z, --zeros show entries with zero activity\n");
> >
> > exit(-1);
> > @@ -581,8 +579,6 @@ static const struct option longopts[] = {
> > { "interval", 1, 0, 't' },
> > { "version", 0, 0, 'V' },
> > { "zeros", 0, 0, 'z' },
> > - { "errors", 0, 0, 'e' },
> > - { "zeros", 0, 0, 'z' },
> > { 0 }
> > };
> >
> >
> >
>
> Did you mean to break --errors?
I dont think so.
These options are already handled, unless I am wrong.
Do you want a separate patch for this trivial fix ?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] iproute2: add 64bit support to ifstat
2010-08-23 20:28 ` Eric Dumazet
@ 2010-08-23 20:39 ` Stephen Hemminger
0 siblings, 0 replies; 9+ messages in thread
From: Stephen Hemminger @ 2010-08-23 20:39 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Ben Hutchings, David Miller, netdev
On Mon, 23 Aug 2010 22:28:31 +0200
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le lundi 23 août 2010 à 13:06 -0700, Stephen Hemminger a écrit :
> > > @@ -563,8 +563,6 @@ static void usage(void)
> > > " -s, --noupdate don;t update history\n"
> > > " -t, --interval=SECS report average over the last SECS\n"
> > > " -V, --version output version information\n"
> > > -" -z, --zeros show entries with zero activity\n"
> > > -" -e, --errors show errors\n"
> > > " -z, --zeros show entries with zero activity\n");
> > >
> > > exit(-1);
> > > @@ -581,8 +579,6 @@ static const struct option longopts[] = {
> > > { "interval", 1, 0, 't' },
> > > { "version", 0, 0, 'V' },
> > > { "zeros", 0, 0, 'z' },
> > > - { "errors", 0, 0, 'e' },
> > > - { "zeros", 0, 0, 'z' },
> > > { 0 }
> > > };
> > >
> > >
> > >
> >
> > Did you mean to break --errors?
>
> I dont think so.
>
> These options are already handled, unless I am wrong.
>
> Do you want a separate patch for this trivial fix ?
I'll tweak your previous patch
--
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-08-23 20:39 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-23 15:32 [PATCH] netdev: re-expose net_device_stats to user headers Stephen Hemminger
2010-08-23 15:43 ` Ben Hutchings
2010-08-23 16:17 ` Stephen Hemminger
2010-08-23 16:54 ` [PATCH] iproute2: add 64bit support to ifstat Eric Dumazet
2010-08-23 17:33 ` Stephen Hemminger
2010-08-23 19:44 ` Eric Dumazet
2010-08-23 20:06 ` Stephen Hemminger
2010-08-23 20:28 ` Eric Dumazet
2010-08-23 20:39 ` Stephen Hemminger
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.