Re: [PATCH v4 nft] Set/print standard chain prios with textual names

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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?

diff --git a/src/parser_bison.y b/src/parser_bison.y
index 98bfeba..db55cc5 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,23 @@ 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 +1783,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 +1806,27 @@ 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



[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux