diff mbox series

[2/2,nft,WIP,v2] jump: Allow jump to a variable when using nft input files

Message ID 20190514152542.23406-2-ffmancera@riseup.net
State RFC
Delegated to: Pablo Neira
Headers show
Series [1/2,nft,WIP,v2] jump: Introduce chain_expr in jump statements | expand

Commit Message

Fernando F. Mancera May 14, 2019, 3:25 p.m. UTC 
This patch introduces the use of nft input files variables in 'jump'
statements, e.g.

define dest = chainame

add rule ip filter input jump $dest

Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
---
 src/parser_bison.y | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Fernando F. Mancera May 14, 2019, 3:43 p.m. UTC | #1 
This last patch does not work. The first one works fine with a string as
chain name.

# nft add table ip foo
# nft add chain ip foo bar {type filter hook input priority 0\;}
# nft add chain ip foo ber
# nft add rule ip foo ber counter
# nft add rule ip foo bar jump ber
# nft list ruleset

table ip foo {
	chain bar {
		type filter hook input priority filter; policy accept;
		jump ber
	}

	chain ber {
		counter packets 69 bytes 6138
	}
}

But when trying to execute "# nft -f file.nft", being file.nft:

> define dest = ber
> add rule ip foo bar jump $dest

I am getting the following error:

file.nft:3:26-30: Error: Can't parse symbolic netfilter verdict expressions
add rule ip foo bar jump $dest
			 ^^^^^
This error comes from symbol_parse() at expr_evaluate_symbol() after the
expr_evaluate() call added in the first patch.

