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 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.

> 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