2020-07-04 08:53:27

by Ignat Korchagin

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

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/[email protected]/

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


2020-07-04 08:55:07

by Ignat Korchagin

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

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 <[email protected]>
---
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

2020-07-04 08:56:59

by Ignat Korchagin

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

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 <[email protected]>
---
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

2020-07-14 08:41:00

by Anton Ivanov

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


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 <[email protected]>
> ---
> 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/

2020-07-14 10:23:51

by Ignat Korchagin

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

On Tue, Jul 14, 2020 at 9:40 AM Anton Ivanov
<[email protected]> 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 <[email protected]>
> > ---
> > 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/

2020-07-14 10:45:18

by Anton Ivanov

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



On 14/07/2020 11:23, Ignat Korchagin wrote:
> On Tue, Jul 14, 2020 at 9:40 AM Anton Ivanov
> <[email protected]> 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 <[email protected]>
>>> ---
>>> 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/

2020-07-14 16:41:08

by Johannes Berg

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

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

2020-07-15 09:12:16

by Brendan Higgins

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

On Sat, Jul 4, 2020 at 1:52 AM Ignat Korchagin <[email protected]> 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/[email protected]/
>
> 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 <[email protected]>

2020-07-15 09:16:22

by Brendan Higgins

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

On Tue, Jul 14, 2020 at 9:02 AM Johannes Berg <[email protected]> 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.

2020-07-15 09:16:48

by Brendan Higgins

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

On Sat, Jul 4, 2020 at 1:52 AM Ignat Korchagin <[email protected]> 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 <[email protected]>

One minor issue below. Other than that:

Reviewed-by: Brendan Higgins <[email protected]>

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

2020-07-15 09:37:14

by Ignat Korchagin

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

On Wed, Jul 15, 2020 at 9:44 AM Brendan Higgins
<[email protected]> wrote:
>
> On Sat, Jul 4, 2020 at 1:52 AM Ignat Korchagin <[email protected]> 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 <[email protected]>
>
> One minor issue below. Other than that:
>
> Reviewed-by: Brendan Higgins <[email protected]>
>
> > ---
> > 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
> >