| Message ID | 20240709130545.882519-1-joshualant@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | xtables: Fix compilation error with musl-libc | expand |
On Tue, Jul 09, 2024 at 01:05:45PM +0000, Joshua Lant wrote: > Error compiling with musl-libc: > The commit hash 810f8568f44f5863c2350a39f4f5c8d60f762958 introduces the > netinet/ether.h header into xtables.h, which causes an error due to the > redefinition of the ethhdr struct, defined in linux/if_ether.h and > netinet/ether.h. > > This is is a known issue with musl-libc, with kernel headers providing > guards against this happening when glibc is used: > https://wiki.musl-libc.org/faq (Q: Why am I getting “error: redefinition > of struct ethhdr/tcphdr/etc”?) > > The only value used from netinet/ether.h is ETH_ALEN, which is already set > manually in libxtables/xtables.c. Move this definition to the header and > eliminate the inclusion of netinet/if_ether.h. Man, this crap keeps popping up. Last time we "fixed" it was in commit 0e7cf0ad306cd ("Revert "fix build for missing ETH_ALEN definition""). There, including netinet/ether.h was OK. Now it's problematic? Interestingly, linux/if_ether.h has this: | /* allow libcs like musl to deactivate this, glibc does not implement this. */ | #ifndef __UAPI_DEF_ETHHDR | #define __UAPI_DEF_ETHHDR 1 | #endif So it's not like the other party is ignoring musl's needs. Does adding -D__UAPI_DEF_ETHHDR=0 fix the build? Should we maybe add a configure option for this instead of shooting the moving target? Cheers, Phil
On Tue, Jul 09, 2024 at 01:05:45PM +0000, Joshua Lant wrote: > Error compiling with musl-libc: > The commit hash 810f8568f44f5863c2350a39f4f5c8d60f762958 introduces the > netinet/ether.h header into xtables.h, which causes an error due to the > redefinition of the ethhdr struct, defined in linux/if_ether.h and > netinet/ether.h. > > This is is a known issue with musl-libc, with kernel headers providing > guards against this happening when glibc is used: > https://wiki.musl-libc.org/faq (Q: Why am I getting “error: redefinition > of struct ethhdr/tcphdr/etc”?) > > The only value used from netinet/ether.h is ETH_ALEN, which is already set > manually in libxtables/xtables.c. Move this definition to the header and > eliminate the inclusion of netinet/if_ether.h. Any chance that musl headers are being used so this can be autodetected? Then, no option to pass -D__UAPI_DEF_ETHHDR=0 is required. thanks
On Mon, Aug 19, 2024 at 07:24:06PM +0200, Pablo Neira Ayuso wrote: > On Tue, Jul 09, 2024 at 01:05:45PM +0000, Joshua Lant wrote: > > Error compiling with musl-libc: > > The commit hash 810f8568f44f5863c2350a39f4f5c8d60f762958 introduces the > > netinet/ether.h header into xtables.h, which causes an error due to the > > redefinition of the ethhdr struct, defined in linux/if_ether.h and > > netinet/ether.h. > > > > This is is a known issue with musl-libc, with kernel headers providing > > guards against this happening when glibc is used: > > https://wiki.musl-libc.org/faq (Q: Why am I getting “error: redefinition > > of struct ethhdr/tcphdr/etc”?) > > > > The only value used from netinet/ether.h is ETH_ALEN, which is already set > > manually in libxtables/xtables.c. Move this definition to the header and > > eliminate the inclusion of netinet/if_ether.h. > > Any chance that musl headers are being used so this can be autodetected? > Then, no option to pass -D__UAPI_DEF_ETHHDR=0 is required. To clarify, what I mean is if it is possible to autodetect that musl headers are used, then add this definition. I'd prefer no new --option as you propose is required to handle this.
On 2024-08-20, at 13:28:22 +0200, Pablo Neira Ayuso wrote: > On Mon, Aug 19, 2024 at 07:24:06PM +0200, Pablo Neira Ayuso wrote: > > On Tue, Jul 09, 2024 at 01:05:45PM +0000, Joshua Lant wrote: > > > Error compiling with musl-libc: > > > The commit hash 810f8568f44f5863c2350a39f4f5c8d60f762958 introduces the > > > netinet/ether.h header into xtables.h, which causes an error due to the > > > redefinition of the ethhdr struct, defined in linux/if_ether.h and > > > netinet/ether.h. > > > > > > This is is a known issue with musl-libc, with kernel headers providing > > > guards against this happening when glibc is used: > > > https://wiki.musl-libc.org/faq (Q: Why am I getting “error: redefinition > > > of struct ethhdr/tcphdr/etc”?) > > > > > > The only value used from netinet/ether.h is ETH_ALEN, which is already set > > > manually in libxtables/xtables.c. Move this definition to the header and > > > eliminate the inclusion of netinet/if_ether.h. > > > > Any chance that musl headers are being used so this can be autodetected? > > Then, no option to pass -D__UAPI_DEF_ETHHDR=0 is required. > > To clarify, what I mean is if it is possible to autodetect that musl > headers are used, then add this definition. > > I'd prefer no new --option as you propose is required to handle this. There are a couple of approaches that I see. 1. Test whether netinet/if_ether.h sets `__UAPI_DEF_ETHHDR` to zero (in which case we are building with musl): saved_CPPFLAGS=${CPPFLAGS} CPPFLAGS=${regular_CPPFLAGS} AC_PREPROC_IFELSE([AC_LANG_SOURCE([[ #include <netinet/if_ether.h> #if defined(__UAPI_DEF_ETHHDR) && __UAPI_DEF_ETHHDR == 0 #else #error No __UAPI_DEF_ETHHDR #endif ]])], [AC_DEFINE([__UAPI_DEF_ETHHDR], [0], [Prevent multiple definitions of `struct ethhdr`])]) CPPFLAGS=${saved_CPPFLAGS} 2. Just check `${host_os}`: AS_IF([test "${host_os}" = "linux-musl"], [AC_DEFINE([__UAPI_DEF_ETHHDR], [0], [Prevent multiple definitions of `struct ethhdr`])]) J.
diff --git a/include/xtables.h b/include/xtables.h index ab856ebc..e80d0a8e 100644 --- a/include/xtables.h +++ b/include/xtables.h @@ -12,13 +12,16 @@ #include <stdbool.h> #include <stddef.h> #include <stdint.h> -#include <netinet/ether.h> #include <netinet/in.h> #include <net/if.h> #include <linux/types.h> #include <linux/netfilter.h> #include <linux/netfilter/x_tables.h> +#ifndef ETH_ALEN +#define ETH_ALEN 6 +#endif + #ifndef IPPROTO_SCTP #define IPPROTO_SCTP 132 #endif diff --git a/libxtables/xtables.c b/libxtables/xtables.c index 7b370d48..b3219751 100644 --- a/libxtables/xtables.c +++ b/libxtables/xtables.c @@ -71,10 +71,6 @@ #define PROC_SYS_MODPROBE "/proc/sys/kernel/modprobe" #endif -#ifndef ETH_ALEN -#define ETH_ALEN 6 -#endif - /* we need this for ip6?tables-restore. ip6?tables-restore.c sets line to the * current line of the input file, in order to give a more precise error * message. ip6?tables itself doesn't need this, so it is initialized to the
Error compiling with musl-libc: The commit hash 810f8568f44f5863c2350a39f4f5c8d60f762958 introduces the netinet/ether.h header into xtables.h, which causes an error due to the redefinition of the ethhdr struct, defined in linux/if_ether.h and netinet/ether.h. This is is a known issue with musl-libc, with kernel headers providing guards against this happening when glibc is used: https://wiki.musl-libc.org/faq (Q: Why am I getting “error: redefinition of struct ethhdr/tcphdr/etc”?) The only value used from netinet/ether.h is ETH_ALEN, which is already set manually in libxtables/xtables.c. Move this definition to the header and eliminate the inclusion of netinet/if_ether.h. Signed-off-by: Joshua Lant joshualant@gmail.com --- include/xtables.h | 5 ++++- libxtables/xtables.c | 4 ---- 2 files changed, 4 insertions(+), 5 deletions(-)