All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexander Duyck <alexander.h.duyck@intel.com>
To: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: shemminger@vyatta.com, netdev@vger.kernel.org,
	jeffrey.t.kirsher@intel.com, Hasan Chowdhury <shemonc@gmail.com>,
	Pablo Neira Ayuso <pablo@netfilter.org>
Subject: Re: [IPROUTE PATCH v2] iproute2: act_ipt fix xtables breakage on older versions.
Date: Tue, 30 Apr 2013 09:07:36 -0700	[thread overview]
Message-ID: <517FEC48.80700@intel.com> (raw)
In-Reply-To: <517FC04E.30705@mojatatu.com>

On 04/30/2013 05:59 AM, Jamal Hadi Salim wrote:
>
> Hi Alex,
> Theres still some confusion on my part.
>
> On 13-04-29 11:50 AM, Alexander Duyck wrote:
>
>> The first is that xtables_merge_options only has 3 parameters.  It
>> appears
>> this is how this code was originally.
>
> what you are describing above is still not making sense:
>
>  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.
>>
>
> More below, lets just pick one of those:
>
>> @@ -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
>>
>
>
> Here's the original equivalent change that you are updating:
> -------
> -                       tcipt_globals.opts =
> -                           xtables_merge_options(
>  #if (XTABLES_VERSION_CODE >= 6)
> -                                                
> tcipt_globals.orig_opts,
> +               opts = xtables_options_xfrm(tcipt_globals.orig_opts,
> +                                           tcipt_globals.opts,
> +                                           m->x6_options,
> +                                           &m->option_offset);
> +#else
> +               opts = xtables_merge_options(tcipt_globals.orig_opts,
> +                                            tcipt_globals.opts,
> +                                            m->extra_opts,
> +                                            &m->option_offset);
>  #endif
> -                                                 tcipt_globals.opts,
> -                                                 m->extra_opts,
> -                                                 &m->option_offset);
> ------------------
>
> I see, originally these parameters for xtables_merge_options():
> tcipt_globals.orig_opts  <---- This is what you are trying to kill
> tcipt_globals.opts
> m->extra_opts
> &m->option_offset
>
> Which says there were 4 parameters to begin with....
> Unless you are saying we had it wrong the first time. Pablo?
>
> cheers,
> jamal

Jamal,

It was 4 parameters for (XTABLES_VERSION_CODE >= 6), for versions prior
to that it was only 3 parameters.  Notice the line that is removing
tcipt_globals.orit_opts was wrapped inside the ifdef while the rest of
the function was originally past the endif.  What the original patch did
is make it so that on 1.4.10 and prior we cannot build.

Just take a look at:
https://git.netfilter.org/iptables/tree/include/xtables.h.in?id=v1.4.10#n222

In the git tree it is obvious that xtables_merge_options only takes 3
parameters.  It is not until v1.4.11 which is XTABLES_VERSION_CODE == 6
that we see xtables_merge_options take 4 parameters.

Thanks,

Alex

  reply	other threads:[~2013-04-30 16:07 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-29 15:50 [IPROUTE PATCH v2] iproute2: act_ipt fix xtables breakage on older versions Alexander Duyck
2013-04-30 12:59 ` Jamal Hadi Salim
2013-04-30 16:07   ` Alexander Duyck [this message]
2013-05-01 13:47     ` Jamal Hadi Salim
2013-05-01 14:21       ` Jamal Hadi Salim
2013-05-01 17:18         ` Alexander Duyck
2013-05-01 14:22 ` Jamal Hadi Salim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=517FEC48.80700@intel.com \
    --to=alexander.h.duyck@intel.com \
    --cc=jeffrey.t.kirsher@intel.com \
    --cc=jhs@mojatatu.com \
    --cc=netdev@vger.kernel.org \
    --cc=pablo@netfilter.org \
    --cc=shemminger@vyatta.com \
    --cc=shemonc@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.