[iptables PATCH] Revert "fix build for missing ETH_ALEN definition"

[iptables PATCH] Revert "fix build for missing ETH_ALEN definition"

[Phil Sutter]

This reverts commit c5d9a723b5159a28f547b577711787295a14fd84 as it broke
compiling against musl libc. Might be a bug in the latter, but for the
time being try to please both by avoiding the include and instead
defining ETH_ALEN if unset.

While being at it, move netinet/ether.h include up.

Fixes: 1bdb5535f561a ("libxtables: Extend MAC address printing/parsing support")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 libxtables/xtables.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/libxtables/xtables.c b/libxtables/xtables.c
index 96fd783a066cf..0638f9271c601 100644
--- a/libxtables/xtables.c
+++ b/libxtables/xtables.c
@@ -28,6 +28,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include <netinet/ether.h>
 #include <sys/socket.h>
 #include <sys/stat.h>
 #include <sys/statfs.h>
@@ -45,7 +46,6 @@
 
 #include <xtables.h>
 #include <limits.h> /* INT_MAX in ip_tables.h/ip6_tables.h */
-#include <linux/if_ether.h> /* ETH_ALEN */
 #include <linux/netfilter_ipv4/ip_tables.h>
 #include <linux/netfilter_ipv6/ip6_tables.h>
 #include <libiptc/libxtc.h>
@@ -72,6 +72,10 @@
 #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
@@ -2245,8 +2249,6 @@ void xtables_print_num(uint64_t number, unsigned int format)
 	printf(FMT("%4lluT ","%lluT "), (unsigned long long)number);
 }
 
-#include <netinet/ether.h>
-
 static const unsigned char mac_type_unicast[ETH_ALEN] =   {};
 static const unsigned char msk_type_unicast[ETH_ALEN] =   {1};
 static const unsigned char mac_type_multicast[ETH_ALEN] = {1};
-- 
2.34.1

Re: [iptables PATCH] Revert "fix build for missing ETH_ALEN definition"

[Phil Sutter]

Nick, Maciej, does this patch work for you?

On Wed, May 18, 2022 at 04:20:46PM +0200, Phil Sutter wrote:
> This reverts commit c5d9a723b5159a28f547b577711787295a14fd84 as it broke
> compiling against musl libc. Might be a bug in the latter, but for the
> time being try to please both by avoiding the include and instead
> defining ETH_ALEN if unset.
> 
> While being at it, move netinet/ether.h include up.
> 
> Fixes: 1bdb5535f561a ("libxtables: Extend MAC address printing/parsing support")
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  libxtables/xtables.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/libxtables/xtables.c b/libxtables/xtables.c
> index 96fd783a066cf..0638f9271c601 100644
> --- a/libxtables/xtables.c
> +++ b/libxtables/xtables.c
> @@ -28,6 +28,7 @@
>  #include <stdlib.h>
>  #include <string.h>
>  #include <unistd.h>
> +#include <netinet/ether.h>
>  #include <sys/socket.h>
>  #include <sys/stat.h>
>  #include <sys/statfs.h>
> @@ -45,7 +46,6 @@
>  
>  #include <xtables.h>
>  #include <limits.h> /* INT_MAX in ip_tables.h/ip6_tables.h */
> -#include <linux/if_ether.h> /* ETH_ALEN */
>  #include <linux/netfilter_ipv4/ip_tables.h>
>  #include <linux/netfilter_ipv6/ip6_tables.h>
>  #include <libiptc/libxtc.h>
> @@ -72,6 +72,10 @@
>  #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
> @@ -2245,8 +2249,6 @@ void xtables_print_num(uint64_t number, unsigned int format)
>  	printf(FMT("%4lluT ","%lluT "), (unsigned long long)number);
>  }
>  
> -#include <netinet/ether.h>
> -
>  static const unsigned char mac_type_unicast[ETH_ALEN] =   {};
>  static const unsigned char msk_type_unicast[ETH_ALEN] =   {1};
>  static const unsigned char mac_type_multicast[ETH_ALEN] = {1};
> -- 
> 2.34.1
> 
> 

Re: [iptables PATCH] Revert "fix build for missing ETH_ALEN definition"

[Maciej Żenczykowski]

On Thu, May 19, 2022 at 8:46 AM Phil Sutter <phil@nwl.cc> wrote:
> Nick, Maciej, does this patch work for you?

Reviewed-by: Maciej Żenczykowski <maze@google.com>

It builds locally, I've also uploaded to:
  https://android-review.googlesource.com/c/platform/external/iptables/+/2101317
