public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
* [PATCH 0/4] Fix build against musl C library
@ 2023-03-08  7:35 Stefano Brivio
  2023-03-08  7:35 ` [PATCH 1/4] netlink: Use 8 KiB * netlink message header size as response buffer Stefano Brivio
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Stefano Brivio @ 2023-03-08  7:35 UTC (permalink / raw)
  To: passt-dev; +Cc: Chris Kuhn, lemmi

This series, partially coming from Chris Kuhn, enables passt to
be built against musl.

Tested on Alpine (x86_64) 3.17.2, musl 1.2.3-r4.

See also:
  https://bugs.passt.top/show_bug.cgi?id=4
  https://github.com/void-linux/void-packages/pull/42517

Chris Kuhn (2):
  conf, passt: Rename stderr to force_stderr
  treewide: Fix header includes to build with musl

Stefano Brivio (2):
  netlink: Use 8 KiB * netlink message header size as response buffer
  util: Carry own definition of __bswap_constant{16,32}

 conf.c       |  8 +++++---
 isolation.c  |  1 +
 netlink.c    | 16 ++++++++++------
 passt.c      |  4 +++-
 passt.h      |  4 ++--
 tap.c        |  1 +
 tcp.c        |  1 +
 tcp_splice.c |  1 +
 udp.c        |  1 +
 util.c       |  1 +
 util.h       | 11 +++++++++++
 11 files changed, 37 insertions(+), 12 deletions(-)

-- 
2.39.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/4] netlink: Use 8 KiB * netlink message header size as response buffer
  2023-03-08  7:35 [PATCH 0/4] Fix build against musl C library Stefano Brivio
@ 2023-03-08  7:35 ` Stefano Brivio
  2023-03-08 22:06   ` David Gibson
  2023-03-08  7:35 ` [PATCH 2/4] conf, passt: Rename stderr to force_stderr Stefano Brivio
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Stefano Brivio @ 2023-03-08  7:35 UTC (permalink / raw)
  To: passt-dev; +Cc: Chris Kuhn, lemmi

...instead of BUFSIZ. On musl, BUFSIZ is 1024, so we'll typically
truncate the response to the request we send in nl_link(). It's
usually 8192 or more with glibc.

There doesn't seem to be any macro defining the rtnetlink maximum
message size, and iproute2 just hardcodes 1024 * 1024 for the receive
buffer, but the example in netlink(7) makes somewhat sense, looking
at the kernel implementation.

It's not very clean, but we're very unlikely to hit that limit,
and if we do, we'll find out painlessly, because NLA_OK() will tell
us right away.

Reported-by: Chris Kuhn <kuhnchris+passt@kuhnchris.eu>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 netlink.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/netlink.c b/netlink.c
index 8f785ca..0e0be4f 100644
--- a/netlink.c
+++ b/netlink.c
@@ -34,6 +34,8 @@
 #include "log.h"
 #include "netlink.h"
 
+#define NLBUFSIZ	(8192 * sizeof(struct nlmsghdr)) /* See netlink(7) */
+
 /* Socket in init, in target namespace, sequence (just needs to be monotonic) */
 static int nl_sock	= -1;
 static int nl_sock_ns	= -1;
@@ -105,7 +107,7 @@ fail:
 static int nl_req(int ns, char *buf, const void *req, ssize_t len)
 {
 	int s = ns ? nl_sock_ns : nl_sock, done = 0;
-	char flush[BUFSIZ];
+	char flush[NLBUFSIZ];
 	ssize_t n;
 
 	while (!done && (n = recv(s, flush, sizeof(flush), MSG_DONTWAIT)) > 0) {
@@ -121,7 +123,8 @@ static int nl_req(int ns, char *buf, const void *req, ssize_t len)
 		}
 	}
 
-	if ((send(s, req, len, 0) < len) || (len = recv(s, buf, BUFSIZ, 0)) < 0)
+	if ((send(s, req, len, 0) < len) ||
+	    (len = recv(s, buf, NLBUFSIZ, 0)) < 0)
 		return -errno;
 
 	return len;
@@ -149,7 +152,7 @@ unsigned int nl_get_ext_if(sa_family_t af)
 	};
 	struct nlmsghdr *nh;
 	struct rtattr *rta;
-	char buf[BUFSIZ];
+	char buf[NLBUFSIZ];
 	ssize_t n;
 	size_t na;
 
@@ -227,7 +230,7 @@ void nl_route(int ns, unsigned int ifi, sa_family_t af, void *gw)
 	struct nlmsghdr *nh;
 	struct rtattr *rta;
 	struct rtmsg *rtm;
