[iptables] extensions: Add tests and description for xt_quota module

Message ID 1539126880-10072-1-git-send-email-chenbofeng.kernel@gmail.com
State Deferred
Delegated to: Pablo Neira
Headers show
Series
  • [iptables] extensions: Add tests and description for xt_quota module
Related show

Commit Message

Chenbo Feng Oct. 9, 2018, 11:14 p.m.
From: Chenbo Feng <fengc@google.com>

Introduces some iptables tests for the new --remain option in xt_quota
module. Add a breif description for how to use the --remain option in
the iptables-extension man page.

Signed-off-by: Chenbo Feng <fengc@google.com>
---
 extensions/libxt_quota.man |  4 ++++
 extensions/libxt_quota.t   | 11 +++++++++++
 2 files changed, 15 insertions(+)

Comments

Maciej ┼╗enczykowski Oct. 9, 2018, 11:47 p.m. | #1
Ah, yes, the (2**64 - 1) + 1 problem.

The fact max allowed remaining is (2**64 - 2) is perhaps surprising...
should we clamp? or warn?

userspace has:
  if (cb->entry->id == O_REMAIN)  info->remain++;
should this error out in userspace if we end up at zero?

+-m quota --quota 18446744073709551615 --remain 18446744073709551614;;FAIL

this one really should also pass...

kernel has:
  if (atomic64_read(&q->counter) > q->quota + 1)
this should probably be:
  if (atomic64_read(&q->counter) && atomic64_read(&q->counter) - 1 > q->quota)

Also I think there's something ugly with
 -m quota --quota 18446744073709551614
vs
 -m quota --quota 18446744073709551615

and thus possibly:
  if (current_count <= skb->len) {
should actually be
  if (current_count && current_count <= skb->len) {

Maybe all of this would actually be easier if we were counting bytes
used instead of bytes remaining.
Pablo Neira Ayuso Oct. 10, 2018, 10:34 a.m. | #2
On Tue, Oct 09, 2018 at 04:14:40PM -0700, Chenbo Feng wrote:
> From: Chenbo Feng <fengc@google.com>
> 
> Introduces some iptables tests for the new --remain option in xt_quota
> module. Add a breif description for how to use the --remain option in
> the iptables-extension man page.

Applied, thanks Chenbo.
Pablo Neira Ayuso Oct. 10, 2018, 10:43 a.m. | #3
On Tue, Oct 09, 2018 at 04:47:20PM -0700, Maciej ┼╗enczykowski wrote:
> Ah, yes, the (2**64 - 1) + 1 problem.
> 
> The fact max allowed remaining is (2**64 - 2) is perhaps surprising...
> should we clamp? or warn?
> 
> userspace has:
>   if (cb->entry->id == O_REMAIN)  info->remain++;
> should this error out in userspace if we end up at zero?
> 
> +-m quota --quota 18446744073709551615 --remain 18446744073709551614;;FAIL
> 
> this one really should also pass...

:-)

> kernel has:
>   if (atomic64_read(&q->counter) > q->quota + 1)
> this should probably be:
>   if (atomic64_read(&q->counter) && atomic64_read(&q->counter) - 1 > q->quota)
> 
> Also I think there's something ugly with
>  -m quota --quota 18446744073709551614
> vs
>  -m quota --quota 18446744073709551615
> 
> and thus possibly:
>   if (current_count <= skb->len) {
> should actually be
>   if (current_count && current_count <= skb->len) {
> 
> Maybe all of this would actually be easier if we were counting bytes
> used instead of bytes remaining.

I think so. This is still net-next, so noone is using it yet apart
from developers? Probably we can still change this to become
--consumed rather than --remain. I would take patches for nf-next if
you follow that path, no problem.

Thanks.

Patch

diff --git a/extensions/libxt_quota.man b/extensions/libxt_quota.man
index fbecf37..388348f 100644
--- a/extensions/libxt_quota.man
+++ b/extensions/libxt_quota.man
@@ -5,3 +5,7 @@  byte counter reaches zero).
 .TP
 [\fB!\fP] \fB\-\-quota\fP \fIbytes\fP
 The quota in bytes.
+.TP
+\fB\-\-remain\fP \fIbytes\fP
+The remaining quota in bytes. If not specified, the remaining quota is equal
+to the value specified in \fB\-\-quota\fP.
diff --git a/extensions/libxt_quota.t b/extensions/libxt_quota.t
index c568427..8768e61 100644
--- a/extensions/libxt_quota.t
+++ b/extensions/libxt_quota.t
@@ -5,3 +5,14 @@ 
 -m quota ! --quota 18446744073709551615;=;OK
 -m quota --quota 18446744073709551616;;FAIL
 -m quota;;FAIL
+-m quota --quota 0 --remain 0;=;OK
+-m quota --quota 0 --remain 1;;FAIL
+-m quota --quota 1000 --remain 0;=;OK
+-m quota ! --quota 1000 --remain 0;=;OK
+-m quota --quota 1000 --remain 1000;=;OK
+-m quota ! --quota 1000 --remain 1000;=;OK
+-m quota --quota 1000 --remain 1001;;FAIL
+-m quota --quota 18446744073709551614 --remain 18446744073709551614;=;OK
+-m quota --quota 18446744073709551615 --remain 18446744073709551614;;FAIL
+-m quota --quota 18446744073709551616 --remain 18446744073709551615;;FAIL
+-m quota --quota 18446744073709551615 --remain 18446744073709551616;;FAIL