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

On Wed, 27 Nov 2024 15:55:00 +0100
Laurent Vivier <lvivier(a)redhat.com> wrote:

...
  On 27/11/2024 15:48, Stefano Brivio wrote:
  On Wed, 27 Nov 2024 15:41:30 +0100
 Laurent Vivier <lvivier(a)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(a)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