netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH iproute2 0/3] Unify output for inet sockets
@ 2015-01-18 20:43 Vadim Kochan
  2015-01-18 20:43 ` [PATCH iproute2 1/3] ss: Make meminfo look little bit more readable Vadim Kochan
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Vadim Kochan @ 2015-01-18 20:43 UTC (permalink / raw)
  To: netdev; +Cc: Vadim Kochan

This is series is about to have output information from /proc and
Netlink in one place.

Some short summary:

    1) Make memory info more readable.
    2) Unify output of inet sockets from /proc and Netlink.
    3) Added new '-H, --human' option to show info in more human format
        which is used for memory info meanwhile.

Vadim Kochan (3):
  ss: Make meminfo look little bit more readable
  ss: Unify inet sockets output
  ss: Unify tcp stats output

 include/utils.h |   3 +
 ip/ipaddress.c  |  31 +--
 lib/utils.c     |  40 ++-
 misc/ss.c       | 737 ++++++++++++++++++++++++++++++--------------------------
 4 files changed, 437 insertions(+), 374 deletions(-)

-- 
2.1.3

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH iproute2 1/3] ss: Make meminfo look little bit more readable
  2015-01-18 20:43 [PATCH iproute2 0/3] Unify output for inet sockets Vadim Kochan
@ 2015-01-18 20:43 ` Vadim Kochan
  2015-01-18 20:43 ` [PATCH iproute2 2/3] ss: Unify inet sockets output Vadim Kochan
  2015-01-18 20:43 ` [PATCH iproute2 3/3] ss: Unify tcp stats output Vadim Kochan
  2 siblings, 0 replies; 19+ messages in thread
From: Vadim Kochan @ 2015-01-18 20:43 UTC (permalink / raw)
  To: netdev; +Cc: Vadim Kochan

From: Vadim Kochan <vadim4j@gmail.com>

Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
 include/utils.h |  3 +++
 ip/ipaddress.c  | 31 ++--------------------
 lib/utils.c     | 40 +++++++++++++++++++++++++++-
 misc/ss.c       | 82 ++++++++++++++++++++++++++++++++++-----------------------
 4 files changed, 93 insertions(+), 63 deletions(-)

diff --git a/include/utils.h b/include/utils.h
index e1fe7cf..6b0b76c 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -5,6 +5,7 @@
 #include <asm/types.h>
 #include <resolv.h>
 #include <stdlib.h>
+#include <stdbool.h>
 
 #include "libnetlink.h"
 #include "ll_map.h"
@@ -162,4 +163,6 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 		char **name, char **type, char **link, char **dev,
 		int *group, int *index);
 
+char *sprint_num(char *str, uint64_t num, bool use_iec);
+
 #endif /* __UTILS_H__ */
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index d5e863d..5a2a956 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -362,41 +362,14 @@ static void print_vfinfo(FILE *fp, struct rtattr *vfinfo)
 
 static void print_num(FILE *fp, unsigned width, uint64_t count)
 {
-	const char *prefix = "kMGTPE";
-	const unsigned int base = use_iec ? 1024 : 1000;
-	uint64_t powi = 1;
-	uint16_t powj = 1;
-	uint8_t precision = 2;
 	char buf[64];
 
-	if (!human_readable || count < base) {
+	if (!human_readable) {
 		fprintf(fp, "%-*"PRIu64" ", width, count);
 		return;
 	}
 
-	/* increase value by a factor of 1000/1024 and print
-	 * if result is something a human can read */
-	for(;;) {
-		powi *= base;
-		if (count / base < powi)
-			break;
-
-		if (!prefix[1])
-			break;
-		++prefix;
-	}
-
-	/* try to guess a good number of digits for precision */
-	for (; precision > 0; precision--) {
-		powj *= 10;
-		if (count / powi < powj)
-			break;
-	}
-
-	snprintf(buf, sizeof(buf), "%.*f%c%s", precision,
-		(double) count / powi, *prefix, use_iec ? "i" : "");
-
-	fprintf(fp, "%-*s ", width, buf);
+	fprintf(fp, "%-*s ", width, sprint_num(buf, count, use_iec));
 }
 
 static void print_link_stats64(FILE *fp, const struct rtnl_link_stats64 *s,
diff --git a/lib/utils.c b/lib/utils.c
index f65ceaa..21e2e78 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -28,7 +28,7 @@
 #include <time.h>
 #include <sys/time.h>
 #include <errno.h>
-
+#include <inttypes.h>
 
 #include "utils.h"
 
@@ -878,3 +878,41 @@ void print_nlmsg_timestamp(FILE *fp, const struct nlmsghdr *n)
 	tstr[strlen(tstr)-1] = 0;
 	fprintf(fp, "Timestamp: %s %lu us\n", tstr, usecs);
 }
+
+char *sprint_num(char *str, uint64_t num, bool use_iec)
+{
+	const char *prefix = "kMGTPE";
+	const unsigned int base = use_iec ? 1024 : 1000;
+	uint64_t powi = 1;
+	uint16_t powj = 1;
+	uint8_t precision = 2;
+
+	if (num < base) {
+		sprintf(str, "%"PRIu64, num);
+		return str;
+	}
+
+	/* increase value by a factor of 1000/1024 and print
+	 * if result is something a human can read */
+	for(;;) {
+		powi *= base;
+		if (num / base < powi)
+			break;
+
+		if (!prefix[1])
+			break;
+		++prefix;
+	}
+
+	/* try to guess a good number of digits for precision */
+	for (; precision > 0; precision--) {
+		powj *= 10;
+		if (num / powi < powj)
+			break;
+	}
+
+	sprintf(str, "%.*f%c%s", precision,
+		(double) num / powi, *prefix, use_iec ? "i" : "");
+
+	return str;
+}
diff --git a/misc/ss.c b/misc/ss.c
index f434f57..c5995ab 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -26,6 +26,7 @@
 #include <fnmatch.h>
 #include <getopt.h>
 #include <stdbool.h>
+#include <inttypes.h>
 
 #include "utils.h"
 #include "rt_names.h"
@@ -96,6 +97,7 @@ int show_tcpinfo = 0;
 int show_bpf = 0;
 int show_proc_ctx = 0;
 int show_sock_ctx = 0;
+bool show_human = 0;
 /* If show_users & show_proc_ctx only do user_ent_hash_build() once */
 int user_ent_hash_build_init = 0;
 
@@ -1598,37 +1600,56 @@ outerr:
 	return ferror(fp) ? -1 : 0;
 }
 
-static char *sprint_bw(char *buf, double bw)
+static char *sprint_bandw(char *buf, double bw)
 {
-	if (bw > 1000000.)
-		sprintf(buf,"%.1fM", bw / 1000000.);
-	else if (bw > 1000.)
-		sprintf(buf,"%.1fK", bw / 1000.);
-	else
-		sprintf(buf, "%g", bw);
+	return sprint_num(buf, bw, false);
+}
 
-	return buf;
+static char *sprint_bytes(char *buf, uint64_t bytes)
+{
+	if (!show_human) {
+		sprintf(buf, "%"PRIu64, bytes);
+		return buf;
+	}
+
+	return sprint_num(buf, bytes, false);
 }
 
 static void print_skmeminfo(struct rtattr *tb[], int attrtype)
 {
+	char buf[64];
 	const __u32 *skmeminfo;
-	if (!tb[attrtype])
+
+	if (!tb[attrtype]) {
+		if (attrtype == INET_DIAG_SKMEMINFO) {
+			if (!tb[INET_DIAG_MEMINFO])
+				return;
+
+			const struct inet_diag_meminfo *minfo =
+				RTA_DATA(tb[INET_DIAG_MEMINFO]);
+
+			printf(" mem:(rd=%s,", sprint_bytes(buf, minfo->idiag_rmem));
+			printf("wr=%s,", sprint_bytes(buf, minfo->idiag_tmem));
+			printf("fwd=%s,", sprint_bytes(buf, minfo->idiag_fmem));
+			printf("wr_queue=%s)", sprint_bytes(buf, minfo->idiag_wmem));
+		}
 		return;
+	}
+
 	skmeminfo = RTA_DATA(tb[attrtype]);
 
-	printf(" skmem:(r%u,rb%u,t%u,tb%u,f%u,w%u,o%u",
-	       skmeminfo[SK_MEMINFO_RMEM_ALLOC],
-	       skmeminfo[SK_MEMINFO_RCVBUF],
-	       skmeminfo[SK_MEMINFO_WMEM_ALLOC],
-	       skmeminfo[SK_MEMINFO_SNDBUF],
-	       skmeminfo[SK_MEMINFO_FWD_ALLOC],
-	       skmeminfo[SK_MEMINFO_WMEM_QUEUED],
-	       skmeminfo[SK_MEMINFO_OPTMEM]);
+	printf(" skmem:(");
+	printf("rd=%s,", sprint_bytes(buf, skmeminfo[SK_MEMINFO_RMEM_ALLOC]));
+	printf("rcv_buf=%s,", sprint_bytes(buf, skmeminfo[SK_MEMINFO_RCVBUF]));
+	printf("wr=%s,", sprint_bytes(buf, skmeminfo[SK_MEMINFO_WMEM_ALLOC]));
+	printf("snd_buf=%s,", sprint_bytes(buf, skmeminfo[SK_MEMINFO_SNDBUF]));
+	printf("fwd=%s,", sprint_bytes(buf, skmeminfo[SK_MEMINFO_FWD_ALLOC]));
+	printf("wr_queue=%s,", sprint_bytes(buf, skmeminfo[SK_MEMINFO_WMEM_QUEUED]));
+	printf("oth=%s", sprint_bytes(buf, skmeminfo[SK_MEMINFO_OPTMEM]));
 
 	if (RTA_PAYLOAD(tb[attrtype]) >=
 		(SK_MEMINFO_BACKLOG + 1) * sizeof(__u32))
-		printf(",bl%u", skmeminfo[SK_MEMINFO_BACKLOG]);
+		printf(",bklog=%s", sprint_bytes(buf, skmeminfo[SK_MEMINFO_BACKLOG]));
 
 	printf(")");
 }
@@ -1639,17 +1660,7 @@ static void tcp_show_info(const struct nlmsghdr *nlh, struct inet_diag_msg *r,
 	char b1[64];
 	double rtt = 0;
 
-	if (tb[INET_DIAG_SKMEMINFO]) {
-		print_skmeminfo(tb, INET_DIAG_SKMEMINFO);
-	} else if (tb[INET_DIAG_MEMINFO]) {
-		const struct inet_diag_meminfo *minfo
-			= RTA_DATA(tb[INET_DIAG_MEMINFO]);
-		printf(" mem:(r%u,w%u,f%u,t%u)",
-		       minfo->idiag_rmem,
-		       minfo->idiag_wmem,
-		       minfo->idiag_fmem,
-		       minfo->idiag_tmem);
-	}
+	print_skmeminfo(tb, INET_DIAG_SKMEMINFO);
 
 	if (tb[INET_DIAG_INFO]) {
 		struct tcp_info *info;
@@ -1723,7 +1734,7 @@ static void tcp_show_info(const struct nlmsghdr *nlh, struct inet_diag_msg *r,
 
 		if (rtt > 0 && info->tcpi_snd_mss && info->tcpi_snd_cwnd) {
 			printf(" send %sbps",
-			       sprint_bw(b1, (double) info->tcpi_snd_cwnd *
+			       sprint_bandw(b1, (double) info->tcpi_snd_cwnd *
 					 (double) info->tcpi_snd_mss * 8000000.
 					 / rtt));
 		}
@@ -1740,12 +1751,12 @@ static void tcp_show_info(const struct nlmsghdr *nlh, struct inet_diag_msg *r,
 		if (info->tcpi_pacing_rate &&
 		    info->tcpi_pacing_rate != ~0ULL) {
 			printf(" pacing_rate %sbps",
-				sprint_bw(b1, info->tcpi_pacing_rate * 8.));
+				sprint_bandw(b1, info->tcpi_pacing_rate * 8.));
 
 			if (info->tcpi_max_pacing_rate &&
 			    info->tcpi_max_pacing_rate != ~0ULL)
 				printf("/%sbps",
-					sprint_bw(b1, info->tcpi_max_pacing_rate * 8.));
+					sprint_bandw(b1, info->tcpi_max_pacing_rate * 8.));
 		}
 		if (info->tcpi_unacked)
 			printf(" unacked:%u", info->tcpi_unacked);
@@ -3207,6 +3218,7 @@ static void _usage(FILE *dest)
 "   -b, --bpf           show bpf filter socket information\n"
 "   -Z, --context       display process SELinux security contexts\n"
 "   -z, --contexts      display process and socket SELinux security contexts\n"
+"   -H, --human         display info in human readable format\n"
 "\n"
 "   -4, --ipv4          display only IP version 4 sockets\n"
 "   -6, --ipv6          display only IP version 6 sockets\n"
@@ -3306,6 +3318,7 @@ static const struct option long_opts[] = {
 	{ "help", 0, 0, 'h' },
 	{ "context", 0, 0, 'Z' },
 	{ "contexts", 0, 0, 'z' },
+	{ "human", 0, 0, 'H' },
 	{ 0 }
 
 };
@@ -3321,7 +3334,7 @@ int main(int argc, char *argv[])
 	struct filter dbs_filter = {};
 	int state_filter = 0;
 
-	while ((ch = getopt_long(argc, argv, "dhaletuwxnro460spbf:miA:D:F:vVzZ",
+	while ((ch = getopt_long(argc, argv, "dhaletuwxnro460spbf:miA:D:F:vVzZH",
 				 long_opts, NULL)) != EOF) {
 		switch(ch) {
 		case 'n':
@@ -3493,6 +3506,9 @@ int main(int argc, char *argv[])
 			show_proc_ctx++;
 			user_ent_hash_build();
 			break;
+		case 'H':
+			show_human = true;
+			break;
 		case 'h':
 		case '?':
 			help();
-- 
2.1.3

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH iproute2 2/3] ss: Unify inet sockets output
  2015-01-18 20:43 [PATCH iproute2 0/3] Unify output for inet sockets Vadim Kochan
  2015-01-18 20:43 ` [PATCH iproute2 1/3] ss: Make meminfo look little bit more readable Vadim Kochan
@ 2015-01-18 20:43 ` Vadim Kochan
  2015-01-18 20:43 ` [PATCH iproute2 3/3] ss: Unify tcp stats output Vadim Kochan
  2 siblings, 0 replies; 19+ messages in thread
From: Vadim Kochan @ 2015-01-18 20:43 UTC (permalink / raw)
  To: netdev; +Cc: Vadim Kochan

From: Vadim Kochan <vadim4j@gmail.com>

Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
 misc/ss.c | 355 +++++++++++++++++++++++++-------------------------------------
 1 file changed, 144 insertions(+), 211 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index c5995ab..40439b3 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -708,6 +708,7 @@ struct tcpstat
 	int		refcnt;
 	unsigned long long sk;
 	int		rto, ato, qack, cwnd, ssthresh;
+	unsigned int	iface;
 };
 
 static const char *tmr_name[] = {
@@ -746,12 +747,6 @@ static const char *print_ms_timer(int timeout)
 	return buf;
 }
 
-static const char *print_hz_timer(int timeout)
-{
-	int hz = get_user_hz();
-	return print_ms_timer(((timeout*1000) + hz-1)/hz);
-}
-
 struct scache
 {
 	struct scache *next;
@@ -1441,55 +1436,124 @@ out:
 	return res;
 }
 
-static int tcp_show_line(char *line, const struct filter *f, int family)
+static char *proto_name(int protocol)
+{
+	switch (protocol) {
+	case IPPROTO_UDP:
+		return "udp";
+	case IPPROTO_TCP:
+		return "tcp";
+	case IPPROTO_DCCP:
+		return "dccp";
+	}
+
+	return "???";
+}
+
+static void inet_stats_print(struct tcpstat *s, int protocol)
+{
+	char *buf = NULL;
+
+	if (netid_width)
+		printf("%-*s ", netid_width, proto_name(protocol));
+	if (state_width)
+		printf("%-*s ", state_width, sstate_name[s->state]);
+
+	printf("%-6d %-6d ", s->rq, s->wq);
+
+	formatted_print(&s->local, s->lport, s->iface);
+	formatted_print(&s->remote, s->rport, 0);
+
+	if (show_options) {
+		if (s->timer) {
+			if (s->timer > 4)
+				s->timer = 5;
+			printf(" timer:(%s,%s,%d)",
+			       tmr_name[s->timer],
+			       print_ms_timer(s->timeout),
+			       s->retrs);
+		}
+	}
+
+	if (show_proc_ctx || show_sock_ctx) {
+		if (find_entry(s->ino, &buf,
+				(show_proc_ctx & show_sock_ctx) ?
+				PROC_SOCK_CTX : PROC_CTX) > 0) {
+			printf(" users:(%s)", buf);
+			free(buf);
+		}
+	} else if (show_users) {
+		if (find_entry(s->ino, &buf, USERS) > 0) {
+			printf(" users:(%s)", buf);
+			free(buf);
+		}
+	}
+}
+
+static int proc_parse_inet_addr(char *loc, char *rem, int family, struct tcpstat *s)
+{
+	s->local.family = s->remote.family = family;
+	if (family == AF_INET) {
+		sscanf(loc, "%x:%x", s->local.data, (unsigned*)&s->lport);
+		sscanf(rem, "%x:%x", s->remote.data, (unsigned*)&s->rport);
+		s->local.bytelen = s->remote.bytelen = 4;
+		return 0;
+	} else {
+		sscanf(loc, "%08x%08x%08x%08x:%x",
+		       s->local.data,
+		       s->local.data + 1,
+		       s->local.data + 2,
+		       s->local.data + 3,
+		       &s->lport);
+		sscanf(rem, "%08x%08x%08x%08x:%x",
+		       s->remote.data,
+		       s->remote.data + 1,
+		       s->remote.data + 2,
+		       s->remote.data + 3,
+		       &s->rport);
+		s->local.bytelen = s->remote.bytelen = 16;
+		return 0;
+	}
+	return -1;
+}
+
+static int proc_inet_split_line(char *line, char **loc, char **rem, char **data)
 {
-	struct tcpstat s;
-	char *loc, *rem, *data;
-	char opt[256];
-	int n;
 	char *p;
 
 	if ((p = strchr(line, ':')) == NULL)
 		return -1;
-	loc = p+2;
 
-	if ((p = strchr(loc, ':')) == NULL)
+	*loc = p+2;
+	if ((p = strchr(*loc, ':')) == NULL)
 		return -1;
-	p[5] = 0;
-	rem = p+6;
 
-	if ((p = strchr(rem, ':')) == NULL)
+	p[5] = 0;
+	*rem = p+6;
+	if ((p = strchr(*rem, ':')) == NULL)
 		return -1;
+
 	p[5] = 0;
-	data = p+6;
+	*data = p+6;
+	return 0;
+}
 
-	do {
-		int state = (data[1] >= 'A') ? (data[1] - 'A' + 10) : (data[1] - '0');
+static int tcp_show_line(char *line, const struct filter *f, int family)
+{
+	struct tcpstat s = {};
+	char *loc, *rem, *data;
+	char opt[256];
+	int n;
+	int hz = get_user_hz();
 
-		if (!(f->states & (1<<state)))
-			return 0;
-	} while (0);
+	if (proc_inet_split_line(line, &loc, &rem, &data))
+		return -1;
 
-	s.local.family = s.remote.family = family;
-	if (family == AF_INET) {
-		sscanf(loc, "%x:%x", s.local.data, (unsigned*)&s.lport);
-		sscanf(rem, "%x:%x", s.remote.data, (unsigned*)&s.rport);
-		s.local.bytelen = s.remote.bytelen = 4;
-	} else {
-		sscanf(loc, "%08x%08x%08x%08x:%x",
-		       s.local.data,
-		       s.local.data+1,
-		       s.local.data+2,
-		       s.local.data+3,
-		       &s.lport);
-		sscanf(rem, "%08x%08x%08x%08x:%x",
-		       s.remote.data,
-		       s.remote.data+1,
-		       s.remote.data+2,
-		       s.remote.data+3,
-		       &s.rport);
-		s.local.bytelen = s.remote.bytelen = 16;
-	}
+	int state = (data[1] >= 'A') ? (data[1] - 'A' + 10) : (data[1] - '0');
+	if (!(f->states & (1 << state)))
+		return 0;
+
+	proc_parse_inet_addr(loc, rem, family, &s);
 
 	if (f->f && run_ssfilter(f->f, &s) == 0)
 		return 0;
@@ -1511,66 +1575,36 @@ static int tcp_show_line(char *line, const struct filter *f, int family)
 		s.ato = s.qack = 0;
 	}
 
-	if (netid_width)
-		printf("%-*s ", netid_width, "tcp");
-	if (state_width)
-		printf("%-*s ", state_width, sstate_name[s.state]);
-
-	printf("%-6d %-6d ", s.rq, s.wq);
+	s.retrs = s.timer != 1 ? s.probes : s.retrs;
+	s.timeout = (s.timeout * 1000 + hz - 1) / hz;
 
-	formatted_print(&s.local, s.lport, 0);
-	formatted_print(&s.remote, s.rport, 0);
+	inet_stats_print(&s, IPPROTO_TCP);
 
-	if (show_options) {
-		if (s.timer) {
-			if (s.timer > 4)
-				s.timer = 5;
-			printf(" timer:(%s,%s,%d)",
-			       tmr_name[s.timer],
-			       print_hz_timer(s.timeout),
-			       s.timer != 1 ? s.probes : s.retrs);
-		}
+	if (show_details) {
+		if (s.uid)
+			printf(" uid:%u", (unsigned)s.uid);
+		printf(" ino:%u", s.ino);
+		printf(" sk:%llx", s.sk);
+		if (opt[0])
+			printf(" opt:\"%s\"", opt);
 	}
+
 	if (show_tcpinfo) {
-		int hz = get_user_hz();
-		if (s.rto && s.rto != 3*hz)
-			printf(" rto:%g", (double)s.rto/hz);
+		if (s.rto && s.rto != 3 * hz)
+			printf(" rto:%g", (double)s.rto / hz);
 		if (s.ato)
-			printf(" ato:%g", (double)s.ato/hz);
+			printf(" ato:%g", (double)s.ato / hz);
 		if (s.cwnd != 2)
 			printf(" cwnd:%d", s.cwnd);
 		if (s.ssthresh != -1)
 			printf(" ssthresh:%d", s.ssthresh);
-		if (s.qack/2)
-			printf(" qack:%d", s.qack/2);
-		if (s.qack&1)
+		if (s.qack / 2)
+			printf(" qack:%d", s.qack / 2);
+		if (s.qack & 1)
 			printf(" bidir");
 	}
-	char *buf = NULL;
-	if (show_proc_ctx || show_sock_ctx) {
-		if (find_entry(s.ino, &buf,
-				(show_proc_ctx & show_sock_ctx) ?
-				PROC_SOCK_CTX : PROC_CTX) > 0) {
-			printf(" users:(%s)", buf);
-			free(buf);
-		}
-	} else if (show_users) {
-		if (find_entry(s.ino, &buf, USERS) > 0) {
-			printf(" users:(%s)", buf);
-			free(buf);
-		}
-	}
 
-	if (show_details) {
-		if (s.uid)
-			printf(" uid:%u", (unsigned)s.uid);
-		printf(" ino:%u", s.ino);
-		printf(" sk:%llx", s.sk);
-		if (opt[0])
-			printf(" opt:\"%s\"", opt);
-	}
 	printf("\n");
-
 	return 0;
 }
 
@@ -1779,25 +1813,11 @@ static void tcp_show_info(const struct nlmsghdr *nlh, struct inet_diag_msg *r,
 	}
 }
 
-static char *proto_name(int protocol)
-{
-	switch (protocol) {
-	case IPPROTO_UDP:
-		return "udp";
-	case IPPROTO_TCP:
-		return "tcp";
-	case IPPROTO_DCCP:
-		return "dccp";
-	}
-
-	return "???";
-}
-
 static int inet_show_sock(struct nlmsghdr *nlh, struct filter *f, int protocol)
 {
 	struct rtattr * tb[INET_DIAG_MAX+1];
 	struct inet_diag_msg *r = NLMSG_DATA(nlh);
-	struct tcpstat s;
+	struct tcpstat s = {};
 
 	parse_rtattr(tb, INET_DIAG_MAX, (struct rtattr*)(r+1),
 		     nlh->nlmsg_len - NLMSG_LENGTH(sizeof(*r)));
@@ -1806,52 +1826,28 @@ static int inet_show_sock(struct nlmsghdr *nlh, struct filter *f, int protocol)
 	s.local.family = s.remote.family = r->idiag_family;
 	s.lport = ntohs(r->id.idiag_sport);
 	s.rport = ntohs(r->id.idiag_dport);
+	s.wq = r->idiag_wqueue;
+	s.rq = r->idiag_rqueue;
+	s.timer = r->idiag_timer;
+	s.timeout = r->idiag_expires;
+	s.retrs = r->idiag_retrans;
+	s.ino = r->idiag_inode;
+	s.uid = r->idiag_uid;
+	s.iface = r->id.idiag_if;
+
 	if (s.local.family == AF_INET) {
 		s.local.bytelen = s.remote.bytelen = 4;
 	} else {
 		s.local.bytelen = s.remote.bytelen = 16;
 	}
+
 	memcpy(s.local.data, r->id.idiag_src, s.local.bytelen);
 	memcpy(s.remote.data, r->id.idiag_dst, s.local.bytelen);
 
 	if (f && f->f && run_ssfilter(f->f, &s) == 0)
 		return 0;
 
-	if (netid_width)
-		printf("%-*s ", netid_width, proto_name(protocol));
-	if (state_width)
-		printf("%-*s ", state_width, sstate_name[s.state]);
-
-	printf("%-6d %-6d ", r->idiag_rqueue, r->idiag_wqueue);
-
-	formatted_print(&s.local, s.lport, r->id.idiag_if);
-	formatted_print(&s.remote, s.rport, 0);
-
-	if (show_options) {
-		if (r->idiag_timer) {
-			if (r->idiag_timer > 4)
-				r->idiag_timer = 5;
-			printf(" timer:(%s,%s,%d)",
-			       tmr_name[r->idiag_timer],
-			       print_ms_timer(r->idiag_expires),
-			       r->idiag_retrans);
-		}
-	}
-	char *buf = NULL;
-
-	if (show_proc_ctx || show_sock_ctx) {
-		if (find_entry(r->idiag_inode, &buf,
-				(show_proc_ctx & show_sock_ctx) ?
-				PROC_SOCK_CTX : PROC_CTX) > 0) {
-			printf(" users:(%s)", buf);
-			free(buf);
-		}
-	} else if (show_users) {
-		if (find_entry(r->idiag_inode, &buf, USERS) > 0) {
-			printf(" users:(%s)", buf);
-			free(buf);
-		}
-	}
+	inet_stats_print(&s, protocol);
 
 	if (show_details) {
 		if (r->idiag_uid)
@@ -1867,13 +1863,13 @@ static int inet_show_sock(struct nlmsghdr *nlh, struct filter *f, int protocol)
 			printf(" %c-%c", mask & 1 ? '-' : '<', mask & 2 ? '-' : '>');
 		}
 	}
+
 	if (show_mem || show_tcpinfo) {
 		printf("\n\t");
 		tcp_show_info(nlh, r, tb);
 	}
 
 	printf("\n");
-
 	return 0;
 }
 
@@ -2194,53 +2190,19 @@ outerr:
 
 static int dgram_show_line(char *line, const struct filter *f, int family)
 {
-	struct tcpstat s;
+	struct tcpstat s = {};
 	char *loc, *rem, *data;
 	char opt[256];
 	int n;
-	char *p;
-
-	if ((p = strchr(line, ':')) == NULL)
-		return -1;
-	loc = p+2;
-
-	if ((p = strchr(loc, ':')) == NULL)
-		return -1;
-	p[5] = 0;
-	rem = p+6;
 
-	if ((p = strchr(rem, ':')) == NULL)
+	if (proc_inet_split_line(line, &loc, &rem, &data))
 		return -1;
-	p[5] = 0;
-	data = p+6;
-
-	do {
-		int state = (data[1] >= 'A') ? (data[1] - 'A' + 10) : (data[1] - '0');
 
-		if (!(f->states & (1<<state)))
-			return 0;
-	} while (0);
+	int state = (data[1] >= 'A') ? (data[1] - 'A' + 10) : (data[1] - '0');
+	if (!(f->states & (1 << state)))
+		return 0;
 
-	s.local.family = s.remote.family = family;
-	if (family == AF_INET) {
-		sscanf(loc, "%x:%x", s.local.data, (unsigned*)&s.lport);
-		sscanf(rem, "%x:%x", s.remote.data, (unsigned*)&s.rport);
-		s.local.bytelen = s.remote.bytelen = 4;
-	} else {
-		sscanf(loc, "%08x%08x%08x%08x:%x",
-		       s.local.data,
-		       s.local.data+1,
-		       s.local.data+2,
-		       s.local.data+3,
-		       &s.lport);
-		sscanf(rem, "%08x%08x%08x%08x:%x",
-		       s.remote.data,
-		       s.remote.data+1,
-		       s.remote.data+2,
-		       s.remote.data+3,
-		       &s.rport);
-		s.local.bytelen = s.remote.bytelen = 16;
-	}
+	proc_parse_inet_addr(loc, rem, family, &s);
 
 	if (f->f && run_ssfilter(f->f, &s) == 0)
 		return 0;
@@ -2254,31 +2216,7 @@ static int dgram_show_line(char *line, const struct filter *f, int family)
 	if (n < 9)
 		opt[0] = 0;
 
-	if (netid_width)
-		printf("%-*s ", netid_width, dg_proto);
-	if (state_width)
-		printf("%-*s ", state_width, sstate_name[s.state]);
-
-	printf("%-6d %-6d ", s.rq, s.wq);
-
-	formatted_print(&s.local, s.lport, 0);
-	formatted_print(&s.remote, s.rport, 0);
-
-	char *buf = NULL;
-
-	if (show_proc_ctx || show_sock_ctx) {
-		if (find_entry(s.ino, &buf,
-				(show_proc_ctx & show_sock_ctx) ?
-				PROC_SOCK_CTX : PROC_CTX) > 0) {
-			printf(" users:(%s)", buf);
-			free(buf);
-		}
-	} else if (show_users) {
-		if (find_entry(s.ino, &buf, USERS) > 0) {
-			printf(" users:(%s)", buf);
-			free(buf);
-		}
-	}
+	inet_stats_print(&s, IPPROTO_UDP);
 
 	if (show_details) {
 		if (s.uid)
@@ -2288,12 +2226,11 @@ static int dgram_show_line(char *line, const struct filter *f, int family)
 		if (opt[0])
 			printf(" opt:\"%s\"", opt);
 	}
-	printf("\n");
 
+	printf("\n");
 	return 0;
 }
 
-
 static int udp_show(struct filter *f)
 {
 	FILE *fp = NULL;
@@ -2362,7 +2299,6 @@ outerr:
 	} while (0);
 }
 
-
 struct unixstat
 {
 	struct unixstat *next;
@@ -2376,12 +2312,9 @@ struct unixstat
 	char *name;
 };
 
-
-
 int unix_state_map[] = { SS_CLOSE, SS_SYN_SENT,
 			 SS_ESTABLISHED, SS_CLOSING };
 
-
 #define MAX_UNIX_REMEMBER (1024*1024/sizeof(struct unixstat))
 
 static void unix_list_free(struct unixstat *list)
-- 
2.1.3

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH iproute2 3/3] ss: Unify tcp stats output
  2015-01-18 20:43 [PATCH iproute2 0/3] Unify output for inet sockets Vadim Kochan
  2015-01-18 20:43 ` [PATCH iproute2 1/3] ss: Make meminfo look little bit more readable Vadim Kochan
  2015-01-18 20:43 ` [PATCH iproute2 2/3] ss: Unify inet sockets output Vadim Kochan
@ 2015-01-18 20:43 ` Vadim Kochan
  2015-01-19 13:57   ` Hagen Paul Pfeifer
  2015-01-20 10:29   ` Hagen Paul Pfeifer
  2 siblings, 2 replies; 19+ messages in thread
From: Vadim Kochan @ 2015-01-18 20:43 UTC (permalink / raw)
  To: netdev; +Cc: Vadim Kochan

From: Vadim Kochan <vadim4j@gmail.com>

Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
 misc/ss.c | 362 +++++++++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 231 insertions(+), 131 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 40439b3..73097b2 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -691,24 +691,59 @@ static const char *sstate_namel[] = {
 	[SS_CLOSING] = "closing",
 };
 
+struct dctcpstat
+{
+	unsigned int	ce_state;
+	unsigned int	alpha;
+	unsigned int	ab_ecn;
+	unsigned int	ab_tot;
+	bool		enabled;
+};
+
 struct tcpstat
 {
-	inet_prefix	local;
-	inet_prefix	remote;
-	int		lport;
-	int		rport;
-	int		state;
-	int		rq, wq;
-	int		timer;
-	int		timeout;
-	int		retrs;
-	unsigned	ino;
-	int		probes;
-	unsigned	uid;
-	int		refcnt;
-	unsigned long long sk;
-	int		rto, ato, qack, cwnd, ssthresh;
-	unsigned int	iface;
+	inet_prefix	    local;
+	inet_prefix	    remote;
+	int		    lport;
+	int		    rport;
+	int		    state;
+	int		    rq, wq;
+	unsigned	    ino;
+	unsigned	    uid;
+	int		    refcnt;
+	unsigned int	    iface;
+	unsigned long long  sk;
+	int		    timer;
+	int		    timeout;
+	int		    probes;
+	char		    *cong_alg;
+	double		    rto, ato, rtt, rttvar;
+	int		    qack, cwnd, ssthresh, backoff;
+	double		    send_bps;
+	int		    snd_wscale;
+	int		    rcv_wscale;
+	int		    mss;
+	unsigned int	    lastsnd;
+	unsigned int	    lastrcv;
+	unsigned int	    lastack;
+	double		    pacing_rate;
+	double		    pacing_rate_max;
+	unsigned int	    unacked;
+	unsigned int	    retrans;
+	unsigned int	    retrans_total;
+	unsigned int	    lost;
+	unsigned int	    sacked;
+	unsigned int	    fackets;
+	unsigned int	    reordering;
+	double		    rcv_rtt;
+	int		    rcv_space;
+	bool		    has_ts_opt;
+	bool		    has_sack_opt;
+	bool		    has_ecn_opt;
+	bool		    has_ecnseen_opt;
+	bool		    has_fastopen_opt;
+	bool		    has_wscale_opt;
+	struct dctcpstat    *dctcp;
 };
 
 static const char *tmr_name[] = {
@@ -1471,7 +1506,7 @@ static void inet_stats_print(struct tcpstat *s, int protocol)
 			printf(" timer:(%s,%s,%d)",
 			       tmr_name[s->timer],
 			       print_ms_timer(s->timeout),
-			       s->retrs);
+			       s->retrans);
 		}
 	}
 
@@ -1538,8 +1573,107 @@ static int proc_inet_split_line(char *line, char **loc, char **rem, char **data)
 	return 0;
 }
 
+static char *sprint_bandw(char *buf, double bw)
+{
+	return sprint_num(buf, bw, false);
+}
+
+static char *sprint_bytes(char *buf, uint64_t bytes)
+{
+	if (!show_human) {
+		sprintf(buf, "%"PRIu64, bytes);
+		return buf;
+	}
+
+	return sprint_num(buf, bytes, false);
+}
+
+static void tcp_stats_print(struct tcpstat *s)
+{
+	char b1[64];
+
+	if (s->has_ts_opt)
+		printf(" ts");
+	if (s->has_sack_opt)
+		printf(" sack");
+	if (s->has_ecn_opt)
+		printf(" ecn");
+	if (s->has_ecnseen_opt)
+		printf(" ecnseen");
+	if (s->has_fastopen_opt)
+		printf(" fastopen");
+	if (s->cong_alg)
+		printf(" %s", s->cong_alg);
+	if (s->has_wscale_opt)
+		printf(" wscale:%d,%d", s->snd_wscale, s->rcv_wscale);
+	if (s->rto)
+		printf(" rto:%g", s->rto);
+	if (s->backoff)
+		printf(" backoff:%u", s->backoff);
+	if (s->rtt)
+		printf(" rtt:%g/%g", s->rtt, s->rttvar);
+	if (s->ato)
+		printf(" ato:%g", s->ato);
+
+	if (s->qack)
+		printf(" qack:%d", s->qack);
+	if (s->qack & 1)
+		printf(" bidir");
+
+	if (s->mss)
+		printf(" mss:%d", s->mss);
+	if (s->cwnd && s->cwnd != 2)
+		printf(" cwnd:%d", s->cwnd);
+	if (s->ssthresh)
+		printf(" ssthresh:%d", s->ssthresh);
+
+	if (s->dctcp && s->dctcp->enabled) {
+		struct dctcpstat *dctcp = s->dctcp;
+
+		printf(" ce_state %u alpha %u ab_ecn %u ab_tot %u",
+				dctcp->ce_state, dctcp->alpha, dctcp->ab_ecn,
+				dctcp->ab_tot);
+	} else if (s->dctcp) {
+		printf(" fallback_mode");
+	}
+
+	if (s->send_bps)
+		printf(" send %sbps", sprint_bandw(b1, s->send_bps));
+	if (s->lastsnd)
+		printf(" lastsnd:%u", s->lastsnd);
+	if (s->lastrcv)
+		printf(" lastrcv:%u", s->lastrcv);
+	if (s->lastack)
+		printf(" lastack:%u", s->lastack);
+
+	if (s->pacing_rate) {
+		printf(" pacing_rate %sbps", sprint_bandw(b1, s->pacing_rate));
+		if (s->pacing_rate_max)
+				printf("/%sbps", sprint_bandw(b1,
+							s->pacing_rate_max));
+	}
+
+	if (s->unacked)
+		printf(" unacked:%u", s->unacked);
+	if (s->retrans || s->retrans_total)
+		printf(" retrans:%u/%u", s->retrans, s->retrans_total);
+	if (s->lost)
+		printf(" lost:%u", s->lost);
+	if (s->sacked && s->state != SS_LISTEN)
+		printf(" sacked:%u", s->sacked);
+	if (s->fackets)
+		printf(" fackets:%u", s->fackets);
+	if (s->reordering != 3)
+		printf(" reordering:%d", s->reordering);
+	if (s->rcv_rtt)
+		printf(" rcv_rtt:%g", s->rcv_rtt);
+	if (s->rcv_space)
+		printf(" rcv_space:%d", s->rcv_space);
+}
+
 static int tcp_show_line(char *line, const struct filter *f, int family)
 {
+	int rto = 0, ato = 0;
 	struct tcpstat s = {};
 	char *loc, *rem, *data;
 	char opt[256];
@@ -1561,22 +1695,27 @@ static int tcp_show_line(char *line, const struct filter *f, int family)
 	opt[0] = 0;
 	n = sscanf(data, "%x %x:%x %x:%x %x %d %d %u %d %llx %d %d %d %d %d %[^\n]\n",
 		   &s.state, &s.wq, &s.rq,
-		   &s.timer, &s.timeout, &s.retrs, &s.uid, &s.probes, &s.ino,
-		   &s.refcnt, &s.sk, &s.rto, &s.ato, &s.qack,
+		   &s.timer, &s.timeout, &s.retrans, &s.uid, &s.probes, &s.ino,
+		   &s.refcnt, &s.sk, &rto, &ato, &s.qack,
 		   &s.cwnd, &s.ssthresh, opt);
 
 	if (n < 17)
 		opt[0] = 0;
 
 	if (n < 12) {
-		s.rto = 0;
+		rto = 0;
 		s.cwnd = 2;
 		s.ssthresh = -1;
-		s.ato = s.qack = 0;
+		ato = s.qack = 0;
 	}
 
-	s.retrs = s.timer != 1 ? s.probes : s.retrs;
-	s.timeout = (s.timeout * 1000 + hz - 1) / hz;
+	s.retrans   = s.timer != 1 ? s.probes : s.retrans;
+	s.timeout   = (s.timeout * 1000 + hz - 1) / hz;
+	s.ato	    = (double)ato / hz;
+	s.qack	   /= 2;
+	s.rto	    = (double)rto;
+	s.ssthresh  = s.ssthresh == -1 ? 0 : s.ssthresh;
+	s.rto	    = s.rto != 3 * hz  ? s.rto / hz : 0;
 
 	inet_stats_print(&s, IPPROTO_TCP);
 
@@ -1589,20 +1728,8 @@ static int tcp_show_line(char *line, const struct filter *f, int family)
 			printf(" opt:\"%s\"", opt);
 	}
 
-	if (show_tcpinfo) {
-		if (s.rto && s.rto != 3 * hz)
-			printf(" rto:%g", (double)s.rto / hz);
-		if (s.ato)
-			printf(" ato:%g", (double)s.ato / hz);
-		if (s.cwnd != 2)
-			printf(" cwnd:%d", s.cwnd);
-		if (s.ssthresh != -1)
-			printf(" ssthresh:%d", s.ssthresh);
-		if (s.qack / 2)
-			printf(" qack:%d", s.qack / 2);
-		if (s.qack & 1)
-			printf(" bidir");
-	}
+	if (show_tcpinfo)
+		tcp_stats_print(&s);
 
 	printf("\n");
 	return 0;
@@ -1634,21 +1761,6 @@ outerr:
 	return ferror(fp) ? -1 : 0;
 }
 
-static char *sprint_bandw(char *buf, double bw)
-{
-	return sprint_num(buf, bw, false);
-}
-
-static char *sprint_bytes(char *buf, uint64_t bytes)
-{
-	if (!show_human) {
-		sprintf(buf, "%"PRIu64, bytes);
-		return buf;
-	}
-
-	return sprint_num(buf, bytes, false);
-}
-
 static void print_skmeminfo(struct rtattr *tb[], int attrtype)
 {
 	char buf[64];
@@ -1688,11 +1800,13 @@ static void print_skmeminfo(struct rtattr *tb[], int attrtype)
 	printf(")");
 }
 
+#define TCPI_HAS_OPT(info, opt) !!(info->tcpi_options & (opt))
+
 static void tcp_show_info(const struct nlmsghdr *nlh, struct inet_diag_msg *r,
 		struct rtattr *tb[])
 {
-	char b1[64];
 	double rtt = 0;
+	struct tcpstat s = {};
 
 	print_skmeminfo(tb, INET_DIAG_SKMEMINFO);
 
@@ -1709,39 +1823,49 @@ static void tcp_show_info(const struct nlmsghdr *nlh, struct inet_diag_msg *r,
 			info = RTA_DATA(tb[INET_DIAG_INFO]);
 
 		if (show_options) {
-			if (info->tcpi_options & TCPI_OPT_TIMESTAMPS)
-				printf(" ts");
-			if (info->tcpi_options & TCPI_OPT_SACK)
-				printf(" sack");
-			if (info->tcpi_options & TCPI_OPT_ECN)
-				printf(" ecn");
-			if (info->tcpi_options & TCPI_OPT_ECN_SEEN)
-				printf(" ecnseen");
-			if (info->tcpi_options & TCPI_OPT_SYN_DATA)
-				printf(" fastopen");
-		}
-
-		if (tb[INET_DIAG_CONG])
-			printf(" %s", rta_getattr_str(tb[INET_DIAG_CONG]));
-
-		if (info->tcpi_options & TCPI_OPT_WSCALE)
-			printf(" wscale:%d,%d", info->tcpi_snd_wscale,
-			       info->tcpi_rcv_wscale);
+			s.has_ts_opt	   = TCPI_HAS_OPT(info, TCPI_OPT_TIMESTAMPS);
+			s.has_sack_opt	   = TCPI_HAS_OPT(info, TCPI_OPT_SACK);
+			s.has_ecn_opt	   = TCPI_HAS_OPT(info, TCPI_OPT_ECN);
+			s.has_ecnseen_opt  = TCPI_HAS_OPT(info, TCPI_OPT_ECN_SEEN);
+			s.has_fastopen_opt = TCPI_HAS_OPT(info, TCPI_OPT_SYN_DATA);
+		}
+
+		if (tb[INET_DIAG_CONG]) {
+			const char *cong_attr = rta_getattr_str(tb[INET_DIAG_CONG]);
+			s.cong_alg = malloc(strlen(cong_attr + 1));
+			strcpy(s.cong_alg, cong_attr);
+		}
+
+		if (TCPI_HAS_OPT(info, TCPI_OPT_WSCALE)) {
+			s.has_wscale_opt  = true;
+			s.snd_wscale	  = info->tcpi_snd_wscale;
+			s.rcv_wscale	  = info->tcpi_rcv_wscale;
+		}
+
 		if (info->tcpi_rto && info->tcpi_rto != 3000000)
-			printf(" rto:%g", (double)info->tcpi_rto/1000);
-		if (info->tcpi_backoff)
-			printf(" backoff:%u", info->tcpi_backoff);
-		if (info->tcpi_rtt)
-			printf(" rtt:%g/%g", (double)info->tcpi_rtt/1000,
-			       (double)info->tcpi_rttvar/1000);
-		if (info->tcpi_ato)
-			printf(" ato:%g", (double)info->tcpi_ato/1000);
-		if (info->tcpi_snd_mss)
-			printf(" mss:%d", info->tcpi_snd_mss);
-		if (info->tcpi_snd_cwnd != 2)
-			printf(" cwnd:%d", info->tcpi_snd_cwnd);
+			s.rto = (double)info->tcpi_rto / 1000;
+
+		s.backoff	 = info->tcpi_backoff;
+		s.rtt		 = (double)info->tcpi_rtt / 1000;
+		s.rttvar	 = (double)info->tcpi_rttvar / 1000;
+		s.ato		 = (double)info->tcpi_rttvar / 1000;
+		s.mss		 = info->tcpi_snd_mss;
+		s.rcv_space	 = info->tcpi_rcv_space;
+		s.rcv_rtt	 = (double)info->tcpi_rcv_rtt / 1000;
+		s.lastsnd	 = info->tcpi_last_data_sent;
+		s.lastrcv	 = info->tcpi_last_data_recv;
+		s.lastack	 = info->tcpi_last_ack_recv;
+		s.unacked	 = info->tcpi_unacked;
+		s.retrans	 = info->tcpi_retrans;
+		s.retrans_total  = info->tcpi_total_retrans;
+		s.lost		 = info->tcpi_lost;
+		s.sacked	 = info->tcpi_sacked;
+		s.reordering	 = info->tcpi_reordering;
+		s.rcv_space	 = info->tcpi_rcv_space;
+		s.cwnd		 = info->tcpi_snd_cwnd;
+
 		if (info->tcpi_snd_ssthresh < 0xFFFF)
-			printf(" ssthresh:%d", info->tcpi_snd_ssthresh);
+			s.ssthresh = info->tcpi_snd_ssthresh;
 
 		rtt = (double) info->tcpi_rtt;
 		if (tb[INET_DIAG_VEGASINFO]) {
@@ -1749,67 +1873,43 @@ static void tcp_show_info(const struct nlmsghdr *nlh, struct inet_diag_msg *r,
 				= RTA_DATA(tb[INET_DIAG_VEGASINFO]);
 
 			if (vinfo->tcpv_enabled &&
-			    vinfo->tcpv_rtt && vinfo->tcpv_rtt != 0x7fffffff)
+					vinfo->tcpv_rtt && vinfo->tcpv_rtt != 0x7fffffff)
 				rtt =  vinfo->tcpv_rtt;
 		}
 
 		if (tb[INET_DIAG_DCTCPINFO]) {
+			struct dctcpstat *dctcp = malloc(sizeof(struct
+						dctcpstat));
+
 			const struct tcp_dctcp_info *dinfo
 				= RTA_DATA(tb[INET_DIAG_DCTCPINFO]);
 
-			if (dinfo->dctcp_enabled) {
-				printf(" ce_state %u alpha %u ab_ecn %u ab_tot %u",
-				       dinfo->dctcp_ce_state, dinfo->dctcp_alpha,
-				       dinfo->dctcp_ab_ecn, dinfo->dctcp_ab_tot);
-			} else {
-				printf(" fallback_mode");
-			}
+			dctcp->enabled	= !!dinfo->dctcp_enabled;
+			dctcp->ce_state = dinfo->dctcp_ce_state;
+			dctcp->alpha	= dinfo->dctcp_alpha;
+			dctcp->ab_ecn	= dinfo->dctcp_ab_ecn;
+			dctcp->ab_tot	= dinfo->dctcp_ab_tot;
+			s.dctcp		= dctcp;
 		}
 
 		if (rtt > 0 && info->tcpi_snd_mss && info->tcpi_snd_cwnd) {
-			printf(" send %sbps",
-			       sprint_bandw(b1, (double) info->tcpi_snd_cwnd *
-					 (double) info->tcpi_snd_mss * 8000000.
-					 / rtt));
+			s.send_bps = (double) info->tcpi_snd_cwnd *
+				(double)info->tcpi_snd_mss * 8000000. / rtt;
 		}
 
-		if (info->tcpi_last_data_sent)
-			printf(" lastsnd:%u", info->tcpi_last_data_sent);
-
-		if (info->tcpi_last_data_recv)
-			printf(" lastrcv:%u", info->tcpi_last_data_recv);
-
-		if (info->tcpi_last_ack_recv)
-			printf(" lastack:%u", info->tcpi_last_ack_recv);
-
 		if (info->tcpi_pacing_rate &&
-		    info->tcpi_pacing_rate != ~0ULL) {
-			printf(" pacing_rate %sbps",
-				sprint_bandw(b1, info->tcpi_pacing_rate * 8.));
+				info->tcpi_pacing_rate != ~0ULL) {
+			s.pacing_rate = info->tcpi_pacing_rate * 8.;
 
 			if (info->tcpi_max_pacing_rate &&
-			    info->tcpi_max_pacing_rate != ~0ULL)
-				printf("/%sbps",
-					sprint_bandw(b1, info->tcpi_max_pacing_rate * 8.));
-		}
-		if (info->tcpi_unacked)
-			printf(" unacked:%u", info->tcpi_unacked);
-		if (info->tcpi_retrans || info->tcpi_total_retrans)
-			printf(" retrans:%u/%u", info->tcpi_retrans,
-			       info->tcpi_total_retrans);
-		if (info->tcpi_lost)
-			printf(" lost:%u", info->tcpi_lost);
-		if (info->tcpi_sacked && r->idiag_state != SS_LISTEN)
-			printf(" sacked:%u", info->tcpi_sacked);
-		if (info->tcpi_fackets)
-			printf(" fackets:%u", info->tcpi_fackets);
-		if (info->tcpi_reordering != 3)
-			printf(" reordering:%d", info->tcpi_reordering);
-		if (info->tcpi_rcv_rtt)
-			printf(" rcv_rtt:%g", (double) info->tcpi_rcv_rtt/1000);
-		if (info->tcpi_rcv_space)
-			printf(" rcv_space:%d", info->tcpi_rcv_space);
-
+					info->tcpi_max_pacing_rate != ~0ULL)
+				s.pacing_rate_max = info->tcpi_max_pacing_rate * 8.;
+		}
+		tcp_stats_print(&s);
+		if (s.dctcp)
+			free(s.dctcp);
+		if (s.cong_alg)
+			free(s.cong_alg);
 	}
 }
 
@@ -1830,7 +1930,7 @@ static int inet_show_sock(struct nlmsghdr *nlh, struct filter *f, int protocol)
 	s.rq = r->idiag_rqueue;
 	s.timer = r->idiag_timer;
 	s.timeout = r->idiag_expires;
-	s.retrs = r->idiag_retrans;
+	s.retrans = r->idiag_retrans;
 	s.ino = r->idiag_inode;
 	s.uid = r->idiag_uid;
 	s.iface = r->id.idiag_if;
-- 
2.1.3

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH iproute2 3/3] ss: Unify tcp stats output
  2015-01-18 20:43 ` [PATCH iproute2 3/3] ss: Unify tcp stats output Vadim Kochan
@ 2015-01-19 13:57   ` Hagen Paul Pfeifer
  2015-01-19 14:04     ` Vadim Kochan
  2015-01-20 10:29   ` Hagen Paul Pfeifer
  1 sibling, 1 reply; 19+ messages in thread
From: Hagen Paul Pfeifer @ 2015-01-19 13:57 UTC (permalink / raw)
  To: Vadim Kochan; +Cc: netdev

On 18 January 2015 at 21:43, Vadim Kochan <vadim4j@gmail.com> wrote:

> Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
> ---
>  misc/ss.c | 362 +++++++++++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 231 insertions(+), 131 deletions(-)

