[PATCH 0/2] Follow up improvements to external interface selection - dev - passt

newer
[PATCH v2 0/5] Sandbox test suite...

[PATCH 0/2] Follow up improvements to external interface selection

older
[PATCH v2] netlink: Adjust...

David Gibson

21 Mar 2024 21 Mar '24

5:04 a.m.

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

Reply

Sign in to reply online Use email software

Show replies by date

David Gibson

21 Mar 21 Mar

5:04 a.m.

New subject: [PATCH 1/2] util: Add helper to return name of address family

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

Reply

Sign in to reply online Use email software

David Gibson

5:04 a.m.

New subject: [PATCH 2/2] netlink: Ignore routes to link-local addresses for selecting interface

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 Signed-off-by: David Gibson --- 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

Reply

Sign in to reply online Use email software

Stefano Brivio

5 Apr 5 Apr

8:07 p.m.

On Thu, 21 Mar 2024 15:04:47 +1100 David Gibson 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 interface

Applied. -- Stefano

Reply

Sign in to reply online Use email software

423

Age (days ago)

438

Last active (days ago)

Download

3 comments

2 participants

tags

participants (2)

David Gibson
Stefano Brivio