[PATCH iproute2-next] ss: Add support for dumping TCP bound-inactive sockets.

[PATCH iproute2-next] ss: Add support for dumping TCP bound-inactive sockets.

[Guillaume Nault]

Make ss aware of the new "bound-inactive" pseudo-state for TCP (see
Linux commit 91051f003948 ("tcp: Dump bound-only sockets in inet_diag.")).
These are TCP sockets that have been bound, but are neither listening nor
connecting.

With this patch, these sockets can now be dumped with:

  * the existing -a (--all) option, to dump all sockets, including
    bound-inactive ones,

  * the new -B (--bound-inactive) option, to dump them exclusively,

  * the new "bound-inactive" state, to be used in a STATE-FILTER.

The SS_NEW_SYN_RECV pseudo-state is added in this patch only for code
consistency, so that SS_BOUND_INACTIVE gets assigned the right value
without manual assignment.

Note that the SS_BOUND_INACTIVE state is a pseudo-state used for queries
only. The kernel returns them as SS_CLOSE.

Signed-off-by: Guillaume Nault <gnault@redhat.com>
---
 man/man8/ss.8 |  7 +++++++
 misc/ss.c     | 13 ++++++++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/man/man8/ss.8 b/man/man8/ss.8
index 073e9f03..4ece41fa 100644
--- a/man/man8/ss.8
+++ b/man/man8/ss.8
@@ -40,6 +40,10 @@ established connections) sockets.
 .B \-l, \-\-listening
 Display only listening sockets (these are omitted by default).
 .TP
+.B \-B, \-\-bound-inactive
+Display only TCP bound but inactive (not listening, connecting, etc.) sockets
+(these are omitted by default).
+.TP
 .B \-o, \-\-options
 Show timer information. For TCP protocol, the output format is:
 .RS
@@ -456,6 +460,9 @@ states except for
 - opposite to
 .B bucket
 
+.B bound-inactive
+- bound but otherwise inactive sockets (not listening, connecting, etc.)
+
 .SH EXPRESSION
 
 .B EXPRESSION
diff --git a/misc/ss.c b/misc/ss.c
index 16ffb6c8..19adc1b7 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -210,6 +210,8 @@ enum {
 	SS_LAST_ACK,
 	SS_LISTEN,
 	SS_CLOSING,
+	SS_NEW_SYN_RECV,
+	SS_BOUND_INACTIVE,
 	SS_MAX
 };
 
@@ -1382,6 +1384,8 @@ static void sock_state_print(struct sockstat *s)
 		[SS_LAST_ACK] = "LAST-ACK",
 		[SS_LISTEN] =	"LISTEN",
 		[SS_CLOSING] = "CLOSING",
+		[SS_NEW_SYN_RECV] = "NEW-SYN-RECV",
+		[SS_BOUND_INACTIVE] = "BOUND-INACTIVE",
 	};
 
 	switch (s->local.family) {
@@ -5339,6 +5343,7 @@ static void _usage(FILE *dest)
 "   -r, --resolve       resolve host names\n"
 "   -a, --all           display all sockets\n"
 "   -l, --listening     display listening sockets\n"
+"   -B, --bound-inactive display TCP bound but inactive sockets\n"
 "   -o, --options       show timer information\n"
 "   -e, --extended      show detailed socket information\n"
 "   -m, --memory        show socket memory usage\n"
@@ -5421,6 +5426,8 @@ static int scan_state(const char *state)
 		[SS_LAST_ACK] = "last-ack",
 		[SS_LISTEN] =	"listening",
 		[SS_CLOSING] = "closing",
+		[SS_NEW_SYN_RECV] = "new-syn-recv",
+		[SS_BOUND_INACTIVE] = "bound-inactive",
 	};
 	int i;
 
