2.6.16-rc1: iptables broken on ppc32?

2.6.16-rc1: iptables broken on ppc32?

[Mikael Pettersson]

When trying out kernel 2.6.16-rc1 on a ppc32 box (G4 eMac),
the kernel refused to load my /etc/sysconfig/iptables. strace
on /sbin/iptables-restore shows that the kernel returns EINVAL
instead of accepting the configuration:

setsockopt(3, SOL_IP, 0x40 /* IP_??? */, "filter\0\214\0p\0\230\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 1664) = -1 EINVAL (Invalid argument)

The exact same configuration is accepted and works on an x86 box
also running 2.6.16-rc1, and of course the configuration worked
in all kernels up to and including 2.6.15 on the ppc32 box.

A much simplified /etc/sysconfig/iptables that fails on ppc32 but
works on x86 is the following:

*filter
:INPUT ACCEPT [0:0]
:FORWARD ACCEPT [0:0]
:OUTPUT ACCEPT [0:0]
:RH-Firewall-1-INPUT - [0:0]
-A INPUT -j RH-Firewall-1-INPUT
-A FORWARD -j RH-Firewall-1-INPUT
-A RH-Firewall-1-INPUT -i lo -j ACCEPT
-A RH-Firewall-1-INPUT -i eth0 -j ACCEPT
COMMIT

My 2.6.16-rc1 kernel configuration includes
CONFIG_NETFILTER_XTABLES=m
CONFIG_NETFILTER_XT_MATCH_STATE=m
CONFIG_IP_NF_CONNTRACK=m
CONFIG_IP_NF_IPTABLES=m
CONFIG_IP_NF_FILTER=m
CONFIG_IP_NF_TARGET_REJECT=m

and the iptable_filter, ip_tables, and x_tables modules were all loaded,
just like they were on the working x86 box.

User-space on the ppc32 box is YDL 4.0 with iptables-1.2.9-2.3.1.

/Mikael

Re: 2.6.16-rc1: iptables broken on ppc32?

[Harald Welte]

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

On Wed, Jan 18, 2006 at 03:09:06PM +0100, Mikael Pettersson wrote:
> When trying out kernel 2.6.16-rc1 on a ppc32 box (G4 eMac),
> the kernel refused to load my /etc/sysconfig/iptables. strace
> on /sbin/iptables-restore shows that the kernel returns EINVAL
> instead of accepting the configuration:

thanks for letting us know, you might have catched a very important bug.

We've introduced a number of changes (x_tables) that haven't received
testing on all architectures yet.

I will try to reproduce the bug on my debian ppc box here.

This is not meant as a fix, but you might try it to narrow down the
problem:  Try recompiling iptables on your own, and report back whether
that works or not.

Please Follow-up-to netfilter-devel@lists.netfilter.org

-- 
- Harald Welte <laforge@netfilter.org>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

[PATCH] x_tables: fix alignment on [at least] ppc32 (was Re: 2.6.16-rc1: iptables broken on ppc32?)

[Harald Welte]

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

Hi,

the patch below fixes the problem on ppc32. Dave: Please apply.

[NETFILTER] x_tables: Fix XT_ALIGN() macro on [at least] ppc32

To keep backwards compatibility with old iptables userspace programs,
the new XT_ALIGN macro always has to return the same value as IPT_ALIGN,
IP6T_ALIGN or ARPT_ALIGN in previous kernels.

However, in those kernels the macro was defined in dependency to the
respective layer3 specifi data structures, which we can no longer do with
x_tables.

The fix is an ugly kludge, but it has been tested to solve the problem. Yet
another reason to move away from the current {ip,ip6,arp,eb}tables like
data structures.

Signed-off-by: Harald Welte <laforge@netfilter.org>

---
commit 470faeb379560fe877b685ca69be6a7e4f0e91ed
tree 5732ecd9bcab28469805752514e5c57ba26189a1
parent 44718bbfa186d58477163418d37df173aa2dd079
author Harald Welte <laforge@netfilter.org> Fri, 20 Jan 2006 01:44:24 +0100
committer Harald Welte <laforge@netfilter.org> Fri, 20 Jan 2006 01:44:24 +0100

 include/linux/netfilter/x_tables.h |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h
index 472f048..65f9cd8 100644
--- a/include/linux/netfilter/x_tables.h
+++ b/include/linux/netfilter/x_tables.h
@@ -19,7 +19,20 @@ struct xt_get_revision
 /* For standard target */
 #define XT_RETURN (-NF_REPEAT - 1)
 
-#define XT_ALIGN(s) (((s) + (__alignof__(void *)-1)) & ~(__alignof__(void *)-1))
+/* this is a dummy structure to find out the alignment requirement for a struct
+ * containing all the fundamental data types that are used in ipt_entry, ip6t_entry
+ * and arpt_entry.  This sucks, and it is a hack.  It will be my personal pleasure
+ * to remove it -HW */
+struct _xt_align
+{
+	u_int8_t u8;
+	u_int16_t u16;
+	u_int32_t u32;
+	u_int64_t u64;
+};
+
+#define XT_ALIGN(s) (((s) + (__alignof__(struct _xt_align)-1)) 	\
+			& ~(__alignof__(struct _xt_align)-1))
 
 /* Standard return verdict, or do jump. */
 #define XT_STANDARD_TARGET ""