Hey Vadin,

your patch do *not* change the output format, right? ss output is
parsed by scripts and tools.

hgn

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH iproute2 3/3] ss: Unify tcp stats output
  2015-01-19 13:57   ` Hagen Paul Pfeifer
@ 2015-01-19 14:04     ` Vadim Kochan
  2015-01-19 14:28       ` Hagen Paul Pfeifer
  0 siblings, 1 reply; 19+ messages in thread
From: Vadim Kochan @ 2015-01-19 14:04 UTC (permalink / raw)
  To: Hagen Paul Pfeifer; +Cc: Vadim Kochan, netdev

On Mon, Jan 19, 2015 at 02:57:50PM +0100, Hagen Paul Pfeifer wrote:
> On 18 January 2015 at 21:43, Vadim Kochan <vadim4j@gmail.com> wrote:
> 
> > Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
> > ---
> >  misc/ss.c | 362 +++++++++++++++++++++++++++++++++++++++-----------------------
> >  1 file changed, 231 insertions(+), 131 deletions(-)
> 
> Hey Vadin,
> 
> your patch do *not* change the output format, right? ss output is
> parsed by scripts and tools.
> 
> hgn

Hi,

It should not for tcp but does for memory info in the 1st patch where
the memeinfo param names were changed.

Regarding parsing of ss by scripts and tools, thats really painful
for me to see how ss outputs additional info, it is not human readable,
actually I'd like to change the output layout in the future, and add
something like online output option '-O'.

May be you can test these patches if they breaks output parsing ?

Anyway I will give up with ss output changes if we really carrying about
to keep the same output for scripts/tools.

Here are some comments from Stephen:
    http://marc.info/?l=linux-netdev&m=142129033800881&w=2

Regards,

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH iproute2 3/3] ss: Unify tcp stats output
  2015-01-19 14:28       ` Hagen Paul Pfeifer
