From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 83B6D5A004F for ; Tue, 09 Jul 2024 02:19:23 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1720484352; bh=L3wWm2rc/K0nM/naEXddAwD8OnqUtM14qTENqzclBZg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=gf+ESdoBCeOG8IKcFpblqDxSir+KgVHK8BdCAZpzSseUDshH4VAd5bWlDxQ3n49Uf Qk2d4/j5SIpfoloWcnNJOEo+jFrroQ07X48VJeyT5Cldgf+Kz/NKyNVoseSgcTrrhq K+FWdNLX/42IYrGLwy0SJVsLWmAxGG9ReGug7wrfVIGKOtpRBVuZ5x7iqmF2w4T7OU W9Ej7SsaFVANCPprls9/9qr6iM5JvDpsr+P6o2A7r9LxgvAL2SNERhT+ZgvQOvS3KN scA/u4LBJjKQsH3051miFb7Wtd54mq3tbZ32krZfYW2GHyGsad06OVRw1n+PomnaD9 nxJWO5sMktu2g== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4WJ1nX5K99z4wb2; Tue, 9 Jul 2024 10:19:12 +1000 (AEST) Date: Tue, 9 Jul 2024 10:19:08 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v7 19/27] fwd: Update flow forwarding logic for UDP Message-ID: References: <20240705020724.3447719-1-david@gibson.dropbear.id.au> <20240705020724.3447719-20-david@gibson.dropbear.id.au> <20240708232655.76b43f59@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="wW9mGRBaXkdy5elH" Content-Disposition: inline In-Reply-To: <20240708232655.76b43f59@elisabeth> Message-ID-Hash: ZRXTNNKAJ4JWBY2EFRBEXFWUVF4ORPCB X-Message-ID-Hash: ZRXTNNKAJ4JWBY2EFRBEXFWUVF4ORPCB 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, jmaloy@redhat.com 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: --wW9mGRBaXkdy5elH Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Jul 08, 2024 at 11:26:55PM +0200, Stefano Brivio wrote: > On Fri, 5 Jul 2024 12:07:16 +1000 > David Gibson wrote: >=20 > > Add logic to the fwd_nat_from_*() functions to forwarding UDP packets. = The > > logic here doesn't exactly match our current forwarding, since our curr= ent > > forwarding has some very strange and buggy edge cases. Instead it's > > attempting to replicate what appears to be the intended logic behind the > > current forwarding. > >=20 > > Signed-off-by: David Gibson > > --- > > fwd.c | 26 ++++++++++++++++++++++---- > > 1 file changed, 22 insertions(+), 4 deletions(-) > >=20 > > diff --git a/fwd.c b/fwd.c > > index 5731a536..4377de44 100644 > > --- a/fwd.c > > +++ b/fwd.c > > @@ -169,12 +169,15 @@ 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) > > { > > - (void)proto; > > - > > tgt->eaddr =3D ini->faddr; > > tgt->eport =3D ini->fport; > > =20 > > - if (!c->no_map_gw) { > > + if (proto =3D=3D IPPROTO_UDP && tgt->eport =3D=3D 53) { > > + if (inany_equals4(&tgt->eaddr, &c->ip4.dns_match)) > > + tgt->eaddr =3D inany_from_v4(c->ip4.dns_host); > > + else if (inany_equals6(&tgt->eaddr, &c->ip6.dns_match)) > > + tgt->eaddr.a6 =3D c->ip6.dns_host; > > + } else if (!c->no_map_gw) { >=20 > There's a subtle difference here compared to the logic you dropped in > 23/27 (udp_tap_handler()), which doesn't look correct to me. >=20 > Earlier, with neither c->ip4.dns_match nor c->ip6.dns_match matching, > we would let UDP traffic directed to port 53 be mapped to the host, if > (!c->no_map_gw). That is, the logic was rather equivalent to this: >=20 > if (proto =3D=3D IPPROTO_UDP && tgt->eport =3D=3D 53 && > (inany_equals4(&tgt->eaddr, &c->ip4.dns_match) || > inany_equals6(&tgt->eaddr, &c->ip6.dns_match)) { > if (inany_equals4(&tgt->eaddr, &c->ip4.dns_match)) > tgt->eaddr =3D inany_from_v4(c->ip4.dns_host); > else if (inany_equals6(&tgt->eaddr, &c->ip6.dns_match)) > tgt->eaddr.a6 =3D c->ip6.dns_host; > } else if (!c->no_map_gw) { > ... >=20 > and I think we should maintain it, because if dns_match doesn't match, > DNS traffic considerations shouldn't affect NAT decisions at all. Good catch, I've adjusted that. --=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 --wW9mGRBaXkdy5elH Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmaMgfsACgkQzQJF27ox 2Gc8dRAAp783aS9irJhmch5czI6dRx3x7xqWTtfss/M4DBS6/9nKWtkmyJvltQLe ZmmvvSzZx3dD39/RlDwDJU3DDvDcN5xtk2RKXWeUf9bDF9luZKdQbGol6PT025i4 HDCrSywKVLJ8B06ajnv3IIToczKPw71T3vWM13k5AUtwJLU/r3h0BiNJoi9lylc+ CBaXzcA5J9qxgJrkY52VSmiSFilhyDsu3TWgSYG3zgZk6ul2GsOZp6bEDxt22YXD sbUz9Lsi48Z7H1yqJSIo4SQdAydz3nxD3DwV0M8JX4qR5c51D+idfFHtHZ6tKSRU vMpOeUsobcMfGRrTlcSYbcraSrX8xSSLdgKi9eWnmC8H1sbag+9w8Z1JmNe03BXD shDSmIYZCHUTdy73sEp1SEBErMalXGPPjo7Xd6KShyhv2544GvjGK6r+G9nE1V7Q tBBlXfVXVz5LgiaDY2cRBkuTPgBfyHaIbTeE9OWbl8ufJzJHze99dzMY3hobA/wl UyQNXRHCTpFDEJgPOYwHvJStX4LO9BG0VGfTJpHsR71aXcyl1TToS/PHtWpbBISO Bbh+yJQ3HMCl0YjnN2vblqf1y4W/OwX0/cuCqht2kj3utgHr7hJSAnztiXdTfwvl 0CNQ7wwuZErn3trc/2kH+HvJX1p9PYL5kyyQsBHTYo4VLCILfZY= =VkgA -----END PGP SIGNATURE----- --wW9mGRBaXkdy5elH--