[IPROUTE PATCH] iproute2: act_ipt fix xtables breakage on older versions.

[IPROUTE PATCH] iproute2: act_ipt fix xtables breakage on older versions.

[Alexander Duyck]

In trying to build on a RHEL6.3 I ran into several build issues that are
addressed in this patch.

The first is that xtables_options_xfrm only has 3 options.  It appears this is
how this code was originally.  As such for the case where the version is less
than 6 I am assuming it would be correct to maintain the original setup that
only had 3 parameters being passed instead of 4.

I also ran into an issue with the define for __ALIGN_KERNEL not being present.
I believe this may be due to the fact that __ALIGN_KERNEL was moved into a
separate header from ALIGN after the uapi changes.  In order to just cover all
of the bases I have moved the main definition for the macros into
__ALIGN_KERNEL_MASK and __ALIGN_KERNEL and if ALIGN is also needed then it is
just a direct redefine to __ALIGN_KERNEL.

Cc: Hasan Chowdhury <shemonc@gmail.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
 tc/m_xt.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/tc/m_xt.c b/tc/m_xt.c
index 3edf520..e918670 100644
--- a/tc/m_xt.c
+++ b/tc/m_xt.c
@@ -38,9 +38,13 @@
 #       define XT_LIB_DIR "/lib/xtables"
 #endif
 
+#ifndef __ALIGN_KERNEL
+#define __ALIGN_KERNEL(x, a)		__ALIGN_KERNEL_MASK(x, (typeof(x))(a) - 1)
+#define __ALIGN_KERNEL_MASK(x, mask)	(((x) + (mask)) & ~(mask))
+#endif
+
 #ifndef ALIGN
-#define ALIGN(x,a)		__ALIGN_MASK(x,(typeof(x))(a)-1)
-#define __ALIGN_MASK(x,mask)	(((x)+(mask))&~(mask))
+#define ALIGN(x,a)	__ALIGN_KERNEL((x), (a))
 #endif
 
 static const char *tname = "mangle";
@@ -166,8 +170,7 @@ static int parse_ipt(struct action_util *a,int *argc_p,
 						    m->x6_options,
 						    &m->option_offset);
 #else
-			opts = xtables_merge_options(tcipt_globals.orig_opts,
-						     tcipt_globals.opts,
+			opts = xtables_merge_options(tcipt_globals.opts,
 						     m->extra_opts,
 						     &m->option_offset);
 #endif
@@ -335,8 +338,7 @@ print_ipt(struct action_util *au,FILE * f, struct rtattr *arg)
 					    m->x6_options,
 					    &m->option_offset);
 #else
