[PATCH 0/3] More graceful handling of kernels without SO_BINDTODEVICE

[PATCH 0/3] More graceful handling of kernels without SO_BINDTODEVICE

[David Gibson]

Kernels before v5.7 don't allow unprivileged processes to use the
SO_BINDTODEVICE socket option, which is how we implement per-device
listening (e.g. "-t ""%eth0/80").  Currently we detect this error
quite late, and don't produce a particularly helpful error message.

Improve error messages in this case by explicitly probing for support
of the option early.

More importantly this will allow me to implement fallback handling for
older kernels in upcoming socket binding cleanups.

Note: this series applies directly on the current main branch, I'm
going to rebase my socket binding series on top of this, instead of
the other way around.

David Gibson (3):
  util: Correct error message on SO_BINDTODEVICE failure
  util: Extend sock_probe_mem() to sock_probe_features()
  conf: More useful errors for kernels without SO_BINDTODEVICE

 conf.c  |  6 ++++++
 passt.c |  2 +-
 passt.h |  1 +
 util.c  | 26 +++++++++++++++++++++-----
 util.h  |  2 +-
 5 files changed, 30 insertions(+), 7 deletions(-)

-- 
2.51.1

[PATCH 1/3] util: Correct error message on SO_BINDTODEVICE failure

[David Gibson]

The warning message we print of SO_BINDTODEVICE fails is incorrect: we
include EPOLL_TYPE_STR(proto), but the proto variable is not an epoll type
so this will generate misleading garbage.  This has been wrong ever since
340164445 but the message is rare enough that we never noticed.

Correct that, and reword the message a bit for clarity while we're
there.

Fixes: 340164445341 ("epoll: Generalize epoll_ref to cover things other than sockets")

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 util.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/util.c b/util.c
index ab23463b..347f34f5 100644
--- a/util.c
+++ b/util.c
@@ -126,9 +126,10 @@ int sock_l4_sa(const struct ctx *c, enum epoll_type type,
 			char str[SOCKADDR_STRLEN];
 
 			ret = -errno;
-			warn("Can't bind %s socket for %s to %s, closing",
-			     EPOLL_TYPE_STR(proto),
-			     sockaddr_ntop(sa, str, sizeof(str)), ifname);
+			warn("SO_BINDTODEVICE %s failed for %s on %s: %s",
+			     ifname, EPOLL_TYPE_STR(type),
+			     sockaddr_ntop(sa, str, sizeof(str)),
+			     strerror_(-ret));
 			close(fd);
 			return ret;
 		}
-- 
2.51.1

[PATCH 2/3] util: Extend sock_probe_mem() to sock_probe_features()

[David Gibson]

sock_probe_mem() currently checks whether we're able to allocate large
socket buffers.  Extend it to also check whether the SO_BINDTODEVICE
socket option is available.  Rename to sock_probe_features() to reflect the
new functionality.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 passt.c |  2 +-
 passt.h |  1 +
 util.c  | 19 +++++++++++++++++--
 util.h  |  2 +-
 4 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/passt.c b/passt.c
index 4964427d..0b84ac6c 100644
--- a/passt.c
+++ b/passt.c
@@ -381,7 +381,7 @@ int main(int argc, char **argv)
 	if (setrlimit(RLIMIT_NOFILE, &limit))
 		die_perror("Failed to set current limit for open files");
 
-	sock_probe_mem(&c);
+	sock_probe_features(&c);
 
 	conf(&c, argc, argv);
 	trace_init(c.trace);
diff --git a/passt.h b/passt.h
index 15801b44..e7c1710a 100644
--- a/passt.h
+++ b/passt.h
@@ -281,6 +281,7 @@ struct ctx {
 
 	int low_wmem;
 	int low_rmem;
+	int no_bindtodevice;
 
 	struct vu_dev *vdev;
 
diff --git a/util.c b/util.c
index 347f34f5..bad38129 100644
--- a/util.c
+++ b/util.c
@@ -233,12 +233,13 @@ int sock_unix(char *sock_path)
 }
 
 /**
- * sock_probe_mem() - Check if setting high SO_SNDBUF and SO_RCVBUF is allowed
+ * sock_probe_features() - Probe for socket features we might use
  * @c:		Execution context
  */
