[PATCH nft 0/10] nftables remove use of meta nfproto

[PATCH nft 0/10] nftables remove use of meta nfproto

[Florian Westphal]

inet family (and others, e.g. bridge) lack context to figure
out the layer 3 address type.

examples:
ct original saddr $addr
rt nexthop $addr

We can't use $addr, because it might be a set reference, e.g.

ct original saddr @whitelist

currently implemented workaround is to use 'meta nfproto'
to provide the l3 context, e.g.

meta nfproto ip rt nexthop 10.2.3.4

i.e. users need to fill dependency manually.

Pablo suggested to instead specify ip saddr, ip6 saddr:

ct original ip saddr $address

and then let nft handle the dependency injection.

This series does just that.

Old syntax is preserved.

 doc/nft.xml                |   23 ++++++++---
 include/ct.h               |    3 -
 include/expression.h       |    2 
 src/ct.c                   |   70 ++++++++++++++++++++++++----------
 src/evaluate.c             |   92 +++++++++++++++++++++++++++++++++++----------
 src/netlink_delinearize.c  |   52 +++++++++++--------------
 src/parser_bison.y         |   42 ++++++++++++++++++--
 src/rt.c                   |   14 ++++++
 tests/py/bridge/icmpX.t    |    4 -
 tests/py/inet/ct.t         |    7 ++-
 tests/py/inet/ct.t.payload |    4 -
 tests/py/inet/icmpX.t      |    4 -
 tests/py/inet/rt.t         |   10 +++-
 tests/py/inet/rt.t.payload |    4 -
 tests/py/ip/ct.t           |   18 ++++----
 tests/py/ip/ct.t.payload   |   16 +++----
 tests/py/ip/rt.t           |    3 -
 tests/py/ip6/rt0.t         |    2 
 18 files changed, 260 insertions(+), 110 deletions(-)

[PATCH nft 01/10] src: add alternate syntax for ct saddr

[Florian Westphal]

current syntax is:

ct original saddr $address

problem is that in inet, bridge etc. we lack context to
figure out if this should fetch ipv6 or ipv4 from the conntrack
structure.

$address might not exist, rhs could e.g. be a set reference.

One way to do this is to have users manually specifiy the dependeny:

ct l3proto ipv4 ct original saddr $address

Thats ugly, and, moreover, only needed for table families
other than ip or ipv6.

Pablo suggested to instead specify ip saddr, ip6 saddr:

ct original ip saddr $address

and let nft handle the dependency injection.

This adds the required parts to the scanner and the grammar, next
commit adds code to eval step to make use of this.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/ct.h              |  3 ++-
 include/expression.h      |  1 +
 src/ct.c                  |  3 ++-
 src/netlink_delinearize.c |  2 +-
 src/parser_bison.y        | 18 +++++++++++++++---
 5 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/include/ct.h b/include/ct.h
index d9a11a3fab81..ec5d55d85dd0 100644
--- a/include/ct.h
+++ b/include/ct.h
@@ -24,7 +24,8 @@ struct ct_template {
 }
 
 extern struct expr *ct_expr_alloc(const struct location *loc,
-				  enum nft_ct_keys key, int8_t direction);
+				  enum nft_ct_keys key, int8_t direction,
+				  uint8_t nfproto);
 extern void ct_expr_update_type(struct proto_ctx *ctx, struct expr *expr);
 
 extern struct stmt *notrack_stmt_alloc(const struct location *loc);