-	char buf[BUFSIZ];
+	char buf[NLBUFSIZ];
 	ssize_t n;
 	size_t na;
 
@@ -336,7 +339,7 @@ void nl_addr(int ns, unsigned int ifi, sa_family_t af,
 	struct ifaddrmsg *ifa;
 	struct nlmsghdr *nh;
 	struct rtattr *rta;
-	char buf[BUFSIZ];
+	char buf[NLBUFSIZ];
 	ssize_t n;
 	size_t na;
 
@@ -446,7 +449,7 @@ void nl_link(int ns, unsigned int ifi, void *mac, int up, int mtu)
 	struct ifinfomsg *ifm;
 	struct nlmsghdr *nh;
 	struct rtattr *rta;
-	char buf[BUFSIZ];
+	char buf[NLBUFSIZ];
 	ssize_t n;
 	size_t na;
 
-- 
@@ -34,6 +34,8 @@
 #include "log.h"
 #include "netlink.h"
 
+#define NLBUFSIZ	(8192 * sizeof(struct nlmsghdr)) /* See netlink(7) */
+
 /* Socket in init, in target namespace, sequence (just needs to be monotonic) */
 static int nl_sock	= -1;
 static int nl_sock_ns	= -1;
@@ -105,7 +107,7 @@ fail:
 static int nl_req(int ns, char *buf, const void *req, ssize_t len)
 {
 	int s = ns ? nl_sock_ns : nl_sock, done = 0;
-	char flush[BUFSIZ];
+	char flush[NLBUFSIZ];
 	ssize_t n;
 
 	while (!done && (n = recv(s, flush, sizeof(flush), MSG_DONTWAIT)) > 0) {
@@ -121,7 +123,8 @@ static int nl_req(int ns, char *buf, const void *req, ssize_t len)
 		}
 	}
 
-	if ((send(s, req, len, 0) < len) || (len = recv(s, buf, BUFSIZ, 0)) < 0)
+	if ((send(s, req, len, 0) < len) ||
+	    (len = recv(s, buf, NLBUFSIZ, 0)) < 0)
 		return -errno;
 
 	return len;
@@ -149,7 +152,7 @@ unsigned int nl_get_ext_if(sa_family_t af)
 	};
 	struct nlmsghdr *nh;
 	struct rtattr *rta;
-	char buf[BUFSIZ];
+	char buf[NLBUFSIZ];
 	ssize_t n;
 	size_t na;
 
@@ -227,7 +230,7 @@ void nl_route(int ns, unsigned int ifi, sa_family_t af, void *gw)
 	struct nlmsghdr *nh;
 	struct rtattr *rta;
 	struct rtmsg *rtm;
-	char buf[BUFSIZ];
+	char buf[NLBUFSIZ];
 	ssize_t n;
 	size_t na;
 
