[PATCH nftables] include: fix for musl with iptables v1.8.11

[PATCH nftables] include: fix for musl with iptables v1.8.11

[Alyssa Ross]

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.

Signed-off-by: Alyssa Ross <hi@alyssa.is>
---
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.

 include/linux/netfilter_bridge.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/netfilter_bridge.h b/include/linux/netfilter_bridge.h
index 6187a558..78ec2cde 100644
--- a/include/linux/netfilter_bridge.h
+++ b/include/linux/netfilter_bridge.h
@@ -4,8 +4,9 @@
 /* bridge-specific defines for netfilter. 
  */
 
+#include <netinet/if_ether.h>
+
 #include <linux/netfilter.h>
-#include <linux/if_ether.h>
 #include <linux/if_vlan.h>
 #include <linux/if_pppox.h>
 

base-commit: 3271d78e70ec75246e8a61a6791fe22b8d89c2c1
-- 
2.47.0

Re: [PATCH nftables] include: fix for musl with iptables v1.8.11

[Phil Sutter]

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.

> Signed-off-by: Alyssa Ross <hi@alyssa.is>
> ---
> 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?

Cheers, Phil

Re: [PATCH nftables] include: fix for musl with iptables v1.8.11

[Alyssa Ross]

[-- Attachment #1: Type: text/plain, Size: 2902 bytes --]

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.

> > Signed-off-by: Alyssa Ross <hi@alyssa.is>
> > ---
> > 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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH nftables] include: fix for musl with iptables v1.8.11

[Phil Sutter]

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@alyssa.is>
> > > ---
> > > 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

Re: [PATCH nftables] include: fix for musl with iptables v1.8.11

[Alyssa Ross]

[-- Attachment #1: Type: text/plain, Size: 3364 bytes --]

Phil Sutter <phil@nwl.cc> writes:

> 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.

Yeah — really it needs to be ensured in review that libc headers are
included before kernel headers, and libc headers are preferred over
kernel headers where both are available.  It's not too much to keep in
mind, but it does need to be taught/learned because the issues it causes
won't always be caught during testing in any particular environment.
The same holds for lots of things in C, though.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

Re: [PATCH nftables] include: fix for musl with iptables v1.8.11

[Pablo Neira Ayuso]

Hi,

On Fri, Dec 20, 2024 at 01:33:42PM +0100, Phil Sutter wrote:
> 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@alyssa.is>
> > > > ---
> > > > 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?

I see, you mean to use this (from iptables):

commit 76fce22826f8e860b5eb5b5a2463040c17ff85da
Author: Joshua Lant <joshualant@googlemail.com>
Date:   Wed Aug 28 13:47:31 2024 +0100

    configure: Determine if musl is used for build

and adapt to use it from nftables (and everywhere else).

Alyssa's patch is more simple, but it is mangling a cache kernel
header.

Is this configure.ac workaround needed everywhere in the netfilter.org
trees to make musl happy?

I don't see any better option at this stage.

Thanks.

Re: [PATCH nftables] include: fix for musl with iptables v1.8.11

[Phil Sutter]

Hi Pablo,

On Thu, Jan 09, 2025 at 12:34:01AM +0100, Pablo Neira Ayuso wrote:
> On Fri, Dec 20, 2024 at 01:33:42PM +0100, Phil Sutter wrote:
> > 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@alyssa.is>
> > > > > ---
> > > > > 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?
> 
> I see, you mean to use this (from iptables):
> 
> commit 76fce22826f8e860b5eb5b5a2463040c17ff85da
> Author: Joshua Lant <joshualant@googlemail.com>
> Date:   Wed Aug 28 13:47:31 2024 +0100
> 
>     configure: Determine if musl is used for build
> 
> and adapt to use it from nftables (and everywhere else).
> 
> Alyssa's patch is more simple, but it is mangling a cache kernel
> header.

Right, so the fixing should start in kernel repo, then update the cached
header in nftables.

> Is this configure.ac workaround needed everywhere in the netfilter.org
> trees to make musl happy?

AIUI, only in those including linux/if_ether.h directly. ;)
Though there is such include in nftables' src/payload.c which remains
unchanged by Alyssa's patch. Not sure if intentional, maybe there are
factors which make the include harmless.

