* [PATCH v2] extensions: libxt_hashlimit: Do not print default timeout and burst
@ 2017-12-28 7:28 Harsha Sharma
2017-12-28 10:52 ` Pablo Neira Ayuso
0 siblings, 1 reply; 13+ messages in thread
From: Harsha Sharma @ 2017-12-28 7:28 UTC (permalink / raw)
To: pablo, harshasharmaiitr; +Cc: netfilter-devel
Do not print timeout and burst in case default values are used.
For e.g.
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 limit rate over 200/second } counter drop
Signed-off-by: Harsha Sharma <harshasharmaiitr@gmail.com>
---
Changes in v2:
-Simple comparison for default values
extensions/libxt_hashlimit.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/extensions/libxt_hashlimit.c b/extensions/libxt_hashlimit.c
index ffe342a7..472d8e7f 100644
--- a/extensions/libxt_hashlimit.c
+++ b/extensions/libxt_hashlimit.c
@@ -7,7 +7,7 @@
* Based on ipt_limit.c by
* Jérôme de Vivie <devivie@info.enserb.u-bordeaux.fr>
* Hervé Eychenne <rv@wallfire.org>
- *
+ *
* Error corections by nmalykh@bilim.com (22.01.2005)
*/
#define _BSD_SOURCE 1
@@ -1209,7 +1209,7 @@ static const struct rates rates_xlate[] = {
{ "second", XT_HASHLIMIT_SCALE_v2 } };
static void print_packets_rate_xlate(struct xt_xlate *xl, uint64_t avg,
- uint64_t burst, int revision)
+ int revision)
{
unsigned int i;
const struct rates *_rates = (revision == 1) ?
@@ -1220,8 +1220,8 @@ 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);
}
static void print_bytes_rate_xlate(struct xt_xlate *xl,
@@ -1341,7 +1341,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 != 1000)
+ 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");
@@ -1349,8 +1351,9 @@ 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
- print_packets_rate_xlate(xl, cfg->avg, cfg->burst, revision);
-
+ print_packets_rate_xlate(xl, cfg->avg, revision);
+ if (cfg->burst != 5)
+ xt_xlate_add(xl, " burst %lu packets", cfg->burst);
xt_xlate_add(xl, "}");
return ret;
@@ -1365,7 +1368,8 @@ static int hashlimit_xlate(struct xt_xlate *xl,
xt_xlate_add(xl, "flow table %s {", info->name);
ret = hashlimit_mode_xlate(xl, info->cfg.mode, NFPROTO_IPV4, 32, 32);
xt_xlate_add(xl, " timeout %us limit rate", info->cfg.expire / 1000);
- print_packets_rate_xlate(xl, info->cfg.avg, info->cfg.burst, 1);
+ print_packets_rate_xlate(xl, info->cfg.avg, 1);
+ xt_xlate_add(xl, " burst %lu packets", info->cfg.burst);
xt_xlate_add(xl, "}");
return ret;
--
2.11.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2] extensions: libxt_hashlimit: Do not print default timeout and burst
2017-12-28 7:28 [PATCH v2] extensions: libxt_hashlimit: Do not print default timeout and burst Harsha Sharma
@ 2017-12-28 10:52 ` Pablo Neira Ayuso
2017-12-30 22:51 ` Duncan Roe
0 siblings, 1 reply; 13+ messages in thread
From: Pablo Neira Ayuso @ 2017-12-28 10:52 UTC (permalink / raw)
To: Harsha Sharma; +Cc: netfilter-devel, Duncan Roe
Cc'ing Duncan.
On Thu, Dec 28, 2017 at 12:58:33PM +0530, Harsha Sharma wrote:
> Do not print timeout and burst in case default values are used.
> For e.g.
> 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 limit rate over 200/second } counter drop
This is what I was asking for.
Applied, thanks Harsha.
@Duncan: I think you can update the wiki again after this, given that
now those values should not be printed.
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2] extensions: libxt_hashlimit: Do not print default timeout and burst
2017-12-28 10:52 ` Pablo Neira Ayuso
@ 2017-12-30 22:51 ` Duncan Roe
2017-12-30 23:16 ` Pablo Neira Ayuso
0 siblings, 1 reply; 13+ messages in thread
From: Duncan Roe @ 2017-12-30 22:51 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: Harsha Sharma, netfilter-devel
On Thu, Dec 28, 2017 at 11:52:36AM +0100, Pablo Neira Ayuso wrote:
> Cc'ing Duncan.
No need - I'm on netfilter-devel ;)
>
> On Thu, Dec 28, 2017 at 12:58:33PM +0530, Harsha Sharma wrote:
> > Do not print timeout and burst in case default values are used.
> > For e.g.
> > 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 limit rate over 200/second } counter drop
>
> This is what I was asking for.
>
> Applied, thanks Harsha.
>
> @Duncan: I think you can update the wiki again after this, given that
> now those values should not be printed.
I tested after applying the patch and now the second example is broken:
> 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
> 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
The traling "burst 6 packets" should not be there.
I propose to leave the wiki until this is fixed.
Cheers ... Duncan.
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2] extensions: libxt_hashlimit: Do not print default timeout and burst
2017-12-30 22:51 ` Duncan Roe
@ 2017-12-30 23:16 ` Pablo Neira Ayuso
2018-01-02 22:47 ` Duncan Roe
0 siblings, 1 reply; 13+ messages in thread
From: Pablo Neira Ayuso @ 2017-12-30 23:16 UTC (permalink / raw)
To: Harsha Sharma, netfilter-devel
Hi Harsha,
On Sun, Dec 31, 2017 at 09:51:10AM +1100, Duncan Roe wrote:
> On Thu, Dec 28, 2017 at 11:52:36AM +0100, Pablo Neira Ayuso wrote:
> > Cc'ing Duncan.
>
> No need - I'm on netfilter-devel ;)
> >
> > On Thu, Dec 28, 2017 at 12:58:33PM +0530, Harsha Sharma wrote:
> > > Do not print timeout and burst in case default values are used.
> > > For e.g.
> > > 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 limit rate over 200/second } counter drop
> >
> > This is what I was asking for.
> >
> > Applied, thanks Harsha.
> >
> > @Duncan: I think you can update the wiki again after this, given that
> > now those values should not be printed.
>
> I tested after applying the patch and now the second example is broken:
>
> > 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
> > 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
>
> The traling "burst 6 packets" should not be there.
Please, send a new patch to fix this as Duncan spots.
Thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2] extensions: libxt_hashlimit: Do not print default timeout and burst
2017-12-30 23:16 ` Pablo Neira Ayuso
@ 2018-01-02 22:47 ` Duncan Roe
2018-01-03 13:53 ` Pablo Neira Ayuso
2018-01-15 1:45 ` Duncan Roe
0 siblings, 2 replies; 13+ messages in thread
From: Duncan Roe @ 2018-01-02 22:47 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: Harsha Sharma, netfilter-devel
Hi Pablo,
On Sun, Dec 31, 2017 at 12:16:30AM +0100, Pablo Neira Ayuso wrote:
> Hi Harsha,
>
> On Sun, Dec 31, 2017 at 09:51:10AM +1100, Duncan Roe wrote:
> > On Thu, Dec 28, 2017 at 11:52:36AM +0100, Pablo Neira Ayuso wrote:
> > > Cc'ing Duncan.
> >
> > No need - I'm on netfilter-devel ;)
> > >
> > > On Thu, Dec 28, 2017 at 12:58:33PM +0530, Harsha Sharma wrote:
> > > > Do not print timeout and burst in case default values are used.
> > > > For e.g.
> > > > 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 limit rate over 200/second } counter drop
> > >
> > > This is what I was asking for.
> > >
> > > Applied, thanks Harsha.
> > >
> > > @Duncan: I think you can update the wiki again after this, given that
> > > now those values should not be printed.
> >
> > I tested after applying the patch and now the second example is broken:
> >
> > > 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
> > > 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
> >
> > The traling "burst 6 packets" should not be there.
>
It gets worse. Substitute 1000 for 3000 in the above:
> $ 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 burst 6 packets} counter drop
Because I happened to specify the arbitrary default in my iptables command, it
did not propogate to nft at all.
I am working on a patch,
Cheers ... Duncan.
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2] extensions: libxt_hashlimit: Do not print default timeout and burst
2018-01-02 22:47 ` Duncan Roe
@ 2018-01-03 13:53 ` Pablo Neira Ayuso
2018-01-15 1:45 ` Duncan Roe
1 sibling, 0 replies; 13+ messages in thread
From: Pablo Neira Ayuso @ 2018-01-03 13:53 UTC (permalink / raw)
To: Harsha Sharma, netfilter-devel
[-- Attachment #1: Type: text/plain, Size: 2172 bytes --]
On Wed, Jan 03, 2018 at 09:47:46AM +1100, Duncan Roe wrote:
> Hi Pablo,
>
> On Sun, Dec 31, 2017 at 12:16:30AM +0100, Pablo Neira Ayuso wrote:
> > Hi Harsha,
> >
> > On Sun, Dec 31, 2017 at 09:51:10AM +1100, Duncan Roe wrote:
> > > On Thu, Dec 28, 2017 at 11:52:36AM +0100, Pablo Neira Ayuso wrote:
> > > > Cc'ing Duncan.
> > >
> > > No need - I'm on netfilter-devel ;)
> > > >
> > > > On Thu, Dec 28, 2017 at 12:58:33PM +0530, Harsha Sharma wrote:
> > > > > Do not print timeout and burst in case default values are used.
> > > > > For e.g.
> > > > > 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 limit rate over 200/second } counter drop
> > > >
> > > > This is what I was asking for.
> > > >
> > > > Applied, thanks Harsha.
> > > >
> > > > @Duncan: I think you can update the wiki again after this, given that
> > > > now those values should not be printed.
> > >
> > > I tested after applying the patch and now the second example is broken:
> > >
> > > > 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
> > > > 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
> > >
> > > The traling "burst 6 packets" should not be there.
> >
> It gets worse. Substitute 1000 for 3000 in the above:
>
> > $ 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 burst 6 packets} counter drop
OK, so the problem is the extra "burst 6 packets".
This patch should fix this.
[-- Attachment #2: x.patch --]
[-- Type: text/x-diff, Size: 643 bytes --]
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;
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2] extensions: libxt_hashlimit: Do not print default timeout and burst
2018-01-02 22:47 ` Duncan Roe
2018-01-03 13:53 ` Pablo Neira Ayuso
@ 2018-01-15 1:45 ` Duncan Roe
2018-01-16 1:23 ` Pablo Neira Ayuso
[not found] ` <20180116011537.b4xm2mxlabn5tsfl@salvia>
1 sibling, 2 replies; 13+ messages in thread
From: Duncan Roe @ 2018-01-15 1:45 UTC (permalink / raw)
To: Pablo Neira Ayuso, netfilter-devel; +Cc: Harsha Sharma
On Wed, Jan 03, 2018 at 09:47:46AM +1100, Duncan Roe wrote:
> On Sun, Dec 31, 2017 at 12:16:30AM +0100, Pablo Neira Ayuso wrote:
...
> > > > @Duncan: I think you can update the wiki again after this, given that
> > > > now those values should not be printed.
> > >
The examples now work as intended, so I updated the wiki. (INPUT is now being
translated to input btw).
It is still the case that if the iptables command specifies default burst or
timeout then nothing is output for these, including the yetch case of specifying
a limit (with or w/out a burst) and getting no timeout.
I plan to look into that in February but have been rather busy getting ready for
LCA2018 where I'm giving a talk (on nftables).
Cheers ... Duncan.
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2] extensions: libxt_hashlimit: Do not print default timeout and burst
2018-01-15 1:45 ` Duncan Roe
@ 2018-01-16 1:23 ` Pablo Neira Ayuso
[not found] ` <20180116011537.b4xm2mxlabn5tsfl@salvia>
1 sibling, 0 replies; 13+ messages in thread
From: Pablo Neira Ayuso @ 2018-01-16 1:23 UTC (permalink / raw)
To: netfilter-devel, Harsha Sharma
On Mon, Jan 15, 2018 at 12:45:32PM +1100, Duncan Roe wrote:
> On Wed, Jan 03, 2018 at 09:47:46AM +1100, Duncan Roe wrote:
> > On Sun, Dec 31, 2017 at 12:16:30AM +0100, Pablo Neira Ayuso wrote:
> ...
> > > > > @Duncan: I think you can update the wiki again after this, given that
> > > > > now those values should not be printed.
> > > >
> The examples now work as intended, so I updated the wiki. (INPUT is now being
> translated to input btw).
>
> It is still the case that if the iptables command specifies default burst or
> timeout then nothing is output for these, including the yetch case of specifying
> a limit (with or w/out a burst) and getting no timeout.
I think we should always print the timeout. This timeout depends on
umult which depends on the specified rate, inconditionally.
Burst, I think it's fine if we just provide a shorter translation
default value is specified.
Well, now the translation is correct as we don't print burst twice.
Anyway, if the translation is semantically correct and it works fine,
then this is fair good enough.
We're just provide a quick translation to ease migration, users will
have to look into specific nftables details sooner or later I guess.
Thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread[parent not found: <20180116011537.b4xm2mxlabn5tsfl@salvia>]
end of thread, other threads:[~2018-01-22 4:13 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-28 7:28 [PATCH v2] extensions: libxt_hashlimit: Do not print default timeout and burst Harsha Sharma
2017-12-28 10:52 ` Pablo Neira Ayuso
2017-12-30 22:51 ` Duncan Roe
2017-12-30 23:16 ` Pablo Neira Ayuso
2018-01-02 22:47 ` Duncan Roe
2018-01-03 13:53 ` Pablo Neira Ayuso
2018-01-15 1:45 ` Duncan Roe
2018-01-16 1:23 ` Pablo Neira Ayuso
[not found] ` <20180116011537.b4xm2mxlabn5tsfl@salvia>
[not found] ` <20180116124143.5s6exg3467ozmobb@salvia>
[not found] ` <20180116204554.GA27044@dimstar.local.net>
[not found] ` <20180116215217.GA27232@dimstar.local.net>
[not found] ` <20180116223930.ftuagn4vtkvd2nka@salvia>
[not found] ` <20180119014815.GA4882@dimstar.local.net>
[not found] ` <20180119022722.duu5l3esazkox43z@salvia>
[not found] ` <20180119022757.z5ir6zcytaabqntc@salvia>
2018-01-20 6:11 ` Duncan Roe
2018-01-20 9:21 ` Pablo Neira Ayuso
2018-01-20 12:47 ` Pablo Neira Ayuso
2018-01-20 13:35 ` Duncan Roe
2018-01-20 13:42 ` Duncan Roe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).