From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202512 header.b=KOM880u6; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 24FAE5A0623 for ; Tue, 13 Jan 2026 06:28:59 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202512; t=1768282136; bh=RaR1CZkXwHR46udDsYLxpFeZ9TAO6Ksq0/wVeZOJKyU=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=KOM880u6bKQhk9tou25UddYxn3h0Nol4L6ybXAfKaMjUCwfcB0uW94DEa/UuQ4lsB w5LsmfV+SK7PKMcHNAnTDIyduSBVzOZibxrfaVMmYp9euh0gmtsoyndyWXKjgYKO6p 9vnroNhU4A8fABa7N5zmiD6yLTjPQvhKw7cTfXwdm7DAvBYTphTxFa59a6cr1lb775 Ei4oA0BM4tPhzJ/2OkXOZbsi5JZFtR0eQ+tMWW7LQE8O0QpM5dK408YxfrpqTxatRv Wg/nkC64azpOcOUxfIPNWidKsOtEJe+6Vave+0u52OCO7ucjfYw4nzigxX7CkS5CM3 8+eN9UkAqb5ZA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4dqyTh0TKJz4wHX; Tue, 13 Jan 2026 16:28:56 +1100 (AEDT) Date: Tue, 13 Jan 2026 16:28:27 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v3 05/14] fwd: Make space to store listening sockets in forward table Message-ID: References: <20260108022948.2657573-1-david@gibson.dropbear.id.au> <20260108022948.2657573-6-david@gibson.dropbear.id.au> <20260113002622.48f32d54@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="s2123GV89YT/uUCD" Content-Disposition: inline In-Reply-To: <20260113002622.48f32d54@elisabeth> Message-ID-Hash: FTZSUB2Y73YJ47ICTK4Z5T6FTZKVIIVT X-Message-ID-Hash: FTZSUB2Y73YJ47ICTK4Z5T6FTZKVIIVT X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --s2123GV89YT/uUCD Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Jan 13, 2026 at 12:26:22AM +0100, Stefano Brivio wrote: > On Thu, 8 Jan 2026 13:29:39 +1100 > David Gibson wrote: >=20 > > 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 di= dn't > > need an alternative source of it for the various handlers. > >=20 > > 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. > >=20 > > 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. > >=20 > > 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. > >=20 > > Signed-off-by: David Gibson > > --- > > fwd.c | 10 +++++++++- > > fwd.h | 13 +++++++++++++ > > 2 files changed, 22 insertions(+), 1 deletion(-) > >=20 > > 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 fl= ags, > > { > > /* Flags which can be set from the caller */ > > const uint8_t allowed_flags =3D FWD_WEAK | FWD_SCAN; > > + unsigned num =3D (unsigned)last - first + 1; > > struct fwd_rule *new; > > unsigned port; > > =20 > > @@ -352,6 +353,8 @@ void fwd_rule_add(struct fwd_ports *fwd, uint8_t fl= ags, > > =20 > > if (fwd->count >=3D 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"); >=20 > 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. I'm aware, but for now, these errors are always fatal, so I was going to defer the error plumbing until later. It's not really any harder, and this keeps things a bit simpler while we establish the basic structure of the table. > > new =3D &fwd->rules[fwd->count++]; > > new->flags =3D flags; > > @@ -373,8 +376,13 @@ void fwd_rule_add(struct fwd_ports *fwd, uint8_t f= lags, > > =20 > > new->to =3D to; > > =20 > > + new->socks =3D &fwd->listen_socks[fwd->listen_sock_count]; > > + fwd->listen_sock_count +=3D num; > > + > > for (port =3D new->first; port <=3D new->last; port++) { > > - /* Fill in the legacy data structures to match the table */ > > + new->socks[port - new->first] =3D -1; >=20 > 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? It is needed. Later patches use the fd array to determine if a specific port is _actually_ listening, or just maybe listening. This matters for FWD_WEAK and FWD_SCAN entries. It's nicer to keep it consistent for regular mappings too - that allows fwd_sync_one() to know if it has to do anything or not. As for compaction: I do intend to keep the subarrays used for each rule compacted together, although we don't get allow rules to be deleted, so there's nothing to be done for that yet. I'm not compacting within the subarray, and have no plans to do so at present. > > + > > + /* Fill in the legacy forwarding data structures to match the table = */ > > if (!(new->flags & FWD_SCAN)) > > bitmap_set(fwd->map, port); > > fwd->delta[port] =3D 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 { > > =20 > > #define PORT_BITMAP_SIZE DIV_ROUND_UP(NUM_PORTS, 8) > > =20 > > +/* Maximum number of listening sockets (per pif & protocol) > > + * > > + * Rationale: This lets us listen on every port for two addresses (whi= ch 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 direct= ion > > * @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 num= ber > > + * @listen_sock_count: Number of entries used in @listen_socks > > + * @listen_socks: Listening sockets for forwarding >=20 > To keep those aligned: >=20 > /** > * 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 > */ Done. I'd also be very open to more succinct names for these fields, but they haven't occurred to me yet. > > */ > > 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]; > > }; > > =20 > > #define FWD_PORT_SCAN_INTERVAL 1000 /* ms */ >=20 > --=20 > Stefano >=20 --=20 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 --s2123GV89YT/uUCD Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmll1/oACgkQzQJF27ox 2Gcu1xAAlK98dTC8V2kdMNfMcROEJiGWYQICaJ34jhXEHJzPlG4FEhgbHD98YJiq jNo0sf5pYQNZYsy5dWeC2ofSoVKEOLocA9jnCvrJi6XAKHdVL2ENVjv75U4d6VkG X7YSLCS48wJC9qXy/VOYUdYpkPBItCqMErKX08nO2f415UEWB/rBSuD5hb5iM2U+ B7B08QUK4aPuVftiLXAmfx4IGLoZ+Ucg5tjgFE7ZntIxM7mJ60E3eaU0tvCl5VHx HDICeDAop8v9pMscntL1u068lPNGCXNaR8s3sVX0teY7DBdg223HPnmQ2mppvGK0 FqFecl+LN6Hd09eFNJil2YBfIqeaTyyTOOnFJ6NX7VHKaEHM46mYd/7M1gUop0dU tsniFEPOJBsmfVnr6tZ/RvgbtpsUk5fQ8MvLKsee0ROb44OZNpYhSMV7zV8gJLud 9vwKpA0zRg19Y7ybK8+ZGdYgTrtaEapl7d7i3ad0HXXf7E2zuMAa5HGPHWDhGsIA oznGs0fQ6JfSHX+pH4EQcA6LP2kgvdrDaiHVVI3G2qc71q+KLOH0ZvJeG94Fc1kv h6OQ6/LkOL03i5Jbxcjvz9/A/qd1kApRolEx6mBRCetvYYS66ECelXZuxoAwCKjy xU7MD1TpqYx7D1LycajOyjdZfMSMTzncWVTCcDbrT7zuB1quptE= =VWwy -----END PGP SIGNATURE----- --s2123GV89YT/uUCD--