@ 2015-01-19 14:28         ` Vadim Kochan
  2015-01-19 15:01           ` Hagen Paul Pfeifer
  2015-01-19 14:48         ` Daniel Borkmann
  1 sibling, 1 reply; 19+ messages in thread
From: Vadim Kochan @ 2015-01-19 14:28 UTC (permalink / raw)
  To: Hagen Paul Pfeifer; +Cc: Vadim Kochan, netdev

On Mon, Jan 19, 2015 at 03:28:21PM +0100, Hagen Paul Pfeifer wrote:
> Hey Vadin,
> 
> to make this short. We already discussed about changing the layout and
> Stephen nacked this. My proposal was to key:value the output. Because
> nearly all outputed data is already in this format - except the
> congestion control algo, ts, sack and tx'ed data. Where I proposed
> cc:<algo>. The key:value format has the advantages that the ordering
> do not mather anymore, An python parser would be something like split
> for whitespaces and later split for colon. Currently parsing this is a
> mess, see [1].
> 
> Anyway, the more clever idea is to add an json outputer like already
> supported by some ss modules and get rid of this mess.
> 
> Hagen
> 
> 
> [1] https://github.com/hgn/captcp/blob/master/captcp.py#L4861

Seems these patches will break your script at least for memory info
output.

I am thinking may be 1st of all it is better to make output in json
format and after this trying to make changes with human readability.

What do you think ?

Regards,
Vadim Kochan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH iproute2 3/3] ss: Unify tcp stats output
  2015-01-19 14:04     ` Vadim Kochan
