[nft PATCH v2 0/3] parser: refactor and extend limit rate rules

[nft PATCH v2 0/3] parser: refactor and extend limit rate rules

[Jeremy Sowden]

The first two patches introduce new rules to deduplicate the code for
parsing `limit rate` expressions and make it easier to extend the
syntax.

The third patch extends the syntax to handle expressions like `limit
rate 1 mbytes / second`, which are not currently supported.

Changes since v1:

 * add patches 1 & 2 in order to simplify the new rule added in patch 3.

Jeremy Sowden (3):
  parser: add new `limit_bytes` rule
  parser: add `limit_rate_pkts` and `limit_rate_bytes` rules
  parser: extend limit syntax

 include/datatype.h           |   4 +
 src/parser_bison.y           | 141 ++++++++++++++++++-----------------
 tests/py/any/limit.t         |   5 ++
 tests/py/any/limit.t.json    |  39 ++++++++++
 tests/py/any/limit.t.payload |  13 ++++
 5 files changed, 134 insertions(+), 68 deletions(-)

-- 
2.33.0

[nft PATCH v2 1/3] parser: add new `limit_bytes` rule

[Jeremy Sowden]

Refactor the `N byte-unit` expression out of the `limit_bytes_burst`
rule into a separate `limit_bytes` rule.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 src/parser_bison.y | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/src/parser_bison.y b/src/parser_bison.y
index c25af6ba114a..3acd80317456 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -689,7 +689,7 @@ int nft_lex(void *, void *, void *);
 %type <val>			level_type log_flags log_flags_tcp log_flag_tcp
 %type <stmt>			limit_stmt quota_stmt connlimit_stmt
 %destructor { stmt_free($$); }	limit_stmt quota_stmt connlimit_stmt
-%type <val>			limit_burst_pkts limit_burst_bytes limit_mode time_unit quota_mode
+%type <val>			limit_burst_pkts limit_burst_bytes limit_mode limit_bytes time_unit quota_mode
 %type <stmt>			reject_stmt reject_stmt_alloc
 %destructor { stmt_free($$); }	reject_stmt reject_stmt_alloc
 %type <stmt>			nat_stmt nat_stmt_alloc masq_stmt masq_stmt_alloc redir_stmt redir_stmt_alloc
@@ -3251,19 +3251,22 @@ limit_burst_pkts	:	/* empty */			{ $$ = 5; }
 			;
 
 limit_burst_bytes	:	/* empty */			{ $$ = 5; }
-			|	BURST	NUM	BYTES		{ $$ = $2; }
-			|	BURST	NUM	STRING
+			|	BURST	limit_bytes		{ $$ = $2; }
+			;
+
+limit_bytes		:	NUM	BYTES		{ $$ = $1; }
+			|	NUM	STRING
 			{
 				struct error_record *erec;
 				uint64_t rate;
 
-				erec = data_unit_parse(&@$, $3, &rate);
-				xfree($3);
+				erec = data_unit_parse(&@$, $2, &rate);
+				xfree($2);
 				if (erec != NULL) {
 					erec_queue(erec, state->msgs);
 					YYERROR;
 				}
-				$$ = $2 * rate;
+				$$ = $1 * rate;
 			}
 			;
 
-- 
2.33.0

[nft PATCH v2 2/3] parser: add `limit_rate_pkts` and `limit_rate_bytes` rules

[Jeremy Sowden]

Factor the `N / time-unit` and `N byte-unit / time-unit` expressions
from limit expressions out into separate `limit_rate_pkts` and
`limit_rate_bytes` rules respectively.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 include/datatype.h |   4 ++
 src/parser_bison.y | 121 ++++++++++++++++++++++-----------------------
 2 files changed, 63 insertions(+), 62 deletions(-)

diff --git a/include/datatype.h b/include/datatype.h
index 448be57fbc7f..7ddd3566d459 100644
--- a/include/datatype.h
+++ b/include/datatype.h
@@ -309,6 +309,10 @@ extern struct error_record *rate_parse(const struct location *loc,
 extern struct error_record *data_unit_parse(const struct location *loc,
 					    const char *str, uint64_t *rate);
 
+struct limit_rate {
+	uint64_t rate, unit;
+};
+
 extern void expr_chain_export(const struct expr *e, char *chain);
 
 #endif /* NFTABLES_DATATYPE_H */
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 3acd80317456..cf1e139d42f3 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -186,6 +186,7 @@ int nft_lex(void *, void *, void *);
 	struct handle_spec	handle_spec;
 	struct position_spec	position_spec;
 	struct prio_spec	prio_spec;
+	struct limit_rate	limit_rate;
 }
 
 %token TOKEN_EOF 0		"end of file"
