Re: [PATCH] musl: fix conflict on ethhdr definition

[Laurent Vivier <lvivier@redhat.com>]

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
>