[PATCH iproute2 0/3] ss: fix slab statistics

[PATCH iproute2 0/3] ss: fix slab statistics

[Stephen Hemminger]

The ss -s command has been broken in several places since the
2.6.13 kernel. The slab values it is looking for have changed
names and kernel started merging the values (see kernel patch
for fixing that).

Stephen Hemminger (3):
  ss: drop unused slabstat for skb's
  ss: convert socket statistics to unsigned
  ss: update slabinfo names and sum IPv4 and IPv6

 misc/ss.c | 125 +++++++++++++++++++++++++++++++-------------------------------
 1 file changed, 62 insertions(+), 63 deletions(-)

-- 
2.16.1

[PATCH iproute2 1/3] ss: drop unused slabstat for skb's

[Stephen Hemminger]

This value is not used anywhere in current code.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 misc/ss.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index e047f9c04582..fefe5c813c44 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -678,7 +678,6 @@ struct slabstat {
 	int tcp_ports;
 	int tcp_tws;
 	int tcp_syns;
-	int skbs;
 };
 
 static struct slabstat slabstat;
@@ -694,7 +693,6 @@ static int get_slabstat(struct slabstat *s)
 		"tcp_bind_bucket",
 		"tcp_tw_bucket",
 		"tcp_open_request",
-		"skbuff_head_cache",
 	};
 
 	if (slabstat_valid)
-- 
2.16.1

[PATCH iproute2 2/3] ss: convert socket statistics to unsigned

[Stephen Hemminger]

The kernel keeps these values as unsigned not signed integer.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 misc/ss.c | 96 +++++++++++++++++++++++++++++++--------------------------------
 1 file changed, 48 insertions(+), 48 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index fefe5c813c44..4c7057198eb2 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -674,10 +674,10 @@ next:
 /* Get stats from slab */
 
 struct slabstat {
-	int socks;
-	int tcp_ports;
-	int tcp_tws;
-	int tcp_syns;
+	unsigned int socks;
+	unsigned int tcp_ports;
+	unsigned int tcp_tws;
+	unsigned int tcp_syns;
 };
 
 static struct slabstat slabstat;
@@ -687,13 +687,14 @@ static int get_slabstat(struct slabstat *s)
 	char buf[256];
 	FILE *fp;
 	int cnt;
-	static int slabstat_valid;
+	static bool slabstat_valid;
 	static const char * const slabstat_ids[] = {
 		"sock",
 		"tcp_bind_bucket",
 		"tcp_tw_bucket",
 		"tcp_open_request",
 	};
+	unsigned int *vals = (unsigned int *) s;
 
 	if (slabstat_valid)
 		return 0;
@@ -715,7 +716,7 @@ static int get_slabstat(struct slabstat *s)
 
 		for (i = 0; i < ARRAY_SIZE(slabstat_ids); i++) {
 			if (memcmp(buf, slabstat_ids[i], strlen(slabstat_ids[i])) == 0) {
-				sscanf(buf, "%*s%d", ((int *)s) + i);
+				sscanf(buf, "%*s%u", vals + i);
 				cnt--;
 				break;
 			}
@@ -724,7 +725,7 @@ static int get_slabstat(struct slabstat *s)
 			break;
 	}
 
-	slabstat_valid = 1;
+	slabstat_valid = true;
 
 	fclose(fp);
 	return 0;
@@ -3287,8 +3288,7 @@ static int tcp_show(struct filter *f)
 	 */
 	if (1) {
 		get_slabstat(&slabstat);
-
-		int guess = slabstat.socks+slabstat.tcp_syns;
+		unsigned int guess = slabstat.socks + slabstat.tcp_syns;
 
 		if (f->states&(1<<SS_TIME_WAIT))
 			guess += slabstat.tcp_tws;
@@ -4317,7 +4317,7 @@ static int handle_follow_request(struct filter *f)
 	return ret;
 }
 
