Re: [PATCH v3 05/14] fwd: Make space to store listening sockets in forward table

[Stefano Brivio <sbrivio@redhat.com>]

On Thu,  8 Jan 2026 13:29:39 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> At present, we don't keep track of the fds for listening sockets (except
> for "auto" ones).  Since the fd is stored in the epoll reference, we didn't
> need an alternative source of it for the various handlers.
> 
> However, we're intending to allow dynamic changes to forwarding
> configuration in future.  That means we need a way to enumerate sockets so
> we can close them on removal of a forward.
> 
> Extend our forwarding table data structure to make space for all the
> listening sockets.  To avoid allocation, this imposes another limit:
> we could run out of space for socket fds before we run out of slots
> for forwarding rules.
> 
> We don't actually do anything with the allocate spaced yet.  For
> "auto" forwards it's redundant with existing arrays.  We'll fix both
> of those in later patches.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  fwd.c | 10 +++++++++-
>  fwd.h | 13 +++++++++++++
>  2 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/fwd.c b/fwd.c
> index 69aca441..f27a4220 100644
> --- a/fwd.c
> +++ b/fwd.c
> @@ -345,6 +345,7 @@ void fwd_rule_add(struct fwd_ports *fwd, uint8_t flags,
>  {
>  	/* Flags which can be set from the caller */
>  	const uint8_t allowed_flags = FWD_WEAK | FWD_SCAN;
> +	unsigned num = (unsigned)last - first + 1;
>  	struct fwd_rule *new;
>  	unsigned port;
>  
> @@ -352,6 +353,8 @@ void fwd_rule_add(struct fwd_ports *fwd, uint8_t flags,
>  
>  	if (fwd->count >= ARRAY_SIZE(fwd->rules))
>  		die("Too many port forwarding ranges");
> +	if ((fwd->listen_sock_count + num) > ARRAY_SIZE(fwd->listen_socks))
> +		die("Too many listening sockets");

Here, and above: we plan to trigger this from a client at runtime, and
if there are too many listening sockets, or too many rules/ranges, we
should fail and report failure (ENOBUFS I guess) instead of quitting.

>  
>  	new = &fwd->rules[fwd->count++];
>  	new->flags = flags;
> @@ -373,8 +376,13 @@ void fwd_rule_add(struct fwd_ports *fwd, uint8_t flags,
>  
>  	new->to = to;
>  
> +	new->socks = &fwd->listen_socks[fwd->listen_sock_count];
> +	fwd->listen_sock_count += num;
> +
>  	for (port = new->first; port <= new->last; port++) {
> -		/* Fill in the legacy data structures to match the table */
> +		new->socks[port - new->first] = -1;

It's probably saner to initialise these, but just to confirm my
understanding: this is not needed as (unlike what you have for rules)
the array is always compacted and its used length described, right?

> +
> +		/* Fill in the legacy forwarding data structures to match the table */
>  		if (!(new->flags & FWD_SCAN))
>  			bitmap_set(fwd->map, port);
>  		fwd->delta[port] = new->to - new->first;
> diff --git a/fwd.h b/fwd.h
> index 94869c2a..3ddcb91d 100644
> --- a/fwd.h
> +++ b/fwd.h
> @@ -23,6 +23,7 @@ bool fwd_port_is_ephemeral(in_port_t port);
>   * @first:	First port number to forward
>   * @last:	Last port number to forward
>   * @to:		Port number to forward port @first to.
> + * @socks:	Array of listening sockets for this entry
>   * @flags:	Flag mask
>   * 	FWD_DUAL_STACK_ANY - match any IPv4 or IPv6 address (@addr should be ::)
>   *	FWD_WEAK - Don't give an error if binds fail for some forwards
> @@ -34,6 +35,7 @@ struct fwd_rule {
>  	union inany_addr addr;
>  	char ifname[IFNAMSIZ];
>  	in_port_t first, last, to;
> +	int *socks;
>  #define FWD_DUAL_STACK_ANY	BIT(0)
>  #define FWD_WEAK		BIT(1)
>  #define FWD_SCAN		BIT(2)
> @@ -65,6 +67,13 @@ enum fwd_ports_mode {
>  
>  #define PORT_BITMAP_SIZE	DIV_ROUND_UP(NUM_PORTS, 8)
>  
> +/* Maximum number of listening sockets (per pif & protocol)
> + *
> + * Rationale: This lets us listen on every port for two addresses (which we need
> + * for -T auto without SO_BINDTODEVICE), plus a comfortable number of extras.
> + */
> +#define MAX_LISTEN_SOCKS	(NUM_PORTS * 3)
> +
>  /**
>   * fwd_ports() - Describes port forwarding for one protocol and direction
>   * @mode:	Overall forwarding mode (all, none, auto, specific ports)
> @@ -74,6 +83,8 @@ enum fwd_ports_mode {
>   * @rules:	Array of forwarding rules
>   * @map:	Bitmap describing which ports are forwarded
>   * @delta:	Offset between the original destination and mapped port number
> + * @listen_sock_count: Number of entries used in @listen_socks
> + * @listen_socks: Listening sockets for forwarding

To keep those aligned:

/**
 * fwd_ports() - Describes port forwarding for one protocol and direction
 * @mode:		Overall forwarding mode (all, none, auto, some ports)
 * @scan4:		/proc/net fd to scan for IPv4 ports when in AUTO mode
 * @scan6:		/proc/net fd to scan for IPv6 ports when in AUTO mode
 * @count:		Number of forwarding rules
 * @rules:		Array of forwarding rules
 * @map:		Bitmap describing which ports are forwarded
 * @delta:		Offset between original and mapped destination port
 * @listen_sock_count:	Number of entries used in @listen_socks
 * @listen_socks:	Listening sockets for forwarding
 */

>   */
>  struct fwd_ports {
>  	enum fwd_ports_mode mode;
> @@ -83,6 +94,8 @@ struct fwd_ports {
>  	struct fwd_rule rules[MAX_FWD_RULES];
>  	uint8_t map[PORT_BITMAP_SIZE];
>  	in_port_t delta[NUM_PORTS];
> +	unsigned listen_sock_count;
> +	int listen_socks[MAX_LISTEN_SOCKS];
>  };
>  
>  #define FWD_PORT_SCAN_INTERVAL		1000	/* ms */

-- 
Stefano