> I don't see any better option at this stage.

According to a quick grep, there are currently six spots including
linux/if_ether.h (including one cached kernel header). I like about the
configure hack that it avoids breaking musl by accident via some
seemingly unrelated patch. Just one less thing to keep in mind, but this
is just me. I appreciate other opinions!

Cheers, Phil

PS: Can't we just fix musl instead? *hides*

Re: [PATCH nftables] include: fix for musl with iptables v1.8.11

[Pablo Neira Ayuso]

Hi Phil,

On Thu, Jan 09, 2025 at 12:49:53AM +0100, Phil Sutter wrote:
> Hi Pablo,
>
> On Thu, Jan 09, 2025 at 12:34:01AM +0100, Pablo Neira Ayuso wrote:
> > On Fri, Dec 20, 2024 at 01:33:42PM +0100, Phil Sutter wrote:
> > > 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@alyssa.is>
> > > > > > ---
> > > > > > 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?
> >
> > I see, you mean to use this (from iptables):
> >
> > commit 76fce22826f8e860b5eb5b5a2463040c17ff85da
> > Author: Joshua Lant <joshualant@googlemail.com>
> > Date:   Wed Aug 28 13:47:31 2024 +0100
> >
> >     configure: Determine if musl is used for build
> >
> > and adapt to use it from nftables (and everywhere else).
> >
> > Alyssa's patch is more simple, but it is mangling a cache kernel
> > header.
>
> Right, so the fixing should start in kernel repo, then update the cached
> header in nftables.
>
> > Is this configure.ac workaround needed everywhere in the netfilter.org
> > trees to make musl happy?
>
> AIUI, only in those including linux/if_ether.h directly. ;)

Hm. I am looking at include/uapi/linux/netfilter_bridge.h and...

#include <linux/in.h>
#include <linux/netfilter.h>
#include <linux/if_ether.h>
#include <linux/if_vlan.h>
#include <linux/if_pppox.h>

I don't understand why all those #include need to be there, this
header file contains only defines an enumeration... git annotate takes
me to:

  607ca46e97a1 ("UAPI: (Scripted) Disintegrate include/linux")

Silly question: Does it break any netfilter userspace software if
#include <linux/if_ether.h> is moved from that kernel header file to
to the non-uapi netfilter_bridge.h header file.

> Though there is such include in nftables' src/payload.c which remains
> unchanged by Alyssa's patch. Not sure if intentional, maybe there are
> factors which make the include harmless.

This is userspace, I guess we can replace it with userspace netinet/
header variant.

> > I don't see any better option at this stage.
>
> According to a quick grep, there are currently six spots including
> linux/if_ether.h (including one cached kernel header). I like about the
> configure hack that it avoids breaking musl by accident via some
> seemingly unrelated patch. Just one less thing to keep in mind, but this
> is just me. I appreciate other opinions!

Agreed, avoiding the configure kludge would be good.

> Cheers, Phil
>
> PS: Can't we just fix musl instead? *hides*

Re: [PATCH nftables] include: fix for musl with iptables v1.8.11

[Phil Sutter]

Hi Pablo,

