evaluate: don't assert on net/transport header conflict

before:
nft: evaluate.c:467: conflict_resolution_gen_dependency: Assertion `expr->payload.base == PROTO_BASE_LL_HDR' failed.
Aborted (core dumped)

conflict_resolution_gen_dependency() can only handle linklayer
conflicts, hence the assert.

Rename it accordingly.  Also rename resolve_protocol_conflict, it doesn't
do anything for != PROTO_BASE_LL_HDR and extend the assertion to that
function too.

Callers now enforce PROTO_BASE_LL_HDR prerequisite.

after:
Error: conflicting transport layer protocols specified: comp vs. udp
 ip6 nexthdr comp udp dport 4789
                  ^^^^^^^^^

Signed-off-by: Florian Westphal <fw@strlen.de>

diff --git a/src/evaluate.c b/src/evaluate.c
index 68cfd77653813fe23b0110ca5a6ca716641e2bcb..89411fa8c7b4b33d369277a0ddef2fcdf57acfbf 100644 (file)
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -499,9 +499,9 @@ int stmt_dependency_evaluate(struct eval_ctx *ctx, struct stmt *stmt)
 }
 
 static int
-conflict_resolution_gen_dependency(struct eval_ctx *ctx, int protocol,
-                                  const struct expr *expr,
-                                  struct stmt **res)
+ll_conflict_resolution_gen_dependency(struct eval_ctx *ctx, int protocol,
+                                     const struct expr *expr,
+                                     struct stmt **res)
 {
        enum proto_bases base = expr->payload.base;
        const struct proto_hdr_template *tmpl;
@@ -764,56 +764,52 @@ static bool proto_is_dummy(const struct proto_desc *desc)
        return desc == &proto_inet || desc == &proto_netdev;
 }
 
-static int resolve_protocol_conflict(struct eval_ctx *ctx,
-                                    const struct proto_desc *desc,
-                                    struct expr *payload)
+static int resolve_ll_protocol_conflict(struct eval_ctx *ctx,
+                                       const struct proto_desc *desc,
+                                       struct expr *payload)
 {
        enum proto_bases base = payload->payload.base;
        struct stmt *nstmt = NULL;
        struct proto_ctx *pctx;
+       unsigned int i;
        int link, err;
 
+       assert(base == PROTO_BASE_LL_HDR);
+
        pctx = eval_proto_ctx(ctx);
 
-       if (payload->payload.base == PROTO_BASE_LL_HDR) {
-               if (proto_is_dummy(desc)) {
-                       if (ctx->inner_desc) {
-                               proto_ctx_update(pctx, PROTO_BASE_LL_HDR, &payload->location, &proto_eth);
-                       } else {
-                               err = meta_iiftype_gen_dependency(ctx, payload, &nstmt);
-                               if (err < 0)
-                                       return err;
-
-                               desc = payload->payload.desc;
-                               rule_stmt_insert_at(ctx->rule, nstmt, ctx->stmt);
-                       }
+       if (proto_is_dummy(desc)) {
+               if (ctx->inner_desc) {
+                       proto_ctx_update(pctx, PROTO_BASE_LL_HDR, &payload->location, &proto_eth);
                } else {
-                       unsigned int i;
+                       err = meta_iiftype_gen_dependency(ctx, payload, &nstmt);
+                       if (err < 0)
+                               return err;
 
-                       /* payload desc stored in the L2 header stack? No conflict. */
-                       for (i = 0; i < pctx->stacked_ll_count; i++) {
-                               if (pctx->stacked_ll[i] == payload->payload.desc)
-                                       return 0;
-                       }
+                       desc = payload->payload.desc;
+                       rule_stmt_insert_at(ctx->rule, nstmt, ctx->stmt);
+               }
+       } else {
+               unsigned int i;
+
+               /* payload desc stored in the L2 header stack? No conflict. */
+               for (i = 0; i < pctx->stacked_ll_count; i++) {
+                       if (pctx->stacked_ll[i] == payload->payload.desc)
+                               return 0;
                }
        }
 
-       assert(base <= PROTO_BASE_MAX);
        /* This payload and the existing context don't match, conflict. */
        if (pctx->protocol[base + 1].desc != NULL)
                return 1;
 
        link = proto_find_num(desc, payload->payload.desc);
        if (link < 0 ||
-           conflict_resolution_gen_dependency(ctx, link, payload, &nstmt) < 0)
+           ll_conflict_resolution_gen_dependency(ctx, link, payload, &nstmt) < 0)
                return 1;
 
-       if (base == PROTO_BASE_LL_HDR) {
-               unsigned int i;
-
-               for (i = 0; i < pctx->stacked_ll_count; i++)
-                       payload->payload.offset += pctx->stacked_ll[i]->length;
-       }
+       for (i = 0; i < pctx->stacked_ll_count; i++)
+               payload->payload.offset += pctx->stacked_ll[i]->length;
 
        rule_stmt_insert_at(ctx->rule, nstmt, ctx->stmt);
 
@@ -855,7 +851,7 @@ static int __expr_evaluate_payload(struct eval_ctx *ctx, struct expr *expr)
 
                        link = proto_find_num(desc, payload->payload.desc);
                        if (link < 0 ||
-                           conflict_resolution_gen_dependency(ctx, link, payload, &nstmt) < 0)
+                           ll_conflict_resolution_gen_dependency(ctx, link, payload, &nstmt) < 0)
                                return expr_error(ctx->msgs, payload,
                                                  "conflicting protocols specified: %s vs. %s",
                                                  desc->name,
@@ -912,8 +908,8 @@ check_icmp:
        /* If we already have context and this payload is on the same
         * base, try to resolve the protocol conflict.
         */
-       if (payload->payload.base == desc->base) {
-               err = resolve_protocol_conflict(ctx, desc, payload);
+       if (base == PROTO_BASE_LL_HDR) {
+               err = resolve_ll_protocol_conflict(ctx, desc, payload);
                if (err <= 0)
                        return err;
 
@@ -922,7 +918,8 @@ check_icmp:
                        return 0;
        }
        return expr_error(ctx->msgs, payload,
-                         "conflicting protocols specified: %s vs. %s",
+                         "conflicting %s protocols specified: %s vs. %s",
+                         proto_base_names[base],
                          pctx->protocol[base].desc->name,
                          payload->payload.desc->name);
 }

diff --git a/tests/shell/testcases/bogons/nft-f/evaluate_conflict_resolution_gen_dependency_base_ll_hdr_assert b/tests/shell/testcases/bogons/nft-f/evaluate_conflict_resolution_gen_dependency_base_ll_hdr_assert
new file mode 100644 (file)
index 0000000..43d72c4
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/evaluate_conflict_resolution_gen_dependency_base_ll_hdr_assert
@@ -0,0 +1,5 @@
+table ip6 t {
+       chain c {
+               ip6 nexthdr comp udp dport 4789
+       }
+}