* [PATCH] musl: fix conflict on ethhdr definition
@ 2024-11-27 14:41 Laurent Vivier
2024-11-27 14:48 ` Stefano Brivio
0 siblings, 1 reply; 6+ messages in thread
From: Laurent Vivier @ 2024-11-27 14:41 UTC (permalink / raw)
To: passt-dev; +Cc: Laurent Vivier
there is a conflict between netinet/if_ether.h provided by musl and
linux/if_ether.h provided by the linux headers:
In file included from passt.h:185,
from tcp_vu.c:21:
/usr/include/netinet/if_ether.h:115:8: error: redefinition of 'struct ethhdr'
115 | struct ethhdr {
| ^~~~~~
In file included from /usr/include/linux/virtio_net.h:32,
from tcp_vu.c:17:
/usr/include/linux/if_ether.h:173:8: note: originally defined here
173 | struct ethhdr {
| ^~~~~~
The kernel headers provide a flag to disable the definition in this case,
__UAPI_DEF_ETHHDR (see /usr/include/linux/if_ether.h comment).
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
Makefile | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Makefile b/Makefile
index cb7448079de5..2aa56ada65fd 100644
--- a/Makefile
+++ b/Makefile
@@ -33,6 +33,9 @@ FLAGS += $(FORTIFY_FLAG) -O2 -pie -fPIE
FLAGS += -DPAGE_SIZE=$(shell getconf PAGE_SIZE)
FLAGS += -DVERSION=\"$(VERSION)\"
FLAGS += -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS)
+ifeq (musl, $(word 4,$(subst -, ,$(TARGET))))
+FLAGS += -D__UAPI_DEF_ETHHDR=0
+endif
PASST_SRCS = arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c flow.c fwd.c \
icmp.c igmp.c inany.c iov.c ip.c isolation.c lineread.c log.c mld.c \
--
@@ -33,6 +33,9 @@ FLAGS += $(FORTIFY_FLAG) -O2 -pie -fPIE
FLAGS += -DPAGE_SIZE=$(shell getconf PAGE_SIZE)
FLAGS += -DVERSION=\"$(VERSION)\"
FLAGS += -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS)
+ifeq (musl, $(word 4,$(subst -, ,$(TARGET))))
+FLAGS += -D__UAPI_DEF_ETHHDR=0
+endif
PASST_SRCS = arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c flow.c fwd.c \
icmp.c igmp.c inany.c iov.c ip.c isolation.c lineread.c log.c mld.c \
--
2.47.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] musl: fix conflict on ethhdr definition
2024-11-27 14:41 [PATCH] musl: fix conflict on ethhdr definition Laurent Vivier
@ 2024-11-27 14:48 ` Stefano Brivio
2024-11-27 14:55 ` Laurent Vivier
0 siblings, 1 reply; 6+ messages in thread
From: Stefano Brivio @ 2024-11-27 14:48 UTC (permalink / raw)
To: Laurent Vivier; +Cc: passt-dev
On Wed, 27 Nov 2024 15:41:30 +0100
Laurent Vivier <lvivier@redhat.com> wrote:
> there is a conflict between netinet/if_ether.h provided by musl and
> linux/if_ether.h provided by the linux headers:
>
> In file included from passt.h:185,
> from tcp_vu.c:21:
> /usr/include/netinet/if_ether.h:115:8: error: redefinition of 'struct ethhdr'
> 115 | struct ethhdr {
> | ^~~~~~
> In file included from /usr/include/linux/virtio_net.h:32,
> from tcp_vu.c:17:
> /usr/include/linux/if_ether.h:173:8: note: originally defined here
> 173 | struct ethhdr {
> | ^~~~~~
>
> The kernel headers provide a flag to disable the definition in this case,
> __UAPI_DEF_ETHHDR (see /usr/include/linux/if_ether.h comment).
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> Makefile | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index cb7448079de5..2aa56ada65fd 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -33,6 +33,9 @@ FLAGS += $(FORTIFY_FLAG) -O2 -pie -fPIE
> FLAGS += -DPAGE_SIZE=$(shell getconf PAGE_SIZE)
> FLAGS += -DVERSION=\"$(VERSION)\"
> FLAGS += -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS)
> +ifeq (musl, $(word 4,$(subst -, ,$(TARGET))))
> +FLAGS += -D__UAPI_DEF_ETHHDR=0
> +endif
Uh oh, I just solved this locally (I was about to push, but I stopped
in time) by simply including <linux/if_ether.h> instead of
<netinet/if_ether.h> from passt.h.
I'm not sure what's the best solution: mine is simpler, but I'm not
really fond of including Linux-specific stuff if there's no need for
it, so I'd slightly prefer yours.
On the other hand, it's not just for musl. If somebody tries to build
this against, say, uClibc-ng (no reports about that yet), they will
probably run into trouble as well.
Or should we just unconditionally #define __UAPI_DEF_ETHHDR=0 just
before including <linux/virtio_net.h>?
--
Stefano
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] musl: fix conflict on ethhdr definition
2024-11-27 14:48 ` Stefano Brivio
@ 2024-11-27 14:55 ` Laurent Vivier
2024-11-27 15:05 ` Stefano Brivio
0 siblings, 1 reply; 6+ messages in thread
From: Laurent Vivier @ 2024-11-27 14:55 UTC (permalink / raw)
To: Stefano Brivio; +Cc: passt-dev
On 27/11/2024 15:48, Stefano Brivio wrote:
> On Wed, 27 Nov 2024 15:41:30 +0100
> Laurent Vivier <lvivier@redhat.com> wrote:
>
>> there is a conflict between netinet/if_ether.h provided by musl and
>> linux/if_ether.h provided by the linux headers:
>>
>> In file included from passt.h:185,
>> from tcp_vu.c:21:
>> /usr/include/netinet/if_ether.h:115:8: error: redefinition of 'struct ethhdr'
>> 115 | struct ethhdr {
>> | ^~~~~~
>> In file included from /usr/include/linux/virtio_net.h:32,
>> from tcp_vu.c:17:
>> /usr/include/linux/if_ether.h:173:8: note: originally defined here
>> 173 | struct ethhdr {
>> | ^~~~~~
>>
>> The kernel headers provide a flag to disable the definition in this case,
>> __UAPI_DEF_ETHHDR (see /usr/include/linux/if_ether.h comment).
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
>> Makefile | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/Makefile b/Makefile
>> index cb7448079de5..2aa56ada65fd 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -33,6 +33,9 @@ FLAGS += $(FORTIFY_FLAG) -O2 -pie -fPIE
>> FLAGS += -DPAGE_SIZE=$(shell getconf PAGE_SIZE)
>> FLAGS += -DVERSION=\"$(VERSION)\"
>> FLAGS += -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS)
>> +ifeq (musl, $(word 4,$(subst -, ,$(TARGET))))
>> +FLAGS += -D__UAPI_DEF_ETHHDR=0
>> +endif
>
> Uh oh, I just solved this locally (I was about to push, but I stopped
> in time) by simply including <linux/if_ether.h> instead of
> <netinet/if_ether.h> from passt.h.
>
> I'm not sure what's the best solution: mine is simpler, but I'm not
> really fond of including Linux-specific stuff if there's no need for
> it, so I'd slightly prefer yours.
>
> On the other hand, it's not just for musl. If somebody tries to build
> this against, say, uClibc-ng (no reports about that yet), they will
> probably run into trouble as well.
In fact, in /usr/include/linux/if_ether.h they say:
/* allow libcs like musl to deactivate this, glibc does not implement this. */
So I think musl should set the value and then I found:
#define __UAPI_DEF_ETHHDR 0
in /usr/include/netinet/if_ether.h
So I think the solution is to include netinet/if_ether.h before linux/if_ether.h
>
> Or should we just unconditionally #define __UAPI_DEF_ETHHDR=0 just
> before including <linux/virtio_net.h>?
If you do that, I think it will not work anymore with glibc
Thanks,
Laurent
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] musl: fix conflict on ethhdr definition
2024-11-27 14:55 ` Laurent Vivier
@ 2024-11-27 15:05 ` Stefano Brivio
2024-11-27 15:37 ` Laurent Vivier
0 siblings, 1 reply; 6+ messages in thread
From: Stefano Brivio @ 2024-11-27 15:05 UTC (permalink / raw)
To: Laurent Vivier; +Cc: passt-dev
On Wed, 27 Nov 2024 15:55:00 +0100
Laurent Vivier <lvivier@redhat.com> wrote:
> On 27/11/2024 15:48, Stefano Brivio wrote:
> > On Wed, 27 Nov 2024 15:41:30 +0100
> > Laurent Vivier <lvivier@redhat.com> wrote:
> >
> >> there is a conflict between netinet/if_ether.h provided by musl and
> >> linux/if_ether.h provided by the linux headers:
> >>
> >> In file included from passt.h:185,
> >> from tcp_vu.c:21:
> >> /usr/include/netinet/if_ether.h:115:8: error: redefinition of 'struct ethhdr'
> >> 115 | struct ethhdr {
> >> | ^~~~~~
> >> In file included from /usr/include/linux/virtio_net.h:32,
> >> from tcp_vu.c:17:
> >> /usr/include/linux/if_ether.h:173:8: note: originally defined here
> >> 173 | struct ethhdr {
> >> | ^~~~~~
> >>
> >> The kernel headers provide a flag to disable the definition in this case,
> >> __UAPI_DEF_ETHHDR (see /usr/include/linux/if_ether.h comment).
> >>
> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> >> ---
> >> Makefile | 3 +++
> >> 1 file changed, 3 insertions(+)
> >>
> >> diff --git a/Makefile b/Makefile
> >> index cb7448079de5..2aa56ada65fd 100644
> >> --- a/Makefile
> >> +++ b/Makefile
> >> @@ -33,6 +33,9 @@ FLAGS += $(FORTIFY_FLAG) -O2 -pie -fPIE
> >> FLAGS += -DPAGE_SIZE=$(shell getconf PAGE_SIZE)
> >> FLAGS += -DVERSION=\"$(VERSION)\"
> >> FLAGS += -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS)
> >> +ifeq (musl, $(word 4,$(subst -, ,$(TARGET))))
> >> +FLAGS += -D__UAPI_DEF_ETHHDR=0
> >> +endif
> >
> > Uh oh, I just solved this locally (I was about to push, but I stopped
> > in time) by simply including <linux/if_ether.h> instead of
> > <netinet/if_ether.h> from passt.h.
> >
> > I'm not sure what's the best solution: mine is simpler, but I'm not
> > really fond of including Linux-specific stuff if there's no need for
> > it, so I'd slightly prefer yours.
> >
> > On the other hand, it's not just for musl. If somebody tries to build
> > this against, say, uClibc-ng (no reports about that yet), they will
> > probably run into trouble as well.
>
> In fact, in /usr/include/linux/if_ether.h they say:
>
> /* allow libcs like musl to deactivate this, glibc does not implement this. */
>
> So I think musl should set the value and then I found:
>
> #define __UAPI_DEF_ETHHDR 0
>
> in /usr/include/netinet/if_ether.h
Ah, I hadn't checked that yet.
> So I think the solution is to include netinet/if_ether.h before linux/if_ether.h
Just in tcp_vu.c you mean? That's where we include <linux/if_ether.h>
now.
Any advantage over simply including linux/if_ether.h in passt.h as I
did?
> > Or should we just unconditionally #define __UAPI_DEF_ETHHDR=0 just
> > before including <linux/virtio_net.h>?
>
> If you do that, I think it will not work anymore with glibc
Right, discarded that option.
--
Stefano
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] musl: fix conflict on ethhdr definition
2024-11-27 15:05 ` Stefano Brivio
@ 2024-11-27 15:37 ` Laurent Vivier
2024-11-27 15:45 ` Stefano Brivio
0 siblings, 1 reply; 6+ messages in thread
From: Laurent Vivier @ 2024-11-27 15:37 UTC (permalink / raw)
To: Stefano Brivio; +Cc: passt-dev
On 27/11/2024 16:05, Stefano Brivio wrote:
> On Wed, 27 Nov 2024 15:55:00 +0100
> Laurent Vivier <lvivier@redhat.com> wrote:
>
>> On 27/11/2024 15:48, Stefano Brivio wrote:
>>> On Wed, 27 Nov 2024 15:41:30 +0100
>>> Laurent Vivier <lvivier@redhat.com> wrote:
>>>
>>>> there is a conflict between netinet/if_ether.h provided by musl and
>>>> linux/if_ether.h provided by the linux headers:
>>>>
>>>> In file included from passt.h:185,
>>>> from tcp_vu.c:21:
>>>> /usr/include/netinet/if_ether.h:115:8: error: redefinition of 'struct ethhdr'
>>>> 115 | struct ethhdr {
>>>> | ^~~~~~
>>>> In file included from /usr/include/linux/virtio_net.h:32,
>>>> from tcp_vu.c:17:
>>>> /usr/include/linux/if_ether.h:173:8: note: originally defined here
>>>> 173 | struct ethhdr {
>>>> | ^~~~~~
>>>>
>>>> The kernel headers provide a flag to disable the definition in this case,
>>>> __UAPI_DEF_ETHHDR (see /usr/include/linux/if_ether.h comment).
>>>>
>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>>> ---
>>>> Makefile | 3 +++
>>>> 1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/Makefile b/Makefile
>>>> index cb7448079de5..2aa56ada65fd 100644
>>>> --- a/Makefile
>>>> +++ b/Makefile
>>>> @@ -33,6 +33,9 @@ FLAGS += $(FORTIFY_FLAG) -O2 -pie -fPIE
>>>> FLAGS += -DPAGE_SIZE=$(shell getconf PAGE_SIZE)
>>>> FLAGS += -DVERSION=\"$(VERSION)\"
>>>> FLAGS += -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS)
>>>> +ifeq (musl, $(word 4,$(subst -, ,$(TARGET))))
>>>> +FLAGS += -D__UAPI_DEF_ETHHDR=0
>>>> +endif
>>>
>>> Uh oh, I just solved this locally (I was about to push, but I stopped
>>> in time) by simply including <linux/if_ether.h> instead of
>>> <netinet/if_ether.h> from passt.h.
>>>
>>> I'm not sure what's the best solution: mine is simpler, but I'm not
>>> really fond of including Linux-specific stuff if there's no need for
>>> it, so I'd slightly prefer yours.
>>>
>>> On the other hand, it's not just for musl. If somebody tries to build
>>> this against, say, uClibc-ng (no reports about that yet), they will
>>> probably run into trouble as well.
>>
>> In fact, in /usr/include/linux/if_ether.h they say:
>>
>> /* allow libcs like musl to deactivate this, glibc does not implement this. */
>>
>> So I think musl should set the value and then I found:
>>
>> #define __UAPI_DEF_ETHHDR 0
>>
>> in /usr/include/netinet/if_ether.h
>
> Ah, I hadn't checked that yet.
>
>> So I think the solution is to include netinet/if_ether.h before linux/if_ether.h
>
> Just in tcp_vu.c you mean? That's where we include <linux/if_ether.h>
> now.
>
> Any advantage over simply including linux/if_ether.h in passt.h as I
> did?
passt.h remains generic by not including linux specific headers.
Something like that:
diff --git a/tcp_vu.c b/tcp_vu.c
index 05e2d1d..943b6d9 100644
--- a/tcp_vu.c
+++ b/tcp_vu.c
@@ -13,7 +13,7 @@
#include <netinet/tcp.h>
#include <sys/socket.h>
-
+#include <netinet/if_ether.h>
#include <linux/virtio_net.h>
#include "util.h"
diff --git a/vu_common.c b/vu_common.c
index 0763ec0..299b5a3 100644
--- a/vu_common.c
+++ b/vu_common.c
@@ -8,6 +8,7 @@
#include <unistd.h>
#include <sys/uio.h>
#include <sys/eventfd.h>
+#include <netinet/if_ether.h>
#include <linux/virtio_net.h>
#include "util.h"
Thanks,
Laurent
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] musl: fix conflict on ethhdr definition
2024-11-27 15:37 ` Laurent Vivier
@ 2024-11-27 15:45 ` Stefano Brivio
0 siblings, 0 replies; 6+ messages in thread
From: Stefano Brivio @ 2024-11-27 15:45 UTC (permalink / raw)
To: Laurent Vivier; +Cc: passt-dev
On Wed, 27 Nov 2024 16:37:05 +0100
Laurent Vivier <lvivier@redhat.com> wrote:
> On 27/11/2024 16:05, Stefano Brivio wrote:
> > On Wed, 27 Nov 2024 15:55:00 +0100
> > Laurent Vivier <lvivier@redhat.com> wrote:
> >
> >> On 27/11/2024 15:48, Stefano Brivio wrote:
> >>> On Wed, 27 Nov 2024 15:41:30 +0100
> >>> Laurent Vivier <lvivier@redhat.com> wrote:
> >>>
> >>>> there is a conflict between netinet/if_ether.h provided by musl and
> >>>> linux/if_ether.h provided by the linux headers:
> >>>>
> >>>> In file included from passt.h:185,
> >>>> from tcp_vu.c:21:
> >>>> /usr/include/netinet/if_ether.h:115:8: error: redefinition of 'struct ethhdr'
> >>>> 115 | struct ethhdr {
> >>>> | ^~~~~~
> >>>> In file included from /usr/include/linux/virtio_net.h:32,
> >>>> from tcp_vu.c:17:
> >>>> /usr/include/linux/if_ether.h:173:8: note: originally defined here
> >>>> 173 | struct ethhdr {
> >>>> | ^~~~~~
> >>>>
> >>>> The kernel headers provide a flag to disable the definition in this case,
> >>>> __UAPI_DEF_ETHHDR (see /usr/include/linux/if_ether.h comment).
> >>>>
> >>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> >>>> ---
> >>>> Makefile | 3 +++
> >>>> 1 file changed, 3 insertions(+)
> >>>>
> >>>> diff --git a/Makefile b/Makefile
> >>>> index cb7448079de5..2aa56ada65fd 100644
> >>>> --- a/Makefile
> >>>> +++ b/Makefile
> >>>> @@ -33,6 +33,9 @@ FLAGS += $(FORTIFY_FLAG) -O2 -pie -fPIE
> >>>> FLAGS += -DPAGE_SIZE=$(shell getconf PAGE_SIZE)
> >>>> FLAGS += -DVERSION=\"$(VERSION)\"
> >>>> FLAGS += -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS)
> >>>> +ifeq (musl, $(word 4,$(subst -, ,$(TARGET))))
> >>>> +FLAGS += -D__UAPI_DEF_ETHHDR=0
> >>>> +endif
> >>>
> >>> Uh oh, I just solved this locally (I was about to push, but I stopped
> >>> in time) by simply including <linux/if_ether.h> instead of
> >>> <netinet/if_ether.h> from passt.h.
> >>>
> >>> I'm not sure what's the best solution: mine is simpler, but I'm not
> >>> really fond of including Linux-specific stuff if there's no need for
> >>> it, so I'd slightly prefer yours.
> >>>
> >>> On the other hand, it's not just for musl. If somebody tries to build
> >>> this against, say, uClibc-ng (no reports about that yet), they will
> >>> probably run into trouble as well.
> >>
> >> In fact, in /usr/include/linux/if_ether.h they say:
> >>
> >> /* allow libcs like musl to deactivate this, glibc does not implement this. */
> >>
> >> So I think musl should set the value and then I found:
> >>
> >> #define __UAPI_DEF_ETHHDR 0
> >>
> >> in /usr/include/netinet/if_ether.h
> >
> > Ah, I hadn't checked that yet.
> >
> >> So I think the solution is to include netinet/if_ether.h before linux/if_ether.h
> >
> > Just in tcp_vu.c you mean? That's where we include <linux/if_ether.h>
> > now.
> >
> > Any advantage over simply including linux/if_ether.h in passt.h as I
> > did?
>
> passt.h remains generic by not including linux specific headers.
>
> Something like that:
>
> diff --git a/tcp_vu.c b/tcp_vu.c
> index 05e2d1d..943b6d9 100644
> --- a/tcp_vu.c
> +++ b/tcp_vu.c
> @@ -13,7 +13,7 @@
> #include <netinet/tcp.h>
>
> #include <sys/socket.h>
> -
> +#include <netinet/if_ether.h>
> #include <linux/virtio_net.h>
>
> #include "util.h"
> diff --git a/vu_common.c b/vu_common.c
> index 0763ec0..299b5a3 100644
> --- a/vu_common.c
> +++ b/vu_common.c
> @@ -8,6 +8,7 @@
> #include <unistd.h>
> #include <sys/uio.h>
> #include <sys/eventfd.h>
> +#include <netinet/if_ether.h>
> #include <linux/virtio_net.h>
>
> #include "util.h"
>
Okay, that works as well, just tested on glibc and musl. I'll simply
fold it in 7/9 so that we don't break bisection with musl.
--
Stefano
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-11-27 15:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-11-27 14:41 [PATCH] musl: fix conflict on ethhdr definition Laurent Vivier
2024-11-27 14:48 ` Stefano Brivio
2024-11-27 14:55 ` Laurent Vivier
2024-11-27 15:05 ` Stefano Brivio
2024-11-27 15:37 ` Laurent Vivier
2024-11-27 15:45 ` Stefano Brivio
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).