On Fri, Dec 20, 2024 at 01:07:56PM +0100, Alyssa Ross wrote:
> On Fri, Dec 20, 2024 at 12:50:42PM +0100, Phil Sutter wrote:
> > Hi Alyssa,
> >
> > On Fri, Dec 20, 2024 at 12:10:02AM +0100, Alyssa Ross wrote:
> > > Since iptables commit 810f8568 (libxtables: xtoptions: Implement
> > > XTTYPE_ETHERMACMASK), nftables failed to build for musl libc:
> > >
> > > In file included from /nix/store/bvffdqfhyxvx66bqlqqdmjmkyklkafv6-musl-1.2.5-dev/include/netinet/et…
> > > from /nix/store/kz6fymqpgbrj6330s6wv4idcf9pwsqs4-iptables-1.8.10-de…
> > > from src/xt.c:30:
> > > /nix/store/bvffdqfhyxvx66bqlqqdmjmkyklkafv6-musl-1.2.5-dev/include/netinet/if_ether.h:115:8: error: redefinition of 'struct ethhdr'
> > > 115 | struct ethhdr {
> > > | ^~~~~~
> > > In file included from ./include/linux/netfilter_bridge.h:8,
> > > from ./include/linux/netfilter_bridge/ebtables.h:1,
> > > from src/xt.c:27:
> > > /nix/store/bvffdqfhyxvx66bqlqqdmjmkyklkafv6-musl-1.2.5-dev/include/linux/if_ether.h:173:8: note: originally defined here
> > > 173 | struct ethhdr {
> > > | ^~~~~~
> > >
> > > The fix is to use libc's version of if_ether.h before any kernel
> > > headers, which takes care of conflicts with the kernel's struct ethhdr
> > > definition by defining __UAPI_DEF_ETHHDR, which will tell the kernel's
> > > header not to define its own version.
> >
> > What I don't like about this is how musl tries to force projects to not
> > include linux/if_ether.h directly. From the project's view, this is a
> > workaround not a fix.
>
> My understanding is that it's a general principle of using any libc on
> Linux that if there's both a libc and kernel header for the same thing,
> the libc header should be used. libc headers will of course include
> other libc headers in preference to kernel headers, so if you also
> include the kernel headers you're likely to end up with conflicts.
> Whether conflicts occur in any particular case depends on how a
> particular libc chooses to expose a particular kernel API. I could be
> misremembering, but I believe the same thing can happen with Glibc —
> some headers under sys/ cause conflicts with their corresponding kernel
> headers if both are included. While this case is musl specific, I
> think the principle applies to all libcs.
While this may be true for the vast majority of user space programs,
netfilter tools and libraries are a bit special in how close they
interface with the kernel. Not all netfilter-related kernel API is
exposed by glibc, for instance. Including (some) kernel headers is
therefore unavoidable, and (as your patch shows) order of inclusion
becomes subtly relevant in ways which won't show when compile-testing
against glibc only.
> > > Signed-off-by: Alyssa Ross <hi@xxxxxxxxx>
> > > ---
> > > A similar fix would solve the problem properly in iptables, which was
> > > worked around with 76fce228 ("configure: Determine if musl is used for build").
> > > The __UAPI_DEF_ETHHDR is supposed to be set by netinet/if_ether.h,
> > > rather than manually by users.
> >
> > Why does 76fce228 not work for you?
>
> It does work, but that's a fix for iptables. This is a fix for
> nftables. I could have submitted a copy of the iptables fix, but I
> don't think it's the best fix due to its reliance on internal macros
> that are not part of the public interface.
Ah, sorry! Patch subject and description managed to confuse me.
Pablo, what's your opinion? Maybe we should strive for the same solution
for the problem in all netfilter user space, so either take what we have
in iptables or adjust iptables to what nftables decides how things
should be?
Cheers, Phil