-void sock_probe_mem(struct ctx *c)
+void sock_probe_features(struct ctx *c)
 {
 	int v = INT_MAX / 2, s;
+	const char lo[] = "lo";
 	socklen_t sl;
 
 	s = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, IPPROTO_TCP);
@@ -247,6 +248,7 @@ void sock_probe_mem(struct ctx *c)
 		return;
 	}
 
+	/* Check if setting high SO_SNDBUF and SO_RCVBUF is allowed */
 	sl = sizeof(v);
 	if (setsockopt(s, SOL_SOCKET, SO_SNDBUF, &v, sizeof(v))	||
 	    getsockopt(s, SOL_SOCKET, SO_SNDBUF, &v, &sl) ||
@@ -259,6 +261,19 @@ void sock_probe_mem(struct ctx *c)
 	    (size_t)v < RCVBUF_BIG)
 		c->low_rmem = 1;
 
+	/* Check if SO_BINDTODEVICE is available
+	 *
+	 * Supported since kernel version 5.7, commit c427bfec18f2 ("net: core:
+	 * enable SO_BINDTODEVICE for non-root users").  Some distro kernels may
+	 * have backports, of course.  Record whether we can use it so that we
+	 * can give more useful diagnostics.
+	 */
+	if (setsockopt(s, SOL_SOCKET, SO_BINDTODEVICE, lo, sizeof(lo)-1)) {
+		if (errno != EPERM)
+			warn_perror("Unexpected error probing SO_BINDTODEVICE");
+		c->no_bindtodevice = 1;
+	}
+
 	close(s);
 }
 
diff --git a/util.h b/util.h
index a0b2ada6..200b6408 100644
--- a/util.h
+++ b/util.h
@@ -211,7 +211,7 @@ int sock_l4_sa(const struct ctx *c, enum epoll_type type,
 	       const void *sa, socklen_t sl,
 	       const char *ifname, bool v6only);
 int sock_unix(char *sock_path);
-void sock_probe_mem(struct ctx *c);
+void sock_probe_features(struct ctx *c);
 long timespec_diff_ms(const struct timespec *a, const struct timespec *b);
 int64_t timespec_diff_us(const struct timespec *a, const struct timespec *b);
 void bitmap_set(uint8_t *map, unsigned bit);
-- 
2.51.1

[PATCH 3/3] conf: More useful errors for kernels without SO_BINDTODEVICE

[David Gibson]

Before 5.7, the kernel didn't allow SO_BINDTODEVICE to be called
unprivileged.  That means for earlier kernels, we can't implement binding
listening sockets to a specific interface (e.g. -t %eth0/80).

Currently we'll generate an error on this at the point we actually attempt
the SO_BINDTODEVICE setsockopt(), at which point the connection to the
command line option might not be entirely clear.

Use the fact we now probe for SO_BINDTODEVICE support to make a clearer
error message at the time we parse the forwarding option.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 conf.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/conf.c b/conf.c
index 66b9e634..43bcaf31 100644
--- a/conf.c
+++ b/conf.c
@@ -156,6 +156,12 @@ static void conf_ports_range_except(const struct ctx *c, char optname,
 		    optname, optarg);
 	}
 
+	if (ifname && c->no_bindtodevice) {
+		die(
+"Device binding for '-%c %s' unsupported (requires kernel 5.7+)",
+		    optname, optarg);
+	}
+
 	for (i = first; i <= last; i++) {
 		if (bitmap_isset(exclude, i))
 			continue;
-- 
2.51.1