* [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.