parser_bison: consolidate stmt_expr rule

PATCH New Pablo Neira Ayuso 1 participant 3 messages Sep 26, 2017 netfilter

Share & tools

Pablo Neira Ayuso

author maintainer

2017-09-26 17:46 UTC #0 lore raw

PATCH v2 1/3 parser_bison: consolidate stmt_expr rule

Extend stmt_expr and use it from all of our statement rules. Add more rules to describe what we take from statement expressions, instead of reusing rhs_expr which is allowing way more things that we actually need here. This is causing us problems when extending the grammar.

After this patch, you will hit this:

parser_bison.y: warning: 2 shift/reduce conflicts [-Wconflicts-sr]

However, this is fixed by the follow up patches:

parser_bison: allow helper keyword in ct object kind
parser_bison: use keywords in ct expression

Signed-off-by Pablo Neira Ayuso <[email protected]>

---
v2: add keyword_expr to primary_stmt_expr so one of the tests/py don't break.
add new map_stmt_expr_set rule to fix one of the shell tests.

 src/parser_bison.y | 139 ++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 112 insertions(+), 27 deletions(-)

src/parser_bison.y +89 ~~-12~~

index 31a7e8be2bcd..163fbb4b6729 100644
@@ -557,8 +557,20 @@ static void location_update(struct location *loc, struct location *rhs, int n)
 %type <expr>			prefix_rhs_expr range_rhs_expr wildcard_rhs_expr
 %destructor { expr_free($$); }	prefix_rhs_expr range_rhs_expr wildcard_rhs_expr
 
-%type <expr>			stmt_expr concat_stmt_expr map_stmt_expr
-%destructor { expr_free($$); }	stmt_expr concat_stmt_expr map_stmt_expr
+%type <expr>			stmt_expr concat_stmt_expr map_stmt_expr map_stmt_expr_set
+%destructor { expr_free($$); }	stmt_expr concat_stmt_expr map_stmt_expr map_stmt_expr_set
+
+%type <expr>			multiton_stmt_expr
+%destructor { expr_free($$); }	multiton_stmt_expr
+%type <expr>			prefix_stmt_expr range_stmt_expr wildcard_stmt_expr
+%destructor { expr_free($$); }	prefix_stmt_expr range_stmt_expr wildcard_stmt_expr
+
+%type <expr>			primary_stmt_expr basic_stmt_expr
+%destructor { expr_free($$); }	primary_stmt_expr basic_stmt_expr
+%type <expr>			list_stmt_expr shift_stmt_expr
+%destructor { expr_free($$); }	list_stmt_expr shift_stmt_expr
+%type <expr>			and_stmt_expr exclusive_or_stmt_expr inclusive_or_stmt_expr
+%destructor { expr_free($$); }	and_stmt_expr exclusive_or_stmt_expr inclusive_or_stmt_expr
 
 %type <expr>			concat_expr
 %destructor { expr_free($$); }	concat_expr

@@ -582,8 +594,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 keyword_rhs_expr
-%destructor { expr_free($$); }	expr initializer_expr keyword_rhs_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

@@ -644,11 +656,8 @@ static void location_update(struct location *loc, struct location *rhs, int n)
 %destructor { expr_free($$); }	rt_expr
 %type <val>			rt_key
 
-%type <expr>			list_stmt_expr
-%destructor { expr_free($$); }	list_stmt_expr
-
-%type <expr>			ct_expr		ct_stmt_expr
-%destructor { expr_free($$); }	ct_expr		ct_stmt_expr
+%type <expr>			ct_expr
+%destructor { expr_free($$); }	ct_expr
 %type <val>			ct_key		ct_key_dir	ct_key_dir_optional
 
 %type <expr>			fib_expr

@@ -2206,8 +2215,55 @@ nat_stmt_alloc		:	SNAT
 			}
 			;
 
