src: expand table command before evaluation

The nested syntax notation results in one single table command which
includes all other objects. This differs from the flat notation where
there is usually one command per object.

This patch adds a previous step to the evaluation phase to expand the
objects that are contained in the table into independent commands, so
both notations have similar representations.

Remove the code to evaluate the nested representation in the evaluation
phase since commands are independently evaluated after the expansion.

The commands are expanded after the set element collapse step, in case
that there is a long list of singleton element commands to be added to
the set, to shorten the command list iteration.

This approach also avoids interference with the object cache that is
populated in the evaluation, which might refer to objects coming in the
existing command list that is being processed.

There is still a post_expand phase to detach the elements from the set
which could be consolidated by updating the evaluation step to handle
the CMD_OBJ_SETELEMS command type.

This patch fixes 27c753e4a8d4 ("rule: expand standalone chain that
contains rules") which broke rule addition/insertion by index because
the expansion code after the evaluation messes up the cache.

Fixes: 27c753e4a8d4 ("rule: expand standalone chain that contains rules")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/include/rule.h b/include/rule.h
index 0828d16122aca7a024ead11f7d410de22fb2afc6..f3db6aabf1ffa0bdad48f73a9bd5afbc076aa785 100644 (file)
--- a/include/rule.h
+++ b/include/rule.h
@@ -736,6 +736,7 @@ extern struct cmd *cmd_alloc(enum cmd_ops op, enum cmd_obj obj,
                             const struct handle *h, const struct location *loc,
                             void *data);
 extern void nft_cmd_expand(struct cmd *cmd);
+extern void nft_cmd_post_expand(struct cmd *cmd);
 extern bool nft_cmd_collapse(struct list_head *cmds);
 extern void nft_cmd_uncollapse(struct list_head *cmds);
 extern struct cmd *cmd_alloc_obj_ct(enum cmd_ops op, int type,

diff --git a/src/evaluate.c b/src/evaluate.c
index d24f8b66b0de8df6ad41afb0c10eecb0b246aea1..506c2414b9e86fb6c5890e82679950e8ead9f7ee 100644 (file)
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -4821,7 +4821,6 @@ static uint32_t str2hooknum(uint32_t family, const char *hook)
 static int chain_evaluate(struct eval_ctx *ctx, struct chain *chain)
 {
        struct table *table;
-       struct rule *rule;
 
        table = table_cache_find(&ctx->nft->cache.table_cache,
                                 ctx->cmd->handle.table.name,
@@ -4877,11 +4876,6 @@ static int chain_evaluate(struct eval_ctx *ctx, struct chain *chain)
                }
        }
 
-       list_for_each_entry(rule, &chain->rules, list) {
-               handle_merge(&rule->handle, &chain->handle);
-               if (rule_evaluate(ctx, rule, CMD_INVALID) < 0)
-                       return -1;
-       }
        return 0;
 }
 
@@ -4949,11 +4943,6 @@ static int obj_evaluate(struct eval_ctx *ctx, struct obj *obj)
 
 static int table_evaluate(struct eval_ctx *ctx, struct table *table)
 {
-       struct flowtable *ft;
-       struct chain *chain;
-       struct set *set;
-       struct obj *obj;
-
        if (!table_cache_find(&ctx->nft->cache.table_cache,
                              ctx->cmd->handle.table.name,
                              ctx->cmd->handle.family)) {
@@ -4966,34 +4955,6 @@ static int table_evaluate(struct eval_ctx *ctx, struct table *table)
                }
        }
 
-       if (ctx->cmd->table == NULL)
-               return 0;
-
-       ctx->table = table;
-       list_for_each_entry(set, &table->sets, list) {
-               expr_set_context(&ctx->ectx, NULL, 0);
-               handle_merge(&set->handle, &table->handle);
-               if (set_evaluate(ctx, set) < 0)
-                       return -1;
-       }
-       list_for_each_entry(chain, &table->chains, list) {
-               handle_merge(&chain->handle, &table->handle);
-               ctx->cmd->handle.chain.location = chain->location;
-               if (chain_evaluate(ctx, chain) < 0)
-                       return -1;
-       }
-       list_for_each_entry(ft, &table->flowtables, list) {
-               handle_merge(&ft->handle, &table->handle);
-               if (flowtable_evaluate(ctx, ft) < 0)
-                       return -1;
-       }
-       list_for_each_entry(obj, &table->objs, list) {
-               handle_merge(&obj->handle, &table->handle);
-               if (obj_evaluate(ctx, obj) < 0)
-                       return -1;
-       }
-
-       ctx->table = NULL;
        return 0;
 }
 

diff --git a/src/libnftables.c b/src/libnftables.c
index ec01a427d6cc147292dc6ba81ef76b110426b30b..4f538c44b998a6868a1c62630751c15359b2f6da 100644 (file)
--- a/src/libnftables.c
+++ b/src/libnftables.c
@@ -519,6 +519,13 @@ static int nft_evaluate(struct nft_ctx *nft, struct list_head *msgs,
        if (nft_cmd_collapse(cmds))
                collapsed = true;
 
+       list_for_each_entry(cmd, cmds, list) {
+               if (cmd->op != CMD_ADD)
+                       continue;
+
+               nft_cmd_expand(cmd);
+       }
+
        list_for_each_entry_safe(cmd, next, cmds, list) {
                struct eval_ctx ectx = {
                        .nft    = nft,
@@ -542,7 +549,7 @@ static int nft_evaluate(struct nft_ctx *nft, struct list_head *msgs,
                if (cmd->op != CMD_ADD)
                        continue;
 
-               nft_cmd_expand(cmd);
+               nft_cmd_post_expand(cmd);
        }
 
        return 0;

diff --git a/src/rule.c b/src/rule.c
index c162059bfd9e5bda3a54e85bbc38740551eae0d6..047e405707f67bd265a201dc15cdd44c8a756358 100644 (file)
--- a/src/rule.c
+++ b/src/rule.c
@@ -1318,8 +1318,9 @@ static void nft_cmd_expand_chain(struct chain *chain, struct list_head *new_cmds
 
        list_for_each_entry_safe(rule, next, &chain->rules, list) {
                list_del(&rule->list);
+               handle_merge(&rule->handle, &chain->handle);
                memset(&h, 0, sizeof(h));
-               handle_merge(&h, &rule->handle);
+               handle_merge(&h, &chain->handle);
                if (chain->flags & CHAIN_F_BINDING) {
                        rule->handle.chain_id = chain->handle.chain_id;
                        rule->handle.chain.location = chain->location;
@@ -1350,6 +1351,7 @@ void nft_cmd_expand(struct cmd *cmd)
                        return;
 
                list_for_each_entry(chain, &table->chains, list) {
+                       handle_merge(&chain->handle, &table->handle);
                        memset(&h, 0, sizeof(h));
                        handle_merge(&h, &chain->handle);
                        h.chain_id = chain->handle.chain_id;
@@ -1394,6 +1396,21 @@ void nft_cmd_expand(struct cmd *cmd)
                nft_cmd_expand_chain(chain, &new_cmds);
                list_splice(&new_cmds, &cmd->list);
                break;
+       default:
+               break;
+       }
+}
+
+void nft_cmd_post_expand(struct cmd *cmd)
+{
+       struct list_head new_cmds;
+       struct set *set;
+       struct cmd *new;
+       struct handle h;
+
+       init_list_head(&new_cmds);
+
+       switch (cmd->obj) {
        case CMD_OBJ_SET:
        case CMD_OBJ_MAP:
                set = cmd->set;