[PATCH nft 0/5] ct: add support for directional keys

[PATCH nft 0/5] ct: add support for directional keys

[Florian Westphal]

Here are the patches for directional key support that I have.
They pass our regression test suite, another advantage is that
nft will no longer parse 'ct saddr 1.2.3.4', it notices that
a direction is missing.

The disadvantage is that we also have (not-yet added) keys
'packets' and 'bytes'.  These are trivial to add, since
they need a direction, just like ct (s|d)addr.

But in byte/packet case it would be good to also allow
matching on total bytes combined (original+reply).

And that either needs 'ambiguos' keys, i.e. allow
ct packets original > 42  (tells kernel: I want packet count in original direction)
ct packets > 42  (tells kernel: I want original+reply direction).

or a '+' expression so that we can
ct packets original + ct packets reply and so sum via intermediate expression.

So don't apply this yet, I'll have another stab at attemting to
not change the parser at all but instead attempt to resolve
this during evaluation, as Pablo suggested, i.e.

ct direction original
-> ct direction = original

BUT
ct direction = original ct saddr
-> merge into single a single ct expression, asking for saddr
in original direction.

Patrick, if you have any advice wrt the nft grammar I'd be glad to hear it.

[PATCH nft 1/5] ct: add support for directional keys

[Florian Westphal]

A few keys in the ct expression are directional, i.e.
we need to tell kernel if it should fetch REPLY or ORIGINAL direction.

Split ct_keys into ct_keys & ct_keys_dir, the latter are those keys
that the kernel rejects unless also given a direction.

During postprocessing we also need to invoke ct_expr_update_type,
problem is that e.g. ct saddr can be any family (ip, ipv6) so we need
to update the expected data type based on the network base.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/ct.h              |  2 +-
 include/expression.h      |  1 +
 src/ct.c                  | 35 ++++++++++++++++++++++++++++++++---
 src/evaluate.c            | 14 ++++++++++++++
 src/netlink_delinearize.c | 24 +++++++++++++++++++++---
 src/netlink_linearize.c   |  4 ++++
 src/parser_bison.y        | 22 ++++++++++++++++------
 7 files changed, 89 insertions(+), 13 deletions(-)

diff --git a/include/ct.h b/include/ct.h
index 64366ab..c04b3bb 100644
--- a/include/ct.h
+++ b/include/ct.h
@@ -24,7 +24,7 @@ struct ct_template {
 }
 
 extern struct expr *ct_expr_alloc(const struct location *loc,
-				  enum nft_ct_keys key);
+				  enum nft_ct_keys key, struct expr *expr);
 extern void ct_expr_update_type(struct proto_ctx *ctx, struct expr *expr);
 
 #endif /* NFTABLES_CT_H */
diff --git a/include/expression.h b/include/expression.h
index 010cb95..130cc1f 100644
--- a/include/expression.h
+++ b/include/expression.h
@@ -273,6 +273,7 @@ struct expr {
 		struct {
 			/* EXPR_CT */
 			enum nft_ct_keys	key;
+			struct expr		*dir_expr;
 		} ct;
 	};
 };
