diff mbox series

[nft,14/14] meter: Don't print default size value

Message ID 20180528134819.13625-15-phil@nwl.cc
State Changes Requested
Delegated to: Pablo Neira
Headers show
Series JSON: Some minor schema changes | expand

Commit Message

Phil Sutter May 28, 2018, 1:48 p.m. UTC 
A meter size of 0xffff is the default, so regardless of whether it was
explicitly specified by user or not, don't print it. This is in line
with nft's tendency of shortening rules down to the minimal required
form.

While being at it, clean things up a bit:
- Introduce a macro to hold the default size.
- Make meter_stmt_alloc() assign the default size value.

Also adjust testcases in tests/py/ip{,6}/flowtable.t accordingly.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 include/statement.h             |  2 ++
 src/json.c                      |  9 ++++++---
 src/parser_bison.y              |  1 -
 src/parser_json.c               |  5 ++---
 src/statement.c                 | 10 ++++++++--
 tests/py/ip/flowtable.t         |  2 +-
 tests/py/ip/flowtable.t.json    |  3 ++-
 tests/py/ip/flowtable.t.payload |  4 ++--
 tests/py/ip6/flowtable.t        |  4 ++--
 9 files changed, 25 insertions(+), 15 deletions(-)

Comments

Florian Westphal May 28, 2018, 1:59 p.m. UTC | #1 
Phil Sutter <phil@nwl.cc> wrote:
> A meter size of 0xffff is the default, so regardless of whether it was
> explicitly specified by user or not, don't print it. This is in line
> with nft's tendency of shortening rules down to the minimal required
> form.

I think it should be printed to not depend on future versions
keeping this default.  nft always prints the size of meters, even if
its 0xffff.  The version of meters without explicit size was only
retained for backwards compat.

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Phil Sutter May 28, 2018, 3:55 p.m. UTC | #2 
Hi,

On Mon, May 28, 2018 at 03:59:39PM +0200, Florian Westphal wrote:
> Phil Sutter <phil@nwl.cc> wrote:
> > A meter size of 0xffff is the default, so regardless of whether it was
> > explicitly specified by user or not, don't print it. This is in line
> > with nft's tendency of shortening rules down to the minimal required
> > form.
> 
> I think it should be printed to not depend on future versions
> keeping this default.  nft always prints the size of meters, even if
> its 0xffff.  The version of meters without explicit size was only
> retained for backwards compat.

OK, fair enough. I'll respin with this patch replaced by one which fixes
the JSON equivalent instead.

Thanks, Phil
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox series

Patch

diff --git a/include/statement.h b/include/statement.h
index de26549b32f4c..82e95bcc9071a 100644
--- a/include/statement.h
+++ b/include/statement.h
@@ -189,6 +189,8 @@  struct meter_stmt {
 	uint32_t		size;
 };
 
