Message ID | 1465388607-26771-1-git-send-email-zlpnobody@163.com |
---|---|
State | Not Applicable |
Delegated to: | Pablo Neira |
Headers | show |
On Wed, Jun 08, 2016 at 08:23:27PM +0800, Liping Zhang wrote: > From: Liping Zhang <liping.zhang@spreadtrum.com> > > Instead, we can convert invert flag and ensure it is 1 or 0. > > Signed-off-by: Liping Zhang <liping.zhang@spreadtrum.com> > --- > net/netfilter/xt_cpu.c | 12 +----------- > 1 file changed, 1 insertion(+), 11 deletions(-) > > diff --git a/net/netfilter/xt_cpu.c b/net/netfilter/xt_cpu.c > index c7a2e54..ca1eaaf 100644 > --- a/net/netfilter/xt_cpu.c > +++ b/net/netfilter/xt_cpu.c > @@ -25,27 +25,17 @@ MODULE_DESCRIPTION("Xtables: CPU match"); > MODULE_ALIAS("ipt_cpu"); > MODULE_ALIAS("ip6t_cpu"); > > -static int cpu_mt_check(const struct xt_mtchk_param *par) > -{ > - const struct xt_cpu_info *info = par->matchinfo; > - > - if (info->invert & ~1) > - return -EINVAL; > - return 0; > -} This trick is there so we can convert info->invert to info->flags in the future without a new revision (given the binary interface did not change). I'm not convinced there is much of benefit from getting rid of this little extra _check() code that runs from the control plane path. > static bool cpu_mt(const struct sk_buff *skb, struct xt_action_param *par) > { > const struct xt_cpu_info *info = par->matchinfo; > > - return (info->cpu == smp_processor_id()) ^ info->invert; > + return (info->cpu == smp_processor_id()) ^ !!info->invert; > } > > static struct xt_match cpu_mt_reg __read_mostly = { > .name = "cpu", > .revision = 0, > .family = NFPROTO_UNSPEC, > - .checkentry = cpu_mt_check, > .match = cpu_mt, > .matchsize = sizeof(struct xt_cpu_info), > .me = THIS_MODULE, > -- > 2.5.5 > > -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Pablo, 2016-06-23 19:11 GMT+08:00 Pablo Neira Ayuso <pablo@netfilter.org>: >> -static int cpu_mt_check(const struct xt_mtchk_param *par) >> -{ >> - const struct xt_cpu_info *info = par->matchinfo; >> - >> - if (info->invert & ~1) >> - return -EINVAL; >> - return 0; >> -} > > This trick is there so we can convert info->invert to info->flags in > the future without a new revision (given the binary interface did not > change). I'm not convinced there is much of benefit from getting rid > of this little extra _check() code that runs from the control plane > path. > Thanks for pointing this out. At my first glace, I think this _check is tricky and a little ugly, so I try to remove it and send this patch. As you said, if we add new flags in the future, for example, we support a new flag like this "iptables -A INPUT -m cpu --cpu 0 --flagXXX". When the user use the new iptables utility but the kernel is old, currently kernel will reject this request, because we don't recognize the "flagXXX". But apply my patch, kernel will just ignore this unknown flag, this will confuse the user. And change a new revision seems unworthy. So I'd rather not apply this pacth. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" 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/net/netfilter/xt_cpu.c b/net/netfilter/xt_cpu.c index c7a2e54..ca1eaaf 100644 --- a/net/netfilter/xt_cpu.c +++ b/net/netfilter/xt_cpu.c @@ -25,27 +25,17 @@ MODULE_DESCRIPTION("Xtables: CPU match"); MODULE_ALIAS("ipt_cpu"); MODULE_ALIAS("ip6t_cpu"); -static int cpu_mt_check(const struct xt_mtchk_param *par) -{ - const struct xt_cpu_info *info = par->matchinfo; - - if (info->invert & ~1) - return -EINVAL; - return 0; -} - static bool cpu_mt(const struct sk_buff *skb, struct xt_action_param *par) { const struct xt_cpu_info *info = par->matchinfo; - return (info->cpu == smp_processor_id()) ^ info->invert; + return (info->cpu == smp_processor_id()) ^ !!info->invert; } static struct xt_match cpu_mt_reg __read_mostly = { .name = "cpu", .revision = 0, .family = NFPROTO_UNSPEC, - .checkentry = cpu_mt_check, .match = cpu_mt, .matchsize = sizeof(struct xt_cpu_info), .me = THIS_MODULE,