* [PATCH 1/2] tcp: Combine tcp_sock_init_one() and tcp_sock_init() into tcp_listen()
2025-12-18 6:22 [PATCH 0/2] Cleanups to listening socket init functions David Gibson
@ 2025-12-18 6:22 ` David Gibson
2025-12-18 6:22 ` [PATCH 2/2] udp: Rename udp_sock_init() to udp_listen() with small cleanups David Gibson
1 sibling, 0 replies; 3+ messages in thread
From: David Gibson @ 2025-12-18 6:22 UTC (permalink / raw)
To: passt-dev, Stefano Brivio; +Cc: David Gibson
Despite the name, these two functions are specifically for creating
listening sockets, not any others. Recent changes mean that there's
always exactly one call of tcp_sock_init_one() call per call to
tcp_sock_init(). So combine them into tcp_listen().
While we're there remove a redundant check for (s > FD_REF_MAX).
pif_sock_l4() already checks for this (and must, in order to properly
populate the epoll reference).
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
conf.c | 2 +-
tcp.c | 94 +++++++++++++++++++++-------------------------------------
tcp.h | 6 ++--
3 files changed, 38 insertions(+), 64 deletions(-)
diff --git a/conf.c b/conf.c
index 2942c8c2..dada3b1f 100644
--- a/conf.c
+++ b/conf.c
@@ -175,7 +175,7 @@ static void conf_ports_range_except(const struct ctx *c, char optname,
fwd->delta[i] = to - first;
if (optname == 't')
- ret = tcp_sock_init(c, PIF_HOST, addr, ifname, i);
+ ret = tcp_listen(c, PIF_HOST, addr, ifname, i);
else if (optname == 'u')
ret = udp_sock_init(c, PIF_HOST, addr, ifname, i);
else
diff --git a/tcp.c b/tcp.c
index b179e399..71c6d632 100644
--- a/tcp.c
+++ b/tcp.c
@@ -2673,66 +2673,24 @@ void tcp_sock_handler(const struct ctx *c, union epoll_ref ref,
}
/**
- * tcp_sock_init_one() - Initialise listening socket for address and port
+ * tcp_listen() - Create listening socket
* @c: Execution context
* @pif: Interface to open the socket for (PIF_HOST or PIF_SPLICE)
- * @addr: Pointer to address for binding, NULL for dual stack any
- * @ifname: Name of interface to bind to, NULL if not configured
+ * @addr: Pointer to address for binding, NULL for any
+ * @ifname: Name of interface to bind to, NULL for any
* @port: Port, host order
*
- * Return: fd for the new listening socket, negative error code on failure
- *
- * If pif == PIF_SPLICE, the caller must have already entered the guest ns.
+ * Return: 0 on success, negative error code on failure
*/
-static int tcp_sock_init_one(const struct ctx *c, uint8_t pif,
- const union inany_addr *addr, const char *ifname,
- in_port_t port)
+int tcp_listen(const struct ctx *c, uint8_t pif,
+ const union inany_addr *addr, const char *ifname, in_port_t port)
{
union tcp_listen_epoll_ref tref = {
.port = port,
.pif = pif,
};
const struct fwd_ports *fwd;
- int s;
-
- if (pif == PIF_HOST)
- fwd = &c->tcp.fwd_in;
- else
- fwd = &c->tcp.fwd_out;
-
- s = pif_sock_l4(c, EPOLL_TYPE_TCP_LISTEN, pif, addr, ifname,
- port, tref.u32);
-
- if (fwd->mode == FWD_AUTO) {
- int (*socks)[IP_VERSIONS] = pif == PIF_SPLICE ?
- tcp_sock_ns : tcp_sock_init_ext;
-
- if (!addr || inany_v4(addr))
- socks[port][V4] = s < 0 ? -1 : s;
- if (!addr || !inany_v4(addr))
- socks[port][V6] = s < 0 ? -1 : s;
- }
-
- if (s < 0)
- return s;
-
- return s;
-}
-
-/**
- * tcp_sock_init() - Create listening socket for a given host ("inbound") port
- * @c: Execution context
- * @pif: Interface to open the socket for (PIF_HOST or PIF_SPLICE)
- * @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 success, negative error code on failure
- */
-int tcp_sock_init(const struct ctx *c, uint8_t pif,
- const union inany_addr *addr, const char *ifname,
- in_port_t port)
-{
+ int (*socks)[IP_VERSIONS];
int s;
ASSERT(!c->no_tcp);
@@ -2754,33 +2712,49 @@ int tcp_sock_init(const struct ctx *c, uint8_t pif,
return 0;
}
- s = tcp_sock_init_one(c, pif, addr, ifname, port);
+ if (pif == PIF_HOST) {
+ fwd = &c->tcp.fwd_in;
+ socks = tcp_sock_init_ext;
+ } else {
+ ASSERT(pif == PIF_SPLICE);
+ fwd = &c->tcp.fwd_out;
+ socks = tcp_sock_ns;
+ }
+
+ s = pif_sock_l4(c, EPOLL_TYPE_TCP_LISTEN, pif, addr, ifname,
+ port, tref.u32);
+
+ if (fwd->mode == FWD_AUTO) {
+ if (!addr || inany_v4(addr))
+ socks[port][V4] = s < 0 ? -1 : s;
+ if (!addr || !inany_v4(addr))
+ socks[port][V6] = s < 0 ? -1 : s;
+ }
+
if (s < 0)
return s;
- if (s > FD_REF_MAX)
- return -EIO;
return 0;
}
/**
- * tcp_ns_sock_init() - Init socket to listen for spliced outbound connections
+ * tcp_ns_listen() - Init socket to listen for spliced outbound connections
* @c: Execution context
* @port: Port, host order
*/
-static void tcp_ns_sock_init(const struct ctx *c, in_port_t port)
+static void tcp_ns_listen(const struct ctx *c, in_port_t port)
{
ASSERT(!c->no_tcp);
if (!c->no_bindtodevice) {
- tcp_sock_init(c, PIF_SPLICE, NULL, "lo", port);
+ tcp_listen(c, PIF_SPLICE, NULL, "lo", port);
return;
}
if (c->ifi4)
- tcp_sock_init_one(c, PIF_SPLICE, &inany_loopback4, NULL, port);
+ tcp_listen(c, PIF_SPLICE, &inany_loopback4, NULL, port);
if (c->ifi6)
- tcp_sock_init_one(c, PIF_SPLICE, &inany_loopback6, NULL, port);
+ tcp_listen(c, PIF_SPLICE, &inany_loopback6, NULL, port);
}
/**
@@ -2801,7 +2775,7 @@ static int tcp_ns_socks_init(void *arg)
if (!bitmap_isset(c->tcp.fwd_out.map, port))
continue;
- tcp_ns_sock_init(c, port);
+ tcp_ns_listen(c, port);
}
return 0;
@@ -3003,9 +2977,9 @@ static void tcp_port_rebind(struct ctx *c, bool outbound)
if ((c->ifi4 && socks[port][V4] == -1) ||
(c->ifi6 && socks[port][V6] == -1)) {
if (outbound)
- tcp_ns_sock_init(c, port);
+ tcp_ns_listen(c, port);
else
- tcp_sock_init(c, PIF_HOST, NULL, NULL, port);
+ tcp_listen(c, PIF_HOST, NULL, NULL, port);
}
}
}
diff --git a/tcp.h b/tcp.h
index 3f21e755..9dd88762 100644
--- a/tcp.h
+++ b/tcp.h
@@ -18,9 +18,9 @@ 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, uint32_t flow_lbl,
const struct pool *p, int idx, const struct timespec *now);
-int tcp_sock_init(const struct ctx *c, uint8_t pif,
- const union inany_addr *addr, const char *ifname,
- in_port_t port);
+int tcp_listen(const struct ctx *c, uint8_t pif,
+ const union inany_addr *addr, const char *ifname,
+ in_port_t port);
int tcp_init(struct ctx *c);
void tcp_port_rebind_all(struct ctx *c);
void tcp_timer(const struct ctx *c, const struct timespec *now);
--
2.52.0
^ permalink raw reply [flat|nested] 3+ messages in thread* [PATCH 2/2] udp: Rename udp_sock_init() to udp_listen() with small cleanups
2025-12-18 6:22 [PATCH 0/2] Cleanups to listening socket init functions David Gibson
2025-12-18 6:22 ` [PATCH 1/2] tcp: Combine tcp_sock_init_one() and tcp_sock_init() into tcp_listen() David Gibson
@ 2025-12-18 6:22 ` David Gibson
1 sibling, 0 replies; 3+ messages in thread
From: David Gibson @ 2025-12-18 6:22 UTC (permalink / raw)
To: passt-dev, Stefano Brivio; +Cc: David Gibson
Despite the name, this functions is specifically for creating
"listening" sockets, not any others. While we're there remove a redundant
check for (s > FD_REF_MAX). pif_sock_l4() already checks for this (and
must, in order to properly populate the epoll reference).
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
conf.c | 2 +-
udp.c | 32 ++++++++++++++------------------
udp.h | 6 +++---
3 files changed, 18 insertions(+), 22 deletions(-)
diff --git a/conf.c b/conf.c
index dada3b1f..84ae12b2 100644
--- a/conf.c
+++ b/conf.c
@@ -177,7 +177,7 @@ static void conf_ports_range_except(const struct ctx *c, char optname,
if (optname == 't')
ret = tcp_listen(c, PIF_HOST, addr, ifname, i);
else if (optname == 'u')
- ret = udp_sock_init(c, PIF_HOST, addr, ifname, i);
+ ret = udp_listen(c, PIF_HOST, addr, ifname, i);
else
/* No way to check in advance for -T and -U */
ret = 0;
diff --git a/udp.c b/udp.c
index 08bec50a..eda55c39 100644
--- a/udp.c
+++ b/udp.c
@@ -1129,7 +1129,7 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif,
}
/**
- * udp_sock_init() - Initialise listening socket for a given port
+ * udp_listen() - Initialise listening socket for a given port
* @c: Execution context
* @pif: Interface to open the socket for (PIF_HOST or PIF_SPLICE)
* @addr: Pointer to address for binding, NULL if not configured
@@ -1138,9 +1138,8 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif,
*
* Return: 0 on success, negative error code on failure
*/
-int udp_sock_init(const struct ctx *c, uint8_t pif,
- const union inany_addr *addr, const char *ifname,
- in_port_t port)
+int udp_listen(const struct ctx *c, uint8_t pif,
+ const union inany_addr *addr, const char *ifname, in_port_t port)
{
union udp_listen_epoll_ref uref = {
.pif = pif,
@@ -1150,12 +1149,13 @@ int udp_sock_init(const struct ctx *c, uint8_t pif,
int s;
ASSERT(!c->no_udp);
- ASSERT(pif_is_socket(pif));
- if (pif == PIF_HOST)
+ if (pif == PIF_HOST) {
socks = udp_splice_init;
- else
+ } else {
+ ASSERT(pif == PIF_SPLICE);
socks = udp_splice_ns;
+ }
if (!c->ifi4) {
if (!addr)
@@ -1176,10 +1176,6 @@ int udp_sock_init(const struct ctx *c, uint8_t pif,
s = pif_sock_l4(c, EPOLL_TYPE_UDP_LISTEN, pif,
addr, ifname, port, uref.u32);
- if (s > FD_REF_MAX) {
- close(s);
- s = -EIO;
- }
if (!addr || inany_v4(addr))
socks[V4][port] = s < 0 ? -1 : s;
@@ -1210,23 +1206,23 @@ static void udp_splice_iov_init(void)
}
/**
- * udp_ns_sock_init() - Init socket to listen for spliced outbound connections
+ * udp_ns_listen() - Init socket to listen for spliced outbound connections
* @c: Execution context
* @port: Port, host order
*/
-static void udp_ns_sock_init(const struct ctx *c, in_port_t port)
+static void udp_ns_listen(const struct ctx *c, in_port_t port)
{
ASSERT(!c->no_udp);
if (!c->no_bindtodevice) {
- udp_sock_init(c, PIF_SPLICE, NULL, "lo", port);
+ udp_listen(c, PIF_SPLICE, NULL, "lo", port);
return;
}
if (c->ifi4)
- udp_sock_init(c, PIF_SPLICE, &inany_loopback4, NULL, port);
+ udp_listen(c, PIF_SPLICE, &inany_loopback4, NULL, port);
if (c->ifi6)
- udp_sock_init(c, PIF_SPLICE, &inany_loopback6, NULL, port);
+ udp_listen(c, PIF_SPLICE, &inany_loopback6, NULL, port);
}
/**
@@ -1261,9 +1257,9 @@ static void udp_port_rebind(struct ctx *c, bool outbound)
if ((c->ifi4 && socks[V4][port] == -1) ||
(c->ifi6 && socks[V6][port] == -1)) {
if (outbound)
- udp_ns_sock_init(c, port);
+ udp_ns_listen(c, port);
else
- udp_sock_init(c, PIF_HOST, NULL, NULL, port);
+ udp_listen(c, PIF_HOST, NULL, NULL, port);
}
}
}
diff --git a/udp.h b/udp.h
index 03e8dc54..5407db3b 100644
--- a/udp.h
+++ b/udp.h
@@ -15,9 +15,9 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif,
sa_family_t af, const void *saddr, const void *daddr,
uint8_t ttl, const struct pool *p, int idx,
const struct timespec *now);
-int udp_sock_init(const struct ctx *c, uint8_t pif,
- const union inany_addr *addr, const char *ifname,
- in_port_t port);
+int udp_listen(const struct ctx *c, uint8_t pif,
+ const union inany_addr *addr, const char *ifname,
+ in_port_t port);
int udp_init(struct ctx *c);
void udp_port_rebind_all(struct ctx *c);
void udp_update_l2_buf(const unsigned char *eth_d);
--
2.52.0
^ permalink raw reply [flat|nested] 3+ messages in thread