We just had several rapid passt/pasta releases which last minute fixes for the podman 5.0 release. One of these was to correct pasta's selection of which host interface to use. Here are a couple of less urgent follow ups to improve that logic. David Gibson (2): util: Add helper to return name of address family netlink: Ignore routes to link-local addresses for selecting interface ip.h | 9 +++++++++ netlink.c | 19 ++++++++++++++++--- util.h | 18 ++++++++++++++++++ 3 files changed, 43 insertions(+), 3 deletions(-) -- 2.44.0
We have a few places where we want to include the name of the internet protocol version (IPv4 or IPv6) in a message, which we handle with an open-coded ?: expression. This seems like something that might be more widely useful, so make a trivial helper to return the correct string based on the address family. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- netlink.c | 6 +++--- util.h | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/netlink.c b/netlink.c index 9b3dba21..b068aef2 100644 --- a/netlink.c +++ b/netlink.c @@ -309,7 +309,7 @@ unsigned int nl_get_ext_if(int s, sa_family_t af) if (defifi) { if (ndef > 1) info("Multiple default %s routes, picked first", - af == AF_INET ? "IPv4" : "IPv6"); + af_name(af)); return defifi; } @@ -318,11 +318,11 @@ unsigned int nl_get_ext_if(int s, sa_family_t af) return anyifi; info("Multiple interfaces with %s routes, use -i to select one", - af == AF_INET ? "IPv4" : "IPv6"); + af_name(af)); } if (!nany) - info("No interfaces with %s routes", af == AF_INET ? "IPv4" : "IPv6"); + info("No interfaces with %s routes", af_name(af)); return 0; } diff --git a/util.h b/util.h index 0069df34..b89e5c93 100644 --- a/util.h +++ b/util.h @@ -159,6 +159,24 @@ int fls(unsigned long x); int write_file(const char *path, const char *buf); int write_remainder(int fd, const struct iovec *iov, int iovcnt, size_t skip); +/** + * af_name() - Return name of an address family + * @af: Address/protocol family (AF_INET or AF_INET6) + * + * Returns: Name of the protocol family as a string + */ +static inline const char *af_name(sa_family_t af) +{ + switch (af) { + case AF_INET: + return "IPv4"; + case AF_INET6: + return "IPv6"; + default: + return "<unknown address family>"; + } +} + /** * mod_sub() - Modular arithmetic subtraction * @a: Minued, unsigned value < @m -- 2.44.0
Since f919dc7a4b1c ("conf, netlink: Don't require a default route to start"), and since 639fdf06ede ("netlink: Fix selection of template interface") less buggily, we haven't required a default route on the host in order to operate. Instead, if we lack a default route we'll pick an interface with any route, as long as there's only one such interface. If there's more than one, we don't have a good criterion to pick, so we give up with an informational message. Paul Holzinger pointed out that this code considers it ambiguous even if all but one of the interfaces has only routes to link-local addresses (fe80::/10). A route to link-local addresses isn't really useful from pasta's point of view, so ignore them instead. This removes a misleading message in many cases, and a spurious failure in some cases. Suggested-by: Paul Holzinger <pholzing(a)redhat.com> Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- ip.h | 9 +++++++++ netlink.c | 15 ++++++++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/ip.h b/ip.h index b9aedf65..b8d4a5bf 100644 --- a/ip.h +++ b/ip.h @@ -24,6 +24,11 @@ #define IN4ADDR_ANY_INIT \ { .s_addr = htonl_constant(INADDR_ANY) } +#define IN4_IS_ADDR_LINKLOCAL(a) \ + ((ntohl(((struct in_addr *)(a))->s_addr) >> 16) == 0xa9fe) +#define IN4_IS_PREFIX_LINKLOCAL(a, len) \ + ((len) >= 16 && IN4_IS_ADDR_LINKLOCAL(a)) + #define L2_BUF_IP4_INIT(proto) \ { \ .version = 4, \ @@ -40,6 +45,10 @@ #define L2_BUF_IP4_PSUM(proto) ((uint32_t)htons_constant(0x4500) + \ (uint32_t)htons(0xff00 | (proto))) + +#define IN6_IS_PREFIX_LINKLOCAL(a, len) \ + ((len) >= 10 && IN6_IS_ADDR_LINKLOCAL(a)) + #define L2_BUF_IP6_INIT(proto) \ { \ .priority = 0, \ diff --git a/netlink.c b/netlink.c index b068aef2..a722d272 100644 --- a/netlink.c +++ b/netlink.c @@ -33,6 +33,7 @@ #include "util.h" #include "passt.h" #include "log.h" +#include "ip.h" #include "netlink.h" /* Netlink expects a buffer of at least 8kiB or the system page size, @@ -270,6 +271,7 @@ unsigned int nl_get_ext_if(int s, sa_family_t af) seq = nl_send(s, &req, RTM_GETROUTE, NLM_F_DUMP, sizeof(req)); nl_foreach_oftype(nh, status, s, buf, seq, RTM_NEWROUTE) { struct rtmsg *rtm = (struct rtmsg *)NLMSG_DATA(nh); + const void *dst = NULL; unsigned thisifi = 0; if (rtm->rtm_family != af) @@ -284,12 +286,23 @@ unsigned int nl_get_ext_if(int s, sa_family_t af) rtnh = (struct rtnexthop *)RTA_DATA(rta); thisifi = rtnh->rtnh_ifindex; + } else if (rta->rta_type == RTA_DST) { + dst = RTA_DATA(rta); } } if (!thisifi) continue; /* No interface for this route */ + /* Skip routes to link-local addresses */ + if (af == AF_INET && dst && + IN4_IS_PREFIX_LINKLOCAL(dst, rtm->rtm_dst_len)) + continue; + + if (af == AF_INET6 && dst && + IN6_IS_PREFIX_LINKLOCAL(dst, rtm->rtm_dst_len)) + continue; + if (rtm->rtm_dst_len == 0) { /* Default route */ ndef++; @@ -322,7 +335,7 @@ unsigned int nl_get_ext_if(int s, sa_family_t af) } if (!nany) - info("No interfaces with %s routes", af_name(af)); + info("No interfaces with usable %s routes", af_name(af)); return 0; } -- 2.44.0
On Thu, 21 Mar 2024 15:04:47 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote:We just had several rapid passt/pasta releases which last minute fixes for the podman 5.0 release. One of these was to correct pasta's selection of which host interface to use. Here are a couple of less urgent follow ups to improve that logic. David Gibson (2): util: Add helper to return name of address family netlink: Ignore routes to link-local addresses for selecting interfaceApplied. -- Stefano