-- 
- Harald Welte <laforge@netfilter.org>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] x_tables: fix alignment on [at least] ppc32

[David S. Miller]

From: Harald Welte <laforge@netfilter.org>
Date: Fri, 20 Jan 2006 01:45:12 +0100

> [NETFILTER] x_tables: Fix XT_ALIGN() macro on [at least] ppc32
> 
> To keep backwards compatibility with old iptables userspace programs,
> the new XT_ALIGN macro always has to return the same value as IPT_ALIGN,
> IP6T_ALIGN or ARPT_ALIGN in previous kernels.
> 
> However, in those kernels the macro was defined in dependency to the
> respective layer3 specifi data structures, which we can no longer do with
> x_tables.
> 
> The fix is an ugly kludge, but it has been tested to solve the problem. Yet
> another reason to move away from the current {ip,ip6,arp,eb}tables like
> data structures.
> 
> Signed-off-by: Harald Welte <laforge@netfilter.org>

Harald, I'm going to modify this to just use u_int64_t as that
should be totally sufficient.

It is the largest type, and will produce the desired results without
the silly structure.

Some malloc() implementations use "long double" to figure out the
largest type size and alignment requirements any C type might have
on the machine.  But there is no reason to use that here.

Re: [PATCH] x_tables: fix alignment on [at least] ppc32

[Mikael Pettersson]

David S. Miller writes:
 > From: Harald Welte <laforge@netfilter.org>
 > Date: Fri, 20 Jan 2006 01:45:12 +0100
 > 
 > > [NETFILTER] x_tables: Fix XT_ALIGN() macro on [at least] ppc32
 > > 
 > > To keep backwards compatibility with old iptables userspace programs,
 > > the new XT_ALIGN macro always has to return the same value as IPT_ALIGN,
 > > IP6T_ALIGN or ARPT_ALIGN in previous kernels.
 > > 
 > > However, in those kernels the macro was defined in dependency to the
 > > respective layer3 specifi data structures, which we can no longer do with
 > > x_tables.
 > > 
 > > The fix is an ugly kludge, but it has been tested to solve the problem. Yet
 > > another reason to move away from the current {ip,ip6,arp,eb}tables like
 > > data structures.
 > > 
 > > Signed-off-by: Harald Welte <laforge@netfilter.org>
 > 
 > Harald, I'm going to modify this to just use u_int64_t as that
 > should be totally sufficient.

ACK. Both Harald's patch and DaveM's simplification of it
(simply s/void */u_int64_t/g in XT_ALIGN()) fix the iptables
problems on my ppc32 box.

/Mikael

Re: [PATCH] x_tables: fix alignment on [at least] ppc32

[David S. Miller]

From: Mikael Pettersson <mikpe@csd.uu.se>
Date: Fri, 20 Jan 2006 10:28:17 +0100

> ACK. Both Harald's patch and DaveM's simplification of it
> (simply s/void */u_int64_t/g in XT_ALIGN()) fix the iptables
> problems on my ppc32 box.

Thanks for testing.

Re: [PATCH] x_tables: fix alignment on [at least] ppc32

[Harald Welte]

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

On Thu, Jan 19, 2006 at 04:56:35PM -0800, David S. Miller wrote:
> > [NETFILTER] x_tables: Fix XT_ALIGN() macro on [at least] ppc32
> > [...]
> > The fix is an ugly kludge, but it has been tested to solve the problem. Yet
> > another reason to move away from the current {ip,ip6,arp,eb}tables like
> > data structures.
> > 
> > Signed-off-by: Harald Welte <laforge@netfilter.org>
> 
> Harald, I'm going to modify this to just use u_int64_t as that
> should be totally sufficient.
>
> It is the largest type, and will produce the desired results without
> the silly structure.

Sorry dave, as I just learned, it isn't.   As reported by Jiri Slaby
<xslaby@fi.muni.cz>, Linus' tree now breaks on i386 :(

Interestingly, on i386:

__alignof__(struct _xt_align)	4
__alignof__(u_int64_t)		8
__alignof__(void *)		4

whereas on ppc:

__alignof__(struct _xt_align)	8
__alignof__(u_int64_t)		8
__alignof__(void *)		4

So your assumption that __alignof__(u_int64_t) == __alignof__(struct xt_align)
doesn't hold true for all archs.

I would therefore recommend applying my unmodified patch, and hope that
it then works on all archs simultaneously.

> Some malloc() implementations use "long double" to figure out the
> largest type size and alignment requirements any C type might have
> on the machine.  But there is no reason to use that here.

Our main problem is that we have to stay compatible with old userspace
programs that had a different definition for what has now become
XT_ALIGN().  So independent what might be the best solution from an
alignment point of view, we must match what old userspace thinks.

Yes, this all sucks.  And yes, we will see a new interface this year.
Promised.

Cheers,
	Harald.

-- 
- Harald Welte <laforge@netfilter.org>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]