Skip to contentSkip to conversation

ct: make ct event set work with COMMA syntax

PATCH In review Florian Westphal 2 participants 7 messages Sep 27, 2017 netfilter

lore
Share & tools
lore

Florian Westphal

author maintainer
#0 lore raw

PATCH 1/3 ct: make ct event set work with COMMA syntax

'ct event set label' and 'ct event set new or label' work, but 'ct event set new, label' did not:

nft add rule filter input ct event set new,label
Error: syntax error, unexpected label

First two patches do a bit of refactoring, 3rd patch makes the real change.

I have a few more patches on top of this to avoid need of 'meta nfproto' to indicate address type when 'ct saddr' is used in inet family.

I will followup with those once these patches are in.

author maintainer
#1 lore raw

PATCH 1/3 src: prepare for future ct timeout policy support

Change all places that expect ct helper tokens (ct helper configuration) to CT HELPER. ct_obj_kind is removed.

When we add ct timeout support, we will add a new ct_timeout_block, plus extra rules. We won't extend ct_block, it prevents the parser from detecting bogus syntax that only makes sense for ct helper but not for something else for instance.

ct_block should be renamed to ct_helper_block, will be done in followup patch.

Signed-off-by Florian Westphal <[email protected]>
---
 include/ct.h       |  2 --
 include/rule.h     |  2 +-
 src/ct.c           | 10 ------
 src/parser_bison.y | 98 ++++++++++--------------------------------------------
 src/rule.c         |  7 ++--
 5 files changed, 23 insertions(+), 96 deletions(-)
include/ct.h +0 -2
index 895a6ee3b255..d9a11a3fab81 100644
@@ -27,8 +27,6 @@ extern struct expr *ct_expr_alloc(const struct location *loc,
 				  enum nft_ct_keys key, int8_t direction);
 extern void ct_expr_update_type(struct proto_ctx *ctx, struct expr *expr);
 
-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);
 
 extern const struct datatype ct_dir_type;
include/rule.h +1 -1
index e2a5c87b7002..6f52b17fc08c 100644
@@ -463,7 +463,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>
src/ct.c +0 -10
index 2b0e2a0924a4..0e9b17cd1a22 100644
@@ -334,16 +334,6 @@ static const struct expr_ops ct_expr_ops = {
 	.pctx_update	= ct_expr_pctx_update,
 };
 
-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)
 {
src/parser_bison.y +17 -81

173 lines not on this page · raw message

src/rule.c +5 -2
index 1e0558eaf075..30d3693afa54 100644
@@ -1742,10 +1742,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;

@@ -1754,7 +1757,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)
Signature · 1 line
2.13.5
maintainer Acked-by on v1 1/3
#2 lore raw

Re: [PATCH nft 1/3] src: prepare for future ct timeout policy support

Quoted 8 lines · Florian Westphal

Change all places that expect ct helper tokens (ct helper configuration) to CT HELPER. ct_obj_kind is removed.

When we add ct timeout support, we will add a new ct_timeout_block, plus extra rules. We won't extend ct_block, it prevents the parser from detecting bogus syntax that only makes sense for ct helper but not for something else for instance.

Quoted 4 lines

ct_block should be renamed to ct_helper_block, will be done in followup patch.

Signed-off-by Florian Westphal <[email protected]>
Acked-by Pablo Neira Ayuso <[email protected]>
Quoted 6 lines
 include/ct.h       |  2 --
 include/rule.h     |  2 +-
 src/ct.c           | 10 ------
 src/parser_bison.y | 98 ++++++++++--------------------------------------------
 src/rule.c         |  7 ++--
 5 files changed, 23 insertions(+), 96 deletions(-)

Nice diffstats.

author maintainer
#3 lore raw

PATCH 2/3 src: parser_bison: rename ct_block to ct_helper_block

In reply to Florian Westphal
Signed-off-by Florian Westphal <[email protected]>
---
 src/parser_bison.y | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
src/parser_bison.y +8 -8
index b9eb2b9d4c01..ce6a77a89847 100644
@@ -514,7 +514,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 limit_block
+%type <obj>			obj_block_alloc counter_block quota_block ct_helper_block limit_block
 %destructor { obj_free($$); }	obj_block_alloc
 
 %type <list>			stmt_list

@@ -876,7 +876,7 @@ add_cmd			:	TABLE		table_spec
 			{
 				$$ = cmd_alloc(CMD_ADD, CMD_OBJ_QUOTA, &$2, &@$, $3);
 			}
