[PATCH nft] src: provide elem_stmt structure to shrink compact counters

[Pablo Neira Ayuso <pablo@netfilter.org>]

Add struct elem_stmt with same layout as struct stmt, this introduces a
variant of the statement object with a flexible array to allocate the
data area for counter/quota/limit/ct count/last.

Shrink from 52.48 Mbytes to 48.86 Mbytes for a set of 100k elements with
counters, this provides a reduction of 6.89% in memory footprint.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/statement.h | 23 +++++++++++++++
 src/intervals.c     |  8 +++--
 src/optimize.c      | 18 +++++++-----
 src/parser_bison.y  | 72 ++++++++++++++++++++++++++++++++++++++++-----
 src/statement.c     | 58 ++++++++++++++++++++++++++++++++----
 5 files changed, 157 insertions(+), 22 deletions(-)

diff --git a/include/statement.h b/include/statement.h
index e8724dde63d0..ab1ff2476db8 100644
--- a/include/statement.h
+++ b/include/statement.h
@@ -348,6 +348,7 @@ enum stmt_types {
  * struct stmt_ops
  *
  * @type:	statement type
+ * @size:	size
  * @name:	name
  * @destroy:	destructor
  * @print:	function to print statement
@@ -355,6 +356,7 @@ enum stmt_types {
 struct stmt;
 struct stmt_ops {
 	enum stmt_types		type;
+	uint32_t		size;
 	const char		*name;
 	void			(*destroy)(struct stmt *stmt);
 	void			(*print)(const struct stmt *stmt,
@@ -426,4 +428,25 @@ const struct stmt_ops *stmt_ops(const struct stmt *stmt);
 const char *get_rate(uint64_t byte_rate, uint64_t *rate);
 const char *get_unit(uint64_t u);
 
+struct elem_stmt {
+	/* same layout as struct stmt. */
+	struct list_head		list;
+	struct location			location;
+	enum stmt_flags			flags;
+	enum stmt_types			type:8;
+
+	char				data[] __attribute__ ((aligned (__alignof__(sizeof(long)))));;
+};
+
+struct elem_stmt *elem_stmt_alloc(const struct location *loc,
+				  const struct stmt_ops *ops);
+void elem_stmt_free(const struct elem_stmt *elem_stmt);
+const struct stmt_ops *elem_stmt_ops(const struct elem_stmt *stmt);
+
+extern const struct stmt_ops counter_stmt_ops;
+extern const struct stmt_ops limit_stmt_ops;
+extern const struct stmt_ops quota_stmt_ops;
+extern const struct stmt_ops connlimit_stmt_ops;
+extern const struct stmt_ops last_stmt_ops;
+
 #endif /* NFTABLES_STATEMENT_H */
diff --git a/src/intervals.c b/src/intervals.c
index bf125a0c59d3..5462eca29685 100644
--- a/src/intervals.c
+++ b/src/intervals.c
@@ -723,11 +723,13 @@ int set_to_intervals(const struct set *set, struct expr *init, bool add)
 /* This only works for the supported stateful statements. */
 static void set_elem_stmt_clone(struct expr *dst, const struct expr *src)
 {
-	struct stmt *stmt, *nstmt;
+	struct elem_stmt *stmt, *nstmt;
+	const struct stmt_ops *ops;
 
 	list_for_each_entry(stmt, &src->stmt_list, list) {
-		nstmt = xzalloc(sizeof(*stmt));
-		*nstmt = *stmt;
+		ops = elem_stmt_ops(stmt);
+		nstmt = xzalloc(sizeof(*stmt) + ops->size);
+		memcpy(nstmt, stmt, sizeof(*stmt) + ops->size);
 		list_add_tail(&nstmt->list, &dst->stmt_list);
 	}
 }
diff --git a/src/optimize.c b/src/optimize.c
index 89ba0d9dee6a..999e89c206ff 100644
--- a/src/optimize.c
+++ b/src/optimize.c
@@ -746,14 +746,15 @@ static void build_verdict_map(struct expr *expr, struct stmt *verdict,
 			      struct expr *set, struct stmt *counter)
 {
 	struct expr *item, *elem, *mapping;
-	struct stmt *counter_elem;
+	struct elem_stmt *counter_elem;
 
 	switch (expr->etype) {
 	case EXPR_LIST:
 		list_for_each_entry(item, &expr->expressions, list) {
 			elem = set_elem_expr_alloc(&internal_location, expr_get(item));
 			if (counter) {
-				counter_elem = counter_stmt_alloc(&counter->location);
+				counter_elem = elem_stmt_alloc(&counter->location,
+							       &counter_stmt_ops);
 				list_add_tail(&counter_elem->list, &elem->stmt_list);
 			}
 
@@ -761,13 +762,13 @@ static void build_verdict_map(struct expr *expr, struct stmt *verdict,
 						     expr_get(verdict->expr));
 			compound_expr_add(set, mapping);
 		}
-		stmt_free(counter);
 		break;
 	case EXPR_SET:
 		list_for_each_entry(item, &expr->expressions, list) {
 			elem = set_elem_expr_alloc(&internal_location, expr_get(item->key));
 			if (counter) {
-				counter_elem = counter_stmt_alloc(&counter->location);
+				counter_elem = elem_stmt_alloc(&counter->location,
+							       &counter_stmt_ops);
 				list_add_tail(&counter_elem->list, &elem->stmt_list);
 			}
 
@@ -775,7 +776,6 @@ static void build_verdict_map(struct expr *expr, struct stmt *verdict,
 						     expr_get(verdict->expr));
 			compound_expr_add(set, mapping);
 		}
-		stmt_free(counter);
 		break;
 	case EXPR_PREFIX:
 	case EXPR_RANGE_SYMBOL:
@@ -785,8 +785,11 @@ static void build_verdict_map(struct expr *expr, struct stmt *verdict,
 	case EXPR_SYMBOL:
 	case EXPR_CONCAT:
 		elem = set_elem_expr_alloc(&internal_location, expr_get(expr));
-		if (counter)
-			list_add_tail(&counter->list, &elem->stmt_list);
+		if (counter) {
+			counter_elem = elem_stmt_alloc(&counter->location,
+						       &counter_stmt_ops);
+			list_add_tail(&counter_elem->list, &elem->stmt_list);
+		}
 
 		mapping = mapping_expr_alloc(&internal_location, elem,
 					     expr_get(verdict->expr));
@@ -796,6 +799,7 @@ static void build_verdict_map(struct expr *expr, struct stmt *verdict,
 		assert(0);
 		break;
 	}
+	stmt_free(counter);
 }
 
 static void remove_counter(const struct optimize_ctx *ctx, uint32_t from)
diff --git a/src/parser_bison.y b/src/parser_bison.y
index f9cc909836bc..11de82155ab1 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -247,6 +247,7 @@ int nft_lex(void *, void *, void *);
 		uint8_t field;
 	} tcp_kind_field;
 	struct timeout_state	*timeout_state;
+	struct elem_stmt	*elem_stmt;
 }
 
 %token TOKEN_EOF 0		"end of file"
@@ -766,8 +767,8 @@ int nft_lex(void *, void *, void *);
 
 %type <list>			stmt_list stateful_stmt_list set_elem_stmt_list
 %destructor { stmt_list_free($$); free($$); } stmt_list stateful_stmt_list set_elem_stmt_list
-%type <stmt>			stmt match_stmt verdict_stmt set_elem_stmt
-%destructor { stmt_free($$); }	stmt match_stmt verdict_stmt set_elem_stmt
+%type <stmt>			stmt match_stmt verdict_stmt
+%destructor { stmt_free($$); }	stmt match_stmt verdict_stmt
 %type <stmt>			counter_stmt counter_stmt_alloc stateful_stmt last_stmt
 %destructor { stmt_free($$); }	counter_stmt counter_stmt_alloc stateful_stmt last_stmt
 %type <stmt>			limit_stmt_alloc quota_stmt_alloc last_stmt_alloc ct_limit_stmt_alloc
@@ -775,6 +776,16 @@ int nft_lex(void *, void *, void *);
 %type <stmt>			objref_stmt objref_stmt_counter objref_stmt_limit objref_stmt_quota objref_stmt_ct objref_stmt_synproxy
 %destructor { stmt_free($$); }	objref_stmt objref_stmt_counter objref_stmt_limit objref_stmt_quota objref_stmt_ct objref_stmt_synproxy
 
+%type <elem_stmt>		counter_elem_stmt_alloc quota_elem_stmt_alloc ct_limit_elem_stmt_alloc
+%destructor { elem_stmt_free($$); } counter_elem_stmt_alloc quota_elem_stmt_alloc ct_limit_elem_stmt_alloc
+%type <elem_stmt>		limit_elem_stmt_alloc last_elem_stmt_alloc
+%destructor { elem_stmt_free($$); } limit_elem_stmt_alloc last_elem_stmt_alloc
+
+%type <elem_stmt>		set_elem_stmt counter_elem_stmt
+%destructor { elem_stmt_free($$); } set_elem_stmt counter_elem_stmt
+%type <elem_stmt>		quota_elem_stmt limit_elem_stmt connlimit_elem_stmt last_elem_stmt
+%destructor { elem_stmt_free($$); } quota_elem_stmt limit_elem_stmt connlimit_elem_stmt last_elem_stmt
+
 %type <stmt>			payload_stmt
 %destructor { stmt_free($$); }	payload_stmt
 %type <stmt>			ct_stmt
@@ -4663,11 +4674,58 @@ set_elem_stmt_list	:	set_elem_stmt
 			}
 			;
 
-set_elem_stmt		:	counter_stmt	close_scope_counter
-			|	limit_stmt	close_scope_limit
-			|	connlimit_stmt	close_scope_ct
-			|	quota_stmt	close_scope_quota
-			|	last_stmt	close_scope_last
+set_elem_stmt		:	counter_elem_stmt	close_scope_counter
+			|	limit_elem_stmt		close_scope_limit
+			|	connlimit_elem_stmt	close_scope_ct
+			|	quota_elem_stmt		close_scope_quota
+			|	last_elem_stmt		close_scope_last
+			;
+
+counter_elem_stmt	:	counter_elem_stmt_alloc
+			|	counter_elem_stmt_alloc	counter_args
+			;
+
+counter_elem_stmt_alloc	:	COUNTER
+			{
+				$$ = elem_stmt_alloc(&@$, &counter_stmt_ops);
+			}
+			;
+
+limit_elem_stmt_alloc	:	LIMIT RATE
+			{
+				$$ = elem_stmt_alloc(&@$, &limit_stmt_ops);
+			}
+			;
+
+limit_elem_stmt		:	limit_elem_stmt_alloc	limit_args
+			;
+
+quota_elem_stmt_alloc	:	QUOTA
+			{
+				$$ = elem_stmt_alloc(&@$, &quota_stmt_ops);
+			}
+			;
+
+quota_elem_stmt		:	quota_elem_stmt_alloc	quota_args
+			;
+
+ct_limit_elem_stmt_alloc:	CT	COUNT
+			{
+				$$ = elem_stmt_alloc(&@$, &connlimit_stmt_ops);
+			}
+			;
+
+connlimit_elem_stmt	:	ct_limit_elem_stmt_alloc	ct_limit_args
+			;
+
+last_elem_stmt_alloc	:	LAST
+			{
+				$$ = elem_stmt_alloc(&@$, &last_stmt_ops);
+			}
+			;
+
+last_elem_stmt		:	last_elem_stmt_alloc
+			|	last_elem_stmt_alloc	last_args
 			;
 
 set_elem_expr_option	:	TIMEOUT		set_elem_time_spec
diff --git a/src/statement.c b/src/statement.c
index 695b57a6cc65..ed2e37a32566 100644
--- a/src/statement.c
+++ b/src/statement.c
@@ -74,6 +74,23 @@ void stmt_print(const struct stmt *stmt, struct output_ctx *octx)
 	ops->print(stmt, octx);
 }
 
+struct elem_stmt *elem_stmt_alloc(const struct location *loc,
+				  const struct stmt_ops *ops)
+{
+	struct elem_stmt *elem_stmt;
+
+	elem_stmt = xzalloc(sizeof(*elem_stmt) + ops->size);
+	init_list_head(&elem_stmt->list);
+	elem_stmt->location = *loc;
+	elem_stmt->type = ops->type;
+	return elem_stmt;
+}
+
+void elem_stmt_free(const struct elem_stmt *elem_stmt)
+{
+	stmt_free((struct stmt *)elem_stmt);
+}
+
 static void expr_stmt_print(const struct stmt *stmt, struct output_ctx *octx)
 {
 	expr_print(stmt->expr, octx);
@@ -210,9 +227,10 @@ static void connlimit_stmt_print(const struct stmt *stmt, struct output_ctx *oct
 		  stmt->connlimit.flags ? "over " : "", stmt->connlimit.count);
 }
 
-static const struct stmt_ops connlimit_stmt_ops = {
+const struct stmt_ops connlimit_stmt_ops = {
 	.type		= STMT_CONNLIMIT,
 	.name		= "connlimit",
+	.size		= sizeof(struct connlimit_stmt),
 	.print		= connlimit_stmt_print,
 	.json		= connlimit_stmt_json,
 };
@@ -237,9 +255,10 @@ static void counter_stmt_print(const struct stmt *stmt, struct output_ctx *octx)
 		  stmt->counter.packets, stmt->counter.bytes);
 }
 
-static const struct stmt_ops counter_stmt_ops = {
+const struct stmt_ops counter_stmt_ops = {
 	.type		= STMT_COUNTER,
 	.name		= "counter",
+	.size		= sizeof(struct counter_stmt),
 	.print		= counter_stmt_print,
 	.json		= counter_stmt_json,
 };
@@ -268,9 +287,10 @@ static void last_stmt_print(const struct stmt *stmt, struct output_ctx *octx)
 		nft_print(octx, "never");
 }
 
-static const struct stmt_ops last_stmt_ops = {
+const struct stmt_ops last_stmt_ops = {
 	.type		= STMT_LAST,
 	.name		= "last",
+	.size		= sizeof(struct last_stmt),
 	.print		= last_stmt_print,
 	.json		= last_stmt_json,
 };
@@ -505,9 +525,10 @@ static void limit_stmt_print(const struct stmt *stmt, struct output_ctx *octx)
 	}
 }
 
