[nft,6/8] monitor: Fix printing of ct objects

Message ID 20181011154901.20082-7-phil@nwl.cc
State Accepted
Delegated to: Pablo Neira
Headers show
Series
  • monitor: Use libnftables for JSON output
Related show

Commit Message

Phil Sutter Oct. 11, 2018, 3:48 p.m.
Monitor output is supposed to be single lined without tabs, but ct
object were printed with newlines and tabs hard-coded. Fixing this
wasn't too hard given that there is 'stmt_separator' to also include
semi-colons where required if newline was removed.

A more obvious mistake was position of object type in monitor output:
Like with other object types, it has to occur between command and table
spec. As a positive side-effect, this aligns ct objects better with
others (see obj_type_name_array for instance).

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 src/json.c                                    |  2 -
 src/rule.c                                    | 47 +++++++++++--------
 tests/monitor/testcases/object.t              | 33 +++++++++++++
 tests/shell/testcases/listing/0013objects_0   |  2 +-
 .../testcases/nft-f/0017ct_timeout_obj_0      |  2 +-
 .../nft-f/dumps/0017ct_timeout_obj_0.nft      |  2 +-
 6 files changed, 63 insertions(+), 25 deletions(-)
 create mode 100644 tests/monitor/testcases/object.t

Patch

diff --git a/src/json.c b/src/json.c
index b8d9333e877a8..a0a2b9db65b0b 100644
--- a/src/json.c
+++ b/src/json.c
@@ -282,7 +282,6 @@  static json_t *obj_print_json(struct output_ctx *octx, const struct obj *obj)
 		json_decref(tmp);
 		break;
 	case NFT_OBJECT_CT_HELPER:
-		type = "ct helper";
 		tmp = json_pack("{s:s, s:o, s:s}",
 				"type", obj->ct_helper.name, "protocol",
 				proto_name_json(obj->ct_helper.l4proto),
@@ -291,7 +290,6 @@  static json_t *obj_print_json(struct output_ctx *octx, const struct obj *obj)
 		json_decref(tmp);
 		break;
 	case NFT_OBJECT_CT_TIMEOUT:
-		type = "ct timeout";
 		tmp = timeout_policy_json(obj->ct_timeout.l4proto,
 					  obj->ct_timeout.timeout);
 		tmp = json_pack("{s:o, s:s, s:o}",
diff --git a/src/rule.c b/src/rule.c
index b00a17d652005..58ac94f6716ca 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -1662,12 +1662,13 @@  static void print_proto_name_proto(uint8_t l4, struct output_ctx *octx)
 }
 
 static void print_proto_timeout_policy(uint8_t l4, const uint32_t *timeout,
+				       struct print_fmt_options *opts,
 				       struct output_ctx *octx)
 {
 	bool comma = false;
 	unsigned int i;
 
-	nft_print(octx, "\t\tpolicy = {");
+	nft_print(octx, "%s%spolicy = { ", opts->tab, opts->tab);
 	for (i = 0; i < timeout_protocol[l4].array_size; i++) {
 		if (timeout[i] != timeout_protocol[l4].dflt_timeout[i]) {
 			if (comma)
@@ -1678,7 +1679,7 @@  static void print_proto_timeout_policy(uint8_t l4, const uint32_t *timeout,
 			comma = true;
 		}
 	}
-	nft_print(octx, "}");
+	nft_print(octx, " }%s", opts->stmt_separator);
 }
 
 static void obj_print_data(const struct obj *obj,
@@ -1695,8 +1696,8 @@  static void obj_print_data(const struct obj *obj,
 			nft_print(octx, "packets 0 bytes 0");
 			break;
 		}
-		nft_print(octx, "packets %" PRIu64 " bytes %" PRIu64 "",
-			  obj->counter.packets, obj->counter.bytes);
+		nft_print(octx, "packets %" PRIu64 " bytes %" PRIu64 "%s",
+			  obj->counter.packets, obj->counter.bytes, opts->nl);
 		break;
 	case NFT_OBJECT_QUOTA: {
 		const char *data_unit;
@@ -1715,33 +1716,37 @@  static void obj_print_data(const struct obj *obj,
 			nft_print(octx, " used %" PRIu64 " %s",
 				  bytes, data_unit);
 		}
+		nft_print(octx, "%s", opts->nl);
 		}
 		break;
 	case NFT_OBJECT_CT_HELPER:
-		nft_print(octx, "ct helper %s {", obj->handle.obj.name);
+		nft_print(octx, " %s {", obj->handle.obj.name);
 		if (octx->handle > 0)
 			nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id);
 		nft_print(octx, "%s", opts->nl);
-		nft_print(octx, "\t\ttype \"%s\" protocol ",
-			  obj->ct_helper.name);
+		nft_print(octx, "%s%stype \"%s\" protocol ",
+			  opts->tab, opts->tab, obj->ct_helper.name);
 		print_proto_name_proto(obj->ct_helper.l4proto, octx);
-		nft_print(octx, "\n");
-		nft_print(octx, "\t\tl3proto %s",
-			  family2str(obj->ct_helper.l3proto));
+		nft_print(octx, "%s", opts->stmt_separator);
+		nft_print(octx, "%s%sl3proto %s%s",
+			  opts->tab, opts->tab,
+			  family2str(obj->ct_helper.l3proto),
+			  opts->stmt_separator);
 		break;
 	case NFT_OBJECT_CT_TIMEOUT:
-		nft_print(octx, "ct timeout %s {", obj->handle.obj.name);
+		nft_print(octx, " %s {", obj->handle.obj.name);
 		if (octx->handle > 0)
 			nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id);
 		nft_print(octx, "%s", opts->nl);
-		nft_print(octx, "\t\tprotocol ");
+		nft_print(octx, "%s%sprotocol ", opts->tab, opts->tab);
 		print_proto_name_proto(obj->ct_timeout.l4proto, octx);
 		nft_print(octx, ";%s", opts->nl);
-		nft_print(octx, "\t\tl3proto %s",
-			  family2str(obj->ct_timeout.l3proto));
-		nft_print(octx, "%s", opts->nl);
+		nft_print(octx, "%s%sl3proto %s%s",
+			  opts->tab, opts->tab,
+			  family2str(obj->ct_timeout.l3proto),
+			  opts->stmt_separator);
 		print_proto_timeout_policy(obj->ct_timeout.l4proto,
-					   obj->ct_timeout.timeout, octx);
+					   obj->ct_timeout.timeout, opts, octx);
 		break;
 	case NFT_OBJECT_LIMIT: {
 		bool inv = obj->limit.flags & NFT_LIMIT_F_INV;
@@ -1776,10 +1781,11 @@  static void obj_print_data(const struct obj *obj,
 			}
 			break;
 		}
