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

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

[David Gibson]

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

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

[David Gibson]

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

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

[David Gibson]

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@redhat.com>
Signed-off-by: David Gibson <david@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;
 }
-- 
@@ -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

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

[Stefano Brivio]

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

Applied.

-- 
Stefano