@@ -336,7 +339,7 @@ void nl_addr(int ns, unsigned int ifi, sa_family_t af,
 	struct ifaddrmsg *ifa;
 	struct nlmsghdr *nh;
 	struct rtattr *rta;
-	char buf[BUFSIZ];
+	char buf[NLBUFSIZ];
 	ssize_t n;
 	size_t na;
 
@@ -446,7 +449,7 @@ void nl_link(int ns, unsigned int ifi, void *mac, int up, int mtu)
 	struct ifinfomsg *ifm;
 	struct nlmsghdr *nh;
 	struct rtattr *rta;
-	char buf[BUFSIZ];
+	char buf[NLBUFSIZ];
 	ssize_t n;
 	size_t na;
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/4] conf, passt: Rename stderr to force_stderr
  2023-03-08  7:35 [PATCH 0/4] Fix build against musl C library Stefano Brivio
  2023-03-08  7:35 ` [PATCH 1/4] netlink: Use 8 KiB * netlink message header size as response buffer Stefano Brivio
@ 2023-03-08  7:35 ` Stefano Brivio
  2023-03-08 22:10   ` David Gibson
  2023-03-08  7:35 ` [PATCH 3/4] treewide: Fix header includes to build with musl Stefano Brivio
  2023-03-08  7:35 ` [PATCH 4/4] util: Carry own definition of __bswap_constant{16,32} Stefano Brivio
  3 siblings, 1 reply; 9+ messages in thread
From: Stefano Brivio @ 2023-03-08  7:35 UTC (permalink / raw)
  To: passt-dev; +Cc: Chris Kuhn, lemmi

From: Chris Kuhn <kuhnchris+github@kuhnchris.eu>

While building against musl, gcc informs us that 'stderr' is a
protected keyword. This probably comes from a #define stderr (stderr)
in musl's stdio.h, to avoid a clash with extern FILE *const stderr,
but I didn't really track it down. Just rename it to force_stderr, it
makes more sense.

[sbrivio: Added commit message]
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 conf.c  | 6 +++---
 passt.c | 2 +-
 passt.h | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/conf.c b/conf.c
index 15506ec..07b0b7b 100644
--- a/conf.c
+++ b/conf.c
@@ -1356,13 +1356,13 @@ void conf(struct ctx *c, int argc, char **argv)
 			if (logfile)
 				die("Can't log to both file and stderr");
 
-			if (c->stderr)
+			if (c->force_stderr)
 				die("Multiple --stderr options given");
 
-			c->stderr = 1;
+			c->force_stderr = 1;
 			break;
 		case 'l':
-			if (c->stderr)
+			if (c->force_stderr)
 				die("Can't log to both stderr and file");
 
 			if (logfile)
diff --git a/passt.c b/passt.c
index 5b8146e..f67213a 100644
--- a/passt.c
+++ b/passt.c
@@ -241,7 +241,7 @@ int main(int argc, char **argv)
 	conf(&c, argc, argv);
 	trace_init(c.trace);
 
-	if (c.stderr || isatty(fileno(stdout)))
+	if (c.force_stderr || isatty(fileno(stdout)))
 		__openlog(log_name, LOG_PERROR, LOG_DAEMON);
 
 	quit_fd = pasta_netns_quit_init(&c);
diff --git a/passt.h b/passt.h
index b73f4ff..006d1c1 100644
--- a/passt.h
+++ b/passt.h
@@ -158,7 +158,7 @@ struct ip6_ctx {
  * @trace:		Enable tracing (extra debug) mode
  * @quiet:		Don't print informational messages
  * @foreground:		Run in foreground, don't log to stderr by default
- * @stderr:		Force logging to stderr
+ * @force_stderr:	Force logging to stderr
  * @nofile:		Maximum number of open files (ulimit -n)
  * @sock_path:		Path for UNIX domain socket
  * @pcap:		Path for packet capture file
@@ -207,7 +207,7 @@ struct ctx {
 	int trace;
 	int quiet;
 	int foreground;
-	int stderr;
+	int force_stderr;
 	int nofile;
 	char sock_path[UNIX_PATH_MAX];
 	char pcap[PATH_MAX];
-- 
@@ -158,7 +158,7 @@ struct ip6_ctx {
  * @trace:		Enable tracing (extra debug) mode
  * @quiet:		Don't print informational messages
  * @foreground:		Run in foreground, don't log to stderr by default
- * @stderr:		Force logging to stderr
+ * @force_stderr:	Force logging to stderr
  * @nofile:		Maximum number of open files (ulimit -n)
  * @sock_path:		Path for UNIX domain socket
  * @pcap:		Path for packet capture file
@@ -207,7 +207,7 @@ struct ctx {
 	int trace;
 	int quiet;
 	int foreground;
-	int stderr;
+	int force_stderr;
 	int nofile;
 	char sock_path[UNIX_PATH_MAX];
 	char pcap[PATH_MAX];
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/4] treewide: Fix header includes to build with musl
  2023-03-08  7:35 [PATCH 0/4] Fix build against musl C library Stefano Brivio
  2023-03-08  7:35 ` [PATCH 1/4] netlink: Use 8 KiB * netlink message header size as response buffer Stefano Brivio
  2023-03-08  7:35 ` [PATCH 2/4] conf, passt: Rename stderr to force_stderr Stefano Brivio
@ 2023-03-08  7:35 ` Stefano Brivio
  2023-03-08 22:11   ` David Gibson
  2023-03-08  7:35 ` [PATCH 4/4] util: Carry own definition of __bswap_constant{16,32} Stefano Brivio
  3 siblings, 1 reply; 9+ messages in thread
From: Stefano Brivio @ 2023-03-08  7:35 UTC (permalink / raw)
  To: passt-dev; +Cc: Chris Kuhn, lemmi

From: Chris Kuhn <kuhnchris+github@kuhnchris.eu>

Roughly inspired from a patch by Chris Kuhn: fix up includes so that
we can build against musl: glibc is more lenient as headers generally
include a larger amount of other headers.

Compared to the original patch, I only included what was needed
directly in C files, instead of adding blanket includes in local
header files. It's a bit more involved, but more consistent with the
current (not ideal) situation.

