diff mbox

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

Message ID 20170628223139.GB9307@breakpoint.cc
State RFC
Delegated to: Pablo Neira
Headers show

Commit Message

Florian Westphal June 28, 2017, 10:31 p.m. UTC 
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:

(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.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Florian Westphal June 29, 2017, 12:39 a.m. UTC | #1 
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.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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
 			;