Subject: Re: [PATCH iproute2 v5 2/4] ifstat: Add extended statistics to ifstat

From: Stephen Hemminger <[email-masked]>
Date: Tue, 7 Feb 2017 08:48:53 -0800

On Tue, 7 Feb 2017 12:59:11 +0000
Nogah Frankel <[email-masked]> wrote:

> > -----Original Message-----
> > From: Stephen Hemminger [mailto:[email-masked]]
> > Sent: Friday, February 03, 2017 8:07 PM
> > To: Nogah Frankel <[email-masked]>
> > Cc: [email-masked]; [email-masked]; [email-masked];
> > [email-masked]; [email-masked]; Jiri Pirko
> > <[email-masked]>; Elad Raz <[email-masked]>; Ido Schimmel
> > <[email-masked]>; Yotam Gigi <[email-masked]>; Or Gerlitz
> > <[email-masked]>
> > Subject: Re: [PATCH iproute2 v5 2/4] ifstat: Add extended statistics to ifstat
> > 
> > On Thu, 26 Jan 2017 14:44:39 +0200
> > Nogah Frankel <[email-masked]> wrote:
> >   
> > > Extended stats are part of the RTM_GETSTATS method. This patch adds them
> > > to ifstat.
> > > While extended stats can come in many forms, we support only the
> > > rtnl_link_stats64 struct for them (which is the 64 bits version of struct
> > > rtnl_link_stats).
> > > We support stats in the main nesting level, or one lower.
> > > The extension can be called by its name or any shorten of it. If there is
> > > more than one matched, the first one will be picked.
> > >
> > > To get the extended stats the flag -x <stats type> is used.
> > >
> > > Signed-off-by: Nogah Frankel <[email-masked]>
> > > Reviewed-by: Jiri Pirko <[email-masked]>  
> > 
> > Sorry I was confused because RTM_GETSTATS contains multiple statistics.
> > Your patch is about getting LINK_XSTATS and after looking in more detail, you are
> > correct this should be an option. Although it would make sense to show this as addition
> > to the basic statistics. And when I tested it no output happens which seems confusing.
> > 
> > $ ./misc/ifstat -p -x cpu_hits
> > #kernel
> > Interface        RX Pkts/Rate    TX Pkts/Rate    RX Data/Rate    TX Data/Rate
> >                  RX Errs/Drop    TX Errs/Drop    RX Over/Rate    TX Coll/Rate
> >   
> 
> Not all devices support this xstat.
> Do you prefer another print in this case?
> 
> About printing both the xstat and the default stats together, it may be problematic since 
> ifstat is about diffs. I think it is better that one ifstat call for a specific stats, won't change
> the other stats data. (And since we are talking about diffs, reading data meaning changing
> it).

Maybe an error would be a better experience for  user.
I merged the current version but try and think about how to make it work better in general.