Re: [nft PATCH] scanner: Accept all statements' first words in all scopes

[Florian Westphal <fw@strlen.de>]

Phil Sutter <phil@nwl.cc> wrote:
> To fix for token lookahead with exclusive start conditions, we must
> accept all keywords which may immediately follow the exclusive scope in
> that scope as well. This affects basically the first word of every
> statement which may follow a limit statement.

Hmm.  Can you give examples for some of these?

> -"@"			{ scanner_push_start_cond(yyscanner, SCANSTATE_AT); return AT; }
> +<*>"@"			{ scanner_push_start_cond(yyscanner, SCANSTATE_AT); return AT; }
> +<*>"set"		{ return SET; }

I have a hard time figureing these two out.

> +<*>"socket"		{ scanner_push_start_cond(yyscanner, SCANSTATE_EXPR_SOCKET); return SOCKET; }
> +<*>"tproxy"		{ scanner_push_start_cond(yyscanner, SCANSTATE_STMT_TPROXY); return TPROXY; }

Yes, I can see those at least theoretically.

> +<*>"delete"		{ return DELETE; }
> +<*>"update"		{ return UPDATE; }
> +<*>"add"		{ return ADD; }

Hmm.  Care to enlighten us?  Is this for a theoretical thing only?
(limit + flowtable...?)

> +<*>"reset"		{ scanner_push_start_cond(yyscanner, SCANSTATE_CMD_RESET); return RESET; }

?

> -"last"				{ scanner_push_start_cond(yyscanner, SCANSTATE_LAST); return LAST; }
> +<*>"last"		{ scanner_push_start_cond(yyscanner, SCANSTATE_LAST); return LAST; }

This one is also strange.  Normally, after limit, one would expect a
meaningful action (verdict, log, etc. -- something that has side
effects).

> +<*>"log"		{ scanner_push_start_cond(yyscanner, SCANSTATE_STMT_LOG); return LOG; }
> +<*>"queue"		{ scanner_push_start_cond(yyscanner, SCANSTATE_EXPR_QUEUE); return QUEUE;}

Makes sense.

> -"limit"			{ scanner_push_start_cond(yyscanner, SCANSTATE_LIMIT); return LIMIT; }
> +<*>"limit"		{ scanner_push_start_cond(yyscanner, SCANSTATE_LIMIT); return LIMIT; }

limit limit?


> -"quota"			{ scanner_push_start_cond(yyscanner, SCANSTATE_QUOTA); return QUOTA; }
> +<*>"quota"		{ scanner_push_start_cond(yyscanner, SCANSTATE_QUOTA); return QUOTA; }

limit + quota?  Strange combination, but ok.

> +<*>"reject"		{ scanner_push_start_cond(yyscanner, SCANSTATE_STMT_REJECT); return _REJECT; }

Makes sense.

> -"snat"			{ scanner_push_start_cond(yyscanner, SCANSTATE_STMT_NAT); return SNAT; }
> -"dnat"			{ scanner_push_start_cond(yyscanner, SCANSTATE_STMT_NAT); return DNAT; }
> -"masquerade"		{ scanner_push_start_cond(yyscanner, SCANSTATE_STMT_NAT); return MASQUERADE; }
> -"redirect"		{ scanner_push_start_cond(yyscanner, SCANSTATE_STMT_NAT); return REDIRECT; }
> +<*>"snat"		{ scanner_push_start_cond(yyscanner, SCANSTATE_STMT_NAT); return SNAT; }
> +<*>"dnat"		{ scanner_push_start_cond(yyscanner, SCANSTATE_STMT_NAT); return DNAT; }
> +<*>"masquerade"		{ scanner_push_start_cond(yyscanner, SCANSTATE_STMT_NAT); return MASQUERADE; }
> +<*>"redirect"		{ scanner_push_start_cond(yyscanner, SCANSTATE_STMT_NAT); return REDIRECT; }

Make no sense IMO, combining limit with nat table?
Is there a use case for this or are you just being conservative to not
break some random stuff?

> +<*>"th"			{ scanner_push_start_cond(yyscanner, SCANSTATE_EXPR_TH); return TRANSPORT_HDR; }

Yes, however, I'm not sure its worth it.  Because its a strange flow.
th ... limit ... -> makes sense to me.

limit ... th ... -> not so much.

'meta mark' or 'mark', or 'ct' , yes those make sense because it would
be natual to e.g. 'mark set x' for traffic shaping for example.

[ This is not a nack, I am just curious ]