-static int get_snmp_int(char *proto, char *key, int *result)
+static int get_snmp_int(const char *proto, const char *key, unsigned int *result)
 {
 	char buf[1024];
 	FILE *fp;
@@ -4350,7 +4350,7 @@ static int get_snmp_int(char *proto, char *key, int *result)
 		while ((p = strchr(p, ' ')) != NULL) {
 			p++;
 			if (--pos == 0) {
-				sscanf(p, "%d", result);
+				sscanf(p, "%u", result);
 				fclose(fp);
 				return 0;
 			}
@@ -4366,21 +4366,21 @@ static int get_snmp_int(char *proto, char *key, int *result)
 /* Get stats from sockstat */
 
 struct ssummary {
-	int socks;
-	int tcp_mem;
-	int tcp_total;
-	int tcp_orphans;
-	int tcp_tws;
-	int tcp4_hashed;
-	int udp4;
-	int raw4;
-	int frag4;
-	int frag4_mem;
-	int tcp6_hashed;
-	int udp6;
-	int raw6;
-	int frag6;
-	int frag6_mem;
+	unsigned int socks;
+	unsigned int tcp_mem;
+	unsigned int tcp_total;
+	unsigned int tcp_orphans;
+	unsigned int tcp_tws;
+	unsigned int tcp4_hashed;
+	unsigned int udp4;
+	unsigned int raw4;
+	unsigned int frag4;
+	unsigned int frag4_mem;
+	unsigned int tcp6_hashed;
+	unsigned int udp6;
+	unsigned int raw6;
+	unsigned int frag6;
+	unsigned int frag6_mem;
 };
 
 static void get_sockstat_line(char *line, struct ssummary *s)
@@ -4391,23 +4391,23 @@ static void get_sockstat_line(char *line, struct ssummary *s)
 		return;
 
 	if (strcmp(id, "sockets:") == 0)
-		sscanf(rem, "%*s%d", &s->socks);
+		sscanf(rem, "%*s%u", &s->socks);
 	else if (strcmp(id, "UDP:") == 0)
-		sscanf(rem, "%*s%d", &s->udp4);
+		sscanf(rem, "%*s%u", &s->udp4);
 	else if (strcmp(id, "UDP6:") == 0)
-		sscanf(rem, "%*s%d", &s->udp6);
+		sscanf(rem, "%*s%u", &s->udp6);
 	else if (strcmp(id, "RAW:") == 0)
-		sscanf(rem, "%*s%d", &s->raw4);
+		sscanf(rem, "%*s%u", &s->raw4);
 	else if (strcmp(id, "RAW6:") == 0)
-		sscanf(rem, "%*s%d", &s->raw6);
+		sscanf(rem, "%*s%u", &s->raw6);
 	else if (strcmp(id, "TCP6:") == 0)
-		sscanf(rem, "%*s%d", &s->tcp6_hashed);
+		sscanf(rem, "%*s%u", &s->tcp6_hashed);
 	else if (strcmp(id, "FRAG:") == 0)
-		sscanf(rem, "%*s%d%*s%d", &s->frag4, &s->frag4_mem);
+		sscanf(rem, "%*s%u%*s%u", &s->frag4, &s->frag4_mem);
 	else if (strcmp(id, "FRAG6:") == 0)
-		sscanf(rem, "%*s%d%*s%d", &s->frag6, &s->frag6_mem);
+		sscanf(rem, "%*s%u%*s%u", &s->frag6, &s->frag6_mem);
 	else if (strcmp(id, "TCP:") == 0)
-		sscanf(rem, "%*s%d%*s%d%*s%d%*s%d%*s%d",
+		sscanf(rem, "%*s%u%*s%u%*s%u%*s%u%*s%u",
 		       &s->tcp4_hashed,
 		       &s->tcp_orphans, &s->tcp_tws, &s->tcp_total, &s->tcp_mem);
 }
@@ -4436,8 +4436,8 @@ static int get_sockstat(struct ssummary *s)
 
 static int print_summary(void)
 {
+	unsigned int tcp_estab;
 	struct ssummary s;
-	int tcp_estab;
 
 	if (get_sockstat(&s) < 0)
 		perror("ss: get_sockstat");
@@ -4446,9 +4446,9 @@ static int print_summary(void)
 
 	get_slabstat(&slabstat);
 
-	printf("Total: %d (kernel %d)\n", s.socks, slabstat.socks);
+	printf("Total: %u (kernel %u)\n", s.socks, slabstat.socks);
 
-	printf("TCP:   %d (estab %d, closed %d, orphaned %d, synrecv %d, timewait %d/%d), ports %d\n",
+	printf("TCP:   %u (estab %u, closed %u, orphaned %u, synrecv %u, timewait %u/%u), ports %u\n",
 	       s.tcp_total + slabstat.tcp_syns + s.tcp_tws,
 	       tcp_estab,
 	       s.tcp_total - (s.tcp4_hashed+s.tcp6_hashed-s.tcp_tws),
@@ -4460,16 +4460,16 @@ static int print_summary(void)
 
 	printf("\n");
 	printf("Transport Total     IP        IPv6\n");
-	printf("*	  %-9d %-9s %-9s\n", slabstat.socks, "-", "-");
-	printf("RAW	  %-9d %-9d %-9d\n", s.raw4+s.raw6, s.raw4, s.raw6);
-	printf("UDP	  %-9d %-9d %-9d\n", s.udp4+s.udp6, s.udp4, s.udp6);
-	printf("TCP	  %-9d %-9d %-9d\n", s.tcp4_hashed+s.tcp6_hashed, s.tcp4_hashed, s.tcp6_hashed);
-	printf("INET	  %-9d %-9d %-9d\n",
-	       s.raw4+s.udp4+s.tcp4_hashed+
-	       s.raw6+s.udp6+s.tcp6_hashed,
-	       s.raw4+s.udp4+s.tcp4_hashed,
-	       s.raw6+s.udp6+s.tcp6_hashed);
-	printf("FRAG	  %-9d %-9d %-9d\n", s.frag4+s.frag6, s.frag4, s.frag6);
+	printf("*	  %-9u %-9s %-9s\n", slabstat.socks, "-", "-");
+	printf("RAW	  %-9u %-9u %-9u\n", s.raw4+s.raw6, s.raw4, s.raw6);
+	printf("UDP	  %-9u %-9u %-9u\n", s.udp4+s.udp6, s.udp4, s.udp6);
+	printf("TCP	  %-9u %-9u %-9u\n", s.tcp4_hashed+s.tcp6_hashed, s.tcp4_hashed, s.tcp6_hashed);
+	printf("INET	  %-9u %-9u %-9u\n",
+	       s.raw4+s.udp4 + s.tcp4_hashed + s.raw6+s.udp6 + s.tcp6_hashed,
+	       s.raw4+s.udp4 + s.tcp4_hashed,
+	       s.raw6+s.udp6 + s.tcp6_hashed);
+	printf("FRAG	  %-9u %-9u %-9u\n",
+	       s.frag4+s.frag6, s.frag4, s.frag6);
 
 	printf("\n");
 
-- 
2.16.1

[PATCH iproute2 3/3] ss: update slabinfo names and sum IPv4 and IPv6

[Stephen Hemminger]

For a long time the name of slabs for TCP sockets have changed.
And ss summary was missing them. Also it never handled IPv6.
Change the code to sum values from both protocols.

Also, print error if slabinfo can not be opened.
Typically this is attempt to run 'ss -s' as non-root user
and /proc/slabinfo is now protected.

Reported-by: Igor Komyagin <ikomyagin@gmail.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 misc/ss.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 4c7057198eb2..f98acdba6e24 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -686,15 +686,14 @@ static int get_slabstat(struct slabstat *s)
 {
 	char buf[256];
 	FILE *fp;
-	int cnt;
 	static bool slabstat_valid;
 	static const char * const slabstat_ids[] = {
-		"sock",
+		"sock_inode_cache",
 		"tcp_bind_bucket",
-		"tcp_tw_bucket",
-		"tcp_open_request",
+		"tw_sock_TCP",
+		"request_sock_TCP",
 	};
-	unsigned int *vals = (unsigned int *) s;
+	unsigned int *slabstat_vals = (unsigned int *) s;
 
 	if (slabstat_valid)
 		return 0;
@@ -702,27 +701,29 @@ static int get_slabstat(struct slabstat *s)
 	memset(s, 0, sizeof(*s));
 
 	fp = slabinfo_open();
-	if (!fp)
+	if (!fp) {
+		fprintf(stderr, "slabinfo unavailable: %s\n",
+			strerror(errno));
 		return -1;
-
-	cnt = sizeof(*s)/sizeof(int);
+	}
 
 	if (!fgets(buf, sizeof(buf), fp)) {
 		fclose(fp);
 		return -1;
 	}
+
 	while (fgets(buf, sizeof(buf), fp) != NULL) {
 		int i;
 
 		for (i = 0; i < ARRAY_SIZE(slabstat_ids); i++) {
-			if (memcmp(buf, slabstat_ids[i], strlen(slabstat_ids[i])) == 0) {
-				sscanf(buf, "%*s%u", vals + i);
-				cnt--;
-				break;
-			}
+			unsigned long val;
+
+			if (memcmp(buf, slabstat_ids[i], strlen(slabstat_ids[i])))
+				continue;
+
+			if (sscanf(buf, "%*s%lu", &val))
+				slabstat_vals[i] += val;
 		}
-		if (cnt <= 0)
-			break;
 	}
 
 	slabstat_valid = true;
-- 
2.16.1

Re: [PATCH iproute2 0/3] ss: fix slab statistics

[David Ahern]

On 2/24/18 12:16 PM, Stephen Hemminger wrote:
> The ss -s command has been broken in several places since the
> 2.6.13 kernel. The slab values it is looking for have changed
> names and kernel started merging the values (see kernel patch
> for fixing that).
> 
> Stephen Hemminger (3):
>   ss: drop unused slabstat for skb's
>   ss: convert socket statistics to unsigned
>   ss: update slabinfo names and sum IPv4 and IPv6
> 
>  misc/ss.c | 125 +++++++++++++++++++++++++++++++-------------------------------
>  1 file changed, 62 insertions(+), 63 deletions(-)
> 

LGTM. Acked-by: David Ahern <dsahern@gmail.com>