Re: [PATCH] iproute2: add 64bit support to ifstat

[Stephen Hemminger <shemminger@vyatta.com>]

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.


--