[PATCH iproute2-next] ss: Make leading ":" always optional for sport and dport

[PATCH iproute2-next] ss: Make leading ":" always optional for sport and dport

[Thayne McCombs]

The sport and dport conditions in expressions were inconsistent on
whether there should be a ":" at the beginning of the port when only a
port was provided depending on the family. The link and netlink
families required a ":" to work. The vsock family required the ":"
to be absent. The inet and inet6 families work with or without a leading
":".

This makes the leading ":" optional in all cases, so if sport or dport
are used, then it works with a leading ":" or without one, as inet and
inet6 did.
---
 misc/ss.c | 46 ++++++++++++++++++++++++----------------------
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index aefa1c2f..5c934fa0 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -2111,6 +2111,18 @@ static void vsock_set_inet_prefix(inet_prefix *a, __u32 cid)
 	memcpy(a->data, &cid, sizeof(cid));
 }
 
+static char* find_port(char *addr, bool is_port)
+{
+	char *port = NULL;
+	if (is_port)
+		port = addr;
+	else
+		port = strchr(addr, ':');
+	if (port && *port == ':')
+		*port++ = '\0';
+	return port;
+}
+
 void *parse_hostcond(char *addr, bool is_port)
 {
 	char *port = NULL;
@@ -2152,17 +2164,16 @@ void *parse_hostcond(char *addr, bool is_port)
 	if (fam == AF_PACKET) {
 		a.addr.family = AF_PACKET;
 		a.addr.bitlen = 0;
-		port = strchr(addr, ':');
+		port = find_port(addr, is_port);
 		if (port) {
-			*port = 0;
-			if (port[1] && strcmp(port+1, "*")) {
-				if (get_integer(&a.port, port+1, 0)) {
-					if ((a.port = xll_name_to_index(port+1)) <= 0)
+			if (*port && strcmp(port, "*")) {
+				if (get_integer(&a.port, port, 0)) {
+					if ((a.port = xll_name_to_index(port)) <= 0)
 						return NULL;
 				}
 			}
 		}
-		if (addr[0] && strcmp(addr, "*")) {
+		if (!is_port && addr[0] && strcmp(addr, "*")) {
 			unsigned short tmp;
 
 			a.addr.bitlen = 32;
@@ -2176,19 +2187,18 @@ void *parse_hostcond(char *addr, bool is_port)
 	if (fam == AF_NETLINK) {
 		a.addr.family = AF_NETLINK;
 		a.addr.bitlen = 0;
-		port = strchr(addr, ':');
+		port = find_port(addr, is_port);
 		if (port) {
-			*port = 0;
-			if (port[1] && strcmp(port+1, "*")) {
-				if (get_integer(&a.port, port+1, 0)) {
-					if (strcmp(port+1, "kernel") == 0)
+			if (*port && strcmp(port, "*")) {
+				if (get_integer(&a.port, port, 0)) {
+					if (strcmp(port, "kernel") == 0)
 						a.port = 0;
 					else
 						return NULL;
 				}
 			}
 		}
-		if (addr[0] && strcmp(addr, "*")) {
+		if (!is_port && addr[0] && strcmp(addr, "*")) {
 			a.addr.bitlen = 32;
 			if (nl_proto_a2n(&a.addr.data[0], addr) == -1)
 				return NULL;
@@ -2201,21 +2211,13 @@ void *parse_hostcond(char *addr, bool is_port)
 
 		a.addr.family = AF_VSOCK;
 
-		if (is_port)
-			port = addr;
-		else {
-			port = strchr(addr, ':');
-			if (port) {
-				*port = '\0';
-				port++;
-			}
-		}
+		port = find_port(addr, is_port);
 
 		if (port && strcmp(port, "*") &&
 		    get_u32((__u32 *)&a.port, port, 0))
 			return NULL;
 
-		if (addr[0] && strcmp(addr, "*")) {
+		if (!is_port && addr[0] && strcmp(addr, "*")) {
 			a.addr.bitlen = 32;
 			if (get_u32(&cid, addr, 0))
 				return NULL;
-- 
2.30.0

Re: [PATCH iproute2-next] ss: Make leading ":" always optional for sport and dport

[David Ahern]

On 2/5/21 11:36 PM, Thayne McCombs wrote:
> The sport and dport conditions in expressions were inconsistent on
> whether there should be a ":" at the beginning of the port when only a
> port was provided depending on the family. The link and netlink
> families required a ":" to work. The vsock family required the ":"
> to be absent. The inet and inet6 families work with or without a leading
> ":".
> 
> This makes the leading ":" optional in all cases, so if sport or dport
> are used, then it works with a leading ":" or without one, as inet and
> inet6 did.
> ---
>  misc/ss.c | 46 ++++++++++++++++++++++++----------------------
>  1 file changed, 24 insertions(+), 22 deletions(-)
> 

change looks fine, but you are missing a sign off.

[PATCH] ss: Make leading ":" always optional for sport and dport

[Thayne McCombs]

Doh! Sorry about that, here it is with the sign-off.

-- >8 --

The sport and dport conditions in expressions were inconsistent on
whether there should be a ":" at the beginning of the port when only a
port was provided depending on the family. The link and netlink
families required a ":" to work. The vsock family required the ":"
to be absent. The inet and inet6 families work with or without a leading
":".

This makes the leading ":" optional in all cases, so if sport or dport
are used, then it works with a leading ":" or without one, as inet and
inet6 did.

Signed-off-by: Thayne McCombs <astrothayne@gmail.com>
---
 misc/ss.c | 46 ++++++++++++++++++++++++----------------------
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index aefa1c2f..5c934fa0 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -2111,6 +2111,18 @@ static void vsock_set_inet_prefix(inet_prefix *a, __u32 cid)
 	memcpy(a->data, &cid, sizeof(cid));
 }
 
+static char* find_port(char *addr, bool is_port)
+{
+	char *port = NULL;
+	if (is_port)
+		port = addr;
+	else
+		port = strchr(addr, ':');
+	if (port && *port == ':')
+		*port++ = '\0';
+	return port;
+}
+
 void *parse_hostcond(char *addr, bool is_port)
 {
 	char *port = NULL;
@@ -2152,17 +2164,16 @@ void *parse_hostcond(char *addr, bool is_port)
 	if (fam == AF_PACKET) {
 		a.addr.family = AF_PACKET;
 		a.addr.bitlen = 0;
-		port = strchr(addr, ':');
+		port = find_port(addr, is_port);
 		if (port) {
-			*port = 0;
-			if (port[1] && strcmp(port+1, "*")) {
-				if (get_integer(&a.port, port+1, 0)) {
-					if ((a.port = xll_name_to_index(port+1)) <= 0)
+			if (*port && strcmp(port, "*")) {
+				if (get_integer(&a.port, port, 0)) {
+					if ((a.port = xll_name_to_index(port)) <= 0)
 						return NULL;
 				}
 			}
 		}
-		if (addr[0] && strcmp(addr, "*")) {
+		if (!is_port && addr[0] && strcmp(addr, "*")) {
 			unsigned short tmp;
 
 			a.addr.bitlen = 32;
@@ -2176,19 +2187,18 @@ void *parse_hostcond(char *addr, bool is_port)
 	if (fam == AF_NETLINK) {
 		a.addr.family = AF_NETLINK;
 		a.addr.bitlen = 0;
-		port = strchr(addr, ':');
+		port = find_port(addr, is_port);
 		if (port) {
-			*port = 0;
-			if (port[1] && strcmp(port+1, "*")) {
-				if (get_integer(&a.port, port+1, 0)) {
-					if (strcmp(port+1, "kernel") == 0)
+			if (*port && strcmp(port, "*")) {
+				if (get_integer(&a.port, port, 0)) {
+					if (strcmp(port, "kernel") == 0)
 						a.port = 0;
 					else
 						return NULL;
 				}
 			}
 		}
-		if (addr[0] && strcmp(addr, "*")) {
+		if (!is_port && addr[0] && strcmp(addr, "*")) {
 			a.addr.bitlen = 32;
 			if (nl_proto_a2n(&a.addr.data[0], addr) == -1)
 				return NULL;
@@ -2201,21 +2211,13 @@ void *parse_hostcond(char *addr, bool is_port)
 
 		a.addr.family = AF_VSOCK;
 
-		if (is_port)
-			port = addr;
-		else {
-			port = strchr(addr, ':');
-			if (port) {
-				*port = '\0';
-				port++;
-			}
-		}
+		port = find_port(addr, is_port);
 
 		if (port && strcmp(port, "*") &&
 		    get_u32((__u32 *)&a.port, port, 0))
 			return NULL;
 
-		if (addr[0] && strcmp(addr, "*")) {
+		if (!is_port && addr[0] && strcmp(addr, "*")) {
 			a.addr.bitlen = 32;
 			if (get_u32(&cid, addr, 0))
 				return NULL;
-- 
2.30.1

Re: [PATCH] ss: Make leading ":" always optional for sport and dport

[David Ahern]

On 2/14/21 1:09 AM, Thayne McCombs wrote:
> Doh! Sorry about that, here it is with the sign-off.
> 
> -- >8 --

Don't put text here.

> 
> The sport and dport conditions in expressions were inconsistent on
> whether there should be a ":" at the beginning of the port when only a
> port was provided depending on the family. The link and netlink
> families required a ":" to work. The vsock family required the ":"
> to be absent. The inet and inet6 families work with or without a leading
> ":".
> 
> This makes the leading ":" optional in all cases, so if sport or dport
> are used, then it works with a leading ":" or without one, as inet and
> inet6 did.
> 
> Signed-off-by: Thayne McCombs <astrothayne@gmail.com>
> ---

put extra comments here

>  misc/ss.c | 46 ++++++++++++++++++++++++----------------------
>  1 file changed, 24 insertions(+), 22 deletions(-)
> 

Also, put iproute2-next in the subject line along with a version number.

applied to iproute2-next.

Re: [PATCH] ss: Make leading ":" always optional for sport and dport

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to iproute2/iproute2-next.git (refs/heads/main):

On Sun, 14 Feb 2021 01:09:13 -0700 you wrote:
> Doh! Sorry about that, here it is with the sign-off.
> 
> -- >8 --
> 
> The sport and dport conditions in expressions were inconsistent on
> whether there should be a ":" at the beginning of the port when only a
> port was provided depending on the family. The link and netlink
> families required a ":" to work. The vsock family required the ":"
> to be absent. The inet and inet6 families work with or without a leading
> ":".
> 
> [...]

Here is the summary with links:
  - ss: Make leading ":" always optional for sport and dport
    https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=c7897ec2a68b

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html