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

[Jon Maloy <jmaloy@redhat.com>]

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?

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