On Thu, Jan 09, 2025 at 01:03:25AM +0100, Pablo Neira Ayuso wrote:
> On Thu, Jan 09, 2025 at 12:49:53AM +0100, Phil Sutter wrote:
> > On Thu, Jan 09, 2025 at 12:34:01AM +0100, Pablo Neira Ayuso wrote:
> > > On Fri, Dec 20, 2024 at 01:33:42PM +0100, Phil Sutter wrote:
> > > > 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@alyssa.is>
> > > > > > > ---
> > > > > > > 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?
> > >
> > > I see, you mean to use this (from iptables):
> > >
> > > commit 76fce22826f8e860b5eb5b5a2463040c17ff85da
> > > Author: Joshua Lant <joshualant@googlemail.com>
> > > Date:   Wed Aug 28 13:47:31 2024 +0100
> > >
> > >     configure: Determine if musl is used for build
> > >
> > > and adapt to use it from nftables (and everywhere else).
> > >
> > > Alyssa's patch is more simple, but it is mangling a cache kernel
> > > header.
> >
> > Right, so the fixing should start in kernel repo, then update the cached
> > header in nftables.
> >
> > > Is this configure.ac workaround needed everywhere in the netfilter.org
> > > trees to make musl happy?
> >
> > AIUI, only in those including linux/if_ether.h directly. ;)
> 
> Hm. I am looking at include/uapi/linux/netfilter_bridge.h and...
> 
> #include <linux/in.h>
> #include <linux/netfilter.h>
> #include <linux/if_ether.h>
> #include <linux/if_vlan.h>
> #include <linux/if_pppox.h>
> 
> I don't understand why all those #include need to be there, this
> header file contains only defines an enumeration... git annotate takes
> me to:
> 
>   607ca46e97a1 ("UAPI: (Scripted) Disintegrate include/linux")
> 
> Silly question: Does it break any netfilter userspace software if
> #include <linux/if_ether.h> is moved from that kernel header file to
> to the non-uapi netfilter_bridge.h header file.

Silly counter question: Does removing an include statement break UAPI? I
was once told source files shall directly include everything they need
and not depend on includes in headers, but "now you need this extra
header" is not far away from "now you need this other header", right?

Even sillier, is Alyssa's change legal regarding UAPI? All of glibc,
musl and uClibc seem fine, but in theory there might be a libc which has
odd things in netinet/if_ether.h.

BTW: I see only a single UAPI kernel header include from netinet space
(include/uapi/linux/mptcp.h) and it's for compat reasons (06e445f740c1
("mptcp: fix conflict with <netinet/in.h>")). Speaking of which, what if
we added:

| #ifndef __KERNEL__
| #include <netinet/if_ether.h>
| #endif

early into include/uapi/linux/if_ether.h? Aside from any header caches,
this should fix all of user space at once, no?

> > Though there is such include in nftables' src/payload.c which remains
> > unchanged by Alyssa's patch. Not sure if intentional, maybe there are
> > factors which make the include harmless.
> 
> This is userspace, I guess we can replace it with userspace netinet/
> header variant.
> 
> > > I don't see any better option at this stage.
> >
> > According to a quick grep, there are currently six spots including
> > linux/if_ether.h (including one cached kernel header). I like about the
> > configure hack that it avoids breaking musl by accident via some
> > seemingly unrelated patch. Just one less thing to keep in mind, but this
> > is just me. I appreciate other opinions!
> 
> Agreed, avoiding the configure kludge would be good.

OK! I could probably write some git hooks to support my lossy memory.

Cheers, Phil

Re: [PATCH nftables] include: fix for musl with iptables v1.8.11

[Florian Westphal]

Phil Sutter <phil@nwl.cc> wrote:
> On Thu, Jan 09, 2025 at 01:03:25AM +0100, Pablo Neira Ayuso wrote:
> > Hm. I am looking at include/uapi/linux/netfilter_bridge.h and...
> > 
> > #include <linux/in.h>
> > #include <linux/netfilter.h>
> > #include <linux/if_ether.h>
> > #include <linux/if_vlan.h>
> > #include <linux/if_pppox.h>
> > 
> > I don't understand why all those #include need to be there, this
> > header file contains only defines an enumeration... git annotate takes
> > me to:
> > 
> >   607ca46e97a1 ("UAPI: (Scripted) Disintegrate include/linux")
> > 
> > Silly question: Does it break any netfilter userspace software if
> > #include <linux/if_ether.h> is moved from that kernel header file to
> > to the non-uapi netfilter_bridge.h header file.
> 
> Silly counter question: Does removing an include statement break UAPI?

Yes, it risks uapi breakage.

> BTW: I see only a single UAPI kernel header include from netinet space
> (include/uapi/linux/mptcp.h) and it's for compat reasons (06e445f740c1
> ("mptcp: fix conflict with <netinet/in.h>")). Speaking of which, what if
> we added:
> 
> | #ifndef __KERNEL__
> | #include <netinet/if_ether.h>
> | #endif
> 
> early into include/uapi/linux/if_ether.h? Aside from any header caches,
> this should fix all of user space at once, no?

