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=oChTqQ3R; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id B23805A0657 for ; Wed, 14 Jan 2026 01:42:07 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202512; t=1768351319; bh=yTF1RibMo8Hqy1spLuBPMrr0OfgYumPMRWIRps51MwE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=oChTqQ3Rw132VH3hmuwjFPJvyLrKW9ovA8lS1MBgL87GuoL1Rv8KdEGv1Mmzhr6M5 cKh/cG9lVmpI03MDUS7XZBZ6smK/bkqA4z49d+E98Vcl1C72npUXUx7F6xIEO600rI pQ1YTwGOS/YsjUHvxvj8Etr7B8Oe/jvbz9GHgKyIgU4JCF2Ktkq1wIlU+heD2kyeJZ i0pEvbu+1gH1eFTg8J7EmNQDOPibHwU0kwpAz7kCsthgRGMtZe6MoDF0+dU5ez8okg zH1Fro7T8shSM4/7JbdaXvKop6nVw+F3Hja4P7fWHnNhlLzcQ3iPgMuuul+0Ctj43q 3MrPhEur4wltQ== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4drS473RdSz4wRQ; Wed, 14 Jan 2026 11:41:59 +1100 (AEDT) Date: Wed, 14 Jan 2026 11:37:41 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v3 13/14] fwd, tcp, udp: Add forwarding rule to listening socket epoll references Message-ID: References: <20260108022948.2657573-1-david@gibson.dropbear.id.au> <20260108022948.2657573-14-david@gibson.dropbear.id.au> <20260113231235.544d72f8@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="WWf8gfTCMDru8wg9" Content-Disposition: inline In-Reply-To: <20260113231235.544d72f8@elisabeth> Message-ID-Hash: 6FJWTWV3JPUVIEYDBVELLXCOTESOL7UU X-Message-ID-Hash: 6FJWTWV3JPUVIEYDBVELLXCOTESOL7UU 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: --WWf8gfTCMDru8wg9 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Jan 13, 2026 at 11:12:35PM +0100, Stefano Brivio wrote: > On Thu, 8 Jan 2026 13:29:47 +1100 > David Gibson wrote: >=20 > > Now that we have a table of all our forwarding rules, every listening > > socket can be associated with a specific rule. Add an index allowing u= s to > > locate that rule from the socket's epoll reference. We don't use it ye= t, > > but we'll use it to optimise rule lookup when forwarding new flows. > >=20 > > Signed-off-by: David Gibson > > --- > > fwd.c | 15 ++++++++++----- > > fwd.h | 5 +++++ > > tcp.c | 4 +++- > > tcp.h | 5 ++--- > > udp.c | 4 +++- > > udp.h | 5 ++--- > > 6 files changed, 25 insertions(+), 13 deletions(-) > >=20 > > diff --git a/fwd.c b/fwd.c > > index 7c4575ff..6727d26f 100644 > > --- a/fwd.c > > +++ b/fwd.c > > @@ -474,6 +474,7 @@ void fwd_rules_print(const struct fwd_ports *fwd) > > =20 > > /** fwd_sync_one() - Create or remove listening sockets for a forward = entry > > * @c: Execution context > > + * @fwd: Forwarding table > > * @rule: Forwarding rule > > * @pif: Interface to create listening sockets for > > * @proto: Protocol to listen for > > @@ -481,19 +482,23 @@ void fwd_rules_print(const struct fwd_ports *fwd) > > * > > * Return: 0 on success, -1 on failure > > */ > > -static int fwd_sync_one(const struct ctx *c, const struct fwd_rule *ru= le, > > +static int fwd_sync_one(const struct ctx *c, > > + const struct fwd_ports *fwd, const struct fwd_rule *rule, > > uint8_t pif, uint8_t proto, const uint8_t *scanmap) > > { > > const union inany_addr *addr =3D fwd_rule_addr(rule); > > const char *ifname =3D rule->ifname; > > bool bound_one =3D false; > > - unsigned port; > > + unsigned port, idx; > > =20 > > ASSERT(pif_is_socket(pif)); > > =20 > > if (!*ifname) > > ifname =3D NULL; > > =20 > > + idx =3D rule - fwd->rules; > > + ASSERT(idx < MAX_FWD_RULES); > > +=09 > > for (port =3D rule->first; port <=3D rule->last; port++) { > > int fd =3D rule->socks[port - rule->first]; > > =20 > > @@ -514,9 +519,9 @@ static int fwd_sync_one(const struct ctx *c, const = struct fwd_rule *rule, > > } > > =20 > > if (proto =3D=3D IPPROTO_TCP) > > - fd =3D tcp_listen(c, pif, addr, ifname, port); > > + fd =3D tcp_listen(c, pif, idx, addr, ifname, port); > > else if (proto =3D=3D IPPROTO_UDP) > > - fd =3D udp_listen(c, pif, addr, ifname, port); > > + fd =3D udp_listen(c, pif, idx, addr, ifname, port); > > else > > ASSERT(0); > > =20 > > @@ -588,7 +593,7 @@ static int fwd_listen_sync_(void *arg) > > ns_enter(a->c); > > =20 > > for (i =3D 0; i < a->fwd->count; i++) { > > - a->ret =3D fwd_sync_one(a->c, &a->fwd->rules[i], > > + a->ret =3D fwd_sync_one(a->c, a->fwd, &a->fwd->rules[i], > > a->pif, a->proto, a->fwd->map); > > if (a->ret < 0) > > break; > > diff --git a/fwd.h b/fwd.h > > index cfe9ed46..435f422a 100644 > > --- a/fwd.h > > +++ b/fwd.h > > @@ -48,14 +48,19 @@ struct fwd_rule { > > * 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 > > + * @rule: Index of forwarding rule > > */ > > union fwd_listen_ref { > > struct { > > in_port_t port; > > uint8_t pif; > > +#define FWD_RULE_BITS 8 > > + unsigned rule :FWD_RULE_BITS; > > }; > > uint32_t u32; > > }; > > +static_assert(sizeof(union fwd_listen_ref) =3D=3D sizeof(uint32_t)); >=20 > Why do we need this, specifically? It goes into the data field of the epoll_ref so it has to be exactly 32-bits. With the bitfields, it's maybe not instantly obvious that the structure isn't larger than that. In particular, this relies on the compiler not inserting padding between @pif and @rule; since alignof(unsigned) =3D=3D 4, typically, I was concerned it might. Even if that is guaranteed by the C standard, I think it's nicer not to require the reader to know that. > > +static_assert(MAX_FWD_RULES <=3D (1U << FWD_RULE_BITS)); >=20 > I start wondering if instead of having a 'rule' field supporting 256 > rules, with 128 as maximum number of rules, we could just have 256 as > maximum number of rules and use the usual MAX_FROM_BITS() macro to keep > things simpler. Good idea, done. Btw, as a later change, I'm considering merging the four forwarding tables into one. If that's done we don't need @pif in the epoll_ref any more (it will be in the rule), and we'll have 16-bits of space if we need to expand the rule table > After all, it's not really rules[] taking space: Certainly. > struct fwd_ports { > enum fwd_ports_mode mode; /* 0 4 */ > int scan4; /* 4 4 */ > int scan6; /* 8 4 */ > unsigned int count; /* 12 4 */ > struct fwd_rule rules[128]; /* 16 7168 */ > /* --- cacheline 112 boundary (7168 bytes) was 16 bytes ago --- */ > uint8_t map[8192]; /* 7184 8192 */ > /* --- cacheline 240 boundary (15360 bytes) was 16 bytes ago --- = */ > unsigned int listen_sock_count; /* 15376 4 */ > int listen_socks[196608]; /* 15380 786432 = */ >=20 > /* size: 801816, cachelines: 12529, members: 8 */ > /* padding: 4 */ > /* last cacheline: 24 bytes */ > }; >=20 > > enum fwd_ports_mode { > > FWD_UNSET =3D 0, > > diff --git a/tcp.c b/tcp.c > > index e9b440da..fc03e38f 100644 > > --- a/tcp.c > > +++ b/tcp.c > > @@ -2672,18 +2672,20 @@ void tcp_sock_handler(const struct ctx *c, unio= n epoll_ref ref, > > * tcp_listen() - Create listening socket > > * @c: Execution context > > * @pif: Interface to open the socket for (PIF_HOST or PIF_SPLICE) > > + * @rule: Index of relevant forwarding rule > > * @addr: Pointer to address for binding, NULL for any > > * @ifname: Name of interface to bind to, NULL for any > > * @port: Port, host order > > * > > * Return: Socket fd on success, negative error code on failure > > */ > > -int tcp_listen(const struct ctx *c, uint8_t pif, > > +int tcp_listen(const struct ctx *c, uint8_t pif, unsigned rule, > > const union inany_addr *addr, const char *ifname, in_port_t po= rt) > > { > > union fwd_listen_ref ref =3D { > > .port =3D port, > > .pif =3D pif, > > + .rule =3D rule, > > }; > > int s; > > =20 > > diff --git a/tcp.h b/tcp.h > > index 45f97d93..24b90870 100644 > > --- a/tcp.h > > +++ b/tcp.h > > @@ -18,9 +18,8 @@ void tcp_sock_handler(const struct ctx *c, union epol= l_ref ref, > > int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, > > const void *saddr, const void *daddr, uint32_t flow_lbl, > > const struct pool *p, int idx, const struct timespec *now); > > -int tcp_listen(const struct ctx *c, uint8_t pif, > > - const union inany_addr *addr, const char *ifname, > > - in_port_t port); > > +int tcp_listen(const struct ctx *c, uint8_t pif, unsigned rule, > > + const union inany_addr *addr, const char *ifname, in_port_t po= rt); > > int tcp_init(struct ctx *c); > > void tcp_timer(const struct ctx *c, const struct timespec *now); > > void tcp_defer_handler(struct ctx *c); > > diff --git a/udp.c b/udp.c > > index 92a87198..761221f6 100644 > > --- a/udp.c > > +++ b/udp.c > > @@ -1115,18 +1115,20 @@ int udp_tap_handler(const struct ctx *c, uint8_= t pif, > > * udp_listen() - Initialise listening socket for a given port > > * @c: Execution context > > * @pif: Interface to open the socket for (PIF_HOST or PIF_SPLICE) > > + * @rule: Index of relevant forwarding rule > > * @addr: Pointer to address for binding, NULL if not configured > > * @ifname: Name of interface to bind to, NULL if not configured > > * @port: Port, host order > > * > > * Return: Socket fd on success, negative error code on failure > > */ > > -int udp_listen(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 po= rt) > > { > > union fwd_listen_ref ref =3D { > > .pif =3D pif, > > .port =3D port, > > + .rule =3D rule, > > }; > > int s; > > =20 > > diff --git a/udp.h b/udp.h > > index 3c6f90a9..2b91d728 100644 > > --- a/udp.h > > +++ b/udp.h > > @@ -14,9 +14,8 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif, > > sa_family_t af, const void *saddr, const void *daddr, > > uint8_t ttl, const struct pool *p, int idx, > > const struct timespec *now); > > -int udp_listen(const struct ctx *c, uint8_t pif, > > - const union inany_addr *addr, const char *ifname, > > - in_port_t port); > > +int udp_listen(const struct ctx *c, uint8_t pif, unsigned rule, > > + const union inany_addr *addr, const char *ifname, in_port_t po= rt); > > int udp_init(struct ctx *c); > > void udp_update_l2_buf(const unsigned char *eth_d); > > =20 >=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 --WWf8gfTCMDru8wg9 Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmlm5VQACgkQzQJF27ox 2GeaDxAAnTgq+ePU6DL9sh6oqeAVOvzoQgNntTIpN5nXnb9HK8+mxZDDf1zFlO+E UpF7aRD90mTKGNHnj/Go2h6uASd3V2mt06Kxwn5z1yAI6Ntt7DJYVeVGxS+l5m01 XD9J44gAbAiekBB7C54VAJ3YLOxPaig2BN7jjKHijh52SvdLdbi9XOUacKWHoDz1 rTCaT3xeNy8TNw7YCBYNMVG9N5pvCUjBaqqhaavVhWcGwpVfBtu/t9LaapncdqY/ aijZIVU2zUgyuPtWIX4AaQk4rAbBjQEKK4yAbPH/xB1s7isoYVXu2iV3UupsiOAG /b24ZGHvB3NK36UpzgadlIg6/MvGqZR0eY1YLtCztUWDfgNx8+92x7D72bvQMF6h uf2UqqCuYCuexlD8F8XNDBcO/pJpZQQVK/3r/6xfGZSr38qBJZzvE6OXFkqseca8 YatTl2w5bwcXnUh7pQQ9PD9ROlSHnbZ9W/RvuBvXit3NYJj49rES02BoOZnJIA2d sIGLpyNGx2rx0JhkgwQvx0lUmLS5KPlZBYghf2R1PCz3uxiy8P2oKPjg7BbrF7aQ xtMvrNnJ7/DoHXfAd0Kq2h7fK2N31pAL3V31cgDk2dO4nLf31yjiOvb5/Q4vojpz 3Rx5MJRN2TNNLLuBqlOiM2qS+55E0GgJMiKLTwsKGRnqE9cZ8IQ= =hpGI -----END PGP SIGNATURE----- --WWf8gfTCMDru8wg9--