On 5/14/19 5:25 PM, Fernando Fernandez Mancera wrote:
> This patch introduces the use of nft input files variables in 'jump'
> statements, e.g.
> 
> define dest = chainame
> 
> add rule ip filter input jump $dest
> 
> Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
> ---
>  src/parser_bison.y | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/src/parser_bison.y b/src/parser_bison.y
> index 69b5773..42fd71f 100644
> --- a/src/parser_bison.y
> +++ b/src/parser_bison.y
> @@ -3841,7 +3841,13 @@ verdict_expr		:	ACCEPT
>  			}
>  			;
>  
> -chain_expr		:	identifier
> +chain_expr		:	variable_expr
> +			{
> +				$$ = symbol_expr_alloc(&@$, SYMBOL_VALUE,
> +						       current_scope(state),
> +						       $1->sym->identifier);
> +			}
> +			|	identifier
>  			{
>  				$$ = constant_expr_alloc(&@$, &string_type,
>  							 BYTEORDER_HOST_ENDIAN,
>
Phil Sutter May 14, 2019, 4:17 p.m. UTC | #2 
Hi Fernando,

On Tue, May 14, 2019 at 05:43:39PM +0200, Fernando Fernandez Mancera wrote:
> This last patch does not work. The first one works fine with a string as
> chain name.
> 
[...]
> 
> But when trying to execute "# nft -f file.nft", being file.nft:
> 
> > define dest = ber
> > add rule ip foo bar jump $dest
> 
> I am getting the following error:
> 
> file.nft:3:26-30: Error: Can't parse symbolic netfilter verdict expressions
> add rule ip foo bar jump $dest
> 			 ^^^^^
> This error comes from symbol_parse() at expr_evaluate_symbol() after the
> expr_evaluate() call added in the first patch.

Yes, symbol_expr is used only for symbolic constants, therefore
symbol_parse() is very restrictive.

[...]
> > diff --git a/src/parser_bison.y b/src/parser_bison.y
> > index 69b5773..42fd71f 100644
> > --- a/src/parser_bison.y
> > +++ b/src/parser_bison.y
> > @@ -3841,7 +3841,13 @@ verdict_expr		:	ACCEPT
> >  			}
> >  			;
> >  
> > -chain_expr		:	identifier
> > +chain_expr		:	variable_expr
> > +			{
> > +				$$ = symbol_expr_alloc(&@$, SYMBOL_VALUE,
> > +						       current_scope(state),
> > +						       $1->sym->identifier);
> > +			}

I didn't test it, but you can probably just drop the curly braces and
everything inside here. 'variable_expr' already turns into an
expression (a variable_expr, not symbol_expr), which is probably what
you want.

> > +			|	identifier
> >  			{
> >  				$$ = constant_expr_alloc(&@$, &string_type,
> >  							 BYTEORDER_HOST_ENDIAN,
> > 
> 

Cheers, Phil
Fernando F. Mancera May 14, 2019, 4:24 p.m. UTC | #3 
Hi Phil,

On 5/14/19 6:17 PM, Phil Sutter wrote:
> Hi Fernando,
> 
> On Tue, May 14, 2019 at 05:43:39PM +0200, Fernando Fernandez Mancera wrote:
>> This last patch does not work. The first one works fine with a string as
>> chain name.
>>
> [...]
>> [...]
>> This error comes from symbol_parse() at expr_evaluate_symbol() after the
>> expr_evaluate() call added in the first patch.
> 
> Yes, symbol_expr is used only for symbolic constants, therefore
> symbol_parse() is very restrictive.
> 
> [...]>>> diff --git a/src/parser_bison.y b/src/parser_bison.y
>>> index 69b5773..42fd71f 100644
>>> --- a/src/parser_bison.y
>>> +++ b/src/parser_bison.y
>>> @@ -3841,7 +3841,13 @@ verdict_expr		:	ACCEPT
>>>  			}
>>>  			;
>>>  
>>> -chain_expr		:	identifier
>>> +chain_expr		:	variable_expr
>>> +			{
>>> +				$$ = symbol_expr_alloc(&@$, SYMBOL_VALUE,
>>> +						       current_scope(state),
>>> +						       $1->sym->identifier);
>>> +			}
> 
> I didn't test it, but you can probably just drop the curly braces and
> everything inside here. 'variable_expr' already turns into an
> expression (a variable_expr, not symbol_expr), which is probably what
> you want.
> 

I tried that first and I got the same error. I have tried it again.. and
I am getting the same error.

file.nft:1:15-17: Error: Can't parse symbolic netfilter verdict expressions
define dest = ber
              ^^^

Thanks! :-)
>>> +			|	identifier
>>>  			{
>>>  				$$ = constant_expr_alloc(&@$, &string_type,
>>>  							 BYTEORDER_HOST_ENDIAN,
>>>
>>
> 
> Cheers, Phil
>
Phil Sutter May 14, 2019, 7:31 p.m. UTC | #4 
Hi Fernando,

On Tue, May 14, 2019 at 06:24:48PM +0200, Fernando Fernandez Mancera wrote:
> Hi Phil,
> 
> On 5/14/19 6:17 PM, Phil Sutter wrote:
> > Hi Fernando,
> > 
> > On Tue, May 14, 2019 at 05:43:39PM +0200, Fernando Fernandez Mancera wrote:
> >> This last patch does not work. The first one works fine with a string as
> >> chain name.
> >>
> > [...]
> >> [...]
> >> This error comes from symbol_parse() at expr_evaluate_symbol() after the
> >> expr_evaluate() call added in the first patch.
> > 
> > Yes, symbol_expr is used only for symbolic constants, therefore
> > symbol_parse() is very restrictive.
> > 
> > [...]>>> diff --git a/src/parser_bison.y b/src/parser_bison.y
> >>> index 69b5773..42fd71f 100644
> >>> --- a/src/parser_bison.y
> >>> +++ b/src/parser_bison.y
> >>> @@ -3841,7 +3841,13 @@ verdict_expr		:	ACCEPT
> >>>  			}
> >>>  			;
> >>>  
> >>> -chain_expr		:	identifier
> >>> +chain_expr		:	variable_expr
> >>> +			{
> >>> +				$$ = symbol_expr_alloc(&@$, SYMBOL_VALUE,
> >>> +						       current_scope(state),
> >>> +						       $1->sym->identifier);
> >>> +			}
> > 
> > I didn't test it, but you can probably just drop the curly braces and
> > everything inside here. 'variable_expr' already turns into an
> > expression (a variable_expr, not symbol_expr), which is probably what
> > you want.
> > 
> 
> I tried that first and I got the same error. I have tried it again.. and
> I am getting the same error.
> 
> file.nft:1:15-17: Error: Can't parse symbolic netfilter verdict expressions
> define dest = ber
>               ^^^

OK, at least it didn't get worse. :)

I looked at the code and it seems you need to implement a 'parse'
callback for struct verdict_type. I guess existing 'parse' callback in
struct integer_type is a good example of how to do it - basically you
need to convert the symbol expression into a constant expression.

Sorry if that's not much help, I'm not really familiar with these
details. :)

Cheers, Phil
Fernando F. Mancera May 14, 2019, 8:34 p.m. UTC | #5 
On 5/14/19 9:31 PM, Phil Sutter wrote:
> Hi Fernando,
> 
> On Tue, May 14, 2019 at 06:24:48PM +0200, Fernando Fernandez Mancera wrote:
>> Hi Phil,
>>
>> On 5/14/19 6:17 PM, Phil Sutter wrote:
>>> Hi Fernando,
>>>
>>> On Tue, May 14, 2019 at 05:43:39PM +0200, Fernando Fernandez Mancera wrote:
>>>> This last patch does not work. The first one works fine with a string as
>>>> chain name.
>>>>
>>> [...]
>>>> [...]
>>>> This error comes from symbol_parse() at expr_evaluate_symbol() after the
>>>> expr_evaluate() call added in the first patch.
>>>
>>> Yes, symbol_expr is used only for symbolic constants, therefore
>>> symbol_parse() is very restrictive.
>>>
>>> [...]>>> diff --git a/src/parser_bison.y b/src/parser_bison.y
>>>>> index 69b5773..42fd71f 100644
>>>>> --- a/src/parser_bison.y
>>>>> +++ b/src/parser_bison.y
>>>>> @@ -3841,7 +3841,13 @@ verdict_expr		:	ACCEPT
>>>>>  			}
>>>>>  			;
>>>>>  
>>>>> -chain_expr		:	identifier
>>>>> +chain_expr		:	variable_expr
>>>>> +			{
>>>>> +				$$ = symbol_expr_alloc(&@$, SYMBOL_VALUE,
>>>>> +						       current_scope(state),
>>>>> +						       $1->sym->identifier);
>>>>> +			}
>>>
>>> I didn't test it, but you can probably just drop the curly braces and
>>> everything inside here. 'variable_expr' already turns into an
>>> expression (a variable_expr, not symbol_expr), which is probably what
>>> you want.
>>>
>>
>> I tried that first and I got the same error. I have tried it again.. and
>> I am getting the same error.
>>
>> file.nft:1:15-17: Error: Can't parse symbolic netfilter verdict expressions
>> define dest = ber
>>               ^^^
> 
> OK, at least it didn't get worse. :)
> 
> I looked at the code and it seems you need to implement a 'parse'
> callback for struct verdict_type. I guess existing 'parse' callback in
> struct integer_type is a good example of how to do it - basically you
> need to convert the symbol expression into a constant expression.
> 
> Sorry if that's not much help, I'm not really familiar with these
> details. :)
> 

That is very useful! I can continue with this information, thanks! :-)

> Cheers, Phil
>
diff mbox series

Patch

diff --git a/src/parser_bison.y b/src/parser_bison.y
index 69b5773..42fd71f 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -3841,7 +3841,13 @@  verdict_expr		:	ACCEPT
 			}
 			;
 
-chain_expr		:	identifier
+chain_expr		:	variable_expr
+			{
+				$$ = symbol_expr_alloc(&@$, SYMBOL_VALUE,
+						       current_scope(state),
+						       $1->sym->identifier);
+			}
+			|	identifier
 			{
 				$$ = constant_expr_alloc(&@$, &string_type,
 							 BYTEORDER_HOST_ENDIAN,