Re: [PATCH v2 3/3] fwd, tcp, udp: Consolidate epoll refs for listening sockets

[Laurent Vivier <lvivier@redhat.com>]

On 1/8/26 03:14, David Gibson wrote:
> The epoll references we use for TCP listening sockets and UDP "listening"
> sockets have identical information.  Combine them into a single structure.
> Note that, despite the name, epoll_ref.udp was only ever used for
> "listening" sockets, not flow sockets.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>   epoll_ctl.h |  6 ++----
>   fwd.h       | 13 +++++++++++++
>   tcp.c       |  8 ++++----
>   tcp.h       | 14 --------------
>   udp.c       |  6 +++---
>   udp.h       | 15 ---------------
>   6 files changed, 22 insertions(+), 40 deletions(-)
> 
> diff --git a/epoll_ctl.h b/epoll_ctl.h
> index 02b081e2..3f802e76 100644
> --- a/epoll_ctl.h
> +++ b/epoll_ctl.h
> @@ -21,8 +21,7 @@
>    * @fd:		File descriptor number (implies < 2^24 total descriptors)
>    * @flow:	Index of the flow this fd is linked to
>    * @flowside:	Index and side of a flow this fd is linked to
> - * @tcp_listen:	TCP-specific reference part for listening sockets
> - * @udp:	UDP-specific reference part
> + * @listen:	Information for listening sockets
>    * @data:	Data handled by protocol handlers
>    * @nsdir_fd:	netns dirfd for fallback timer checking if namespace is gone
>    * @queue:	vhost-user queue index for this fd
> @@ -35,8 +34,7 @@ union epoll_ref {
>   		union {
>   			uint32_t flow;
>   			flow_sidx_t flowside;
> -			union tcp_listen_epoll_ref tcp_listen;
> -			union udp_listen_epoll_ref udp;
> +			union fwd_listen_ref listen;
>   			uint32_t data;
>   			int nsdir_fd;
>   			int queue;
> diff --git a/fwd.h b/fwd.h
> index 77925822..934bab33 100644
> --- a/fwd.h
> +++ b/fwd.h
> @@ -16,6 +16,19 @@ struct flowside;
>   void fwd_probe_ephemeral(void);
>   bool fwd_port_is_ephemeral(in_port_t port);
>   
> +/**
> + * union fwd_listen_ref - information about a single listening socket
> + * @port:	Bound port number of the socket
> + * @pif:	pif in which the socket is listening
> + */
> +union fwd_listen_ref {
> +	struct {
> +		in_port_t	port;
> +		uint8_t		pif;
> +	};
> +	uint32_t u32;
> +};
> +
>   enum fwd_ports_mode {
>   	FWD_UNSET = 0,
>   	FWD_SPEC = 1,
> diff --git a/tcp.c b/tcp.c
> index e7fa85f3..9fc385d2 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -2485,8 +2485,8 @@ void tcp_listen_handler(const struct ctx *c, union epoll_ref ref,
>   	 * address, record that as our address, as implemented for vhost-user
>   	 * mode only, below.
>   	 */
> -	ini = flow_initiate_sa(flow, ref.tcp_listen.pif, &sa,
> -			       NULL, ref.tcp_listen.port);
> +	ini = flow_initiate_sa(flow, ref.listen.pif, &sa,
> +			       NULL, ref.listen.port);
>   
>   	if (getsockname(s, &sa.sa, &sl) ||
>   	    inany_from_sockaddr(&ini->oaddr, &ini->oport, &sa) < 0)
> @@ -2685,7 +2685,7 @@ void tcp_sock_handler(const struct ctx *c, union epoll_ref ref,
>   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 = {
> +	union fwd_listen_ref ref = {
>   		.port = port,
>   		.pif = pif,
>   	};
> @@ -2722,7 +2722,7 @@ int tcp_listen(const struct ctx *c, uint8_t pif,
>   	}
>   
>   	s = pif_sock_l4(c, EPOLL_TYPE_TCP_LISTEN, pif, addr, ifname,
> -			port, tref.u32);
> +			port, ref.u32);
>   
>   	if (fwd->mode == FWD_AUTO) {
>   		if (!addr || inany_v4(addr))
> diff --git a/tcp.h b/tcp.h
> index 516dfef5..ef1e3544 100644
> --- a/tcp.h
> +++ b/tcp.h
> @@ -30,20 +30,6 @@ void tcp_update_l2_buf(const unsigned char *eth_d);
>   
>   extern bool peek_offset_cap;
>   
> -/**
> - * union tcp_listen_epoll_ref - epoll reference portion for TCP listening
> - * @port:	Bound port number of the socket
> - * @pif:	pif in which the socket is listening
> - * @u32:	Opaque u32 value of reference
> - */
> -union tcp_listen_epoll_ref {
> -	struct {
> -		in_port_t	port;
> -		uint8_t		pif;
> -	};
> -	uint32_t u32;
> -};
> -
>   /**
>    * struct tcp_ctx - Execution context for TCP routines
>    * @port_to_tap:	Ports bound host-side, packets to tap or spliced
> diff --git a/udp.c b/udp.c
> index eda55c39..747d4dd4 100644
> --- a/udp.c
> +++ b/udp.c
> @@ -928,7 +928,7 @@ void udp_listen_sock_handler(const struct ctx *c,
>   			     const struct timespec *now)
>   {
>   	if (events & (EPOLLERR | EPOLLIN))
> -		udp_sock_fwd(c, ref.fd, ref.udp.pif, ref.udp.port, now);
> +		udp_sock_fwd(c, ref.fd, ref.listen.pif, ref.listen.port, now);
>   }
>   
>   /**
> @@ -1141,7 +1141,7 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif,
>   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 = {
> +	union fwd_listen_ref ref = {
>   		.pif = pif,
>   		.port = port,
>   	};
> @@ -1175,7 +1175,7 @@ int udp_listen(const struct ctx *c, uint8_t pif,
>   	}
>   
>   	s = pif_sock_l4(c, EPOLL_TYPE_UDP_LISTEN, pif,
> -			addr, ifname, port, uref.u32);
> +			addr, ifname, port, ref.u32);
>   
>   	if (!addr || inany_v4(addr))
>   		socks[V4][port] = s < 0 ? -1 : s;
> diff --git a/udp.h b/udp.h
> index 5407db3b..94c698e2 100644
> --- a/udp.h
> +++ b/udp.h
> @@ -22,21 +22,6 @@ int udp_init(struct ctx *c);
>   void udp_port_rebind_all(struct ctx *c);
>   void udp_update_l2_buf(const unsigned char *eth_d);
>   
> -/**
> - * union udp_listen_epoll_ref - epoll reference for "listening" UDP sockets
> - * @port:		Source port for connected sockets, bound port otherwise
> - * @pif:		pif for this socket
> - * @u32:		Opaque u32 value of reference
> - */
> -union udp_listen_epoll_ref {
> -	struct {
> -		in_port_t	port;
> -		uint8_t		pif;
> -	};
> -	uint32_t u32;
> -};
> -
> -
>   /**
>    * struct udp_ctx - Execution context for UDP
>    * @fwd_in:		Port forwarding configuration for inbound packets

Reviewed-by: Laurent Vivier <lvivier@redhat.com>