@ 2015-01-19 14:28       ` Hagen Paul Pfeifer
  2015-01-19 14:28         ` Vadim Kochan
  2015-01-19 14:48         ` Daniel Borkmann
  0 siblings, 2 replies; 19+ messages in thread
From: Hagen Paul Pfeifer @ 2015-01-19 14:28 UTC (permalink / raw)
  To: Vadim Kochan; +Cc: netdev

Hey Vadin,

to make this short. We already discussed about changing the layout and
Stephen nacked this. My proposal was to key:value the output. Because
nearly all outputed data is already in this format - except the
congestion control algo, ts, sack and tx'ed data. Where I proposed
cc:<algo>. The key:value format has the advantages that the ordering
do not mather anymore, An python parser would be something like split
for whitespaces and later split for colon. Currently parsing this is a
mess, see [1].

Anyway, the more clever idea is to add an json outputer like already
supported by some ss modules and get rid of this mess.

Hagen


[1] https://github.com/hgn/captcp/blob/master/captcp.py#L4861

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH iproute2 3/3] ss: Unify tcp stats output
  2015-01-19 14:28       ` Hagen Paul Pfeifer
  2015-01-19 14:28         ` Vadim Kochan
@ 2015-01-19 14:48         ` Daniel Borkmann
  2015-01-19 14:50           ` Vadim Kochan
  1 sibling, 1 reply; 19+ messages in thread
From: Daniel Borkmann @ 2015-01-19 14:48 UTC (permalink / raw)
  To: Hagen Paul Pfeifer; +Cc: Vadim Kochan, netdev, stephen

On 01/19/2015 03:28 PM, Hagen Paul Pfeifer wrote:
> Hey Vadin,
>
> to make this short. We already discussed about changing the layout and
> Stephen nacked this. My proposal was to key:value the output. Because
> nearly all outputed data is already in this format - except the
> congestion control algo, ts, sack and tx'ed data. Where I proposed
> cc:<algo>. The key:value format has the advantages that the ordering
> do not mather anymore, An python parser would be something like split
> for whitespaces and later split for colon. Currently parsing this is a
> mess, see [1].
>
> Anyway, the more clever idea is to add an json outputer like already
> supported by some ss modules and get rid of this mess.

+1

I was also thinking in addition to json, that it might be useful to have
an optional ncurses top-like mode in ss. The level of detail could be
unfolded for a specific entry on demand, etc. I would not add it as a
hard library requirement, but in case ncurses headers are detected by
the configure script, it could be compiled in then. It's also easily
changeable since there's no such requirement that the way data is being
displayed needs to be stable for scripts.

> Hagen
>
> [1] https://github.com/hgn/captcp/blob/master/captcp.py#L4861

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH iproute2 3/3] ss: Unify tcp stats output
  2015-01-19 14:48         ` Daniel Borkmann
