[nft crap] ct original ip saddr ... handling

[nft crap] ct original ip saddr ... handling

[Florian Westphal]

I am running out of time so I have to send this unfinished/non-working
state.

It breaks because ct is riddled with conflicts,
in nft we've handled original/reply as STRING to avoid conflicts
with 'arp reply', so we cannot add

ct original ip saddr

because it is
ct STRING	IP	SADDR

and that conflicts with basic use where 'ip saddr' could be payload
expression, and STRING is one of the normal ct tokens and not a direction.

I am trying to fix this here by moving all ct keywords back to tokens.
There are no shift/reduce errors, things compile fine, and all
test cases work.  Its just that we break 'ct event set label':

Works:
ct event set new or reply
ct event set new,reply
ct event set new,label
fails:
ct event set label ('expects COMMA')

Other than that it should work, this also adds dependency removal
for meta and ct when de-linearizing rulesets and gets rid of
the uneeded meta dependency when using rt nexthop in inet table.

[PATCH 01/17] rename struct ct to ct_helper

[Florian Westphal]

Its misleading, this structure holds members for ct_helper object
infrastructure, rename it.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/rule.h     |  6 +++---
 src/netlink.c      | 14 +++++++-------
 src/parser_bison.y | 12 ++++++------
 src/rule.c         |  6 +++---
 4 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/include/rule.h b/include/rule.h
index 7424b21c6e01..24c73d85f83b 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -263,8 +263,8 @@ struct quota {
 	uint32_t	flags;
 };
 
-struct ct {
-	char helper_name[16];
+struct ct_helper {
+	char name[16];
 	uint16_t l3proto;
 	uint8_t l4proto;
 };
@@ -287,7 +287,7 @@ struct obj {
 	union {
 		struct counter		counter;
 		struct quota		quota;
-		struct ct		ct;
+		struct ct_helper	ct_helper;
 	};
 };
 
diff --git a/src/netlink.c b/src/netlink.c
index 880502cde20c..b9731a91b9bf 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -319,12 +319,12 @@ alloc_nftnl_obj(const struct handle *h, struct obj *obj)
 		break;
 	case NFT_OBJECT_CT_HELPER:
 		nftnl_obj_set_str(nlo, NFTNL_OBJ_CT_HELPER_NAME,
-				  obj->ct.helper_name);
+				  obj->ct_helper.name);
 		nftnl_obj_set_u8(nlo, NFTNL_OBJ_CT_HELPER_L4PROTO,
-				  obj->ct.l4proto);
-		if (obj->ct.l3proto)
+				  obj->ct_helper.l4proto);
+		if (obj->ct_helper.l3proto)
 			nftnl_obj_set_u16(nlo, NFTNL_OBJ_CT_HELPER_L3PROTO,
-					  obj->ct.l3proto);
+					  obj->ct_helper.l3proto);
 		break;
 	default:
 		BUG("Unknown type %d\n", obj->type);
