[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAP962rREvFEjKTVfVxNDAjAej8UkARCNQBji6UTr3_ujDjzTJQ@mail.gmail.com>
Date: Fri, 30 Nov 2018 20:22:35 +0100
From: Alexis Vachette <avachette@...zer.com>
To: Stephen Hemminger <stephen@...workplumber.org>
Cc: netdev@...r.kernel.org
Subject: Re: [PATCH iproute2] stats output
Hi Stephen,
Thanks for your kind reply.
It's sad to hear that, I am aware of your concern too.
But it's not the best behavior for a network engineer.
Is it possible to add a new option or everything is stone graved ?
If yes I will fix my e-mail client to submit a correct PR.
On Fri, 30 Nov 2018 at 19:12, 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.
>
> Also, your patch was mangled by your mail client. It no longer has
> proper indentation and whitespace.
Powered by blists - more mailing lists