-		opts = xtables_merge_options(tcipt_globals.orig_opts,
-					     tcipt_globals.opts,
+		opts = xtables_merge_options(tcipt_globals.opts,
 					     m->extra_opts,
 					     &m->option_offset);
 #endif

Re: [IPROUTE PATCH] iproute2: act_ipt fix xtables breakage on older versions.

[Stephen Hemminger]

On Thu, 25 Apr 2013 15:07:16 -0700
Alexander Duyck <alexander.h.duyck@intel.com> wrote:

> In trying to build on a RHEL6.3 I ran into several build issues that are
> addressed in this patch.
> 
> The first is that xtables_options_xfrm only has 3 options.  It appears this is
> how this code was originally.  As such for the case where the version is less
> than 6 I am assuming it would be correct to maintain the original setup that
> only had 3 parameters being passed instead of 4.
> 
> I also ran into an issue with the define for __ALIGN_KERNEL not being present.
> I believe this may be due to the fact that __ALIGN_KERNEL was moved into a
> separate header from ALIGN after the uapi changes.  In order to just cover all
> of the bases I have moved the main definition for the macros into
> __ALIGN_KERNEL_MASK and __ALIGN_KERNEL and if ALIGN is also needed then it is
> just a direct redefine to __ALIGN_KERNEL.
> 
> Cc: Hasan Chowdhury <shemonc@gmail.com>
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>


I want some feedback from Jamal and libxt netfilter experts.
This code has been fragile in the past.

Re: [IPROUTE PATCH] iproute2: act_ipt fix xtables breakage on older versions.

[Jamal Hadi Salim]

On 13-04-25 06:07 PM, Alexander Duyck wrote:
> In trying to build on a RHEL6.3 I ran into several build issues that are
> addressed in this patch.
>
> The first is that xtables_options_xfrm only has 3 options.  It appears this is
> how this code was originally.  As such for the case where the version is less
> than 6 I am assuming it would be correct to maintain the original setup that
> only had 3 parameters being passed instead of 4.
>


Getting it to compile is insufficient get it to work.
We've gone over the specific issues you are running into and infact 
patches exist for both iproute2 and the kernel. I'll see if i can get 
something out this weekend.
Hasan, you never got back to me on our last email exchange whether the
proposed changes worked out. Can you please provide me some feedback?

cheers,
jamal

Re: [IPROUTE PATCH] iproute2: act_ipt fix xtables breakage on older versions.

[Alexander Duyck]

On 04/26/2013 06:20 AM, Jamal Hadi Salim wrote:
> On 13-04-25 06:07 PM, Alexander Duyck wrote:
>> In trying to build on a RHEL6.3 I ran into several build issues that are
>> addressed in this patch.
>>
>> The first is that xtables_options_xfrm only has 3 options.  It
>> appears this is
>> how this code was originally.  As such for the case where the version
>> is less
>> than 6 I am assuming it would be correct to maintain the original
>> setup that
>> only had 3 parameters being passed instead of 4.
>>
>
>
> Getting it to compile is insufficient get it to work.
> We've gone over the specific issues you are running into and infact
> patches exist for both iproute2 and the kernel. I'll see if i can get
> something out this weekend.
> Hasan, you never got back to me on our last email exchange whether the
> proposed changes worked out. Can you please provide me some feedback?
>
> cheers,
> jamal

Getting it to compile is kind of important since it was preventing me
from completing the build for iproute2 as I had another fix I needed to
test.

If you have some test case you want me to run just let me know.  All I
did here is revert the changes that I believe were made incorrectly for
versions prior to 1.4.10 in "iproute2: act_ipt fix xtables breakage". 
It states it was fixing it for versions starting in 1.4.10 which I am
assuming is the XTABLES_VERSION_CODE >= 6 case so it should not have
modified the 3 parameter case for versions prior to 6 which is what I
corrected.

Thanks,

Alex

Re: [IPROUTE PATCH] iproute2: act_ipt fix xtables breakage on older versions.

[Jamal Hadi Salim]

Hi Alex,

On 13-04-26 11:58 AM, Alexander Duyck wrote:

> Getting it to compile is kind of important since it was preventing me
> from completing the build for iproute2 as I had another fix I needed to
> test.

So i just went back and looked. I was under the impression the original
patch from Hasan was not incorporated, it turns out it was (I just didnt
get the usual "applied" email; ands unfortunately for some reason i
showed up as the author instead of Hasan). It is the patch you are 
referring to.

Second - I did compile this on Debian squeeze which is iptables 1.4.8
based; lucky, I still have that machine around, so i just verified again
that the current git tree compiles fine there.
So this could be a fedora specific issue.
What does the generated Config file have for you?
on squeeze:
IPT_LIB_DIR:=/lib/xtables

>
> If you have some test case you want me to run just let me know.  All I
> did here is revert the changes that I believe were made incorrectly for
> versions prior to 1.4.10 in "iproute2: act_ipt fix xtables breakage".
> It states it was fixing it for versions starting in 1.4.10 which I am
> assuming is the XTABLES_VERSION_CODE >= 6 case so it should not have
> modified the 3 parameter case for versions prior to 6 which is what I
> corrected.
>

Are you sure about that? Please double check again, i see 4 parameters.
Having said that the mystery and breakages may have to do with the way
the different distros package iptables.

cheers,
jamal

Re: [IPROUTE PATCH] iproute2: act_ipt fix xtables breakage on older versions.

[Alexander Duyck]

On 04/28/2013 07:31 AM, Jamal Hadi Salim wrote:
> Hi Alex,
> 
> On 13-04-26 11:58 AM, Alexander Duyck wrote:
> 
>> Getting it to compile is kind of important since it was preventing me
>> from completing the build for iproute2 as I had another fix I needed to
>> test.
> 
> So i just went back and looked. I was under the impression the original
> patch from Hasan was not incorporated, it turns out it was (I just didnt
> get the usual "applied" email; ands unfortunately for some reason i
> showed up as the author instead of Hasan). It is the patch you are
> referring to.
> 
> Second - I did compile this on Debian squeeze which is iptables 1.4.8
> based; lucky, I still have that machine around, so i just verified again
> that the current git tree compiles fine there.
> So this could be a fedora specific issue.
> What does the generated Config file have for you?
> on squeeze:
> IPT_LIB_DIR:=/lib/xtables
> 

I just realized my patch description is incorrect.  I had called out
xtables_options_xfrm as the function I was fixing when it was actually
references to xtables_merge_options I was fixing in the patch.  From
what I can tell, all the way up to 1.4.10 xtables_merge_options only had
3 parameters in include/xtables.h.in.  It looks like things switch to
version 6 in 1.4.11 and then xtables_options_xfrm takes 4 parameters.

I will update the patch description and send out a new copy tomorrow.

Thanks,

Alex