-			|	CT	HELPER	obj_spec	ct_obj_alloc	'{' ct_block '}'	stmt_separator
+			|	CT	HELPER	obj_spec	ct_obj_alloc	'{' ct_helper_block '}'	stmt_separator
 			{
 
 				$$ = cmd_alloc_obj_ct(CMD_ADD, NFT_OBJECT_CT_HELPER, &$3, &@$, $4);

@@ -951,7 +951,7 @@ create_cmd		:	TABLE		table_spec
 			{
 				$$ = cmd_alloc(CMD_CREATE, CMD_OBJ_QUOTA, &$2, &@$, $3);
 			}
-			|	CT	HELPER	obj_spec	ct_obj_alloc	'{' ct_block '}'	stmt_separator
+			|	CT	HELPER	obj_spec	ct_obj_alloc	'{' ct_helper_block '}'	stmt_separator
 			{
 				$$ = cmd_alloc_obj_ct(CMD_CREATE, NFT_OBJECT_CT_HELPER, &$3, &@$, $4);
 			}

@@ -1293,7 +1293,7 @@ table_block		:	/* empty */	{ $$ = $<table>-1; }
 				list_add_tail(&$4->list, &$1->objs);
 				$$ = $1;
 			}
-			|	table_block	CT	HELPER	obj_identifier  obj_block_alloc '{'     ct_block     '}' stmt_separator
+			|	table_block	CT	HELPER	obj_identifier  obj_block_alloc '{'     ct_helper_block     '}' stmt_separator
 			{
 				$5->location = @4;
 				$5->type = NFT_OBJECT_CT_HELPER;

@@ -1512,10 +1512,10 @@ quota_block		:	/* empty */	{ $$ = $<obj>-1; }
 			}
 			;
 
-ct_block		:	/* empty */	{ $$ = $<obj>-1; }
-			|       ct_block     common_block
-			|       ct_block     stmt_separator
-			|       ct_block     ct_helper_config
+ct_helper_block		:	/* empty */	{ $$ = $<obj>-1; }
+			|       ct_helper_block     common_block
+			|       ct_helper_block     stmt_separator
+			|       ct_helper_block     ct_helper_config
 			{
 				$$ = $1;
 			}
Signature · 1 line
2.13.5
author maintainer
#5 lore raw

PATCH 3/3 bison: permit keywords in list_stmt_expressions

In reply to Florian Westphal

'ct event set label' and 'ct event set new or label' work, but 'ct event set new, label' did not:

nft add rule filter input ct event set new,label
Error: syntax error, unexpected label

This changes the definition to also contain keyword symbol expressions.

Signed-off-by Florian Westphal <[email protected]>
---
 src/parser_bison.y        | 11 +++++++----
 tests/py/any/ct.t         |  1 +
 tests/py/any/ct.t.payload |  5 +++++
 3 files changed, 13 insertions(+), 4 deletions(-)
src/parser_bison.y +7 -4
index ce6a77a89847..87ae4c30712a 100644
@@ -609,8 +609,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
 

@@ -3302,14 +3302,17 @@ ct_key_dir_optional	:	BYTES		{ $$ = NFT_CT_BYTES; }
 			|	ZONE		{ $$ = NFT_CT_ZONE; }
 			;
 
+symbol_rhs_expr		:	symbol_expr
+			|	keyword_expr
+			;
 
-list_stmt_expr		:	symbol_expr	COMMA	symbol_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);
tests/py/any/ct.t +1 -0
index f02bd0420e68..fb3ae098b14a 100644
@@ -107,6 +107,7 @@ 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,destroy,label;ok
 ct event set new,destroy;ok
 ct event set 1;ok;ct event set new
 ct event set 0x0;ok
tests/py/any/ct.t.payload +5 -0
index 20acbb9b63d1..7ebf3f8d327e 100644
@@ -401,6 +401,11 @@ ip test-ip4 output
   [ immediate reg 1 0x00000407 ]
   [ 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 ]
Signature · 1 line
2.13.5
maintainer Acked-by on v1 3/3
#6 lore raw

Re: [PATCH nft 3/3] bison: permit keywords in list_stmt_expressions

Quoted 31 lines · Florian Westphal

'ct event set label' and 'ct event set new or label' work, but 'ct event set new, label' did not:

nft add rule filter input ct event set new,label
Error: syntax error, unexpected label

This changes the definition to also contain keyword symbol expressions.

Signed-off-by Florian Westphal <[email protected]>
---
 src/parser_bison.y        | 11 +++++++----
 tests/py/any/ct.t         |  1 +
 tests/py/any/ct.t.payload |  5 +++++
 3 files changed, 13 insertions(+), 4 deletions(-)
src/parser_bison.y +2 -2
index ce6a77a89847..87ae4c30712a 100644
@@ -609,8 +609,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

src/parser_bison.y ct_key_dir_optional : BYTES { $$ = NFT_CT_BYTES; } · line 3306

Quoted 5 lines
src/parser_bison.y +2 -0
@@ -3302,14 +3302,17 @@ ct_key_dir_optional	:	BYTES		{ $$ = NFT_CT_BYTES; }
 			|	ZONE		{ $$ = NFT_CT_ZONE; }
 			;
 
+symbol_rhs_expr		:	symbol_expr
+			|	keyword_expr

Could you rename this to symbol_stmt_expr?

So it's clear this refers to the grammar tree that deals with *stmt_expr. Apart from that nitpick.

Acked-by Pablo Neira Ayuso <[email protected]>