-concat_stmt_expr	:	primary_expr
-			|	concat_stmt_expr	DOT	primary_expr
+primary_stmt_expr	:	symbol_expr		{ $$ = $1; }
+			|	integer_expr		{ $$ = $1; }
+			|	boolean_expr		{ $$ = $1; }
+			|	meta_expr		{ $$ = $1; }
+			|	rt_expr			{ $$ = $1; }
+			|	ct_expr			{ $$ = $1; }
+			|	numgen_expr             { $$ = $1; }
+			|	hash_expr               { $$ = $1; }
+			|	payload_expr		{ $$ = $1; }
+			|	keyword_expr		{ $$ = $1; }
+			;
+
+shift_stmt_expr		:	primary_stmt_expr
+			|	shift_stmt_expr		LSHIFT		primary_stmt_expr
+			{
+				$$ = binop_expr_alloc(&@$, OP_LSHIFT, $1, $3);
+			}
+			|	shift_stmt_expr		RSHIFT		primary_rhs_expr
+			{
+				$$ = binop_expr_alloc(&@$, OP_RSHIFT, $1, $3);
+			}
+			;
+
+and_stmt_expr		:	shift_stmt_expr
+			|	and_stmt_expr		AMPERSAND	shift_stmt_expr
+			{
+				$$ = binop_expr_alloc(&@$, OP_AND, $1, $3);
+			}
+			;
+
+exclusive_or_stmt_expr	:	and_stmt_expr
+			|	exclusive_or_stmt_expr	CARET		and_stmt_expr
+			{
+				$$ = binop_expr_alloc(&@$, OP_XOR, $1, $3);
+			}
+			;
+
+inclusive_or_stmt_expr	:	exclusive_or_stmt_expr
+			|	inclusive_or_stmt_expr	'|'		exclusive_or_stmt_expr
+			{
+				$$ = binop_expr_alloc(&@$, OP_OR, $1, $3);
+			}
+			;
+
+basic_stmt_expr		:	inclusive_or_stmt_expr
+			;
+
+concat_stmt_expr	:	basic_stmt_expr
+			|	concat_stmt_expr	DOT	primary_stmt_expr
 			{
 				if ($$->ops->type != EXPR_CONCAT) {
 					$$ = concat_expr_alloc(&@$);

@@ -2226,15 +2282,48 @@ concat_stmt_expr	:	primary_expr
 			}
 			;
 
-map_stmt_expr		:	concat_stmt_expr	MAP	rhs_expr
+map_stmt_expr_set	:	set_expr
+			|	symbol_expr
+			;
+
+map_stmt_expr		:	concat_stmt_expr	MAP	map_stmt_expr_set
 			{
 				$$ = map_expr_alloc(&@$, $1, $3);
 			}
+			|	concat_stmt_expr	{ $$ = $1; }
+			;
+
+prefix_stmt_expr	:	basic_stmt_expr	SLASH	NUM
+			{
+				$$ = prefix_expr_alloc(&@$, $1, $3);
+			}
+			;
+
+range_stmt_expr		:	basic_stmt_expr	DASH	basic_stmt_expr
+			{
+				$$ = range_expr_alloc(&@$, $1, $3);
+			}
+			;
+
+wildcard_stmt_expr	:	ASTERISK
+			{

:1
+ struct expr *expr;
+
+ expr = constant_expr_alloc(&@$, &integer_type,
+ BYTEORDER_HOST_ENDIAN,
+ 0, NULL);
+ $$ = prefix_expr_alloc(&@$, expr, 0);
+ }
+ ;
+
+multiton_stmt_expr : prefix_stmt_expr
+ | range_stmt_expr
+ | wildcard_stmt_expr
;

stmt_expr : map_stmt_expr

-			|	multiton_rhs_expr
-			|	primary_rhs_expr
+			|	multiton_stmt_expr
+			|	list_stmt_expr

;

nat_stmt_args : stmt_expr
@@ -2967,7 +3056,7 @@ boolean_expr : boolean_keys
}
;

-keyword_rhs_expr : ETHER { $$ = symbol_value(&@$, "ether"); }
+keyword_expr : ETHER { $$ = symbol_value(&@$, "ether"); }

 			|	IP			{ $$ = symbol_value(&@$, "ip"); }
 			|	IP6			{ $$ = symbol_value(&@$, "ip6"); }
 			|	VLAN			{ $$ = symbol_value(&@$, "vlan"); }
@@ -2981,7 +3070,7 @@ keyword_rhs_expr	:	ETHER                   { $$ = symbol_value(&@$, "ether"); }
 primary_rhs_expr	:	symbol_expr		{ $$ = $1; }
 			|	integer_expr		{ $$ = $1; }
 			|	boolean_expr		{ $$ = $1; }
