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=202510 header.b=CEuizx5K; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 96CE55A026F for ; Mon, 20 Oct 2025 02:18:08 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202510; t=1760919485; bh=SXUihI0YFXipsZwdBx55tR6WKte3CiKedj8eLarVzCI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=CEuizx5KvFwVvBNDEXk9q7600pCGXvI4cBn6YWF5mA5YvxoytZulQvgoUC6JwAI3G ssKmL7TV0nhElh8e+gjoY4nUItHNMWiIajj+WZburpY0ssiWDQoRAVFe65z9OIGNlv KWUVyhJRy8HhBBP4TCu7LsH0fO5T5d6ZTZIhMjtS/YOrLvDvtvV2KrC1pC7LPpHNEQ YzUaK8pvW7crflkh/M9nQo3eQuNrGoqrd2rqkPuNyzgnSwq4aR50qCHjpgn2MUR4QV JuaIwFa83rNhQu9MiBBl1M9p2x10d5PJO2SxT4zOkZiSk1bSiInQhy3ZlGbZfdGDbe NkSAcFoZYv8vw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4cqbcF1WBYz4wBJ; Mon, 20 Oct 2025 11:18:05 +1100 (AEDT) Date: Mon, 20 Oct 2025 11:06:23 +1100 From: David Gibson To: Jon Maloy Subject: Re: [PATCH v14 03/10] fwd: Add cache table for ARP/NDP contents Message-ID: References: <20251015025521.1449156-1-jmaloy@redhat.com> <20251015025521.1449156-4-jmaloy@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="iqpmS85Z9Hb2JFIw" Content-Disposition: inline In-Reply-To: Message-ID-Hash: RWGMHGD54ADJFYGYIQ3LTVTTMAKZN4W6 X-Message-ID-Hash: RWGMHGD54ADJFYGYIQ3LTVTTMAKZN4W6 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: --iqpmS85Z9Hb2JFIw Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Oct 17, 2025 at 02:49:33PM -0400, Jon Maloy wrote: >=20 >=20 > On 2025-10-16 23:05, David Gibson wrote: > > On Tue, Oct 14, 2025 at 10:55:14PM -0400, Jon Maloy wrote: > > > We add a cache table to keep track of the contents of the kernel ARP > > > and NDP tables. The table is fed from the just introduced netlink bas= ed > > > neigbour subscription function. > > >=20 > > > Signed-off-by: Jon Maloy > >=20 > > Reviewed-by: David Gibson > >=20 > > I do see one error here, but it's fairly harmless, so I think a follow > > up makes more sense than a respin. > >=20 > > [snip] > > > + /* Blocker entries to stop events from hosts using these addresses = */ > > > + if (!inany_is_unspecified4(&mhl)) > > > + fwd_neigh_table_update(c, &mhl, c->our_tap_mac, true); > > > + > > > + if (!inany_is_unspecified4(&ggw) && !c->no_map_gw) > > > + fwd_neigh_table_update(c, &ggw, c->our_tap_mac, true); > > > + > > > + if (!inany_is_unspecified4(&mga) && !inany_equals(&mhl, &mga)) { > >=20 > > That made me realise that we should throw an error during > > configuration if map_host_loopback =3D=3D map_guest_addr. It doesn't m= ake > > sense for these to be the same - if they are, we have no way of > > knowing if a packet should be mapped to 127.0.0.1 or to guest_addr. > > *checks* looks like map_host_loopback will take precedence in this > > case, because of the way nat_outbound() is ordered. >=20 > I also noticed it is possible to set guest_gw to some random address while > at the same time setting no_map_gw. It seems to be harmless, since no_map= _gw > takes precedence, but it is an inconsistency we should fence > against. No, the existing behaviour is correct. It gets confusing, because the guest_gw is used for magic NATs, that's often what we're referring to when we discuss the guest_gw address. But the guest_gw is *also* exactly what it says on the tin: the gateway address the guest uses. Without that, the guest won't have connectivity at all, so we need it. --no-map-gw means we don't have the magic NATs, but there's still a gateway, and -g still does and should control its address. > > In any case I think you can drop the inany_equals() test - the > > permanent bit will stop the second update from clobbering the first, > > even if we are misconfigured. > >=20 > > > + uint8_t mac[ETH_ALEN]; > > > + int rc; > > > + > > > + rc =3D nl_link_get_mac(nl_sock, c->ifi4, mac); > > > + if (rc < 0) { > > > + debug("Couldn't get ip4 MAC addr: %s", strerror_(-rc)); > > > + memcpy(mac, c->our_tap_mac, ETH_ALEN); > > > + } > >=20 > > Using the host's MAC for --map-guest-addr only makes sense if the > > guest address is the same as the host address. If -a is used to make > > the guest address different, then it may shadow some other random > > node, not the host. We don't need special handling for that case - > > the nat_inbound() you already have will do what we need. > >=20 > > IIUC, the host itself doesn't appear in the neighbour table, so we do > > need special handling if we want to use the host MAC when > > --map-guest-addr *does* refer to the host. To handle that, I think > > what we want is pseudo-codishly: > >=20 > > fwd_neigh_table_update(c, nat_inbound(host_addr), host_mac, true); > >=20 > > The wrinkle is that while we do get the host address at some point, > > I'm not sure we keep it around (it's typically irrelevant after init). > >=20 > > Strictly speaking 'permanent' isn't really correct here, but it's not > > worth the hassle of setting up a whole other netlink monitor to watch > > for changes in the host's MAC address. > >=20 > > In fact.. I'm not sure it's worth handling this case at all. I think > > it would be ok to just drop this clause. That means we'll use > > our_tap_mac by default for things NATted to the (non loopback) host, > > which is probably fine. > >=20 > Fine with me, but I do think we need a blocker entry just in case somebody > else comes up with the same address. We don't need a blocker. If someone else (let's call them X) comes up with that address, that implies it's not the host's address. If that's so then --map-guest-gw will NAT to X, and that's intended. In which case it also makes sense to use X's MAC address. We don't need to do anything special to make that happen - X will appear in the host neigh table, and the nat_inbound() call will put it into the slot of the --map-guest-gw address. > I'll post a complementary commit once the series has been applied. >=20 > ///jon >=20 >=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 --iqpmS85Z9Hb2JFIw Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmj1fPsACgkQzQJF27ox 2Gc3Zw//Whx0khHO4GB1Qw9IduJMX/Rp4QUX17zyyPHhlCr2gO7oSAkPnl9P5oCV yspK/nW+Jtn1/GRAPRCQJwrzJGUXkENmmNkxXNkXoBLUgowgIrAgI9ltDOOq9gRf 6IAQxFbhkukMkEUFS3vO3aXLZ35oWREWYr/lxWGv3AODzsMXlBrq33j7A8OQHxRb EepDHtsVqGRnQ5nbfXZWDx8ZgVrfhMJaOuDIwW1U+nHrlxkYkMzJ5SqayPKVPiUD ci5g1beA2NtmxeMvktx5WTGHelrfZmixxouIH3m33VIHviyv2zQNvMooOy0C6K5C PjBC4Kuy9ChszIBdqesjAGCUhgDhLUiMGwPBM4E4gumddmaRuGI1tLnBm3Mk3P0z P34LiiYWWVfmRrjGoX9PDEjtJWJ8PUbVae2sRG9jjRVmNRvQb+c7IfI5kDJDF7c2 lNWfYzPvW54dfEd3JwbL/t+Hw53JrbLKYTnSdk424hVgnDI+VAfaJ4G53i+upveD +KWSLaqbO8g+BZNY2pW5975P0zmuB6v/L+19CmGGwsE65j+esnFKzWZXdV5VUTVO DMIjGFfFAsscrOW799pWz3q1DtvyNqLRkxtVL6ZKFQrsuI//0Uw0edno+z5tN4k5 x0Z1PTNX/YYNKYfMjRGdvTZM2VpyEKPbsOR3s7rgvBKi1RT01fg= =JPhq -----END PGP SIGNATURE----- --iqpmS85Z9Hb2JFIw--