-static const struct stmt_ops limit_stmt_ops = {
+const struct stmt_ops limit_stmt_ops = {
 	.type		= STMT_LIMIT,
 	.name		= "limit",
+	.size		= sizeof(struct limit_stmt),
 	.print		= limit_stmt_print,
 	.json		= limit_stmt_json,
 };
@@ -584,9 +605,10 @@ static void quota_stmt_print(const struct stmt *stmt, struct output_ctx *octx)
 	}
 }
 
-static const struct stmt_ops quota_stmt_ops = {
+const struct stmt_ops quota_stmt_ops = {
 	.type		= STMT_QUOTA,
 	.name		= "quota",
+	.size		= sizeof(struct quota_stmt),
 	.print		= quota_stmt_print,
 	.json		= quota_stmt_json,
 };
@@ -1085,6 +1107,32 @@ struct stmt *synproxy_stmt_alloc(const struct location *loc)
 	return stmt_alloc(loc, &synproxy_stmt_ops);
 }
 
+static const struct stmt_ops *__elem_stmt_ops_by_type(enum stmt_types type)
+{
+	switch (type) {
+	case STMT_COUNTER: return &counter_stmt_ops;
+	case STMT_LIMIT: return &limit_stmt_ops;
+	case STMT_QUOTA: return &quota_stmt_ops;
+	case STMT_CONNLIMIT: return &connlimit_stmt_ops;
+	case STMT_LAST: return &last_stmt_ops;
+	default:
+		break;
+	}
+
+	return NULL;
+}
+
+const struct stmt_ops *elem_stmt_ops(const struct elem_stmt *stmt)
+{
+	const struct stmt_ops *ops;
+
+	ops = __elem_stmt_ops_by_type(stmt->type);
+	if (!ops)
+		BUG("Unknown element statement type %d\n", stmt->type);
+
+	return ops;
+}
+
 /* For src/optimize.c */
 static struct stmt_ops invalid_stmt_ops = {
 	.type	= STMT_INVALID,
-- 
2.30.2