Re: [PATCH v14 03/10] fwd: Add cache table for ARP/NDP contents

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 5800 bytes --]

On Mon, Oct 20, 2025 at 06:00:37AM -0400, Jon Maloy wrote:
> 
> 
> On 2025-10-19 20:06, David Gibson wrote:
> > On Fri, Oct 17, 2025 at 02:49:33PM -0400, Jon Maloy wrote:
> [...]
> > > 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.
> 
> So, your are telling me this is expected behaviour?

As discussed on our call, yes.  It's weird, but it's the most logical
thing we can do under the circumstances.

> 
> jmaloy@mimir:~/passt/tests/udp_test$ /home/jmaloy/passt/passt/pasta
> --config-net --no-splice -d -a 192.168.2.2 -g 192.168.2.3 --no-map-gw
> Template interface: wlp1s0 (IPv4), wlp1s0 (IPv6)
> Namespace interface: wlp1s0
> MAC:
>     host: 9a:55:9a:55:9a:55
> DHCP:
>     assign: 192.168.2.2
>     mask: 255.255.255.0
>     router: 192.168.2.3
> DNS:
>     10.11.5.19
>     10.2.32.1
> DNS search list:
>     redhat.com
>     rmtcaqc.csb
> NDP/DHCPv6:
>     assign: 2001:4958:2193:9901:6217:960c:2ef1:f0f3
>     router: fe80::c23c:4ff:fe04:4638
>     our link-local: fe80::c23c:4ff:fe04:4638
> DNS search list:
>     redhat.com
>     rmtcaqc.csb
> Sending initial ARP request for guest MAC address
> Sending initial NDP NS request for guest MAC address
> SO_PEEK_OFF supported
> TCP_INFO tcpi_snd_wnd field  supported
> TCP_INFO tcpi_bytes_acked field  supported
> TCP_INFO tcpi_min_rtt field  supported
> root@mimir:~/passt/tests/udp_test# ip r
> default via 192.168.2.3 dev wlp1s0
> 192.168.2.0/24 dev wlp1s0 proto kernel scope link src 192.168.2.2
> root@mimir:~/passt/tests/udp_test# ping 192.168.2.3
> PING 192.168.2.3 (192.168.2.3) 56(84) bytes of data.
> ^C^
> --- 192.168.2.3 ping statistics ---
> 5 packets transmitted, 0 received, 100% packet loss, time 4080ms
> 
> root@mimir:~/passt/tests/udp_test# ping 8.8.8.8
> PING 8.8.8.8 (8.8.8.8) 56(84) bytes of data.
> 64 bytes from 8.8.8.8: icmp_seq=1 ttl=255 time=7.68 ms
> 64 bytes from 8.8.8.8: icmp_seq=2 ttl=255 time=20.9 ms
> 64 bytes from 8.8.8.8: icmp_seq=3 ttl=255 time=19.1 ms
> ^C
> --- 8.8.8.8 ping statistics ---
> 3 packets transmitted, 3 received, 0% packet loss, time 2002ms
> rtt min/avg/max/mdev = 7.676/15.889/20.876/5.851 ms
> root@mimir:~/passt/tests/udp_test#
> 
> ///jon
> 
> > 
> > > > 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.
> > > > 
> > > > > +		uint8_t mac[ETH_ALEN];
> > > > > +		int rc;
> > > > > +
> > > > > +		rc = 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);
> > > > > +		}
> > > > 
> > > > 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.
> > > > 
> > > > 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:
> > > > 
> > > >    fwd_neigh_table_update(c, nat_inbound(host_addr), host_mac, true);
> > > > 
> > > > 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).
> > > > 
> > > > 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.
> > > > 
> > > > 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.
> > > > 
> > > 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.
> > > 
> > > ///jon
> > > 
> > > 
> > 
> 

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]