* [PATCH 1/2] util: Add helper to return name of address family
2024-03-21 4:04 [PATCH 0/2] Follow up improvements to external interface selection David Gibson
@ 2024-03-21 4:04 ` David Gibson
2024-03-21 4:04 ` [PATCH 2/2] netlink: Ignore routes to link-local addresses for selecting interface David Gibson
2024-04-05 18:07 ` [PATCH 0/2] Follow up improvements to external interface selection Stefano Brivio
2 siblings, 0 replies; 4+ messages in thread
From: David Gibson @ 2024-03-21 4:04 UTC (permalink / raw)
To: passt-dev, Stefano Brivio; +Cc: Paul Holzinger, 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
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 2/2] netlink: Ignore routes to link-local addresses for selecting interface
2024-03-21 4:04 [PATCH 0/2] Follow up improvements to external interface selection David Gibson
2024-03-21 4:04 ` [PATCH 1/2] util: Add helper to return name of address family David Gibson
@ 2024-03-21 4:04 ` David Gibson
2024-04-05 18:07 ` [PATCH 0/2] Follow up improvements to external interface selection Stefano Brivio
2 siblings, 0 replies; 4+ messages in thread
From: David Gibson @ 2024-03-21 4:04 UTC (permalink / raw)
To: passt-dev, Stefano Brivio; +Cc: Paul Holzinger, 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
^ permalink raw reply related [flat|nested] 4+ messages in thread