On Tue, Aug 20, 2024 at 09:56:24PM +0200, Stefano Brivio wrote:On Fri, 16 Aug 2024 15:39:58 +1000 David Gibson <david(a)gibson.dropbear.id.au> wrote:Currently, when we don't have a gateway address on the host: no connectivity, or a point-to-point link with no gateway, or the like. We used to absolutely require it, but that restriction has been eased and may ease further in future.ip4.gw conflates 3 conceptually different things, which (for now) have the same value: 1. The router/gateway address as seen by the guest 2. An address to NAT to the host with --no-map-gw isn't specified 3. An address to use as source when nothing else makes sense Case 3 occurs in two situations: a) for our DHCP responses - since they come from passt internally there's no naturally meaningful address for them to come from b) for forwarded connections coming from an address that isn't guest accessible (localhost or the guest's own address). (b) occurs even with --no-map-gw, and the expected behaviour of forwarding local connections requires it. For IPv6 role (3) is now taken by ip6.our_tap_ll (which usually has the same value as ip6.gw). For future flexibility we may want to make this "address of last resort" different from the gateway address, so split them logically for IPv4 as well. Specifically, add a new ip4.our_tap_addr field for the address with this role, and initialise it to ip4.gw for now. Unlike IPv6 where we can always get a link-local address, we might not be able to get a (non 0.0.0.0) address here. In that case we have to disable DHCPIt's not entirely clear to me in which case we would not be able to get any address,but at least RFC 2131 doesn't have a problem with this: diff --git a/dhcp.c b/dhcp.c index aa9f59d..3de8a6e 100644 --- a/dhcp.c +++ b/dhcp.c @@ -282,6 +282,7 @@ int dhcp(const struct ctx *c, const struct pool *p) struct in_addr mask; unsigned int i; struct msg *m; + struct in_addr zeroes = { 0 }; eh = packet_get(p, 0, offset, sizeof(*eh), NULL); offset += sizeof(*eh); @@ -378,7 +379,7 @@ int dhcp(const struct ctx *c, const struct pool *p) opt_set_dns_search(c, sizeof(m->o)); dlen = offsetof(struct msg, o) + fill(m); - tap_udp4_send(c, c->ip4.gw, 67, c->ip4.addr, 68, m, dlen); + tap_udp4_send(c, zeroes, 67, c->ip4.addr, 68, m, dlen); return 1; } and: $ ./pasta -p dhcp.pcap Saving packet capture to dhcp.pcap # dhclient # tshark -r dhcp.pcap Running as user "root" and group "root". This could be dangerous. 1 0.000000 :: → ff02::16 ICMPv6 90 Multicast Listener Report Message v2 2 0.016265 0.0.0.0 → 255.255.255.255 DHCP 342 DHCP Discover - Transaction ID 0x75759d11 3 0.016361 0.0.0.0 → 88.198.0.164 DHCP 342 DHCP Offer - Transaction ID 0x75759d11 4 0.016479 0.0.0.0 → 255.255.255.255 DHCP 342 DHCP Request - Transaction ID 0x75759d11 5 0.016493 0.0.0.0 → 88.198.0.164 DHCP 342 DHCP ACK - Transaction ID 0x75759d11 [...] so this could be a reasonable fallback.Fair point. I've removed the disabling of DHCP in this case.Makes sense, done. -- 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/~dgibsonand forwarding of inbound connections with guest-inaccessible source addresses. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- conf.c | 7 ++++++- dhcp.c | 4 ++-- fwd.c | 10 +++++++--- passt.h | 2 ++ 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/conf.c b/conf.c index 954f20ea..9f962fc8 100644 --- a/conf.c +++ b/conf.c @@ -660,6 +660,8 @@ static unsigned int conf_ip4(unsigned int ifi, ip4->addr_seen = ip4->addr; + ip4->our_tap_addr = ip4->gw; + if (MAC_IS_ZERO(mac)) { int rc = nl_link_get_mac(nl_sock, ifi, mac); if (rc < 0) { @@ -1666,7 +1668,10 @@ void conf(struct ctx *c, int argc, char **argv) die("External interface not usable"); if (c->ifi4 && IN4_IS_ADDR_UNSPECIFIED(&c->ip4.gw)) - c->no_map_gw = c->no_dhcp = 1; + c->no_map_gw = 1; + + if (c->ifi4 && IN4_IS_ADDR_UNSPECIFIED(&c->ip4.our_tap_addr)) + c->no_dhcp = 1; if (c->ifi6 && IN6_IS_ADDR_UNSPECIFIED(&c->ip6.gw)) c->no_map_gw = 1; diff --git a/dhcp.c b/dhcp.c index acc5b03e..a935dc94 100644 --- a/dhcp.c +++ b/dhcp.c @@ -347,7 +347,7 @@ int dhcp(const struct ctx *c, const struct pool *p) mask.s_addr = htonl(0xffffffff << (32 - c->ip4.prefix_len)); memcpy(opts[1].s, &mask, sizeof(mask)); memcpy(opts[3].s, &c->ip4.gw, sizeof(c->ip4.gw)); - memcpy(opts[54].s, &c->ip4.gw, sizeof(c->ip4.gw)); + memcpy(opts[54].s, &c->ip4.our_tap_addr, sizeof(c->ip4.our_tap_addr));Nit: this was supposed to look like a table, so it would be nice to add extra whitespace in the lines above this one.