netdev - Re: [PATCH iproute2] stats output

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] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 30 Nov 2018 10:26:41 -0800
From:   Roopa Prabhu <roopa@...ulusnetworks.com>
To:     Stephen Hemminger <stephen@...workplumber.org>
Cc:     avachette@...zer.com, netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH iproute2] stats output

On Fri, Nov 30, 2018 at 10:12 AM Stephen Hemminger
<stephen@...workplumber.org> wrote:
>
> On Fri, 30 Nov 2018 14:33:49 +0100
> Alexis Vachette <avachette@...zer.com> wrote:
>
> > When using:
> >
> > - ip -s link
> >
> > I think it should be better to print errors stats without adding -s
> > option twice.
> >
> > This option print stats for each network interfaces and we want to see
> > if something is off and especially timers with errors.
> >
> > Man page of ip command is updated accordingly.
> >
> > Here is a patchset:
> >
> > Signed-off-by: Alexis Vachette <avachette@...zer.com>
> > ---
> > diff --git a/ip/ipaddress.c b/ip/ipaddress.c
> > index 85f05a2..c70394d 100644
> > --- a/ip/ipaddress.c
> > +++ b/ip/ipaddress.c
> > @@ -323,7 +323,7 @@ int print_linkinfo(const struct sockaddr_nl *who,
> >   fprintf(fp,"\n    alias %s",
> >   (const char *) RTA_DATA(tb[IFLA_IFALIAS]));
> >
> > - if (do_link && tb[IFLA_STATS64] && show_stats) {
> > + if (do_link && tb[IFLA_STATS64]) {
> >   struct rtnl_link_stats64 slocal;
> >   struct rtnl_link_stats64 *s = RTA_DATA(tb[IFLA_STATS64]);
> >   if (((unsigned long)s) & (sizeof(unsigned long)-1)) {
> > @@ -343,16 +343,14 @@ int print_linkinfo(const struct sockaddr_nl *who,
> >   if (s->rx_compressed)
> >   fprintf(fp, " %-7llu",
> >   (unsigned long long)s->rx_compressed);
> > - if (show_stats > 1) {
> > - fprintf(fp, "%s", _SL_);
> > - fprintf(fp, "    RX errors: length  crc     frame   fifo    missed%s", _SL_);
> > - fprintf(fp, "               %-7llu  %-7llu %-7llu %-7llu %-7llu",
> > - (unsigned long long)s->rx_length_errors,
> > - (unsigned long long)s->rx_crc_errors,
> > - (unsigned long long)s->rx_frame_errors,
> > - (unsigned long long)s->rx_fifo_errors,
> > - (unsigned long long)s->rx_missed_errors);
> > - }
> > + fprintf(fp, "%s", _SL_);
> > + fprintf(fp, "    RX errors: length  crc     frame   fifo    missed%s", _SL_);
> > + fprintf(fp, "               %-7llu  %-7llu %-7llu %-7llu %-7llu",
> > + (unsigned long long)s->rx_length_errors,
> > + (unsigned long long)s->rx_crc_errors,
> > + (unsigned long long)s->rx_frame_errors,
> > + (unsigned long long)s->rx_fifo_errors,
> > + (unsigned long long)s->rx_missed_errors);
> >   fprintf(fp, "%s", _SL_);
> >   fprintf(fp, "    TX: bytes  packets  errors  dropped carrier collsns %s%s",
> >   s->tx_compressed ? "compressed" : "", _SL_);
> > @@ -366,17 +364,15 @@ int print_linkinfo(const struct sockaddr_nl *who,
> >   if (s->tx_compressed)
> >   fprintf(fp, " %-7llu",
> >   (unsigned long long)s->tx_compressed);
> > - if (show_stats > 1) {
> > - fprintf(fp, "%s", _SL_);
> > - fprintf(fp, "    TX errors: aborted fifo    window  heartbeat%s", _SL_);
> > - fprintf(fp, "               %-7llu  %-7llu %-7llu %-7llu",
> > - (unsigned long long)s->tx_aborted_errors,
> > - (unsigned long long)s->tx_fifo_errors,
> > - (unsigned long long)s->tx_window_errors,
> > - (unsigned long long)s->tx_heartbeat_errors);
> > - }
> > + fprintf(fp, "%s", _SL_);
> > + fprintf(fp, "    TX errors: aborted fifo    window  heartbeat%s", _SL_);
> > + fprintf(fp, "               %-7llu  %-7llu %-7llu %-7llu",
> > + (unsigned long long)s->tx_aborted_errors,
> > + (unsigned long long)s->tx_fifo_errors,
> > + (unsigned long long)s->tx_window_errors,
> > + (unsigned long long)s->tx_heartbeat_errors);
> >   }
> > - if (do_link && !tb[IFLA_STATS64] && tb[IFLA_STATS] && show_stats) {
> > + if (do_link && !tb[IFLA_STATS64] && tb[IFLA_STATS]) {
> >   struct rtnl_link_stats slocal;
> >   struct rtnl_link_stats *s = RTA_DATA(tb[IFLA_STATS]);
> >   if (((unsigned long)s) & (sizeof(unsigned long)-1)) {
> > @@ -393,17 +389,15 @@ int print_linkinfo(const struct sockaddr_nl *who,
> >   );
> >   if (s->rx_compressed)
> >   fprintf(fp, " %-7u", s->rx_compressed);
> > - if (show_stats > 1) {
> > - fprintf(fp, "%s", _SL_);
> > - fprintf(fp, "    RX errors: length  crc     frame   fifo    missed%s", _SL_);
> > - fprintf(fp, "               %-7u  %-7u %-7u %-7u %-7u",
> > - s->rx_length_errors,
> > - s->rx_crc_errors,
> > - s->rx_frame_errors,
> > - s->rx_fifo_errors,
> > - s->rx_missed_errors
> > - );
> > - }
> > + fprintf(fp, "%s", _SL_);
> > + fprintf(fp, "    RX errors: length  crc     frame   fifo    missed%s", _SL_);
> > + fprintf(fp, "               %-7u  %-7u %-7u %-7u %-7u",
> > + s->rx_length_errors,
> > + s->rx_crc_errors,
> > + s->rx_frame_errors,
> > + s->rx_fifo_errors,
> > + s->rx_missed_errors
> > + );
> >   fprintf(fp, "%s", _SL_);
> >   fprintf(fp, "    TX: bytes  packets  errors  dropped carrier collsns %s%s",
> >   s->tx_compressed ? "compressed" : "", _SL_);
> > @@ -412,16 +406,14 @@ int print_linkinfo(const struct sockaddr_nl *who,
> >   s->tx_dropped, s->tx_carrier_errors, s->collisions);
> >   if (s->tx_compressed)
> >   fprintf(fp, " %-7u", s->tx_compressed);
> > - if (show_stats > 1) {
> > - fprintf(fp, "%s", _SL_);
> > - fprintf(fp, "    TX errors: aborted fifo    window  heartbeat%s", _SL_);
> > - fprintf(fp, "               %-7u  %-7u %-7u %-7u",
> > - s->tx_aborted_errors,
> > - s->tx_fifo_errors,
> > - s->tx_window_errors,
> > - s->tx_heartbeat_errors
> > - );
> > - }
> > + fprintf(fp, "%s", _SL_);
> > + fprintf(fp, "    TX errors: aborted fifo    window  heartbeat%s", _SL_);
> > + fprintf(fp, "               %-7u  %-7u %-7u %-7u",
> > + s->tx_aborted_errors,
> > + s->tx_fifo_errors,
> > + s->tx_window_errors,
> > + s->tx_heartbeat_errors
> > + );
> >   }
> >   if (do_link && tb[IFLA_VFINFO_LIST] && tb[IFLA_NUM_VF]) {
> >   struct rtattr *i, *vflist = tb[IFLA_VFINFO_LIST];
> > diff --git a/man/man8/ip.8 b/man/man8/ip.8
> > index 36431b6..6843f0a 100644
> > --- a/man/man8/ip.8
> > +++ b/man/man8/ip.8
> > @@ -831,8 +831,7 @@ utility and exit.
> >
> >  .TP
> >  .BR "\-s" , " \-stats", " \-statistics"
> > -output more information.  If the option
> > -appears twice or more, the amount of information increases.
> > +output more information.
> >  As a rule, the information is statistics or some time values.
> >
> >  .TP
> > ---
>
> I can understand why you would want this, but it is changing the
> behavior of an existing command that might be used in scripts.
>

+1

> Also, your patch was mangled by your mail client. It no longer has
> proper indentation and whitespace.