@ 2015-01-19 14:50           ` Vadim Kochan
  0 siblings, 0 replies; 19+ messages in thread
From: Vadim Kochan @ 2015-01-19 14:50 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: Hagen Paul Pfeifer, Vadim Kochan, netdev, stephen

On Mon, Jan 19, 2015 at 03:48:56PM +0100, Daniel Borkmann wrote:
> On 01/19/2015 03:28 PM, Hagen Paul Pfeifer wrote:
> >Hey Vadin,
> >
> >to make this short. We already discussed about changing the layout and
> >Stephen nacked this. My proposal was to key:value the output. Because
> >nearly all outputed data is already in this format - except the
> >congestion control algo, ts, sack and tx'ed data. Where I proposed
> >cc:<algo>. The key:value format has the advantages that the ordering
> >do not mather anymore, An python parser would be something like split
> >for whitespaces and later split for colon. Currently parsing this is a
> >mess, see [1].
> >
> >Anyway, the more clever idea is to add an json outputer like already
> >supported by some ss modules and get rid of this mess.
> 
> +1
> 
> I was also thinking in addition to json, that it might be useful to have
> an optional ncurses top-like mode in ss. The level of detail could be
> unfolded for a specific entry on demand, etc. I would not add it as a
> hard library requirement, but in case ncurses headers are detected by
> the configure script, it could be compiled in then. It's also easily
> changeable since there's no such requirement that the way data is being
> displayed needs to be stable for scripts.
> 
> >Hagen
> >
> >[1] https://github.com/hgn/captcp/blob/master/captcp.py#L4861

OK I will re-work series to do only refactoring/cleanups. And in future
I will keep in mind about any surprises for ss parsers.

Regards,

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH iproute2 3/3] ss: Unify tcp stats output
  2015-01-19 14:28         ` Vadim Kochan
@ 2015-01-19 15:01           ` Hagen Paul Pfeifer
  0 siblings, 0 replies; 19+ messages in thread
From: Hagen Paul Pfeifer @ 2015-01-19 15:01 UTC (permalink / raw)
  To: Vadim Kochan; +Cc: netdev

On 19 January 2015 at 15:28, Vadim Kochan <vadim4j@gmail.com> wrote:

> I am thinking may be 1st of all it is better to make output in json
> format and after this trying to make changes with human readability.
>
> What do you think ?

Mhh, if an JSON outputer is also provided a prioi *I* had no problems
with an incompatible change. It is awful to parse the current output.
*But* I am not sure if this opinion is shared by Stephen too. We
probably break several scripts/applications with this change. Breaking
the output *and* do not provide an stable alternative (JSON) is bad.

+1 for JSON (which is not that hard to implement)

hgn

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH iproute2 3/3] ss: Unify tcp stats output
  2015-01-18 20:43 ` [PATCH iproute2 3/3] ss: Unify tcp stats output Vadim Kochan
  2015-01-19 13:57   ` Hagen Paul Pfeifer
@ 2015-01-20 10:29   ` Hagen Paul Pfeifer
  2015-01-20 10:52     ` Vadim Kochan
                       ` (3 more replies)
  1 sibling, 4 replies; 19+ messages in thread
From: Hagen Paul Pfeifer @ 2015-01-20 10:29 UTC (permalink / raw)
  To: Vadim Kochan; +Cc: netdev, Stephen Hemminger, Eric Dumazet

On 18 January 2015 at 21:43, Vadim Kochan <vadim4j@gmail.com> wrote:

Hey Stephen,

it is time to think about the format of the ss output - it starts to
get unreadable. Neither humans nor scripts will parse the output. See
the patch at the end,to get idea what I mean: who will understand
"fallback_mode"? I don't want to blame someone - not at all, it is
just one example. My proposal:

1) support grouping. E.g. DCTCP could be separated in the following
manner. I.e. "dctcp:[ ce_state: %d, alpha: %d, ...  ]" or
"dctcp:ce_state.%d;alpha,%d" like the socket memory output.
    This makes it easier for humans and scripts to parse the output.
Since some time ss output is extended really extensive (especially
Eric make use of this). This is not the end and additional values are
added - make the current babylon even worse.

2) add a JSON formater as soon as possible to make the output
parseable. I would do this - it is required anyway.

Any comments on this?

> +       if (s->ssthresh)
> +               printf(" ssthresh:%d", s->ssthresh);
> +
> +       if (s->dctcp && s->dctcp->enabled) {
> +               struct dctcpstat *dctcp = s->dctcp;
> +
> +               printf(" ce_state %u alpha %u ab_ecn %u ab_tot %u",
> +                               dctcp->ce_state, dctcp->alpha, dctcp->ab_ecn,
> +                               dctcp->ab_tot);
> +       } else if (s->dctcp) {
> +               printf(" fallback_mode");
> +       }
> +
> +       if (s->send_bps)
> +               printf(" send %sbps", sprint_bandw(b1, s->send_bps));

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH iproute2 3/3] ss: Unify tcp stats output
  2015-01-20 10:29   ` Hagen Paul Pfeifer
@ 2015-01-20 10:52     ` Vadim Kochan
  2015-01-20 11:06     ` Vadim Kochan
                       ` (2 subsequent siblings)
  3 siblings, 0 replies; 19+ messages in thread
From: Vadim Kochan @ 2015-01-20 10:52 UTC (permalink / raw)
  To: Hagen Paul Pfeifer; +Cc: Vadim Kochan, netdev, Stephen Hemminger, Eric Dumazet

On Tue, Jan 20, 2015 at 11:29:53AM +0100, Hagen Paul Pfeifer wrote:
> On 18 January 2015 at 21:43, Vadim Kochan <vadim4j@gmail.com> wrote:
> 
> Hey Stephen,
> 
> it is time to think about the format of the ss output - it starts to
> get unreadable. Neither humans nor scripts will parse the output. See
> the patch at the end,to get idea what I mean: who will understand
> "fallback_mode"? I don't want to blame someone - not at all, it is
> just one example. My proposal:
> 
> 1) support grouping. E.g. DCTCP could be separated in the following
> manner. I.e. "dctcp:[ ce_state: %d, alpha: %d, ...  ]" or
> "dctcp:ce_state.%d;alpha,%d" like the socket memory output.
>     This makes it easier for humans and scripts to parse the output.
> Since some time ss output is extended really extensive (especially
> Eric make use of this). This is not the end and additional values are
> added - make the current babylon even worse.
> 
> 2) add a JSON formater as soon as possible to make the output
> parseable. I would do this - it is required anyway.
> 
> Any comments on this?
> 
> > +       if (s->ssthresh)
> > +               printf(" ssthresh:%d", s->ssthresh);
> > +
> > +       if (s->dctcp && s->dctcp->enabled) {
> > +               struct dctcpstat *dctcp = s->dctcp;
> > +
> > +               printf(" ce_state %u alpha %u ab_ecn %u ab_tot %u",
> > +                               dctcp->ce_state, dctcp->alpha, dctcp->ab_ecn,
> > +                               dctcp->ab_tot);
> > +       } else if (s->dctcp) {
> > +               printf(" fallback_mode");
> > +       }
> > +
> > +       if (s->send_bps)
> > +               printf(" send %sbps", sprint_bandw(b1, s->send_bps));

This is not topic related but I think that it would be good to move
ss to separate dir (like bridge, tc) and split this 3700 lines into
smaller modules.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH iproute2 3/3] ss: Unify tcp stats output
  2015-01-20 10:29   ` Hagen Paul Pfeifer
  2015-01-20 10:52     ` Vadim Kochan
@ 2015-01-20 11:06     ` Vadim Kochan
  2015-01-20 11:17     ` Daniel Borkmann
  2015-01-20 18:36     ` Cong Wang
  3 siblings, 0 replies; 19+ messages in thread
From: Vadim Kochan @ 2015-01-20 11:06 UTC (permalink / raw)
  To: Hagen Paul Pfeifer; +Cc: Vadim Kochan, netdev, Stephen Hemminger, Eric Dumazet

On Tue, Jan 20, 2015 at 11:29:53AM +0100, Hagen Paul Pfeifer wrote:
> On 18 January 2015 at 21:43, Vadim Kochan <vadim4j@gmail.com> wrote:
> 
> Hey Stephen,
> 
> it is time to think about the format of the ss output - it starts to
> get unreadable. Neither humans nor scripts will parse the output. See
> the patch at the end,to get idea what I mean: who will understand
> "fallback_mode"? I don't want to blame someone - not at all, it is
> just one example. My proposal:
> 
> 1) support grouping. E.g. DCTCP could be separated in the following
> manner. I.e. "dctcp:[ ce_state: %d, alpha: %d, ...  ]" or
> "dctcp:ce_state.%d;alpha,%d" like the socket memory output.

For me the current socket memory output looks weird, I think this one can be
better:
    skmem:(rd=%s,rcv_buf=%s,wr=%s,snd_buf=%s,fwd=%s,wr_queue=%s,oth=%s)

>     This makes it easier for humans and scripts to parse the output.
> Since some time ss output is extended really extensive (especially
> Eric make use of this). This is not the end and additional values are
> added - make the current babylon even worse.
> 
> 2) add a JSON formater as soon as possible to make the output
> parseable. I would do this - it is required anyway.
> 
> Any comments on this?
> 
> > +       if (s->ssthresh)
> > +               printf(" ssthresh:%d", s->ssthresh);
> > +
> > +       if (s->dctcp && s->dctcp->enabled) {
> > +               struct dctcpstat *dctcp = s->dctcp;
> > +
> > +               printf(" ce_state %u alpha %u ab_ecn %u ab_tot %u",
> > +                               dctcp->ce_state, dctcp->alpha, dctcp->ab_ecn,
> > +                               dctcp->ab_tot);
> > +       } else if (s->dctcp) {
> > +               printf(" fallback_mode");
> > +       }
> > +
> > +       if (s->send_bps)
> > +               printf(" send %sbps", sprint_bandw(b1, s->send_bps));

And what about processes list ? I think it would be good to have
something like:

u_str  ESTAB      0      0                  * 14046            * 8801
    process:("process#0",pid=535,fd=2)
            ("process#1",pid=535,fd=1)
            ("process#2",pid=535,fd=1)

Instead of:
u_str  ESTAB      0      0                  * 14046            * 8801
    users:(("qtile",pid=535,fd=2),("qtile",pid=535,fd=1))

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH iproute2 3/3] ss: Unify tcp stats output
  2015-01-20 11:17     ` Daniel Borkmann
