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 based neigbour subscription function.
Signed-off-by: Jon Maloy
Reviewed-by: David Gibson
I do see one error here, but it's fairly harmless, so I think a follow up makes more sense than a respin.
[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)) {
That made me realise that we should throw an error during configuration if map_host_loopback == map_guest_addr. It doesn't make 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.
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.
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. I'll post a complementary commit once the series has been applied. ///jon