@@ -1832,10 +1832,10 @@ static struct obj *netlink_delinearize_obj(struct netlink_ctx *ctx,
 			nftnl_obj_get_u32(nlo, NFTNL_OBJ_QUOTA_FLAGS);
 		break;
 	case NFT_OBJECT_CT_HELPER:
-		snprintf(obj->ct.helper_name, sizeof(obj->ct.helper_name), "%s",
+		snprintf(obj->ct_helper.name, sizeof(obj->ct_helper.name), "%s",
 			 nftnl_obj_get_str(nlo, NFTNL_OBJ_CT_HELPER_NAME));
-		obj->ct.l3proto = nftnl_obj_get_u16(nlo, NFTNL_OBJ_CT_HELPER_L3PROTO);
-		obj->ct.l4proto = nftnl_obj_get_u8(nlo, NFTNL_OBJ_CT_HELPER_L4PROTO);
+		obj->ct_helper.l3proto = nftnl_obj_get_u16(nlo, NFTNL_OBJ_CT_HELPER_L3PROTO);
+		obj->ct_helper.l4proto = nftnl_obj_get_u8(nlo, NFTNL_OBJ_CT_HELPER_L4PROTO);
 		break;
 	}
 	obj->type = type;
diff --git a/src/parser_bison.y b/src/parser_bison.y
index a8448e14ef1f..9a9ff6f5ce44 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -2706,14 +2706,14 @@ ct_l4protoname		:	TCP	{ $$ = IPPROTO_TCP; }
 
 ct_config		:	TYPE	QUOTED_STRING	PROTOCOL	ct_l4protoname	stmt_seperator
 			{
-				struct ct *ct;
+				struct ct_helper *ct;
 				int ret;
 
-				ct = &$<obj>0->ct;
+				ct = &$<obj>0->ct_helper;
 
-				ret = snprintf(ct->helper_name, sizeof(ct->helper_name), "%s", $2);
-				if (ret <= 0 || ret >= (int)sizeof(ct->helper_name)) {
-					erec_queue(error(&@2, "invalid name '%s', max length is %u\n", $2, (int)sizeof(ct->helper_name)), state->msgs);
+				ret = snprintf(ct->name, sizeof(ct->name), "%s", $2);
+				if (ret <= 0 || ret >= (int)sizeof(ct->name)) {
+					erec_queue(error(&@2, "invalid name '%s', max length is %u\n", $2, (int)sizeof(ct->name)), state->msgs);
 					YYERROR;
 				}
 
@@ -2721,7 +2721,7 @@ ct_config		:	TYPE	QUOTED_STRING	PROTOCOL	ct_l4protoname	stmt_seperator
 			}
 			|	L3PROTOCOL	family_spec_explicit	stmt_seperator
 			{
-				$<obj>0->ct.l3proto = $2;
+				$<obj>0->ct_helper.l3proto = $2;
 			}
 			;
 
diff --git a/src/rule.c b/src/rule.c
index f65674c0e2d4..ee510fe0123b 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -1258,9 +1258,9 @@ static void obj_print_data(const struct obj *obj,
 		break;
 	case NFT_OBJECT_CT_HELPER: {
 		printf("ct helper %s {\n", obj->handle.obj);
-		printf("\t\ttype \"%s\" protocol ", obj->ct.helper_name);
-		print_proto_name_proto(obj->ct.l4proto);
-		printf("\t\tl3proto %s", family2str(obj->ct.l3proto));
+		printf("\t\ttype \"%s\" protocol ", obj->ct_helper.name);
+		print_proto_name_proto(obj->ct_helper.l4proto);
+		printf("\t\tl3proto %s", family2str(obj->ct_helper.l3proto));
 		break;
 		}
 	default:
-- 
2.13.0

Re: [PATCH 01/17] rename struct ct to ct_helper

[Pablo Neira Ayuso]

Hi,

I apply these series, from 2 to 17, but this doesn't compile.

make[1]: Entering directory
'/home/pablo/devel/scm/git-netfilter/nftables'
Making all in src
make[2]: Entering directory
'/home/pablo/devel/scm/git-netfilter/nftables/src'
  YACC     parser_bison.c
/home/pablo/devel/scm/git-netfilter/nftables/src/parser_bison.y:3146.33-44: error: symbol keyword_expr is used, but is not defined as a token and has no rules
                        |       keyword_expr
                                ^^^^^^^^^^^^
/home/pablo/devel/scm/git-netfilter/nftables/src/parser_bison.y:3146.33-44: warning: type clash on default action: <expr> != <> [-Wother]
                        |       keyword_expr
                                ^^^^^^^^^^^^

This changeset is large and it is making it a bit hard for me to
review.

Could you send me a smaller batch?

I want to understand why the parser is hitting shift/reduce problems
from set statements.

Thanks.

[PATCH 02/17] src: prepare for future ct timeout policy support

[Florian Westphal]

Change all places that expect ct helper tokens (ct helper configuration)
to CT 	HELPER.  Also rename ct_block to ct_helper_block; we want to
add a ct_timeout_pol_block (or similar), notg extend ct_block.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/rule.h     |   2 +-
 src/ct.c           |  10 -----
 src/parser_bison.y | 109 +++++++++++++++--------------------------------------
 src/rule.c         |   7 +++-
 src/scanner.l      |   1 +
 5 files changed, 38 insertions(+), 91 deletions(-)

diff --git a/include/rule.h b/include/rule.h
index 24c73d85f83b..b96d1bba0311 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -448,7 +448,7 @@ extern struct cmd *cmd_alloc(enum cmd_ops op, enum cmd_obj obj,
 extern void nft_cmd_expand(struct cmd *cmd);
 extern struct cmd *cmd_alloc_obj_ct(enum cmd_ops op, int type,
 				    const struct handle *h,
-				    const struct location *loc, void *data);
+				    const struct location *loc, struct obj *obj);
 extern void cmd_free(struct cmd *cmd);
 
 #include <payload.h>
diff --git a/src/ct.c b/src/ct.c
index c705750db4f7..87fe08bc62f8 100644
--- a/src/ct.c
+++ b/src/ct.c
@@ -384,16 +384,6 @@ struct error_record *ct_key_parse(const struct location *loc, const char *str,
 	return error(loc, "syntax error, unexpected %s, known keys are %s", str, buf);
 }
 
-struct error_record *ct_objtype_parse(const struct location *loc, const char *str, int *type)
-{
-	if (strcmp(str, "helper") == 0) {
-		*type = NFT_OBJECT_CT_HELPER;
-		return NULL;
-	}
-
-	return error(loc, "unknown ct class '%s', want 'helper'", str);
-}
-
 struct expr *ct_expr_alloc(const struct location *loc, enum nft_ct_keys key,
 			   int8_t direction)
 {
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 9a9ff6f5ce44..79918399368e 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -374,6 +374,7 @@ static void location_update(struct location *loc, struct location *rhs, int n)
 %token NEXTHOP			"nexthop"
 
 %token CT			"ct"
+%token HELPER			"helper"
 %token L3PROTOCOL		"l3proto"
 %token PROTO_SRC		"proto-src"
 %token PROTO_DST		"proto-dst"
@@ -495,7 +496,7 @@ static void location_update(struct location *loc, struct location *rhs, int n)
 %type <set>			map_block_alloc map_block
 %destructor { set_free($$); }	map_block_alloc
 
-%type <obj>			obj_block_alloc counter_block quota_block ct_block
+%type <obj>			obj_block_alloc counter_block quota_block ct_helper_block
 %destructor { obj_free($$); }	obj_block_alloc
 
 %type <list>			stmt_list
@@ -669,9 +670,7 @@ static void location_update(struct location *loc, struct location *rhs, int n)
 %destructor { expr_free($$); }	exthdr_exists_expr
 %type <val>			exthdr_key
 
-%type <val>			ct_l4protoname
-%type <string>			ct_obj_kind
-%destructor { xfree($$); }     	ct_obj_kind
+%type <val>			ct_l4protoname ct_obj_type
 
 %%
 
@@ -843,18 +842,9 @@ add_cmd			:	TABLE		table_spec
 			{
 				$$ = cmd_alloc(CMD_ADD, CMD_OBJ_QUOTA, &$2, &@$, $3);
 			}
-			|	CT	STRING	obj_spec	ct_obj_alloc	'{' ct_block '}'	stmt_seperator
+			|	CT	HELPER	obj_spec	ct_obj_alloc	'{' ct_helper_block '}'	stmt_seperator
 			{
-				struct error_record *erec;
-				int type;
-
-				erec = ct_objtype_parse(&@$, $2, &type);
-				if (erec != NULL) {
-					erec_queue(erec, state->msgs);
-					YYERROR;
-				}
-
-				$$ = cmd_alloc_obj_ct(CMD_ADD, type, &$3, &@$, $4);
+				$$ = cmd_alloc_obj_ct(CMD_ADD, NFT_OBJECT_CT_HELPER, &$3, &@$, $4);
 			}
 			;
 
@@ -922,18 +912,9 @@ create_cmd		:	TABLE		table_spec
 			{
 				$$ = cmd_alloc(CMD_CREATE, CMD_OBJ_QUOTA, &$2, &@$, $3);
 			}
-			|	CT	STRING	obj_spec	ct_obj_alloc	'{' ct_block '}'	stmt_seperator
+			|	CT	HELPER	obj_spec	ct_obj_alloc	'{' ct_helper_block '}'	stmt_seperator
 			{
-				struct error_record *erec;
-				int type;
-
-				erec = ct_objtype_parse(&@$, $2, &type);
-				if (erec != NULL) {
-					erec_queue(erec, state->msgs);
-					YYERROR;
-				}
-
-				$$ = cmd_alloc_obj_ct(CMD_CREATE, type, &$3, &@$, $4);
+				$$ = cmd_alloc_obj_ct(CMD_CREATE, NFT_OBJECT_CT_HELPER, &$3, &@$, $4);
 			}
 			;
 
@@ -975,18 +956,9 @@ delete_cmd		:	TABLE		table_spec
 			{
 				$$ = cmd_alloc(CMD_DELETE, CMD_OBJ_QUOTA, &$2, &@$, NULL);
 			}
-			|	CT	STRING	obj_spec	ct_obj_alloc
+			|	CT	ct_obj_type	obj_spec	ct_obj_alloc
 			{
-				struct error_record *erec;
-				int type;
-
-				erec = ct_objtype_parse(&@$, $2, &type);
-				if (erec != NULL) {
-					erec_queue(erec, state->msgs);
-					YYERROR;
-				}
-
-				$$ = cmd_alloc_obj_ct(CMD_DELETE, type, &$3, &@$, $4);
+				$$ = cmd_alloc_obj_ct(CMD_DELETE, $2, &$3, &@$, $4);
 			}
 			;
 
@@ -1062,18 +1034,9 @@ list_cmd		:	TABLE		table_spec
 			{
 				$$ = cmd_alloc(CMD_LIST, CMD_OBJ_MAP, &$2, &@$, NULL);
 			}
-			|	CT		STRING	obj_spec
+			|	CT		ct_obj_type	obj_spec
 			{
-				struct error_record *erec;
-				int type;
-
-				erec = ct_objtype_parse(&@$, $2, &type);
-				if (erec != NULL) {
-					erec_queue(erec, state->msgs);
-					YYERROR;
-				}
-
-				$$ = cmd_alloc_obj_ct(CMD_LIST, type, &$3, &@$, NULL);
+				$$ = cmd_alloc_obj_ct(CMD_LIST, $2, &$3, &@$, NULL);
 			}
 			|       CT              STRING  TABLE   table_spec
 			{
@@ -1282,19 +1245,10 @@ table_block		:	/* empty */	{ $$ = $<table>-1; }
 				list_add_tail(&$4->list, &$1->objs);
 				$$ = $1;
 			}
-			|	table_block	CT	ct_obj_kind	obj_identifier  obj_block_alloc '{'     ct_block     '}' stmt_seperator
+			|	table_block	CT	HELPER	obj_identifier  obj_block_alloc '{'     ct_helper_block     '}' stmt_seperator
 			{
-				struct error_record *erec;
-				int type;
-
-				erec = ct_objtype_parse(&@$, $3, &type);
-				if (erec != NULL) {
-					erec_queue(erec, state->msgs);
-					YYERROR;
-				}
-
 				$5->location = @4;
-				$5->type = type;
+				$5->type = NFT_OBJECT_CT_HELPER;
 				handle_merge(&$5->handle, &$4);
 				handle_free(&$4);
 				list_add_tail(&$5->list, &$1->objs);
@@ -1494,10 +1448,10 @@ quota_block		:	/* empty */	{ $$ = $<obj>-1; }
 			}
 			;
 
-ct_block		:	/* empty */	{ $$ = $<obj>-1; }
-			|       ct_block     common_block
-			|       ct_block     stmt_seperator
-			|       ct_block     ct_config
+ct_helper_block		:	/* empty */	{ $$ = $<obj>-1; }
+			|       ct_helper_block     common_block
+			|       ct_helper_block     stmt_seperator
+			|       ct_helper_block     ct_helper_config
 			{
 				$$ = $1;
 			}
@@ -2697,14 +2651,14 @@ quota_obj		:	quota_config
 			}
 			;
 
-ct_obj_kind		:	STRING		{ $$ = $1; }
+ct_obj_type		:	HELPER		{ $$ = NFT_OBJECT_CT_HELPER; }
 			;
 
 ct_l4protoname		:	TCP	{ $$ = IPPROTO_TCP; }
 			|	UDP	{ $$ = IPPROTO_UDP; }
 			;
 
-ct_config		:	TYPE	QUOTED_STRING	PROTOCOL	ct_l4protoname	stmt_seperator
+ct_helper_config		:	TYPE	QUOTED_STRING	PROTOCOL	ct_l4protoname	stmt_seperator
 			{
 				struct ct_helper *ct;
 				int ret;
@@ -2728,7 +2682,6 @@ ct_config		:	TYPE	QUOTED_STRING	PROTOCOL	ct_l4protoname	stmt_seperator
 ct_obj_alloc		:
 			{
 				$$ = obj_alloc(&@$);
-				$$->type = NFT_OBJECT_CT_HELPER;
 			}
 			;
 
@@ -3160,6 +3113,7 @@ ct_expr			: 	CT	ct_key
 ct_key			:	L3PROTOCOL	{ $$ = NFT_CT_L3PROTOCOL; }
 			|	PROTOCOL	{ $$ = NFT_CT_PROTOCOL; }
 			|	MARK		{ $$ = NFT_CT_MARK; }
+			|	HELPER		{ $$ = NFT_CT_HELPER; }
 			|	ct_key_dir_optional
 			;
 ct_key_dir		:	SADDR		{ $$ = NFT_CT_SRC; }
@@ -3197,7 +3151,16 @@ ct_stmt_expr		:	expr
 
 ct_stmt			:	CT	ct_key		SET	expr
 			{
-				$$ = ct_stmt_alloc(&@$, $2, -1, $4);
+				switch ($2) {
+				case NFT_CT_HELPER:
+					$$ = objref_stmt_alloc(&@$);
+					$$->objref.type = NFT_OBJECT_CT_HELPER;
+					$$->objref.expr = $4;
+					break;
+				default:
+					$$ = ct_stmt_alloc(&@$, $2, -1, $4);
+					break;
+				}
 			}
 			|	CT	STRING		SET	ct_stmt_expr
 			{
@@ -3209,17 +3172,7 @@ ct_stmt			:	CT	ct_key		SET	expr
 					erec_queue(erec, state->msgs);
 					YYERROR;
 				}
-
-				switch (key) {
-				case NFT_CT_HELPER:
-					$$ = objref_stmt_alloc(&@$);
-					$$->objref.type = NFT_OBJECT_CT_HELPER;
-					$$->objref.expr = $4;
-					break;
-				default:
-					$$ = ct_stmt_alloc(&@$, key, -1, $4);
-					break;
-				}
+				$$ = ct_stmt_alloc(&@$, key, -1, $4);
 			}
 			|	CT	STRING	ct_key_dir_optional SET	expr
 			{
diff --git a/src/rule.c b/src/rule.c
index ee510fe0123b..ed6654b1f821 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -1664,10 +1664,13 @@ static int do_command_describe(struct netlink_ctx *ctx, struct cmd *cmd)
 }
 
 struct cmd *cmd_alloc_obj_ct(enum cmd_ops op, int type, const struct handle *h,
-			     const struct location *loc, void *data)
+			     const struct location *loc, struct obj *obj)
 {
 	enum cmd_obj cmd_obj;
 
+	if (obj)
+		obj->type = type;
+
 	switch (type) {
 	case NFT_OBJECT_CT_HELPER:
 		cmd_obj = CMD_OBJ_CT_HELPER;
@@ -1676,7 +1679,7 @@ struct cmd *cmd_alloc_obj_ct(enum cmd_ops op, int type, const struct handle *h,
 		BUG("missing type mapping");
 	}
 
-	return cmd_alloc(op, cmd_obj, h, loc, data);
+	return cmd_alloc(op, cmd_obj, h, loc, obj);
 }
 
 int do_command(struct netlink_ctx *ctx, struct cmd *cmd)
diff --git a/src/scanner.l b/src/scanner.l
index 86a03f3b3bdb..c0c48a0dea29 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -478,6 +478,7 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
 "nexthop"		{ return NEXTHOP; }
 
 "ct"			{ return CT; }
+"helper"		{ return HELPER; }
 "l3proto"		{ return L3PROTOCOL; }
 "proto-src"		{ return PROTO_SRC; }
 "proto-dst"		{ return PROTO_DST; }
-- 
2.13.0

[PATCH 03/17] parser: use scanner tokens again for ct key handling

[Florian Westphal]

This partially reverts commit
c992153402c78d91e8beba791171bced21c62d3f
("ct: allow resolving ct keys at run time").

It was a bad idea; problem is that if we want to support
a syntax like

ct origin ip saddr @foo

(to indicate that we want to match ip addresses, not ipv6), then we get
a failure here because "ip" is a token and not a string.

We could work around this by convertig ip to a string in that case
but thats worse than using tokens again.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/ct.h       |  2 --
 src/ct.c           | 35 -----------------------------------
 src/parser_bison.y | 54 ++++++++++++++++++++++--------------------------------
 src/scanner.l      |  6 ++++++
 4 files changed, 28 insertions(+), 69 deletions(-)

diff --git a/include/ct.h b/include/ct.h
index ae900ee4fb61..69ccc913dd74 100644
--- a/include/ct.h
+++ b/include/ct.h
@@ -29,8 +29,6 @@ extern void ct_expr_update_type(struct proto_ctx *ctx, struct expr *expr);
 
 extern struct error_record *ct_dir_parse(const struct location *loc,
 					 const char *str, int8_t *dir);
-extern struct error_record *ct_key_parse(const struct location *loc, const char *str,
-					 unsigned int *key);
 extern struct error_record *ct_objtype_parse(const struct location *loc, const char *str, int *type);
 
 extern struct stmt *notrack_stmt_alloc(const struct location *loc);
diff --git a/src/ct.c b/src/ct.c
index 87fe08bc62f8..f76f7867a77d 100644
--- a/src/ct.c
+++ b/src/ct.c
@@ -349,41 +349,6 @@ struct error_record *ct_dir_parse(const struct location *loc, const char *str,
 	return error(loc, "Could not parse direction %s", str);
 }
 
-struct error_record *ct_key_parse(const struct location *loc, const char *str,
-				  unsigned int *key)
-{
-	int ret, len, offset = 0;
-	const char *sep = "";
-	unsigned int i;
-	char buf[1024];
-	size_t size;
-
-	for (i = 0; i < array_size(ct_templates); i++) {
-		if (!ct_templates[i].token || strcmp(ct_templates[i].token, str))
-			continue;
-
-		*key = i;
-		return NULL;
-	}
-
-	len = (int)sizeof(buf);
-	size = sizeof(buf);
-
-	for (i = 0; i < array_size(ct_templates); i++) {
-		if (!ct_templates[i].token)
-			continue;
-
-		if (offset)
-			sep = ", ";
-
-		ret = snprintf(buf+offset, len, "%s%s", sep, ct_templates[i].token);
-		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
-		assert(offset < (int)sizeof(buf));
-	}
-
-	return error(loc, "syntax error, unexpected %s, known keys are %s", str, buf);
-}
-
 struct expr *ct_expr_alloc(const struct location *loc, enum nft_ct_keys key,
 			   int8_t direction)
 {
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 79918399368e..86f0464295eb 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -379,6 +379,12 @@ static void location_update(struct location *loc, struct location *rhs, int n)
 %token PROTO_SRC		"proto-src"
 %token PROTO_DST		"proto-dst"
 %token ZONE			"zone"
+%token DIRECTION		"direction"
+%token STATE			"state"
+%token STATUS			"status"
+%token EXPIRATION		"expiration"
+%token LABEL			"label"
+%token EVENT			"event"
 
 %token COUNTER			"counter"
 %token NAME			"name"
@@ -3082,19 +3088,6 @@ ct_expr			: 	CT	ct_key
 			{
 				$$ = ct_expr_alloc(&@$, $2, -1);
 			}
-			| 	CT	STRING
-			{
-				struct error_record *erec;
-				unsigned int key;
-
-				erec = ct_key_parse(&@$, $2, &key);
-				if (erec != NULL) {
-					erec_queue(erec, state->msgs);
-					YYERROR;
-				}
-
-				$$ = ct_expr_alloc(&@$, key, -1);
-			}
 			|	CT	STRING	ct_key_dir
 			{
 				struct error_record *erec;
@@ -3110,18 +3103,25 @@ ct_expr			: 	CT	ct_key
 			}
 			;
 
-ct_key			:	L3PROTOCOL	{ $$ = NFT_CT_L3PROTOCOL; }
-			|	PROTOCOL	{ $$ = NFT_CT_PROTOCOL; }
+ct_key			:	STATE		{ $$ = NFT_CT_STATE; }
+			|	DIRECTION	{ $$ = NFT_CT_DIRECTION; }
+			|	STATUS		{ $$ = NFT_CT_STATUS; }
 			|	MARK		{ $$ = NFT_CT_MARK; }
 			|	HELPER		{ $$ = NFT_CT_HELPER; }
+			|	EXPIRATION	{ $$ = NFT_CT_EXPIRATION; }
+			|	LABEL		{ $$ = NFT_CT_LABELS; }
+			|	L3PROTOCOL	{ $$ = NFT_CT_L3PROTOCOL; }
+			|	PROTOCOL	{ $$ = NFT_CT_PROTOCOL; }
+			|	EVENT		{ $$ = NFT_CT_EVENTMASK; }
 			|	ct_key_dir_optional
 			;
-ct_key_dir		:	SADDR		{ $$ = NFT_CT_SRC; }
-			|	DADDR		{ $$ = NFT_CT_DST; }
-			|	L3PROTOCOL	{ $$ = NFT_CT_L3PROTOCOL; }
+
+ct_key_dir		:	L3PROTOCOL	{ $$ = NFT_CT_L3PROTOCOL; }
 			|	PROTOCOL	{ $$ = NFT_CT_PROTOCOL; }
 			|	PROTO_SRC	{ $$ = NFT_CT_PROTO_SRC; }
 			|	PROTO_DST	{ $$ = NFT_CT_PROTO_DST; }
+			|	SADDR		{ $$ = NFT_CT_SRC; }
+			|	DADDR		{ $$ = NFT_CT_DST; }
 			|	ct_key_dir_optional
 			;
 
@@ -3149,9 +3149,11 @@ ct_stmt_expr		:	expr
 			|	list_stmt_expr
 			;
 
-ct_stmt			:	CT	ct_key		SET	expr
+ct_stmt			:	CT	ct_key		SET	ct_stmt_expr
 			{
-				switch ($2) {
+				unsigned int key = $2;
+
+				switch (key) {
 				case NFT_CT_HELPER:
 					$$ = objref_stmt_alloc(&@$);
 					$$->objref.type = NFT_OBJECT_CT_HELPER;
@@ -3162,18 +3164,6 @@ ct_stmt			:	CT	ct_key		SET	expr
 					break;
 				}
 			}
-			|	CT	STRING		SET	ct_stmt_expr
-			{
-				struct error_record *erec;
-				unsigned int key;
-
-				erec = ct_key_parse(&@$, $2, &key);
-				if (erec != NULL) {
-					erec_queue(erec, state->msgs);
-					YYERROR;
-				}
-				$$ = ct_stmt_alloc(&@$, key, -1, $4);
-			}
 			|	CT	STRING	ct_key_dir_optional SET	expr
 			{
 				struct error_record *erec;
diff --git a/src/scanner.l b/src/scanner.l
index c0c48a0dea29..f7717eb92f33 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -483,6 +483,12 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
 "proto-src"		{ return PROTO_SRC; }
 "proto-dst"		{ return PROTO_DST; }
 "zone"			{ return ZONE; }
+"direction"		{ return DIRECTION; }
+"state"			{ return STATE; }
+"status"		{ return STATUS; }
+"expiration"		{ return EXPIRATION; }
+"event"			{ return EVENT; }
+"label"			{ return LABEL; }
 
 "numgen"		{ return NUMGEN; }
 "inc"			{ return INC; }
-- 
2.13.0

[PATCH 04/17] parser: compact list of rhs keyword expressions

[Florian Westphal]

Condenses the copy/pastry via a define, will make it less of a hassle
to extend this list later if needed.

Based on earlier patch from Pablo.

Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/parser_bison.y | 72 ++++++++++++------------------------------------------
 1 file changed, 16 insertions(+), 56 deletions(-)

diff --git a/src/parser_bison.y b/src/parser_bison.y
index 86f0464295eb..a95c16adde61 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -97,6 +97,8 @@ static void location_update(struct location *loc, struct location *rhs, int n)
 
 #define YYLLOC_DEFAULT(Current, Rhs, N)	location_update(&Current, Rhs, N)
 
+#define symbol_value(loc, str) \
+	symbol_expr_alloc(loc, SYMBOL_VALUE, current_scope(state), str)
 %}
 
 /* Declaration section */
@@ -580,8 +582,8 @@ static void location_update(struct location *loc, struct location *rhs, int n)
 %type <expr>			flow_key_expr flow_key_expr_alloc
 %destructor { expr_free($$); }	flow_key_expr flow_key_expr_alloc
 
-%type <expr>			expr initializer_expr
-%destructor { expr_free($$); }	expr initializer_expr
+%type <expr>			expr initializer_expr keyword_expr
+%destructor { expr_free($$); }	expr initializer_expr keyword_expr
 
 %type <expr>			rhs_expr concat_rhs_expr basic_rhs_expr
 %destructor { expr_free($$); }	rhs_expr concat_rhs_expr basic_rhs_expr
@@ -2795,39 +2797,21 @@ boolean_expr		:	boolean_keys
 			}
 			;
 
+keyword_expr		:	ETHER                   { $$ = symbol_value(&@$, "ether"); }
+			|	IP			{ $$ = symbol_value(&@$, "ip"); }
+			|	IP6			{ $$ = symbol_value(&@$, "ip6"); }
+			|	VLAN			{ $$ = symbol_value(&@$, "vlan"); }
+			|	ARP			{ $$ = symbol_value(&@$, "arp"); }
+			|	DNAT			{ $$ = symbol_value(&@$, "dnat"); }
+			|	SNAT			{ $$ = symbol_value(&@$, "snat"); }
+			|	ECN			{ $$ = symbol_value(&@$, "ecn"); }
+			|	RESET			{ $$ = symbol_value(&@$, "reset"); }
+			;
+
 primary_rhs_expr	:	symbol_expr		{ $$ = $1; }
 			|	integer_expr		{ $$ = $1; }
 			|	boolean_expr		{ $$ = $1; }
-			|	ETHER
-			{
-				$$ = symbol_expr_alloc(&@$, SYMBOL_VALUE,
-						       current_scope(state),
-						       "ether");
-			}
-			|	IP
-			{
-				$$ = symbol_expr_alloc(&@$, SYMBOL_VALUE,
-						       current_scope(state),
-						       "ip");
-			}
-			|	IP6
-			{
-				$$ = symbol_expr_alloc(&@$, SYMBOL_VALUE,
-						       current_scope(state),
-						       "ip6");
-			}
-			|	VLAN
-			{
-				$$ = symbol_expr_alloc(&@$, SYMBOL_VALUE,
-						       current_scope(state),
-						       "vlan");
-			}
-			|	ARP
-			{
-				$$ = symbol_expr_alloc(&@$, SYMBOL_VALUE,
-						       current_scope(state),
-						       "arp");
-			}
+			|	keyword_expr		{ $$ = $1; }
 			|	TCP
 			{
 				uint8_t data = IPPROTO_TCP;
@@ -2905,30 +2889,6 @@ primary_rhs_expr	:	symbol_expr		{ $$ = $1; }
 							 BYTEORDER_HOST_ENDIAN,
 							 sizeof(data) * BITS_PER_BYTE, &data);
 			}
-			|	SNAT
-			{
-				$$ = symbol_expr_alloc(&@$, SYMBOL_VALUE,
-						       current_scope(state),
-						       "snat");
-			}
-			|	DNAT
-			{
-				$$ = symbol_expr_alloc(&@$, SYMBOL_VALUE,
-						       current_scope(state),
-						       "dnat");
-			}
-			|	ECN
-			{
-				$$ = symbol_expr_alloc(&@$, SYMBOL_VALUE,
-						       current_scope(state),
-						       "ecn");
-			}
-			|	RESET
-			{
-				$$ = symbol_expr_alloc(&@$, SYMBOL_VALUE,
-						       current_scope(state),
-						       "reset");
-			}
 			;
 
 relational_op		:	EQ		{ $$ = OP_EQ; }
-- 
2.13.0

[PATCH 05/17] bison: permit 'label' on rhs side of expression

[Florian Westphal]

needed to make 'ct event set label' work, as its also a token.
This change is broken:
 ct event set label
 and
 ct event set label,new

 cause conflict in grammar.

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

diff --git a/src/parser_bison.y b/src/parser_bison.y
index a95c16adde61..66f031d3ca69 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -2806,6 +2806,7 @@ keyword_expr		:	ETHER                   { $$ = symbol_value(&@$, "ether"); }
 			|	SNAT			{ $$ = symbol_value(&@$, "snat"); }
 			|	ECN			{ $$ = symbol_value(&@$, "ecn"); }
 			|	RESET			{ $$ = symbol_value(&@$, "reset"); }
+			|	LABEL			{ $$ = symbol_value(&@$, "label"); }
 			;
 
 primary_rhs_expr	:	symbol_expr		{ $$ = $1; }
-- 
2.13.0

[PATCH 06/17] bison: permit keywords in list_stmt_expressions

[Florian Westphal]

previous patch makes 'ct event set label' work, but we also need
to allow "set new, label".

This changes the definition to also contain keyword symbol expressions.
Unfortunately, the older "|" syntax, "ct event new | label" (not in any release)
doesn't work anymore because it requires primary_expr to work and that
causes ambiguities in the parser (we can only re-use keywords if they're
restricted to RHS expressions).

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/parser_bison.y | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/parser_bison.y b/src/parser_bison.y
index 66f031d3ca69..577f4bee167e 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -587,8 +587,8 @@ static void location_update(struct location *loc, struct location *rhs, int n)
 
 %type <expr>			rhs_expr concat_rhs_expr basic_rhs_expr
 %destructor { expr_free($$); }	rhs_expr concat_rhs_expr basic_rhs_expr
-%type <expr>			primary_rhs_expr list_rhs_expr shift_rhs_expr
-%destructor { expr_free($$); }	primary_rhs_expr list_rhs_expr shift_rhs_expr
+%type <expr>			primary_rhs_expr list_rhs_expr shift_rhs_expr	symbol_rhs_expr
+%destructor { expr_free($$); }	primary_rhs_expr list_rhs_expr shift_rhs_expr	symbol_rhs_expr
 %type <expr>			and_rhs_expr exclusive_or_rhs_expr inclusive_or_rhs_expr
 %destructor { expr_free($$); }	and_rhs_expr exclusive_or_rhs_expr inclusive_or_rhs_expr
 
@@ -3092,13 +3092,17 @@ ct_key_dir_optional	:	BYTES		{ $$ = NFT_CT_BYTES; }
 			|	ZONE		{ $$ = NFT_CT_ZONE; }
 			;
 
-list_stmt_expr		:	symbol_expr	COMMA	symbol_expr
+symbol_rhs_expr		:	symbol_expr
+			|	keyword_expr
+			;
+
+list_stmt_expr		:	symbol_rhs_expr	COMMA	symbol_rhs_expr
 			{
 				$$ = list_expr_alloc(&@$);
 				compound_expr_add($$, $1);
 				compound_expr_add($$, $3);
 			}
-			|	list_stmt_expr	COMMA		symbol_expr
+			|	list_stmt_expr	COMMA		symbol_rhs_expr
 			{
 				$1->location = @$;
 				compound_expr_add($1, $3);
-- 
2.13.0

[PATCH 07/17] tests: ct: remove unsupported syntax

[Florian Westphal]

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 tests/py/any/ct.t         | 3 +--
 tests/py/any/ct.t.payload | 5 -----
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/tests/py/any/ct.t b/tests/py/any/ct.t
index f02bd0420e68..833ffcf8bf4d 100644
--- a/tests/py/any/ct.t
+++ b/tests/py/any/ct.t
@@ -105,8 +105,7 @@ ct original mark 42;fail
 ct mark original;fail
 
 ct event set new;ok
-ct event set new or related or destroy or foobar;fail
-ct event set 'new | related | destroy | label';ok;ct event set new,related,destroy,label
+ct event set new, related , foobar;fail
 ct event set new,destroy;ok
 ct event set 1;ok;ct event set new
 ct event set 0x0;ok
diff --git a/tests/py/any/ct.t.payload b/tests/py/any/ct.t.payload
index 20acbb9b63d1..3679ce0e58d4 100644
--- a/tests/py/any/ct.t.payload
+++ b/tests/py/any/ct.t.payload
@@ -396,11 +396,6 @@ ip test-ip4 output
   [ immediate reg 1 0x00000001 ]
   [ ct set event with reg 1 ]
 
-# ct event set 'new | related | destroy | label'
-ip test-ip4 output
-  [ immediate reg 1 0x00000407 ]
-  [ ct set event with reg 1 ]
-
 # ct event set new,destroy
 ip test-ip4 output
   [ immediate reg 1 0x00000005 ]
-- 
2.13.0

[PATCH 08/17] 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        | 27 ++++++++++++++++++++++++---
 5 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/include/ct.h b/include/ct.h
index 69ccc913dd74..1c0ef0e423ae 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 error_record *ct_dir_parse(const struct location *loc,
diff --git a/include/expression.h b/include/expression.h
index 3e67938a6390..0a07646ac4ad 100644
--- a/include/expression.h
+++ b/include/expression.h
@@ -300,6 +300,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 f76f7867a77d..ffdc6a52ac97 100644
--- a/src/ct.c
+++ b/src/ct.c
@@ -350,7 +350,7 @@ struct error_record *ct_dir_parse(const struct location *loc, const char *str,
 }
 
 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;
@@ -359,6 +359,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 3ee07c0a1306..c523c8ed4862 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -699,7 +699,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 577f4bee167e..352acb5dda20 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -649,7 +649,7 @@ static void location_update(struct location *loc, struct location *rhs, int n)
 
 %type <expr>			ct_expr		ct_stmt_expr
 %destructor { expr_free($$); }	ct_expr		ct_stmt_expr
-%type <val>			ct_key		ct_key_dir	ct_key_dir_optional
+%type <val>			ct_key		ct_key_dir	ct_key_dir_optional	ct_key_proto	ct_key_proto_field
 
 %type <expr>			fib_expr
 %destructor { expr_free($$); }	fib_expr
@@ -3047,7 +3047,7 @@ rt_key			:	CLASSID		{ $$ = NFT_RT_CLASSID; }
 
 ct_expr			: 	CT	ct_key
 			{
-				$$ = ct_expr_alloc(&@$, $2, -1);
+				$$ = ct_expr_alloc(&@$, $2, -1, NFPROTO_UNSPEC);
 			}
 			|	CT	STRING	ct_key_dir
 			{
@@ -3060,7 +3060,20 @@ ct_expr			: 	CT	ct_key
 					YYERROR;
 				}
 
-				$$ = ct_expr_alloc(&@$, $3, direction);
+				$$ = ct_expr_alloc(&@$, $3, direction, NFPROTO_UNSPEC);
+			}
+			|	CT	STRING	ct_key_proto ct_key_proto_field
+			{
+				struct error_record *erec;
+				int8_t direction;
+
+				erec = ct_dir_parse(&@$, $2, &direction);
+				if (erec != NULL) {
+					erec_queue(erec, state->msgs);
+					YYERROR;
+				}
+
+				$$ = ct_expr_alloc(&@$, $4, direction, $3);
 			}
 			;
 
@@ -3086,6 +3099,14 @@ ct_key_dir		:	L3PROTOCOL	{ $$ = NFT_CT_L3PROTOCOL; }
 			|	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.0

[PATCH 09/17] 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 0a07646ac4ad..10df3918c946 100644
--- a/include/expression.h
+++ b/include/expression.h
@@ -299,6 +299,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 ffdc6a52ac97..a4ea6192074a 100644
--- a/src/ct.c
+++ b/src/ct.c
@@ -362,8 +362,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.0

[PATCH 10/17] 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 a4ea6192074a..a5fd04ffbfaa 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 = {
@@ -389,10 +389,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)
@@ -402,7 +403,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 ca8b63b74fdc..2d63de0c84e4 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -685,27 +685,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.0

[PATCH 11/17] 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 c523c8ed4862..d24af46c89a8 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -1367,12 +1367,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;
@@ -1388,40 +1405,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.0

[PATCH 12/17] 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.0

[PATCH 13/17] 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 a5fd04ffbfaa..c0568874a748 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)
@@ -394,10 +409,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;
@@ -414,7 +432,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.0

[PATCH 14/17] 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.0

[PATCH 15/17] 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 2d63de0c84e4..12ee091771aa 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -651,32 +651,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 352acb5dda20..ae3bbcf54801 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -642,7 +642,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>			list_stmt_expr
 %destructor { expr_free($$); }	list_stmt_expr
@@ -3035,10 +3035,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 eb5f9c33a595..a39545bfeb27 100644
--- a/src/rt.c
+++ b/src/rt.c
@@ -77,7 +77,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.0

[PATCH 16/17] 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         | 9 +++++++--
 tests/py/inet/rt.t.payload | 4 +---
 tests/py/ip/rt.t           | 3 ++-
 tests/py/ip6/rt0.t         | 2 +-
 4 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/tests/py/inet/rt.t b/tests/py/inet/rt.t
index f2dcbdc41da9..2135a7938f75 100644
--- a/tests/py/inet/rt.t
+++ b/tests/py/inet/rt.t
@@ -4,5 +4,10 @@
 
 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
diff --git a/tests/py/inet/rt.t.payload b/tests/py/inet/rt.t.payload
index d94973e03ffa..49098475e1b6 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.0

[PATCH 17/17] 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 970acb541e00..99cdfe42c633 100644
--- a/doc/nft.xml
+++ b/doc/nft.xml
@@ -492,7 +492,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>
 
@@ -2265,8 +2264,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>
@@ -3385,8 +3384,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>
@@ -3395,6 +3392,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.0

Re: [nft crap] ct original ip saddr ... handling

[Pablo Neira Ayuso]

On Wed, Jun 28, 2017 at 12:06:42PM +0200, Florian Westphal wrote:
> I am running out of time so I have to send this unfinished/non-working
> state.
> 
> It breaks because ct is riddled with conflicts,
> in nft we've handled original/reply as STRING to avoid conflicts
> with 'arp reply', so we cannot add
> 
> ct original ip saddr
> 
> because it is
> ct STRING	IP	SADDR
> 
> and that conflicts with basic use where 'ip saddr' could be payload
> expression, and STRING is one of the normal ct tokens and not a direction.
> 
> I am trying to fix this here by moving all ct keywords back to tokens.

Yes. We have a very compact syntax here, tokens helps bison decide
where to go. If we would have a well-structure syntax, looking less
human-readable, it would be more simple. But given what we have, we
have to use many tokens.

> There are no shift/reduce errors, things compile fine, and all
> test cases work.  Its just that we break 'ct event set label':
> 
> Works:
> ct event set new or reply
> ct event set new,reply
> ct event set new,label
> fails:
> ct event set label ('expects COMMA')

This can be fixed, it's just a matter we need more time, right?

No problem, we can take the time.

Thanks for working on this!

Re: [nft crap] ct original ip saddr ... handling

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > There are no shift/reduce errors, things compile fine, and all
> > test cases work.  Its just that we break 'ct event set label':
> > 
> > Works:
> > ct event set new or reply
> > ct event set new,reply
> > ct event set new,label
> > fails:
> > ct event set label ('expects COMMA')
> 
> This can be fixed, it's just a matter we need more time, right?

Actually this makes it work for me:

diff --git a/src/parser_bison.y b/src/parser_bison.y
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -2432,11 +2432,11 @@ fib_tuple		:  	fib_flag	DOT	fib_tuple
 			;
 
 shift_expr		:	primary_expr
-			|	shift_expr		LSHIFT		primary_expr
+			|	shift_expr		LSHIFT		primary_rhs_expr
 			{
 				$$ = binop_expr_alloc(&@$, OP_LSHIFT, $1, $3);
 			}
-			|	shift_expr		RSHIFT		primary_expr
+			|	shift_expr		RSHIFT		primary_rhs_expr
 			{
 				$$ = binop_expr_alloc(&@$, OP_RSHIFT, $1, $3);
 			}
@@ -3154,6 +3154,7 @@ list_stmt_expr		:	symbol_rhs_expr	COMMA	symbol_rhs_expr
 			;
 
 ct_stmt_expr		:	expr
+			|	keyword_expr
 			|	list_stmt_expr
 			;

(I added keyword_expr before and that causes reduce error, but did
 not have time to figure out where they come from).

Restricting shift_expr RHS to primary_rhs_expr makes this work for
me, all test cases that are expected to pass work.

Only effect AFAICS this has is that this:

nft add rule f i iif lshift mark eq 0
<cmdline>:1:25-28: Error: Right hand side of binary operation (<<) must be constant

turns into
<cmdline>:1:25-28: Error: syntax error, unexpected mark
add rule f i iif lshift mark eq 0

so I think restricting shift_expr rhs in grammar is fine.

Re: [nft crap] ct original ip saddr ... handling

[Florian Westphal]

Florian Westphal <fw@strlen.de> wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > There are no shift/reduce errors, things compile fine, and all
> > > test cases work.  Its just that we break 'ct event set label':
> > > 
> > > Works:
> > > ct event set new or reply
> > > ct event set new,reply
> > > ct event set new,label
> > > fails:
> > > ct event set label ('expects COMMA')
 
> > This can be fixed, it's just a matter we need more time, right?

Actually 'event set label' is simple to fix; just add keyword_expr
to the ct_stmt_expr list.

But another problem(?) is this:

works:
ct event label or new
ct event set reply or new
doesn't work:
ct event set label or new

(not strictly related to 'label', any other keyword like tcp, ip, etc.
 has same problem, they just don't overlap with event names so would not
 work anyway).

I currently see no way to resolve this, unfortunately.
For ct statements (and meta) we need to support plain expressions as SET
argument, at least in some cases, such as:

meta set mark or 42

This is ambiguous because we have both tokens and symbolic constants.

If we can live with the 'or' not being supported for ct event mask I
think we're fine (it will work when forcing string type, i.e.
ct event set "label" or new).

Also, the 'label, new' format will work fine.