+		nft_print(octx, "%s", opts->nl);
 		}
 		break;
 	default:
-		nft_print(octx, "unknown {%s", opts->nl);
+		nft_print(octx, " unknown {%s", opts->nl);
 		break;
 	}
 }
@@ -1787,9 +1793,9 @@  static void obj_print_data(const struct obj *obj,
 static const char * const obj_type_name_array[] = {
 	[NFT_OBJECT_COUNTER]	= "counter",
 	[NFT_OBJECT_QUOTA]	= "quota",
-	[NFT_OBJECT_CT_HELPER]	= "",
+	[NFT_OBJECT_CT_HELPER]	= "ct helper",
 	[NFT_OBJECT_LIMIT]	= "limit",
-	[NFT_OBJECT_CT_TIMEOUT] = "",
+	[NFT_OBJECT_CT_TIMEOUT] = "ct timeout",
 };
 
 const char *obj_type_name(enum stmt_types type)
@@ -1828,7 +1834,7 @@  static void obj_print_declaration(const struct obj *obj,
 
 	obj_print_data(obj, opts, octx);
 
-	nft_print(octx, "%s%s}%s", opts->nl, opts->tab, opts->nl);
+	nft_print(octx, "%s}%s", opts->tab, opts->nl);
 }
 
 void obj_print(const struct obj *obj, struct output_ctx *octx)
@@ -1849,6 +1855,7 @@  void obj_print_plain(const struct obj *obj, struct output_ctx *octx)
 		.nl		= " ",
 		.table		= obj->handle.table.name,
 		.family		= family2str(obj->handle.family),
+		.stmt_separator = "; ",
 	};
 
 	obj_print_declaration(obj, &opts, octx);
diff --git a/tests/monitor/testcases/object.t b/tests/monitor/testcases/object.t
new file mode 100644
index 0000000000000..6695b0f0ba8b1
--- /dev/null
+++ b/tests/monitor/testcases/object.t
@@ -0,0 +1,33 @@ 
+# first the setup
+I add table ip t
+O -
+
+I add counter ip t c
+O add counter ip t c { packets 0 bytes 0 }
+
+I delete counter ip t c
+O -
+
+I add quota ip t q 25 mbytes
+O add quota ip t q { 25 mbytes }
+
+I delete quota ip t q
+O -
+
+I add limit ip t l rate 1/second
+O add limit ip t l { rate 1/second }
+
+I delete limit ip t l
+O -
+
+I add ct helper ip t cth { type "sip" protocol tcp; l3proto ip; }
+O -
+
+I delete ct helper ip t cth
+O -
+
+I add ct timeout ip t ctt { protocol udp; l3proto ip; policy = { unreplied: 15, replied: 12 }; }
+O -
+
+I delete ct timeout ip t ctt
+O -
diff --git a/tests/shell/testcases/listing/0013objects_0 b/tests/shell/testcases/listing/0013objects_0
index 2d72dbb53cc3d..713c783e0baa5 100755
--- a/tests/shell/testcases/listing/0013objects_0
+++ b/tests/shell/testcases/listing/0013objects_0
@@ -15,7 +15,7 @@  EXPECTED="table ip test {
 	ct timeout cttime {
 		protocol udp;
 		l3proto ip
-		policy = {unreplied: 15, replied: 12}
+		policy = { unreplied: 15, replied: 12 }
 	}
 
 	chain input {
diff --git a/tests/shell/testcases/nft-f/0017ct_timeout_obj_0 b/tests/shell/testcases/nft-f/0017ct_timeout_obj_0
index 1d03dbfc838e4..448a8207bac7d 100755
--- a/tests/shell/testcases/nft-f/0017ct_timeout_obj_0
+++ b/tests/shell/testcases/nft-f/0017ct_timeout_obj_0
@@ -4,7 +4,7 @@  EXPECTED='table ip filter {
 	ct timeout cttime{
 		protocol tcp;
 		l3proto ip
-		policy = {established: 123, close: 12}
+		policy = { established: 123, close: 12 }
 	}
 
 	chain c {
diff --git a/tests/shell/testcases/nft-f/dumps/0017ct_timeout_obj_0.nft b/tests/shell/testcases/nft-f/dumps/0017ct_timeout_obj_0.nft
index af0c627bec72c..bca36580f9c38 100755
--- a/tests/shell/testcases/nft-f/dumps/0017ct_timeout_obj_0.nft
+++ b/tests/shell/testcases/nft-f/dumps/0017ct_timeout_obj_0.nft
@@ -2,7 +2,7 @@  table ip filter {
 	ct timeout cttime {
 		protocol tcp;
 		l3proto ip
-		policy = {established: 123, close: 12}
+		policy = { established: 123, close: 12 }
 	}
 
 	chain c {