| Message ID | 20130425220716.6520.94862.stgit@ahduyck-cp1.jf.intel.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | stephen hemminger |
| Headers | show |
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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(-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html