On Tue, Nov 26, 2024 at 06:54:29AM +0100, Stefano
Brivio wrote:
There are setups where no host interface is
available or configured
at all, intentionally or not, temporarily or not, but users expect
(Podman) containers to run in any case as they did with slirp4netns,
and we're now getting reports that we broke such setups at a rather
alarming rate.
To this end, if we don't find any usable host interface, instead of
exiting:
- for IPv4, use 169.254.2.1 as guest/container address and 169.254.2.2
as default gateway
- for IPv6, don't assign any address (forcibly disable DHCPv6), and
use the *first* link-local address we observe to represent the
guest/container. Advertise fe80::1 as default gateway
- use 'tap0' as default interface name for pasta
Change ifi4 and ifi6 in struct ctx to int and accept a special -1
value meaning that no host interface was selected, but the IP family
is enabled. The fact that the kernel uses unsigned int values for
those is not an issue as 1. one can't create so many interfaces
anyway and 2. we otherwise handle those values transparently.
Fix a botched conditional in conf_print() to actually skip printing
DHCPv6 information if DHCPv6 is disabled (and skip printing NDP
information if NDP is disabled).
Link:
https://github.com/containers/podman/issues/24614
Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>
One remaining concern noted below. Sorry I didn't pick it up on the
previous round.
---
v3: Coverity reports that, in conf(), we might supply a negative
c->ifi4 to if_indextoname() after checking that (!*c->pasta_ifn).
That's a false positive, because if c->ifi4 is -1, we already set
c->pasta_ifn to "tap0", so we won't call if_indextoname() at all,
but, to make my life simpler, add a redundant check on c->ifi4
and c->ifi6 before calling if_indextoname() on them.
conf.c | 96 ++++++++++++++++++++++++++++++++++++++++++++-------------
passt.1 | 33 +++++++++++++++++---
passt.h | 8 ++---
pasta.c | 7 +++--
tap.c | 3 ++
5 files changed, 115 insertions(+), 32 deletions(-)
diff --git a/conf.c b/conf.c
index 86566db..c7aabeb 100644
--- a/conf.c
+++ b/conf.c
@@ -48,6 +48,20 @@
#define NETNS_RUN_DIR "/run/netns"
+#define IP4_LL_GUEST_ADDR (struct in_addr){ htonl_constant(0xa9fe0201) }
+ /* 169.254.2.1, libslirp default: 10.0.2.1 */
+
+#define IP4_LL_GUEST_GW (struct in_addr){ htonl_constant(0xa9fe0202) }
+ /* 169.254.2.2, libslirp default: 10.0.2.2 */
+
+#define IP4_LL_PREFIX_LEN 16
+
+#define IP6_LL_GUEST_GW (struct in6_addr) \
+ {{{ 0xfe, 0x80, 0, 0, 0, 0, 0, 0, \
+ 0, 0, 0, 0, 0, 0, 0, 0x01 }}}
+
+const char *pasta_default_ifn = "tap0";
+
/**
* next_chunk - Return the next piece of a string delimited by a character
* @s: String to search
@@ -631,7 +645,7 @@ static unsigned int conf_ip4(unsigned int ifi, struct ip4_ctx *ip4)
ifi = nl_get_ext_if(nl_sock, AF_INET);
if (!ifi) {
- info("Couldn't pick external interface: disabling IPv4");
+ debug("Failed to detect external interface for IPv4");
return 0;
}
@@ -639,8 +653,8 @@ static unsigned int conf_ip4(unsigned int ifi, struct ip4_ctx *ip4)
int rc = nl_route_get_def(nl_sock, ifi, AF_INET,
&ip4->guest_gw);
if (rc < 0) {
- err("Couldn't discover IPv4 gateway address: %s",
- strerror(-rc));
+ debug("Couldn't discover IPv4 gateway address: %s",
+ strerror(-rc));
return 0;
}
}
@@ -649,8 +663,8 @@ static unsigned int conf_ip4(unsigned int ifi, struct ip4_ctx *ip4)
int rc = nl_addr_get(nl_sock, ifi, AF_INET,
&ip4->addr, &ip4->prefix_len, NULL);
if (rc < 0) {
- err("Couldn't discover IPv4 address: %s",
- strerror(-rc));
+ debug("Couldn't discover IPv4 address: %s",
+ strerror(-rc));
return 0;
}
}
@@ -677,6 +691,19 @@ static unsigned int conf_ip4(unsigned int ifi, struct ip4_ctx *ip4)
return ifi;
}
+/**
+ * conf_ip4_local() - Configure IPv4 addresses and attributes for local mode
+ * @ip4: IPv4 context (will be written)
+ */
+static void conf_ip4_local(struct ip4_ctx *ip4)
+{
+ ip4->addr_seen = ip4->addr = IP4_LL_GUEST_ADDR;
+ ip4->our_tap_addr = ip4->guest_gw = IP4_LL_GUEST_GW;
+ ip4->prefix_len = IP4_LL_PREFIX_LEN;
+
+ ip4->no_copy_addrs = ip4->no_copy_routes = true;
+}
+
/**
* conf_ip6() - Verify or detect IPv6 support, get relevant addresses
* @ifi: Host interface to attempt (0 to determine one)
@@ -693,15 +720,15 @@ static unsigned int conf_ip6(unsigned int ifi, struct ip6_ctx
*ip6)
ifi = nl_get_ext_if(nl_sock, AF_INET6);
if (!ifi) {
- info("Couldn't pick external interface: disabling IPv6");
+ debug("Failed to detect external interface for IPv6");
return 0;
}
if (IN6_IS_ADDR_UNSPECIFIED(&ip6->guest_gw)) {
rc = nl_route_get_def(nl_sock, ifi, AF_INET6, &ip6->guest_gw);
if (rc < 0) {
- err("Couldn't discover IPv6 gateway address: %s",
- strerror(-rc));
+ debug("Couldn't discover IPv6 gateway address: %s",
+ strerror(-rc));
return 0;
}
}
@@ -710,7 +737,7 @@ static unsigned int conf_ip6(unsigned int ifi, struct ip6_ctx *ip6)
IN6_IS_ADDR_UNSPECIFIED(&ip6->addr) ? &ip6->addr : NULL,
&prefix_len, &ip6->our_tap_ll);
if (rc < 0) {
- err("Couldn't discover IPv6 address: %s", strerror(-rc));
+ debug("Couldn't discover IPv6 address: %s", strerror(-rc));
return 0;
}
@@ -726,6 +753,17 @@ static unsigned int conf_ip6(unsigned int ifi, struct ip6_ctx *ip6)
return ifi;
}
+/**
+ * conf_ip6_local() - Configure IPv6 addresses and attributes for local mode
+ * @ip6: IPv6 context (will be written)
+ */
+static void conf_ip6_local(struct ip6_ctx *ip6)
+{
+ ip6->our_tap_ll = ip6->guest_gw = IP6_LL_GUEST_GW;
+
+ ip6->no_copy_addrs = ip6->no_copy_routes = true;
+}
+
/**
* usage() - Print usage, exit with given status code
* @name: Executable name
@@ -948,12 +986,14 @@ static void conf_print(const struct ctx *c)
char bufmac[ETH_ADDRSTRLEN], ifn[IFNAMSIZ];
int i;
- info("Template interface: %s%s%s%s%s",
- c->ifi4 ? if_indextoname(c->ifi4, ifn) : "",
- c->ifi4 ? " (IPv4)" : "",
- (c->ifi4 && c->ifi6) ? ", " : "",
- c->ifi6 ? if_indextoname(c->ifi6, ifn) : "",
- c->ifi6 ? " (IPv6)" : "");
+ if (c->ifi4 > 0 || c->ifi6 > 0) {
+ info("Template interface: %s%s%s%s%s",
+ c->ifi4 > 0 ? if_indextoname(c->ifi4, ifn) : "",
+ c->ifi4 > 0 ? " (IPv4)" : "",
+ (c->ifi4 && c->ifi6) ? ", " : "",
+ c->ifi6 > 0 ? if_indextoname(c->ifi6, ifn) : "",
+ c->ifi6 > 0 ? " (IPv6)" : "");
+ }
if (*c->ip4.ifname_out || *c->ip6.ifname_out) {
info("Outbound interface: %s%s%s%s%s",
@@ -1024,9 +1064,9 @@ static void conf_print(const struct ctx *c)
if (!c->no_ndp && !c->no_dhcpv6)
info("NDP/DHCPv6:");
- else if (!c->no_ndp)
- info("DHCPv6:");
else if (!c->no_dhcpv6)
+ info("DHCPv6:");
+ else if (!c->no_ndp)
info("NDP:");
else
goto dns6;
@@ -1735,8 +1775,20 @@ void conf(struct ctx *c, int argc, char **argv)
c->ifi6 = conf_ip6(ifi6, &c->ip6);
if ((!c->ifi4 && !c->ifi6) ||
(*c->ip4.ifname_out && !c->ifi4) ||
- (*c->ip6.ifname_out && !c->ifi6))
- die("External interface not usable");
+ (*c->ip6.ifname_out && !c->ifi6)) {
The fallback path makes sense for !c->ifi4 && !c->ifi6, but I don't
think it makes sense for the other conditions on this if. Those cover
where the user explicitly gave an interface, but we couldn't use it
for whatever reason. In those cases I think we should outright fail,
rather than falling back to local mode.