+#define METER_DEFAULT_SIZE	0xffff
+
 extern struct stmt *meter_stmt_alloc(const struct location *loc);
 
 /**
diff --git a/src/json.c b/src/json.c
index 11607b677ffbb..b6ebf940fea03 100644
--- a/src/json.c
+++ b/src/json.c
@@ -1211,10 +1211,13 @@  json_t *meter_stmt_json(const struct stmt *stmt, struct output_ctx *octx)
 	tmp = stmt_print_json(stmt->meter.stmt, octx);
 	octx->stateless--;
 
-	root = json_pack("{s:o, s:o, s:i}",
+	root = json_pack("{s:o, s:o}",
 			 "key", expr_print_json(stmt->meter.key, octx),
-			 "stmt", tmp,
-			 "size", stmt->meter.size);
+			 "stmt", tmp);
+	if (stmt->meter.size != METER_DEFAULT_SIZE) {
+		tmp = json_integer(stmt->meter.size);
+		json_object_set_new(root, "size", tmp);
+	}
 	if (stmt->meter.set) {
 		tmp = json_string(stmt->meter.set->set->handle.set.name);
 		json_object_set_new(root, "name", tmp);
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 0e3ee84f3c400..333013aed3fd7 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -2789,7 +2789,6 @@  meter_stmt_alloc	:	METER	identifier		'{' meter_key_expr stmt '}'
 			{
 				$$ = meter_stmt_alloc(&@$);
 				$$->meter.name = $2;
-				$$->meter.size = 0xffff;
 				$$->meter.key  = $4;
 				$$->meter.stmt = $5;
 				$$->location  = @$;
diff --git a/src/parser_json.c b/src/parser_json.c
index 336092b77c6ad..a794529679b97 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -1967,16 +1967,15 @@  static struct stmt *json_parse_meter_stmt(struct json_ctx *ctx,
 	json_t *jkey, *jstmt;
 	struct stmt *stmt;
 	const char *name;
-	uint32_t size = 0xffff;
 
 	if (json_unpack_err(ctx, value, "{s:s, s:o, s:o}",
 			    "name", &name, "key", &jkey, "stmt", &jstmt))
 		return NULL;
-	json_unpack(value, "{s:i}", "size", &size);
 
 	stmt = meter_stmt_alloc(int_loc);
 	stmt->meter.name = xstrdup(name);
-	stmt->meter.size = size;
+
+	json_unpack(value, "{s:i}", "size", &stmt->meter.size);
 
 	stmt->meter.key = json_parse_set_elem_expr_stmt(ctx, jkey);
 	if (!stmt->meter.key) {
diff --git a/src/statement.c b/src/statement.c
index 687ee351b0ca9..9e18bbe7400eb 100644
--- a/src/statement.c
+++ b/src/statement.c
@@ -117,7 +117,9 @@  static void meter_stmt_print(const struct stmt *stmt, struct output_ctx *octx)
 		expr_print(stmt->meter.set, octx);
 		nft_print(octx, " ");
 	}
-	nft_print(octx, "size %u { ", stmt->meter.size);
+	if (stmt->meter.size != METER_DEFAULT_SIZE)
+		nft_print(octx, "size %u ", stmt->meter.size);
+	nft_print(octx, "{ ");
 	expr_print(stmt->meter.key, octx);
 	nft_print(octx, " ");
 
@@ -146,7 +148,11 @@  static const struct stmt_ops meter_stmt_ops = {
 
 struct stmt *meter_stmt_alloc(const struct location *loc)
 {
-	return stmt_alloc(loc, &meter_stmt_ops);
+	struct stmt *stmt = stmt_alloc(loc, &meter_stmt_ops);
+
+	stmt->meter.size = METER_DEFAULT_SIZE;
+
+	return stmt;
 }
 
 static void counter_stmt_print(const struct stmt *stmt, struct output_ctx *octx)
diff --git a/tests/py/ip/flowtable.t b/tests/py/ip/flowtable.t
index 7a68788a7df14..f7daeedda7119 100644
--- a/tests/py/ip/flowtable.t
+++ b/tests/py/ip/flowtable.t
@@ -2,4 +2,4 @@ 
 
 *ip;test-ip;input
 
-meter xyz { ip saddr timeout 30s counter};ok;meter xyz size 65535 { ip saddr timeout 30s counter}
+meter xyz size 65535 { ip saddr timeout 30s counter};ok;meter xyz { ip saddr timeout 30s counter}
diff --git a/tests/py/ip/flowtable.t.json b/tests/py/ip/flowtable.t.json
index 3b0664bd5c8c5..0d9d8d0721deb 100644
--- a/tests/py/ip/flowtable.t.json
+++ b/tests/py/ip/flowtable.t.json
@@ -1,4 +1,4 @@ 
-# meter xyz { ip saddr timeout 30s counter}
+# meter xyz size 65535 { ip saddr timeout 30s counter}
 [
     {
         "meter": {
@@ -14,6 +14,7 @@ 
                 }
             },
             "name": "xyz",
+	    "size": 65535,
             "stmt": {
                 "counter": null
             }
diff --git a/tests/py/ip/flowtable.t.payload b/tests/py/ip/flowtable.t.payload
index 34a584994b64b..bb32cad99caee 100644
--- a/tests/py/ip/flowtable.t.payload
+++ b/tests/py/ip/flowtable.t.payload
@@ -1,5 +1,5 @@ 
-# meter xyz { ip saddr timeout 30s counter}
-xyz test-ip 31
+# meter xyz size 65535 { ip saddr timeout 30s counter}
+xyz test-ip 31 size 65535
 xyz test-ip 0
 ip test-ip input 
   [ payload load 4b @ network header + 12 => reg 1 ]
diff --git a/tests/py/ip6/flowtable.t b/tests/py/ip6/flowtable.t
index d89e90c3edc46..5c048935d726c 100644
--- a/tests/py/ip6/flowtable.t
+++ b/tests/py/ip6/flowtable.t
@@ -2,5 +2,5 @@ 
 
 *ip6;test-ip6;input
 
-meter acct_out { meta iif . ip6 saddr timeout 600s counter };ok;meter acct_out size 65535 { iif . ip6 saddr timeout 10m counter}
-meter acct_out { ip6 saddr . meta iif timeout 600s counter };ok;meter acct_out size 65535 { ip6 saddr . iif timeout 10m counter}
+meter acct_out { meta iif . ip6 saddr timeout 600s counter };ok;meter acct_out { iif . ip6 saddr timeout 10m counter}
+meter acct_out { ip6 saddr . meta iif timeout 600s counter };ok;meter acct_out { ip6 saddr . iif timeout 10m counter}