@ 2015-01-20 11:09       ` Vadim Kochan
  2015-01-20 11:43       ` Hagen Paul Pfeifer
  1 sibling, 0 replies; 19+ messages in thread
From: Vadim Kochan @ 2015-01-20 11:09 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Hagen Paul Pfeifer, Vadim Kochan, netdev, Stephen Hemminger,
	Eric Dumazet

On Tue, Jan 20, 2015 at 12:17:03PM +0100, Daniel Borkmann wrote:
> Hi Hagen,
> 
> On 01/20/2015 11:29 AM, Hagen Paul Pfeifer wrote:
> >On 18 January 2015 at 21:43, Vadim Kochan <vadim4j@gmail.com> wrote:
> ...
> >My proposal:
> >
> >1) support grouping. E.g. DCTCP could be separated in the following
> >manner. I.e. "dctcp:[ ce_state: %d, alpha: %d, ...  ]" or
> >"dctcp:ce_state.%d;alpha,%d" like the socket memory output.
> >     This makes it easier for humans and scripts to parse the output.
> >Since some time ss output is extended really extensive (especially
> >Eric make use of this). This is not the end and additional values are
> >added - make the current babylon even worse.
> 
> I have no strong opinion on this, but I also have a limited view on
> what applications try to parse ss output in general.
> 
> As mentioned, for human readability, we should implement a top-like
> display option which is allowed to have a rather 'instable' output
> by nature and levels of detail can be folded/unfolded on demand.

I think it would be good to have as option and output some default basic
info ?
> 
> >2) add a JSON formater as soon as possible to make the output
> >parseable. I would do this - it is required anyway.
> 
> Given the recent discussion on web10g, json output option might
> be very useful to provide i.e. when stats are being further extended.
> 
> Cheers,
> Daniel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH iproute2 3/3] ss: Unify tcp stats output
  2015-01-20 10:29   ` Hagen Paul Pfeifer
  2015-01-20 10:52     ` Vadim Kochan
  2015-01-20 11:06     ` Vadim Kochan
@ 2015-01-20 11:17     ` Daniel Borkmann
  2015-01-20 11:09       ` Vadim Kochan
  2015-01-20 11:43       ` Hagen Paul Pfeifer
  2015-01-20 18:36     ` Cong Wang
  3 siblings, 2 replies; 19+ messages in thread
From: Daniel Borkmann @ 2015-01-20 11:17 UTC (permalink / raw)
  To: Hagen Paul Pfeifer; +Cc: Vadim Kochan, netdev, Stephen Hemminger, Eric Dumazet

Hi Hagen,

On 01/20/2015 11:29 AM, Hagen Paul Pfeifer wrote:
> On 18 January 2015 at 21:43, Vadim Kochan <vadim4j@gmail.com> wrote:
...
> My proposal:
>
> 1) support grouping. E.g. DCTCP could be separated in the following
> manner. I.e. "dctcp:[ ce_state: %d, alpha: %d, ...  ]" or
> "dctcp:ce_state.%d;alpha,%d" like the socket memory output.
>      This makes it easier for humans and scripts to parse the output.
> Since some time ss output is extended really extensive (especially
> Eric make use of this). This is not the end and additional values are
> added - make the current babylon even worse.

I have no strong opinion on this, but I also have a limited view on
what applications try to parse ss output in general.

As mentioned, for human readability, we should implement a top-like
display option which is allowed to have a rather 'instable' output
by nature and levels of detail can be folded/unfolded on demand.

> 2) add a JSON formater as soon as possible to make the output
> parseable. I would do this - it is required anyway.

Given the recent discussion on web10g, json output option might
be very useful to provide i.e. when stats are being further extended.

Cheers,
Daniel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH iproute2 3/3] ss: Unify tcp stats output
  2015-01-20 11:17     ` Daniel Borkmann
  2015-01-20 11:09       ` Vadim Kochan
@ 2015-01-20 11:43       ` Hagen Paul Pfeifer
  1 sibling, 0 replies; 19+ messages in thread
From: Hagen Paul Pfeifer @ 2015-01-20 11:43 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: Vadim Kochan, netdev, Stephen Hemminger, Eric Dumazet

On 20 January 2015 at 12:17, Daniel Borkmann <dborkman@redhat.com> wrote:

> I have no strong opinion on this, but I also have a limited view on
> what applications try to parse ss output in general.
>
> As mentioned, for human readability, we should implement a top-like
> display option which is allowed to have a rather 'instable' output
> by nature and levels of detail can be folded/unfolded on demand.

+1 on that.

> Given the recent discussion on web10g, json output option might
> be very useful to provide i.e. when stats are being further extended.

Ok, there seems conses on JSON output.

Stephan, what do you think?

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH iproute2 3/3] ss: Unify tcp stats output
  2015-01-20 10:29   ` Hagen Paul Pfeifer
                       ` (2 preceding siblings ...)
  2015-01-20 11:17     ` Daniel Borkmann
@ 2015-01-20 18:36     ` Cong Wang
  2015-01-21  8:54       ` Hagen Paul Pfeifer
  3 siblings, 1 reply; 19+ messages in thread
From: Cong Wang @ 2015-01-20 18:36 UTC (permalink / raw)
  To: Hagen Paul Pfeifer; +Cc: Vadim Kochan, netdev, Stephen Hemminger, Eric Dumazet

On Tue, Jan 20, 2015 at 2:29 AM, Hagen Paul Pfeifer <hagen@jauu.net> wrote:
> On 18 January 2015 at 21:43, Vadim Kochan <vadim4j@gmail.com> wrote:
>
> Hey Stephen,
>
> it is time to think about the format of the ss output - it starts to
> get unreadable. Neither humans nor scripts will parse the output. See
> the patch at the end,to get idea what I mean: who will understand
> "fallback_mode"? I don't want to blame someone - not at all, it is
> just one example. My proposal:
>

On the other hand, what blocks you from parsing the netlink message
by yourself? Don't get me wrong, I am not a fan of netlink, but
generally speaking, ss is not alone, too many scripts and applications
nowadays parse iproute2 tools output.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH iproute2 3/3] ss: Unify tcp stats output
  2015-01-20 18:36     ` Cong Wang
@ 2015-01-21  8:54       ` Hagen Paul Pfeifer
  0 siblings, 0 replies; 19+ messages in thread
From: Hagen Paul Pfeifer @ 2015-01-21  8:54 UTC (permalink / raw)
  To: Cong Wang; +Cc: Vadim Kochan, netdev, Stephen Hemminger, Eric Dumazet

20 January 2015 at 19:36, Cong Wang <cwang@twopensource.com> wrote:

> On the other hand, what blocks you from parsing the netlink message
> by yourself? Don't get me wrong, I am not a fan of netlink, but
> generally speaking, ss is not alone, too many scripts and applications
> nowadays parse iproute2 tools output.

Agree, thus the proposal:

1) *new* ss values are "prefixed" by an name to make it possible that
*humans* can understand the value ("make ss output human readable").
Concrete rule for scalar value: key:value. Concrete rule for list of
values: subsystem:(key1:val1,key1:val2). The actual ss output is
nearly formatted matching these rules. E.g. see socket memory for list
formated output. Sure, there are values like SACK that do not need any
prefix and can assumed to be known.

2) add JSON format for scripts and tools.

Hagen

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2015-01-21  8:54 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-18 20:43 [PATCH iproute2 0/3] Unify output for inet sockets Vadim Kochan
2015-01-18 20:43 ` [PATCH iproute2 1/3] ss: Make meminfo look little bit more readable Vadim Kochan
2015-01-18 20:43 ` [PATCH iproute2 2/3] ss: Unify inet sockets output Vadim Kochan
2015-01-18 20:43 ` [PATCH iproute2 3/3] ss: Unify tcp stats output Vadim Kochan
2015-01-19 13:57   ` Hagen Paul Pfeifer
2015-01-19 14:04     ` Vadim Kochan
2015-01-19 14:28       ` Hagen Paul Pfeifer
2015-01-19 14:28         ` Vadim Kochan
2015-01-19 15:01           ` Hagen Paul Pfeifer
2015-01-19 14:48         ` Daniel Borkmann
2015-01-19 14:50           ` Vadim Kochan
2015-01-20 10:29   ` Hagen Paul Pfeifer
2015-01-20 10:52     ` Vadim Kochan
2015-01-20 11:06     ` Vadim Kochan
2015-01-20 11:17     ` Daniel Borkmann
2015-01-20 11:09       ` Vadim Kochan
2015-01-20 11:43       ` Hagen Paul Pfeifer
2015-01-20 18:36     ` Cong Wang
2015-01-21  8:54       ` Hagen Paul Pfeifer

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).