[PATCH] util, tcp: Add helper to display socket addresses

[PATCH] util, tcp: Add helper to display socket addresses

[David Gibson]

When reporting errors, we sometimes want to show a relevant socket address.
Doing so by extracting the various relevant fields can be pretty awkward,
so introduce a sockaddr_ntop() helper to make it simpler.  For now we just
have one user in tcp.c, but I have further upcoming patches which can make
use of it.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 tcp.c  | 23 +++++++++--------------
 util.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 util.h |  4 ++++
 3 files changed, 57 insertions(+), 14 deletions(-)

diff --git a/tcp.c b/tcp.c
index 21d0af0..efbbc1c 100644
--- a/tcp.c
+++ b/tcp.c
@@ -2758,6 +2758,7 @@ static void tcp_tap_conn_from_sock(struct ctx *c, in_port_t dstport,
 void tcp_listen_handler(struct ctx *c, union epoll_ref ref,
 			const struct timespec *now)
 {
+	char sastr[SOCKADDR_STRLEN];
 	union sockaddr_inany sa;
 	socklen_t sl = sizeof(sa);
 	union flow *flow;
@@ -2776,25 +2777,15 @@ void tcp_listen_handler(struct ctx *c, union epoll_ref ref,
 
 		if (IN4_IS_ADDR_UNSPECIFIED(addr) ||
 		    IN4_IS_ADDR_BROADCAST(addr) ||
-		    IN4_IS_ADDR_MULTICAST(addr) || port == 0) {
-			char str[INET_ADDRSTRLEN];
-
-			err("Invalid endpoint from TCP accept(): %s:%hu",
-			    inet_ntop(AF_INET, addr, str, sizeof(str)), port);
-			goto cancel;
-		}
+		    IN4_IS_ADDR_MULTICAST(addr) || port == 0)
+			goto bad_endpoint;
 	} else if (sa.sa_family == AF_INET6) {
 		const struct in6_addr *addr = &sa.sa6.sin6_addr;
 		in_port_t port = sa.sa6.sin6_port;
 
 		if (IN6_IS_ADDR_UNSPECIFIED(addr) ||
-		    IN6_IS_ADDR_MULTICAST(addr) || port == 0) {
-			char str[INET6_ADDRSTRLEN];
-
-			err("Invalid endpoint from TCP accept(): %s:%hu",
-			    inet_ntop(AF_INET6, addr, str, sizeof(str)), port);
-			goto cancel;
-		}
+		    IN6_IS_ADDR_MULTICAST(addr) || port == 0)
+			goto bad_endpoint;
 	}
 
 	if (tcp_splice_conn_from_sock(c, ref.tcp_listen.pif,
@@ -2804,6 +2795,10 @@ void tcp_listen_handler(struct ctx *c, union epoll_ref ref,
 	tcp_tap_conn_from_sock(c, ref.tcp_listen.port, flow, s, &sa, now);
 	return;
 
+bad_endpoint:
+	err("Invalid endpoint from TCP accept(): %s",
+	    sockaddr_ntop(&sa, sastr, sizeof(sastr)));
+
 cancel:
 	flow_alloc_cancel(flow);
 }
diff --git a/util.c b/util.c
index 849fa7f..c78a5f3 100644
--- a/util.c
+++ b/util.c
@@ -553,3 +553,47 @@ int write_remainder(int fd, const struct iovec *iov, int iovcnt, size_t skip)
 
 	return 0;
 }
+
+
+/** sockaddr_ntop - Convert a socket address to text format
+ * @src:	Socket address
+ * @dst:	output buffer, minimum SOCKADDR_STRLEN bytes
+ * @size:	size of buffer at @dst
+ *
+ * Return: On success, a non-null pointer to @dst, NULL on failure
+ */
+const char *sockaddr_ntop(const void *sa, char *dst, socklen_t size)
+{
+	sa_family_t family = ((const struct sockaddr *)sa)->sa_family;
+
+	switch (family) {
+	case AF_INET: {
+		const struct sockaddr_in *sa4 = sa;
+		char addr[INET_ADDRSTRLEN];
+
+		if (!inet_ntop(AF_INET, &sa4->sin_addr, addr, sizeof(addr)))
+			break;
+
+		snprintf(dst, size, "%s:%hu", addr, ntohs(sa4->sin_port));
+		return dst;
+	}
+
+	case AF_INET6: {
+		const struct sockaddr_in6 *sa6 = sa;
+		char addr[INET6_ADDRSTRLEN];
+
+		if (!inet_ntop(AF_INET6, &sa6->sin6_addr, addr, sizeof(addr)))
+			break;
+
+		snprintf(dst, size, "[%s]:%hu", addr, ntohs(sa6->sin6_port));
+		return dst;
+	}
+
+		/* FIXME: Implement AF_UNIX */
+	default:
+		errno = EAFNOSUPPORT;
+	}
+
+	/* Failure */
+	return NULL;
+}
diff --git a/util.h b/util.h
index 264423b..2c7154e 100644
--- a/util.h
+++ b/util.h
@@ -180,6 +180,10 @@ static inline const char *af_name(sa_family_t af)
 	}
 }
 
+#define SOCKADDR_STRLEN		MAX(INET_ADDRSTRLEN + 6,	\
+				    INET6_ADDRSTRLEN + 8)
+const char *sockaddr_ntop(const void *sa, char *dst, socklen_t size);
+
 /**
  * mod_sub() - Modular arithmetic subtraction
  * @a:		Minued, unsigned value < @m
-- 
@@ -180,6 +180,10 @@ static inline const char *af_name(sa_family_t af)
 	}
 }
 
+#define SOCKADDR_STRLEN		MAX(INET_ADDRSTRLEN + 6,	\
+				    INET6_ADDRSTRLEN + 8)
+const char *sockaddr_ntop(const void *sa, char *dst, socklen_t size);
+
 /**
  * mod_sub() - Modular arithmetic subtraction
  * @a:		Minued, unsigned value < @m
-- 
2.45.0

Re: [PATCH] util, tcp: Add helper to display socket addresses

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 698 bytes --]

On Thu, May 16, 2024 at 03:25:56PM +1000, David Gibson wrote:
> When reporting errors, we sometimes want to show a relevant socket address.
> Doing so by extracting the various relevant fields can be pretty awkward,
> so introduce a sockaddr_ntop() helper to make it simpler.  For now we just
> have one user in tcp.c, but I have further upcoming patches which can make
> use of it.

Realised I had an off-by-one error in my calculation of the max buffer
size for this.  Fixed version coming.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] util, tcp: Add helper to display socket addresses

[Stefano Brivio]

On Thu, 16 May 2024 16:54:24 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Thu, May 16, 2024 at 03:25:56PM +1000, David Gibson wrote:
> > When reporting errors, we sometimes want to show a relevant socket address.
> > Doing so by extracting the various relevant fields can be pretty awkward,
> > so introduce a sockaddr_ntop() helper to make it simpler.  For now we just
> > have one user in tcp.c, but I have further upcoming patches which can make
> > use of it.  
> 
> Realised I had an off-by-one error in my calculation of the max buffer
> size for this.  Fixed version coming.

While at it, I was also wondering: would it be reasonably feasible,
without killing readability, to get inet_ntop() to write directly to
the buffer? I haven't tried to do it though.

-- 
Stefano