@@ -5487,6 +5494,7 @@ static const struct option long_opts[] = {
 	{ "vsock", 0, 0, OPT_VSOCK },
 	{ "all", 0, 0, 'a' },
 	{ "listening", 0, 0, 'l' },
+	{ "bound-inactive", 0, 0, 'B' },
 	{ "ipv4", 0, 0, '4' },
 	{ "ipv6", 0, 0, '6' },
 	{ "packet", 0, 0, '0' },
@@ -5525,7 +5533,7 @@ int main(int argc, char *argv[])
 	int state_filter = 0;
 
 	while ((ch = getopt_long(argc, argv,
-				 "dhaletuwxnro460spTbEf:mMiA:D:F:vVzZN:KHSO",
+				 "dhalBetuwxnro460spTbEf:mMiA:D:F:vVzZN:KHSO",
 				 long_opts, NULL)) != EOF) {
 		switch (ch) {
 		case 'n':
@@ -5590,6 +5598,9 @@ int main(int argc, char *argv[])
 		case 'l':
 			state_filter = (1 << SS_LISTEN) | (1 << SS_CLOSE);
 			break;
+		case 'B':
+			state_filter = 1 << SS_BOUND_INACTIVE;
+			break;
 		case '4':
 			filter_af_set(&current_filter, AF_INET);
 			break;
-- 
2.39.2

[PATCH iproute2-next] ss: Add support for dumping TCP bound-inactive sockets.

[Kuniyuki Iwashima]

From: Guillaume Nault <gnault@redhat.com>
Date: Mon, 11 Dec 2023 14:35:15 +0100
> Make ss aware of the new "bound-inactive" pseudo-state for TCP (see
> Linux commit 91051f003948 ("tcp: Dump bound-only sockets in inet_diag.")).
> These are TCP sockets that have been bound, but are neither listening nor
> connecting.
> 
> With this patch, these sockets can now be dumped with:
> 
>   * the existing -a (--all) option, to dump all sockets, including
>     bound-inactive ones,
> 
>   * the new -B (--bound-inactive) option, to dump them exclusively,
> 
>   * the new "bound-inactive" state, to be used in a STATE-FILTER.
> 
> The SS_NEW_SYN_RECV pseudo-state is added in this patch only for code
> consistency, so that SS_BOUND_INACTIVE gets assigned the right value
> without manual assignment.
> 
> Note that the SS_BOUND_INACTIVE state is a pseudo-state used for queries
> only. The kernel returns them as SS_CLOSE.
> 
> Signed-off-by: Guillaume Nault <gnault@redhat.com>
> ---
>  man/man8/ss.8 |  7 +++++++
>  misc/ss.c     | 13 ++++++++++++-
>  2 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/man/man8/ss.8 b/man/man8/ss.8
> index 073e9f03..4ece41fa 100644
> --- a/man/man8/ss.8
> +++ b/man/man8/ss.8
> @@ -40,6 +40,10 @@ established connections) sockets.
>  .B \-l, \-\-listening
>  Display only listening sockets (these are omitted by default).
>  .TP
> +.B \-B, \-\-bound-inactive
> +Display only TCP bound but inactive (not listening, connecting, etc.) sockets
> +(these are omitted by default).
> +.TP
>  .B \-o, \-\-options
>  Show timer information. For TCP protocol, the output format is:
>  .RS
> @@ -456,6 +460,9 @@ states except for
>  - opposite to
>  .B bucket
>  
> +.B bound-inactive
> +- bound but otherwise inactive sockets (not listening, connecting, etc.)
> +
>  .SH EXPRESSION
>  
>  .B EXPRESSION
> diff --git a/misc/ss.c b/misc/ss.c
> index 16ffb6c8..19adc1b7 100644
> --- a/misc/ss.c
> +++ b/misc/ss.c
> @@ -210,6 +210,8 @@ enum {
>  	SS_LAST_ACK,
>  	SS_LISTEN,
>  	SS_CLOSING,
> +	SS_NEW_SYN_RECV,

I think this is bit confusing as TCP_NEW_SYN_RECV is usually
invisible from user.

TCP_NEW_SYN_RECV was originally split from TCP_SYN_RECV for a
non-{TFO,cross-SYN} request.

So, both get_openreq4() (/proc/net/tcp) and inet_req_diag_fill()
(inet_diag) maps TCP_NEW_SYN_RECV to TCP_SYN_RECV.


> +	SS_BOUND_INACTIVE,

I prefer explicitly assigning a number to SS_BOUND_INACTIVE.


>  	SS_MAX
>  };
>  
> @@ -1382,6 +1384,8 @@ static void sock_state_print(struct sockstat *s)
>  		[SS_LAST_ACK] = "LAST-ACK",
>  		[SS_LISTEN] =	"LISTEN",
>  		[SS_CLOSING] = "CLOSING",
> +		[SS_NEW_SYN_RECV] = "NEW-SYN-RECV",

If we need to define SS_NEW_SYN_RECV, I prefer not setting
it or setting "" or "SYN-RECV".


> +		[SS_BOUND_INACTIVE] = "BOUND-INACTIVE",
>  	};
>  
>  	switch (s->local.family) {
> @@ -5339,6 +5343,7 @@ static void _usage(FILE *dest)
>  "   -r, --resolve       resolve host names\n"
>  "   -a, --all           display all sockets\n"
>  "   -l, --listening     display listening sockets\n"
> +"   -B, --bound-inactive display TCP bound but inactive sockets\n"
>  "   -o, --options       show timer information\n"
>  "   -e, --extended      show detailed socket information\n"
>  "   -m, --memory        show socket memory usage\n"
> @@ -5421,6 +5426,8 @@ static int scan_state(const char *state)
>  		[SS_LAST_ACK] = "last-ack",
>  		[SS_LISTEN] =	"listening",
>  		[SS_CLOSING] = "closing",
> +		[SS_NEW_SYN_RECV] = "new-syn-recv",

Same here.

Thanks!


> +		[SS_BOUND_INACTIVE] = "bound-inactive",
>  	};
>  	int i;
>  
> @@ -5487,6 +5494,7 @@ static const struct option long_opts[] = {
>  	{ "vsock", 0, 0, OPT_VSOCK },
>  	{ "all", 0, 0, 'a' },
>  	{ "listening", 0, 0, 'l' },
> +	{ "bound-inactive", 0, 0, 'B' },
>  	{ "ipv4", 0, 0, '4' },
>  	{ "ipv6", 0, 0, '6' },
>  	{ "packet", 0, 0, '0' },
> @@ -5525,7 +5533,7 @@ int main(int argc, char *argv[])
>  	int state_filter = 0;
>  
>  	while ((ch = getopt_long(argc, argv,
> -				 "dhaletuwxnro460spTbEf:mMiA:D:F:vVzZN:KHSO",
> +				 "dhalBetuwxnro460spTbEf:mMiA:D:F:vVzZN:KHSO",
>  				 long_opts, NULL)) != EOF) {
>  		switch (ch) {
>  		case 'n':
> @@ -5590,6 +5598,9 @@ int main(int argc, char *argv[])
>  		case 'l':
>  			state_filter = (1 << SS_LISTEN) | (1 << SS_CLOSE);
>  			break;
> +		case 'B':
> +			state_filter = 1 << SS_BOUND_INACTIVE;
> +			break;
>  		case '4':
>  			filter_af_set(&current_filter, AF_INET);
>  			break;
> -- 
> 2.39.2

Re: [PATCH iproute2-next] ss: Add support for dumping TCP bound-inactive sockets.

[Guillaume Nault]

On Mon, Dec 11, 2023 at 11:19:17PM +0900, Kuniyuki Iwashima wrote:
> > diff --git a/misc/ss.c b/misc/ss.c
> > index 16ffb6c8..19adc1b7 100644
> > --- a/misc/ss.c
> > +++ b/misc/ss.c
> > @@ -210,6 +210,8 @@ enum {
> >  	SS_LAST_ACK,
> >  	SS_LISTEN,
> >  	SS_CLOSING,
> > +	SS_NEW_SYN_RECV,
> 
> I think this is bit confusing as TCP_NEW_SYN_RECV is usually
> invisible from user.
> 
> TCP_NEW_SYN_RECV was originally split from TCP_SYN_RECV for a
> non-{TFO,cross-SYN} request.
> 
> So, both get_openreq4() (/proc/net/tcp) and inet_req_diag_fill()
> (inet_diag) maps TCP_NEW_SYN_RECV to TCP_SYN_RECV.

I think we need to explicitely set SS_NEW_SYN_RECV anyway, because we
have to set its entry in the sstate_namel array in scan_state().

But I can add a comment like this:
@@ -210,6 +210,8 @@ enum {
 	SS_LAST_ACK,
 	SS_LISTEN,
 	SS_CLOSING,
+	SS_NEW_SYN_RECV, /* Kernel only value, not for use in user space */
+	SS_BOUND_INACTIVE,
 	SS_MAX
 };

> > +	SS_BOUND_INACTIVE,
> 
> I prefer explicitly assigning a number to SS_BOUND_INACTIVE.
> 
> 
> >  	SS_MAX
> >  };
> >  
> > @@ -1382,6 +1384,8 @@ static void sock_state_print(struct sockstat *s)
> >  		[SS_LAST_ACK] = "LAST-ACK",
> >  		[SS_LISTEN] =	"LISTEN",
> >  		[SS_CLOSING] = "CLOSING",
> > +		[SS_NEW_SYN_RECV] = "NEW-SYN-RECV",
> 
> If we need to define SS_NEW_SYN_RECV, I prefer not setting
> it or setting "" or "SYN-RECV".

I originally considered the string wasn't important as the kernel isn't
supposed to return this value. But I agree we can do better.

I don't really like "SYN-RECV" though, as I find it even more confusing.
I'd prefer your other proposal using "", or maybe "UNDEF", together with
a comment saying something like "Never returned by kernel".

> > +		[SS_BOUND_INACTIVE] = "BOUND-INACTIVE",

And whatever we decide for SS_NEW_SYN_RECV, we can apply the same to
SS_BOUND_INACTIVE, since the kernel isn't supposed set this state on
output too.

So we'd have something like:
@@ -1382,6 +1384,8 @@ static void sock_state_print(struct sockstat *s)
 		[SS_LAST_ACK] = "LAST-ACK",
 		[SS_LISTEN] =	"LISTEN",
 		[SS_CLOSING] = "CLOSING",
+		[SS_NEW_SYN_RECV] = "UNDEF", /* Never returned by kernel */
+		[SS_BOUND_INACTIVE] = "UNDEF", /* Never returned by kernel */
 	};

> > @@ -5421,6 +5426,8 @@ static int scan_state(const char *state)
> >  		[SS_LAST_ACK] = "last-ack",
> >  		[SS_LISTEN] =	"listening",
> >  		[SS_CLOSING] = "closing",
> > +		[SS_NEW_SYN_RECV] = "new-syn-recv",
> 
> Same here.

Well, here, whatever we do, the string associated with SS_NEW_SYN_RECV
will be usable by the user. So I'd prefer to have a specific and
unambiguous string and then explicitly refuse it.

What do you think of something like the following?

@@ -5421,9 +5426,17 @@ static int scan_state(const char *state)
 		[SS_LAST_ACK] = "last-ack",
 		[SS_LISTEN] =	"listening",
 		[SS_CLOSING] = "closing",
+		[SS_NEW_SYN_RECV] = "new-syn-recv",
+		[SS_BOUND_INACTIVE] = "bound-inactive",
 	};
 	int i;
 
+	/* NEW_SYN_RECV is a kernel implementation detail. It shouldn't be used
+	 * or even be visible to the user.
+	 */
+	if (strcasecmp(state, "new-syn-recv") == 0)
+		goto wrong_state;
+
 	if (strcasecmp(state, "close") == 0 ||
 	    strcasecmp(state, "closed") == 0)
 		return (1<<SS_CLOSE);
@@ -5446,6 +5459,7 @@ static int scan_state(const char *state)
 			return (1<<i);
 	}
 
+wrong_state:
 	fprintf(stderr, "ss: wrong state name: %s\n", state);
 	exit(-1);
 }

Re: [PATCH iproute2-next] ss: Add support for dumping TCP bound-inactive sockets.

[David Ahern]

On 12/11/23 7:19 AM, Kuniyuki Iwashima wrote:
>> diff --git a/misc/ss.c b/misc/ss.c
>> index 16ffb6c8..19adc1b7 100644
>> --- a/misc/ss.c
>> +++ b/misc/ss.c
>> @@ -210,6 +210,8 @@ enum {
>>  	SS_LAST_ACK,
>>  	SS_LISTEN,
>>  	SS_CLOSING,
>> +	SS_NEW_SYN_RECV,
> 
> I think this is bit confusing as TCP_NEW_SYN_RECV is usually
> invisible from user.
> 
> TCP_NEW_SYN_RECV was originally split from TCP_SYN_RECV for a
> non-{TFO,cross-SYN} request.
> 
> So, both get_openreq4() (/proc/net/tcp) and inet_req_diag_fill()
> (inet_diag) maps TCP_NEW_SYN_RECV to TCP_SYN_RECV.
> 
> 
>> +	SS_BOUND_INACTIVE,
> 
> I prefer explicitly assigning a number to SS_BOUND_INACTIVE.
> 

this is internal to the ss command; explicit value is not relevant.