Re: [PATCH 2/3] fwd, pif: Replace with pif_sock_l4() with pif_listen()

[Stefano Brivio <sbrivio@redhat.com>]

On Mon,  2 Mar 2026 15:31:34 +1100
David Gibson <david@gibson.dropbear.id.au> 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 <david@gibson.dropbear.id.au>
> ---
>  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 <pefoley@google.com>

>  
>  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