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=202506 header.b=oeIhiBbz; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id B31165A0283 for ; Tue, 22 Jul 2025 04:21:13 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202506; t=1753150713; bh=06ofTW+cR3ZRk3ZfpGh0JV07t1/5DQ8AmmzmN/5U4R4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=oeIhiBbz/UvekxNSHgatiCRLeVKK+Emh54Id3ZCIhdof+mtE+U3VtzSnMNLD2U85T +n6hKP4qm2NOfNN+9l5xowMudnxPm5NU1sp+Veb2aRpn7EnlmxXEE7qql96YO0CxCY PYAe04VcinPFGts3Nz3SDxnCXGGPPU6/Uox7W3VgogA56YCpkFgFyK0v2PBF1MDrH+ GT6Fr7dfVcnuP8L7KA0jvXIssDas/JugxqH7i0iH/jZDbnyXoYoC2CC5jK800XVzgU TYiRE1yPX89tQMPtm5jb/xkS+gc5YZ7JjUSMa2n+VrSxvl0awhzY4B8jFXmt6KogFv Jy+P46+D+7bLQ== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4bmLXn22khz4x21; Tue, 22 Jul 2025 12:18:33 +1000 (AEST) Date: Tue, 22 Jul 2025 11:55:57 +1000 From: David Gibson To: Jon Maloy Subject: Re: [PATCH v3 2/8] arp/ndp: respond with true MAC address of LAN local remote hosts Message-ID: References: <20250629171348.86323-1-jmaloy@redhat.com> <20250629171348.86323-3-jmaloy@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="1u2BMxTRuyXhguwW" Content-Disposition: inline In-Reply-To: <20250629171348.86323-3-jmaloy@redhat.com> Message-ID-Hash: 6H3U3RMM4DZLVCW7SUKWWAXYGQY7BT6X X-Message-ID-Hash: 6H3U3RMM4DZLVCW7SUKWWAXYGQY7BT6X 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: sbrivio@redhat.com, dgibson@redhat.com, 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: --1u2BMxTRuyXhguwW Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Jun 29, 2025 at 01:13:41PM -0400, Jon Maloy wrote: > When we receive an ARP request or NDP neigbor solicitation over > the tap interface for a host on the local network segment attached > to the template interface, we respond with that host's real MAC > address. >=20 > The local host, which is acting as a proxy for the default gateway, > is still exempted from this rule. >=20 > Signed-off-by: Jon Maloy >=20 > --- > v3: - Added helper function to find out if a remote ip address is subject > to NAT. This filters out local host addresses which should be > presented with the passt/pasta local MAC address 9a:55:9a:55:9a:55 = even > though it is on the local segment. > - Adapted to the change in nl_mac_get() function, so that we now cons= ider > only the template interface when checking the ARP/NDP table. > --- > arp.c | 9 +++++++++ > fwd.c | 2 +- > fwd.h | 3 ++- > inany.c | 15 +++++++++++++++ > inany.h | 1 + > ndp.c | 9 +++++++++ > 6 files changed, 37 insertions(+), 2 deletions(-) >=20 > diff --git a/arp.c b/arp.c > index fc482bb..1952a63 100644 > --- a/arp.c > +++ b/arp.c > @@ -29,6 +29,7 @@ > #include "dhcp.h" > #include "passt.h" > #include "tap.h" > +#include "netlink.h" > =20 > /** > * arp() - Check if this is a supported ARP message, reply as needed > @@ -40,6 +41,7 @@ > int arp(const struct ctx *c, const struct pool *p) > { > unsigned char swap[4]; > + union inany_addr tgt; > struct ethhdr *eh; > struct arphdr *ah; > struct arpmsg *am; > @@ -72,6 +74,13 @@ int arp(const struct ctx *c, const struct pool *p) > memcpy(am->tha, am->sha, sizeof(am->tha)); > memcpy(am->sha, c->our_tap_mac, sizeof(am->sha)); > =20 > + /* Respond with true MAC address if remote host is on > + * the template interface's network segment > + */ > + inany_from_af(&tgt, AF_INET, am->tip); > + if (!inany_nat(c, &tgt)) > + nl_mac_get(nl_sock, &tgt, c->ifi4, am->sha); > + Hmm. Here's one concern about the overall concept here. If neither the guest nor the host has contacted this neighbour before, it probably won't be in the host's arp/neighbour table so this lookup will fail, and we'll use our_tap_mac. The guest then contacts it, so the host ARPs it. When the guest's ARP times out, it re-ARPs and this time gets the actual MAC address. i.e. it seems to me this approach may substantially increase the odds of the guest seeing a peer change MAC. Not sure if that's a problem. > memcpy(swap, am->tip, sizeof(am->tip)); > memcpy(am->tip, am->sip, sizeof(am->tip)); > memcpy(am->sip, swap, sizeof(am->sip)); > diff --git a/fwd.c b/fwd.c > index 250cf56..02ebc9d 100644 > --- a/fwd.c > +++ b/fwd.c > @@ -332,7 +332,7 @@ static bool fwd_guest_accessible(const struct ctx *c, > * Only handles translations that depend *only* on the address. Anything > * related to specific ports or flows is handled elsewhere. > */ > -static void nat_outbound(const struct ctx *c, const union inany_addr *ad= dr, > +void nat_outbound(const struct ctx *c, const union inany_addr *addr, > union inany_addr *translated) > { > if (inany_equals4(addr, &c->ip4.map_host_loopback)) > diff --git a/fwd.h b/fwd.h > index 0458a3c..61632f2 100644 > --- a/fwd.h > +++ b/fwd.h > @@ -56,5 +56,6 @@ uint8_t fwd_nat_from_splice(const struct ctx *c, uint8_= t proto, > const struct flowside *ini, struct flowside *tgt); > uint8_t fwd_nat_from_host(const struct ctx *c, uint8_t proto, > const struct flowside *ini, struct flowside *tgt); > - > +void nat_outbound(const struct ctx *c, const union inany_addr *addr, > + union inany_addr *translated); > #endif /* FWD_H */ > diff --git a/inany.c b/inany.c > index f5483bf..0ecf10a 100644 > --- a/inany.c > +++ b/inany.c > @@ -16,6 +16,7 @@ > #include "ip.h" > #include "siphash.h" > #include "inany.h" > +#include "fwd.h" > =20 > const union inany_addr inany_loopback4 =3D INANY_INIT4(IN4ADDR_LOOPBACK_= INIT); > const union inany_addr inany_any4 =3D INANY_INIT4(IN4ADDR_ANY_INIT); > @@ -56,3 +57,17 @@ int inany_pton(const char *src, union inany_addr *dst) > =20 > return 0; > } > + > +/** inany_nat - Find if a remote IPv[46] address is subject to NAT > + * @c: Execution context > + * @addr: IPv[46] address > + * > + * Return: true if translated, false otherwise > + */ > +bool inany_nat(const struct ctx *c, const union inany_addr *addr) This certainly doesn't belong in inany.h, it should be in fwd.[ch]. inany.h is about the mechanics of handling inany addresses. This function is about NAT, a much higher level concept. =2E.. and indeed it might be better just to extend nat_outbound() with this functionality rather than writing a new wrapper. > +{ > + union inany_addr addr_nat; > + > + nat_outbound(c, addr, &addr_nat); > + return !inany_equals(addr, &addr_nat); Hrm... I'm having trouble convincing myself is this is the correct logic. Having an address NATted doesn't necessarily preclude sensibly maintaining it's MAC, as long as the translation is consistent and 1 to 1. Though you could certainly argue that if you're translating the L3 address, maintaining the L2 address is kind of meaningless. In the other direction, I'm not sure if we should be preserving MACs for things that are "logically" NATted. That is, they do hit one of the translation cases in nat_outbound(), but it happens that the translated address is the same as the original address. > +} > diff --git a/inany.h b/inany.h > index 7ca5cbd..b2a66b0 100644 > --- a/inany.h > +++ b/inany.h > @@ -278,5 +278,6 @@ static inline void inany_siphash_feed(struct siphash_= state *state, > =20 > const char *inany_ntop(const union inany_addr *src, char *dst, socklen_t= size); > int inany_pton(const char *src, union inany_addr *dst); > +bool inany_nat(const struct ctx *c, const union inany_addr *addr); > =20 > #endif /* INANY_H */ > diff --git a/ndp.c b/ndp.c > index 3e15494..2e7f0bc 100644 > --- a/ndp.c > +++ b/ndp.c > @@ -32,6 +32,7 @@ > #include "passt.h" > #include "tap.h" > #include "log.h" > +#include "netlink.h" > =20 > #define RT_LIFETIME 65535 > =20 > @@ -196,6 +197,7 @@ static void ndp_send(const struct ctx *c, const struc= t in6_addr *dst, > static void ndp_na(const struct ctx *c, const struct in6_addr *dst, > const struct in6_addr *addr) > { > + union inany_addr tgt; > struct ndp_na na =3D { > .ih =3D { > .icmp6_type =3D NA, > @@ -215,6 +217,13 @@ static void ndp_na(const struct ctx *c, const struct= in6_addr *dst, > =20 > memcpy(na.target_l2_addr.mac, c->our_tap_mac, ETH_ALEN); > =20 > + /* Respond with true link-layer address if remote host is on > + * the template interface's network segment > + */ > + inany_from_af(&tgt, AF_INET6, addr); > + if (!inany_nat(c, &tgt)) > + nl_mac_get(nl_sock, &tgt, c->ifi6, na.target_l2_addr.mac); > + > ndp_send(c, dst, &na, sizeof(na)); > } > =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 --1u2BMxTRuyXhguwW Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmh+760ACgkQzQJF27ox 2Gd90hAApDXv3EnwcfHIgHo529FTB+aVMCmDDoLPieMACU6INAf7liE2mMsmblfv 1glNPPHzcfqLEHhXn5OvW0I1L8cY+Cuezy1oQJ5NoF9OfBYwjNPXZ38xUVIzotK2 n5Igvbm0fWfT1Dj+OXh+5Yr/cF4z+xbdPgEm6Y7mTGvqF0UZc0Ilo5D0Z3/RMqDU tI68qJnklBg8QuR7jh45j1jmSFLNSIzbs34zmfpaVvSJR5FsTPd0nLXG8uMwRhBM jMzIn7F/Kvo0Po6Flvr6cmn+AwmliuZfV1lBAgyBxFK+Qmq0YDZHa7l5kZ1yvcit bOi0XUpq5E0Fd6jlYTjsaxFgWlZAuiEJU+UKunQUIlkSB/x1MIqiPafu05cLGR8W Elr8mXRkYLdNu9qXrIgi8oV4I9ZjfO2Mu3jyJcKjH0iauBcnH6S45vxEV0qouUww 907KinzCdFE97CL9gAy6yCWaBXebwaPzp6Z2SW/C2PAHiJ8vH13GjFbRbDuZBIYM DDcfOlauejnJ9zJXRO5chg01FyB4eq31js1eJZDnh0ZUgojEO9EokXAWHX0uV9Sj UulsSYl1d0tdbsqx9fxmaSWDj/QWG0CZ5Br1P3i/5iyXuAdIxtip4WX5tO40kqZe u0uABLeS9hLVlt23dUTxvGoxpEdHhGLO28y5ObyCgHZ8OyVAgqY= =APwA -----END PGP SIGNATURE----- --1u2BMxTRuyXhguwW--