[PATCH 0/3] Miscellaneous minor fixes

[PATCH 0/3] Miscellaneous minor fixes

[David Gibson]

I've just gotten back from holiday, so getting back into things with a
collection of minor fixes, for things that arose while I was away.

David Gibson (3):
  tcp: Use flow_foreach_of_type() in tcp_{keepalive,inactivity}
  fwd, pif: Replace with pif_sock_l4() with pif_listen()
  clang-tidy: Don't insist on #ifdef over #if defined()

 .clang-tidy |  5 +++++
 epoll_ctl.h |  2 +-
 fwd.h       | 14 +++++---------
 pif.c       | 14 ++++++++------
 pif.h       |  6 +++---
 tcp.c       | 18 +++---------------
 udp.c       |  8 +-------
 7 files changed, 26 insertions(+), 41 deletions(-)

-- 
2.53.0

[PATCH 1/3] tcp: Use flow_foreach_of_type() in tcp_{keepalive,inactivity}

[David Gibson]

These functions step through all TCP flows.  I forgot there was already
a flow_foreach_of_type() macro which makes this easier.  Use it.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 tcp.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/tcp.c b/tcp.c
index 88ad886d..16470703 100644
--- a/tcp.c
+++ b/tcp.c
@@ -2907,12 +2907,9 @@ static void tcp_keepalive(struct ctx *c, const struct timespec *now)
 
 	c->tcp.keepalive_run = now->tv_sec;
 
-	flow_foreach(flow) {
+	flow_foreach_of_type(flow, FLOW_TCP) {
 		struct tcp_tap_conn *conn = &flow->tcp;
 
-		if (flow->f.type != FLOW_TCP)
-			continue;
-
 		if (conn->tap_inactive) {
 			flow_dbg(conn, "No tap activity for least %us, send keepalive",
 				 KEEPALIVE_INTERVAL);
@@ -2938,12 +2935,9 @@ static void tcp_inactivity(struct ctx *c, const struct timespec *now)
 	debug("TCP inactivity scan");
 	c->tcp.inactivity_run = now->tv_sec;
 
-	flow_foreach(flow) {
+	flow_foreach_of_type(flow, FLOW_TCP) {
 		struct tcp_tap_conn *conn = &flow->tcp;
 
-		if (flow->f.type != FLOW_TCP)
-			continue;
-
 		if (conn->inactive) {
 			/* No activity in this interval, reset */
 			flow_dbg(conn, "Inactive for at least %us, resetting",
-- 
2.53.0

Re: [PATCH 1/3] tcp: Use flow_foreach_of_type() in tcp_{keepalive,inactivity}

[Stefano Brivio]

On Mon,  2 Mar 2026 15:31:33 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> These functions step through all TCP flows.  I forgot there was already
> a flow_foreach_of_type() macro which makes this easier.  Use it.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Applied with:

Suggested-by: Stefano Brivio <sbrivio@redhat.com>

> ---
>  tcp.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/tcp.c b/tcp.c
> index 88ad886d..16470703 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -2907,12 +2907,9 @@ static void tcp_keepalive(struct ctx *c, const struct timespec *now)
>  
>  	c->tcp.keepalive_run = now->tv_sec;
>  
> -	flow_foreach(flow) {
> +	flow_foreach_of_type(flow, FLOW_TCP) {
>  		struct tcp_tap_conn *conn = &flow->tcp;
>  
> -		if (flow->f.type != FLOW_TCP)
> -			continue;
> -
>  		if (conn->tap_inactive) {
>  			flow_dbg(conn, "No tap activity for least %us, send keepalive",
>  				 KEEPALIVE_INTERVAL);
> @@ -2938,12 +2935,9 @@ static void tcp_inactivity(struct ctx *c, const struct timespec *now)
>  	debug("TCP inactivity scan");
>  	c->tcp.inactivity_run = now->tv_sec;
>  
> -	flow_foreach(flow) {
> +	flow_foreach_of_type(flow, FLOW_TCP) {
>  		struct tcp_tap_conn *conn = &flow->tcp;
>  
> -		if (flow->f.type != FLOW_TCP)
> -			continue;
> -
>  		if (conn->inactive) {
>  			/* No activity in this interval, reset */
>  			flow_dbg(conn, "Inactive for at least %us, resetting",

-- 
Stefano

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

[David Gibson]

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));
 
 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;
 }
-- 
2.53.0

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

[Stefano Brivio]

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

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

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 7121 bytes --]

On Tue, Mar 03, 2026 at 08:11:39PM +0100, Stefano Brivio wrote:
> 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>

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Stefano Brivio]

On Wed, 4 Mar 2026 09:55:21 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Tue, Mar 03, 2026 at 08:11:39PM +0100, Stefano Brivio wrote:
> > 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>  
> 
> Good point.  Can you add this on merge?

Sure, done. Applied now.

-- 
Stefano

[PATCH 3/3] clang-tidy: Don't insist on #ifdef over #if defined()

[David Gibson]

Commit 036fb8770 ("checksum: add VSX fast path for POWER8/POWER9") changed
an #ifdef to #if defined, in order to match a newly introduced #if which
needs to check two different symbols.  This causes clang-tidy to complain
that the directive could be written more concisely.

In this case consistency with the other #if branch seems more important,
and in general insisting on #ifdef over #if seems unhelpfully pedantic.
Suppress that warning globally.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 .clang-tidy | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/.clang-tidy b/.clang-tidy
index 9d346ec2..773121f5 100644
--- a/.clang-tidy
+++ b/.clang-tidy
@@ -81,6 +81,11 @@ Checks:
     #	precedence over addition in modern mathematical notation. Adding
     #	parentheses to reinforce that certainly won't improve readability.
     - "-readability-math-missing-parentheses"
+
+    # #if defined(FOO) is fine, and can be more consistent with other
+    # #if directives.  Don't insist on #ifdef instead.
+    - "-readability-use-concise-preprocessor-directives"
+
 WarningsAsErrors: "*"
 HeaderFileExtensions:
     - h
-- 
2.53.0

Re: [PATCH 3/3] clang-tidy: Don't insist on #ifdef over #if defined()

[Stefano Brivio]

On Mon,  2 Mar 2026 15:31:35 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> Commit 036fb8770 ("checksum: add VSX fast path for POWER8/POWER9") changed
> an #ifdef to #if defined, in order to match a newly introduced #if which
> needs to check two different symbols.  This causes clang-tidy to complain
> that the directive could be written more concisely.
> 
> In this case consistency with the other #if branch seems more important,
> and in general insisting on #ifdef over #if seems unhelpfully pedantic.
> Suppress that warning globally.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Applied with:

Fixes: 036fb8770cc2 ("checksum: add VSX fast path for POWER8/POWER9")
Reported-by: Laurent Vivier <lvivier@redhat.com>

> ---
>  .clang-tidy | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/.clang-tidy b/.clang-tidy
> index 9d346ec2..773121f5 100644
> --- a/.clang-tidy
> +++ b/.clang-tidy
> @@ -81,6 +81,11 @@ Checks:
>      #	precedence over addition in modern mathematical notation. Adding
>      #	parentheses to reinforce that certainly won't improve readability.
>      - "-readability-math-missing-parentheses"
> +
> +    # #if defined(FOO) is fine, and can be more consistent with other
> +    # #if directives.  Don't insist on #ifdef instead.
> +    - "-readability-use-concise-preprocessor-directives"
> +
>  WarningsAsErrors: "*"
>  HeaderFileExtensions:
>      - h

-- 
Stefano