| Message ID | 1409961244.26422.142.camel@edumazet-glaptop2.roam.corp.google.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | stephen hemminger |
| Headers | show |
On Fri, 05 Sep 2014 16:54:04 -0700 Eric Dumazet <eric.dumazet@gmail.com> wrote: > + if (i != TCP_METRIC_RTT && > + i != TCP_METRIC_RTT_US && > + i != TCP_METRIC_RTTVAR && > + i != TCP_METRIC_RTTVAR_US) { > + if (metric_name[i]) > + fprintf(fp, " %s ", metric_name[i]); > + else > + fprintf(fp, " metric_%d ", i); Why not put new metrics in metric_name array? and make the check something like: if (i < ARRAY_SIZE(metric_name) && metric_name[i]) fprintf(fp, " %s ", metric_name[i]); else fprintf(fp, " metric_%d ", i) This makes it future proof, and gets rid of the silly test. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2014-09-05 at 17:02 -0700, Stephen Hemminger wrote: > On Fri, 05 Sep 2014 16:54:04 -0700 > Eric Dumazet <eric.dumazet@gmail.com> wrote: > > > + if (i != TCP_METRIC_RTT && > > + i != TCP_METRIC_RTT_US && > > + i != TCP_METRIC_RTTVAR && > > + i != TCP_METRIC_RTTVAR_US) { > > + if (metric_name[i]) > > + fprintf(fp, " %s ", metric_name[i]); > > + else > > + fprintf(fp, " metric_%d ", i); > > Why not put new metrics in metric_name array? and make the check something like: > > if (i < ARRAY_SIZE(metric_name) && metric_name[i]) > fprintf(fp, " %s ", metric_name[i]); > else > fprintf(fp, " metric_%d ", i) > > This makes it future proof, and gets rid of the silly test. Because for compatibility reasons, kernel gives all values, we want to output one of them, the most accurate one. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 05 Sep 2014 16:54:04 -0700 Eric Dumazet <eric.dumazet@gmail.com> wrote: > From: Eric Dumazet <edumazet@google.com> > > Starting from linux-3.15, kernel supports new tcp metric attributes : > TCP_METRIC_RTT_US & TCP_METRIC_RTTVAR_US > > Update ip command to detect their use. > > Signed-off-by: Eric Dumazet <edumazet@google.com> Applied thanks -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/ip/tcp_metrics.c b/ip/tcp_metrics.c index e0f0344..bbbb4cc 100644 --- a/ip/tcp_metrics.c +++ b/ip/tcp_metrics.c @@ -216,6 +216,7 @@ static int process_msg(const struct sockaddr_nl *who, struct nlmsghdr *n, a = attrs[TCP_METRICS_ATTR_VALS]; if (a) { struct rtattr *m[TCP_METRIC_MAX + 1 + 1]; + unsigned long rtt = 0, rttvar = 0; parse_rtattr_nested(m, TCP_METRIC_MAX + 1, a); @@ -225,18 +226,30 @@ static int process_msg(const struct sockaddr_nl *who, struct nlmsghdr *n, a = m[i + 1]; if (!a) continue; - if (metric_name[i]) - fprintf(fp, " %s ", metric_name[i]); - else - fprintf(fp, " metric_%d ", i); - + if (i != TCP_METRIC_RTT && + i != TCP_METRIC_RTT_US && + i != TCP_METRIC_RTTVAR && + i != TCP_METRIC_RTTVAR_US) { + if (metric_name[i]) + fprintf(fp, " %s ", metric_name[i]); + else + fprintf(fp, " metric_%d ", i); + } val = rta_getattr_u32(a); switch (i) { case TCP_METRIC_RTT: - fprintf(fp, "%luus", (val * 1000UL) >> 3); + if (!rtt) + rtt = (val * 1000UL) >> 3; break; case TCP_METRIC_RTTVAR: - fprintf(fp, "%luus", (val * 1000UL) >> 2); + if (!rttvar) + rttvar = (val * 1000UL) >> 2; + break; + case TCP_METRIC_RTT_US: + rtt = val >> 3; + break; + case TCP_METRIC_RTTVAR_US: + rttvar = val >> 2; break; case TCP_METRIC_SSTHRESH: case TCP_METRIC_CWND: @@ -246,6 +259,10 @@ static int process_msg(const struct sockaddr_nl *who, struct nlmsghdr *n, break; } } + if (rtt) + fprintf(fp, " rtt %luus", rtt); + if (rttvar) + fprintf(fp, " rttvar %luus", rttvar); } a = attrs[TCP_METRICS_ATTR_FOPEN_MSS];