Re: [PATCH] extensions: libxt_hashlimit: Do not print default timeout and burst

[Duncan Roe <duncan_roe@optusnet.com.au>]

On Tue, Dec 19, 2017 at 08:20:31PM +0530, Harsha Sharma wrote:
> On Tue, Dec 19, 2017 at 7:31 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Tue, Dec 19, 2017 at 05:57:16PM +0530, Harsha Sharma wrote:
> >> @@ -1340,7 +1345,9 @@ static int hashlimit_mt_xlate(struct xt_xlate *xl, const char *name,
> >>       xt_xlate_add(xl, "flow table %s {", name);
> >>       ret = hashlimit_mode_xlate(xl, cfg->mode, family,
> >>                                  cfg->srcmask, cfg->dstmask);
> >> -     xt_xlate_add(xl, " timeout %us limit rate", cfg->expire / 1000);
> >> +     if (!XT_HASHLIMIT_BYTE_EXPIRE_DEFAULT)
> >> +             xt_xlate_add(xl, " timeout %us", cfg->expire / 1000);
> >
> > Better print cfg->expire if only if the default timeout is set.
> >
> >         if (cfg->expire != XT_HASHLIMIT_XLATE_DEFAULT_TIMEOUT)
> >                 ...
> >
> > Same thing for burst. Thanks.
>
> This would result into not printing the timeout 1s (default value)
> even if user specifies it.
> For e.g.
> iptables-translate -A INPUT -m tcp -p tcp --dport 80 -m hashlimit
> --hashlimit-above 200kb/s --hashlimit-mode srcip,dstport
> --hashlimit-name http2 --hashlimit-htable-expire 1000 -j DROP
> nft add rule ip filter INPUT tcp dport 80 flow table http2 { tcp dport
> . ip saddr limit rate over 200 kbytes/second} counter drop
>
> and the expected output is
> nft add rule ip filter INPUT tcp dport 80 flow table http2 { tcp dport
> . ip saddr timeout 1s limit rate over 200 kbytes/second} counter drop
> (This patch allows this.)
> I hope this makes sense and same for burst.
> Thanks a lot for the review.
>
Have to say I agree with Harsha on this. You need a logical flag to say whether
a timeout was encountered.

Once this goes through, I'll fix the wiki,

Cheers ... Duncan.