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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20170207084853.3ed6988e@xeon-e3>
Date:   Tue, 7 Feb 2017 08:48:53 -0800
From:   Stephen Hemminger <stephen@...workplumber.org>
To:     Nogah Frankel <nogahf@...lanox.com>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "roopa@...ulusnetworks.com" <roopa@...ulusnetworks.com>,
        "roszenrami@...il.com" <roszenrami@...il.com>,
        "jbenc@...hat.com" <jbenc@...hat.com>,
        "sergei.shtylyov@...entembedded.com" 
        <sergei.shtylyov@...entembedded.com>,
        Jiri Pirko <jiri@...lanox.com>,
        "Elad Raz" <eladr@...lanox.com>,
        Ido Schimmel <idosch@...lanox.com>,
        Yotam Gigi <yotamg@...lanox.com>,
        Or Gerlitz <ogerlitz@...lanox.com>
Subject: Re: [PATCH iproute2 v5 2/4] ifstat: Add extended statistics to
 ifstat

On Tue, 7 Feb 2017 12:59:11 +0000
Nogah Frankel <nogahf@...lanox.com> wrote:

> > -----Original Message-----
> > From: Stephen Hemminger [mailto:stephen@...workplumber.org]
> > Sent: Friday, February 03, 2017 8:07 PM
> > To: Nogah Frankel <nogahf@...lanox.com>
> > Cc: netdev@...r.kernel.org; roopa@...ulusnetworks.com; roszenrami@...il.com;
> > jbenc@...hat.com; sergei.shtylyov@...entembedded.com; Jiri Pirko
> > <jiri@...lanox.com>; Elad Raz <eladr@...lanox.com>; Ido Schimmel
> > <idosch@...lanox.com>; Yotam Gigi <yotamg@...lanox.com>; Or Gerlitz
> > <ogerlitz@...lanox.com>
> > Subject: Re: [PATCH iproute2 v5 2/4] ifstat: Add extended statistics to ifstat
> > 
> > On Thu, 26 Jan 2017 14:44:39 +0200
> > Nogah Frankel <nogahf@...lanox.com> 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 <nogahf@...lanox.com>
> > > Reviewed-by: Jiri Pirko <jiri@...lanox.com>  
> > 
> > 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.