From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 23A365A004F for ; Wed, 24 Jul 2024 09:51:17 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1721807474; bh=AK5EOVbw6jLfypktgsk9+kDldVV5xWavkcD1Ut9TOUI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=J+A2s5hKKU+p764XlMCX29NskbTFk1H/8PprpTJCVyMrZhSWqzF3PeyQcvO0vvvlV JueTfYZ1b58d1ZWZZSdcdvLtCJZ8rWHjiAslNx07sUJ9uwyJBwHq10Yi47R4fFUlAC HnamvkB9Ki25Ff3JzsIv+CI2B9w/qMsr/ejZte4/iUxjyFfe3HJQaecgnqv18rpQ62 TedtV4jxzlrDUqOyE29DyaNGdiPRJqXaIpkSq1xFFReVJWcq2kBLmua9oXcuUqglAH A7XOdoPpOplAb8wbgltjUXKYjl79quPH13a8F5rU+W0eQNaAK/UUUP+DwmZy5fzFvO L+H+YpD7XAUdA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4WTR6B6p2kz4x3J; Wed, 24 Jul 2024 17:51:14 +1000 (AEST) Date: Wed, 24 Jul 2024 17:46:03 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 1/2] fwd: Refactor tests in fwd_nat_from_tap() for clarity Message-ID: References: <20240724062058.1259033-1-david@gibson.dropbear.id.au> <20240724062058.1259033-2-david@gibson.dropbear.id.au> <20240724083859.2a97e66c@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="BCVQ41UgOuhHgi7V" Content-Disposition: inline In-Reply-To: <20240724083859.2a97e66c@elisabeth> Message-ID-Hash: 67WFJU2NG2GLPM3MQ247AW6EDEVXWP3L X-Message-ID-Hash: 67WFJU2NG2GLPM3MQ247AW6EDEVXWP3L 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: --BCVQ41UgOuhHgi7V Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jul 24, 2024 at 08:38:59AM +0200, Stefano Brivio wrote: > On Wed, 24 Jul 2024 16:20:57 +1000 > David Gibson wrote: >=20 > > Currently, we start by handling the common case, where we don't transla= te > > the destination address, then we modify the tgt side for the special ca= ses. > > In the process we do comparisons on the tentatively set fields in tgt, > > which obscures the fact that tgt should be an essentially pure function= of > > ini, and risks people examining fields of tgt that are not yet initiali= zed. > >=20 > > To make this clearer, do all our tests on 'ini', constructing tgt from > > scratch on that basis. > >=20 > > Signed-off-by: David Gibson > > --- > > fwd.c | 22 +++++++++++----------- > > 1 file changed, 11 insertions(+), 11 deletions(-) > >=20 > > diff --git a/fwd.c b/fwd.c > > index 8c1f3d91..bd16ac42 100644 > > --- a/fwd.c > > +++ b/fwd.c > > @@ -169,22 +169,22 @@ void fwd_scan_ports_init(struct ctx *c) > > uint8_t fwd_nat_from_tap(const struct ctx *c, uint8_t proto, > > const struct flowside *ini, struct flowside *tgt) > > { > > - tgt->eaddr =3D ini->faddr; > > - tgt->eport =3D ini->fport; > > - > > - if (proto =3D=3D IPPROTO_UDP && tgt->eport =3D=3D 53 && > > - inany_equals4(&tgt->eaddr, &c->ip4.dns_match)) { >=20 > These blocks are all one-liners now and curly brackets could go away, I > guess? Good point, done. > > + if (proto =3D=3D IPPROTO_UDP && ini->fport =3D=3D 53 && > > + inany_equals4(&ini->faddr, &c->ip4.dns_match)) { > > tgt->eaddr =3D inany_from_v4(c->ip4.dns_host); > > - } else if (proto =3D=3D IPPROTO_UDP && tgt->eport =3D=3D 53 && > > - inany_equals6(&tgt->eaddr, &c->ip6.dns_match)) { > > + } else if (proto =3D=3D IPPROTO_UDP && ini->fport =3D=3D 53 && > > + inany_equals6(&ini->faddr, &c->ip6.dns_match)) { > > tgt->eaddr.a6 =3D c->ip6.dns_host; > > - } else if (!c->no_map_gw) { > > - if (inany_equals4(&tgt->eaddr, &c->ip4.gw)) > > - tgt->eaddr =3D inany_loopback4; > > - else if (inany_equals6(&tgt->eaddr, &c->ip6.gw)) > > + } else if (!c->no_map_gw && inany_equals4(&ini->faddr, &c->ip4.gw)) { > > + tgt->eaddr =3D inany_loopback4; > > + } else if (!c->no_map_gw && inany_equals6(&ini->faddr, &c->ip6.gw)) { > > tgt->eaddr =3D inany_loopback6; >=20 > Resulting excess tab here. Fixed. > > + } else { > > + tgt->eaddr =3D ini->faddr; > > } > > =20 > > + tgt->eport =3D ini->fport; > > + > > /* The relevant addr_out controls the host side source address. This > > * may be unspecified, which allows the kernel to pick an address. > > */ >=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 --BCVQ41UgOuhHgi7V Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmagsToACgkQzQJF27ox 2Gcw+Q//dtuwhtsnWppaJ98OYDZ5MLGbylghLS++1i0zfRPV2nVAPz5Lc/PG+icF DGpHnQ38kgV+sIaCzGZ31HPfycRwpUi0YuFWalvIxoppsIkZl3e1cHLSDlFmeZza 43IA+KFj4/JaG7gjbHXVKweydUZ3xLuoYGp0hivPkhVpDeVhiJ+rUpVWWfwUXcrb bA18QhoiUbvXMtyGcagFLDp6sixxIt0Sq06ZLng3QMEKslRqmibNjPpC3t6RIzBx tyRIywZgXypMcT26HbLNMpFkdhcNe735682Y0OO/ug1wOxDlWGj6qTaMGAmp36D4 0fz4Ab5FXMNjvoy2kMP35NeBPLGDiSZB703HLSj2EdaV4IGFrrC4HFkVpsC75kaP UIVb8mYaxY4R8gH4NnlwbvX1tAHLIAcARAfY4nPBzd/nzBPiMeAGMBFTbliXbzS/ 3pd0sjECZJRNMWIxV4PAYolaYwzNKfjztla/Af1yabK0R9KTH2Ue603alIHq1dRB xXDyvpCDGMgF/dCRjmKejD8er71svI18qeUhUGWzViRCo+d6Qgo8kx7HTSeqFCQ/ 1oeSGD0IGBxCTv3cdECtBlF7M1t1xh2pK5soLks8qloVuoLgWnZsOjNe5OUUTfHt ffzCUnWcXAqZkflj6x8RYh6FfAygww1aLHP1fdgziFB0bBSPjrs= =ld3D -----END PGP SIGNATURE----- --BCVQ41UgOuhHgi7V--