diff --git a/include/expression.h b/include/expression.h
index 32d4423a5e06..90caaf51c36a 100644
--- a/include/expression.h
+++ b/include/expression.h
@@ -301,6 +301,7 @@ struct expr {
 			/* EXPR_CT */
 			enum nft_ct_keys	key;
 			int8_t			direction;
+			uint8_t			nfproto;
 		} ct;
 		struct {
 			/* EXPR_NUMGEN */
diff --git a/src/ct.c b/src/ct.c
index 0e9b17cd1a22..2b2578732066 100644
--- a/src/ct.c
+++ b/src/ct.c
@@ -335,7 +335,7 @@ static const struct expr_ops ct_expr_ops = {
 };
 
 struct expr *ct_expr_alloc(const struct location *loc, enum nft_ct_keys key,
-			   int8_t direction)
+			   int8_t direction, uint8_t nfproto)
 {
 	const struct ct_template *tmpl = &ct_templates[key];
 	struct expr *expr;
@@ -344,6 +344,7 @@ struct expr *ct_expr_alloc(const struct location *loc, enum nft_ct_keys key,
 			  tmpl->byteorder, tmpl->len);
 	expr->ct.key = key;
 	expr->ct.direction = direction;
+	expr->ct.nfproto = nfproto;
 
 	switch (key) {
 	case NFT_CT_PROTOCOL:
diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 42206ebcfff2..7c61cd0c5bbc 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -716,7 +716,7 @@ static void netlink_parse_ct_expr(struct netlink_parse_ctx *ctx,
 		dir = nftnl_expr_get_u8(nle, NFTNL_EXPR_CT_DIR);
 
 	key  = nftnl_expr_get_u32(nle, NFTNL_EXPR_CT_KEY);
-	expr = ct_expr_alloc(loc, key, dir);
+	expr = ct_expr_alloc(loc, key, dir, NFPROTO_UNSPEC);
 
 	dreg = netlink_parse_register(nle, NFTNL_EXPR_CT_DREG);
 	netlink_set_register(ctx, dreg, expr);
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 11f4716aa06a..98df4bd142d4 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -668,7 +668,7 @@ static void location_update(struct location *loc, struct location *rhs, int n)
 
 %type <expr>			ct_expr
 %destructor { expr_free($$); }	ct_expr
-%type <val>			ct_key		ct_dir	ct_key_dir_optional	ct_key_dir
+%type <val>			ct_key		ct_dir	ct_key_dir_optional	ct_key_dir	ct_key_proto	ct_key_proto_field
 
 %type <expr>			fib_expr
 %destructor { expr_free($$); }	fib_expr
@@ -3258,11 +3258,15 @@ rt_key			:	CLASSID		{ $$ = NFT_RT_CLASSID; }
 
 ct_expr			: 	CT	ct_key
 			{
-				$$ = ct_expr_alloc(&@$, $2, -1);
+				$$ = ct_expr_alloc(&@$, $2, -1, NFPROTO_UNSPEC);
 			}
 			|	CT	ct_dir	ct_key_dir
 			{
-				$$ = ct_expr_alloc(&@$, $3, $2);
+				$$ = ct_expr_alloc(&@$, $3, $2, NFPROTO_UNSPEC);
+			}
+			|	CT	ct_dir	ct_key_proto ct_key_proto_field
+			{
+				$$ = ct_expr_alloc(&@$, $4, $2, $3);
 			}
 			;
 
@@ -3296,6 +3300,14 @@ ct_key_dir		:	SADDR		{ $$ = NFT_CT_SRC; }
 			|	ct_key_dir_optional
 			;
 
+ct_key_proto		:	IP		{ $$ = NFPROTO_IPV4; }
+			|	IP6		{ $$ = NFPROTO_IPV6; }
+			;
+
+ct_key_proto_field	:	SADDR		{ $$ = NFT_CT_SRC; }
+			|	DADDR		{ $$ = NFT_CT_DST; }
+			;
+
 ct_key_dir_optional	:	BYTES		{ $$ = NFT_CT_BYTES; }
 			|	PACKETS		{ $$ = NFT_CT_PKTS; }
 			|	AVGPKT		{ $$ = NFT_CT_AVGPKT; }
-- 
2.13.5

[PATCH nft 02/10] src: ct: store proto base of ct key, if any

[Florian Westphal]

ct keys can match on network and tranasport header protocol
elements, such as port numbers or ip addresses.

Store this base type so a followup commit can store and kill
dependencies, e.g. if bsae is network header we might be able
to kill an earlier expression because the dependency is implicit.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/expression.h |  1 +
 src/ct.c             | 13 +++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/include/expression.h b/include/expression.h
index 90caaf51c36a..5b34d13d8527 100644
--- a/include/expression.h
+++ b/include/expression.h
@@ -300,6 +300,7 @@ struct expr {
 		struct {
 			/* EXPR_CT */
 			enum nft_ct_keys	key;
+			enum proto_bases	base;
 			int8_t			direction;
 			uint8_t			nfproto;
 		} ct;
diff --git a/src/ct.c b/src/ct.c
index 2b2578732066..99e4430bf173 100644
--- a/src/ct.c
+++ b/src/ct.c
@@ -347,8 +347,21 @@ struct expr *ct_expr_alloc(const struct location *loc, enum nft_ct_keys key,
 	expr->ct.nfproto = nfproto;
 
 	switch (key) {
+	case NFT_CT_SRC:
+	case NFT_CT_DST:
+		expr->ct.base = PROTO_BASE_NETWORK_HDR;
+		break;
+	case NFT_CT_PROTO_SRC:
+	case NFT_CT_PROTO_DST:
+		expr->ct.base = PROTO_BASE_TRANSPORT_HDR;
+		break;
 	case NFT_CT_PROTOCOL:
 		expr->flags = EXPR_F_PROTOCOL;
+		expr->ct.base = PROTO_BASE_NETWORK_HDR;
+		break;
+	case NFT_CT_L3PROTOCOL:
+		expr->flags = EXPR_F_PROTOCOL;
+		expr->ct.base = PROTO_BASE_LL_HDR;
 		break;
 	default:
 		break;
-- 
2.13.5

[PATCH nft 03/10] src: ct: add eval part to inject dependencies for ct saddr/daddr

[Florian Westphal]

nft has enough context to determine if a dependeny is needed.

add rule ip filter ct original ip6 saddr
allows nft to generate an error due to conflicting bases (ip vs ip6).

add rule inet filter ct original ip6 saddr
allows nft to inject an ipv6 dependency expression.

add rule inet filter ct original saddr
will print an error and will suggest to add ip/ip6 keyword.

Delinerize and print support will be added in followup patches.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/ct.c       | 26 +++++++++++-----------
 src/evaluate.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 77 insertions(+), 19 deletions(-)

diff --git a/src/ct.c b/src/ct.c
index 99e4430bf173..e8298975e692 100644
--- a/src/ct.c
+++ b/src/ct.c
@@ -308,21 +308,21 @@ static void ct_expr_clone(struct expr *new, const struct expr *expr)
 static void ct_expr_pctx_update(struct proto_ctx *ctx, const struct expr *expr)
 {
 	const struct expr *left = expr->left, *right = expr->right;
-	const struct proto_desc *base, *desc;
+	const struct proto_desc *base = NULL, *desc;
+	uint32_t nhproto;
 
 	assert(expr->op == OP_EQ);
 
-	switch (left->ct.key) {
-	case NFT_CT_PROTOCOL:
-		base = ctx->protocol[PROTO_BASE_NETWORK_HDR].desc;
-		desc = proto_find_upper(base, mpz_get_uint32(right->value));
+	nhproto = mpz_get_uint32(right->value);
 
-		proto_ctx_update(ctx, PROTO_BASE_TRANSPORT_HDR,
-				 &expr->location, desc);
-		break;
-	default:
-		break;
-	}
+	base = ctx->protocol[left->ct.base].desc;
+	if (!base)
+		return;
+	desc = proto_find_upper(base, nhproto);
+	if (!desc)
+		return;
+
+	proto_ctx_update(ctx, left->ct.base + 1, &expr->location, desc);
 }
 
 static const struct expr_ops ct_expr_ops = {
@@ -374,10 +374,11 @@ void ct_expr_update_type(struct proto_ctx *ctx, struct expr *expr)
 {
 	const struct proto_desc *desc;
 
+	desc = ctx->protocol[expr->ct.base].desc;
+
 	switch (expr->ct.key) {
 	case NFT_CT_SRC:
 	case NFT_CT_DST:
-		desc = ctx->protocol[PROTO_BASE_NETWORK_HDR].desc;
 		if (desc == &proto_ip)
 			expr->dtype = &ipaddr_type;
 		else if (desc == &proto_ip6)
@@ -387,7 +388,6 @@ void ct_expr_update_type(struct proto_ctx *ctx, struct expr *expr)
 		break;
 	case NFT_CT_PROTO_SRC:
 	case NFT_CT_PROTO_DST:
-		desc = ctx->protocol[PROTO_BASE_TRANSPORT_HDR].desc;
 		if (desc == NULL)
 			break;
 		expr->dtype = &inet_service_type;
diff --git a/src/evaluate.c b/src/evaluate.c
index 836c95288fda..d379a07ec000 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -696,27 +696,85 @@ static int expr_evaluate_rt(struct eval_ctx *ctx, struct expr **expr)
 	return expr_evaluate_primary(ctx, expr);
 }
 
+static int ct_gen_nh_dependency(struct eval_ctx *ctx, struct expr *ct)
+{
+	const struct proto_desc *base, *base_now;
+	struct expr *left, *right, *dep;
+	struct stmt *nstmt = NULL;
+
+	base_now = ctx->pctx.protocol[PROTO_BASE_NETWORK_HDR].desc;
+
+	switch (ct->ct.nfproto) {
+	case NFPROTO_IPV4:
+		base = &proto_ip;
+		break;
+	case NFPROTO_IPV6:
+		base = &proto_ip6;
+		break;
+	default:
+		base = ctx->pctx.protocol[PROTO_BASE_NETWORK_HDR].desc;
+		if (base == &proto_ip)
+			ct->ct.nfproto = NFPROTO_IPV4;
+		else if (base == &proto_ip)
+			ct->ct.nfproto = NFPROTO_IPV6;
+
+		if (base)
+			break;
+
+		return expr_error(ctx->msgs, ct,
+				  "cannot determine ip protocol version, use \"ip %1$caddr\" or \"ip6 %1$caddr\" instead",
+				  ct->ct.key == NFT_CT_SRC ? 's' : 'd');
+	}
+
+	/* no additional dependency needed? */
+	if (base == base_now)
+		return 0;
+
+	if (base_now && base_now != base)
+		return expr_error(ctx->msgs, ct,
+				  "conflicting dependencies: %s vs. %s\n",
+				  base->name,
+				  ctx->pctx.protocol[PROTO_BASE_NETWORK_HDR].desc->name);
+	switch (ctx->pctx.family) {
+	case NFPROTO_IPV4:
+	case NFPROTO_IPV6:
+		return 0;
+	}
+
+	left = ct_expr_alloc(&ct->location, NFT_CT_L3PROTOCOL, ct->ct.direction, ct->ct.nfproto);
+
+	right = constant_expr_alloc(&ct->location, left->dtype,
+				    left->dtype->byteorder, left->len,
+				    constant_data_ptr(ct->ct.nfproto, left->len));
+	dep = relational_expr_alloc(&ct->location, OP_EQ, left, right);
+
+	left->ops->pctx_update(&ctx->pctx, dep);
+
+	nstmt = expr_stmt_alloc(&dep->location, dep);
+
+	list_add_tail(&nstmt->list, &ctx->stmt->list);
+	return 0;
+}
+
 /*
  * CT expression: update the protocol dependant types bases on the protocol
  * context.
  */
 static int expr_evaluate_ct(struct eval_ctx *ctx, struct expr **expr)
 {
-	const struct proto_desc *base;
 	struct expr *ct = *expr;
 
-	ct_expr_update_type(&ctx->pctx, ct);
-
-	base = ctx->pctx.protocol[PROTO_BASE_NETWORK_HDR].desc;
 	switch (ct->ct.key) {
 	case NFT_CT_SRC:
 	case NFT_CT_DST:
-		if (base != &proto_ip && base != &proto_ip6)
-			return expr_error_base(ctx->msgs, ct);
+		ct_gen_nh_dependency(ctx, ct);
+		break;
 	default:
 		break;
 	}
 
+	ct_expr_update_type(&ctx->pctx, ct);
+
 	return expr_evaluate_primary(ctx, expr);
 }
 
-- 
2.13.5

[PATCH nft 04/10] src: unifiy meta and ct postprocessing

[Florian Westphal]

>From postprocess point of view meta and ct are logically the same,
except that their storage area overlaps (union type), so if we
extract the relevant fields we can move all of it into a single
helper and support dependency store/kill for both expressions.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/netlink_delinearize.c | 50 +++++++++++++++++++++--------------------------
 1 file changed, 22 insertions(+), 28 deletions(-)

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 7c61cd0c5bbc..44328879ebb8 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -1385,12 +1385,29 @@ static void payload_match_postprocess(struct rule_pp_ctx *ctx,
 	}
 }
 
-static void ct_meta_common_postprocess(const struct expr *expr)
+static void ct_meta_common_postprocess(struct rule_pp_ctx *ctx,
+				       const struct expr *expr,
+				       enum proto_bases base)
 {
 	const struct expr *left = expr->left;
 	struct expr *right = expr->right;
 
 	switch (expr->op) {
+	case OP_EQ:
+		if (expr->right->ops->type == EXPR_RANGE)
+			break;
+
+		expr->left->ops->pctx_update(&ctx->pctx, expr);
+
+		if (ctx->pdctx.pbase == PROTO_BASE_INVALID &&
+		    left->flags & EXPR_F_PROTOCOL) {
+			payload_dependency_store(&ctx->pdctx, ctx->stmt, base);
+		} else if (ctx->pdctx.pbase < PROTO_BASE_TRANSPORT_HDR) {
+			__payload_dependency_kill(&ctx->pdctx, base);
+			if (left->flags & EXPR_F_PROTOCOL)
+				payload_dependency_store(&ctx->pdctx, ctx->stmt, base);
+		}
+		break;
 	case OP_NEQ:
 		if (right->ops->type != EXPR_SET && right->ops->type != EXPR_SET_REF)
 			break;
@@ -1406,40 +1423,17 @@ static void ct_meta_common_postprocess(const struct expr *expr)
 static void meta_match_postprocess(struct rule_pp_ctx *ctx,
 				   const struct expr *expr)
 {
-	struct expr *left = expr->left;
-
-	switch (expr->op) {
-	case OP_EQ:
-		if (expr->right->ops->type == EXPR_RANGE)
-			break;
-
-		expr->left->ops->pctx_update(&ctx->pctx, expr);
+	const struct expr *left = expr->left;
 
-		if (ctx->pdctx.pbase == PROTO_BASE_INVALID &&
-		    left->flags & EXPR_F_PROTOCOL)
-			payload_dependency_store(&ctx->pdctx, ctx->stmt,
-						 left->meta.base);
-		break;
-	default:
-		ct_meta_common_postprocess(expr);
-		break;
-	}
+	ct_meta_common_postprocess(ctx, expr, left->meta.base);
 }
 
 static void ct_match_postprocess(struct rule_pp_ctx *ctx,
 				 const struct expr *expr)
 {
-	switch (expr->op) {
-	case OP_EQ:
-		if (expr->right->ops->type == EXPR_RANGE)
-			break;
+	const struct expr *left = expr->left;
 
-		expr->left->ops->pctx_update(&ctx->pctx, expr);
-		break;
-	default:
-		ct_meta_common_postprocess(expr);
-		break;
-	}
+	ct_meta_common_postprocess(ctx, expr, left->ct.base);
 }
 
 /* Convert a bitmask to a prefix length */
-- 
2.13.5

[PATCH nft 05/10] tests: update inet/bridge icmp test case

[Florian Westphal]

after previous change nft now culls the dependency chain:
'icmpv6 type echo-request' is shown as-is, and not
'meta nfproto ipv6 meta l4proto 58 icmpv6 type echo-request' anymore.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 tests/py/bridge/icmpX.t | 4 ++--
 tests/py/inet/icmpX.t   | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/py/bridge/icmpX.t b/tests/py/bridge/icmpX.t
index 0a1019c3a309..8c0a59799871 100644
--- a/tests/py/bridge/icmpX.t
+++ b/tests/py/bridge/icmpX.t
@@ -3,6 +3,6 @@
 *bridge;test-bridge;input
 
 ip protocol icmp icmp type echo-request;ok;icmp type echo-request
-icmp type echo-request;ok;ether type ip meta l4proto 1 icmp type echo-request
+icmp type echo-request;ok
 ip6 nexthdr icmpv6 icmpv6 type echo-request;ok;icmpv6 type echo-request
-icmpv6 type echo-request;ok;ether type ip6 meta l4proto 58 icmpv6 type echo-request
+icmpv6 type echo-request;ok
diff --git a/tests/py/inet/icmpX.t b/tests/py/inet/icmpX.t
index 7617e701a6d7..1b467a18e764 100644
--- a/tests/py/inet/icmpX.t
+++ b/tests/py/inet/icmpX.t
@@ -3,6 +3,6 @@
 *inet;test-inet;input
 
 ip protocol icmp icmp type echo-request;ok;icmp type echo-request
-icmp type echo-request;ok;meta nfproto ipv4 meta l4proto 1 icmp type echo-request
+icmp type echo-request;ok
 ip6 nexthdr icmpv6 icmpv6 type echo-request;ok;icmpv6 type echo-request
-icmpv6 type echo-request;ok;meta nfproto ipv6 meta l4proto 58 icmpv6 type echo-request
+icmpv6 type echo-request;ok
-- 
2.13.5

[PATCH nft 06/10] src: ct: print nfproto name for some header fields

[Florian Westphal]

this prints "original saddr|daddr $NFPROTO" to make output
symmetric with the syntax that we now prefer on input side.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/ct.c | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/src/ct.c b/src/ct.c
index e8298975e692..de5b0b904cdf 100644
--- a/src/ct.c
+++ b/src/ct.c
@@ -16,6 +16,8 @@
 #include <inttypes.h>
 #include <string.h>
 
+#include <netinet/ip.h>
+#include <linux/netfilter.h>
 #include <linux/netfilter/nf_tables.h>
 #include <linux/netfilter/nf_conntrack_common.h>
 #include <linux/netfilter/nf_conntrack_tuple_common.h>
@@ -269,9 +271,10 @@ static const struct ct_template ct_templates[] = {
 					      BYTEORDER_HOST_ENDIAN, 32),
 };
 
-static void ct_print(enum nft_ct_keys key, int8_t dir)
+static void ct_print(enum nft_ct_keys key, int8_t dir, uint8_t nfproto)
 {
 	const struct symbolic_constant *s;
+	const struct proto_desc *desc;
 
 	printf("ct ");
 	if (dir < 0)
@@ -283,13 +286,25 @@ static void ct_print(enum nft_ct_keys key, int8_t dir)
 			break;
 		}
 	}
+
+	switch (key) {
+	case NFT_CT_SRC: /* fallthrough */
+	case NFT_CT_DST:
+		desc = proto_find_upper(&proto_inet, nfproto);
+		if (desc)
+			printf("%s ", desc->name);
+		break;
+	default:
+		break;
+	}
+
  done:
 	printf("%s", ct_templates[key].token);
 }
 
 static void ct_expr_print(const struct expr *expr, struct output_ctx *octx)
 {
-	ct_print(expr->ct.key, expr->ct.direction);
+	ct_print(expr->ct.key, expr->ct.direction, expr->ct.nfproto);
 }
 
 static bool ct_expr_cmp(const struct expr *e1, const struct expr *e2)
@@ -379,10 +394,13 @@ void ct_expr_update_type(struct proto_ctx *ctx, struct expr *expr)
 	switch (expr->ct.key) {
 	case NFT_CT_SRC:
 	case NFT_CT_DST:
-		if (desc == &proto_ip)
+		if (desc == &proto_ip) {
 			expr->dtype = &ipaddr_type;
-		else if (desc == &proto_ip6)
+			expr->ct.nfproto = NFPROTO_IPV4;
+		} else if (desc == &proto_ip6) {
 			expr->dtype = &ip6addr_type;
+			expr->ct.nfproto = NFPROTO_IPV6;
+		}
 
 		expr->len = expr->dtype->size;
 		break;
@@ -399,7 +417,7 @@ void ct_expr_update_type(struct proto_ctx *ctx, struct expr *expr)
 
 static void ct_stmt_print(const struct stmt *stmt, struct output_ctx *octx)
 {
-	ct_print(stmt->ct.key, stmt->ct.direction);
+	ct_print(stmt->ct.key, stmt->ct.direction, 0);
 	printf(" set ");
 	expr_print(stmt->ct.expr, octx);
 }
-- 
2.13.5

[PATCH nft 07/10] tests: ct: adjust test case commands

[Florian Westphal]

use 'ip saddr', 'ip6 saddr', etc.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 tests/py/inet/ct.t         |  7 +++++--
 tests/py/inet/ct.t.payload |  4 ++--
 tests/py/ip/ct.t           | 18 +++++++++---------
 tests/py/ip/ct.t.payload   | 16 ++++++++--------
 4 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/tests/py/inet/ct.t b/tests/py/inet/ct.t
index c56c3bc86151..1a656aa4375f 100644
--- a/tests/py/inet/ct.t
+++ b/tests/py/inet/ct.t
@@ -3,8 +3,11 @@
 
 *inet;test-inet;input
 
-meta nfproto ipv4 ct original saddr 1.2.3.4;ok
-meta nfproto ipv6 ct original saddr ::1;ok
+meta nfproto ipv4 ct original saddr 1.2.3.4;ok;ct original ip saddr 1.2.3.4
+ct original ip6 saddr ::1;ok
 
 # missing protocol context
 ct original saddr ::1;fail
+
+# wrong protocol context
+ct original ip saddr ::1;fail
diff --git a/tests/py/inet/ct.t.payload b/tests/py/inet/ct.t.payload
index 21c74581de3a..97128eccf540 100644
--- a/tests/py/inet/ct.t.payload
+++ b/tests/py/inet/ct.t.payload
@@ -5,9 +5,9 @@ ip test-ip4 output
   [ ct load src => reg 1 , dir original ]
   [ cmp eq reg 1 0x04030201 ]
 
-# meta nfproto ipv6 ct original saddr ::1
+# ct original ip6 saddr ::1
 inet test-inet input
-  [ meta load nfproto => reg 1 ]
+  [ ct load l3protocol => reg 1 , dir original ]
   [ cmp eq reg 1 0x0000000a ]
   [ ct load src => reg 1 , dir original ]
   [ cmp eq reg 1 0x00000000 0x00000000 0x00000000 0x01000000 ]
diff --git a/tests/py/ip/ct.t b/tests/py/ip/ct.t
index d0f16c51fa4c..d3247f79113f 100644
--- a/tests/py/ip/ct.t
+++ b/tests/py/ip/ct.t
@@ -2,16 +2,16 @@
 
 *ip;test-ip4;output
 
-ct original saddr 192.168.0.1;ok
-ct reply saddr 192.168.0.1;ok
-ct original daddr 192.168.0.1;ok
-ct reply daddr 192.168.0.1;ok
+ct original ip saddr 192.168.0.1;ok
+ct reply ip saddr 192.168.0.1;ok
+ct original ip daddr 192.168.0.1;ok
+ct reply ip daddr 192.168.0.1;ok
 
 # same, but with a netmask
-ct original saddr 192.168.1.0/24;ok
-ct reply saddr 192.168.1.0/24;ok
-ct original daddr 192.168.1.0/24;ok
-ct reply daddr 192.168.1.0/24;ok
+ct original ip saddr 192.168.1.0/24;ok
+ct reply ip saddr 192.168.1.0/24;ok
+ct original ip daddr 192.168.1.0/24;ok
+ct reply ip daddr 192.168.1.0/24;ok
 
 ct l3proto ipv4;ok
 ct l3proto foobar;fail
@@ -20,4 +20,4 @@ ct protocol 6 ct original proto-dst 22;ok
 ct original protocol 17 ct reply proto-src 53;ok;ct protocol 17 ct reply proto-src 53
 
 # wrong address family
-ct reply daddr dead::beef;fail
+ct reply ip daddr dead::beef;fail
diff --git a/tests/py/ip/ct.t.payload b/tests/py/ip/ct.t.payload
index 56633a248c8c..b7cd130dbea2 100644
--- a/tests/py/ip/ct.t.payload
+++ b/tests/py/ip/ct.t.payload
@@ -1,42 +1,42 @@
-# ct original saddr 192.168.0.1
+# ct original ip saddr 192.168.0.1
 ip test-ip4 output
   [ ct load src => reg 1 , dir original ]
   [ cmp eq reg 1 0x0100a8c0 ]
 
-# ct reply saddr 192.168.0.1
+# ct reply ip saddr 192.168.0.1
 ip test-ip4 output
   [ ct load src => reg 1 , dir reply ]
   [ cmp eq reg 1 0x0100a8c0 ]
 
-# ct original daddr 192.168.0.1
+# ct original ip daddr 192.168.0.1
 ip test-ip4 output
   [ ct load dst => reg 1 , dir original ]
   [ cmp eq reg 1 0x0100a8c0 ]
 
-# ct reply daddr 192.168.0.1
+# ct reply ip daddr 192.168.0.1
 ip test-ip4 output
   [ ct load dst => reg 1 , dir reply ]
   [ cmp eq reg 1 0x0100a8c0 ]
 
-# ct original saddr 192.168.1.0/24
+# ct original ip saddr 192.168.1.0/24
 ip test-ip4 output
   [ ct load src => reg 1 , dir original ]
   [ bitwise reg 1 = (reg=1 & 0x00ffffff ) ^ 0x00000000 ]
   [ cmp eq reg 1 0x0001a8c0 ]
 
-# ct reply saddr 192.168.1.0/24
+# ct reply ip saddr 192.168.1.0/24
 ip test-ip4 output
   [ ct load src => reg 1 , dir reply ]
   [ bitwise reg 1 = (reg=1 & 0x00ffffff ) ^ 0x00000000 ]
   [ cmp eq reg 1 0x0001a8c0 ]
 
-# ct original daddr 192.168.1.0/24
+# ct original ip daddr 192.168.1.0/24
 ip test-ip4 output
   [ ct load dst => reg 1 , dir original ]
   [ bitwise reg 1 = (reg=1 & 0x00ffffff ) ^ 0x00000000 ]
   [ cmp eq reg 1 0x0001a8c0 ]
 
-# ct reply daddr 192.168.1.0/24
+# ct reply ip daddr 192.168.1.0/24
 ip test-ip4 output
   [ ct load dst => reg 1 , dir reply ]
   [ bitwise reg 1 = (reg=1 & 0x00ffffff ) ^ 0x00000000 ]
-- 
2.13.5

[PATCH nft 08/10] src: rt: add keyword distinction for nexthop vs nexthop6

[Florian Westphal]

the rt expression currently always sets NFT_RT_NEXTHOP4 and then
uses the network base to determine if its really supposed to be
NEXTHOP6.

For inet, this will fail because the network base is not known,
so this currently enforces need for "meta nfproto" to dermine the
type.

Allow following syntax instead:
  rt ip nexthop
  rt ip6 nexthop

There is no need for a dependency anymore, as rt expression
checks the hook protocol, ie. NEXTHOP4 will break if the hook pf
is not NFPROTO_IPV4.

Cc: Anders K. Pedersen <akp@cohaesio.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/evaluate.c     | 22 +++++++++-------------
 src/parser_bison.y | 24 +++++++++++++++++++++++-
 src/rt.c           | 14 +++++++++++++-
 3 files changed, 45 insertions(+), 15 deletions(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index d379a07ec000..58211198380b 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -662,32 +662,28 @@ static int expr_evaluate_payload(struct eval_ctx *ctx, struct expr **exprp)
 	return 0;
 }
 
-static int expr_error_base(struct list_head *msgs, const struct expr *e)
-{
-	return expr_error(msgs, e,
-			  "meta nfproto ipv4 or ipv6 must be specified "
-			  "before %s expression", e->ops->name);
-}
-
 /*
  * RT expression: validate protocol dependencies.
  */
 static int expr_evaluate_rt(struct eval_ctx *ctx, struct expr **expr)
 {
-	const struct proto_desc *base;
+	static const char emsg[] = "cannot determine ip protocol version, use \"ip nexthop\" or \"ip6 nexthop\" instead";
 	struct expr *rt = *expr;
 
 	rt_expr_update_type(&ctx->pctx, rt);
 
-	base = ctx->pctx.protocol[PROTO_BASE_NETWORK_HDR].desc;
 	switch (rt->rt.key) {
 	case NFT_RT_NEXTHOP4:
-		if (base != &proto_ip)
-			return expr_error_base(ctx->msgs, rt);
+		if (rt->dtype != &ipaddr_type)
+			return expr_error(ctx->msgs, rt, "%s", emsg);
+		if (ctx->pctx.family == NFPROTO_IPV6)
+			return expr_error(ctx->msgs, rt, "%s nexthop will not match", "ip");
 		break;
 	case NFT_RT_NEXTHOP6:
-		if (base != &proto_ip6)
-			return expr_error_base(ctx->msgs, rt);
+		if (rt->dtype != &ip6addr_type)
+			return expr_error(ctx->msgs, rt, "%s", emsg);
+		if (ctx->pctx.family == NFPROTO_IPV4)
+			return expr_error(ctx->msgs, rt, "%s nexthop will not match", "ip6");
 		break;
 	default:
 		break;
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 98df4bd142d4..1a60c1e6a7d4 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -664,7 +664,7 @@ static void location_update(struct location *loc, struct location *rhs, int n)
 
 %type <expr>			rt_expr
 %destructor { expr_free($$); }	rt_expr
-%type <val>			rt_key
+%type <val>			rt_key_proto	rt_key
 
 %type <expr>			ct_expr
 %destructor { expr_free($$); }	ct_expr
@@ -3245,10 +3245,32 @@ hash_expr		:	JHASH		expr	MOD	NUM	SEED	NUM	offset_opt
 			}
 			;
 
+rt_key_proto		:	IP		{ $$ = NFPROTO_IPV4; }
+			|	IP6		{ $$ = NFPROTO_IPV6; }
+			;
+
 rt_expr			:	RT	rt_key
 			{
 				$$ = rt_expr_alloc(&@$, $2, true);
 			}
+			|	RT	rt_key_proto	rt_key
+			{
+				enum nft_rt_keys rtk = $3;
+
+				switch ($2) {
+				case NFPROTO_IPV4:
+					break;
+				case NFPROTO_IPV6:
+					if ($3 == NFT_RT_NEXTHOP4)
+						rtk = NFT_RT_NEXTHOP6;
+					break;
+				default:
+					YYERROR;
+					break;
+				}
+
+				$$ = rt_expr_alloc(&@$, rtk, false);
+			}
 			;
 
 rt_key			:	CLASSID		{ $$ = NFT_RT_CLASSID; }
diff --git a/src/rt.c b/src/rt.c
index 91be5a11c5a2..73638c69d9a5 100644
--- a/src/rt.c
+++ b/src/rt.c
@@ -82,7 +82,19 @@ static const struct rt_template rt_templates[] = {
 
 static void rt_expr_print(const struct expr *expr, struct output_ctx *octx)
 {
-	printf("rt %s", rt_templates[expr->rt.key].token);
+	printf("rt ");
+
+	switch (expr->rt.key) {
+	case NFT_RT_NEXTHOP4:
+		printf("ip ");
+		break;
+	case NFT_RT_NEXTHOP6:
+		printf("ip6 ");
+		break;
+	default:
+		break;
+	}
+	printf("%s", rt_templates[expr->rt.key].token);
 }
 
 static bool rt_expr_cmp(const struct expr *e1, const struct expr *e2)
-- 
2.13.5

[PATCH nft 09/10] tests: rt: fix test cases

[Florian Westphal]

nfproto meta dependency is no longer needed, keep one test
since we still support this syntax.

When meta is not provided, no need to add a dependency because
nft_rt already checks pf number before checking skb->dst.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 tests/py/inet/rt.t         | 10 ++++++++--
 tests/py/inet/rt.t.payload |  4 +---
 tests/py/ip/rt.t           |  3 ++-
 tests/py/ip6/rt0.t         |  2 +-
 4 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/tests/py/inet/rt.t b/tests/py/inet/rt.t
index 9543738b786c..23608ab2c2f9 100644
--- a/tests/py/inet/rt.t
+++ b/tests/py/inet/rt.t
@@ -4,7 +4,13 @@
 
 rt nexthop 192.168.0.1;fail
 rt nexthop fd00::1;fail
-meta nfproto ipv4 rt nexthop 192.168.0.1;ok
-meta nfproto ipv6 rt nexthop fd00::1;ok
+
+meta nfproto ipv4 rt nexthop 192.168.0.1;ok;meta nfproto ipv4 rt ip nexthop 192.168.0.1
+rt ip6 nexthop fd00::1;ok
+
+# missing context
+rt nexthop fd00::1;fail
+# wrong context
+rt ip nexthop fd00::1;fail
 
 tcp option maxseg size set rt mtu;ok
diff --git a/tests/py/inet/rt.t.payload b/tests/py/inet/rt.t.payload
index 928e0095faf5..84dea12caad5 100644
--- a/tests/py/inet/rt.t.payload
+++ b/tests/py/inet/rt.t.payload
@@ -5,10 +5,8 @@ inet test-inet output
   [ rt load nexthop4 => reg 1 ]
   [ cmp eq reg 1 0x0100a8c0 ]
 
-# meta nfproto ipv6 rt nexthop fd00::1
+# rt ip6 nexthop fd00::1
 inet test-inet output
-  [ meta load nfproto => reg 1 ]
-  [ cmp eq reg 1 0x0000000a ]
   [ rt load nexthop6 => reg 1 ]
   [ cmp eq reg 1 0x000000fd 0x00000000 0x00000000 0x01000000 ]
 
diff --git a/tests/py/ip/rt.t b/tests/py/ip/rt.t
index 99750c5d76aa..986bf3417179 100644
--- a/tests/py/ip/rt.t
+++ b/tests/py/ip/rt.t
@@ -2,5 +2,6 @@
 
 *ip;test-ip4;output
 
-rt nexthop 192.168.0.1;ok
+rt nexthop 192.168.0.1;ok;rt ip nexthop 192.168.0.1
 rt nexthop fd00::1;fail
+rt ip6 nexthop fd00::1;fail
diff --git a/tests/py/ip6/rt0.t b/tests/py/ip6/rt0.t
index 92614de3936f..1d50a89cf2f7 100644
--- a/tests/py/ip6/rt0.t
+++ b/tests/py/ip6/rt0.t
@@ -3,4 +3,4 @@
 *ip6;test-ip6;output
 
 rt nexthop 192.168.0.1;fail
-rt nexthop fd00::1;ok
+rt nexthop fd00::1;ok;rt ip6 nexthop fd00::1
-- 
2.13.5

[PATCH nft 10/10] doc: update man page

[Florian Westphal]

you can now use "rt ip|ip6 nexthop" and "ct original|reply ip|ip6 saddr|daddr"
to tell nft if you want to match ipv4 or ipv6.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 doc/nft.xml | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/doc/nft.xml b/doc/nft.xml
index 9d97a782d3a5..c0f42ddccbeb 100644
--- a/doc/nft.xml
+++ b/doc/nft.xml
@@ -608,7 +608,6 @@ filter input iif $int_ifs accept
 			hybrid IPv4/IPv6 tables.  The <literal>meta</literal> expression <literal>nfproto</literal>
 			keyword can be used to test which family (ipv4 or ipv6) context the packet is being processed in.
 
-
 			When no address family is specified, <literal>ip</literal> is used by default.
 		</para>
 
@@ -2905,8 +2904,8 @@ filter output rt classid 10
 # IP family dependent rt expressions
 ip filter output rt nexthop 192.168.0.1
 ip6 filter output rt nexthop fd00::1
-inet filter meta nfproto ipv4 output rt nexthop 192.168.0.1
-inet filter meta nfproto ipv6 output rt nexthop fd00::1
+inet filter output rt ip nexthop 192.168.0.1
+inet filter output rt ip6 nexthop fd00::1
 					</programlisting>
 				</example>
 			</para>
@@ -4025,8 +4024,6 @@ ip6 filter input frag more-fragments 1 counter
 					<group choice="req">
 						<arg>l3proto</arg>
 						<arg>protocol</arg>
-						<arg>saddr</arg>
-						<arg>daddr</arg>
 						<arg>proto-src</arg>
 						<arg>proto-dst</arg>
 						<arg>bytes</arg>
@@ -4035,6 +4032,22 @@ ip6 filter input frag more-fragments 1 counter
 						<arg>zone</arg>
 					</group>
 				</cmdsynopsis>
+				<cmdsynopsis>
+					<command>ct</command>
+					<group choice="req">
+						<arg>original</arg>
+						<arg>reply</arg>
+					</group>
+					<group choice="req">
+						<arg>ip</arg>
+						<arg>ip6</arg>
+					</group>
+					<group choice="req">
+						<arg>saddr</arg>
+						<arg>daddr</arg>
+					</group>
+				</cmdsynopsis>
+
 			</para>
 			<para>
 				<table frame="all">
-- 
2.13.5

Re: [PATCH nft 0/10] nftables remove use of meta nfproto

[Pablo Neira Ayuso]

On Wed, Sep 27, 2017 at 08:16:44PM +0200, Florian Westphal wrote:
> inet family (and others, e.g. bridge) lack context to figure
> out the layer 3 address type.
> 
> examples:
> ct original saddr $addr
> rt nexthop $addr
> 
> We can't use $addr, because it might be a set reference, e.g.
> 
> ct original saddr @whitelist
> 
> currently implemented workaround is to use 'meta nfproto'
> to provide the l3 context, e.g.
> 
> meta nfproto ip rt nexthop 10.2.3.4
> 
> i.e. users need to fill dependency manually.
> 
> Pablo suggested to instead specify ip saddr, ip6 saddr:
> 
> ct original ip saddr $address
> 
> and then let nft handle the dependency injection.
> 
> This series does just that.
> 
> Old syntax is preserved.

Nice series, thanks Florian.

Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>