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=202602 header.b=LXu1BQmK; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 4E8465A026D for ; Wed, 11 Mar 2026 02:39:46 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1773193182; bh=YpFE5WR/Mt4xBfI19ujC0PBdMkBsEXfvktdErYBQxu0=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=LXu1BQmKyEZKIKco0kAAwfOirvbNqx6DUrWTkVrdwtvb9WKNWSOAySmMOdA5kyVzl uCumonKFLenMXHEmu0hbP8f/p7DBUV4Pdu/aATCUSoOnpEusDJGcnGHr+OXduZrVxY ALcbr/zbznfRy99BQtzWHkPEl9DwfeLSZ4b++QAj6pSFhZKPIXa1jZ35X8RQRoZI/I nx7qZ/pAv3mP7Ub6xh+3hFfn66yfo7bmmSURXj5HjSvnmc+lo5CMC7M1uB6yW/v16q 6nUfSakJ+LWTQ4eYZ2qIs+9dx34CJxfovdIh/pjFqlmb4NI7kEzyC2PT6uFqlwSbb4 ZQgBHlxYjxY/w== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4fVtht1CKXz4w9W; Wed, 11 Mar 2026 12:39:42 +1100 (AEDT) Date: Wed, 11 Mar 2026 12:39:35 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 6/6] fwd: Unify TCP and UDP forwarding tables Message-ID: References: <20260310041605.1322552-1-david@gibson.dropbear.id.au> <20260310041605.1322552-7-david@gibson.dropbear.id.au> <20260310203317.06d95fb5@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="hwzo+3v7yHDJK3C7" Content-Disposition: inline In-Reply-To: <20260310203317.06d95fb5@elisabeth> Message-ID-Hash: S5DT2ZTKQAAQAG4ZCO3GVAWGYYHYU3BJ X-Message-ID-Hash: S5DT2ZTKQAAQAG4ZCO3GVAWGYYHYU3BJ 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: --hwzo+3v7yHDJK3C7 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Mar 10, 2026 at 08:33:17PM +0100, Stefano Brivio wrote: > Two nits and one thing that might be a bit more substantial: >=20 > On Tue, 10 Mar 2026 15:16:05 +1100 > David Gibson wrote: >=20 > > Currently TCP and UDP each have their own forwarding tables. This is > > awkward in a few places, where we need switch statements to select the > > correct table. More importantly, it would make things awkward and mess= y to > > extend to other protocols in future, which we're likely to want to do. > >=20 > > Merge the TCP and UDP tables into a single table per (source) pif, with= the > > protocol given in each rule entry. > >=20 > > Signed-off-by: David Gibson [snip] > > static bool fwd_rule_match(const struct fwd_rule *rule, > > - const struct flowside *ini) > > + const struct flowside *ini, uint8_t proto) > > { > > - return inany_matches(&ini->oaddr, fwd_rule_addr(rule)) && > > - ini->oport >=3D rule->first && ini->oport <=3D rule->last; > > + return rule->proto =3D=3D proto && > > + inany_matches(&ini->oaddr, fwd_rule_addr(rule)) && > > + ini->oport >=3D rule->first && ini->oport <=3D rule->last; >=20 > Nit: we usually align things for return clauses like we do with > function calls (like in the existing version), that is, here: >=20 > return rule->proto =3D=3D proto && > inany_matches(&ini->oaddr, fwd_rule_addr(rule)) && > ini->oport >=3D rule->first && ini->oport <=3D rule->last; Ah, right. That's one of the handful of cases where I didn't figure out how to make emacs auto-indent match our style. I tend to forget because it doesn't arise very often [snip] > > diff --git a/fwd.h b/fwd.h > > index 1af13ad4..f6f0fa81 100644 > > --- a/fwd.h > > +++ b/fwd.h > > @@ -31,11 +31,12 @@ bool fwd_port_is_ephemeral(in_port_t port); > > * @first: First port number to forward > > * @last: Last port number to forward > > * @to: Target port for @first, port n goes to @to + (n - @first) > > - * @socks: Array of listening sockets for this entry > > + * @proto: Protocol to forward > > * @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 > > * FWD_SCAN - Only forward if the matching port in the target is liste= ning > > + * @socks: Array of listening sockets for this entry > > * > > * FIXME: @addr and @ifname currently ignored for outbound tables > > */ > > @@ -45,11 +46,12 @@ struct fwd_rule { > > in_port_t first; > > in_port_t last; > > in_port_t to; > > - int *socks; > > + uint8_t proto; >=20 > Nit: as a result, the comment to struct fwd_table should change to: >=20 > * struct fwd_table - Table of forwarding rules, per initiating pif >=20 > as those are not per-protocol anymore. Good point, done. >=20 > And the comment to MAX_LISTEN_SOCKS ("Maximum number of listening > sockets (per pif & protocol)") should also be changed but, at this > point, shouldn't we double that? Or at least NUM_PORTS * 5? Oh, good catch, I didn't think of that. I've updated it to NUM_PORTS * 5, and fixed the comment. > The rest of the series good to me, so I could apply it with the couple > of style fixes I pointed out if you agree... but I guess you should > change MAX_LISTEN_SOCKS (unless I'm missing something), so maybe a > respin would be more convenient at that point. Will do. --=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 --hwzo+3v7yHDJK3C7 Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmmwx9YACgkQzQJF27ox 2GdxIhAApUI21aKskJzWJ+8UneEzwyP0xTR/0RP2Mse7Y76KtrcsyhqOkuZth+Gd Pn2/2e0ozGIsUkTMQj7lLpVoE5BTdMHaSnBnJ1p0R96kWCgwfOWBPdI5d6qZ+Rcy D4W6YhX4rdFPorTDWwsbd7wgM20qKVqGKvkdL92twMOM3iY42npH+9Rk9fF0gklF j9uGxvT9kC8+IpLwzmxJCxQEde9Tt5Od6rYUS6k8NDVvb3D9PXvYQniUNzuSedtM fty6R+6x2v1z6e3NAdg5r25hme4Xni6GlBIOQIR3AgRQCHEsqnjUuyFLXyx/At8m BTLozgRQm1mZSmAH8cuDo/nDlWj++T4Ndv2jBp62Vkm0mxsEQsLy/FOEVi4V5bJs GIlIvuz6ycYzcwjYUDLLMQNxytJCjd29xdQ2NKoMiqr9v8SktCu+miGkacJvAOL6 /Kj8MbkyadhjMkplRGWn4g7NS0UdAnbsrTv0Krrc1nW00dyuRaqEl9GXleLsgHIl Vd/uQGEvP2857HwmMfCenxODZ67VjSsvhKYkEOQDJRUcg47X2lM3wSbAqZxeBUUb KEuawun0xmQ8ljGf3r7oYXPNI7zs784mIE52WmgwikpAxu0Vf3JPt22/7CLThsYn sow/yiEFp1AYcGbZe5+2pWAKSTT1ED+mKasDENt4A11iRSI8En4= =5Q2J -----END PGP SIGNATURE----- --hwzo+3v7yHDJK3C7--