diff --git a/src/ct.c b/src/ct.c
index aa80138..c705838 100644
--- a/src/ct.c
+++ b/src/ct.c
@@ -208,16 +208,38 @@ static const struct ct_template ct_templates[] = {
 static void ct_expr_print(const struct expr *expr)
 {
 	printf("ct %s", ct_templates[expr->ct.key].token);
+	if (expr->ct.dir_expr) {
+		printf(" ");
+		expr_print(expr->ct.dir_expr);
+	}
 }
 
 static bool ct_expr_cmp(const struct expr *e1, const struct expr *e2)
 {
-	return e1->ct.key == e2->ct.key;
+	if (e1->ct.key != e2->ct.key)
+		return false;
+
+	e1 = e1->ct.dir_expr;
+	e2 = e2->ct.dir_expr;
+
+	if (!e1)
+		return e2 == NULL;
+	if (!e2)
+		return false;
+
+	return expr_cmp(e1, e2);
 }
 
 static void ct_expr_clone(struct expr *new, const struct expr *expr)
 {
-	new->ct.key = expr->ct.key;
+	new->ct = expr->ct;
+	if (expr->ct.dir_expr)
+		new->ct.dir_expr = expr_clone(expr->ct.dir_expr);
+}
+
+static void ct_expr_destroy(struct expr *expr)
+{
+	expr_free(expr->ct.dir_expr);
 }
 
 static void ct_expr_pctx_update(struct proto_ctx *ctx, const struct expr *expr)
@@ -246,10 +268,12 @@ static const struct expr_ops ct_expr_ops = {
 	.print		= ct_expr_print,
 	.cmp		= ct_expr_cmp,
 	.clone		= ct_expr_clone,
+	.destroy	= ct_expr_destroy,
 	.pctx_update	= ct_expr_pctx_update,
 };
 
-struct expr *ct_expr_alloc(const struct location *loc, enum nft_ct_keys key)
+struct expr *ct_expr_alloc(const struct location *loc, enum nft_ct_keys key,
+			   struct expr *dir_expr)
 {
 	const struct ct_template *tmpl = &ct_templates[key];
 	struct expr *expr;
@@ -258,6 +282,11 @@ struct expr *ct_expr_alloc(const struct location *loc, enum nft_ct_keys key)
 			  tmpl->byteorder, tmpl->len);
 	expr->ct.key = key;
 
+	if (dir_expr) {
+		expr_set_type(dir_expr, &ct_dir_type, BYTEORDER_INVALID);
+		expr->ct.dir_expr = dir_expr;
+	}
+
 	switch (key) {
 	case NFT_CT_PROTOCOL:
 		expr->flags = EXPR_F_PROTOCOL;
diff --git a/src/evaluate.c b/src/evaluate.c
index 7aab6aa..8dd1373 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -471,10 +471,24 @@ static int expr_evaluate_payload(struct eval_ctx *ctx, struct expr **expr)
  */
 static int expr_evaluate_ct(struct eval_ctx *ctx, struct expr **expr)
 {
+	struct expr *direction = NULL;
+	struct error_record *erec;
 	struct expr *ct = *expr;
 
 	ct_expr_update_type(&ctx->pctx, ct);
 
+	if (ct->ct.dir_expr &&
+	    ct->ct.dir_expr->ops->type == EXPR_SYMBOL) {
+		erec = symbol_parse(ct->ct.dir_expr, &direction);
+		if (erec != NULL) {
+			erec_queue(erec, ctx->msgs);
+			return -1;
+		}
+
+		expr_free(ct->ct.dir_expr);
+		ct->ct.dir_expr = direction;
+	}
+
 	return expr_evaluate_primary(ctx, expr);
 }
 
diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index f68fca0..306d1b8 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -536,12 +536,19 @@ static void netlink_parse_ct_expr(struct netlink_parse_ctx *ctx,
 				  const struct location *loc,
 				  const struct nftnl_expr *nle)
 {
+	struct expr *expr = NULL;
 	enum nft_registers dreg;
 	uint32_t key;
-	struct expr *expr;
+
+	if (nftnl_expr_is_set(nle, NFTNL_EXPR_CT_DIR)) {
+		uint8_t dir = nftnl_expr_get_u8(nle, NFTNL_EXPR_CT_DIR);
+
+		expr = constant_expr_alloc(loc, &integer_type,
+					   BYTEORDER_HOST_ENDIAN, 8, &dir);
+	}
 
 	key  = nftnl_expr_get_u32(nle, NFTNL_EXPR_CT_KEY);
-	expr = ct_expr_alloc(loc, key);
+	expr = ct_expr_alloc(loc, key, expr);
 
 	dreg = netlink_parse_register(nle, NFTNL_EXPR_CT_DREG);
 	netlink_set_register(ctx, dreg, expr);
@@ -1117,6 +1124,12 @@ static void meta_match_postprocess(struct rule_pp_ctx *ctx,
 	}
 }
 
+static void ct_match_postprocess(struct rule_pp_ctx *ctx,
+				 const struct expr *expr)
+{
+	return meta_match_postprocess(ctx, expr);
+}
+
 /* Convert a bitmask to a prefix length */
 static unsigned int expr_mask_to_prefix(const struct expr *expr)
 {
@@ -1394,6 +1407,9 @@ static void expr_postprocess(struct rule_pp_ctx *ctx, struct expr **exprp)
 		expr_postprocess(ctx, &expr->right);
 
 		switch (expr->left->ops->type) {
+		case EXPR_CT:
+			ct_match_postprocess(ctx, expr);
+			break;
 		case EXPR_META:
 			meta_match_postprocess(ctx, expr);
 			break;
@@ -1431,9 +1447,11 @@ static void expr_postprocess(struct rule_pp_ctx *ctx, struct expr **exprp)
 	case EXPR_SET_REF:
 	case EXPR_EXTHDR:
 	case EXPR_META:
-	case EXPR_CT:
 	case EXPR_VERDICT:
 		break;
+	case EXPR_CT:
+		ct_expr_update_type(&ctx->pctx, expr);
+		break;
 	default:
 		BUG("unknown expression type %s\n", expr->ops->name);
 	}
diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c
index 131c3f9..81fe754 100644
--- a/src/netlink_linearize.c
+++ b/src/netlink_linearize.c
@@ -209,6 +209,10 @@ static void netlink_gen_ct(struct netlink_linearize_ctx *ctx,
 	nle = alloc_nft_expr("ct");
 	netlink_put_register(nle, NFTNL_EXPR_CT_DREG, dreg);
 	nftnl_expr_set_u32(nle, NFTNL_EXPR_CT_KEY, expr->ct.key);
+	if (expr->ct.dir_expr)
+		nftnl_expr_set_u8(nle, NFTNL_EXPR_CT_DIR,
+			mpz_get_uint8(expr->ct.dir_expr->value));
+
 	nftnl_rule_add_expr(ctx->nlr, nle);
 }
 
diff --git a/src/parser_bison.y b/src/parser_bison.y
index fbfe7ea..93fa7d3 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -555,7 +555,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
+%type <val>			ct_key		ct_key_dir
 
 %type <val>			export_format
 %type <string>			monitor_event
@@ -2037,9 +2037,18 @@ meta_stmt		:	META	meta_key	SET	expr
 			}
 			;
 
-ct_expr			:	CT	ct_key
+ct_expr			: 	CT	ct_key
 			{
-				$$ = ct_expr_alloc(&@$, $2);
+				$$ = ct_expr_alloc(&@$, $2, NULL);
+			}
+			|	CT	ct_key_dir 	STRING
+			{
+				struct expr *e = NULL;
+
+				e = symbol_expr_alloc(&@$, SYMBOL_VALUE,
+							       current_scope(state), $3);
+
+				$$ = ct_expr_alloc(&@$, $2, e);
 			}
 			;
 
@@ -2049,13 +2058,14 @@ ct_key			:	STATE		{ $$ = NFT_CT_STATE; }
 			|	MARK		{ $$ = NFT_CT_MARK; }
 			|	EXPIRATION	{ $$ = NFT_CT_EXPIRATION; }
 			|	HELPER		{ $$ = NFT_CT_HELPER; }
-			|	L3PROTOCOL	{ $$ = NFT_CT_L3PROTOCOL; }
-			|	SADDR		{ $$ = NFT_CT_SRC; }
+			|	LABEL		{ $$ = NFT_CT_LABELS; }
+			;
+ct_key_dir		:	SADDR		{ $$ = NFT_CT_SRC; }
 			|	DADDR		{ $$ = NFT_CT_DST; }
+			|	L3PROTOCOL	{ $$ = NFT_CT_L3PROTOCOL; }
 			|	PROTOCOL	{ $$ = NFT_CT_PROTOCOL; }
 			|	PROTO_SRC	{ $$ = NFT_CT_PROTO_SRC; }
 			|	PROTO_DST	{ $$ = NFT_CT_PROTO_DST; }
-			|	LABEL		{ $$ = NFT_CT_LABELS; }
 			;
 
 ct_stmt			:	CT	ct_key		SET	expr
-- 
2.4.10

[PATCH nft 2/5] netlink: don't handle lhs zero-length expression as concat type

[Florian Westphal]

expr->len 0 can appear for some data types whose size can be different
based on some external state, e.g. the conntrack src/dst addresses.

The nft type is 'invalid/0-length' in the template definition, the
size is set (on linearization) based on the network base family,
i.e. the type is changed to ip or ipv6 address at a later stage.

For delinarization, skip zero-length expression as concat type
and give expr_postprocess a chance to fix the types.

Without this change the previous patch will result in nft consuming all
available memory when trying to display e.g. a 'ct saddr' rule.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/netlink_delinearize.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 306d1b8..a983fce 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -236,7 +236,7 @@ static void netlink_parse_cmp(struct netlink_parse_ctx *ctx,
 	    left->dtype != &string_type) {
 		return netlink_error(ctx, loc,
 				     "Relational expression size mismatch");
-	} else if (left->len < right->len) {
+	} else if (left->len > 0 && left->len < right->len) {
 		left = netlink_parse_concat_expr(ctx, loc, sreg, right->len);
 		if (left == NULL)
 			return;
-- 
2.4.10

[PATCH nft 3/5] netlink: only drop mask if it matches left known-size operand

[Florian Westphal]

During delinearization we attempt to remove masks, for instance
ip saddr $x/32. (mask matches the entire size).

However, in some special cases the lhs size is unknown (0), this
happens f.e. with

'ct saddr original 1.2.3.4/24' which had its '/24' chopped off.

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

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index a983fce..c9db27d 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -331,12 +331,14 @@ static void netlink_parse_bitwise(struct netlink_parse_ctx *ctx,
 		mpz_ior(m, m, o);
 	}
 
-	if (mpz_scan0(m, 0) != left->len) {
+	if (left->len > 0 && mpz_scan0(m, 0) == left->len) {
+		/* mask encompasses the entire value */
+		expr_free(mask);
+	} else {
 		mpz_set(mask->value, m);
 		expr = binop_expr_alloc(loc, OP_AND, expr, mask);
 		expr->len = left->len;
-	} else
-		expr_free(mask);
+	}
 
 	if (mpz_cmp_ui(x, 0)) {
 		mpz_set(xor->value, x);
-- 
2.4.10

[PATCH nft 4/5] src: ct: make ct l3proto work

[Florian Westphal]

ct l3proto original == ipv6
<cmdline>:1:56-59: Error: Can't parse symbolic invalid expressions

Its just the nf protocol number -- no dependencies.  Just set right type.

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

diff --git a/src/ct.c b/src/ct.c
index c705838..5a3fda3 100644
--- a/src/ct.c
+++ b/src/ct.c
@@ -184,8 +184,8 @@ static const struct ct_template ct_templates[] = {
 	[NFT_CT_HELPER]		= CT_TEMPLATE("helper",	    &string_type,
 					      BYTEORDER_HOST_ENDIAN,
 					      NF_CT_HELPER_NAME_LEN * BITS_PER_BYTE),
-	[NFT_CT_L3PROTOCOL]	= CT_TEMPLATE("l3proto",    &invalid_type,
-					      BYTEORDER_INVALID,
+	[NFT_CT_L3PROTOCOL]	= CT_TEMPLATE("l3proto",    &nfproto_type,
+					      BYTEORDER_HOST_ENDIAN,
 					      BITS_PER_BYTE),
 	[NFT_CT_SRC]		= CT_TEMPLATE("saddr",	    &invalid_type,
 					      BYTEORDER_BIG_ENDIAN, 0),
-- 
2.4.10

[PATCH nft 5/5] tests: add ct tests for ip family

[Florian Westphal]

Cannot check e.g. saddr for 192.168.0.1 for 'any' protocol, nft
needs to expect arguments of a specific address type.

So e.g. when using 'inet' we need to add a rule that makes the expected
family explicit, e.g. 'meta nfproto ipv4'.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 tests/py/ip/ct.t         | 23 ++++++++++++++++++
 tests/py/ip/ct.t.payload | 63 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 86 insertions(+)
 create mode 100644 tests/py/ip/ct.t
 create mode 100644 tests/py/ip/ct.t.payload

diff --git a/tests/py/ip/ct.t b/tests/py/ip/ct.t
new file mode 100644
index 0000000..28ad766
--- /dev/null
+++ b/tests/py/ip/ct.t
@@ -0,0 +1,23 @@
+*ip;test-ip4
+
+:output;type filter hook output priority 0
+
+ct saddr original 192.168.0.1;ok
+ct saddr reply 192.168.0.1;ok
+ct daddr original 192.168.0.1;ok
+ct daddr reply 192.168.0.1;ok
+
+# same, but with a netmask
+ct saddr original 192.168.1.0/24;ok
+ct saddr reply 192.168.1.0/24;ok
+ct daddr original 192.168.1.0/24;ok
+ct daddr reply 192.168.1.0/24;ok
+
+ct l3proto original ipv4;ok
+ct l3proto reply foobar;fail
+
+ct protocol original 6 ct proto-dst original 22;ok
+ct protocol original 17 ct proto-src reply 53;ok
+
+# wrong address family
+ct daddr reply dead::beef;fail
diff --git a/tests/py/ip/ct.t.payload b/tests/py/ip/ct.t.payload
new file mode 100644
index 0000000..4a23455
--- /dev/null
+++ b/tests/py/ip/ct.t.payload
@@ -0,0 +1,63 @@
+# ct saddr original 192.168.0.1
+ip test-ip4 output
+  [ ct load src => reg 1 , dir original ]
+  [ cmp eq reg 1 0x0100a8c0 ]
+
+# ct saddr reply 192.168.0.1
+ip test-ip4 output
+  [ ct load src => reg 1 , dir reply ]
+  [ cmp eq reg 1 0x0100a8c0 ]
+
+# ct daddr original 192.168.0.1
+ip test-ip4 output
+  [ ct load dst => reg 1 , dir original ]
+  [ cmp eq reg 1 0x0100a8c0 ]
+
+# ct daddr reply 192.168.0.1
+ip test-ip4 output
+  [ ct load dst => reg 1 , dir reply ]
+  [ cmp eq reg 1 0x0100a8c0 ]
+
+# ct saddr original 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 saddr reply 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 daddr original 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 daddr reply 192.168.1.0/24
+ip test-ip4 output
+  [ ct load dst => reg 1 , dir reply ]
+  [ bitwise reg 1 = (reg=1 & 0x00ffffff ) ^ 0x00000000 ]
+  [ cmp eq reg 1 0x0001a8c0 ]
+
+# ct l3proto original ipv4
+ip test-ip4 output
+  [ ct load l3protocol => reg 1 , dir original ]
+  [ cmp eq reg 1 0x00000002 ]
+
+# ct protocol original 6 ct proto-dst original 22
+ip test-ip4 output
+  [ ct load protocol => reg 1 , dir original ]
+  [ cmp eq reg 1 0x00000006 ]
+  [ ct load proto_dst => reg 1 , dir original ]
+  [ cmp eq reg 1 0x00001600 ]
+
+# ct protocol original 17 ct proto-src reply 53
+ip test-ip4 output
+  [ ct load protocol => reg 1 , dir original ]
+  [ cmp eq reg 1 0x00000011 ]
+  [ ct load proto_src => reg 1 , dir reply ]
+  [ cmp eq reg 1 0x00003500 ]
+
-- 
2.4.10

Re: [PATCH nft 1/5] ct: add support for directional keys

[Pablo Neira Ayuso]

On Fri, Dec 18, 2015 at 10:07:59PM +0100, Florian Westphal wrote:
> A few keys in the ct expression are directional, i.e.
> we need to tell kernel if it should fetch REPLY or ORIGINAL direction.
> 
> Split ct_keys into ct_keys & ct_keys_dir, the latter are those keys
> that the kernel rejects unless also given a direction.
> 
> During postprocessing we also need to invoke ct_expr_update_type,
> problem is that e.g. ct saddr can be any family (ip, ipv6) so we need
> to update the expected data type based on the network base.

I think this is the way to go. My original proposal will not allow
things like:

        ct direction reply ct daddr original 2.2.2.2

which seems to me like a valid matching, specifically when NAT comes
into place (where original and reply tuples are not the same). This is
kind of corner case, but I think we shouldn't reduce the expressiveness.

Several comments below:

> diff --git a/include/expression.h b/include/expression.h
> index 010cb95..130cc1f 100644
> --- a/include/expression.h
> +++ b/include/expression.h
> @@ -273,6 +273,7 @@ struct expr {
>  		struct {
>  			/* EXPR_CT */
>  			enum nft_ct_keys	key;
> +			struct expr		*dir_expr;

I think you can store the direction as a value.

>  		} ct;
>  	};
>  };
[...]
> diff --git a/src/evaluate.c b/src/evaluate.c
> index 7aab6aa..8dd1373 100644
> --- a/src/evaluate.c
> +++ b/src/evaluate.c
> @@ -471,10 +471,24 @@ static int expr_evaluate_payload(struct eval_ctx *ctx, struct expr **expr)
>   */
>  static int expr_evaluate_ct(struct eval_ctx *ctx, struct expr **expr)
>  {
> +	struct expr *direction = NULL;
> +	struct error_record *erec;
>  	struct expr *ct = *expr;
>  
>  	ct_expr_update_type(&ctx->pctx, ct);
>  
> +	if (ct->ct.dir_expr &&
> +	    ct->ct.dir_expr->ops->type == EXPR_SYMBOL) {
> +		erec = symbol_parse(ct->ct.dir_expr, &direction);
> +		if (erec != NULL) {
> +			erec_queue(erec, ctx->msgs);
> +			return -1;
> +		}
> +
> +		expr_free(ct->ct.dir_expr);
> +		ct->ct.dir_expr = direction;
> +	}

I'd suggest you parse the direction from the parser, I don't find any
good reason to postpone this error handling. This should also simplify
the delinearize step since we'll get a value when parsing the netlink
attributes.

> +
>  	return expr_evaluate_primary(ctx, expr);
>  }
>  
> diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
> index f68fca0..306d1b8 100644
> --- a/src/netlink_delinearize.c
> +++ b/src/netlink_delinearize.c
> @@ -536,12 +536,19 @@ static void netlink_parse_ct_expr(struct netlink_parse_ctx *ctx,
>  				  const struct location *loc,
>  				  const struct nftnl_expr *nle)
>  {
> +	struct expr *expr = NULL;
>  	enum nft_registers dreg;
>  	uint32_t key;
> -	struct expr *expr;
> +
> +	if (nftnl_expr_is_set(nle, NFTNL_EXPR_CT_DIR)) {
> +		uint8_t dir = nftnl_expr_get_u8(nle, NFTNL_EXPR_CT_DIR);
> +
> +		expr = constant_expr_alloc(loc, &integer_type,
> +					   BYTEORDER_HOST_ENDIAN, 8, &dir);
> +	}
>  
>  	key  = nftnl_expr_get_u32(nle, NFTNL_EXPR_CT_KEY);
> -	expr = ct_expr_alloc(loc, key);
> +	expr = ct_expr_alloc(loc, key, expr);
>  
>  	dreg = netlink_parse_register(nle, NFTNL_EXPR_CT_DREG);
>  	netlink_set_register(ctx, dreg, expr);
> @@ -1117,6 +1124,12 @@ static void meta_match_postprocess(struct rule_pp_ctx *ctx,
>  	}
>  }
>  
> +static void ct_match_postprocess(struct rule_pp_ctx *ctx,
> +				 const struct expr *expr)
> +{
> +	return meta_match_postprocess(ctx, expr);
> +}
> +
>  /* Convert a bitmask to a prefix length */
>  static unsigned int expr_mask_to_prefix(const struct expr *expr)
>  {
> @@ -1394,6 +1407,9 @@ static void expr_postprocess(struct rule_pp_ctx *ctx, struct expr **exprp)
>  		expr_postprocess(ctx, &expr->right);
>  
>  		switch (expr->left->ops->type) {
> +		case EXPR_CT:
> +			ct_match_postprocess(ctx, expr);
> +			break;
>  		case EXPR_META:
>  			meta_match_postprocess(ctx, expr);
>  			break;
> @@ -1431,9 +1447,11 @@ static void expr_postprocess(struct rule_pp_ctx *ctx, struct expr **exprp)
>  	case EXPR_SET_REF:
>  	case EXPR_EXTHDR:
>  	case EXPR_META:
> -	case EXPR_CT:
>  	case EXPR_VERDICT:
>  		break;
> +	case EXPR_CT:
> +		ct_expr_update_type(&ctx->pctx, expr);
> +		break;
>  	default:
>  		BUG("unknown expression type %s\n", expr->ops->name);
>  	}
> diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c
> index 131c3f9..81fe754 100644
> --- a/src/netlink_linearize.c
> +++ b/src/netlink_linearize.c
> @@ -209,6 +209,10 @@ static void netlink_gen_ct(struct netlink_linearize_ctx *ctx,
>  	nle = alloc_nft_expr("ct");
>  	netlink_put_register(nle, NFTNL_EXPR_CT_DREG, dreg);
>  	nftnl_expr_set_u32(nle, NFTNL_EXPR_CT_KEY, expr->ct.key);
> +	if (expr->ct.dir_expr)
> +		nftnl_expr_set_u8(nle, NFTNL_EXPR_CT_DIR,
> +			mpz_get_uint8(expr->ct.dir_expr->value));
> +
>  	nftnl_rule_add_expr(ctx->nlr, nle);
>  }
>  
> diff --git a/src/parser_bison.y b/src/parser_bison.y
> index fbfe7ea..93fa7d3 100644
> --- a/src/parser_bison.y
> +++ b/src/parser_bison.y
> @@ -555,7 +555,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
> +%type <val>			ct_key		ct_key_dir
>  
>  %type <val>			export_format
>  %type <string>			monitor_event
> @@ -2037,9 +2037,18 @@ meta_stmt		:	META	meta_key	SET	expr
>  			}
>  			;
>  
> -ct_expr			:	CT	ct_key
> +ct_expr			: 	CT	ct_key
>  			{
> -				$$ = ct_expr_alloc(&@$, $2);
> +				$$ = ct_expr_alloc(&@$, $2, NULL);
> +			}
> +			|	CT	ct_key_dir 	STRING
> +			{
> +				struct expr *e = NULL;
> +
> +				e = symbol_expr_alloc(&@$, SYMBOL_VALUE,
> +							       current_scope(state), $3);

ie. parse the string and obtain the value from here.

> +
> +				$$ = ct_expr_alloc(&@$, $2, e);
>  			}
>  			;
>  

Re: [PATCH nft 1/5] ct: add support for directional keys

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Fri, Dec 18, 2015 at 10:07:59PM +0100, Florian Westphal wrote:
> > A few keys in the ct expression are directional, i.e.
> > we need to tell kernel if it should fetch REPLY or ORIGINAL direction.
> > 
> > Split ct_keys into ct_keys & ct_keys_dir, the latter are those keys
> > that the kernel rejects unless also given a direction.
> > 
> > During postprocessing we also need to invoke ct_expr_update_type,
> > problem is that e.g. ct saddr can be any family (ip, ipv6) so we need
> > to update the expected data type based on the network base.
> 
> I think this is the way to go. My original proposal will not allow
> things like:
> 
>         ct direction reply ct daddr original 2.2.2.2
> 
> which seems to me like a valid matching, specifically when NAT comes
> into place (where original and reply tuples are not the same). This is
> kind of corner case, but I think we shouldn't reduce the expressiveness.

Great, so this seems ready so I pushed the series to nft master.

> > diff --git a/include/expression.h b/include/expression.h
> > --- a/include/expression.h
> > +++ b/include/expression.h
> > @@ -273,6 +273,7 @@ struct expr {
> >  		struct {
> >  			/* EXPR_CT */
> >  			enum nft_ct_keys	key;
> > +			struct expr		*dir_expr;
> 
> I think you can store the direction as a value.

Ok, changed to int8_t ..

> > +		expr_free(ct->ct.dir_expr);
> > +		ct->ct.dir_expr = direction;
> > +	}
> 
> I'd suggest you parse the direction from the parser, I don't find any

... and added a helper to do just that.

There were no other changes.

If you get
ip/ct.t: WARNING: line: 16: 'src/nft add rule --debug=netlink ip test-ip4 output ct l3proto original ipv4': 'ct l3proto original ipv4' mismatches 'ct l3proto ipv4'

that means kernel lacks d5f79b6e4d1 ('netfilter: nft_ct: include direction when dumping NFT_CT_L3PROTOCOL key').