and we'll see if TreeHugger is happy with it, but I don't see how it
could not be.

---

Note: AOSP is still only at v1.8.7 baseline, with minimal modifications:

diff --stat f485d324e99fc9a9a7fe310b97e1ebf8114b36c6..HEAD
 .gitignore                          |   2 +-
 Android.bp                          |  70 ++++++++++++++++++++++++++++
 METADATA                            |  17 +++++++
 MODULE_LICENSE_GPL                  |   0
 NOTICE                              |   1 +
 OWNERS                              |   2 +
 TEST_MAPPING                        |   9 ++++
 config.h                            |  86 ++++++++++++++++++++++++++++++++++
 extensions/Android.bp               | 139
+++++++++++++++++++++++++++++++++++++++++++++++++++++++
 extensions/filter_init              |   7 +++
 extensions/gen_init                 |  36 +++++++++++++++
 extensions/libxt_IDLETIMER.c        |   9 ++++
 extensions/libxt_IDLETIMER.man      |   4 ++
 extensions/libxt_quota2.c           | 141
++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 extensions/libxt_quota2.man         |  37 +++++++++++++++
 include/linux/netfilter/xt_quota2.h |  25 ++++++++++
 include/xtables-version.h           |   2 +
 iptables/Android.bp                 |  87 ++++++++++++++++++++++++++++++++++
 iptables/NOTICE                     |   1 +
 iptables/iptables-standalone.c      |   3 ++
 iptables/xtables.lock               |   0
 libiptc/Android.bp                  |  31 +++++++++++++
 libxtables/Android.bp               |  36 +++++++++++++++
 libxtables/xtables.c                |   5 ++
 24 files changed, 749 insertions(+), 1 deletion(-)

I still need to sit down for real and figure out how to upstream the
IDLETIMER/quota2 deviations.
Just never have enough time to figure out what they really do and
which parts are actually truly required...
(there's no documentation or real tests... and some tests that do
exist actively appear to test that stuff *doesn't* work due to bad
assumptions in the test code... see
https://android-review.googlesource.com/c/platform/system/netd/+/1728122
)

I also don't see us switching to nftables any time soon because we
still need to support 4.14 kernels pretty much indefinitely as I've
just learnt.
(I'd also like to switch over from iptables to ebpf for everything
eventually... but that also feels like a pipe dream).

> On Wed, May 18, 2022 at 04:20:46PM +0200, Phil Sutter wrote:
> > This reverts commit c5d9a723b5159a28f547b577711787295a14fd84 as it broke
> > compiling against musl libc. Might be a bug in the latter, but for the
> > time being try to please both by avoiding the include and instead
> > defining ETH_ALEN if unset.
> >
> > While being at it, move netinet/ether.h include up.
> >
> > Fixes: 1bdb5535f561a ("libxtables: Extend MAC address printing/parsing support")
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > ---
> >  libxtables/xtables.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/libxtables/xtables.c b/libxtables/xtables.c
> > index 96fd783a066cf..0638f9271c601 100644
> > --- a/libxtables/xtables.c
> > +++ b/libxtables/xtables.c
> > @@ -28,6 +28,7 @@
> >  #include <stdlib.h>
> >  #include <string.h>
> >  #include <unistd.h>
> > +#include <netinet/ether.h>
> >  #include <sys/socket.h>
> >  #include <sys/stat.h>
> >  #include <sys/statfs.h>
> > @@ -45,7 +46,6 @@
> >
> >  #include <xtables.h>
> >  #include <limits.h> /* INT_MAX in ip_tables.h/ip6_tables.h */
> > -#include <linux/if_ether.h> /* ETH_ALEN */
> >  #include <linux/netfilter_ipv4/ip_tables.h>
> >  #include <linux/netfilter_ipv6/ip6_tables.h>
> >  #include <libiptc/libxtc.h>
> > @@ -72,6 +72,10 @@
> >  #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
> > @@ -2245,8 +2249,6 @@ void xtables_print_num(uint64_t number, unsigned int format)
> >       printf(FMT("%4lluT ","%lluT "), (unsigned long long)number);
> >  }
> >
> > -#include <netinet/ether.h>
> > -
> >  static const unsigned char mac_type_unicast[ETH_ALEN] =   {};
> >  static const unsigned char msk_type_unicast[ETH_ALEN] =   {1};
> >  static const unsigned char mac_type_multicast[ETH_ALEN] = {1};
> > --
> > 2.34.1
> >
> >Maciej Żenczykowski, Kernel Networking Developer @ Google