[PATCH] ss: Enclose IPv6 address in brackets

This patch adds support for RFC2732 IPv6 address format with brackets
for the tool ss. So output for ss changes from
2a00:1450:400a:804::200e:443 to [2a00:1450:400a:804::200e]:443 for IPv6
addresses with attached port number.

Signed-off-by: Lehner Florian <redacted>
---
 misc/ss.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/misc/ss.c b/misc/ss.c
index 12763c9..db39c93 100644
--- a/misc/ss.c
+++ b/misc/ss.c

@@ -1059,7 +1059,11 @@ static void inet_addr_print(const inet_prefix *a,

int port, unsigned int ifindex
 			ap = format_host(AF_INET, 4, a->data);
 		}
 	} else {
-		ap = format_host(a->family, 16, a->data);
+		if (a->family == AF_INET6) {
+			sprintf(buf, "[%s]", format_host(a->family, 16, a->data));
+		} else {
+			ap = format_host(a->family, 16, a->data);
+		}
 		est_len = strlen(ap);
 		if (est_len <= addr_width)
 			est_len = addr_width;
-- 
2.9.4

On Sat, 29 Jul 2017 14:29:10 +0200
Florian Lehner [off-list ref] wrote:

This patch adds support for RFC2732 IPv6 address format with brackets
for the tool ss. So output for ss changes from
2a00:1450:400a:804::200e:443 to [2a00:1450:400a:804::200e]:443 for IPv6
addresses with attached port number.

Signed-off-by: Lehner Florian <redacted>
---

This is the right solution but it might break someone parsing the output of ss
command. Will apply it anyway unless someone has an objection.

On Sat, Jul 29, 2017 at 02:29:10PM +0200, Florian Lehner wrote:

Note that this will enclosed resolved hostnames in brackets as well, not
sure if that's intended. Looks like fixing that is not exactly trivial:
Hostname resolution is buried in format_host() which resides in
lib/utils.c so is shared code with ip, tc, etc. Hence, adding the
brackets in rt_addr_n2a_r() is not an option, either. Adding a 'bool *'
param to format_host() and format_host_r() indicating that name
resolution has happened might help here.

Cheers, Phil

On Mon, 31 Jul 2017 12:30:10 +0200
Phil Sutter [off-list ref] wrote:

On Sat, Jul 29, 2017 at 02:29:10PM +0200, Florian Lehner wrote:

quoted

This patch adds support for RFC2732 IPv6 address format with brackets
for the tool ss. So output for ss changes from
2a00:1450:400a:804::200e:443 to [2a00:1450:400a:804::200e]:443 for IPv6
addresses with attached port number.

Signed-off-by: Lehner Florian <redacted>
---
 misc/ss.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/misc/ss.c b/misc/ss.c
index 12763c9..db39c93 100644
--- a/misc/ss.c
+++ b/misc/ss.c

@@ -1059,7 +1059,11 @@ static void inet_addr_print(const inet_prefix *a,

int port, unsigned int ifindex
 			ap = format_host(AF_INET, 4, a->data);
 		}
 	} else {
-		ap = format_host(a->family, 16, a->data);
+		if (a->family == AF_INET6) {
+			sprintf(buf, "[%s]", format_host(a->family, 16, a->data));
+		} else {
+			ap = format_host(a->family, 16, a->data);
+		}
 		est_len = strlen(ap);
 		if (est_len <= addr_width)
 			est_len = addr_width;  

Note that this will enclosed resolved hostnames in brackets as well, not
sure if that's intended. Looks like fixing that is not exactly trivial:
Hostname resolution is buried in format_host() which resides in
lib/utils.c so is shared code with ip, tc, etc. Hence, adding the
brackets in rt_addr_n2a_r() is not an option, either. Adding a 'bool *'
param to format_host() and format_host_r() indicating that name
resolution has happened might help here.

Cheers, Phil

Also, this code should return "*" for IN6ADDR_ANY

On Mon, Jul 31, 2017 at 09:27:55AM -0700, Stephen Hemminger wrote:

On Mon, 31 Jul 2017 12:30:10 +0200
Phil Sutter [off-list ref] wrote:

quoted

On Sat, Jul 29, 2017 at 02:29:10PM +0200, Florian Lehner wrote:

quoted

This patch adds support for RFC2732 IPv6 address format with brackets
for the tool ss. So output for ss changes from
2a00:1450:400a:804::200e:443 to [2a00:1450:400a:804::200e]:443 for IPv6
addresses with attached port number.

Signed-off-by: Lehner Florian <redacted>
---
 misc/ss.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/misc/ss.c b/misc/ss.c
index 12763c9..db39c93 100644
--- a/misc/ss.c
+++ b/misc/ss.c

@@ -1059,7 +1059,11 @@ static void inet_addr_print(const inet_prefix *a,

int port, unsigned int ifindex
 			ap = format_host(AF_INET, 4, a->data);
 		}
 	} else {
-		ap = format_host(a->family, 16, a->data);
+		if (a->family == AF_INET6) {
+			sprintf(buf, "[%s]", format_host(a->family, 16, a->data));
+		} else {
+			ap = format_host(a->family, 16, a->data);
+		}
 		est_len = strlen(ap);
 		if (est_len <= addr_width)
 			est_len = addr_width;  

Note that this will enclosed resolved hostnames in brackets as well, not
sure if that's intended. Looks like fixing that is not exactly trivial:
Hostname resolution is buried in format_host() which resides in
lib/utils.c so is shared code with ip, tc, etc. Hence, adding the
brackets in rt_addr_n2a_r() is not an option, either. Adding a 'bool *'
param to format_host() and format_host_r() indicating that name
resolution has happened might help here.

Cheers, Phil

Also, this code should return "*" for IN6ADDR_ANY

Oh, really? It doesn't do that currently, and I always thought the IPv6
all-zero address was written '::' (or '[::]:1234' if a port is present).

Cheers, Phil

On 07/31/2017 12:30 PM, Phil Sutter wrote:

On Sat, Jul 29, 2017 at 02:29:10PM +0200, Florian Lehner wrote:

quoted

This patch adds support for RFC2732 IPv6 address format with brackets
for the tool ss. So output for ss changes from
2a00:1450:400a:804::200e:443 to [2a00:1450:400a:804::200e]:443 for IPv6
addresses with attached port number.

Signed-off-by: Lehner Florian <redacted>
---
 misc/ss.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/misc/ss.c b/misc/ss.c
index 12763c9..db39c93 100644
--- a/misc/ss.c
+++ b/misc/ss.c

@@ -1059,7 +1059,11 @@ static void inet_addr_print(const inet_prefix *a,

int port, unsigned int ifindex
 			ap = format_host(AF_INET, 4, a->data);
 		}
 	} else {
-		ap = format_host(a->family, 16, a->data);
+		if (a->family == AF_INET6) {
+			sprintf(buf, "[%s]", format_host(a->family, 16, a->data));
+		} else {
+			ap = format_host(a->family, 16, a->data);
+		}
 		est_len = strlen(ap);
 		if (est_len <= addr_width)
 			est_len = addr_width;

Note that this will enclosed resolved hostnames in brackets as well, not
sure if that's intended. Looks like fixing that is not exactly trivial:
Hostname resolution is buried in format_host() which resides in
lib/utils.c so is shared code with ip, tc, etc. Hence, adding the
brackets in rt_addr_n2a_r() is not an option, either. Adding a 'bool *'
param to format_host() and format_host_r() indicating that name
resolution has happened might help here.

Cheers, Phil

By adding a check in the if statement on the global variable
$resolve_hosts the resolved hostnames will not enclosed with brackets.

This updated patch adds support for RFC2732 IPv6 address format with
brackets for the tool ss. Resolved hostnames will not be enclosed in
brackets, therefore the global variable resolve_hosts is initialized and
checked.

Signed-off-by: Lehner Florian <redacted>
---
 misc/ss.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 12763c9..ac94537 100644
--- a/misc/ss.c
+++ b/misc/ss.c

@@ -88,7 +88,7 @@ static int security_get_initial_context(char *name,

char **context)
 }
 #endif

