* [PATCH 1/4] udp: Don't attempt to get dual-stack sockets in nonsensical cases
2024-09-20 4:12 [PATCH 0/4] Use inany addresses through port forwarding configuration David Gibson
@ 2024-09-20 4:12 ` David Gibson
2024-09-20 4:12 ` [PATCH 2/4] util, pif: Replace sock_l4() with pif_sock_l4() David Gibson
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: David Gibson @ 2024-09-20 4:12 UTC (permalink / raw)
To: Stefano Brivio, passt-dev; +Cc: David Gibson
To save some kernel memory we try to use "dual stack" sockets (that listen
to both IPv4 and IPv6 traffic) when possible. However udp_sock_init()
attempts to do this in some cases that can't work. Specifically we can
only do this when listening on any address. That's never true for the
ns (splicing) case, because we always listen on loopback. For the !ns
case and AF_UNSPEC case, addr should always be NULL, but add an assert to
verify.
This is harmless: if addr is non-NULL, sock_l4() will just fail and we'll
fall back to the other path. But, it's messy and makes some upcoming
changes harder, so avoid attempting this in cases we know can't work.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
udp.c | 19 +++++++------------
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/udp.c b/udp.c
index 7b283138..8cea80cd 100644
--- a/udp.c
+++ b/udp.c
@@ -803,21 +803,16 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
ASSERT(!c->no_udp);
- if (af == AF_UNSPEC && c->ifi4 && c->ifi6) {
+ if (af == AF_UNSPEC && c->ifi4 && c->ifi6 && !ns) {
int s;
+ ASSERT(!addr);
+
/* Attempt to get a dual stack socket */
- if (!ns) {
- s = sock_l4(c, AF_UNSPEC, EPOLL_TYPE_UDP_LISTEN,
- addr, ifname, port, uref.u32);
- udp_splice_init[V4][port] = s < 0 ? -1 : s;
- udp_splice_init[V6][port] = s < 0 ? -1 : s;
- } else {
- s = sock_l4(c, AF_UNSPEC, EPOLL_TYPE_UDP_LISTEN,
- &in4addr_loopback, ifname, port, uref.u32);
- udp_splice_ns[V4][port] = s < 0 ? -1 : s;
- udp_splice_ns[V6][port] = s < 0 ? -1 : s;
- }
+ s = sock_l4(c, AF_UNSPEC, EPOLL_TYPE_UDP_LISTEN,
+ NULL, ifname, port, uref.u32);
+ udp_splice_init[V4][port] = s < 0 ? -1 : s;
+ udp_splice_init[V6][port] = s < 0 ? -1 : s;
if (IN_INTERVAL(0, FD_REF_MAX, s))
return 0;
}
--
@@ -803,21 +803,16 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
ASSERT(!c->no_udp);
- if (af == AF_UNSPEC && c->ifi4 && c->ifi6) {
+ if (af == AF_UNSPEC && c->ifi4 && c->ifi6 && !ns) {
int s;
+ ASSERT(!addr);
+
/* Attempt to get a dual stack socket */
- if (!ns) {
- s = sock_l4(c, AF_UNSPEC, EPOLL_TYPE_UDP_LISTEN,
- addr, ifname, port, uref.u32);
- udp_splice_init[V4][port] = s < 0 ? -1 : s;
- udp_splice_init[V6][port] = s < 0 ? -1 : s;
- } else {
- s = sock_l4(c, AF_UNSPEC, EPOLL_TYPE_UDP_LISTEN,
- &in4addr_loopback, ifname, port, uref.u32);
- udp_splice_ns[V4][port] = s < 0 ? -1 : s;
- udp_splice_ns[V6][port] = s < 0 ? -1 : s;
- }
+ s = sock_l4(c, AF_UNSPEC, EPOLL_TYPE_UDP_LISTEN,
+ NULL, ifname, port, uref.u32);
+ udp_splice_init[V4][port] = s < 0 ? -1 : s;
+ udp_splice_init[V6][port] = s < 0 ? -1 : s;
if (IN_INTERVAL(0, FD_REF_MAX, s))
return 0;
}
--
2.46.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/4] util, pif: Replace sock_l4() with pif_sock_l4()
2024-09-20 4:12 [PATCH 0/4] Use inany addresses through port forwarding configuration David Gibson
2024-09-20 4:12 ` [PATCH 1/4] udp: Don't attempt to get dual-stack sockets in nonsensical cases David Gibson
@ 2024-09-20 4:12 ` David Gibson
2024-09-20 4:12 ` [PATCH 3/4] tcp, udp: Make {tcp,udp}_sock_init() take an inany address David Gibson
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: David Gibson @ 2024-09-20 4:12 UTC (permalink / raw)
To: Stefano Brivio, passt-dev; +Cc: David Gibson
The sock_l4() function is very convenient for creating sockets bound to
a given address, but its interface has some problems.
Most importantly, the address and port alone aren't enough in some cases.
For link-local addresses (at least) we also need the pif in order to
properly construct a socket adddress. This case doesn't yet arise, but
it might cause us trouble in future.
Additionally, sock_l4() can take AF_UNSPEC with the special meaning that it
should attempt to create a "dual stack" socket which will respond to both
IPv4 and IPv6 traffic. This only makes sense if there is no specific
address given. We verify this at runtime, but it would be nicer if we
could enforce it structurally.
For sockets associated specifically with a single flow we already replaced
sock_l4() with flowside_sock_l4() which avoids those problems. Now,
replace all the remaining users with a new pif_sock_l4() which also takes
an explicit pif.
The new function takes the address as an inany *, with NULL indicating the
dual stack case. This does add some complexity in some of the callers,
however future planned cleanups should make this go away again.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
pif.c | 42 ++++++++++++++++++++++++++++++++++++++++++
pif.h | 3 +++
tcp.c | 22 +++++++++++++++++-----
udp.c | 34 ++++++++++++++++++++++------------
util.c | 52 ----------------------------------------------------
util.h | 3 ---
6 files changed, 84 insertions(+), 72 deletions(-)
diff --git a/pif.c b/pif.c
index a099e31b..592fafaa 100644
--- a/pif.c
+++ b/pif.c
@@ -59,3 +59,45 @@ void pif_sockaddr(const struct ctx *c, union sockaddr_inany *sa, socklen_t *sl,
*sl = sizeof(sa->sa6);
}
}
+
+/** pif_sock_l4() - Open a socket bound to an address on a specified interface
+ * @c: Execution context
+ * @type: Socket epoll type
+ * @pif: Interface for this socket
+ * @addr: Address to bind to, or NULL for dual-stack any
+ * @ifname: Interface for binding, NULL for any
+ * @port: Port number to bind to (host byte order)
+ * @data: epoll reference portion for protocol handlers
+ *
+ * NOTE: For namespace pifs, this must be called having already entered the
+ * relevant namespace.
+ *
+ * Return: newly created socket, negative error code on failure
+ */
+int pif_sock_l4(const struct ctx *c, enum epoll_type type, uint8_t pif,
+ const union inany_addr *addr, const char *ifname,
+ in_port_t port, uint32_t data)
+{
+ union sockaddr_inany sa = {
+ .sa6.sin6_family = AF_INET6,
+ .sa6.sin6_addr = in6addr_any,
+ .sa6.sin6_port = htons(port),
+ };
+ socklen_t sl;
+
+ ASSERT(pif_is_socket(pif));
+
+ if (pif == PIF_SPLICE) {
+ /* Sanity checks */
+ ASSERT(!ifname);
+ ASSERT(addr && inany_is_loopback(addr));
+ }
+
+ if (!addr)
+ return sock_l4_sa(c, type, &sa, sizeof(sa.sa6),
+ ifname, false, data);
+
+ pif_sockaddr(c, &sa, &sl, pif, addr, port);
+ return sock_l4_sa(c, type, &sa, sl,
+ ifname, sa.sa_family == AF_INET6, data);
+}
diff --git a/pif.h b/pif.h
index 8777bb5b..f029282b 100644
--- a/pif.h
+++ b/pif.h
@@ -59,5 +59,8 @@ static inline bool pif_is_socket(uint8_t pif)
void pif_sockaddr(const struct ctx *c, union sockaddr_inany *sa, socklen_t *sl,
uint8_t pif, const union inany_addr *addr, in_port_t port);
+int pif_sock_l4(const struct ctx *c, enum epoll_type type, uint8_t pif,
+ const union inany_addr *addr, const char *ifname,
+ in_port_t port, uint32_t data);
#endif /* PIF_H */
diff --git a/tcp.c b/tcp.c
index 1962fcc4..49e0cfe3 100644
--- a/tcp.c
+++ b/tcp.c
@@ -2291,7 +2291,19 @@ static int tcp_sock_init_af(const struct ctx *c, sa_family_t af, in_port_t port,
};
int s;
- s = sock_l4(c, af, EPOLL_TYPE_TCP_LISTEN, addr, ifname, port, tref.u32);
+ if (af == AF_UNSPEC) {
+ ASSERT(!addr);
+ s = pif_sock_l4(c, EPOLL_TYPE_TCP_LISTEN, PIF_HOST, NULL,
+ ifname, port, tref.u32);
+ } else {
+ union inany_addr aany = af == AF_INET ? inany_any4 : inany_any6;
+
+ if (addr)
+ inany_from_af(&aany, af, addr);
+
+ s = pif_sock_l4(c, EPOLL_TYPE_TCP_LISTEN, PIF_HOST, &aany,
+ ifname, port, tref.u32);
+ }
if (c->tcp.fwd_in.mode == FWD_AUTO) {
if (af == AF_INET || af == AF_UNSPEC)
@@ -2357,8 +2369,8 @@ static void tcp_ns_sock_init4(const struct ctx *c, in_port_t port)
ASSERT(c->mode == MODE_PASTA);
- s = sock_l4(c, AF_INET, EPOLL_TYPE_TCP_LISTEN, &in4addr_loopback,
- NULL, port, tref.u32);
+ s = pif_sock_l4(c, EPOLL_TYPE_TCP_LISTEN, PIF_SPLICE, &inany_loopback4,
+ NULL, port, tref.u32);
if (s >= 0)
tcp_sock_set_bufsize(c, s);
else
@@ -2383,8 +2395,8 @@ static void tcp_ns_sock_init6(const struct ctx *c, in_port_t port)
ASSERT(c->mode == MODE_PASTA);
- s = sock_l4(c, AF_INET6, EPOLL_TYPE_TCP_LISTEN, &in6addr_loopback,
- NULL, port, tref.u32);
+ s = pif_sock_l4(c, EPOLL_TYPE_TCP_LISTEN, PIF_SPLICE, &inany_loopback6,
+ NULL, port, tref.u32);
if (s >= 0)
tcp_sock_set_bufsize(c, s);
else
diff --git a/udp.c b/udp.c
index 8cea80cd..b3d4a64c 100644
--- a/udp.c
+++ b/udp.c
@@ -809,8 +809,8 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
ASSERT(!addr);
/* Attempt to get a dual stack socket */
- s = sock_l4(c, AF_UNSPEC, EPOLL_TYPE_UDP_LISTEN,
- NULL, ifname, port, uref.u32);
+ s = pif_sock_l4(c, EPOLL_TYPE_UDP_LISTEN, PIF_HOST,
+ NULL, ifname, port, uref.u32);
udp_splice_init[V4][port] = s < 0 ? -1 : s;
udp_splice_init[V6][port] = s < 0 ? -1 : s;
if (IN_INTERVAL(0, FD_REF_MAX, s))
@@ -819,28 +819,38 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
if ((af == AF_INET || af == AF_UNSPEC) && c->ifi4) {
if (!ns) {
- r4 = sock_l4(c, AF_INET, EPOLL_TYPE_UDP_LISTEN,
- addr, ifname, port, uref.u32);
+ union inany_addr aany = inany_any4;
+
+ if (addr)
+ inany_from_af(&aany, AF_INET, addr);
+
+ r4 = pif_sock_l4(c, EPOLL_TYPE_UDP_LISTEN, PIF_HOST,
+ &aany, ifname, port, uref.u32);
udp_splice_init[V4][port] = r4 < 0 ? -1 : r4;
} else {
- r4 = sock_l4(c, AF_INET, EPOLL_TYPE_UDP_LISTEN,
- &in4addr_loopback,
- ifname, port, uref.u32);
+ r4 = pif_sock_l4(c, EPOLL_TYPE_UDP_LISTEN, PIF_SPLICE,
+ &inany_loopback4, ifname,
+ port, uref.u32);
udp_splice_ns[V4][port] = r4 < 0 ? -1 : r4;
}
}
if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6) {
if (!ns) {
- r6 = sock_l4(c, AF_INET6, EPOLL_TYPE_UDP_LISTEN,
- addr, ifname, port, uref.u32);
+ union inany_addr aany = inany_any6;
+
+ if (addr)
+ inany_from_af(&aany, AF_INET6, addr);
+
+ r6 = pif_sock_l4(c, EPOLL_TYPE_UDP_LISTEN, PIF_HOST,
+ &aany, ifname, port, uref.u32);
udp_splice_init[V6][port] = r6 < 0 ? -1 : r6;
} else {
- r6 = sock_l4(c, AF_INET6, EPOLL_TYPE_UDP_LISTEN,
- &in6addr_loopback,
- ifname, port, uref.u32);
+ r6 = pif_sock_l4(c, EPOLL_TYPE_UDP_LISTEN, PIF_SPLICE,
+ &inany_loopback6, ifname,
+ port, uref.u32);
udp_splice_ns[V6][port] = r6 < 0 ? -1 : r6;
}
}
diff --git a/util.c b/util.c
index 87309c51..ebd93ed2 100644
--- a/util.c
+++ b/util.c
@@ -157,58 +157,6 @@ int sock_l4_sa(const struct ctx *c, enum epoll_type type,
return fd;
}
-/**
- * sock_l4() - Create and bind socket for given L4, add to epoll list
- * @c: Execution context
- * @af: Address family, AF_INET or AF_INET6
- * @type: epoll type
- * @bind_addr: Address for binding, NULL for any
- * @ifname: Interface for binding, NULL for any
- * @port: Port, host order
- * @data: epoll reference portion for protocol handlers
- *
- * Return: newly created socket, negative error code on failure
- */
-int sock_l4(const struct ctx *c, sa_family_t af, enum epoll_type type,
- const void *bind_addr, const char *ifname, uint16_t port,
- uint32_t data)
-{
- switch (af) {
- case AF_INET: {
- struct sockaddr_in addr4 = {
- .sin_family = AF_INET,
- .sin_port = htons(port),
- { 0 }, { 0 },
- };
- if (bind_addr)
- addr4.sin_addr = *(struct in_addr *)bind_addr;
- return sock_l4_sa(c, type, &addr4, sizeof(addr4), ifname,
- false, data);
- }
-
- case AF_UNSPEC:
- if (!DUAL_STACK_SOCKETS || bind_addr)
- return -EINVAL;
- /* fallthrough */
- case AF_INET6: {
- struct sockaddr_in6 addr6 = {
- .sin6_family = AF_INET6,
- .sin6_port = htons(port),
- 0, IN6ADDR_ANY_INIT, 0,
- };
- if (bind_addr) {
- addr6.sin6_addr = *(struct in6_addr *)bind_addr;
-
- if (IN6_IS_ADDR_LINKLOCAL(bind_addr))
- addr6.sin6_scope_id = c->ifi6;
- }
- return sock_l4_sa(c, type, &addr6, sizeof(addr6), ifname,
- af == AF_INET6, data);
- }
- default:
- return -EINVAL;
- }
-}
/**
* sock_probe_mem() - Check if setting high SO_SNDBUF and SO_RCVBUF is allowed
diff --git a/util.h b/util.h
index 5e67f1fa..2c1e08e0 100644
--- a/util.h
+++ b/util.h
@@ -181,9 +181,6 @@ int close_range(unsigned int first, unsigned int last, int flags) {
int sock_l4_sa(const struct ctx *c, enum epoll_type type,
const void *sa, socklen_t sl,
const char *ifname, bool v6only, uint32_t data);
-int sock_l4(const struct ctx *c, sa_family_t af, enum epoll_type type,
- const void *bind_addr, const char *ifname, uint16_t port,
- uint32_t data);
void sock_probe_mem(struct ctx *c);
long timespec_diff_ms(const struct timespec *a, const struct timespec *b);
int64_t timespec_diff_us(const struct timespec *a, const struct timespec *b);
--
@@ -181,9 +181,6 @@ int close_range(unsigned int first, unsigned int last, int flags) {
int sock_l4_sa(const struct ctx *c, enum epoll_type type,
const void *sa, socklen_t sl,
const char *ifname, bool v6only, uint32_t data);
-int sock_l4(const struct ctx *c, sa_family_t af, enum epoll_type type,
- const void *bind_addr, const char *ifname, uint16_t port,
- uint32_t data);
void sock_probe_mem(struct ctx *c);
long timespec_diff_ms(const struct timespec *a, const struct timespec *b);
int64_t timespec_diff_us(const struct timespec *a, const struct timespec *b);
--
2.46.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/4] tcp, udp: Make {tcp,udp}_sock_init() take an inany address
2024-09-20 4:12 [PATCH 0/4] Use inany addresses through port forwarding configuration David Gibson
2024-09-20 4:12 ` [PATCH 1/4] udp: Don't attempt to get dual-stack sockets in nonsensical cases David Gibson
2024-09-20 4:12 ` [PATCH 2/4] util, pif: Replace sock_l4() with pif_sock_l4() David Gibson
@ 2024-09-20 4:12 ` David Gibson
2024-09-20 4:12 ` [PATCH 4/4] inany: Add inany_pton() helper David Gibson
2024-09-25 17:39 ` [PATCH 0/4] Use inany addresses through port forwarding configuration Stefano Brivio
4 siblings, 0 replies; 6+ messages in thread
From: David Gibson @ 2024-09-20 4:12 UTC (permalink / raw)
To: Stefano Brivio, passt-dev; +Cc: David Gibson
tcp_sock_init() and udp_sock_init() take an address to bind to as an
address family and void * pair. Use an inany instead. Formerly AF_UNSPEC
was used to indicate that we want to listen on both 0.0.0.0 and ::, now use
a NULL inany to indicate that.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
conf.c | 28 ++++++++++++++--------------
tcp.c | 47 ++++++++++++++++++-----------------------------
tcp.h | 2 +-
udp.c | 31 ++++++++++---------------------
udp.h | 4 ++--
5 files changed, 45 insertions(+), 67 deletions(-)
diff --git a/conf.c b/conf.c
index b2758864..9f1cd835 100644
--- a/conf.c
+++ b/conf.c
@@ -116,11 +116,10 @@ static int parse_port_range(const char *s, char **endptr,
static void conf_ports(const struct ctx *c, char optname, const char *optarg,
struct fwd_ports *fwd)
{
- char addr_buf[sizeof(struct in6_addr)] = { 0 }, *addr = addr_buf;
+ union inany_addr addr_buf = inany_any6, *addr = &addr_buf;
char buf[BUFSIZ], *spec, *ifname = NULL, *p;
bool exclude_only = true, bound_one = false;
uint8_t exclude[PORT_BITMAP_SIZE] = { 0 };
- sa_family_t af = AF_UNSPEC;
unsigned i;
int ret;
@@ -166,15 +165,13 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
bitmap_set(fwd->map, i);
if (optname == 't') {
- ret = tcp_sock_init(c, AF_UNSPEC, NULL, NULL,
- i);
+ ret = tcp_sock_init(c, NULL, NULL, i);
if (ret == -ENFILE || ret == -EMFILE)
goto enfile;
if (!ret)
bound_one = true;
} else if (optname == 'u') {
- ret = udp_sock_init(c, 0, AF_UNSPEC, NULL, NULL,
- i);
+ ret = udp_sock_init(c, 0, NULL, NULL, i);
if (ret == -ENFILE || ret == -EMFILE)
goto enfile;
if (!ret)
@@ -218,6 +215,9 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
if (ifname == buf + 1) { /* Interface without address */
addr = NULL;
} else {
+ struct in6_addr a6;
+ struct in_addr a4;
+
p = buf;
/* Allow square brackets for IPv4 too for convenience */
@@ -226,10 +226,10 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
p++;
}
- if (inet_pton(AF_INET, p, addr))
- af = AF_INET;
- else if (inet_pton(AF_INET6, p, addr))
- af = AF_INET6;
+ if (inet_pton(AF_INET, p, &a4))
+ inany_from_af(addr, AF_INET, &a4);
+ else if (inet_pton(AF_INET6, p, &a6))
+ inany_from_af(addr, AF_INET6, &a6);
else
goto bad;
}
@@ -276,13 +276,13 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
bitmap_set(fwd->map, i);
if (optname == 't') {
- ret = tcp_sock_init(c, af, addr, ifname, i);
+ ret = tcp_sock_init(c, addr, ifname, i);
if (ret == -ENFILE || ret == -EMFILE)
goto enfile;
if (!ret)
bound_one = true;
} else if (optname == 'u') {
- ret = udp_sock_init(c, 0, af, addr, ifname, i);
+ ret = udp_sock_init(c, 0, addr, ifname, i);
if (ret == -ENFILE || ret == -EMFILE)
goto enfile;
if (!ret)
@@ -338,9 +338,9 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
ret = 0;
if (optname == 't')
- ret = tcp_sock_init(c, af, addr, ifname, i);
+ ret = tcp_sock_init(c, addr, ifname, i);
else if (optname == 'u')
- ret = udp_sock_init(c, 0, af, addr, ifname, i);
+ ret = udp_sock_init(c, 0, addr, ifname, i);
if (ret)
goto bind_fail;
}
diff --git a/tcp.c b/tcp.c
index 49e0cfe3..6ca3700e 100644
--- a/tcp.c
+++ b/tcp.c
@@ -2273,17 +2273,16 @@ void tcp_sock_handler(const struct ctx *c, union epoll_ref ref,
}
/**
- * tcp_sock_init_af() - Initialise listening socket for a given af and port
+ * tcp_sock_init_one() - Initialise listening socket for address and port
* @c: Execution context
- * @af: Address family to listen on
- * @port: Port, host order
- * @addr: Pointer to address for binding, NULL if not configured
+ * @addr: Pointer to address for binding, NULL for dual stack any
* @ifname: Name of interface to bind to, NULL if not configured
+ * @port: Port, host order
*
* Return: fd for the new listening socket, negative error code on failure
*/
-static int tcp_sock_init_af(const struct ctx *c, sa_family_t af, in_port_t port,
- const void *addr, const char *ifname)
+static int tcp_sock_init_one(const struct ctx *c, const union inany_addr *addr,
+ const char *ifname, in_port_t port)
{
union tcp_listen_epoll_ref tref = {
.port = port,
@@ -2291,24 +2290,13 @@ static int tcp_sock_init_af(const struct ctx *c, sa_family_t af, in_port_t port,
};
int s;
- if (af == AF_UNSPEC) {
- ASSERT(!addr);
- s = pif_sock_l4(c, EPOLL_TYPE_TCP_LISTEN, PIF_HOST, NULL,
- ifname, port, tref.u32);
- } else {
- union inany_addr aany = af == AF_INET ? inany_any4 : inany_any6;
-
- if (addr)
- inany_from_af(&aany, af, addr);
-
- s = pif_sock_l4(c, EPOLL_TYPE_TCP_LISTEN, PIF_HOST, &aany,
+ s = pif_sock_l4(c, EPOLL_TYPE_TCP_LISTEN, PIF_HOST, addr,
ifname, port, tref.u32);
- }
if (c->tcp.fwd_in.mode == FWD_AUTO) {
- if (af == AF_INET || af == AF_UNSPEC)
+ if (!addr || inany_v4(addr))
tcp_sock_init_ext[port][V4] = s < 0 ? -1 : s;
- if (af == AF_INET6 || af == AF_UNSPEC)
+ if (!addr || !inany_v4(addr))
tcp_sock_init_ext[port][V6] = s < 0 ? -1 : s;
}
@@ -2322,31 +2310,32 @@ static int tcp_sock_init_af(const struct ctx *c, sa_family_t af, in_port_t port,
/**
* tcp_sock_init() - Create listening sockets for a given host ("inbound") port
* @c: Execution context
- * @af: Address family to select a specific IP version, or AF_UNSPEC
* @addr: Pointer to address for binding, NULL if not configured
* @ifname: Name of interface to bind to, NULL if not configured
* @port: Port, host order
*
* Return: 0 on (partial) success, negative error code on (complete) failure
*/
-int tcp_sock_init(const struct ctx *c, sa_family_t af, const void *addr,
+int tcp_sock_init(const struct ctx *c, const union inany_addr *addr,
const char *ifname, in_port_t port)
{
int r4 = FD_REF_MAX + 1, r6 = FD_REF_MAX + 1;
ASSERT(!c->no_tcp);
- if (af == AF_UNSPEC && c->ifi4 && c->ifi6)
+ if (!addr && c->ifi4 && c->ifi6)
/* Attempt to get a dual stack socket */
- if (tcp_sock_init_af(c, AF_UNSPEC, port, addr, ifname) >= 0)
+ if (tcp_sock_init_one(c, NULL, ifname, port) >= 0)
return 0;
/* Otherwise create a socket per IP version */
- if ((af == AF_INET || af == AF_UNSPEC) && c->ifi4)
- r4 = tcp_sock_init_af(c, AF_INET, port, addr, ifname);
+ if ((!addr || inany_v4(addr)) && c->ifi4)
+ r4 = tcp_sock_init_one(c, addr ? addr : &inany_any4,
+ ifname, port);
- if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6)
- r6 = tcp_sock_init_af(c, AF_INET6, port, addr, ifname);
+ if ((!addr || !inany_v4(addr)) && c->ifi6)
+ r6 = tcp_sock_init_one(c, addr ? addr : &inany_any6,
+ ifname, port);
if (IN_INTERVAL(0, FD_REF_MAX, r4) || IN_INTERVAL(0, FD_REF_MAX, r6))
return 0;
@@ -2629,7 +2618,7 @@ static void tcp_port_rebind(struct ctx *c, bool outbound)
if (outbound)
tcp_ns_sock_init(c, port);
else
- tcp_sock_init(c, AF_UNSPEC, NULL, NULL, port);
+ tcp_sock_init(c, NULL, NULL, port);
}
}
}
diff --git a/tcp.h b/tcp.h
index 5585924f..cf30744d 100644
--- a/tcp.h
+++ b/tcp.h
@@ -18,7 +18,7 @@ void tcp_sock_handler(const struct ctx *c, union epoll_ref ref,
int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
const void *saddr, const void *daddr,
const struct pool *p, int idx, const struct timespec *now);
-int tcp_sock_init(const struct ctx *c, sa_family_t af, const void *addr,
+int tcp_sock_init(const struct ctx *c, const union inany_addr *addr,
const char *ifname, in_port_t port);
int tcp_init(struct ctx *c);
void tcp_timer(struct ctx *c, const struct timespec *now);
diff --git a/udp.c b/udp.c
index b3d4a64c..08faaecf 100644
--- a/udp.c
+++ b/udp.c
@@ -785,15 +785,14 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif,
* udp_sock_init() - Initialise listening sockets for a given port
* @c: Execution context
* @ns: In pasta mode, if set, bind with loopback address in namespace
- * @af: Address family to select a specific IP version, or AF_UNSPEC
* @addr: Pointer to address for binding, NULL if not configured
* @ifname: Name of interface to bind to, NULL if not configured
* @port: Port, host order
*
* Return: 0 on (partial) success, negative error code on (complete) failure
*/
-int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
- const void *addr, const char *ifname, in_port_t port)
+int udp_sock_init(const struct ctx *c, int ns, const union inany_addr *addr,
+ const char *ifname, in_port_t port)
{
union udp_listen_epoll_ref uref = {
.pif = ns ? PIF_SPLICE : PIF_HOST,
@@ -803,11 +802,9 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
ASSERT(!c->no_udp);
- if (af == AF_UNSPEC && c->ifi4 && c->ifi6 && !ns) {
+ if (!addr && c->ifi4 && c->ifi6 && !ns) {
int s;
- ASSERT(!addr);
-
/* Attempt to get a dual stack socket */
s = pif_sock_l4(c, EPOLL_TYPE_UDP_LISTEN, PIF_HOST,
NULL, ifname, port, uref.u32);
@@ -817,15 +814,11 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
return 0;
}
- if ((af == AF_INET || af == AF_UNSPEC) && c->ifi4) {
+ if ((!addr || inany_v4(addr)) && c->ifi4) {
if (!ns) {
- union inany_addr aany = inany_any4;
-
- if (addr)
- inany_from_af(&aany, AF_INET, addr);
-
r4 = pif_sock_l4(c, EPOLL_TYPE_UDP_LISTEN, PIF_HOST,
- &aany, ifname, port, uref.u32);
+ addr ? addr : &inany_any4, ifname,
+ port, uref.u32);
udp_splice_init[V4][port] = r4 < 0 ? -1 : r4;
} else {
@@ -836,15 +829,11 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
}
}
- if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6) {
+ if ((!addr || !inany_v4(addr)) && c->ifi6) {
if (!ns) {
- union inany_addr aany = inany_any6;
-
- if (addr)
- inany_from_af(&aany, AF_INET6, addr);
-
r6 = pif_sock_l4(c, EPOLL_TYPE_UDP_LISTEN, PIF_HOST,
- &aany, ifname, port, uref.u32);
+ addr ? addr : &inany_any6, ifname,
+ port, uref.u32);
udp_splice_init[V6][port] = r6 < 0 ? -1 : r6;
} else {
@@ -918,7 +907,7 @@ static void udp_port_rebind(struct ctx *c, bool outbound)
if ((c->ifi4 && socks[V4][port] == -1) ||
(c->ifi6 && socks[V6][port] == -1))
- udp_sock_init(c, outbound, AF_UNSPEC, NULL, NULL, port);
+ udp_sock_init(c, outbound, NULL, NULL, port);
}
}
diff --git a/udp.h b/udp.h
index a8e76bfe..de2df6de 100644
--- a/udp.h
+++ b/udp.h
@@ -16,8 +16,8 @@ void udp_reply_sock_handler(const struct ctx *c, union epoll_ref ref,
int udp_tap_handler(const struct ctx *c, uint8_t pif,
sa_family_t af, const void *saddr, const void *daddr,
const struct pool *p, int idx, const struct timespec *now);
-int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
- const void *addr, const char *ifname, in_port_t port);
+int udp_sock_init(const struct ctx *c, int ns, const union inany_addr *addr,
+ const char *ifname, in_port_t port);
int udp_init(struct ctx *c);
void udp_timer(struct ctx *c, const struct timespec *now);
void udp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s);
--
@@ -16,8 +16,8 @@ void udp_reply_sock_handler(const struct ctx *c, union epoll_ref ref,
int udp_tap_handler(const struct ctx *c, uint8_t pif,
sa_family_t af, const void *saddr, const void *daddr,
const struct pool *p, int idx, const struct timespec *now);
-int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
- const void *addr, const char *ifname, in_port_t port);
+int udp_sock_init(const struct ctx *c, int ns, const union inany_addr *addr,
+ const char *ifname, in_port_t port);
int udp_init(struct ctx *c);
void udp_timer(struct ctx *c, const struct timespec *now);
void udp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s);
--
2.46.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 4/4] inany: Add inany_pton() helper
2024-09-20 4:12 [PATCH 0/4] Use inany addresses through port forwarding configuration David Gibson
` (2 preceding siblings ...)
2024-09-20 4:12 ` [PATCH 3/4] tcp, udp: Make {tcp,udp}_sock_init() take an inany address David Gibson
@ 2024-09-20 4:12 ` David Gibson
2024-09-25 17:39 ` [PATCH 0/4] Use inany addresses through port forwarding configuration Stefano Brivio
4 siblings, 0 replies; 6+ messages in thread
From: David Gibson @ 2024-09-20 4:12 UTC (permalink / raw)
To: Stefano Brivio, passt-dev; +Cc: David Gibson
We already have an inany_ntop() function to format inany addresses into
text. Add inany_pton() to parse them from text, and use it in
conf_ports().
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
conf.c | 9 +--------
inany.c | 20 ++++++++++++++++++++
inany.h | 1 +
3 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/conf.c b/conf.c
index 9f1cd835..6e62510d 100644
--- a/conf.c
+++ b/conf.c
@@ -215,9 +215,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
if (ifname == buf + 1) { /* Interface without address */
addr = NULL;
} else {
- struct in6_addr a6;
- struct in_addr a4;
-
p = buf;
/* Allow square brackets for IPv4 too for convenience */
@@ -226,11 +223,7 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
p++;
}
- if (inet_pton(AF_INET, p, &a4))
- inany_from_af(addr, AF_INET, &a4);
- else if (inet_pton(AF_INET6, p, &a6))
- inany_from_af(addr, AF_INET6, &a6);
- else
+ if (!inany_pton(p, addr))
goto bad;
}
} else {
diff --git a/inany.c b/inany.c
index 5e391dc7..f5483bfc 100644
--- a/inany.c
+++ b/inany.c
@@ -36,3 +36,23 @@ const char *inany_ntop(const union inany_addr *src, char *dst, socklen_t size)
return inet_ntop(AF_INET6, &src->a6, dst, size);
}
+
+/** inany_pton - Parse an IPv[46] address from text format
+ * @src: IPv[46] address
+ * @dst: output buffer, filled with parsed address
+ *
+ * Return: On success, 1, if no parseable address is found, 0
+ */
+int inany_pton(const char *src, union inany_addr *dst)
+{
+ if (inet_pton(AF_INET, src, &dst->v4mapped.a4)) {
+ memset(&dst->v4mapped.zero, 0, sizeof(dst->v4mapped.zero));
+ memset(&dst->v4mapped.one, 0xff, sizeof(dst->v4mapped.one));
+ return 1;
+ }
+
+ if (inet_pton(AF_INET6, src, &dst->a6))
+ return 1;
+
+ return 0;
+}
diff --git a/inany.h b/inany.h
index d2893cec..6a12c292 100644
--- a/inany.h
+++ b/inany.h
@@ -270,5 +270,6 @@ static inline void inany_siphash_feed(struct siphash_state *state,
#define INANY_ADDRSTRLEN MAX(INET_ADDRSTRLEN, INET6_ADDRSTRLEN)
const char *inany_ntop(const union inany_addr *src, char *dst, socklen_t size);
+int inany_pton(const char *src, union inany_addr *dst);
#endif /* INANY_H */
--
@@ -270,5 +270,6 @@ static inline void inany_siphash_feed(struct siphash_state *state,
#define INANY_ADDRSTRLEN MAX(INET_ADDRSTRLEN, INET6_ADDRSTRLEN)
const char *inany_ntop(const union inany_addr *src, char *dst, socklen_t size);
+int inany_pton(const char *src, union inany_addr *dst);
#endif /* INANY_H */
--
2.46.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 0/4] Use inany addresses through port forwarding configuration
2024-09-20 4:12 [PATCH 0/4] Use inany addresses through port forwarding configuration David Gibson
` (3 preceding siblings ...)
2024-09-20 4:12 ` [PATCH 4/4] inany: Add inany_pton() helper David Gibson
@ 2024-09-25 17:39 ` Stefano Brivio
4 siblings, 0 replies; 6+ messages in thread
From: Stefano Brivio @ 2024-09-25 17:39 UTC (permalink / raw)
To: David Gibson; +Cc: passt-dev
On Fri, 20 Sep 2024 14:12:40 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:
> We use inany addresses, which can represent either an IPv4 or IPv6
> address heavily through the flow table. However, for the
> configuration of port forwarding: both the command line parsing and
> initialization of listening sockets, we use address family+(void *)
> pairs instead.
>
> Using inany through these paths makes them slightly neater now, but
> more importantly will make it easier to add more robust tracking of
> forwarding configuration.
>
> David Gibson (4):
> udp: Don't attempt to get dual-stack sockets in nonsensical cases
> util, pif: Replace sock_l4() with pif_sock_l4()
> tcp, udp: Make {tcp,udp}_sock_init() take an inany address
> inany: Add inany_pton() helper
Applied, sorry for the delay.
--
Stefano
^ permalink raw reply [flat|nested] 6+ messages in thread