[PATCH v2 0/3] um: allow static linking for non-glibc libc implementations

[PATCH v2 0/3] um: allow static linking for non-glibc libc implementations

[Ignat Korchagin]

This is a continuation of [1]. Since I was able to produce a working UML binary
with UML_NET_VECTOR linked with musl with the changes included in the patches
here. I was compiling on Arch Linux, so hopefully all the latest versions of
the compiler, libraries and binutils.

I also tested allyesconfig with both musl and glibc. The compilation succeeds
with both, however both binaries (glibc one being dynamically linked) segfault
on start. This is probably of some incompatible config option/module being
included and not related to musl/glibc.

[1]: https://patchwork.ozlabs.org/project/linux-um/patch/20200624212319.403689-1-ignat@cloudflare.com/

Ignat Korchagin (3):
  um/kconfig: introduce CC_CAN_LINK_STATIC_NO_RUNTIME_DEPS
  um: some fixes to build UML with musl
  um: allow static linking for non-glibc implementations

 arch/um/Kconfig               |  2 +-
 arch/um/drivers/Kconfig       |  3 ---
 arch/um/drivers/daemon_user.c |  1 +
 arch/um/drivers/pcap_user.c   | 12 ++++++------
 arch/um/drivers/slip_user.c   |  2 +-
 arch/um/drivers/vector_user.c |  4 +---
 arch/um/os-Linux/util.c       |  2 +-
 arch/x86/um/user-offsets.c    |  2 +-
 init/Kconfig                  |  6 ++++++
 scripts/cc-can-link.sh        |  5 +++--
 10 files changed, 21 insertions(+), 18 deletions(-)

-- 
2.20.1

[PATCH v2 1/3] um/kconfig: introduce CC_CAN_LINK_STATIC_NO_RUNTIME_DEPS

[Ignat Korchagin]

For statically linked UML build it is important to take into account the
standard C-library implementation. Some implementations, notably glibc have
caveats: even when linked statically, the final program might require some
runtime dependencies, if certain functions are used within the code.

Consider the following program:
int main(void)
{
	getpwent();
	return 0;
}

Compiling this program and linking statically with glibc produces the following
warning from the linker:
/usr/sbin/ld: /tmp/ccuthw1o.o: in function `main':
test.c:(.text+0x5): warning: Using 'getpwent' in statically linked
applications requires at runtime the shared libraries from the glibc version
used for linking

We will use the flag to detect such C-library implementation build time and
possibly disable static linking for UML to avoid producing a binary with
unexpected behaviour and dependencies.

Signed-off-by: Ignat Korchagin <ignat@cloudflare.com>
---
 init/Kconfig           | 6 ++++++
 scripts/cc-can-link.sh | 5 +++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index a46aa8f3174d..717a3ada765e 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -57,6 +57,12 @@ config CC_CAN_LINK_STATIC
 	default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) -static $(m64-flag)) if 64BIT
 	default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) -static $(m32-flag))
 
+config CC_CAN_LINK_STATIC_NO_RUNTIME_DEPS
+	bool
+	depends on UML && CC_CAN_LINK_STATIC
+	default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) -static -Xlinker --fatal-warnings $(m64-flag)) if 64BIT
+	default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) -static -Xlinker --fatal-warnings $(m32-flag))
+
 config CC_HAS_ASM_GOTO
 	def_bool $(success,$(srctree)/scripts/gcc-goto.sh $(CC))
 
diff --git a/scripts/cc-can-link.sh b/scripts/cc-can-link.sh
index 6efcead31989..e5011a46103e 100755
--- a/scripts/cc-can-link.sh
+++ b/scripts/cc-can-link.sh
@@ -2,10 +2,11 @@
 # SPDX-License-Identifier: GPL-2.0
 
 cat << "END" | $@ -x c - -o /dev/null >/dev/null 2>&1
-#include <stdio.h>
+#include <sys/types.h>
+#include <pwd.h>
 int main(void)
 {
-	printf("");
+	getpwent();
 	return 0;
 }
 END
-- 
2.20.1

Re: [PATCH v2 1/3] um/kconfig: introduce CC_CAN_LINK_STATIC_NO_RUNTIME_DEPS

[Brendan Higgins]

On Sat, Jul 4, 2020 at 1:52 AM Ignat Korchagin <ignat@cloudflare.com> wrote:
>
> For statically linked UML build it is important to take into account the
> standard C-library implementation. Some implementations, notably glibc have
> caveats: even when linked statically, the final program might require some
> runtime dependencies, if certain functions are used within the code.
>
> Consider the following program:
> int main(void)
> {
>         getpwent();
>         return 0;
> }
>
> Compiling this program and linking statically with glibc produces the following
> warning from the linker:
> /usr/sbin/ld: /tmp/ccuthw1o.o: in function `main':
> test.c:(.text+0x5): warning: Using 'getpwent' in statically linked
> applications requires at runtime the shared libraries from the glibc version
> used for linking
>
> We will use the flag to detect such C-library implementation build time and
> possibly disable static linking for UML to avoid producing a binary with
> unexpected behaviour and dependencies.
>
> Signed-off-by: Ignat Korchagin <ignat@cloudflare.com>

Nice.

Reviewed-by: Brendan Higgins <brendanhiggins@google.com>

[PATCH v2 2/3] um: some fixes to build UML with musl

[Ignat Korchagin]

musl toolchain and headers are a bit more strict. These fixes enable building
UML with musl as well as seem not to break on glibc.

Signed-off-by: Ignat Korchagin <ignat@cloudflare.com>
---
 arch/um/drivers/daemon_user.c |  1 +
 arch/um/drivers/pcap_user.c   | 12 ++++++------
 arch/um/drivers/slip_user.c   |  2 +-
 arch/um/drivers/vector_user.c |  4 +---
 arch/um/os-Linux/util.c       |  2 +-
 arch/x86/um/user-offsets.c    |  2 +-
 6 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/arch/um/drivers/daemon_user.c b/arch/um/drivers/daemon_user.c