-			|	keyword_rhs_expr	{ $$ = $1; }
+			|	keyword_expr		{ $$ = $1; }
 			|	TCP

{
uint8_t data = IPPROTO_TCP;
@@ -3148,15 +3237,15 @@ meta_key_unqualified : MARK { $$ = NFT_META_MARK; }
| CGROUP { $$ = NFT_META_CGROUP; }
;

-meta_stmt : META meta_key SET expr
+meta_stmt : META meta_key SET stmt_expr
{
$$ = meta_stmt_alloc(&@$, $2, $4);
}
- | meta_key_unqualified SET expr
+ | meta_key_unqualified SET stmt_expr
{
$$ = meta_stmt_alloc(&@$, $1, $3);
}
- | META STRING SET expr
+ | META STRING SET stmt_expr
{
struct error_record *erec; unsigned int key;
@@ -3285,15 +3374,11 @@ list_stmt_expr : symbol_expr COMMA symbol_expr
}
;

-ct_stmt_expr : expr
- | list_stmt_expr
- ;
-
-ct_stmt : CT ct_key SET expr
+ct_stmt : CT ct_key SET stmt_expr
{
$$ = ct_stmt_alloc(&@$, $2, -1, $4);
}
- | CT STRING SET ct_stmt_expr
+ | CT STRING SET stmt_expr
{
struct error_record *erec; unsigned int key;
@@ -3316,7 +3401,7 @@ ct_stmt : CT ct_key SET expr
break;
}
}
- | CT STRING ct_key_dir_optional SET expr
+ | CT STRING ct_key_dir_optional SET stmt_expr
{
struct error_record *erec; int8_t direction;
@@ -3332,7 +3417,7 @@ ct_stmt : CT ct_key SET expr
}
;

