[RFC iproute2-next 0/5] ss statistics fixes

[RFC iproute2-next 0/5] ss statistics fixes

[Stephen Hemminger]

From: Stephen Hemminger <sthemmin@microsoft.com>

The output of the ss -s command has been broken for a long time
because of kernel changes (ie since 2.6).

This is an attempt to resolve most of the issues. Still don't like
the way it is using slabinfo to get the data but some of this information
would be expensive for kernel to account for otherwise.

Stephen Hemminger (5):
  ss: make args to get_snmp_int const
  ss: make tcp_mem long
  ss: use sockstat to get TCP bind ports
  ss: don't look for skbuff_head_cache
  ss: use correct slab statistics

 misc/ss.c | 39 +++++++++++++++++----------------------
 1 file changed, 17 insertions(+), 22 deletions(-)

-- 
2.17.0

[RFC iproute2-next 1/5] ss: make args to get_snmp_int const

[Stephen Hemminger]

These are keys for lookup and should be const.

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

diff --git a/misc/ss.c b/misc/ss.c
index 3ed7e66962f3..22c76e34f83b 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -4539,7 +4539,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, int *result)
 {
 	char buf[1024];
 	FILE *fp;
-- 
2.17.0

[RFC iproute2-next 2/5] ss: make tcp_mem long

[Stephen Hemminger]

The tcp_memory field in /proc/net/sockstat is formatted as
a long value by kernel. Change ss to keep this as full value.

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

diff --git a/misc/ss.c b/misc/ss.c
index 22c76e34f83b..c88a25581755 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -4589,7 +4589,7 @@ static int get_snmp_int(const char *proto, const char *key, int *result)
 
 struct ssummary {
 	int socks;
-	int tcp_mem;
+	long tcp_mem;
 	int tcp_total;
 	int tcp_orphans;
 	int tcp_tws;
@@ -4629,7 +4629,7 @@ static void get_sockstat_line(char *line, struct ssummary *s)
 	else if (strcmp(id, "FRAG6:") == 0)
 		sscanf(rem, "%*s%d%*s%d", &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%d%*s%d%*s%d%*s%d%*s%ld",
 		       &s->tcp4_hashed,
 		       &s->tcp_orphans, &s->tcp_tws, &s->tcp_total, &s->tcp_mem);
 }
-- 
2.17.0

Re: [RFC iproute2-next 2/5] ss: make tcp_mem long

[Eric Dumazet]

On 05/02/2018 01:27 PM, Stephen Hemminger wrote:
> The tcp_memory field in /proc/net/sockstat is formatted as
> a long value by kernel. Change ss to keep this as full value.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  misc/ss.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/misc/ss.c b/misc/ss.c
> index 22c76e34f83b..c88a25581755 100644
> --- a/misc/ss.c
> +++ b/misc/ss.c
> @@ -4589,7 +4589,7 @@ static int get_snmp_int(const char *proto, const char *key, int *result)
>  
>  struct ssummary {
>  	int socks;
> -	int tcp_mem;
> +	long tcp_mem;
>  	int tcp_total;
>  	int tcp_orphans;
>  	int tcp_tws;
> @@ -4629,7 +4629,7 @@ static void get_sockstat_line(char *line, struct ssummary *s)
>  	else if (strcmp(id, "FRAG6:") == 0)
>  		sscanf(rem, "%*s%d%*s%d", &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%d%*s%d%*s%d%*s%d%*s%ld",
>  		       &s->tcp4_hashed,
>  		       &s->tcp_orphans, &s->tcp_tws, &s->tcp_total, &s->tcp_mem);
>  }
> 

Hi Stephen

It seems nothing uses yet the value ?

Also, do we care of iproute2 being compiled in 32bit mode, but eventually running on 64bit kernel ?

Re: [RFC iproute2-next 2/5] ss: make tcp_mem long

[Stephen Hemminger]

On Wed, 2 May 2018 14:08:53 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> On 05/02/2018 01:27 PM, Stephen Hemminger wrote:
> > The tcp_memory field in /proc/net/sockstat is formatted as
> > a long value by kernel. Change ss to keep this as full value.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> >  misc/ss.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/misc/ss.c b/misc/ss.c
> > index 22c76e34f83b..c88a25581755 100644
> > --- a/misc/ss.c
> > +++ b/misc/ss.c
> > @@ -4589,7 +4589,7 @@ static int get_snmp_int(const char *proto, const char *key, int *result)
> >  
> >  struct ssummary {
> >  	int socks;
> > -	int tcp_mem;
> > +	long tcp_mem;
> >  	int tcp_total;
> >  	int tcp_orphans;
> >  	int tcp_tws;
> > @@ -4629,7 +4629,7 @@ static void get_sockstat_line(char *line, struct ssummary *s)
> >  	else if (strcmp(id, "FRAG6:") == 0)
> >  		sscanf(rem, "%*s%d%*s%d", &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%d%*s%d%*s%d%*s%d%*s%ld",
> >  		       &s->tcp4_hashed,
> >  		       &s->tcp_orphans, &s->tcp_tws, &s->tcp_total, &s->tcp_mem);
> >  }
> >   
> 
> Hi Stephen
> 
> It seems nothing uses yet the value ?

Yup. let's just drop it from the scan 

[RFC iproute2-next 3/5] ss: use sockstat to get TCP bind ports

[Stephen Hemminger]

From: Stephen Hemminger <sthemmin@microsoft.com>

Using slabinfo to try and get the number of bind_buckets no longer
works because of slab cache merging. Instead use proposed enhancment
of /proc/net/sockstat to get the same data.

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

diff --git a/misc/ss.c b/misc/ss.c
index c88a25581755..4f76999c0fee 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -732,7 +732,6 @@ next:
 
 struct slabstat {
 	int socks;
-	int tcp_ports;
 	int tcp_tws;
 	int tcp_syns;
 	int skbs;
@@ -748,7 +747,6 @@ static int get_slabstat(struct slabstat *s)
 	static int slabstat_valid;
 	static const char * const slabstat_ids[] = {
 		"sock",
-		"tcp_bind_bucket",
 		"tcp_tw_bucket",
 		"tcp_open_request",
 		"skbuff_head_cache",
@@ -4594,6 +4592,7 @@ struct ssummary {
 	int tcp_orphans;
 	int tcp_tws;
 	int tcp4_hashed;
+	int tcp_ports;
 	int udp4;
 	int raw4;
 	int frag4;
@@ -4629,9 +4628,9 @@ static void get_sockstat_line(char *line, struct ssummary *s)
 	else if (strcmp(id, "FRAG6:") == 0)
 		sscanf(rem, "%*s%d%*s%d", &s->frag6, &s->frag6_mem);
 	else if (strcmp(id, "TCP:") == 0)
-		sscanf(rem, "%*s%d%*s%d%*s%d%*s%d%*s%ld",
+		sscanf(rem, "%*s%d%*s%d%*s%d%*s%d%*s%ld%*s%d",
 		       &s->tcp4_hashed,
-		       &s->tcp_orphans, &s->tcp_tws, &s->tcp_total, &s->tcp_mem);
+		       &s->tcp_orphans, &s->tcp_tws, &s->tcp_total, &s->tcp_mem, &s->tcp_ports);
 }
 
 static int get_sockstat(struct ssummary *s)
@@ -4676,8 +4675,7 @@ static int print_summary(void)
 	       s.tcp_total - (s.tcp4_hashed+s.tcp6_hashed-s.tcp_tws),
 	       s.tcp_orphans,
 	       slabstat.tcp_syns,
-	       s.tcp_tws, slabstat.tcp_tws,
-	       slabstat.tcp_ports
+	       s.tcp_tws, slabstat.tcp_tws, s.tcp_ports
 	       );
 
 	printf("\n");
-- 
2.17.0

[RFC iproute2-next 4/5] ss: don't look for skbuff_head_cache

[Stephen Hemminger]

From: Stephen Hemminger <sthemmin@microsoft.com>

Not used 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 4f76999c0fee..97304cd8abfc 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -734,7 +734,6 @@ struct slabstat {
 	int socks;
 	int tcp_tws;
 	int tcp_syns;
-	int skbs;
 };
 
 static struct slabstat slabstat;
@@ -749,7 +748,6 @@ static int get_slabstat(struct slabstat *s)
 		"sock",
 		"tcp_tw_bucket",
 		"tcp_open_request",
-		"skbuff_head_cache",
 	};
 
 	if (slabstat_valid)
-- 
2.17.0

[RFC iproute2-next 5/5] ss: use correct slab statistics

[Stephen Hemminger]

From: Stephen Hemminger <sthemmin@microsoft.com>

The slabinfo names changed years ago, and ss statistics were broken.
This changes to use current slab names and handle TCP IPv6.

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

diff --git a/misc/ss.c b/misc/ss.c
index 97304cd8abfc..66c767cc415b 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -742,12 +742,12 @@ static int get_slabstat(struct slabstat *s)
 {
 	char buf[256];
 	FILE *fp;
-	int cnt;
+	int *stats = (int *) s;
 	static int slabstat_valid;
 	static const char * const slabstat_ids[] = {
-		"sock",
-		"tcp_tw_bucket",
-		"tcp_open_request",
+		"sock_inode_cache",
+		"tw_sock_TCP",
+		"request_sock_TCP",
 	};
 
 	if (slabstat_valid)
@@ -759,24 +759,23 @@ static int get_slabstat(struct slabstat *s)
 	if (!fp)
 		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;
+		int i, v;
 
 		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);
-				cnt--;
+			if (memcmp(buf, slabstat_ids[i], strlen(slabstat_ids[i])) != 0)
+				continue;
+
+			if (sscanf(buf, "%*s%d", &v) == 1) {
+				stats[i] += v;
 				break;
 			}
 		}
-		if (cnt <= 0)
-			break;
 	}
 
 	slabstat_valid = 1;
-- 
2.17.0