-int resolve_hosts;
+int resolve_hosts = 0;
 int resolve_services = 1;
 int preferred_family = AF_UNSPEC;
 int show_options;

@@ -1059,7 +1059,11 @@ static void inet_addr_print(const inet_prefix *a,

int port, unsigned int ifindex
 			ap = format_host(AF_INET, 4, a->data);
 		}
 	} else {
-		ap = format_host(a->family, 16, a->data);
+		if (a->family == AF_INET6 && !resolve_hosts) {
+			sprintf(buf, "[%s]", format_host(a->family, 16, a->data));
+		} else {
+			ap = format_host(a->family, 16, a->data);
+		}
 		est_len = strlen(ap);
 		if (est_len <= addr_width)
 			est_len = addr_width;
-- 
2.9.4

On Mon, Jul 31, 2017 at 09:50:04PM +0200, Florian Lehner wrote:

Global variables are guaranteed to be initialized to zero. According to
the web this is by C89.

This won't work if name resolution was requested but failed. In that
case an IPv6 address is returned but not enclosed in brackets.

Cheers, Phil

This updated patch adds support for RFC2732 IPv6 address format with
brackets for the tool ss.
It implements the suggestion by Phil Sutter to use a further value,
whether an address was resolved to a hostname.

Signed-off-by: Lehner Florian <redacted>
---
 include/utils.h | 10 +++++++---
 lib/utils.c     | 11 +++++++----
 misc/ss.c       | 20 +++++++++++++++-----
 3 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/include/utils.h b/include/utils.h
index 6080b96..ffacb49 100644
--- a/include/utils.h
+++ b/include/utils.h

@@ -114,9 +114,13 @@ int addr64_n2a(__u64 addr, char *buff, size_t len);
 int af_bit_len(int af);
 int af_byte_len(int af);

-const char *format_host_r(int af, int len, const void *addr,
-			       char *buf, int buflen);
-const char *format_host(int af, int lne, const void *addr);
+const char *format_host_rb(int af, int len, const void *addr,
+			       char *buf, int buflen, bool *resolved);
+#define format_host_r(af, len, addr, buf, buflen) \
+	format_host_rb(af, len, addr, buf, buflen, NULL)
+const char *format_host_b(int af, int lne, const void *addr, bool

*resolved);
+#define format_host(af, lne, addr) \
+	format_host_b(af, lne, addr, NULL)
 #define format_host_rta(af, rta) \
 	format_host(af, RTA_PAYLOAD(rta), RTA_DATA(rta))
 const char *rt_addr_n2a_r(int af, int len, const void *addr,

diff --git a/lib/utils.c b/lib/utils.c
index 9aa3219..42c3bf5 100644
--- a/lib/utils.c
+++ b/lib/utils.c

@@ -898,8 +898,8 @@ static const char *resolve_address(const void *addr,

int len, int af)
 }
 #endif

-const char *format_host_r(int af, int len, const void *addr,
-			char *buf, int buflen)
+const char *format_host_rb(int af, int len, const void *addr,
+			char *buf, int buflen, bool *resolved)
 {
 #ifdef RESOLVE_HOSTNAMES
 	if (resolve_hosts) {

@@ -909,17 +909,20 @@ const char *format_host_r(int af, int len, const

void *addr,

 		if (len > 0 &&
 		    (n = resolve_address(addr, len, af)) != NULL)
+		{
+			*resolved = true;
 			return n;
+		}
 	}
 #endif
 	return rt_addr_n2a_r(af, len, addr, buf, buflen);
 }

-const char *format_host(int af, int len, const void *addr)
+const char *format_host_b(int af, int len, const void *addr, bool
*resolved)
 {
 	static char buf[256];

-	return format_host_r(af, len, addr, buf, 256);
+	return format_host_rb(af, len, addr, buf, 256, resolved);
 }

diff --git a/misc/ss.c b/misc/ss.c
index 12763c9..d37bd1d 100644
--- a/misc/ss.c
+++ b/misc/ss.c

@@ -1046,20 +1046,30 @@ do_numeric:

 static void inet_addr_print(const inet_prefix *a, int port, unsigned

int ifindex)
 {
-	char buf[1024];
-	const char *ap = buf;
+	char b1[1024], b2[1024];
+	const char *ap = b1;
 	int est_len = addr_width;
 	const char *ifname = NULL;
+	bool resolved = false;

 	if (a->family == AF_INET) {
 		if (a->data[0] == 0) {
-			buf[0] = '*';
-			buf[1] = 0;
+			b1[0] = '*';
+			b1[1] = 0;
 		} else {
 			ap = format_host(AF_INET, 4, a->data);
 		}
 	} else {
-		ap = format_host(a->family, 16, a->data);
+		if (a->family == AF_INET6) {
+			ap = format_host_b(a->family, 16, a->data, &resolved);
+			if (!resolved)
+			{
+				sprintf(b2, "[%s]", ap);
+				ap = b2;
+			}
+		} else {
+			ap = format_host(a->family, 16, a->data);
+		}
 		est_len = strlen(ap);
 		if (est_len <= addr_width)
 			est_len = addr_width;
-- 
2.9.4

From: Florian Lehner

...

There are some strange things going on with global variables if this works at all.
The text form of the address is in buf[] in one path and *ap in the other.

One option might be to call format_host() then use strchr(ap, ':')
to add [] if the string contains any ':'.

	David

On Tue, Aug 01, 2017 at 12:05:13PM +0200, Florian Lehner wrote:
[...]

Did you test that? I guess calling format_host() will lead to
dereference of a NULL pointer.

Cheers, Phil

On Tue, 1 Aug 2017 11:11:03 +0000
David Laight [off-list ref] wrote:

From: Florian Lehner

quoted

Sent: 29 July 2017 13:29
This patch adds support for RFC2732 IPv6 address format with brackets
for the tool ss. So output for ss changes from
2a00:1450:400a:804::200e:443 to [2a00:1450:400a:804::200e]:443 for IPv6
addresses with attached port number.

Signed-off-by: Lehner Florian <redacted>
---
 misc/ss.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/misc/ss.c b/misc/ss.c
index 12763c9..db39c93 100644
--- a/misc/ss.c
+++ b/misc/ss.c

@@ -1059,7 +1059,11 @@ static void inet_addr_print(const inet_prefix *a,

int port, unsigned int ifindex
 			ap = format_host(AF_INET, 4, a->data);
 		}
 	} else {
-		ap = format_host(a->family, 16, a->data);
+		if (a->family == AF_INET6) {
+			sprintf(buf, "[%s]", format_host(a->family, 16, a->data));
+		} else {
+			ap = format_host(a->family, 16, a->data);
+		}
 		est_len = strlen(ap);  

...

There are some strange things going on with global variables if this works at all.
The text form of the address is in buf[] in one path and *ap in the other.

One option might be to call format_host() then use strchr(ap, ':')
to add [] if the string contains any ':'.

	David

That sounds like a better solution.

Also what about IN6ADDR_ANY

On 08/01/2017 04:11 PM, Phil Sutter wrote:

On Tue, Aug 01, 2017 at 12:05:13PM +0200, Florian Lehner wrote:
[...]

quoted

@@ -114,9 +114,13 @@ int addr64_n2a(__u64 addr, char *buff, size_t len);
 int af_bit_len(int af);
 int af_byte_len(int af);

-const char *format_host_r(int af, int len, const void *addr,
-			       char *buf, int buflen);
-const char *format_host(int af, int lne, const void *addr);
+const char *format_host_rb(int af, int len, const void *addr,
+			       char *buf, int buflen, bool *resolved);
+#define format_host_r(af, len, addr, buf, buflen) \
+	format_host_rb(af, len, addr, buf, buflen, NULL)
+const char *format_host_b(int af, int lne, const void *addr, bool

*resolved);
+#define format_host(af, lne, addr) \
+	format_host_b(af, lne, addr, NULL)
 #define format_host_rta(af, rta) \
 	format_host(af, RTA_PAYLOAD(rta), RTA_DATA(rta))
 const char *rt_addr_n2a_r(int af, int len, const void *addr,

diff --git a/lib/utils.c b/lib/utils.c
index 9aa3219..42c3bf5 100644
--- a/lib/utils.c
+++ b/lib/utils.c

@@ -898,8 +898,8 @@ static const char *resolve_address(const void *addr,

int len, int af)
 }
 #endif

-const char *format_host_r(int af, int len, const void *addr,
-			char *buf, int buflen)
+const char *format_host_rb(int af, int len, const void *addr,
+			char *buf, int buflen, bool *resolved)
 {
 #ifdef RESOLVE_HOSTNAMES
 	if (resolve_hosts) {

@@ -909,17 +909,20 @@ const char *format_host_r(int af, int len, const

void *addr,

 		if (len > 0 &&
 		    (n = resolve_address(addr, len, af)) != NULL)
+		{
+			*resolved = true;
 			return n;
+		}
 	}
 #endif
 	return rt_addr_n2a_r(af, len, addr, buf, buflen);
 }

Did you test that? I guess calling format_host() will lead to
dereference of a NULL pointer.

Yes, I did. And it just worked.
David Laight suggested to use strchr(). Instead of changing stuff in
lib/* I will try this.

This updated patch adds support for RFC2732 IPv6 address format with
brackets for the tool ss.

Following the advice by David Laight I used strchr().
Also, IN6ADDR_ANY and INADDR_ANY will return "*".


Signed-off-by: Lehner Florian <redacted>
---
 misc/ss.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 12763c9..d40ad00 100644
--- a/misc/ss.c
+++ b/misc/ss.c

@@ -1046,25 +1046,31 @@ do_numeric:

 static void inet_addr_print(const inet_prefix *a, int port, unsigned

int ifindex)
 {
-	char buf[1024];
+	char buf[1024], buf2[1024];
 	const char *ap = buf;
+	char *c = NULL;
 	int est_len = addr_width;
 	const char *ifname = NULL;

-	if (a->family == AF_INET) {
-		if (a->data[0] == 0) {
+	if (a->data[0] == 0) {
 			buf[0] = '*';
 			buf[1] = 0;
-		} else {
+	} else {
+		if (a->family == AF_INET) {
 			ap = format_host(AF_INET, 4, a->data);
+		} else {
+			ap = format_host(a->family, 16, a->data);
+			c = strchr(ap, ':');
+			if (c != NULL && a->family == AF_INET6) {
+				sprintf(buf2, "[%s]", ap);
+				ap = buf2;
+			}
+			est_len = strlen(ap);
+			if (est_len <= addr_width)
+				est_len = addr_width;
+			else
+				est_len = addr_width + ((est_len-addr_width+3)/4)*4;
 		}
-	} else {
-		ap = format_host(a->family, 16, a->data);
-		est_len = strlen(ap);
-		if (est_len <= addr_width)
-			est_len = addr_width;
-		else
-			est_len = addr_width + ((est_len-addr_width+3)/4)*4;
 	}

 	if (ifindex) {
-- 
2.9.4

On Tue, 1 Aug 2017 18:54:33 +0200
Florian Lehner [off-list ref] wrote:

-	if (a->family == AF_INET) {
-		if (a->data[0] == 0) {
+	if (a->data[0] == 0) {
 			buf[0] = '*';
 			buf[1] = 0;

This won't work right with IPv6 you need to look at the whole address being 0
not just a->data[0]

This updated patch adds support for RFC2732 IPv6 address format with
brackets for the tool ss.

Now checking the complete IPv6 address if it is IN6ADDR_ANY.

Signed-off-by: Lehner Florian <redacted>
---
 misc/ss.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 12763c9..83683b5 100644
--- a/misc/ss.c
+++ b/misc/ss.c

@@ -1046,8 +1046,9 @@ do_numeric:

 static void inet_addr_print(const inet_prefix *a, int port, unsigned

int ifindex)
 {
-	char buf[1024];
+	char buf[1024], buf2[1024];
 	const char *ap = buf;
+	char *c = NULL;
 	int est_len = addr_width;
 	const char *ifname = NULL;

@@ -1059,7 +1060,18 @@ static void inet_addr_print(const inet_prefix *a,

int port, unsigned int ifindex
 			ap = format_host(AF_INET, 4, a->data);
 		}
 	} else {
-		ap = format_host(a->family, 16, a->data);
+		if (a->data[0] == 0 && a->data[1] == 0 &&
+		    a->data[2] == 0 && a->data[3] == 0) {
+				buf[0] = '*';
+				buf[1] = 0;
+		} else {
+			ap = format_host(a->family, 16, a->data);
+			c = strchr(ap, ':');
+			if (c != NULL && a->family == AF_INET6) {
+				sprintf(buf2, "[%s]", ap);
+				ap = buf2;
+			}
+		}
 		est_len = strlen(ap);
 		if (est_len <= addr_width)
 			est_len = addr_width;
-- 
2.9.4

On Fri, 4 Aug 2017 20:02:52 +0200
Florian Lehner [off-list ref] wrote:

Your email client is wrapping long lines which leads to malformed patch.

You didn't need buf2, and the code was more complex than it needed to be.

Rather than waiting for yet another version, I just merged in similar
code.

On Fri, 2017-08-04 at 12:05 -0700, Stephen Hemminger wrote:

On Fri, 4 Aug 2017 20:02:52 +0200
Florian Lehner [off-list ref] wrote:

quoted

diff --git a/misc/ss.c b/misc/ss.c
index 12763c9..83683b5 100644
--- a/misc/ss.c
+++ b/misc/ss.c

@@ -1046,8 +1046,9 @@ do_numeric:

 static void inet_addr_print(const inet_prefix *a, int port, unsigned

int ifindex)
 {

Your email client is wrapping long lines which leads to malformed patch.

You didn't need buf2, and the code was more complex than it needed to be.

Rather than waiting for yet another version, I just merged in similar
code.

Also, is this new format accepted for the filter ?

ss src [::1]:22

On Fri, 04 Aug 2017 13:46:27 -0700
Eric Dumazet [off-list ref] wrote:

On Fri, 2017-08-04 at 12:05 -0700, Stephen Hemminger wrote:

quoted

On Fri, 4 Aug 2017 20:02:52 +0200
Florian Lehner [off-list ref] wrote:
  

quoted

diff --git a/misc/ss.c b/misc/ss.c
index 12763c9..83683b5 100644
--- a/misc/ss.c
+++ b/misc/ss.c

@@ -1046,8 +1046,9 @@ do_numeric:

 static void inet_addr_print(const inet_prefix *a, int port, unsigned

int ifindex)
 {  

Your email client is wrapping long lines which leads to malformed patch.

You didn't need buf2, and the code was more complex than it needed to be.

Rather than waiting for yet another version, I just merged in similar
code.  

Also, is this new format accepted for the filter ?

ss src [::1]:22

It looks like ss would always do that (from earliest git version).