Re: [NETFILTER][PATCH] Re: Question about the hashlimit network mask patch

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Mar 26, 2008 at 9:44 AM, Jan Engelhardt <jengelh@xxxxxxxxxxxxxxx> wrote:
>
> On Tuesday 2008-03-25 15:04, Iavor Stoev wrote:
> >
> > xtables-1.5.3 release works flawlessly but I found a strange behavior with
> > srcmask option.
> >
> > If you want to use /32 as a mask it doesn't work, however /31 works fine:
> >
> > --- xt_hashlimit.c.orig 2008-03-25 08:59:10.000000000 -0400
> > +++ xt_hashlimit.c      2008-03-25 08:59:26.000000000 -0400
> > @@ -466,7 +466,7 @@
> >
> > static inline __be32 maskl(__be32 a, unsigned int l)
> > {
> > -       return htonl(ntohl(a) & ~(~(u_int32_t)0 >> l));
> > +       return htonl(ntohl(a) & ~(~(u_int32_t)0 >> (l-1)));
> > }
> >
>
> I hate this. It's just stupid that  x>>32  is not 0, but, surprisingly, x.
>
> This crap is _so_ common that the networking code is *sprinkled* with
> x==32 exceptions and it happens we still fall into the trap.
>
> So this patch is needed ... but I had submitted one that entirely got
> rid of such pitfalls anyway ("Deploy a prefix len...") but that was not
> merged unfrotunately.
>
> ====
> [NETFILTER]: xt_hashlimit: add workaround for >>32 case
>
> Hardware surprisingly does nothing when a 32-bit right-shift is
> to be done. Worse yet, compilers do not even work around it.
>
> Signed-off-by: Jan Engelhardt <jengelh@xxxxxxxxxxxxxxx>
>
> diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
> index 5418ce5..1072174 100644
> --- a/net/netfilter/xt_hashlimit.c
> +++ b/net/netfilter/xt_hashlimit.c
> @@ -466,7 +466,10 @@ static inline void rateinfo_recalc(struct dsthash_ent *dh, unsigned long now)
>
>  static inline __be32 maskl(__be32 a, unsigned int l)
>  {
> -       return htonl(ntohl(a) & ~(~(u_int32_t)0 >> l));
> +       if (l == 32)
> +               return a;
> +       else
> +               return htonl(ntohl(a) & ~(~(u_int32_t)0 >> l));
>  }
>
>  #if defined(CONFIG_IP6_NF_IPTABLES) || defined(CONFIG_IP6_NF_IPTABLES_MODULE)

Hi Jan,

As far as i understand the maskl() function is called on every match.
wouldn't it be more appropriate(and faster) if all these checks and
bitshifts are done
in userspace by the iptables utility, so the kernel sees only the
"prepared" mask ready for AND-ing.
Of course this would require some mask_show() function in iptables to
convert it to bit count,
when we want to show the rules.

Regards,
Niki
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux