* [IPROUTE PATCH] iproute2: act_ipt fix xtables breakage on older versions.
@ 2013-04-25 22:07 Alexander Duyck
2013-04-25 23:16 ` Stephen Hemminger
2013-04-26 13:20 ` Jamal Hadi Salim
0 siblings, 2 replies; 6+ messages in thread
From: Alexander Duyck @ 2013-04-25 22:07 UTC (permalink / raw)
To: shemminger; +Cc: netdev, jeffrey.t.kirsher, Jamal Hadi Salim, Hasan Chowdhury
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [IPROUTE PATCH] iproute2: act_ipt fix xtables breakage on older versions.
2013-04-25 22:07 [IPROUTE PATCH] iproute2: act_ipt fix xtables breakage on older versions Alexander Duyck
@ 2013-04-25 23:16 ` Stephen Hemminger
2013-04-26 13:20 ` Jamal Hadi Salim
1 sibling, 0 replies; 6+ messages in thread
From: Stephen Hemminger @ 2013-04-25 23:16 UTC (permalink / raw)
To: Alexander Duyck
Cc: shemminger, netdev, jeffrey.t.kirsher, Jamal Hadi Salim,
Hasan Chowdhury
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [IPROUTE PATCH] iproute2: act_ipt fix xtables breakage on older versions.
2013-04-25 22:07 [IPROUTE PATCH] iproute2: act_ipt fix xtables breakage on older versions Alexander Duyck
2013-04-25 23:16 ` Stephen Hemminger
@ 2013-04-26 13:20 ` Jamal Hadi Salim
2013-04-26 15:58 ` Alexander Duyck
1 sibling, 1 reply; 6+ messages in thread
From: Jamal Hadi Salim @ 2013-04-26 13:20 UTC (permalink / raw)
To: Alexander Duyck; +Cc: shemminger, netdev, jeffrey.t.kirsher, Hasan Chowdhury
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [IPROUTE PATCH] iproute2: act_ipt fix xtables breakage on older versions.
2013-04-26 13:20 ` Jamal Hadi Salim
@ 2013-04-26 15:58 ` Alexander Duyck
2013-04-28 14:31 ` Jamal Hadi Salim
0 siblings, 1 reply; 6+ messages in thread
From: Alexander Duyck @ 2013-04-26 15:58 UTC (permalink / raw)
To: Jamal Hadi Salim; +Cc: shemminger, netdev, jeffrey.t.kirsher, Hasan Chowdhury
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [IPROUTE PATCH] iproute2: act_ipt fix xtables breakage on older versions.
2013-04-26 15:58 ` Alexander Duyck
@ 2013-04-28 14:31 ` Jamal Hadi Salim
2013-04-28 20:53 ` Alexander Duyck
0 siblings, 1 reply; 6+ messages in thread
From: Jamal Hadi Salim @ 2013-04-28 14:31 UTC (permalink / raw)
To: Alexander Duyck; +Cc: shemminger, netdev, jeffrey.t.kirsher, Hasan Chowdhury
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [IPROUTE PATCH] iproute2: act_ipt fix xtables breakage on older versions.
2013-04-28 14:31 ` Jamal Hadi Salim
@ 2013-04-28 20:53 ` Alexander Duyck
0 siblings, 0 replies; 6+ messages in thread
From: Alexander Duyck @ 2013-04-28 20:53 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Alexander Duyck, shemminger, netdev, jeffrey.t.kirsher,
Hasan Chowdhury
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-04-28 20:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-25 22:07 [IPROUTE PATCH] iproute2: act_ipt fix xtables breakage on older versions Alexander Duyck
2013-04-25 23:16 ` Stephen Hemminger
2013-04-26 13:20 ` Jamal Hadi Salim
2013-04-26 15:58 ` Alexander Duyck
2013-04-28 14:31 ` Jamal Hadi Salim
2013-04-28 20:53 ` Alexander Duyck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).