* [PATCH 0/2 nft WIP] Using variables in chain priority
@ 2019-07-16 9:08 Fernando Fernandez Mancera
2019-07-16 9:08 ` [PATCH 1/2 nft WIP] src: introduce prio_expr " Fernando Fernandez Mancera
2019-07-16 9:08 ` [PATCH 2/2 nft WIP] src: allow variables " Fernando Fernandez Mancera
0 siblings, 2 replies; 6+ messages in thread
From: Fernando Fernandez Mancera @ 2019-07-16 9:08 UTC (permalink / raw)
To: netfilter-devel; +Cc: Fernando Fernandez Mancera
I am getting the following error when I try to load the following file using "nft -f"
File:
define pri = filter
table inet global {
chain prerouting {
type filter hook prerouting priority $pri
policy accept
}
}
Error:
priority_test:1:14-19: Error: No symbol type information
define pri = filter
^^^^^^
priority_test:4:37-49: Error: invalid priority expression symbol in this context.
type filter hook prerouting priority $pri
^^^^^^^^^^^^^
The original idea was to evaluate the prio_expr and check the result expression
datatype. This way we could use variables with number priority number and also
strings. It seems that the symbol does not have symbol type information at the
evaluation phase. I have a workaround that consist in allocating a constant
expression with the symbol identifier in the parser but then we should check
the datatype manually in the evaluation. I don't like that solution at all.
Is there any other way to accomplish that? I would like to find a better
solution. Thanks!
Fernando Fernandez Mancera (2):
src: introduce prio_expr in chain priority
src: allow variables in chain priority
include/rule.h | 8 ++++----
src/evaluate.c | 29 +++++++++++++++++++----------
src/parser_bison.y | 26 ++++++++++++++++++--------
src/rule.c | 4 ++--
4 files changed, 43 insertions(+), 24 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/2 nft WIP] src: introduce prio_expr in chain priority 2019-07-16 9:08 [PATCH 0/2 nft WIP] Using variables in chain priority Fernando Fernandez Mancera @ 2019-07-16 9:08 ` Fernando Fernandez Mancera 2019-07-16 18:06 ` Pablo Neira Ayuso 2019-07-16 9:08 ` [PATCH 2/2 nft WIP] src: allow variables " Fernando Fernandez Mancera 1 sibling, 1 reply; 6+ messages in thread From: Fernando Fernandez Mancera @ 2019-07-16 9:08 UTC (permalink / raw) To: netfilter-devel; +Cc: Fernando Fernandez Mancera Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net> --- include/rule.h | 8 ++++---- src/evaluate.c | 29 +++++++++++++++++++---------- src/parser_bison.y | 25 +++++++++++++++++-------- src/rule.c | 4 ++-- 4 files changed, 42 insertions(+), 24 deletions(-) diff --git a/include/rule.h b/include/rule.h index aefb24d..4d7cec8 100644 --- a/include/rule.h +++ b/include/rule.h @@ -173,13 +173,13 @@ enum chain_flags { * struct prio_spec - extendend priority specification for mixed * textual/numerical parsing. * - * @str: name of the standard priority value - * @num: Numerical value. This MUST contain the parsed value of str after + * @prio_expr: expr of the standard priority value + * @num: Numerical value. This MUST contain the parsed value of prio_expr after * evaluation. */ struct prio_spec { - const char *str; - int num; + struct expr *prio_expr; + int num; struct location loc; }; diff --git a/src/evaluate.c b/src/evaluate.c index 8086f75..cee65cd 100644 --- a/src/evaluate.c +++ b/src/evaluate.c @@ -3181,15 +3181,24 @@ static int set_evaluate(struct eval_ctx *ctx, struct set *set) return 0; } -static bool evaluate_priority(struct prio_spec *prio, int family, int hook) +static bool evaluate_priority(struct eval_ctx *ctx, struct prio_spec *prio, + int family, int hook) { int priority; + char prio_str[NFT_NAME_MAXLEN]; /* A numeric value has been used to specify priority. */ - if (prio->str == NULL) + if (prio->prio_expr == NULL) return true; - priority = std_prio_lookup(prio->str, family, hook); + if (expr_evaluate(ctx, &prio->prio_expr) < 0) + return false; + if (prio->prio_expr->etype == EXPR_VALUE) + mpz_export_data(prio_str, prio->prio_expr->value, + BYTEORDER_HOST_ENDIAN, + NFT_NAME_MAXLEN); + + priority = std_prio_lookup(prio_str, family, hook); if (priority == NF_IP_PRI_LAST) return false; prio->num += priority; @@ -3211,10 +3220,10 @@ static int flowtable_evaluate(struct eval_ctx *ctx, struct flowtable *ft) if (ft->hooknum == NF_INET_NUMHOOKS) return chain_error(ctx, ft, "invalid hook %s", ft->hookstr); - if (!evaluate_priority(&ft->priority, NFPROTO_NETDEV, ft->hooknum)) + if (!evaluate_priority(ctx, &ft->priority, NFPROTO_NETDEV, ft->hooknum)) return __stmt_binary_error(ctx, &ft->priority.loc, NULL, - "'%s' is invalid priority.", - ft->priority.str); + "invalid priority expression %s.", + expr_name(ft->priority.prio_expr)); if (!ft->dev_expr) return chain_error(ctx, ft, "Unbound flowtable not allowed (must specify devices)"); @@ -3410,11 +3419,11 @@ static int chain_evaluate(struct eval_ctx *ctx, struct chain *chain) return chain_error(ctx, chain, "invalid hook %s", chain->hookstr); - if (!evaluate_priority(&chain->priority, chain->handle.family, - chain->hooknum)) + if (!evaluate_priority(ctx, &chain->priority, + chain->handle.family, chain->hooknum)) return __stmt_binary_error(ctx, &chain->priority.loc, NULL, - "'%s' is invalid priority in this context.", - chain->priority.str); + "invalid priority expression %s in this context.", + expr_name(chain->priority.prio_expr)); } list_for_each_entry(rule, &chain->rules, list) { diff --git a/src/parser_bison.y b/src/parser_bison.y index a4905f2..c6a43cf 100644 --- a/src/parser_bison.y +++ b/src/parser_bison.y @@ -626,8 +626,8 @@ int nft_lex(void *, void *, void *); %type <stmt> meter_stmt meter_stmt_alloc flow_stmt_legacy_alloc %destructor { stmt_free($$); } meter_stmt meter_stmt_alloc flow_stmt_legacy_alloc -%type <expr> symbol_expr verdict_expr integer_expr variable_expr chain_expr -%destructor { expr_free($$); } symbol_expr verdict_expr integer_expr variable_expr chain_expr +%type <expr> symbol_expr verdict_expr integer_expr variable_expr chain_expr prio_expr +%destructor { expr_free($$); } symbol_expr verdict_expr integer_expr variable_expr chain_expr prio_expr %type <expr> primary_expr shift_expr and_expr %destructor { expr_free($$); } primary_expr shift_expr and_expr %type <expr> exclusive_or_expr inclusive_or_expr @@ -1926,30 +1926,39 @@ extended_prio_name : OUT | STRING ; +prio_expr : extended_prio_name + { + $$ = constant_expr_alloc(&@$, &string_type, + BYTEORDER_HOST_ENDIAN, + NFT_NAME_MAXLEN * + BITS_PER_BYTE, $1); + } + ; + extended_prio_spec : int_num { struct prio_spec spec = {0}; spec.num = $1; $$ = spec; } - | extended_prio_name + | prio_expr { struct prio_spec spec = {0}; - spec.str = $1; + spec.prio_expr = $1; $$ = spec; } - | extended_prio_name PLUS NUM + | prio_expr PLUS NUM { struct prio_spec spec = {0}; spec.num = $3; - spec.str = $1; + spec.prio_expr = $1; $$ = spec; } - | extended_prio_name DASH NUM + | prio_expr DASH NUM { struct prio_spec spec = {0}; spec.num = -$3; - spec.str = $1; + spec.prio_expr = $1; $$ = spec; } ; diff --git a/src/rule.c b/src/rule.c index 0a91917..59a97ac 100644 --- a/src/rule.c +++ b/src/rule.c @@ -823,7 +823,7 @@ void chain_free(struct chain *chain) xfree(chain->type); if (chain->dev != NULL) xfree(chain->dev); - xfree(chain->priority.str); + expr_free(chain->priority.prio_expr); xfree(chain); } @@ -2020,7 +2020,7 @@ void flowtable_free(struct flowtable *flowtable) if (--flowtable->refcnt > 0) return; handle_free(&flowtable->handle); - xfree(flowtable->priority.str); + expr_free(flowtable->priority.prio_expr); xfree(flowtable); } -- 2.20.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2 nft WIP] src: introduce prio_expr in chain priority 2019-07-16 9:08 ` [PATCH 1/2 nft WIP] src: introduce prio_expr " Fernando Fernandez Mancera @ 2019-07-16 18:06 ` Pablo Neira Ayuso 2019-07-16 22:42 ` Fernando Fernandez Mancera 0 siblings, 1 reply; 6+ messages in thread From: Pablo Neira Ayuso @ 2019-07-16 18:06 UTC (permalink / raw) To: Fernando Fernandez Mancera; +Cc: netfilter-devel On Tue, Jul 16, 2019 at 11:08:12AM +0200, Fernando Fernandez Mancera wrote: > Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net> > --- > include/rule.h | 8 ++++---- > src/evaluate.c | 29 +++++++++++++++++++---------- > src/parser_bison.y | 25 +++++++++++++++++-------- > src/rule.c | 4 ++-- > 4 files changed, 42 insertions(+), 24 deletions(-) > > diff --git a/include/rule.h b/include/rule.h > index aefb24d..4d7cec8 100644 > --- a/include/rule.h > +++ b/include/rule.h > @@ -173,13 +173,13 @@ enum chain_flags { > * struct prio_spec - extendend priority specification for mixed > * textual/numerical parsing. > * > - * @str: name of the standard priority value > - * @num: Numerical value. This MUST contain the parsed value of str after > + * @prio_expr: expr of the standard priority value > + * @num: Numerical value. This MUST contain the parsed value of prio_expr after > * evaluation. > */ > struct prio_spec { > - const char *str; > - int num; > + struct expr *prio_expr; Use: struct expr *expr; instead. > + int num; You could just store this in the expression, no need for this num field. > struct location loc; > }; > > diff --git a/src/evaluate.c b/src/evaluate.c > index 8086f75..cee65cd 100644 > --- a/src/evaluate.c > +++ b/src/evaluate.c > @@ -3181,15 +3181,24 @@ static int set_evaluate(struct eval_ctx *ctx, struct set *set) > return 0; > } > > -static bool evaluate_priority(struct prio_spec *prio, int family, int hook) > +static bool evaluate_priority(struct eval_ctx *ctx, struct prio_spec *prio, > + int family, int hook) > { > int priority; > + char prio_str[NFT_NAME_MAXLEN]; > > /* A numeric value has been used to specify priority. */ > - if (prio->str == NULL) > + if (prio->prio_expr == NULL) prio_expr == NULL never happens. > return true; > > - priority = std_prio_lookup(prio->str, family, hook); > + if (expr_evaluate(ctx, &prio->prio_expr) < 0) > + return false; > + if (prio->prio_expr->etype == EXPR_VALUE) You should bail out here with expr_error() is this is not an EXPR_VALUE. > + mpz_export_data(prio_str, prio->prio_expr->value, > + BYTEORDER_HOST_ENDIAN, > + NFT_NAME_MAXLEN); Remove the branch above, expr_evalute() already deals with transforming the symbol to value expression. > + > + priority = std_prio_lookup(prio_str, family, hook); > if (priority == NF_IP_PRI_LAST) > return false; > prio->num += priority; > @@ -3211,10 +3220,10 @@ static int flowtable_evaluate(struct eval_ctx *ctx, struct flowtable *ft) > if (ft->hooknum == NF_INET_NUMHOOKS) > return chain_error(ctx, ft, "invalid hook %s", ft->hookstr); > > - if (!evaluate_priority(&ft->priority, NFPROTO_NETDEV, ft->hooknum)) > + if (!evaluate_priority(ctx, &ft->priority, NFPROTO_NETDEV, ft->hooknum)) > return __stmt_binary_error(ctx, &ft->priority.loc, NULL, Better use expr_error() here? > - "'%s' is invalid priority.", > - ft->priority.str); > + "invalid priority expression %s.", > + expr_name(ft->priority.prio_expr)); > > if (!ft->dev_expr) > return chain_error(ctx, ft, "Unbound flowtable not allowed (must specify devices)"); > @@ -3410,11 +3419,11 @@ static int chain_evaluate(struct eval_ctx *ctx, struct chain *chain) > return chain_error(ctx, chain, "invalid hook %s", > chain->hookstr); > > - if (!evaluate_priority(&chain->priority, chain->handle.family, > - chain->hooknum)) > + if (!evaluate_priority(ctx, &chain->priority, > + chain->handle.family, chain->hooknum)) > return __stmt_binary_error(ctx, &chain->priority.loc, NULL, > - "'%s' is invalid priority in this context.", > - chain->priority.str); > + "invalid priority expression %s in this context.", > + expr_name(chain->priority.prio_expr)); > } > > list_for_each_entry(rule, &chain->rules, list) { > diff --git a/src/parser_bison.y b/src/parser_bison.y > index a4905f2..c6a43cf 100644 > --- a/src/parser_bison.y > +++ b/src/parser_bison.y > @@ -626,8 +626,8 @@ int nft_lex(void *, void *, void *); > %type <stmt> meter_stmt meter_stmt_alloc flow_stmt_legacy_alloc > %destructor { stmt_free($$); } meter_stmt meter_stmt_alloc flow_stmt_legacy_alloc > > -%type <expr> symbol_expr verdict_expr integer_expr variable_expr chain_expr > -%destructor { expr_free($$); } symbol_expr verdict_expr integer_expr variable_expr chain_expr > +%type <expr> symbol_expr verdict_expr integer_expr variable_expr chain_expr prio_expr > +%destructor { expr_free($$); } symbol_expr verdict_expr integer_expr variable_expr chain_expr prio_expr > %type <expr> primary_expr shift_expr and_expr > %destructor { expr_free($$); } primary_expr shift_expr and_expr > %type <expr> exclusive_or_expr inclusive_or_expr > @@ -1926,30 +1926,39 @@ extended_prio_name : OUT > | STRING > ; > > +prio_expr : extended_prio_name > + { > + $$ = constant_expr_alloc(&@$, &string_type, > + BYTEORDER_HOST_ENDIAN, > + NFT_NAME_MAXLEN * this should be strlen($1) * BITS_PER_BYTE > + BITS_PER_BYTE, $1); > + } > + ; ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2 nft WIP] src: introduce prio_expr in chain priority 2019-07-16 18:06 ` Pablo Neira Ayuso @ 2019-07-16 22:42 ` Fernando Fernandez Mancera 0 siblings, 0 replies; 6+ messages in thread From: Fernando Fernandez Mancera @ 2019-07-16 22:42 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: netfilter-devel Hi Pablo, thanks for reviewing. Comments below. On 7/16/19 8:06 PM, Pablo Neira Ayuso wrote: > On Tue, Jul 16, 2019 at 11:08:12AM +0200, Fernando Fernandez Mancera wrote: >> Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net> >> --- >> include/rule.h | 8 ++++---- >> src/evaluate.c | 29 +++++++++++++++++++---------- >> src/parser_bison.y | 25 +++++++++++++++++-------- >> src/rule.c | 4 ++-- >> 4 files changed, 42 insertions(+), 24 deletions(-) >> >> diff --git a/include/rule.h b/include/rule.h >> index aefb24d..4d7cec8 100644 >> --- a/include/rule.h >> +++ b/include/rule.h >> @@ -173,13 +173,13 @@ enum chain_flags { >> * struct prio_spec - extendend priority specification for mixed >> * textual/numerical parsing. >> * >> - * @str: name of the standard priority value >> - * @num: Numerical value. This MUST contain the parsed value of str after >> + * @prio_expr: expr of the standard priority value >> + * @num: Numerical value. This MUST contain the parsed value of prio_expr after >> * evaluation. >> */ >> struct prio_spec { >> - const char *str; >> - int num; >> + struct expr *prio_expr; > > Use: > > struct expr *expr; > > instead. > >> + int num; > > You could just store this in the expression, no need for this num > field. > I think that would not be possible. Right now, the priority specification supports combinations of a string and a number. e.g table inet global { chain prerouting { type filter hook prerouting priority filter + 3 policy accept } } or table inet global { chain prerouting { type filter hook prerouting priority filter - 3 policy accept } } I don't think we are going to be able to do that using only a single "struct expr *". >> struct location loc; >> }; >> >> diff --git a/src/evaluate.c b/src/evaluate.c >> index 8086f75..cee65cd 100644 >> --- a/src/evaluate.c >> +++ b/src/evaluate.c >> @@ -3181,15 +3181,24 @@ static int set_evaluate(struct eval_ctx *ctx, struct set *set) >> return 0; >> } >> >> -static bool evaluate_priority(struct prio_spec *prio, int family, int hook) >> +static bool evaluate_priority(struct eval_ctx *ctx, struct prio_spec *prio, >> + int family, int hook) >> { >> int priority; >> + char prio_str[NFT_NAME_MAXLEN]; >> >> /* A numeric value has been used to specify priority. */ >> - if (prio->str == NULL) >> + if (prio->prio_expr == NULL) > > prio_expr == NULL never happens. > It never happens if we only have a single field in the "struct prio_spec". Thanks! ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2 nft WIP] src: allow variables in chain priority 2019-07-16 9:08 [PATCH 0/2 nft WIP] Using variables in chain priority Fernando Fernandez Mancera 2019-07-16 9:08 ` [PATCH 1/2 nft WIP] src: introduce prio_expr " Fernando Fernandez Mancera @ 2019-07-16 9:08 ` Fernando Fernandez Mancera 2019-07-16 18:07 ` Pablo Neira Ayuso 1 sibling, 1 reply; 6+ messages in thread From: Fernando Fernandez Mancera @ 2019-07-16 9:08 UTC (permalink / raw) To: netfilter-devel; +Cc: Fernando Fernandez Mancera Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net> --- src/parser_bison.y | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/parser_bison.y b/src/parser_bison.y index c6a43cf..d55a5fc 100644 --- a/src/parser_bison.y +++ b/src/parser_bison.y @@ -1926,7 +1926,8 @@ extended_prio_name : OUT | STRING ; -prio_expr : extended_prio_name +prio_expr : variable_expr + | extended_prio_name { $$ = constant_expr_alloc(&@$, &string_type, BYTEORDER_HOST_ENDIAN, -- 2.20.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2 nft WIP] src: allow variables in chain priority 2019-07-16 9:08 ` [PATCH 2/2 nft WIP] src: allow variables " Fernando Fernandez Mancera @ 2019-07-16 18:07 ` Pablo Neira Ayuso 0 siblings, 0 replies; 6+ messages in thread From: Pablo Neira Ayuso @ 2019-07-16 18:07 UTC (permalink / raw) To: Fernando Fernandez Mancera; +Cc: netfilter-devel On Tue, Jul 16, 2019 at 11:08:14AM +0200, Fernando Fernandez Mancera wrote: > Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net> > --- > src/parser_bison.y | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/src/parser_bison.y b/src/parser_bison.y > index c6a43cf..d55a5fc 100644 > --- a/src/parser_bison.y > +++ b/src/parser_bison.y > @@ -1926,7 +1926,8 @@ extended_prio_name : OUT > | STRING > ; > > -prio_expr : extended_prio_name > +prio_expr : variable_expr > + | extended_prio_name This patch is small and I think it belongs to the previous patch, please squash it to 1/2. Thanks Fernando. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-07-16 22:42 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-07-16 9:08 [PATCH 0/2 nft WIP] Using variables in chain priority Fernando Fernandez Mancera 2019-07-16 9:08 ` [PATCH 1/2 nft WIP] src: introduce prio_expr " Fernando Fernandez Mancera 2019-07-16 18:06 ` Pablo Neira Ayuso 2019-07-16 22:42 ` Fernando Fernandez Mancera 2019-07-16 9:08 ` [PATCH 2/2 nft WIP] src: allow variables " Fernando Fernandez Mancera 2019-07-16 18:07 ` Pablo Neira Ayuso
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).