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(a)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 -- 2.45.0
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
On Thu, 16 May 2024 16:54:24 +1000 David Gibson <david(a)gibson.dropbear.id.au> wrote:On Thu, May 16, 2024 at 03:25:56PM +1000, David Gibson wrote: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. -- StefanoWhen 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.