-payload_stmt : payload_expr SET expr
+payload_stmt : payload_expr SET stmt_expr
{
if ($1->ops->type == EXPR_EXTHDR)
$$ = exthdr_stmt_alloc(&@$, $1, $3);

Signature · 1 line

2.1.4

Pablo Neira Ayuso

author maintainer

2017-09-26 17:46 UTC #1 lore raw

PATCH v2 2/3 parser_bison: use keywords in ct expression

Using string give us more chances to hit shift/reduce conflicts when extending this grammar, more specifically, from the stmt_expr rule, so add keywords for this.

Signed-off-by Pablo Neira Ayuso <[email protected]>

---
 src/parser_bison.y | 89 +++++++++++++++++++++---------------------------------
 src/scanner.l      |  9 ++++++
 2 files changed, 44 insertions(+), 54 deletions(-)

src/parser_bison.y +35 ~~-54~~

index 163fbb4b6729..0d916190e298 100644
@@ -386,6 +386,15 @@ 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 EVENT			"event"
+%token EXPIRATION		"expiration"
+%token HELPER			"helper"
+%token LABEL			"label"
+%token STATE			"state"
+%token STATUS			"status"
+%token ORIGINAL			"original"
+%token REPLY			"reply"
 
 %token COUNTER			"counter"
 %token NAME			"name"

@@ -658,7 +667,7 @@ static void location_update(struct location *loc, struct location *rhs, int n)
 
 %type <expr>			ct_expr
 %destructor { expr_free($$); }	ct_expr
-%type <val>			ct_key		ct_key_dir	ct_key_dir_optional
+%type <val>			ct_key		ct_dir	ct_key_dir_optional	ct_key_dir
 
 %type <expr>			fib_expr
 %destructor { expr_free($$); }	fib_expr

@@ -3065,6 +3074,9 @@ keyword_expr		:	ETHER                   { $$ = symbol_value(&@$, "ether"); }
 			|	SNAT			{ $$ = symbol_value(&@$, "snat"); }
 			|	ECN			{ $$ = symbol_value(&@$, "ecn"); }
 			|	RESET			{ $$ = symbol_value(&@$, "reset"); }
+			|	ORIGINAL		{ $$ = symbol_value(&@$, "original"); }
+			|	REPLY			{ $$ = symbol_value(&@$, "reply"); }
+			|	LABEL			{ $$ = symbol_value(&@$, "label"); }
 			;
 
 primary_rhs_expr	:	symbol_expr		{ $$ = $1; }

@@ -3310,41 +3322,33 @@ ct_expr			: 	CT	ct_key
 			{
 				$$ = ct_expr_alloc(&@$, $2, -1);
 			}
-			| 	CT	STRING
+			|	CT	ct_dir	ct_key_dir
 			{
-				struct error_record *erec;
-				unsigned int key;
-
-				erec = ct_key_parse(&@$, $2, &key);
-				xfree($2);
-				if (erec != NULL) {
-					erec_queue(erec, state->msgs);
-					YYERROR;
-				}
-
-				$$ = ct_expr_alloc(&@$, key, -1);
+				$$ = ct_expr_alloc(&@$, $3, $2);
 			}
-			|	CT	STRING	ct_key_dir
-			{
-				struct error_record *erec;
-				int8_t direction;
-
-				erec = ct_dir_parse(&@$, $2, &direction);
-				xfree($2);
-				if (erec != NULL) {
-					erec_queue(erec, state->msgs);
-					YYERROR;
-				}
+			;
 
-				$$ = ct_expr_alloc(&@$, $3, direction);
-			}
+ct_dir			:	ORIGINAL	{ $$ = IP_CT_DIR_ORIGINAL; }
+			|	REPLY		{ $$ = IP_CT_DIR_REPLY; }
 			;
 
 ct_key			:	L3PROTOCOL	{ $$ = NFT_CT_L3PROTOCOL; }
 			|	PROTOCOL	{ $$ = NFT_CT_PROTOCOL; }
 			|	MARK		{ $$ = NFT_CT_MARK; }
+			|	STATE		{ $$ = NFT_CT_STATE; }
+			|	DIRECTION	{ $$ = NFT_CT_DIRECTION; }
+			|	STATUS		{ $$ = NFT_CT_STATUS; }
+			|	EXPIRATION	{ $$ = NFT_CT_EXPIRATION; }
+			|	HELPER		{ $$ = NFT_CT_HELPER; }
+			|	SADDR		{ $$ = NFT_CT_SRC; }
+			|	DADDR		{ $$ = NFT_CT_DST; }
+			|	PROTO_SRC	{ $$ = NFT_CT_PROTO_SRC; }
+			|	PROTO_DST	{ $$ = NFT_CT_PROTO_DST; }
+			|	LABEL		{ $$ = NFT_CT_LABELS; }
+			|	EVENT		{ $$ = NFT_CT_EVENTMASK; }
 			|	ct_key_dir_optional
 			;
+
 ct_key_dir		:	SADDR		{ $$ = NFT_CT_SRC; }
 			|	DADDR		{ $$ = NFT_CT_DST; }
 			|	L3PROTOCOL	{ $$ = NFT_CT_L3PROTOCOL; }

@@ -3360,6 +3364,7 @@ ct_key_dir_optional	:	BYTES		{ $$ = NFT_CT_BYTES; }
 			|	ZONE		{ $$ = NFT_CT_ZONE; }
 			;
 
+
 list_stmt_expr		:	symbol_expr	COMMA	symbol_expr
 			{
 				$$ = list_expr_alloc(&@$);

@@ -3376,44 +3381,20 @@ list_stmt_expr		:	symbol_expr	COMMA	symbol_expr
 
 ct_stmt			:	CT	ct_key		SET	stmt_expr
 			{
-				$$ = ct_stmt_alloc(&@$, $2, -1, $4);
-			}
-			|	CT	STRING		SET	stmt_expr
-			{
-				struct error_record *erec;
-				unsigned int key;
-
-				erec = ct_key_parse(&@$, $2, &key);
-				xfree($2);
-				if (erec != NULL) {
-					erec_queue(erec, state->msgs);
-					YYERROR;
-				}
-
-				switch (key) {
+				switch ($2) {
 				case NFT_CT_HELPER:
 					$$ = objref_stmt_alloc(&@$);
 					$$->objref.type = NFT_OBJECT_CT_HELPER;
 					$$->objref.expr = $4;
 					break;
 				default:
-					$$ = ct_stmt_alloc(&@$, key, -1, $4);
+					$$ = ct_stmt_alloc(&@$, $2, -1, $4);
 					break;
 				}
 			}
-			|	CT	STRING	ct_key_dir_optional SET	stmt_expr
+			|	CT	ct_dir	ct_key_dir_optional SET	stmt_expr
 			{
-				struct error_record *erec;
-				int8_t direction;
-
-				erec = ct_dir_parse(&@$, $2, &direction);
-				xfree($2);
-				if (erec != NULL) {
-					erec_queue(erec, state->msgs);
-					YYERROR;
-				}
-
-				$$ = ct_stmt_alloc(&@$, $3, direction, $5);
+				$$ = ct_stmt_alloc(&@$, $3, $2, $5);
 			}
 			;

src/scanner.l +9 -0

index 0cfb6c50e418..186fb47eb763 100644
@@ -484,6 +484,15 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
 "proto-src"		{ return PROTO_SRC; }
 "proto-dst"		{ return PROTO_DST; }
 "zone"			{ return ZONE; }
+"original"		{ return ORIGINAL; }
+"reply"			{ return REPLY; }
+"direction"		{ return DIRECTION; }
+"event"			{ return EVENT; }
+"expiration"		{ return EXPIRATION; }
+"helper"		{ return HELPER; }
+"label"			{ return LABEL; }
+"state"			{ return STATE; }
+"status"		{ return STATUS; }
 
 "numgen"		{ return NUMGEN; }
 "inc"			{ return INC; }

Signature · 1 line

2.1.4

Pablo Neira Ayuso

author maintainer

2017-09-26 17:46 UTC #2 lore raw

PATCH v2 3/3 parser_bison: allow helper keyword in ct object kind

In reply to Pablo Neira Ayuso

The helper keyword clashes with the string rule, make sure we still accept ct helper object types from the parser.

Signed-off-by Pablo Neira Ayuso <[email protected]>

---
 src/parser_bison.y | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

src/parser_bison.y +6 -5

index 0d916190e298..970d773edc4f 100644
@@ -877,7 +877,7 @@ add_cmd			:	TABLE		table_spec
 			{
 				$$ = cmd_alloc(CMD_ADD, CMD_OBJ_QUOTA, &$2, &@$, $3);
 			}
-			|	CT	STRING	obj_spec	ct_obj_alloc	'{' ct_block '}'	stmt_separator
+			|	CT	ct_obj_kind	obj_spec	ct_obj_alloc	'{' ct_block '}'	stmt_separator
 			{
 				struct error_record *erec;
 				int type;

@@ -961,7 +961,7 @@ create_cmd		:	TABLE		table_spec
 			{
 				$$ = cmd_alloc(CMD_CREATE, CMD_OBJ_QUOTA, &$2, &@$, $3);
 			}
-			|	CT	STRING	obj_spec	ct_obj_alloc	'{' ct_block '}'	stmt_separator
+			|	CT	ct_obj_kind	obj_spec	ct_obj_alloc	'{' ct_block '}'	stmt_separator
 			{
 				struct error_record *erec;
 				int type;

@@ -1019,7 +1019,7 @@ delete_cmd		:	TABLE		table_spec
 			{
 				$$ = cmd_alloc(CMD_DELETE, CMD_OBJ_QUOTA, &$2, &@$, NULL);
 			}
-			|	CT	STRING	obj_spec	ct_obj_alloc
+			|	CT	ct_obj_kind	obj_spec	ct_obj_alloc
 			{
 				struct error_record *erec;
 				int type;

@@ -1123,7 +1123,7 @@ list_cmd		:	TABLE		table_spec
 			{
 				$$ = cmd_alloc(CMD_LIST, CMD_OBJ_MAP, &$2, &@$, NULL);
 			}
-			|	CT		STRING	obj_spec
+			|	CT		ct_obj_kind	obj_spec
 			{
 				struct error_record *erec;
 				int type;

@@ -1137,7 +1137,7 @@ list_cmd		:	TABLE		table_spec
 
 				$$ = cmd_alloc_obj_ct(CMD_LIST, type, &$3, &@$, NULL);
 			}
-			|       CT              STRING  TABLE   table_spec
+			|       CT		ct_obj_kind	TABLE   table_spec
 			{
 				int cmd;

@@ -2886,6 +2886,7 @@ quota_obj		:	quota_config
 			;
 
 ct_obj_kind		:	STRING		{ $$ = $1; }
+			|	HELPER		{ $$ = xstrdup("helper"); }
 			;
 
 ct_l4protoname		:	TCP	{ $$ = IPPROTO_TCP; }

Signature · 1 line

2.1.4