Phil, would you send a formal patch to make this workaround *ONLY* in
netfilter_bridge.h (06e445f740c1)?

That way the fix can be propagated to nftables.git without having to
adjust cached headers on every update.

Re: [PATCH nftables] include: fix for musl with iptables v1.8.11

[Phil Sutter]

On Wed, Feb 11, 2026 at 02:49:42PM +0100, Florian Westphal wrote:
> Phil Sutter <phil@nwl.cc> wrote:
> > On Thu, Jan 09, 2025 at 01:03:25AM +0100, Pablo Neira Ayuso wrote:
> > > Hm. I am looking at include/uapi/linux/netfilter_bridge.h and...
> > > 
> > > #include <linux/in.h>
> > > #include <linux/netfilter.h>
> > > #include <linux/if_ether.h>
> > > #include <linux/if_vlan.h>
> > > #include <linux/if_pppox.h>
> > > 
> > > I don't understand why all those #include need to be there, this
> > > header file contains only defines an enumeration... git annotate takes
> > > me to:
> > > 
> > >   607ca46e97a1 ("UAPI: (Scripted) Disintegrate include/linux")
> > > 
> > > Silly question: Does it break any netfilter userspace software if
> > > #include <linux/if_ether.h> is moved from that kernel header file to
> > > to the non-uapi netfilter_bridge.h header file.
> > 
> > Silly counter question: Does removing an include statement break UAPI?
> 
> Yes, it risks uapi breakage.

I hope this extra include is safe in that regard.

> > BTW: I see only a single UAPI kernel header include from netinet space
> > (include/uapi/linux/mptcp.h) and it's for compat reasons (06e445f740c1
> > ("mptcp: fix conflict with <netinet/in.h>")). Speaking of which, what if
> > we added:
> > 
> > | #ifndef __KERNEL__
> > | #include <netinet/if_ether.h>
> > | #endif
> > 
> > early into include/uapi/linux/if_ether.h? Aside from any header caches,
> > this should fix all of user space at once, no?
> 
> Phil, would you send a formal patch to make this workaround *ONLY* in
> netfilter_bridge.h (06e445f740c1)?

DONE.

> That way the fix can be propagated to nftables.git without having to
> adjust cached headers on every update.

I assume you prefer to keep headers untouched which are
iptables-specific. But why not attempt to patch if_ether.h itself?

Cheers, Phil

Re: [PATCH nftables] include: fix for musl with iptables v1.8.11

[Florian Westphal]

Phil Sutter <phil@nwl.cc> wrote:
> > Phil, would you send a formal patch to make this workaround *ONLY* in
> > netfilter_bridge.h (06e445f740c1)?
> 
> DONE.

Thanks.

> > That way the fix can be propagated to nftables.git without having to
> > adjust cached headers on every update.
> 
> I assume you prefer to keep headers untouched which are
> iptables-specific. But why not attempt to patch if_ether.h itself?

Because this is a quagmire and I prefer to only break netfiler_bridge.h
users (which should be few) rather than everyone using if_ether.h.

Re: [PATCH nftables] include: fix for musl with iptables v1.8.11

[Phil Sutter]

On Wed, Feb 11, 2026 at 08:09:30PM +0100, Florian Westphal wrote:
> Phil Sutter <phil@nwl.cc> wrote:
> > > Phil, would you send a formal patch to make this workaround *ONLY* in
> > > netfilter_bridge.h (06e445f740c1)?
> > 
> > DONE.
> 
> Thanks.
> 
> > > That way the fix can be propagated to nftables.git without having to
> > > adjust cached headers on every update.
> > 
> > I assume you prefer to keep headers untouched which are
> > iptables-specific. But why not attempt to patch if_ether.h itself?
> 
> Because this is a quagmire and I prefer to only break netfiler_bridge.h
> users (which should be few) rather than everyone using if_ether.h.

Fine with me! The resulting flame^Wdiscussion from such a patch would
have been entertaining though, maybe even productive in a sense of
"either tell musl to go fix themselves or finish what has been started
by introducing __UAPI_DEF_ETHHDR." :)

Cheers, Phil