[PATCH v2 3/6] util: Move epoll registration out of sock_l4_sa()

[Laurent Vivier <lvivier@redhat.com>]

Move epoll_add() calls from sock_l4_sa() to the protocol-specific code
(icmp.c, pif.c, udp_flow.c) to give callers more control over epoll
registration. This allows sock_l4_sa() to focus solely on socket
creation and binding, while epoll management happens at a higher level.

Remove the data parameter from sock_l4_sa() and flowside_sock_l4() as
it's no longer needed - callers now construct the full epoll_ref and
register the socket themselves after creation.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 flow.c     | 10 ++++------
 flow.h     |  2 +-
 icmp.c     | 15 +++++++++++----
 pif.c      | 32 +++++++++++++++++++++++++-------
 udp_flow.c | 16 ++++++++++++++--
 util.c     | 10 +---------
 util.h     |  2 +-
 7 files changed, 57 insertions(+), 30 deletions(-)

diff --git a/flow.c b/flow.c
index feefda3ce74e..b14e9d8b63ff 100644
--- a/flow.c
+++ b/flow.c
@@ -163,7 +163,6 @@ static void flowside_from_af(struct flowside *side, sa_family_t af,
  * @type:	Socket epoll type
  * @sa:		Socket address
  * @sl:		Length of @sa
- * @data:	epoll reference data
  */
 struct flowside_sock_args {
 	const struct ctx *c;
@@ -173,7 +172,6 @@ struct flowside_sock_args {
 	const struct sockaddr *sa;
 	socklen_t sl;
 	const char *path;
-	uint32_t data;
 };
 
 /** flowside_sock_splice() - Create and bind socket for PIF_SPLICE based on flowside
@@ -188,7 +186,7 @@ static int flowside_sock_splice(void *arg)
 	ns_enter(a->c);
 
 	a->fd = sock_l4_sa(a->c, a->type, a->sa, a->sl, NULL,
-	                   a->sa->sa_family == AF_INET6, a->data);
+	                   a->sa->sa_family == AF_INET6);
 	a->err = errno;
 
 	return 0;
@@ -205,7 +203,7 @@ static int flowside_sock_splice(void *arg)
  *         (if specified).
  */
 int flowside_sock_l4(const struct ctx *c, enum epoll_type type, uint8_t pif,
-		     const struct flowside *tgt, uint32_t data)
+		     const struct flowside *tgt)
 {
 	const char *ifname = NULL;
 	union sockaddr_inany sa;
@@ -225,12 +223,12 @@ int flowside_sock_l4(const struct ctx *c, enum epoll_type type, uint8_t pif,
 			ifname = c->ip6.ifname_out;
 
 		return sock_l4_sa(c, type, &sa, sl, ifname,
-				  sa.sa_family == AF_INET6, data);
+				  sa.sa_family == AF_INET6);
 
 	case PIF_SPLICE: {
 		struct flowside_sock_args args = {
 			.c = c, .type = type,
-			.sa = &sa.sa, .sl = sl, .data = data,
+			.sa = &sa.sa, .sl = sl,
 		};
 		NS_CALL(flowside_sock_splice, &args);
 		errno = args.err;
diff --git a/flow.h b/flow.h
index cac618ad0ca1..ef138b83add8 100644
--- a/flow.h
+++ b/flow.h
@@ -167,7 +167,7 @@ static inline bool flowside_eq(const struct flowside *left,
 }
 
 int flowside_sock_l4(const struct ctx *c, enum epoll_type type, uint8_t pif,
-		     const struct flowside *tgt, uint32_t data);
+		     const struct flowside *tgt);
 int flowside_connect(const struct ctx *c, int s,
 		     uint8_t pif, const struct flowside *tgt);
 
diff --git a/icmp.c b/icmp.c
index c26561da80bf..56dfac6c958e 100644
--- a/icmp.c
+++ b/icmp.c
@@ -170,10 +170,10 @@ static struct icmp_ping_flow *icmp_ping_new(const struct ctx *c,
 {
 	uint8_t proto = af == AF_INET ? IPPROTO_ICMP : IPPROTO_ICMPV6;
 	uint8_t flowtype = af == AF_INET ? FLOW_PING4 : FLOW_PING6;
-	union epoll_ref ref = { .type = EPOLL_TYPE_PING };
 	union flow *flow = flow_alloc();
 	struct icmp_ping_flow *pingf;
 	const struct flowside *tgt;
+	union epoll_ref ref;
 
 	if (!flow)
 		return NULL;
@@ -194,9 +194,7 @@ static struct icmp_ping_flow *icmp_ping_new(const struct ctx *c,
 
 	pingf->seq = -1;
 
-	ref.flowside = FLOW_SIDX(flow, TGTSIDE);
-	pingf->sock = flowside_sock_l4(c, EPOLL_TYPE_PING, PIF_HOST,
-				       tgt, ref.data);
+	pingf->sock = flowside_sock_l4(c, EPOLL_TYPE_PING, PIF_HOST, tgt);
 
 	if (pingf->sock < 0) {
 		warn("Cannot open \"ping\" socket. You might need to:");
@@ -208,6 +206,15 @@ static struct icmp_ping_flow *icmp_ping_new(const struct ctx *c,
 	if (pingf->sock > FD_REF_MAX)
 		goto cancel;
 
+	ref.type = EPOLL_TYPE_PING;
+	ref.flowside = FLOW_SIDX(flow, TGTSIDE);
+	ref.fd = pingf->sock;
+
+	if (epoll_add(c->epollfd, EPOLLIN, &ref) < 0) {
+		close(pingf->sock);
+		goto cancel;
+	}
+
 	flow_dbg(pingf, "new socket %i for echo ID %"PRIu16, pingf->sock, id);
 
 	flow_hash_insert(c, FLOW_SIDX(pingf, INISIDE));
diff --git a/pif.c b/pif.c
index 592fafaab58a..17b41705183c 100644
--- a/pif.c
+++ b/pif.c
@@ -5,6 +5,7 @@
  * Passt/pasta interface types and IDs
  */
 
+#include <errno.h>
 #include <stdint.h>
 #include <assert.h>
 #include <netinet/in.h>
@@ -14,7 +15,7 @@
 #include "siphash.h"
 #include "ip.h"
 #include "inany.h"
-#include "passt.h"
+#include "epoll_ctl.h"
 
 const char *pif_type_str[] = {
 	[PIF_NONE]		= "<none>",
@@ -83,7 +84,9 @@ int pif_sock_l4(const struct ctx *c, enum epoll_type type, uint8_t pif,
 		.sa6.sin6_addr = in6addr_any,
 		.sa6.sin6_port = htons(port),
 	};
+	union epoll_ref ref;
 	socklen_t sl;
+	int ret;
 
 	ASSERT(pif_is_socket(pif));
 
@@ -93,11 +96,26 @@ int pif_sock_l4(const struct ctx *c, enum epoll_type type, uint8_t pif,
 		ASSERT(addr && inany_is_loopback(addr));
 	}
 
-	if (!addr)
-		return sock_l4_sa(c, type, &sa, sizeof(sa.sa6),
-				  ifname, false, data);
+	if (!addr) {
+		ref.fd = sock_l4_sa(c, type, &sa, sizeof(sa.sa6),
+				    ifname, false);
+	} else {
+		pif_sockaddr(c, &sa, &sl, pif, addr, port);
+		ref.fd = sock_l4_sa(c, type, &sa, sl,
+				    ifname, sa.sa_family == AF_INET6);
+	}
+
+	if (ref.fd < 0)
+		return ref.fd;
+
+	ref.type = type;
+	ref.data = data;
+
+	ret = epoll_add(c->epollfd, EPOLLIN, &ref);
+	if (ret < 0) {
+		close(ref.fd);
+		return ret;
+	}
 
-	pif_sockaddr(c, &sa, &sl, pif, addr, port);
-	return sock_l4_sa(c, type, &sa, sl,
-			  ifname, sa.sa_family == AF_INET6, data);
+	return ref.fd;
 }
diff --git a/udp_flow.c b/udp_flow.c
index d9c75f1bb1d8..caaf3dababb1 100644
--- a/udp_flow.c
+++ b/udp_flow.c
@@ -78,16 +78,28 @@ static int udp_flow_sock(const struct ctx *c,
 		flow_sidx_t sidx;
 		uint32_t data;
 	} fref = { .sidx = FLOW_SIDX(uflow, sidei) };
+	union epoll_ref ref;
+	int rc;
 	int s;
 
-	s = flowside_sock_l4(c, EPOLL_TYPE_UDP, pif, side, fref.data);
+	s = flowside_sock_l4(c, EPOLL_TYPE_UDP, pif, side);
 	if (s < 0) {
 		flow_dbg_perror(uflow, "Couldn't open flow specific socket");
 		return s;
 	}
 
+	ref.type = EPOLL_TYPE_UDP;
+	ref.data = fref.data;
+	ref.fd = s;
+
+	rc = epoll_add(c->epollfd, EPOLLIN, &ref);
+	if (rc < 0) {
+		close(s);
+		return rc;
+	}
+
 	if (flowside_connect(c, s, pif, side) < 0) {
-		int rc = -errno;
+		rc = -errno;
 
 		epoll_del(c->epollfd, s);
 		close(s);
diff --git a/util.c b/util.c
index b2490123590a..707ab3388d13 100644
--- a/util.c
+++ b/util.c
@@ -47,16 +47,14 @@
  * @sl:		Length of @sa
  * @ifname:	Interface for binding, NULL for any
  * @v6only:	Set IPV6_V6ONLY socket option
- * @data:	epoll reference portion for protocol handlers
  *
  * Return: newly created socket, negative error code on failure
  */
 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)
+	       const char *ifname, bool v6only)
 {
 	sa_family_t af = ((const struct sockaddr *)sa)->sa_family;
-	union epoll_ref ref = { .type = type, .data = data };
 	bool freebind = false;
 	int fd, y = 1, ret;
 	uint8_t proto;
@@ -99,8 +97,6 @@ int sock_l4_sa(const struct ctx *c, enum epoll_type type,
 		return -EBADF;
 	}
 
-	ref.fd = fd;
-
 	if (v6only)
 		if (setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, &y, sizeof(y)))
 			debug("Failed to set IPV6_V6ONLY on socket %i", fd);
@@ -171,10 +167,6 @@ int sock_l4_sa(const struct ctx *c, enum epoll_type type,
 		return ret;
 	}
 
-	ret = epoll_add(c->epollfd, EPOLLIN, &ref);
-	if (ret < 0)
-		return ret;
-
 	return fd;
 }
 
diff --git a/util.h b/util.h
index 8e4b4c5c6032..9108f8f19e14 100644
--- a/util.h
+++ b/util.h
@@ -207,7 +207,7 @@ struct ctx;
 
 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);
+	       const char *ifname, bool v6only);
 int sock_unix(char *sock_path);
 void sock_probe_mem(struct ctx *c);
 long timespec_diff_ms(const struct timespec *a, const struct timespec *b);
-- 
2.50.1