On Sun, Jul 21, 2019 at 06:29:48PM +0200, Florian Westphal wrote: > nft dumps core when a statement contains a prefix expression: > iifname ens3 snat to 10.0.0.0/28 > > yields: > BUG: unknown expression type prefix > nft: netlink_linearize.c:688: netlink_gen_expr: Assertion `0' failed. > > This assertion is correct -- we can't linearize a prefix because kernel > doesn't know what that is. > > For LHS, they get converted to a binary 'and' such as > '10.0.0.0 & 255.255.255.240'. For RHS, we can convert them into a range: > > iifname "ens3" snat to 10.0.0.0-10.0.0.15 > > Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1187 > Signed-off-by: Florian Westphal <fw@xxxxxxxxx> > --- > src/evaluate.c | 61 +++++++++++++++++++++++++++++++++ > tests/py/ip6/dnat.t | 2 ++ > tests/py/ip6/dnat.t.json | 27 +++++++++++++++ > tests/py/ip6/dnat.t.payload.ip6 | 12 +++++++ > 4 files changed, 102 insertions(+) > > diff --git a/src/evaluate.c b/src/evaluate.c > index 8c1c82abed4e..d55fe8ebb78e 100644 > --- a/src/evaluate.c > +++ b/src/evaluate.c > @@ -1933,6 +1933,65 @@ static int stmt_evaluate_expr(struct eval_ctx *ctx, struct stmt *stmt) > return expr_evaluate(ctx, &stmt->expr); > } > > +static int stmt_prefix_conversion(struct eval_ctx *ctx, struct expr **expr, > + enum byteorder byteorder) > +{ > + struct expr *mask, *and, *or, *prefix, *base, *range; > + > + prefix = *expr; > + base = prefix->prefix; > + > + if (base->etype != EXPR_VALUE) > + return expr_error(ctx->msgs, prefix, > + "you cannot specify a prefix here, " > + "unknown type %s", base->dtype->name); > + > + if (!expr_is_constant(base)) > + return expr_error(ctx->msgs, prefix, > + "Prefix expression is undefined for " > + "non-constant expressions"); > + > + if (expr_basetype(base)->type != TYPE_INTEGER) > + return expr_error(ctx->msgs, prefix, > + "Prefix expression expected integer value"); > + > + mask = constant_expr_alloc(&prefix->location, expr_basetype(base), > + BYTEORDER_HOST_ENDIAN, base->len, NULL); > + > + mpz_prefixmask(mask->value, base->len, prefix->prefix_len); > + and = binop_expr_alloc(&prefix->location, OP_AND, expr_get(base), mask); > + > + if (expr_evaluate(ctx, &and) < 0) { I think we don't need this expr_evaluate() call. The one at the bottom of this function (for the range expression) should take care of this already since expr_evaluate() makes recursive calls to left and right hand side. > + expr_free(and); > + goto err_evaluation; > + } > + > + mask = constant_expr_alloc(&prefix->location, expr_basetype(base), > + BYTEORDER_HOST_ENDIAN, base->len, NULL); > + mpz_bitmask(mask->value, prefix->len - prefix->prefix_len); > + or = binop_expr_alloc(&prefix->location, OP_OR, expr_get(base), mask); > + > + if (expr_evaluate(ctx, &or) < 0) { Same here. > + expr_free(and); > + expr_free(or); > + goto err_evaluation; > + } > + > + range = range_expr_alloc(&prefix->location, and, or); > + if (expr_evaluate(ctx, &range) < 0) { > + expr_free(range); > + goto err_evaluation; > + } > + > + expr_free(*expr); > + *expr = range; > + return 0; > + > +err_evaluation: > + return expr_error(ctx->msgs, prefix, > + "Could not expand prefix expression"); If this happens, then expr_error() will reported via recursive call inside expr_evaluate(). I mean, expr_evaluate() itself should give us an error already. So I would remove this, which is already actually telling about an internal problem. The user cannot do much about this. So either remove it or BUG() here I'd suggest. Thanks!