Reported-by: Chris Kuhn <kuhnchris+github@kuhnchris.eu>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 conf.c       | 2 ++
 isolation.c  | 1 +
 netlink.c    | 1 +
 passt.c      | 2 ++
 tap.c        | 1 +
 tcp.c        | 1 +
 tcp_splice.c | 1 +
 udp.c        | 1 +
 util.c       | 1 +
 9 files changed, 11 insertions(+)

diff --git a/conf.c b/conf.c
index 07b0b7b..582c391 100644
--- a/conf.c
+++ b/conf.c
@@ -23,8 +23,10 @@
 #include <limits.h>
 #include <grp.h>
 #include <pwd.h>
+#include <signal.h>
 #include <stdlib.h>
 #include <stdint.h>
+#include <stdio.h>
 #include <stdbool.h>
 #include <unistd.h>
 #include <syslog.h>
diff --git a/isolation.c b/isolation.c
index 6bae4d4..20dc879 100644
--- a/isolation.c
+++ b/isolation.c
@@ -65,6 +65,7 @@
 #include <stdbool.h>
 #include <stddef.h>
 #include <stdlib.h>
+#include <stdio.h>
 #include <string.h>
 #include <time.h>
 #include <unistd.h>
diff --git a/netlink.c b/netlink.c
index 0e0be4f..c8d39a1 100644
--- a/netlink.c
+++ b/netlink.c
@@ -18,6 +18,7 @@
 #include <errno.h>
 #include <sys/types.h>
 #include <limits.h>
+#include <signal.h>
 #include <stdlib.h>
 #include <stdbool.h>
 #include <stdint.h>
diff --git a/passt.c b/passt.c
index f67213a..dfec9d4 100644
--- a/passt.c
+++ b/passt.c
@@ -27,6 +27,8 @@
 #include <stdlib.h>
 #include <unistd.h>
 #include <netdb.h>
+#include <signal.h>
+#include <stdio.h>
 #include <string.h>
 #include <errno.h>
 #include <time.h>
diff --git a/tap.c b/tap.c
index 88eed88..15fb52e 100644
--- a/tap.c
+++ b/tap.c
@@ -14,6 +14,7 @@
  */
 
 #include <sched.h>
+#include <signal.h>
 #include <stdio.h>
 #include <errno.h>
 #include <limits.h>
diff --git a/tcp.c b/tcp.c
index 8e8d653..96ca5c7 100644
--- a/tcp.c
+++ b/tcp.c
@@ -267,6 +267,7 @@
 #include <sched.h>
 #include <fcntl.h>
 #include <stdio.h>
+#include <signal.h>
 #include <stdlib.h>
 #include <errno.h>
 #include <limits.h>
diff --git a/tcp_splice.c b/tcp_splice.c
index 67af46b..6559762 100644
--- a/tcp_splice.c
+++ b/tcp_splice.c
@@ -32,6 +32,7 @@
  */
 
 #include <sched.h>
+#include <signal.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <limits.h>
diff --git a/udp.c b/udp.c
index 99cfc9f..1077cde 100644
--- a/udp.c
+++ b/udp.c
@@ -91,6 +91,7 @@
  */
 
 #include <sched.h>
+#include <signal.h>
 #include <stdio.h>
 #include <errno.h>
 #include <limits.h>
diff --git a/util.c b/util.c
index 799173f..484889b 100644
--- a/util.c
+++ b/util.c
@@ -13,6 +13,7 @@
  */
 
 #include <sched.h>
+#include <stdio.h>
 #include <stdlib.h>
 #include <unistd.h>
 #include <arpa/inet.h>
-- 
@@ -13,6 +13,7 @@
  */
 
 #include <sched.h>
+#include <stdio.h>
 #include <stdlib.h>
 #include <unistd.h>
 #include <arpa/inet.h>
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/4] util: Carry own definition of __bswap_constant{16,32}
  2023-03-08  7:35 [PATCH 0/4] Fix build against musl C library Stefano Brivio
                   ` (2 preceding siblings ...)
  2023-03-08  7:35 ` [PATCH 3/4] treewide: Fix header includes to build with musl Stefano Brivio
@ 2023-03-08  7:35 ` Stefano Brivio
  2023-03-08 22:11   ` David Gibson
  3 siblings, 1 reply; 9+ messages in thread
From: Stefano Brivio @ 2023-03-08  7:35 UTC (permalink / raw)
  To: passt-dev; +Cc: Chris Kuhn, lemmi

