On Thu, Jan 04, 2018 at 09:56:08PM +1100, Duncan Roe wrote:
> On Thu, Jan 04, 2018 at 10:53:28AM +0100, Pablo Neira Ayuso wrote:
> > On Thu, Jan 04, 2018 at 09:26:40AM +1100, Duncan Roe wrote:
> > > On Wed, Jan 03, 2018 at 03:41:08PM +0100, Pablo Neira Ayuso wrote:
> > > > iptables-translate -A INPUT -m tcp -p tcp --dport 80 -m hashlimit --hashlimit-above 200kb/s --hashlimit-burst 1mb --hashlimit-mode srcip,dstport --hashlimit-name http2 --hashlimit-htable-expire 3000 -j DROP
> > > >
> > > > shows:
> > > >
> > > > nft add rule ip filter INPUT tcp dport 80 flow table http2 { tcp dport . ip saddr timeout 3s limit rate over 200 kbytes/second burst 1 mbytes burst 6 packets} counter drop
> > > >
> > > > which prints burst twice, this is not correct.
> > > >
> > > > Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
> > > > ---
> > > > extensions/libxt_hashlimit.c | 8 +++++---
> > > > 1 file changed, 5 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/extensions/libxt_hashlimit.c b/extensions/libxt_hashlimit.c
> > > > index 472d8e7f6cc2..3fa5719127db 100644
> > > > --- a/extensions/libxt_hashlimit.c
> > > > +++ b/extensions/libxt_hashlimit.c
> > > > @@ -1350,10 +1350,12 @@ static int hashlimit_mt_xlate(struct xt_xlate *xl, const char *name,
> > > >
> > > > if (cfg->mode & XT_HASHLIMIT_BYTES)
> > > > print_bytes_rate_xlate(xl, cfg);
> > > > - else
> > > > + else {
> > > > print_packets_rate_xlate(xl, cfg->avg, revision);
> > > > - if (cfg->burst != 5)
> > > > - xt_xlate_add(xl, " burst %lu packets", cfg->burst);
> > > > + if (cfg->burst != XT_HASHLIMIT_BURST)
> > > > + xt_xlate_add(xl, " burst %lu packets", cfg->burst);
> > > > +
> > > > + }
> > > > xt_xlate_add(xl, "}");
> > > >
> > > > return ret;
> > > > --
> > > > 2.11.0
> > > >
> > > This still discards a timeout of 1s (1000ms):
> > >
> > > > $ iptables-translate -A INPUT -m tcp -p tcp --dport 80 -m hashlimit --hashlimit-above 200kb/s --hashlimit-burst 1mb --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 burst 1 mbytes} counter drop
> > >
> > > This is especially incorrect, since the code deliberately inserts a default
> > > timeout of 1m if no timeout was specified with a burst:
> > >
> > > > $ iptables-translate -A INPUT -m tcp -p tcp --dport 80 -m hashlimit --hashlimit-above 200kb/s --hashlimit-burst 1mb --hashlimit-mode srcip,dstport --hashlimit-name http2 -j DROP
> > > > nft add rule ip filter INPUT tcp dport 80 flow table http2 { tcp dport . ip saddr timeout 60s limit rate over 200 kbytes/second burst 1 mbytes} counter drop
> > >
> > > The patch I suggested doesn't have that problem, because of forcing defaults to
> > > zero. Can doing that have any adverse side-effects?
> >
> > Yes. Problem is that we cannot assume that hashlimit_mt_check() is
> > called. If you compile nftables with --with-xtables, listing of rules
> > that are added via iptables-compat will be translated to nftables.
>
> OK on that. But see my comments to your latest patch. I guess it should be
> harmless to introduce a flags byte that no other code is aware of?
>
Another idea - go back to Harsha's original patch (with the global variable(s?))
but make them thread locals. Would that work?
Cheers ... Duncan.
--
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
