extensions: hashlimit: always print timeout on translations
Pablo Neira Ayuso
maintainerPATCH 1/1 extensions: hashlimit: always print timeout on translations
Meters need an explicit timeout that we cannot skip, otherwise entries remain in the set forever.
This fixes the following translation:
$ 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
that was skipping the timeout option:
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
--- extensions/libxt_hashlimit.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
extensions/libxt_hashlimit.c +1 -2
@@ -1341,8 +1341,7 @@ 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); -if (cfg->expire !=1000 ) -xt_xlate_add (xl," timeout %us" , cfg->expire /1000 ); +xt_xlate_add (xl," timeout %us" , cfg->expire /1000 );xt_xlate_add (xl," limit rate" );if (cfg->mode &XT_HASHLIMIT_INVERT )
Signature · 1 line
2.11.0
Quoted 37 lines · Pablo Neira Ayuso
Meters need an explicit timeout that we cannot skip, otherwise entries remain in the set forever.
This fixes the following translation:
$ 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 DROPthat was skipping the timeout option:
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
Reported-by Duncan Roe <[email protected]>Signed-off-by Pablo Neira Ayuso <[email protected]>--- extensions/libxt_hashlimit.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)extensions/libxt_hashlimit.c +1
-3@@ -1341,8 +1341,7 @@ 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); -if (cfg->expire !=1000 ) -xt_xlate_add (xl," timeout %us" , cfg->expire /1000 ); +xt_xlate_add (xl," timeout %us" , cfg->expire /1000 );xt_xlate_add (xl," limit rate" );if (cfg->mode &XT_HASHLIMIT_INVERT ) --
Quoted 2 lines
2.11.0
Hey Pablo,
After applying this latest patch:
Quoted 2 lines
$ iptables-translate -A INPUT -m tcp -p tcp --dport 80 -m hashlimit --hashlimit-above 200/sec --hashlimit-mode srcip,dstport --hashlimit-name http1 -j DROP
nft add rule ip filter INPUT tcp dport 80 flow table http1 { tcp dport . ip saddr timeout 1s limit rate over 200/second } counter dropThe spurious "timeout 1s" has re-appeared.
Sorry but ignoring default values is just a Bad Idea (tm).
You can either force a recognisable nonsense default (like 0, in the patch below), or use flags in cfg (would need an extra __u8 in struct hashlimit_cfg3 to hold them).
This is the patch I submitted the other night (my time), but diffed against commit 27de281d8aca84e3c841b3ae72a17616b1382ac4 (git diff -w). This is a simpler diff than against HEAD:
Quoted 44 lines
extensions/libxt_hashlimit.c +6
-5@@ -759,15 +759,15 @@ static void hashlimit_mt_check(struct xt_fcheck_call *cb) {const struct hashlimit_mt_udata *udata = cb->udata;struct xt_hashlimit_mtinfo3 *info = cb->data; +uint32_t burst =0 ;if (!(cb->xflags & (F_UPTO |F_ABOVE )))xtables_error (PARAMETER_PROBLEM ,"You have to specify --hashlimit" );if (!(cb->xflags &F_HTABLE_EXPIRE )) - info->cfg.expire = udata->mult *1000 ;/* from s to msec */ + info->cfg.expire =0 ;if (info->cfg.mode &XT_HASHLIMIT_BYTES ) { -uint32_t burst =0 ;if (cb->xflags &F_BURST ) {if (info->cfg.burst <cost_to_bytes (info->cfg.avg)) xtables_error(PARAMETER_PROBLEM,@@ -780,9 +780,9 @@ static void hashlimit_mt_check(struct xt_fcheck_call *cb)if (!(cb->xflags &F_HTABLE_EXPIRE )) info->cfg.expire =XT_HASHLIMIT_BYTE_EXPIRE_BURST *1000 ; } - info->cfg.burst = burst; }else if (info->cfg.burst >XT_HASHLIMIT_BURST_MAX )burst_error (); + info->cfg.burst = burst;if (cb->xflags &F_RATEMATCH ) {if (!(info->cfg.mode &XT_HASHLIMIT_BYTES ))@@ -1220,8 +1220,9 @@ static void print_packets_rate_xlate(struct xt_xlate *xl, uint64_t avg, _rates[i].mult / avg < _rates[i].mult % avg)break ; -xt_xlate_add (xl," %llu/%s burst %lu packets" , - _rates[i-1 ].mult / avg, _rates[i-1 ].name, burst); +xt_xlate_add (xl," %llu/%s" , _rates[i-1 ].mult / avg, _rates[i-1 ].name); +if (burst) +xt_xlate_add (xl," burst %lu packets" , burst); } static void print_bytes_rate_xlate(struct xt_xlate *xl,
extensions/libxt_hashlimit.c static int hashlimit_mt_xlate(struct xt_xlate *xl, const char *name, · line 1350
Quoted 10 lines
extensions/libxt_hashlimit.c +3
-1@@ -1341,7 +1342,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 (cfg->expire) +xt_xlate_add (xl," timeout %us" , cfg->expire /1000 ); +xt_xlate_add (xl," limit rate" );if (cfg->mode &XT_HASHLIMIT_INVERT )xt_xlate_add (xl," over" );
If you would rather leave default values as before, I can re-work it to use flags instead.
Cheers ... Duncan.