On Tue, Mar 03, 2026 at 08:11:39PM +0100, Stefano Brivio wrote: > On Mon, 2 Mar 2026 15:31:34 +1100 > David Gibson wrote: > > > It turns out all users of pif_sock_l4() use it for "listening" sockets, > > which now all have a common epoll reference format. We can take advantage > > of that to pass the necessary epoll information into pif_sock_l4() in a > > more natural way, rather than as an opaque u32. > > > > That in turn allows union fwd_listen_ref can become a struct, since the > > union only exist to allow the meaningful fields to be coerced into a u32 > > for pif_sock_l4(). > > > > Rename pif_sock_l4() to pif_listen() to reflect the new semantics. While > > we're there, remove the static_assert() on the fwd_listen_ref's size. We > > do still need it to fit into 32 bits, but that constraint is imposed only > > by the fact that it needs to fit into the whole, epoll_ref structure, > > which we already check with a static_assert() in epoll_ctl.h. > > > > Signed-off-by: David Gibson > > --- > > epoll_ctl.h | 2 +- > > fwd.h | 14 +++++--------- > > pif.c | 14 ++++++++------ > > pif.h | 6 +++--- > > tcp.c | 8 +------- > > udp.c | 8 +------- > > 6 files changed, 19 insertions(+), 33 deletions(-) > > > > diff --git a/epoll_ctl.h b/epoll_ctl.h > > index 2c103cde..879763cb 100644 > > --- a/epoll_ctl.h > > +++ b/epoll_ctl.h > > @@ -35,7 +35,7 @@ union epoll_ref { > > union { > > uint32_t flow; > > flow_sidx_t flowside; > > - union fwd_listen_ref listen; > > + struct fwd_listen_ref listen; > > uint32_t data; > > int nsdir_fd; > > int queue; > > diff --git a/fwd.h b/fwd.h > > index 16070111..b0e2373d 100644 > > --- a/fwd.h > > +++ b/fwd.h > > @@ -49,20 +49,16 @@ struct fwd_rule { > > #define FWD_NO_HINT (-1) > > > > /** > > - * union fwd_listen_ref - information about a single listening socket > > + * struct fwd_listen_ref - information about a single listening socket > > * @port: Bound port number of the socket > > * @pif: pif in which the socket is listening > > * @rule: Index of forwarding rule > > */ > > -union fwd_listen_ref { > > - struct { > > - in_port_t port; > > - uint8_t pif; > > - unsigned rule :FWD_RULE_BITS; > > - }; > > - uint32_t u32; > > +struct fwd_listen_ref { > > + in_port_t port; > > + uint8_t pif; > > + unsigned rule :FWD_RULE_BITS; > > }; > > -static_assert(sizeof(union fwd_listen_ref) == sizeof(uint32_t)); > > Reported-by: Peter Foley Good point. Can you add this on merge? > > > > > enum fwd_ports_mode { > > FWD_UNSET = 0, > > diff --git a/pif.c b/pif.c > > index 6ae970a0..82a3b5e4 100644 > > --- a/pif.c > > +++ b/pif.c > > @@ -62,23 +62,23 @@ void pif_sockaddr(const struct ctx *c, union sockaddr_inany *sa, > > } > > } > > > > -/** pif_sock_l4() - Open a socket bound to an address on a specified interface > > +/** pif_listen() - Open a listening socket on a specified pif > > * @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 > > + * @rule: Forwarding rule index this socket belongs to > > * > > * 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) > > +int pif_listen(const struct ctx *c, enum epoll_type type, uint8_t pif, > > + const union inany_addr *addr, const char *ifname, > > + in_port_t port, unsigned rule) > > { > > union epoll_ref ref; > > int ret; > > @@ -98,7 +98,9 @@ int pif_sock_l4(const struct ctx *c, enum epoll_type type, uint8_t pif, > > return ref.fd; > > > > ref.type = type; > > - ref.data = data; > > + ref.listen.port = port; > > + ref.listen.pif = pif; > > + ref.listen.rule = rule; > > > > ret = epoll_add(c->epollfd, EPOLLIN, ref); > > if (ret < 0) { > > diff --git a/pif.h b/pif.h > > index 0f7f6672..0eb3626a 100644 > > --- a/pif.h > > +++ b/pif.h > > @@ -59,8 +59,8 @@ static inline bool pif_is_socket(uint8_t pif) > > > > void pif_sockaddr(const struct ctx *c, union sockaddr_inany *sa, > > 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); > > +int pif_listen(const struct ctx *c, enum epoll_type type, uint8_t pif, > > + const union inany_addr *addr, const char *ifname, > > + in_port_t port, unsigned rule); > > > > #endif /* PIF_H */ > > diff --git a/tcp.c b/tcp.c > > index 16470703..a0d7cd86 100644 > > --- a/tcp.c > > +++ b/tcp.c > > @@ -2698,11 +2698,6 @@ void tcp_sock_handler(const struct ctx *c, union epoll_ref ref, > > int tcp_listen(const struct ctx *c, uint8_t pif, unsigned rule, > > const union inany_addr *addr, const char *ifname, in_port_t port) > > { > > - union fwd_listen_ref ref = { > > - .port = port, > > - .pif = pif, > > - .rule = rule, > > - }; > > int s; > > > > ASSERT(!c->no_tcp); > > @@ -2722,8 +2717,7 @@ int tcp_listen(const struct ctx *c, uint8_t pif, unsigned rule, > > return -EAFNOSUPPORT; > > } > > > > - s = pif_sock_l4(c, EPOLL_TYPE_TCP_LISTEN, pif, addr, ifname, > > - port, ref.u32); > > + s = pif_listen(c, EPOLL_TYPE_TCP_LISTEN, pif, addr, ifname, port, rule); > > > > return s; > > } > > diff --git a/udp.c b/udp.c > > index 19adda06..464aa093 100644 > > --- a/udp.c > > +++ b/udp.c > > @@ -1139,11 +1139,6 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif, > > int udp_listen(const struct ctx *c, uint8_t pif, unsigned rule, > > const union inany_addr *addr, const char *ifname, in_port_t port) > > { > > - union fwd_listen_ref ref = { > > - .pif = pif, > > - .port = port, > > - .rule = rule, > > - }; > > int s; > > > > ASSERT(!c->no_udp); > > @@ -1163,8 +1158,7 @@ int udp_listen(const struct ctx *c, uint8_t pif, unsigned rule, > > return -EAFNOSUPPORT; > > } > > > > - s = pif_sock_l4(c, EPOLL_TYPE_UDP_LISTEN, pif, > > - addr, ifname, port, ref.u32); > > + s = pif_listen(c, EPOLL_TYPE_UDP_LISTEN, pif, addr, ifname, port, rule); > > > > return s; > > } > > -- > Stefano > -- David Gibson (he or they) | 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