musl doesn't define those, use our own definition there. This is a
trivial implementation, similar to what's shipped by e.g. uClibc,
glibc, libiio.

Reported-by: Chris Kuhn <kuhnchris+github@kuhnchris.eu>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 util.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/util.h b/util.h
index 570094c..8367f51 100644
--- a/util.h
+++ b/util.h
@@ -88,6 +88,17 @@
 #define MAC_ZERO		((uint8_t [ETH_ALEN]){ 0 })
 #define MAC_IS_ZERO(addr)	(!memcmp((addr), MAC_ZERO, ETH_ALEN))
 
+#ifndef __bswap_constant_16
+#define __bswap_constant_16(x)						\
+	((uint16_t) ((((x) >> 8) & 0xff) | (((x) & 0xff) << 8)))
+#endif
+
+#ifndef __bswap_constant_32
+#define __bswap_constant_32(x)						\
+	((((x) & 0xff000000) >> 24) | (((x) & 0x00ff0000) >>  8) |	\
+	 (((x) & 0x0000ff00) <<  8) | (((x) & 0x000000ff) << 24))
+#endif
+
 #if __BYTE_ORDER == __BIG_ENDIAN
 #define	htons_constant(x)	(x)
 #define	htonl_constant(x)	(x)
-- 
@@ -88,6 +88,17 @@
 #define MAC_ZERO		((uint8_t [ETH_ALEN]){ 0 })
 #define MAC_IS_ZERO(addr)	(!memcmp((addr), MAC_ZERO, ETH_ALEN))
 
+#ifndef __bswap_constant_16
+#define __bswap_constant_16(x)						\
+	((uint16_t) ((((x) >> 8) & 0xff) | (((x) & 0xff) << 8)))
+#endif
+
+#ifndef __bswap_constant_32
+#define __bswap_constant_32(x)						\
+	((((x) & 0xff000000) >> 24) | (((x) & 0x00ff0000) >>  8) |	\
+	 (((x) & 0x0000ff00) <<  8) | (((x) & 0x000000ff) << 24))
+#endif
+
 #if __BYTE_ORDER == __BIG_ENDIAN
 #define	htons_constant(x)	(x)
 #define	htonl_constant(x)	(x)
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/4] netlink: Use 8 KiB * netlink message header size as response buffer
  2023-03-08  7:35 ` [PATCH 1/4] netlink: Use 8 KiB * netlink message header size as response buffer Stefano Brivio
@ 2023-03-08 22:06   ` David Gibson
  0 siblings, 0 replies; 9+ messages in thread
From: David Gibson @ 2023-03-08 22:06 UTC (permalink / raw)
  To: Stefano Brivio; +Cc: passt-dev, Chris Kuhn, lemmi

