netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: Alexis Vachette <avachette@deezer.com>
Cc: netdev@vger.kernel.org
Subject: Re: [PATCH iproute2] stats output
Date: Fri, 30 Nov 2018 10:12:02 -0800	[thread overview]
Message-ID: <20181130101202.38c4bd56@xeon-e3> (raw)
In-Reply-To: <CAP962rTMX-Pe_j5wW7TNr_jcxGgxT8Ru9AxUWzpU5PS9YvSQtw@mail.gmail.com>

On Fri, 30 Nov 2018 14:33:49 +0100
Alexis Vachette <avachette@deezer.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@deezer.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.

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

  reply	other threads:[~2018-12-01  5:22 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-30 13:33 [PATCH iproute2] stats output Alexis Vachette
2018-11-30 18:12 ` Stephen Hemminger [this message]
2018-11-30 18:26   ` Roopa Prabhu
2018-11-30 18:31   ` David Ahern
2018-11-30 19:22   ` Alexis Vachette
2018-11-30 19:55     ` Stephen Hemminger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181130101202.38c4bd56@xeon-e3 \
    --to=stephen@networkplumber.org \
    --cc=avachette@deezer.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).