index 3695821d06a2..785baedc3555 100644
--- a/arch/um/drivers/daemon_user.c
+++ b/arch/um/drivers/daemon_user.c
@@ -7,6 +7,7 @@
  */
 
 #include <stdint.h>
+#include <string.h>
 #include <unistd.h>
 #include <errno.h>
 #include <sys/types.h>
diff --git a/arch/um/drivers/pcap_user.c b/arch/um/drivers/pcap_user.c
index bbd20638788a..52ddda3e3b10 100644
--- a/arch/um/drivers/pcap_user.c
+++ b/arch/um/drivers/pcap_user.c
@@ -32,7 +32,7 @@ static int pcap_user_init(void *data, void *dev)
 	return 0;
 }
 
-static int pcap_open(void *data)
+static int pcap_user_open(void *data)
 {
 	struct pcap_data *pri = data;
 	__u32 netmask;
@@ -44,14 +44,14 @@ static int pcap_open(void *data)
 	if (pri->filter != NULL) {
 		err = dev_netmask(pri->dev, &netmask);
 		if (err < 0) {
-			printk(UM_KERN_ERR "pcap_open : dev_netmask failed\n");
+			printk(UM_KERN_ERR "pcap_user_open : dev_netmask failed\n");
 			return -EIO;
 		}
 
 		pri->compiled = uml_kmalloc(sizeof(struct bpf_program),
 					UM_GFP_KERNEL);
 		if (pri->compiled == NULL) {
-			printk(UM_KERN_ERR "pcap_open : kmalloc failed\n");
+			printk(UM_KERN_ERR "pcap_user_open : kmalloc failed\n");
 			return -ENOMEM;
 		}
 
@@ -59,14 +59,14 @@ static int pcap_open(void *data)
 				   (struct bpf_program *) pri->compiled,
 				   pri->filter, pri->optimize, netmask);
 		if (err < 0) {
-			printk(UM_KERN_ERR "pcap_open : pcap_compile failed - "
+			printk(UM_KERN_ERR "pcap_user_open : pcap_compile failed - "
 			       "'%s'\n", pcap_geterr(pri->pcap));
 			goto out;
 		}
 
 		err = pcap_setfilter(pri->pcap, pri->compiled);
 		if (err < 0) {
-			printk(UM_KERN_ERR "pcap_open : pcap_setfilter "
+			printk(UM_KERN_ERR "pcap_user_open : pcap_setfilter "
 			       "failed - '%s'\n", pcap_geterr(pri->pcap));
 			goto out;
 		}
@@ -127,7 +127,7 @@ int pcap_user_read(int fd, void *buffer, int len, struct pcap_data *pri)
 
 const struct net_user_info pcap_user_info = {
 	.init		= pcap_user_init,
-	.open		= pcap_open,
+	.open		= pcap_user_open,
 	.close	 	= NULL,
 	.remove	 	= pcap_remove,
 	.add_address	= NULL,
diff --git a/arch/um/drivers/slip_user.c b/arch/um/drivers/slip_user.c
index 8016d32b6809..482a19c5105c 100644
--- a/arch/um/drivers/slip_user.c
+++ b/arch/um/drivers/slip_user.c
@@ -9,7 +9,7 @@
 #include <errno.h>
 #include <fcntl.h>
 #include <string.h>
-#include <sys/termios.h>
+#include <termios.h>
 #include <sys/wait.h>
 #include <net_user.h>
 #include <os.h>
diff --git a/arch/um/drivers/vector_user.c b/arch/um/drivers/vector_user.c
index c4a0f26b2824..45d4164ad355 100644
--- a/arch/um/drivers/vector_user.c
+++ b/arch/um/drivers/vector_user.c
@@ -18,9 +18,7 @@
 #include <fcntl.h>
 #include <sys/socket.h>
 #include <sys/un.h>
-#include <net/ethernet.h>
 #include <netinet/ip.h>
-#include <netinet/ether.h>
 #include <linux/if_ether.h>
 #include <linux/if_packet.h>
 #include <sys/wait.h>
@@ -332,7 +330,7 @@ static struct vector_fds *user_init_unix_fds(struct arglist *ifspec, int id)
 	}
 	switch (id) {
 	case ID_BESS:
-		if (connect(fd, remote_addr, sizeof(struct sockaddr_un)) < 0) {
+		if (connect(fd, (const struct sockaddr *) remote_addr, sizeof(struct sockaddr_un)) < 0) {
 			printk(UM_KERN_ERR "bess open:cannot connect to %s %i", remote_addr->sun_path, -errno);
 			goto unix_cleanup;
 		}
diff --git a/arch/um/os-Linux/util.c b/arch/um/os-Linux/util.c
index ecf2f390fad2..07327425d06e 100644
--- a/arch/um/os-Linux/util.c
+++ b/arch/um/os-Linux/util.c
@@ -10,7 +10,7 @@
 #include <signal.h>
 #include <string.h>
 #include <termios.h>
-#include <wait.h>
+#include <sys/wait.h>
 #include <sys/mman.h>
 #include <sys/utsname.h>
 #include <init.h>
diff --git a/arch/x86/um/user-offsets.c b/arch/x86/um/user-offsets.c
index c51dd8363d25..bae61554abcc 100644
--- a/arch/x86/um/user-offsets.c
+++ b/arch/x86/um/user-offsets.c
@@ -2,7 +2,7 @@
 #include <stdio.h>
 #include <stddef.h>
 #include <signal.h>
-#include <sys/poll.h>
+#include <poll.h>
 #include <sys/mman.h>
 #include <sys/user.h>
 #define __FRAME_OFFSETS
-- 
2.20.1

Re: [PATCH v2 2/3] um: some fixes to build UML with musl

[Anton Ivanov]

On 04/07/2020 09:52, Ignat Korchagin wrote:
> musl toolchain and headers are a bit more strict. These fixes enable building
> UML with musl as well as seem not to break on glibc.
> 
> Signed-off-by: Ignat Korchagin <ignat@cloudflare.com>
> ---
>   arch/um/drivers/daemon_user.c |  1 +
>   arch/um/drivers/pcap_user.c   | 12 ++++++------
>   arch/um/drivers/slip_user.c   |  2 +-
>   arch/um/drivers/vector_user.c |  4 +---
>   arch/um/os-Linux/util.c       |  2 +-
>   arch/x86/um/user-offsets.c    |  2 +-
>   6 files changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/um/drivers/daemon_user.c b/arch/um/drivers/daemon_user.c
> index 3695821d06a2..785baedc3555 100644
> --- a/arch/um/drivers/daemon_user.c
> +++ b/arch/um/drivers/daemon_user.c
> @@ -7,6 +7,7 @@
>    */
>   
>   #include <stdint.h>
> +#include <string.h>
>   #include <unistd.h>
>   #include <errno.h>
>   #include <sys/types.h>
> diff --git a/arch/um/drivers/pcap_user.c b/arch/um/drivers/pcap_user.c
> index bbd20638788a..52ddda3e3b10 100644
> --- a/arch/um/drivers/pcap_user.c
> +++ b/arch/um/drivers/pcap_user.c
> @@ -32,7 +32,7 @@ static int pcap_user_init(void *data, void *dev)
>   	return 0;
>   }
>   
> -static int pcap_open(void *data)
> +static int pcap_user_open(void *data)

This change in the function name was introduced on purpose to avoid name clash in some version of libpcap which export pcap_open


>   {
>   	struct pcap_data *pri = data;
>   	__u32 netmask;
> @@ -44,14 +44,14 @@ static int pcap_open(void *data)
>   	if (pri->filter != NULL) {
>   		err = dev_netmask(pri->dev, &netmask);
>   		if (err < 0) {
> -			printk(UM_KERN_ERR "pcap_open : dev_netmask failed\n");
> +			printk(UM_KERN_ERR "pcap_user_open : dev_netmask failed\n");
>   			return -EIO;
>   		}
>   
>   		pri->compiled = uml_kmalloc(sizeof(struct bpf_program),
>   					UM_GFP_KERNEL);
>   		if (pri->compiled == NULL) {
> -			printk(UM_KERN_ERR "pcap_open : kmalloc failed\n");
> +			printk(UM_KERN_ERR "pcap_user_open : kmalloc failed\n");
>   			return -ENOMEM;
>   		}
>   
> @@ -59,14 +59,14 @@ static int pcap_open(void *data)
>   				   (struct bpf_program *) pri->compiled,
>   				   pri->filter, pri->optimize, netmask);
>   		if (err < 0) {
> -			printk(UM_KERN_ERR "pcap_open : pcap_compile failed - "
> +			printk(UM_KERN_ERR "pcap_user_open : pcap_compile failed - "
>   			       "'%s'\n", pcap_geterr(pri->pcap));
>   			goto out;
>   		}
>   
>   		err = pcap_setfilter(pri->pcap, pri->compiled);
>   		if (err < 0) {
> -			printk(UM_KERN_ERR "pcap_open : pcap_setfilter "
> +			printk(UM_KERN_ERR "pcap_user_open : pcap_setfilter "
>   			       "failed - '%s'\n", pcap_geterr(pri->pcap));
>   			goto out;
>   		}
> @@ -127,7 +127,7 @@ int pcap_user_read(int fd, void *buffer, int len, struct pcap_data *pri)
>   
>   const struct net_user_info pcap_user_info = {
>   	.init		= pcap_user_init,
> -	.open		= pcap_open,
> +	.open		= pcap_user_open,
>   	.close	 	= NULL,
>   	.remove	 	= pcap_remove,
>   	.add_address	= NULL,
> diff --git a/arch/um/drivers/slip_user.c b/arch/um/drivers/slip_user.c
> index 8016d32b6809..482a19c5105c 100644
> --- a/arch/um/drivers/slip_user.c
> +++ b/arch/um/drivers/slip_user.c
> @@ -9,7 +9,7 @@
>   #include <errno.h>
>   #include <fcntl.h>
>   #include <string.h>
> -#include <sys/termios.h>
> +#include <termios.h>
>   #include <sys/wait.h>
>   #include <net_user.h>
>   #include <os.h>
> diff --git a/arch/um/drivers/vector_user.c b/arch/um/drivers/vector_user.c
> index c4a0f26b2824..45d4164ad355 100644
> --- a/arch/um/drivers/vector_user.c
> +++ b/arch/um/drivers/vector_user.c
> @@ -18,9 +18,7 @@
>   #include <fcntl.h>
>   #include <sys/socket.h>
>   #include <sys/un.h>
> -#include <net/ethernet.h>
>   #include <netinet/ip.h>
> -#include <netinet/ether.h>
>   #include <linux/if_ether.h>
>   #include <linux/if_packet.h>
>   #include <sys/wait.h>
> @@ -332,7 +330,7 @@ static struct vector_fds *user_init_unix_fds(struct arglist *ifspec, int id)
>   	}
>   	switch (id) {
>   	case ID_BESS:
> -		if (connect(fd, remote_addr, sizeof(struct sockaddr_un)) < 0) {
> +		if (connect(fd, (const struct sockaddr *) remote_addr, sizeof(struct sockaddr_un)) < 0) {
>   			printk(UM_KERN_ERR "bess open:cannot connect to %s %i", remote_addr->sun_path, -errno);
>   			goto unix_cleanup;
>   		}
> diff --git a/arch/um/os-Linux/util.c b/arch/um/os-Linux/util.c
> index ecf2f390fad2..07327425d06e 100644
> --- a/arch/um/os-Linux/util.c
> +++ b/arch/um/os-Linux/util.c
> @@ -10,7 +10,7 @@
>   #include <signal.h>
>   #include <string.h>
>   #include <termios.h>
> -#include <wait.h>
> +#include <sys/wait.h>
>   #include <sys/mman.h>
>   #include <sys/utsname.h>
>   #include <init.h>
> diff --git a/arch/x86/um/user-offsets.c b/arch/x86/um/user-offsets.c
> index c51dd8363d25..bae61554abcc 100644
> --- a/arch/x86/um/user-offsets.c
> +++ b/arch/x86/um/user-offsets.c
> @@ -2,7 +2,7 @@
>   #include <stdio.h>
>   #include <stddef.h>
>   #include <signal.h>
> -#include <sys/poll.h>
> +#include <poll.h>
>   #include <sys/mman.h>
>   #include <sys/user.h>
>   #define __FRAME_OFFSETS
> 

Apologies for the delay in answering, I was buried under OVS for the last month or so.

With the exception of this patch the rest of the series looks OK. Can you please resumbit and if Johannes and Richard are OK with it I will +1 it.

Best regards,

-- 
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/

Re: [PATCH v2 2/3] um: some fixes to build UML with musl

[Ignat Korchagin]

On Tue, Jul 14, 2020 at 9:40 AM Anton Ivanov
<anton.ivanov@cambridgegreys.com> wrote:
>
>
> On 04/07/2020 09:52, Ignat Korchagin wrote:
> > musl toolchain and headers are a bit more strict. These fixes enable building
> > UML with musl as well as seem not to break on glibc.
> >
> > Signed-off-by: Ignat Korchagin <ignat@cloudflare.com>
> > ---
> >   arch/um/drivers/daemon_user.c |  1 +
> >   arch/um/drivers/pcap_user.c   | 12 ++++++------
> >   arch/um/drivers/slip_user.c   |  2 +-
> >   arch/um/drivers/vector_user.c |  4 +---
> >   arch/um/os-Linux/util.c       |  2 +-
> >   arch/x86/um/user-offsets.c    |  2 +-
> >   6 files changed, 11 insertions(+), 12 deletions(-)
> >
> > diff --git a/arch/um/drivers/daemon_user.c b/arch/um/drivers/daemon_user.c
> > index 3695821d06a2..785baedc3555 100644
> > --- a/arch/um/drivers/daemon_user.c
> > +++ b/arch/um/drivers/daemon_user.c
> > @@ -7,6 +7,7 @@
> >    */
> >
> >   #include <stdint.h>
> > +#include <string.h>
> >   #include <unistd.h>
> >   #include <errno.h>
> >   #include <sys/types.h>
> > diff --git a/arch/um/drivers/pcap_user.c b/arch/um/drivers/pcap_user.c
> > index bbd20638788a..52ddda3e3b10 100644
> > --- a/arch/um/drivers/pcap_user.c
> > +++ b/arch/um/drivers/pcap_user.c
> > @@ -32,7 +32,7 @@ static int pcap_user_init(void *data, void *dev)
> >       return 0;
> >   }
> >
> > -static int pcap_open(void *data)
> > +static int pcap_user_open(void *data)
>
> This change in the function name was introduced on purpose to avoid name clash in some version of libpcap which export pcap_open

Yes

>
>
> >   {
> >       struct pcap_data *pri = data;
> >       __u32 netmask;
> > @@ -44,14 +44,14 @@ static int pcap_open(void *data)
> >       if (pri->filter != NULL) {
> >               err = dev_netmask(pri->dev, &netmask);
> >               if (err < 0) {
> > -                     printk(UM_KERN_ERR "pcap_open : dev_netmask failed\n");
> > +                     printk(UM_KERN_ERR "pcap_user_open : dev_netmask failed\n");
> >                       return -EIO;
> >               }
> >
> >               pri->compiled = uml_kmalloc(sizeof(struct bpf_program),
> >                                       UM_GFP_KERNEL);
> >               if (pri->compiled == NULL) {
> > -                     printk(UM_KERN_ERR "pcap_open : kmalloc failed\n");
> > +                     printk(UM_KERN_ERR "pcap_user_open : kmalloc failed\n");
> >                       return -ENOMEM;
> >               }
> >
> > @@ -59,14 +59,14 @@ static int pcap_open(void *data)
> >                                  (struct bpf_program *) pri->compiled,
> >                                  pri->filter, pri->optimize, netmask);
> >               if (err < 0) {
> > -                     printk(UM_KERN_ERR "pcap_open : pcap_compile failed - "
> > +                     printk(UM_KERN_ERR "pcap_user_open : pcap_compile failed - "
> >                              "'%s'\n", pcap_geterr(pri->pcap));
> >                       goto out;
> >               }
> >
> >               err = pcap_setfilter(pri->pcap, pri->compiled);
> >               if (err < 0) {
> > -                     printk(UM_KERN_ERR "pcap_open : pcap_setfilter "
> > +                     printk(UM_KERN_ERR "pcap_user_open : pcap_setfilter "
> >                              "failed - '%s'\n", pcap_geterr(pri->pcap));
> >                       goto out;
> >               }
> > @@ -127,7 +127,7 @@ int pcap_user_read(int fd, void *buffer, int len, struct pcap_data *pri)
> >
> >   const struct net_user_info pcap_user_info = {
> >       .init           = pcap_user_init,
> > -     .open           = pcap_open,
> > +     .open           = pcap_user_open,
> >       .close          = NULL,
> >       .remove         = pcap_remove,
> >       .add_address    = NULL,
> > diff --git a/arch/um/drivers/slip_user.c b/arch/um/drivers/slip_user.c
> > index 8016d32b6809..482a19c5105c 100644
> > --- a/arch/um/drivers/slip_user.c
> > +++ b/arch/um/drivers/slip_user.c
> > @@ -9,7 +9,7 @@
> >   #include <errno.h>
> >   #include <fcntl.h>
> >   #include <string.h>
> > -#include <sys/termios.h>
> > +#include <termios.h>
> >   #include <sys/wait.h>
> >   #include <net_user.h>
> >   #include <os.h>
> > diff --git a/arch/um/drivers/vector_user.c b/arch/um/drivers/vector_user.c
> > index c4a0f26b2824..45d4164ad355 100644
> > --- a/arch/um/drivers/vector_user.c
> > +++ b/arch/um/drivers/vector_user.c
> > @@ -18,9 +18,7 @@
> >   #include <fcntl.h>
> >   #include <sys/socket.h>
> >   #include <sys/un.h>
> > -#include <net/ethernet.h>
> >   #include <netinet/ip.h>
> > -#include <netinet/ether.h>
> >   #include <linux/if_ether.h>
> >   #include <linux/if_packet.h>
> >   #include <sys/wait.h>
> > @@ -332,7 +330,7 @@ static struct vector_fds *user_init_unix_fds(struct arglist *ifspec, int id)
> >       }
> >       switch (id) {
> >       case ID_BESS:
> > -             if (connect(fd, remote_addr, sizeof(struct sockaddr_un)) < 0) {
> > +             if (connect(fd, (const struct sockaddr *) remote_addr, sizeof(struct sockaddr_un)) < 0) {
> >                       printk(UM_KERN_ERR "bess open:cannot connect to %s %i", remote_addr->sun_path, -errno);
> >                       goto unix_cleanup;
> >               }
> > diff --git a/arch/um/os-Linux/util.c b/arch/um/os-Linux/util.c
> > index ecf2f390fad2..07327425d06e 100644
> > --- a/arch/um/os-Linux/util.c
> > +++ b/arch/um/os-Linux/util.c
> > @@ -10,7 +10,7 @@
> >   #include <signal.h>
> >   #include <string.h>
> >   #include <termios.h>
> > -#include <wait.h>
> > +#include <sys/wait.h>
> >   #include <sys/mman.h>
> >   #include <sys/utsname.h>
> >   #include <init.h>
> > diff --git a/arch/x86/um/user-offsets.c b/arch/x86/um/user-offsets.c
> > index c51dd8363d25..bae61554abcc 100644
> > --- a/arch/x86/um/user-offsets.c
> > +++ b/arch/x86/um/user-offsets.c
> > @@ -2,7 +2,7 @@
> >   #include <stdio.h>
> >   #include <stddef.h>
> >   #include <signal.h>
> > -#include <sys/poll.h>
> > +#include <poll.h>
> >   #include <sys/mman.h>
> >   #include <sys/user.h>
> >   #define __FRAME_OFFSETS
> >
>
> Apologies for the delay in answering, I was buried under OVS for the last month or so.
>
> With the exception of this patch the rest of the series looks OK. Can you please resumbit and if Johannes and Richard are OK with it I will +1 it.

I didn't quite understand how I should improve this patch. Could you,
please, clarify?

> Best regards,
>
> --
> Anton R. Ivanov
> Cambridgegreys Limited. Registered in England. Company Number 10273661
> https://www.cambridgegreys.com/

Re: [PATCH v2 2/3] um: some fixes to build UML with musl

[Anton Ivanov]

On 14/07/2020 11:23, Ignat Korchagin wrote:
> On Tue, Jul 14, 2020 at 9:40 AM Anton Ivanov
> <anton.ivanov@cambridgegreys.com> wrote:
>>
>>
>> On 04/07/2020 09:52, Ignat Korchagin wrote:
>>> musl toolchain and headers are a bit more strict. These fixes enable building
>>> UML with musl as well as seem not to break on glibc.
>>>
>>> Signed-off-by: Ignat Korchagin <ignat@cloudflare.com>
>>> ---
>>>    arch/um/drivers/daemon_user.c |  1 +
>>>    arch/um/drivers/pcap_user.c   | 12 ++++++------
>>>    arch/um/drivers/slip_user.c   |  2 +-
>>>    arch/um/drivers/vector_user.c |  4 +---
>>>    arch/um/os-Linux/util.c       |  2 +-
>>>    arch/x86/um/user-offsets.c    |  2 +-
>>>    6 files changed, 11 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/arch/um/drivers/daemon_user.c b/arch/um/drivers/daemon_user.c
>>> index 3695821d06a2..785baedc3555 100644
>>> --- a/arch/um/drivers/daemon_user.c
>>> +++ b/arch/um/drivers/daemon_user.c
>>> @@ -7,6 +7,7 @@
>>>     */
>>>
>>>    #include <stdint.h>
>>> +#include <string.h>
>>>    #include <unistd.h>
>>>    #include <errno.h>
>>>    #include <sys/types.h>
>>> diff --git a/arch/um/drivers/pcap_user.c b/arch/um/drivers/pcap_user.c
>>> index bbd20638788a..52ddda3e3b10 100644
>>> --- a/arch/um/drivers/pcap_user.c
>>> +++ b/arch/um/drivers/pcap_user.c
>>> @@ -32,7 +32,7 @@ static int pcap_user_init(void *data, void *dev)
>>>        return 0;
>>>    }
>>>
>>> -static int pcap_open(void *data)
>>> +static int pcap_user_open(void *data)
>>
>> This change in the function name was introduced on purpose to avoid name clash in some version of libpcap which export pcap_open
> 
> Yes
> 
>>
>>
>>>    {
>>>        struct pcap_data *pri = data;
>>>        __u32 netmask;
>>> @@ -44,14 +44,14 @@ static int pcap_open(void *data)
>>>        if (pri->filter != NULL) {
>>>                err = dev_netmask(pri->dev, &netmask);
>>>                if (err < 0) {
>>> -                     printk(UM_KERN_ERR "pcap_open : dev_netmask failed\n");
>>> +                     printk(UM_KERN_ERR "pcap_user_open : dev_netmask failed\n");
>>>                        return -EIO;
>>>                }
>>>
>>>                pri->compiled = uml_kmalloc(sizeof(struct bpf_program),
>>>                                        UM_GFP_KERNEL);
>>>                if (pri->compiled == NULL) {
>>> -                     printk(UM_KERN_ERR "pcap_open : kmalloc failed\n");
>>> +                     printk(UM_KERN_ERR "pcap_user_open : kmalloc failed\n");
>>>                        return -ENOMEM;
>>>                }
>>>
>>> @@ -59,14 +59,14 @@ static int pcap_open(void *data)
>>>                                   (struct bpf_program *) pri->compiled,
>>>                                   pri->filter, pri->optimize, netmask);
>>>                if (err < 0) {
>>> -                     printk(UM_KERN_ERR "pcap_open : pcap_compile failed - "
>>> +                     printk(UM_KERN_ERR "pcap_user_open : pcap_compile failed - "
>>>                               "'%s'\n", pcap_geterr(pri->pcap));
>>>                        goto out;
>>>                }
>>>
>>>                err = pcap_setfilter(pri->pcap, pri->compiled);
>>>                if (err < 0) {
>>> -                     printk(UM_KERN_ERR "pcap_open : pcap_setfilter "
>>> +                     printk(UM_KERN_ERR "pcap_user_open : pcap_setfilter "
>>>                               "failed - '%s'\n", pcap_geterr(pri->pcap));
>>>                        goto out;
>>>                }
>>> @@ -127,7 +127,7 @@ int pcap_user_read(int fd, void *buffer, int len, struct pcap_data *pri)
>>>
>>>    const struct net_user_info pcap_user_info = {
>>>        .init           = pcap_user_init,
>>> -     .open           = pcap_open,
>>> +     .open           = pcap_user_open,
>>>        .close          = NULL,
>>>        .remove         = pcap_remove,
>>>        .add_address    = NULL,
>>> diff --git a/arch/um/drivers/slip_user.c b/arch/um/drivers/slip_user.c
>>> index 8016d32b6809..482a19c5105c 100644
>>> --- a/arch/um/drivers/slip_user.c
>>> +++ b/arch/um/drivers/slip_user.c
>>> @@ -9,7 +9,7 @@
>>>    #include <errno.h>
>>>    #include <fcntl.h>
>>>    #include <string.h>
>>> -#include <sys/termios.h>
>>> +#include <termios.h>
>>>    #include <sys/wait.h>
>>>    #include <net_user.h>
>>>    #include <os.h>
>>> diff --git a/arch/um/drivers/vector_user.c b/arch/um/drivers/vector_user.c
>>> index c4a0f26b2824..45d4164ad355 100644
>>> --- a/arch/um/drivers/vector_user.c
>>> +++ b/arch/um/drivers/vector_user.c
>>> @@ -18,9 +18,7 @@
>>>    #include <fcntl.h>
>>>    #include <sys/socket.h>
>>>    #include <sys/un.h>
>>> -#include <net/ethernet.h>
>>>    #include <netinet/ip.h>
>>> -#include <netinet/ether.h>
>>>    #include <linux/if_ether.h>
>>>    #include <linux/if_packet.h>
>>>    #include <sys/wait.h>
>>> @@ -332,7 +330,7 @@ static struct vector_fds *user_init_unix_fds(struct arglist *ifspec, int id)
>>>        }
>>>        switch (id) {
>>>        case ID_BESS:
>>> -             if (connect(fd, remote_addr, sizeof(struct sockaddr_un)) < 0) {
>>> +             if (connect(fd, (const struct sockaddr *) remote_addr, sizeof(struct sockaddr_un)) < 0) {
>>>                        printk(UM_KERN_ERR "bess open:cannot connect to %s %i", remote_addr->sun_path, -errno);
>>>                        goto unix_cleanup;
>>>                }
>>> diff --git a/arch/um/os-Linux/util.c b/arch/um/os-Linux/util.c
>>> index ecf2f390fad2..07327425d06e 100644
>>> --- a/arch/um/os-Linux/util.c
>>> +++ b/arch/um/os-Linux/util.c
>>> @@ -10,7 +10,7 @@
>>>    #include <signal.h>
>>>    #include <string.h>
>>>    #include <termios.h>
>>> -#include <wait.h>
>>> +#include <sys/wait.h>
>>>    #include <sys/mman.h>
>>>    #include <sys/utsname.h>
>>>    #include <init.h>
>>> diff --git a/arch/x86/um/user-offsets.c b/arch/x86/um/user-offsets.c
>>> index c51dd8363d25..bae61554abcc 100644
>>> --- a/arch/x86/um/user-offsets.c
>>> +++ b/arch/x86/um/user-offsets.c
>>> @@ -2,7 +2,7 @@
>>>    #include <stdio.h>
>>>    #include <stddef.h>
>>>    #include <signal.h>
>>> -#include <sys/poll.h>
>>> +#include <poll.h>
>>>    #include <sys/mman.h>
>>>    #include <sys/user.h>
>>>    #define __FRAME_OFFSETS
>>>
>>
>> Apologies for the delay in answering, I was buried under OVS for the last month or so.
>>
>> With the exception of this patch the rest of the series looks OK. Can you please resumbit and if Johannes and Richard are OK with it I will +1 it.
> 
> I didn't quite understand how I should improve this patch. Could you,
> please, clarify?

Sorry, not reading it correctly :)

My fault. You actually did exactly what has been in the queue for a while after Brendan noticed it: https://lkml.org/lkml/2019/12/5/868

Patch is OK with me, should not read patches before the 3rd double espresso next time.

I will +1 it, Richard, Johannes, what do you think?

Thanks,

> 
>> Best regards,
>>
>> --
>> Anton R. Ivanov
>> Cambridgegreys Limited. Registered in England. Company Number 10273661
>> https://www.cambridgegreys.com/
> 

-- 
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/

Re: [PATCH v2 2/3] um: some fixes to build UML with musl

[Johannes Berg]

On Tue, 2020-07-14 at 11:43 +0100, Anton Ivanov wrote:

> Patch is OK with me, should not read patches before the 3rd double espresso next time.
> 
> I will +1 it, Richard, Johannes, what do you think?

I got dropped off the list in "The Great Infradead Purge" but I see it
in patchwork ;)

Not sure we should modify cc-can-link.sh without at least CC'ing
Masahiro Yamada?

But yeah, looks fine to me. Not that I have any interest in static
linking.

johannes

Re: [PATCH v2 2/3] um: some fixes to build UML with musl

[Brendan Higgins]

On Tue, Jul 14, 2020 at 9:02 AM Johannes Berg <johannes@sipsolutions.net> wrote:
>
> On Tue, 2020-07-14 at 11:43 +0100, Anton Ivanov wrote:
>
> > Patch is OK with me, should not read patches before the 3rd double espresso next time.
> >
> > I will +1 it, Richard, Johannes, what do you think?
>
> I got dropped off the list in "The Great Infradead Purge" but I see it
> in patchwork ;)
>
> Not sure we should modify cc-can-link.sh without at least CC'ing
> Masahiro Yamada?
>
> But yeah, looks fine to me. Not that I have any interest in static
> linking.

Yeah, looks good to me as well, but I think the others here know the
drivers here better than I.

[PATCH v2 3/3] um: allow static linking for non-glibc implementations

[Ignat Korchagin]

It is possible to produce a statically linked UML binary with UML_NET_VECTOR,
UML_NET_VDE and UML_NET_PCAP options enabled using alternative libc
implementations, which do not rely on NSS, such as musl.

Allow static linking in this case.

Signed-off-by: Ignat Korchagin <ignat@cloudflare.com>
---
 arch/um/Kconfig         | 2 +-
 arch/um/drivers/Kconfig | 3 ---
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/um/Kconfig b/arch/um/Kconfig
index 9318dc6d1a0c..af7ed63f9c74 100644
--- a/arch/um/Kconfig
+++ b/arch/um/Kconfig
@@ -67,7 +67,7 @@ config FORBID_STATIC_LINK
 
 config STATIC_LINK
 	bool "Force a static link"
-	depends on !FORBID_STATIC_LINK
+	depends on CC_CAN_LINK_STATIC_NO_RUNTIME_DEPS || (!UML_NET_VECTOR && !UML_NET_VDE && !UML_NET_PCAP)
 	help
 	  This option gives you the ability to force a static link of UML.
 	  Normally, UML is linked as a shared binary.  This is inconvenient for
diff --git a/arch/um/drivers/Kconfig b/arch/um/drivers/Kconfig
index 9160ead56e33..72d417055782 100644
--- a/arch/um/drivers/Kconfig
+++ b/arch/um/drivers/Kconfig
@@ -234,7 +234,6 @@ config UML_NET_DAEMON
 config UML_NET_VECTOR
 	bool "Vector I/O high performance network devices"
 	depends on UML_NET
-	select FORBID_STATIC_LINK
 	help
 	This User-Mode Linux network driver uses multi-message send
 	and receive functions. The host running the UML guest must have
@@ -246,7 +245,6 @@ config UML_NET_VECTOR
 config UML_NET_VDE
 	bool "VDE transport (obsolete)"
 	depends on UML_NET
-	select FORBID_STATIC_LINK
 	help
 	This User-Mode Linux network transport allows one or more running
 	UMLs on a single host to communicate with each other and also
@@ -294,7 +292,6 @@ config UML_NET_MCAST
 config UML_NET_PCAP
 	bool "pcap transport (obsolete)"
 	depends on UML_NET
-	select FORBID_STATIC_LINK
 	help
 	The pcap transport makes a pcap packet stream on the host look
 	like an ethernet device inside UML.  This is useful for making
-- 
2.20.1

Re: [PATCH v2 3/3] um: allow static linking for non-glibc implementations

[Brendan Higgins]

On Sat, Jul 4, 2020 at 1:52 AM Ignat Korchagin <ignat@cloudflare.com> wrote:
>
> It is possible to produce a statically linked UML binary with UML_NET_VECTOR,
> UML_NET_VDE and UML_NET_PCAP options enabled using alternative libc
> implementations, which do not rely on NSS, such as musl.
>
> Allow static linking in this case.
>
> Signed-off-by: Ignat Korchagin <ignat@cloudflare.com>

One minor issue below. Other than that:

Reviewed-by: Brendan Higgins <brendanhiggins@google.com>

> ---
>  arch/um/Kconfig         | 2 +-
>  arch/um/drivers/Kconfig | 3 ---
>  2 files changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/arch/um/Kconfig b/arch/um/Kconfig
> index 9318dc6d1a0c..af7ed63f9c74 100644
> --- a/arch/um/Kconfig
> +++ b/arch/um/Kconfig
> @@ -67,7 +67,7 @@ config FORBID_STATIC_LINK

Doesn't look like FORBID_STATIC_LINK is used anymore, so you should
probably drop it as well.

With the preceding changes, in this patchset, you can revert my patch
like you did in the RFC - or not, your choice. I am not offended by
people reverting my commits. I just don't like it when people break
allyesconfig. :-)

>  config STATIC_LINK
>         bool "Force a static link"
> -       depends on !FORBID_STATIC_LINK
> +       depends on CC_CAN_LINK_STATIC_NO_RUNTIME_DEPS || (!UML_NET_VECTOR && !UML_NET_VDE && !UML_NET_PCAP)
>         help
>           This option gives you the ability to force a static link of UML.
>           Normally, UML is linked as a shared binary.  This is inconvenient for
> diff --git a/arch/um/drivers/Kconfig b/arch/um/drivers/Kconfig
> index 9160ead56e33..72d417055782 100644
> --- a/arch/um/drivers/Kconfig
> +++ b/arch/um/drivers/Kconfig
> @@ -234,7 +234,6 @@ config UML_NET_DAEMON
>  config UML_NET_VECTOR
>         bool "Vector I/O high performance network devices"
>         depends on UML_NET
> -       select FORBID_STATIC_LINK
>         help
>         This User-Mode Linux network driver uses multi-message send
>         and receive functions. The host running the UML guest must have
> @@ -246,7 +245,6 @@ config UML_NET_VECTOR
>  config UML_NET_VDE
>         bool "VDE transport (obsolete)"
>         depends on UML_NET
> -       select FORBID_STATIC_LINK
>         help
>         This User-Mode Linux network transport allows one or more running
>         UMLs on a single host to communicate with each other and also
> @@ -294,7 +292,6 @@ config UML_NET_MCAST
>  config UML_NET_PCAP
>         bool "pcap transport (obsolete)"
>         depends on UML_NET
> -       select FORBID_STATIC_LINK
>         help
>         The pcap transport makes a pcap packet stream on the host look
>         like an ethernet device inside UML.  This is useful for making
> --
> 2.20.1
>

Re: [PATCH v2 3/3] um: allow static linking for non-glibc implementations

[Ignat Korchagin]

On Wed, Jul 15, 2020 at 9:44 AM Brendan Higgins
<brendanhiggins@google.com> wrote:
>
> On Sat, Jul 4, 2020 at 1:52 AM Ignat Korchagin <ignat@cloudflare.com> wrote:
> >
> > It is possible to produce a statically linked UML binary with UML_NET_VECTOR,
> > UML_NET_VDE and UML_NET_PCAP options enabled using alternative libc
> > implementations, which do not rely on NSS, such as musl.
> >
> > Allow static linking in this case.
> >
> > Signed-off-by: Ignat Korchagin <ignat@cloudflare.com>
>
> One minor issue below. Other than that:
>
> Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
>
> > ---
> >  arch/um/Kconfig         | 2 +-
> >  arch/um/drivers/Kconfig | 3 ---
> >  2 files changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/arch/um/Kconfig b/arch/um/Kconfig
> > index 9318dc6d1a0c..af7ed63f9c74 100644
> > --- a/arch/um/Kconfig
> > +++ b/arch/um/Kconfig
> > @@ -67,7 +67,7 @@ config FORBID_STATIC_LINK
>
> Doesn't look like FORBID_STATIC_LINK is used anymore, so you should
> probably drop it as well.

Right, good catch! I will repost the series with this adjusted as well
cc Masahiro Yamada as mentioned for +1 on the changes to can-link.sh
script.
Seems I also need to rebase now to accommodate changes in
b816b3db15f68690ee72a4a414624f8e82942b25 ("kbuild: fix
CONFIG_CC_CAN_LINK(_STATIC) for cross-compilation with Clang")

> With the preceding changes, in this patchset, you can revert my patch
> like you did in the RFC - or not, your choice. I am not offended by
> people reverting my commits. I just don't like it when people break
> allyesconfig. :-)
>
> >  config STATIC_LINK
> >         bool "Force a static link"
> > -       depends on !FORBID_STATIC_LINK
> > +       depends on CC_CAN_LINK_STATIC_NO_RUNTIME_DEPS || (!UML_NET_VECTOR && !UML_NET_VDE && !UML_NET_PCAP)
> >         help
> >           This option gives you the ability to force a static link of UML.
> >           Normally, UML is linked as a shared binary.  This is inconvenient for
> > diff --git a/arch/um/drivers/Kconfig b/arch/um/drivers/Kconfig
> > index 9160ead56e33..72d417055782 100644
> > --- a/arch/um/drivers/Kconfig
> > +++ b/arch/um/drivers/Kconfig
> > @@ -234,7 +234,6 @@ config UML_NET_DAEMON
> >  config UML_NET_VECTOR
> >         bool "Vector I/O high performance network devices"
> >         depends on UML_NET
> > -       select FORBID_STATIC_LINK
> >         help
> >         This User-Mode Linux network driver uses multi-message send
> >         and receive functions. The host running the UML guest must have
> > @@ -246,7 +245,6 @@ config UML_NET_VECTOR
> >  config UML_NET_VDE
> >         bool "VDE transport (obsolete)"
> >         depends on UML_NET
> > -       select FORBID_STATIC_LINK
> >         help
> >         This User-Mode Linux network transport allows one or more running
> >         UMLs on a single host to communicate with each other and also
> > @@ -294,7 +292,6 @@ config UML_NET_MCAST
> >  config UML_NET_PCAP
> >         bool "pcap transport (obsolete)"
> >         depends on UML_NET
> > -       select FORBID_STATIC_LINK
> >         help
> >         The pcap transport makes a pcap packet stream on the host look
> >         like an ethernet device inside UML.  This is useful for making
> > --
> > 2.20.1
> >

Re: [PATCH v2 0/3] um: allow static linking for non-glibc libc implementations

[Brendan Higgins]

On Sat, Jul 4, 2020 at 1:52 AM Ignat Korchagin <ignat@cloudflare.com> wrote:
>
> This is a continuation of [1]. Since I was able to produce a working UML binary
> with UML_NET_VECTOR linked with musl with the changes included in the patches
> here. I was compiling on Arch Linux, so hopefully all the latest versions of
> the compiler, libraries and binutils.
>
> I also tested allyesconfig with both musl and glibc. The compilation succeeds
> with both, however both binaries (glibc one being dynamically linked) segfault
> on start. This is probably of some incompatible config option/module being
> included and not related to musl/glibc.
>
> [1]: https://patchwork.ozlabs.org/project/linux-um/patch/20200624212319.403689-1-ignat@cloudflare.com/
>
> Ignat Korchagin (3):
>   um/kconfig: introduce CC_CAN_LINK_STATIC_NO_RUNTIME_DEPS
>   um: some fixes to build UML with musl
>   um: allow static linking for non-glibc implementations
>
>  arch/um/Kconfig               |  2 +-
>  arch/um/drivers/Kconfig       |  3 ---
>  arch/um/drivers/daemon_user.c |  1 +
>  arch/um/drivers/pcap_user.c   | 12 ++++++------
>  arch/um/drivers/slip_user.c   |  2 +-
>  arch/um/drivers/vector_user.c |  4 +---
>  arch/um/os-Linux/util.c       |  2 +-
>  arch/x86/um/user-offsets.c    |  2 +-
>  init/Kconfig                  |  6 ++++++
>  scripts/cc-can-link.sh        |  5 +++--
>  10 files changed, 21 insertions(+), 18 deletions(-)

Sorry for taking so long to get to this. I saw this last week or
whenever and then forgot, saw the comments yesterday and remembered.

Looks pretty good overall. I will put my reviewed-by on each
individual patch. Nevertheless, I tested them all together, so being
lazy:

Tested-by: Brendan Higgins <brendanhiggins@google.com>