[-- Attachment #1: Type: text/plain, Size: 3223 bytes --]

On Wed, Mar 08, 2023 at 08:35:13AM +0100, Stefano Brivio wrote:
> ...instead of BUFSIZ. On musl, BUFSIZ is 1024, so we'll typically
> truncate the response to the request we send in nl_link(). It's
> usually 8192 or more with glibc.
> 
> There doesn't seem to be any macro defining the rtnetlink maximum
> message size, and iproute2 just hardcodes 1024 * 1024 for the receive
> buffer, but the example in netlink(7) makes somewhat sense, looking
> at the kernel implementation.
> 
> It's not very clean, but we're very unlikely to hit that limit,
> and if we do, we'll find out painlessly, because NLA_OK() will tell
> us right away.
> 
> Reported-by: Chris Kuhn <kuhnchris+passt@kuhnchris.eu>
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

I think controlling buffer sizes ourselves explicitly makes more
sense, even if it weren't for the musl issues.

> ---
>  netlink.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/netlink.c b/netlink.c
> index 8f785ca..0e0be4f 100644
> --- a/netlink.c
> +++ b/netlink.c
> @@ -34,6 +34,8 @@
>  #include "log.h"
>  #include "netlink.h"
>  
> +#define NLBUFSIZ	(8192 * sizeof(struct nlmsghdr)) /* See netlink(7) */
> +
>  /* Socket in init, in target namespace, sequence (just needs to be monotonic) */
>  static int nl_sock	= -1;
>  static int nl_sock_ns	= -1;
> @@ -105,7 +107,7 @@ fail:
>  static int nl_req(int ns, char *buf, const void *req, ssize_t len)
>  {
>  	int s = ns ? nl_sock_ns : nl_sock, done = 0;
> -	char flush[BUFSIZ];
> +	char flush[NLBUFSIZ];
>  	ssize_t n;
>  
>  	while (!done && (n = recv(s, flush, sizeof(flush), MSG_DONTWAIT)) > 0) {
> @@ -121,7 +123,8 @@ static int nl_req(int ns, char *buf, const void *req, ssize_t len)
>  		}
>  	}
>  
> -	if ((send(s, req, len, 0) < len) || (len = recv(s, buf, BUFSIZ, 0)) < 0)
> +	if ((send(s, req, len, 0) < len) ||
> +	    (len = recv(s, buf, NLBUFSIZ, 0)) < 0)
>  		return -errno;
>  
>  	return len;
> @@ -149,7 +152,7 @@ unsigned int nl_get_ext_if(sa_family_t af)
>  	};
>  	struct nlmsghdr *nh;
>  	struct rtattr *rta;
> -	char buf[BUFSIZ];
> +	char buf[NLBUFSIZ];
>  	ssize_t n;
>  	size_t na;
>  
> @@ -227,7 +230,7 @@ void nl_route(int ns, unsigned int ifi, sa_family_t af, void *gw)
>  	struct nlmsghdr *nh;
>  	struct rtattr *rta;
>  	struct rtmsg *rtm;
> -	char buf[BUFSIZ];
> +	char buf[NLBUFSIZ];
>  	ssize_t n;
>  	size_t na;
>  
> @@ -336,7 +339,7 @@ void nl_addr(int ns, unsigned int ifi, sa_family_t af,
>  	struct ifaddrmsg *ifa;
>  	struct nlmsghdr *nh;
>  	struct rtattr *rta;
> -	char buf[BUFSIZ];
> +	char buf[NLBUFSIZ];
>  	ssize_t n;
>  	size_t na;
>  
> @@ -446,7 +449,7 @@ void nl_link(int ns, unsigned int ifi, void *mac, int up, int mtu)
>  	struct ifinfomsg *ifm;
>  	struct nlmsghdr *nh;
>  	struct rtattr *rta;
> -	char buf[BUFSIZ];
> +	char buf[NLBUFSIZ];
>  	ssize_t n;
>  	size_t na;
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/4] conf, passt: Rename stderr to force_stderr
  2023-03-08  7:35 ` [PATCH 2/4] conf, passt: Rename stderr to force_stderr Stefano Brivio
@ 2023-03-08 22:10   ` David Gibson
  0 siblings, 0 replies; 9+ messages in thread
From: David Gibson @ 2023-03-08 22:10 UTC (permalink / raw)
  To: Stefano Brivio; +Cc: passt-dev, Chris Kuhn, lemmi