@@ -607,6 +608,9 @@ int nft_lex(void *, void *, void *);
 %token IN			"in"
 %token OUT			"out"
 
+%type <limit_rate>		limit_rate_pkts
+%type <limit_rate>		limit_rate_bytes
+
 %type <string>			identifier type_identifier string comment_spec
 %destructor { xfree($$); }	identifier type_identifier string comment_spec
 
@@ -3145,42 +3149,31 @@ log_flag_tcp		:	SEQUENCE
 			}
 			;
 
-limit_stmt		:	LIMIT	RATE	limit_mode	NUM	SLASH	time_unit	limit_burst_pkts	close_scope_limit
+limit_stmt		:	LIMIT	RATE	limit_mode	limit_rate_pkts	limit_burst_pkts	close_scope_limit
 	    		{
-				if ($7 == 0) {
-					erec_queue(error(&@7, "limit burst must be > 0"),
+				if ($5 == 0) {
+					erec_queue(error(&@5, "limit burst must be > 0"),
 						   state->msgs);
 					YYERROR;
 				}
 				$$ = limit_stmt_alloc(&@$);
-				$$->limit.rate	= $4;
-				$$->limit.unit	= $6;
-				$$->limit.burst	= $7;
+				$$->limit.rate	= $4.rate;
+				$$->limit.unit	= $4.unit;
+				$$->limit.burst	= $5;
 				$$->limit.type	= NFT_LIMIT_PKTS;
 				$$->limit.flags = $3;
 			}
-			|	LIMIT	RATE	limit_mode	NUM	STRING	limit_burst_bytes	close_scope_limit
+			|	LIMIT	RATE	limit_mode	limit_rate_bytes	limit_burst_bytes	close_scope_limit
 			{
-				struct error_record *erec;
-				uint64_t rate, unit;
-
-				if ($6 == 0) {
-					erec_queue(error(&@6, "limit burst must be > 0"),
+				if ($5 == 0) {
+					erec_queue(error(&@5, "limit burst must be > 0"),
 						   state->msgs);
 					YYERROR;
 				}
-
-				erec = rate_parse(&@$, $5, &rate, &unit);
-				xfree($5);
-				if (erec != NULL) {
-					erec_queue(erec, state->msgs);
-					YYERROR;
-				}
-
 				$$ = limit_stmt_alloc(&@$);
-				$$->limit.rate	= rate * $4;
-				$$->limit.unit	= unit;
-				$$->limit.burst	= $6;
+				$$->limit.rate	= $4.rate;
+				$$->limit.unit	= $4.unit;
+				$$->limit.burst	= $5;
 				$$->limit.type	= NFT_LIMIT_PKT_BYTES;
 				$$->limit.flags = $3;
 			}
@@ -3250,10 +3243,33 @@ limit_burst_pkts	:	/* empty */			{ $$ = 5; }
 			|	BURST	NUM	PACKETS		{ $$ = $2; }
 			;
 
+limit_rate_pkts		:	NUM     SLASH	time_unit
+			{
+				$$.rate = $1;
+				$$.unit = $3;
+			}
+			;
+
 limit_burst_bytes	:	/* empty */			{ $$ = 5; }
 			|	BURST	limit_bytes		{ $$ = $2; }
 			;
 
+limit_rate_bytes	:	NUM     STRING
+			{
+				struct error_record *erec;
+				uint64_t rate, unit;
+
+				erec = rate_parse(&@$, $2, &rate, &unit);
+				xfree($2);
+				if (erec != NULL) {
+					erec_queue(erec, state->msgs);
+					YYERROR;
+				}
+				$$.rate = rate * $1;
+				$$.unit = unit;
+			}
+			;
+
 limit_bytes		:	NUM	BYTES		{ $$ = $1; }
 			|	NUM	STRING
 			{
@@ -4283,44 +4299,34 @@ set_elem_stmt		:	COUNTER	close_scope_counter
 				$$->counter.packets = $3;
 				$$->counter.bytes = $5;
 			}
-			|	LIMIT   RATE    limit_mode      NUM     SLASH   time_unit       limit_burst_pkts	close_scope_limit
+			|	LIMIT   RATE    limit_mode      limit_rate_pkts       limit_burst_pkts	close_scope_limit
 			{
-				if ($7 == 0) {
-					erec_queue(error(&@7, "limit burst must be > 0"),
+				if ($5 == 0) {
+					erec_queue(error(&@5, "limit burst must be > 0"),
 						   state->msgs);
 					YYERROR;
 				}
 				$$ = limit_stmt_alloc(&@$);
-				$$->limit.rate  = $4;
-				$$->limit.unit  = $6;
-				$$->limit.burst = $7;
+				$$->limit.rate  = $4.rate;
+				$$->limit.unit  = $4.unit;
+				$$->limit.burst = $5;
 				$$->limit.type  = NFT_LIMIT_PKTS;
 				$$->limit.flags = $3;
 			}
-			|       LIMIT   RATE    limit_mode      NUM     STRING  limit_burst_bytes	close_scope_limit
+			|       LIMIT   RATE    limit_mode      limit_rate_bytes  limit_burst_bytes	close_scope_limit
 			{
-				struct error_record *erec;
-				uint64_t rate, unit;
-
-				if ($6 == 0) {
+				if ($5 == 0) {
 					erec_queue(error(&@6, "limit burst must be > 0"),
 						   state->msgs);
 					YYERROR;
 				}
-				erec = rate_parse(&@$, $5, &rate, &unit);
-				xfree($5);
-				if (erec != NULL) {
-					erec_queue(erec, state->msgs);
-					YYERROR;
-				}
-
 				$$ = limit_stmt_alloc(&@$);
-				$$->limit.rate  = rate * $4;
-				$$->limit.unit  = unit;
-				$$->limit.burst = $6;
+				$$->limit.rate  = $4.rate;
+				$$->limit.unit  = $4.unit;
+				$$->limit.burst = $5;
 				$$->limit.type  = NFT_LIMIT_PKT_BYTES;
 				$$->limit.flags = $3;
-                        }
+			}
 			|	CT	COUNT	NUM	close_scope_ct
 			{
 				$$ = connlimit_stmt_alloc(&@$);
@@ -4553,34 +4559,25 @@ ct_obj_alloc		:	/* empty */
 			}
 			;
 
-limit_config		:	RATE	limit_mode	NUM	SLASH	time_unit	limit_burst_pkts
+limit_config		:	RATE	limit_mode	limit_rate_pkts	limit_burst_pkts
 			{
 				struct limit *limit;
 
 				limit = &$<obj>0->limit;
-				limit->rate	= $3;
-				limit->unit	= $5;
-				limit->burst	= $6;
+				limit->rate	= $3.rate;
+				limit->unit	= $3.unit;
+				limit->burst	= $4;
 				limit->type	= NFT_LIMIT_PKTS;
 				limit->flags	= $2;
 			}
-			|	RATE	limit_mode	NUM	STRING	limit_burst_bytes
+			|	RATE	limit_mode	limit_rate_bytes	limit_burst_bytes
 			{
 				struct limit *limit;
-				struct error_record *erec;
-				uint64_t rate, unit;
-
-				erec = rate_parse(&@$, $4, &rate, &unit);
-				xfree($4);
-				if (erec != NULL) {
-					erec_queue(erec, state->msgs);
-					YYERROR;
-				}
 
 				limit = &$<obj>0->limit;
-				limit->rate	= rate * $3;
-				limit->unit	= unit;
-				limit->burst	= $5;
+				limit->rate	= $3.rate;
+				limit->unit	= $3.unit;
+				limit->burst	= $4;
 				limit->type	= NFT_LIMIT_PKT_BYTES;
 				limit->flags	= $2;
 			}
-- 
2.33.0

[nft PATCH v2 3/3] parser: extend limit syntax

[Jeremy Sowden]

The documentation describes the syntax of limit statements thus:

  limit rate [over] packet_number / TIME_UNIT [burst packet_number packets]
  limit rate [over] byte_number BYTE_UNIT / TIME_UNIT [burst byte_number BYTE_UNIT]

  TIME_UNIT := second | minute | hour | day
  BYTE_UNIT := bytes | kbytes | mbytes

From this one might infer that a limit may be specified by any of the
following:

  limit rate 1048576/second
  limit rate 1048576 mbytes/second

  limit rate 1048576 / second
  limit rate 1048576 mbytes / second

However, the last does not currently parse:

  $ sudo /usr/sbin/nft add filter input limit rate 1048576 mbytes / second
  Error: wrong rate format
  add filter input limit rate 1048576 mbytes / second
                   ^^^^^^^^^^^^^^^^^^^^^^^^^

Extend the `limit_rate_bytes` parser rule to support it, and add some
new Python test-cases.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 src/parser_bison.y           |  5 +++++
 tests/py/any/limit.t         |  5 +++++
 tests/py/any/limit.t.json    | 39 ++++++++++++++++++++++++++++++++++++
 tests/py/any/limit.t.payload | 13 ++++++++++++
 4 files changed, 62 insertions(+)

diff --git a/src/parser_bison.y b/src/parser_bison.y
index cf1e139d42f3..65fd35a36cde 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -3268,6 +3268,11 @@ limit_rate_bytes	:	NUM     STRING
 				$$.rate = rate * $1;
 				$$.unit = unit;
 			}
+			|	limit_bytes SLASH time_unit
+			{
+				$$.rate = $1;
+				$$.unit = $3;
+			}
 			;
 
 limit_bytes		:	NUM	BYTES		{ $$ = $1; }
diff --git a/tests/py/any/limit.t b/tests/py/any/limit.t
index 0110e77f2e85..86e8d43009b9 100644
--- a/tests/py/any/limit.t
+++ b/tests/py/any/limit.t
@@ -25,6 +25,11 @@ limit rate 10230 mbytes/second;ok
 limit rate 1023000 mbytes/second;ok
 limit rate 512 kbytes/second burst 5 packets;fail
 
+limit rate 1 bytes / second;ok;limit rate 1 bytes/second
+limit rate 1 kbytes / second;ok;limit rate 1 kbytes/second
+limit rate 1 mbytes / second;ok;limit rate 1 mbytes/second
+limit rate 1 gbytes / second;fail
+
 limit rate 1025 bytes/second burst 512 bytes;ok
 limit rate 1025 kbytes/second burst 1023 kbytes;ok
 limit rate 1025 mbytes/second burst 1025 kbytes;ok
diff --git a/tests/py/any/limit.t.json b/tests/py/any/limit.t.json
index 8bab7e3d79b4..b41ae60a3bd6 100644
--- a/tests/py/any/limit.t.json
+++ b/tests/py/any/limit.t.json
@@ -125,6 +125,45 @@
     }
 ]
 
+# limit rate 1 bytes / second
+[
+    {
+        "limit": {
+            "burst": 5,
+            "burst_unit": "bytes",
+            "per": "second",
+            "rate": 1,
+            "rate_unit": "bytes"
+        }
+    }
+]
+
+# limit rate 1 kbytes / second
+[
+    {
+        "limit": {
+            "burst": 5,
+            "burst_unit": "bytes",
+            "per": "second",
+            "rate": 1,
+            "rate_unit": "kbytes"
+        }
+    }
+]
+
+# limit rate 1 mbytes / second
+[
+    {
+        "limit": {
+            "burst": 5,
+            "burst_unit": "bytes",
+            "per": "second",
+            "rate": 1,
+            "rate_unit": "mbytes"
+        }
+    }
+]
+
 # limit rate 1025 bytes/second burst 512 bytes
 [
     {
diff --git a/tests/py/any/limit.t.payload b/tests/py/any/limit.t.payload
index dc6cea9b2846..3bd85f4ebf45 100644
--- a/tests/py/any/limit.t.payload
+++ b/tests/py/any/limit.t.payload
@@ -46,6 +46,19 @@ ip test-ip4 output
 ip test-ip4 output
   [ limit rate 1072693248000/second burst 5 type bytes flags 0x0 ]
 
+# limit rate 1 bytes / second
+ip
+  [ limit rate 1/second burst 5 type bytes flags 0x0 ]
+
+# limit rate 1 kbytes / second
+ip
+  [ limit rate 1024/second burst 5 type bytes flags 0x0 ]
+
+# limit rate 1 mbytes / second
+ip
+  [ limit rate 1048576/second burst 5 type bytes flags 0x0 ]
+
+
 # limit rate 1025 bytes/second burst 512 bytes
 ip test-ip4 output
   [ limit rate 1025/second burst 512 type bytes flags 0x0 ]
-- 
2.33.0

Re: [nft PATCH v2 0/3] parser: refactor and extend limit rate rules

[Pablo Neira Ayuso]

On Fri, Oct 29, 2021 at 09:40:06PM +0100, Jeremy Sowden wrote:
> The first two patches introduce new rules to deduplicate the code for
> parsing `limit rate` expressions and make it easier to extend the
> syntax.
> 
> The third patch extends the syntax to handle expressions like `limit
> rate 1 mbytes / second`, which are not currently supported.

Series applied, thanks.