From f211921e25e683eb0cbfac08fd0289a07b6d67d1 Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Wed, 5 Jun 2019 17:07:42 +0200
Subject: src: perform evaluation after parsing

Since 61236968b7a1 ("parser: evaluate commands immediately after
parsing"), evaluation is invoked from the parsing phase in order to
improve error reporting.

However, this approach is problematic from the cache perspective since
we don't know if a full or partial netlink dump from the kernel is
needed. If the number of objects in the kernel is significant, the
netlink dump operation to build the cache may significantly slow down
commands.

This patch moves the evaluation phase after the parsing phase as a
preparation update to allow for a better strategy to build the cache.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/parser.h   |  1 -
 src/libnftables.c  | 30 +++++++++++++++++++++++++-----
 src/parser_bison.y | 27 ++-------------------------
 3 files changed, 27 insertions(+), 31 deletions(-)

diff --git a/include/parser.h b/include/parser.h
index a5ae802b..39a75212 100644
--- a/include/parser.h
+++ b/include/parser.h
@@ -27,7 +27,6 @@ struct parser_state {
 	unsigned int			scope;
 
 	struct list_head		*cmds;
-	struct eval_ctx			ectx;
 };
 
 struct mnl_socket;
diff --git a/src/libnftables.c b/src/libnftables.c
index d8de89ca..8720fe2b 100644
--- a/src/libnftables.c
+++ b/src/libnftables.c
@@ -348,7 +348,6 @@ static const struct input_descriptor indesc_cmdline = {
 static int nft_parse_bison_buffer(struct nft_ctx *nft, const char *buf,
 				  struct list_head *msgs, struct list_head *cmds)
 {
-	struct cmd *cmd;
 	int ret;
 
 	parser_init(nft, nft->state, msgs, cmds);
@@ -359,16 +358,12 @@ static int nft_parse_bison_buffer(struct nft_ctx *nft, const char *buf,
 	if (ret != 0 || nft->state->nerrs > 0)
 		return -1;
 
-	list_for_each_entry(cmd, cmds, list)
-		nft_cmd_expand(cmd);
-
 	return 0;
 }
 
 static int nft_parse_bison_filename(struct nft_ctx *nft, const char *filename,
 				    struct list_head *msgs, struct list_head *cmds)
 {
-	struct cmd *cmd;
 	int ret;
 
 	parser_init(nft, nft->state, msgs, cmds);
@@ -380,6 +375,23 @@ static int nft_parse_bison_filename(struct nft_ctx *nft, const char *filename,
 	if (ret != 0 || nft->state->nerrs > 0)
 		return -1;
 
+	return 0;
+}
+
+static int nft_evaluate(struct nft_ctx *nft, struct list_head *msgs,
+			struct list_head *cmds)
+{
+	struct cmd *cmd;
+
+	list_for_each_entry(cmd, cmds, list) {
+		struct eval_ctx ectx = {
+			.nft	= nft,
+			.msgs	= msgs,
+		};
+		if (cmd_evaluate(&ectx, cmd) < 0)
+			return -1;
+	}
+
 	list_for_each_entry(cmd, cmds, list)
 		nft_cmd_expand(cmd);
 
@@ -404,6 +416,10 @@ int nft_run_cmd_from_buffer(struct nft_ctx *nft, const char *buf)
 	if (rc)
 		goto err;
 
+	rc = nft_evaluate(nft, &msgs, &cmds);
+	if (rc < 0)
+		goto err;
+
 	if (nft_netlink(nft, &cmds, &msgs, nft->nf_sock) != 0)
 		rc = -1;
 err:
@@ -448,6 +464,10 @@ int nft_run_cmd_from_filename(struct nft_ctx *nft, const char *filename)
 	if (rc)
 		goto err;
 
+	rc = nft_evaluate(nft, &msgs, &cmds);
+	if (rc < 0)
+		goto err;
+
 	if (nft_netlink(nft, &cmds, &msgs, nft->nf_sock) != 0)
 		rc = -1;
 err:
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 2a39db31..8026708e 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -48,8 +48,6 @@ void parser_init(struct nft_ctx *nft, struct parser_state *state,
 	state->msgs = msgs;
 	state->cmds = cmds;
 	state->scopes[0] = scope_init(&state->top_scope, NULL);
-	state->ectx.nft = nft;
-	state->ectx.msgs = msgs;
 	init_list_head(&state->indesc_list);
 }
 
@@ -792,17 +790,8 @@ input			:	/* empty */
 			|	input		line
 			{
 				if ($2 != NULL) {
-					LIST_HEAD(list);
-
 					$2->location = @2;
-
-					list_add_tail(&$2->list, &list);
-					if (cmd_evaluate(&state->ectx, $2) < 0) {
-						cmd_free($2);
-						if (++state->nerrs == nft->parser_max_errors)
-							YYABORT;
-					} else
-						list_splice_tail(&list, state->cmds);
+					list_add_tail(&$2->list, state->cmds);
 				}
 			}
 			;
@@ -878,22 +867,10 @@ line			:	common_block			{ $$ = NULL; }
 				 * work.
 				 */
 				if ($1 != NULL) {
-					LIST_HEAD(list);
-
 					$1->location = @1;
-
-					list_add_tail(&$1->list, &list);
-					if (cmd_evaluate(&state->ectx, $1) < 0) {
-						cmd_free($1);
-						if (++state->nerrs == nft->parser_max_errors)
-							YYABORT;
-					} else
-						list_splice_tail(&list, state->cmds);
+					list_add_tail(&$1->list, state->cmds);
 				}
-				if (state->nerrs)
-					YYABORT;
 				$$ = NULL;
-
 				YYACCEPT;
 			}
 			;
-- 
cgit v1.2.3