[-- Attachment #1: Type: text/plain, Size: 2994 bytes --]

On Wed, Mar 08, 2023 at 08:35:14AM +0100, Stefano Brivio wrote:
> From: Chris Kuhn <kuhnchris+github@kuhnchris.eu>
> 
> While building against musl, gcc informs us that 'stderr' is a
> protected keyword. This probably comes from a #define stderr (stderr)
> in musl's stdio.h, to avoid a clash with extern FILE *const stderr,
> but I didn't really track it down. Just rename it to force_stderr, it
> makes more sense.
> 
> [sbrivio: Added commit message]
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

Shadowing a libc global is not a great idea.  Since it's a field, not
a variable we're not *exactly* doing that, but it's close enough that
I think this is better regardless of musl constraints.

(I once had a particularly wonderful time tracking down a bizarre bug
because 'index' ended up resolving to index(3) instead of the local
variable I expected it to).

> ---
>  conf.c  | 6 +++---
>  passt.c | 2 +-
>  passt.h | 4 ++--
>  3 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index 15506ec..07b0b7b 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -1356,13 +1356,13 @@ void conf(struct ctx *c, int argc, char **argv)
>  			if (logfile)
>  				die("Can't log to both file and stderr");
>  
> -			if (c->stderr)
> +			if (c->force_stderr)
>  				die("Multiple --stderr options given");
>  
> -			c->stderr = 1;
> +			c->force_stderr = 1;
>  			break;
>  		case 'l':
> -			if (c->stderr)
> +			if (c->force_stderr)
>  				die("Can't log to both stderr and file");
>  
>  			if (logfile)
> diff --git a/passt.c b/passt.c
> index 5b8146e..f67213a 100644
> --- a/passt.c
> +++ b/passt.c
> @@ -241,7 +241,7 @@ int main(int argc, char **argv)
>  	conf(&c, argc, argv);
>  	trace_init(c.trace);
>  
> -	if (c.stderr || isatty(fileno(stdout)))
> +	if (c.force_stderr || isatty(fileno(stdout)))
>  		__openlog(log_name, LOG_PERROR, LOG_DAEMON);
>  
>  	quit_fd = pasta_netns_quit_init(&c);
> diff --git a/passt.h b/passt.h
> index b73f4ff..006d1c1 100644
> --- a/passt.h
> +++ b/passt.h
> @@ -158,7 +158,7 @@ struct ip6_ctx {
>   * @trace:		Enable tracing (extra debug) mode
>   * @quiet:		Don't print informational messages
>   * @foreground:		Run in foreground, don't log to stderr by default
> - * @stderr:		Force logging to stderr
> + * @force_stderr:	Force logging to stderr
>   * @nofile:		Maximum number of open files (ulimit -n)
>   * @sock_path:		Path for UNIX domain socket
>   * @pcap:		Path for packet capture file
> @@ -207,7 +207,7 @@ struct ctx {
>  	int trace;
>  	int quiet;
>  	int foreground;
> -	int stderr;
> +	int force_stderr;
>  	int nofile;
>  	char sock_path[UNIX_PATH_MAX];
>  	char pcap[PATH_MAX];

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/4] treewide: Fix header includes to build with musl
  2023-03-08  7:35 ` [PATCH 3/4] treewide: Fix header includes to build with musl Stefano Brivio
@ 2023-03-08 22:11   ` David Gibson
  0 siblings, 0 replies; 9+ messages in thread
From: David Gibson @ 2023-03-08 22:11 UTC (permalink / raw)
  To: Stefano Brivio; +Cc: passt-dev, Chris Kuhn, lemmi

[-- Attachment #1: Type: text/plain, Size: 3811 bytes --]

On Wed, Mar 08, 2023 at 08:35:15AM +0100, Stefano Brivio wrote:
> From: Chris Kuhn <kuhnchris+github@kuhnchris.eu>
> 
> Roughly inspired from a patch by Chris Kuhn: fix up includes so that
> we can build against musl: glibc is more lenient as headers generally
> include a larger amount of other headers.
> 
> Compared to the original patch, I only included what was needed
> directly in C files, instead of adding blanket includes in local
> header files. It's a bit more involved, but more consistent with the
> current (not ideal) situation.

Best I can tell, there's no ideal way to manage C includes :/.

> Reported-by: Chris Kuhn <kuhnchris+github@kuhnchris.eu>
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  conf.c       | 2 ++
>  isolation.c  | 1 +
>  netlink.c    | 1 +
>  passt.c      | 2 ++
>  tap.c        | 1 +
>  tcp.c        | 1 +
>  tcp_splice.c | 1 +
>  udp.c        | 1 +
>  util.c       | 1 +
>  9 files changed, 11 insertions(+)
> 
> diff --git a/conf.c b/conf.c
> index 07b0b7b..582c391 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -23,8 +23,10 @@
>  #include <limits.h>
>  #include <grp.h>
>  #include <pwd.h>
> +#include <signal.h>
>  #include <stdlib.h>
>  #include <stdint.h>
> +#include <stdio.h>
>  #include <stdbool.h>
>  #include <unistd.h>
>  #include <syslog.h>
> diff --git a/isolation.c b/isolation.c
> index 6bae4d4..20dc879 100644
> --- a/isolation.c
> +++ b/isolation.c
> @@ -65,6 +65,7 @@
>  #include <stdbool.h>
>  #include <stddef.h>
>  #include <stdlib.h>
> +#include <stdio.h>
>  #include <string.h>
>  #include <time.h>
>  #include <unistd.h>
> diff --git a/netlink.c b/netlink.c
> index 0e0be4f..c8d39a1 100644
> --- a/netlink.c
> +++ b/netlink.c
> @@ -18,6 +18,7 @@
>  #include <errno.h>
>  #include <sys/types.h>
>  #include <limits.h>
> +#include <signal.h>
>  #include <stdlib.h>
>  #include <stdbool.h>
>  #include <stdint.h>
> diff --git a/passt.c b/passt.c
> index f67213a..dfec9d4 100644
> --- a/passt.c
> +++ b/passt.c
> @@ -27,6 +27,8 @@
>  #include <stdlib.h>
>  #include <unistd.h>
>  #include <netdb.h>
> +#include <signal.h>
> +#include <stdio.h>
>  #include <string.h>
>  #include <errno.h>
>  #include <time.h>
> diff --git a/tap.c b/tap.c
> index 88eed88..15fb52e 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -14,6 +14,7 @@
>   */
>  
>  #include <sched.h>
> +#include <signal.h>
>  #include <stdio.h>
>  #include <errno.h>
>  #include <limits.h>
> diff --git a/tcp.c b/tcp.c
> index 8e8d653..96ca5c7 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -267,6 +267,7 @@
>  #include <sched.h>
>  #include <fcntl.h>
>  #include <stdio.h>
> +#include <signal.h>
>  #include <stdlib.h>
>  #include <errno.h>
>  #include <limits.h>
> diff --git a/tcp_splice.c b/tcp_splice.c
> index 67af46b..6559762 100644
> --- a/tcp_splice.c
> +++ b/tcp_splice.c
> @@ -32,6 +32,7 @@
>   */
>  
>  #include <sched.h>
> +#include <signal.h>
>  #include <errno.h>
>  #include <fcntl.h>
>  #include <limits.h>
> diff --git a/udp.c b/udp.c
> index 99cfc9f..1077cde 100644
> --- a/udp.c
> +++ b/udp.c
> @@ -91,6 +91,7 @@
>   */
>  
>  #include <sched.h>
> +#include <signal.h>
>  #include <stdio.h>
>  #include <errno.h>
>  #include <limits.h>
> diff --git a/util.c b/util.c
> index 799173f..484889b 100644
> --- a/util.c
> +++ b/util.c
> @@ -13,6 +13,7 @@
>   */
>  
>  #include <sched.h>
> +#include <stdio.h>
>  #include <stdlib.h>
>  #include <unistd.h>
>  #include <arpa/inet.h>

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 4/4] util: Carry own definition of __bswap_constant{16,32}
  2023-03-08  7:35 ` [PATCH 4/4] util: Carry own definition of __bswap_constant{16,32} Stefano Brivio
@ 2023-03-08 22:11   ` David Gibson
  0 siblings, 0 replies; 9+ messages in thread
From: David Gibson @ 2023-03-08 22:11 UTC (permalink / raw)
  To: Stefano Brivio; +Cc: passt-dev, Chris Kuhn, lemmi

[-- Attachment #1: Type: text/plain, Size: 1390 bytes --]

On Wed, Mar 08, 2023 at 08:35:16AM +0100, Stefano Brivio wrote:
> musl doesn't define those, use our own definition there. This is a
> trivial implementation, similar to what's shipped by e.g. uClibc,
> glibc, libiio.
> 
> Reported-by: Chris Kuhn <kuhnchris+github@kuhnchris.eu>
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  util.h | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/util.h b/util.h
> index 570094c..8367f51 100644
> --- a/util.h
> +++ b/util.h
> @@ -88,6 +88,17 @@
>  #define MAC_ZERO		((uint8_t [ETH_ALEN]){ 0 })
>  #define MAC_IS_ZERO(addr)	(!memcmp((addr), MAC_ZERO, ETH_ALEN))
>  
> +#ifndef __bswap_constant_16
> +#define __bswap_constant_16(x)						\
> +	((uint16_t) ((((x) >> 8) & 0xff) | (((x) & 0xff) << 8)))
> +#endif
> +
> +#ifndef __bswap_constant_32
> +#define __bswap_constant_32(x)						\
> +	((((x) & 0xff000000) >> 24) | (((x) & 0x00ff0000) >>  8) |	\
> +	 (((x) & 0x0000ff00) <<  8) | (((x) & 0x000000ff) << 24))
> +#endif
> +
>  #if __BYTE_ORDER == __BIG_ENDIAN
>  #define	htons_constant(x)	(x)
>  #define	htonl_constant(x)	(x)

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-03-08 22:45 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-08  7:35 [PATCH 0/4] Fix build against musl C library Stefano Brivio
2023-03-08  7:35 ` [PATCH 1/4] netlink: Use 8 KiB * netlink message header size as response buffer Stefano Brivio
2023-03-08 22:06   ` David Gibson
2023-03-08  7:35 ` [PATCH 2/4] conf, passt: Rename stderr to force_stderr Stefano Brivio
2023-03-08 22:10   ` David Gibson
2023-03-08  7:35 ` [PATCH 3/4] treewide: Fix header includes to build with musl Stefano Brivio
2023-03-08 22:11   ` David Gibson
2023-03-08  7:35 ` [PATCH 4/4] util: Carry own definition of __bswap_constant{16,32} Stefano Brivio
2023-03-08 22:11   ` David Gibson

Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).