On Sat, Jul 28, 2018 at 12:14:57PM +0200, Pablo Neira Ayuso wrote:
> On Fri, Jul 27, 2018 at 04:21:46PM +0200, Máté Eckl wrote:
> > On Mon, Jul 16, 2018 at 09:58:44AM +0200, Máté Eckl wrote:
> > > On Tue, Jul 10, 2018 at 12:10:22PM +0200, Pablo Neira Ayuso wrote:
> > > > > diff --git a/src/parser_bison.y b/src/parser_bison.y
> > > > > index 98bfeba..2b7d7cc 100644
> > > > > --- a/src/parser_bison.y
> > > > > +++ b/src/parser_bison.y
> > > > > @@ -182,6 +182,8 @@ int nft_lex(void *, void *, void *);
> > > > > %token AT "@"
> > > > > %token VMAP "vmap"
> > > > >
> > > > > +%token PLUS "+"
> > > > > +
> > > > > %token INCLUDE "include"
> > > > > %token DEFINE "define"
> > > > > %token REDEFINE "redefine"
> > > > > @@ -522,6 +524,7 @@ int nft_lex(void *, void *, void *);
> > > > > %type <handle> set_spec setid_spec set_identifier flowtable_identifier obj_spec objid_spec obj_identifier
> > > > > %destructor { handle_free(&$$); } set_spec setid_spec set_identifier obj_spec objid_spec obj_identifier
> > > > > %type <val> family_spec family_spec_explicit chain_policy prio_spec
> > > > > +%type <string> str_prio_spec
> > > > >
> > > > > %type <string> dev_spec quota_unit
> > > > > %destructor { xfree($$); } dev_spec quota_unit
> > > > > @@ -1633,7 +1636,7 @@ flowtable_block_alloc : /* empty */
> > > > > flowtable_block : /* empty */ { $$ = $<flowtable>-1; }
> > > > > | flowtable_block common_block
> > > > > | flowtable_block stmt_separator
> > > > > - | flowtable_block HOOK STRING PRIORITY prio_spec stmt_separator
> > > > > + | flowtable_block HOOK STRING PRIORITY str_prio_spec stmt_separator
> > > > > {
> > > > > $$->hookstr = chain_hookname_lookup($3);
> > > > > if ($$->hookstr == NULL) {
> > > > > @@ -1644,7 +1647,7 @@ flowtable_block : /* empty */ { $$ = $<flowtable>-1; }
> > > > > }
> > > > > xfree($3);
> > > > >
> > > > > - $$->priority = $5;
> > > > > + $$->priostr = $5;
> > > > > }
> > > > > | flowtable_block DEVICES '=' flowtable_expr stmt_separator
> > > > > {
> > > > > @@ -1766,7 +1769,7 @@ type_identifier : STRING { $$ = $1; }
> > > > > | CLASSID { $$ = xstrdup("classid"); }
> > > > > ;
> > > > >
> > > > > -hook_spec : TYPE STRING HOOK STRING dev_spec PRIORITY prio_spec
> > > > > +hook_spec : TYPE STRING HOOK STRING dev_spec PRIORITY str_prio_spec
> > > > > {
> > > > > const char *chain_type = chain_type_name_lookup($2);
> > > > >
> > > > > @@ -1789,13 +1792,34 @@ hook_spec : TYPE STRING HOOK STRING dev_spec PRIORITY prio_spec
> > > > > xfree($4);
> > > > >
> > > > > $<chain>0->dev = $5;
> > > > > - $<chain>0->priority = $7;
> > > > > + $<chain>0->priostr = $7;
> > > > > $<chain>0->flags |= CHAIN_F_BASECHAIN;
> > > > > }
> > > > > ;
> > > > >
> > > > > -prio_spec : NUM { $$ = $1; }
> > > > > - | DASH NUM { $$ = -$2; }
> > > > > +str_prio_spec : prio_spec
> > > > > + {
> > > > > + char buf[STD_PRIO_BUFSIZE];
> > > > > + snprintf(buf, STD_PRIO_BUFSIZE, "%d", (int)$1);
> > > > > + $$ = xstrdup(buf);
> > > > > + }
> > > > > + | STRING { $$ = xstrdup($1); }
> > > > > + | STRING PLUS NUM
> > > > > + {
> > > > > + char buf[STD_PRIO_BUFSIZE];
> > > > > + snprintf(buf, STD_PRIO_BUFSIZE, "%s+%d",$1, (int)$3);
> > > >
> > > > Could you store the string plus offset instead of building this
> > > > string that you need to parse again from the evaluation phase?
> > > >
> > > > Probably you could reuse the existing priority integer field, then, if
> > > > the label is non-NULL, then it means the priority integer becomes an
> > > > offset.
> > >
> > > I thought about different possibilities to do this, and I think the diff below
> > > does this with the less possible code duplication (it's only the parser, the
> > > other components would not be duplicated). And for now it does not even work for
> > > some obscure reason. (The chain pointer is not tha same at the evaluation phase
> > > as in the parser, so the values are in the bad place...)
> > > Of course the evaluation is simpler.
> > >
> > > I personally prefer my former solution as it is more general and results in less
> > > code duplication so I would stay with that.
> > > What do you think?
> >
> > Do you want me to go on with this Pablo? I can make this work, but I don't think
> > this is better than what I've already done.
>
> If you're refering to the string parsing in the evaluation phase that
> this patch is doing, I would indeed prefer an approach that which
> doesn't need any tinkering with str*() functions to do late
> tokenization from there.
I've been trying to achieve what you want, but I cannot find a way to do so. I
am struggling with the parser. Could you help with it? I thought it is similar to
what I was doing on another topic but it is not.
Find the diff [2] of the parser and gdb session [1] below. It seems that after it
assigns mangle to priostr and 81 to priority, when it gets to the hook_spec part
81 is the value of first_line...
I have tried it with $<chain>0 $<chain>$ $<chain>-1 $<chain>1, but all of them caused some
sort of memory corruption...
It is certainly a misuse of the parser, but it is quite deep.
[1]:
gdb --args nft add chain ip x z "{ type filter hook prerouting priority mangle + 81; }"
[...]
(gdb) break parser_bison.c:6871
No source file named parser_bison.c.
Make breakpoint pending on future shared library load? (y or [n]) y
Breakpoint 1 (parser_bison.c:6871) pending.
(gdb) break parser_bison.c:6830
No source file named parser_bison.c.
Make breakpoint pending on future shared library load? (y or [n]) y
Breakpoint 2 (parser_bison.c:6830) pending.
(gdb) run
Starting program: /usr/bin/nft add chain ip x z \{\ type\ filter\ hook\ prerouting\ priority\ mangle\ +\ 81\;\ \}
Breakpoint 1, nft_parse (nft=0x55555575aa20, scanner=0x55555575b3a0, state=0x55555575aaf0) at parser_bison.c:6875
6875 break;
(gdb) print *(yyvsp[-3].chain)
$1 = {list = {next = 0x6974756f72657270, prev = 0x7ffff600676e}, handle = {family = 0, table = {location = {indesc = 0x21, {{token_offset = 111516266160493, line_offset = 140737337281280,
first_line = 0, last_line = 0, first_column = 33, last_column = 0}, {nle = 0x656c676e616d}}}, name = 0x656c676e616d <error: Cannot access memory at address 0x656c676e616d>},
chain = {location = {indesc = 0x7ffff6fecb00 <main_arena+96>, {{token_offset = 0, line_offset = 225, first_line = 4143893248, last_line = 32767, first_column = 4143893248,
last_column = 32767}, {nle = 0x0}}}, name = 0x0}, set = {location = {indesc = 0x0, {{token_offset = 0, line_offset = 0, first_line = 0, last_line = 0, first_column = 0,
last_column = 0}, {nle = 0x0}}}, name = 0x0}, obj = {location = {indesc = 0x0, {{token_offset = 0, line_offset = 0, first_line = 0, last_line = 0, first_column = 0,
last_column = 0}, {nle = 0x0}}}, name = 0x0}, flowtable = 0x0, handle = {location = {indesc = 0x0, {{token_offset = 0, line_offset = 0, first_line = 0, last_line = 0,
first_column = 0, last_column = 0}, {nle = 0x0}}}, id = 0}, position = {location = {indesc = 0x0, {{token_offset = 0, line_offset = 0, first_line = 0, last_line = 0,
first_column = 224, last_column = 0}, {nle = 0x0}}}, id = 32}, index = {location = {indesc = 0x736f6d736f63, {{token_offset = 0, line_offset = 0, first_line = 529,
last_line = 0, first_column = 0, last_column = 0}, {nle = 0x0}}}, id = 0}, set_id = 0}, location = {indesc = 0x0, {{token_offset = 0, line_offset = 0, first_line = 0,
last_line = 0, first_column = 0, last_column = 0}, {nle = 0x0}}}, refcnt = 0, flags = 0, hookstr = 0x0, hooknum = 0, priostr = 0x55555575b5c0 "mangle", priority = 81, policy = 0,
type = 0x0, dev = 0x0, scope = {parent = 0x0, symbols = {next = 0x0, prev = 0x0}}, rules = {next = 0x0, prev = 0x0}}
(gdb) continue
Continuing.
Breakpoint 2, nft_parse (nft=0x55555575aa20, scanner=0x55555575b3a0, state=0x55555575aaf0) at parser_bison.c:6854
6854 break;
(gdb) print *(yyvsp[-7].chain)
$2 = {list = {next = 0x0, prev = 0x0}, handle = {family = 0, table = {location = {indesc = 0x0, {{token_offset = 0, line_offset = 0, first_line = 0, last_line = 0, first_column = 0,
last_column = 0}, {nle = 0x0}}}, name = 0x0}, chain = {location = {indesc = 0x0, {{token_offset = 0, line_offset = 93824994358720, first_line = 81, last_line = 0,
first_column = 0, last_column = 0}, {nle = 0x0}}}, name = 0x0}, set = {location = {indesc = 0x0, {{token_offset = 0, line_offset = 0, first_line = 0, last_line = 0,
first_column = 0, last_column = 0}, {nle = 0x0}}}, name = 0x0}, obj = {location = {indesc = 0x0, {{token_offset = 0, line_offset = 0, first_line = 0, last_line = 0,
first_column = 0, last_column = 0}, {nle = 0x0}}}, name = 0x0}, flowtable = 0x0, handle = {location = {indesc = 0x0, {{token_offset = 0, line_offset = 0, first_line = 0,
last_line = 0, first_column = 0, last_column = 0}, {nle = 0x0}}}, id = 0}, position = {location = {indesc = 0x0, {{token_offset = 0, line_offset = 0, first_line = 0,
last_line = 0, first_column = 0, last_column = 0}, {nle = 0x0}}}, id = 0}, index = {location = {indesc = 0x0, {{token_offset = 0, line_offset = 0, first_line = 0,
last_line = 0, first_column = 0, last_column = 0}, {nle = 0x0}}}, id = 0}, set_id = 0}, location = {indesc = 0x0, {{token_offset = 0, line_offset = 0, first_line = 0,
last_line = 0, first_column = 0, last_column = 0}, {nle = 0x0}}}, refcnt = 1, flags = 1, hookstr = 0x7ffff7b9c476 "prerouting", hooknum = 0, priostr = 0x0, priority = 0,
policy = -1, type = 0x55555575b5e0 "filter", dev = 0x0, scope = {parent = 0x55555575b090, symbols = {next = 0x55555575b8c0, prev = 0x55555575b8c0}}, rules = {next = 0x55555575b8d0,
prev = 0x55555575b8d0}}
(gdb)
[2]:
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 98bfeba..ee76301 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -182,6 +182,8 @@ int nft_lex(void *, void *, void *);
%token AT "@"
%token VMAP "vmap"
+%token PLUS "+"
+
%token INCLUDE "include"
%token DEFINE "define"
%token REDEFINE "redefine"
@@ -1633,7 +1635,7 @@ flowtable_block_alloc : /* empty */
flowtable_block : /* empty */ { $$ = $<flowtable>-1; }
| flowtable_block common_block
| flowtable_block stmt_separator
- | flowtable_block HOOK STRING PRIORITY prio_spec stmt_separator
+ | flowtable_block HOOK STRING PRIORITY flowtable_prio_spec stmt_separator
{
$$->hookstr = chain_hookname_lookup($3);
if ($$->hookstr == NULL) {
@@ -1643,8 +1645,6 @@ flowtable_block : /* empty */ { $$ = $<flowtable>-1; }
YYERROR;
}
xfree($3);
-
- $$->priority = $5;
}
| flowtable_block DEVICES '=' flowtable_expr stmt_separator
{
@@ -1652,6 +1652,20 @@ flowtable_block : /* empty */ { $$ = $<flowtable>-1; }
}
;
+flowtable_prio_spec: prio_spec { $<flowtable>0->priority = $1; }
+ | STRING { $<flowtable>0->priostr = xstrdup($1); }
+ | STRING PLUS NUM
+ {
+ $<flowtable>0->priostr = xstrdup($1);
+ $<flowtable>0->priority = $3;
+ }
+ | STRING DASH NUM
+ {
+ $<flowtable>0->priostr = xstrdup($1);
+ $<flowtable>0->priority = -$3;
+ }
+ ;
+
flowtable_expr : '{' flowtable_list_expr '}'
{
$2->location = @$;
@@ -1766,7 +1780,7 @@ type_identifier : STRING { $$ = $1; }
| CLASSID { $$ = xstrdup("classid"); }
;
-hook_spec : TYPE STRING HOOK STRING dev_spec PRIORITY prio_spec
+hook_spec : TYPE STRING HOOK STRING dev_spec PRIORITY hook_prio_spec
{
const char *chain_type = chain_type_name_lookup($2);
@@ -1789,11 +1803,24 @@ hook_spec : TYPE STRING HOOK STRING dev_spec PRIORITY prio_spec
xfree($4);
$<chain>0->dev = $5;
- $<chain>0->priority = $7;
$<chain>0->flags |= CHAIN_F_BASECHAIN;
}
;
+hook_prio_spec : prio_spec { $<chain>0->priority = $1; }
+ | STRING { $<chain>0->priostr = xstrdup($1); }
+ | STRING PLUS NUM
+ {
+ $<chain>0->priostr = xstrdup($1);
+ $<chain>0->priority = $3;
+ }
+ | STRING DASH NUM
+ {
+ $<chain>0->priostr = xstrdup($1);
+ $<chain>0->priority = -$3;
+ }
+ ;
+
prio_spec : NUM { $$ = $1; }
| DASH NUM { $$ = -$2; }
;
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
