All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3 nft] [RFC] more syntax changes
@ 2014-01-14 11:30 Pablo Neira Ayuso
  2014-01-14 11:30 ` [PATCH 1/3] scanner: replace binary characters '&' '|' and '!' by their names Pablo Neira Ayuso
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Pablo Neira Ayuso @ 2014-01-14 11:30 UTC (permalink / raw)
  To: netfilter-devel; +Cc: kaber

Hi,

The following patchset contains more small syntax and evaluation changes
aiming to improve the interaction user <-> tool via most common shells:

* iifname and oifname mask matching: very simple approach to allow matching
  interface names via meta oifname and eth == eth.

* Rename typical binary operation so we don't need to escape them in
  bash: '&' by 'and', '|' by 'or' and '!' by 'not'. They are just a
  few character longers and the syntax still looks nice to me.

* Rename 'eth' by 'ether', to mimic tcpdump. This also skips problems with
  a valid and simple possible iifname/oifname mask.

Comments welcome. Thanks!

Pablo Neira Ayuso (3):
  scanner: replace binary characters '&' '|' and '!' by their names
  evaluate: allow to use string with binary operations
  scanner: rename address selector from 'eth' to 'ether'

 src/evaluate.c |    3 ++-
 src/parser.y   |    4 ++--
 src/scanner.l  |    8 ++++----
 3 files changed, 8 insertions(+), 7 deletions(-)

-- 
1.7.10.4


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/3] scanner: replace binary characters '&' '|' and '!' by their names
  2014-01-14 11:30 [PATCH 0/3 nft] [RFC] more syntax changes Pablo Neira Ayuso
@ 2014-01-14 11:30 ` Pablo Neira Ayuso
  2014-01-14 12:00   ` Pablo Neira Ayuso
  2014-01-14 12:21   ` Patrick McHardy
  2014-01-14 11:30 ` [PATCH 2/3] evaluate: allow to use string with binary operations Pablo Neira Ayuso
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 15+ messages in thread
From: Pablo Neira Ayuso @ 2014-01-14 11:30 UTC (permalink / raw)
  To: netfilter-devel; +Cc: kaber

These symbol need to be escaped in bash and can lead to confusion,
so let's use their names instead which are still short, eg.

 nft add rule filter output meta mark and 0x3 == 0x1

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/scanner.l |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/scanner.l b/src/scanner.l
index cee6aa6..2d7ac88 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -200,9 +200,9 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
 "<<"			{ return LSHIFT; }
 ">>"			{ return RSHIFT; }
 "^"			{ return CARET; }
-"&"			{ return AMPERSAND; }
-"|"			{ return '|'; }
-"!"			{ return NOT; }
+"and"			{ return AMPERSAND; }
+"or"			{ return '|'; }
+"not"			{ return NOT; }
 "/"			{ return SLASH; }
 "-"			{ return DASH; }
 "*"			{ return ASTERISK; }
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/3] evaluate: allow to use string with binary operations
  2014-01-14 11:30 [PATCH 0/3 nft] [RFC] more syntax changes Pablo Neira Ayuso
  2014-01-14 11:30 ` [PATCH 1/3] scanner: replace binary characters '&' '|' and '!' by their names Pablo Neira Ayuso
@ 2014-01-14 11:30 ` Pablo Neira Ayuso
  2014-01-14 12:22   ` Patrick McHardy
  2014-01-14 11:30 ` [PATCH 3/3] scanner: rename address selector from 'eth' to 'ether' Pablo Neira Ayuso
  2014-01-14 11:47 ` [PATCH 0/3 nft] [RFC] more syntax changes Arturo Borrero Gonzalez
  3 siblings, 1 reply; 15+ messages in thread
From: Pablo Neira Ayuso @ 2014-01-14 11:30 UTC (permalink / raw)
  To: netfilter-devel; +Cc: kaber

This allows us to match ifname masks, eg.

nft add rule filter output meta oifname and eth == eth counter

I've been investigating other possibility, such as adding
ofiname-mask, which requires several patches and transformations
to make it look binop tree, but I still think this looks like
a natural way (and simple, look at the patch, it's rather small)
to represent this in the nftables.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/evaluate.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index 94fee64..49f0f74 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -534,7 +534,8 @@ static int expr_evaluate_binop(struct eval_ctx *ctx, struct expr **expr)
 		return -1;
 	right = op->right;
 
-	if (expr_basetype(left)->type != TYPE_INTEGER)
+	if (expr_basetype(left)->type != TYPE_INTEGER &&
+	    expr_basetype(left)->type != TYPE_STRING)
 		return expr_binary_error(ctx, left, op,
 					 "Binary operation (%s) is undefined "
 					 "for %s types",
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/3] scanner: rename address selector from 'eth' to 'ether'
  2014-01-14 11:30 [PATCH 0/3 nft] [RFC] more syntax changes Pablo Neira Ayuso
  2014-01-14 11:30 ` [PATCH 1/3] scanner: replace binary characters '&' '|' and '!' by their names Pablo Neira Ayuso
  2014-01-14 11:30 ` [PATCH 2/3] evaluate: allow to use string with binary operations Pablo Neira Ayuso
@ 2014-01-14 11:30 ` Pablo Neira Ayuso
  2014-01-14 12:23   ` Patrick McHardy
  2014-01-14 11:47 ` [PATCH 0/3 nft] [RFC] more syntax changes Arturo Borrero Gonzalez
  3 siblings, 1 reply; 15+ messages in thread
From: Pablo Neira Ayuso @ 2014-01-14 11:30 UTC (permalink / raw)
  To: netfilter-devel; +Cc: kaber

eth is may easily ocur when using ifname mask. This could be also
fixed by interpreting 'eth' as a simple string in the parser but
I think this selector also looks more similar to what we use in
tcpdump.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/parser.y  |    4 ++--
 src/scanner.l |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/parser.y b/src/parser.y
index 26e71e3..461eb3f 100644
--- a/src/parser.y
+++ b/src/parser.y
@@ -193,7 +193,7 @@ static void location_update(struct location *loc, struct location *rhs, int n)
 
 %token BRIDGE			"bridge"
 
-%token ETH			"eth"
+%token ETHER			"ether"
 %token SADDR			"saddr"
 %token DADDR			"daddr"
 %token TYPE			"type"
@@ -1392,7 +1392,7 @@ payload_base_spec	:	LL_HDR		{ $$ = PAYLOAD_BASE_LL_HDR; }
 			|	TRANSPORT_HDR	{ $$ = PAYLOAD_BASE_TRANSPORT_HDR; }
 			;
 
-eth_hdr_expr		:	ETH	eth_hdr_field
+eth_hdr_expr		:	ETHER	eth_hdr_field
 			{
 				$$ = payload_expr_alloc(&@$, &payload_eth, $2);
 			}
diff --git a/src/scanner.l b/src/scanner.l
index 2d7ac88..a4db017 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -278,7 +278,7 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
 
 "bridge"		{ return BRIDGE; }
 
-"eth"			{ return ETH; }
+"ether"			{ return ETHER; }
 "saddr"			{ return SADDR; }
 "daddr"			{ return DADDR; }
 "type"			{ return TYPE; }
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/3 nft] [RFC] more syntax changes
  2014-01-14 11:30 [PATCH 0/3 nft] [RFC] more syntax changes Pablo Neira Ayuso
                   ` (2 preceding siblings ...)
  2014-01-14 11:30 ` [PATCH 3/3] scanner: rename address selector from 'eth' to 'ether' Pablo Neira Ayuso
@ 2014-01-14 11:47 ` Arturo Borrero Gonzalez
  3 siblings, 0 replies; 15+ messages in thread
From: Arturo Borrero Gonzalez @ 2014-01-14 11:47 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Netfilter Development Mailing list, Patrick McHardy

On 14 January 2014 12:30, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> Hi,
>
> The following patchset contains more small syntax and evaluation changes
> aiming to improve the interaction user <-> tool via most common shells:
>
> * iifname and oifname mask matching: very simple approach to allow matching
>   interface names via meta oifname and eth == eth.
>
> * Rename typical binary operation so we don't need to escape them in
>   bash: '&' by 'and', '|' by 'or' and '!' by 'not'. They are just a
>   few character longers and the syntax still looks nice to me.

These are important! thanks!

-- 
Arturo Borrero González
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/3] scanner: replace binary characters '&' '|' and '!' by their names
  2014-01-14 11:30 ` [PATCH 1/3] scanner: replace binary characters '&' '|' and '!' by their names Pablo Neira Ayuso
@ 2014-01-14 12:00   ` Pablo Neira Ayuso
  2014-01-14 12:24     ` Patrick McHardy
  2014-01-14 12:21   ` Patrick McHardy
  1 sibling, 1 reply; 15+ messages in thread
From: Pablo Neira Ayuso @ 2014-01-14 12:00 UTC (permalink / raw)
  To: netfilter-devel; +Cc: kaber

On Tue, Jan 14, 2014 at 12:30:28PM +0100, Pablo Neira Ayuso wrote:
> These symbol need to be escaped in bash and can lead to confusion,
> so let's use their names instead which are still short, eg.
> 
>  nft add rule filter output meta mark and 0x3 == 0x1
> 
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  src/scanner.l |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/scanner.l b/src/scanner.l
> index cee6aa6..2d7ac88 100644
> --- a/src/scanner.l
> +++ b/src/scanner.l
> @@ -200,9 +200,9 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
>  "<<"			{ return LSHIFT; }
>  ">>"			{ return RSHIFT; }
>  "^"			{ return CARET; }

Extending this proposal:

For consistency, we can also rename the caret to 'xor'.

Regarding <<, we can use lshift, but that's a bit longer.

We also have >, >=, < and <=. We could use gt, ge, lt, le.

> -"&"			{ return AMPERSAND; }
> -"|"			{ return '|'; }
> -"!"			{ return NOT; }
> +"and"			{ return AMPERSAND; }
> +"or"			{ return '|'; }
> +"not"			{ return NOT; }
>  "/"			{ return SLASH; }
>  "-"			{ return DASH; }
>  "*"			{ return ASTERISK; }
> -- 
> 1.7.10.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/3] scanner: replace binary characters '&' '|' and '!' by their names
  2014-01-14 11:30 ` [PATCH 1/3] scanner: replace binary characters '&' '|' and '!' by their names Pablo Neira Ayuso
  2014-01-14 12:00   ` Pablo Neira Ayuso
@ 2014-01-14 12:21   ` Patrick McHardy
  1 sibling, 0 replies; 15+ messages in thread
From: Patrick McHardy @ 2014-01-14 12:21 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

On Tue, Jan 14, 2014 at 12:30:28PM +0100, Pablo Neira Ayuso wrote:
> These symbol need to be escaped in bash and can lead to confusion,
> so let's use their names instead which are still short, eg.
> 
>  nft add rule filter output meta mark and 0x3 == 0x1

Seems fine to me, although I'd rather have both ways of specifying them.

> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  src/scanner.l |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/scanner.l b/src/scanner.l
> index cee6aa6..2d7ac88 100644
> --- a/src/scanner.l
> +++ b/src/scanner.l
> @@ -200,9 +200,9 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
>  "<<"			{ return LSHIFT; }
>  ">>"			{ return RSHIFT; }
>  "^"			{ return CARET; }
> -"&"			{ return AMPERSAND; }
> -"|"			{ return '|'; }
> -"!"			{ return NOT; }
> +"and"			{ return AMPERSAND; }
> +"or"			{ return '|'; }
> +"not"			{ return NOT; }
>  "/"			{ return SLASH; }
>  "-"			{ return DASH; }
>  "*"			{ return ASTERISK; }
> -- 
> 1.7.10.4

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/3] evaluate: allow to use string with binary operations
  2014-01-14 11:30 ` [PATCH 2/3] evaluate: allow to use string with binary operations Pablo Neira Ayuso
@ 2014-01-14 12:22   ` Patrick McHardy
  2014-01-14 15:25     ` Pablo Neira Ayuso
  0 siblings, 1 reply; 15+ messages in thread
From: Patrick McHardy @ 2014-01-14 12:22 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

On Tue, Jan 14, 2014 at 12:30:29PM +0100, Pablo Neira Ayuso wrote:
> This allows us to match ifname masks, eg.
> 
> nft add rule filter output meta oifname and eth == eth counter
> 
> I've been investigating other possibility, such as adding
> ofiname-mask, which requires several patches and transformations
> to make it look binop tree, but I still think this looks like
> a natural way (and simple, look at the patch, it's rather small)
> to represent this in the nftables.

I was just going to suggest adding a shortcut for this since its exposing
a lot of low-level detail. The transformation should be quite easy during
evaluation, could you elaborate on the problems?

> 
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  src/evaluate.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/evaluate.c b/src/evaluate.c
> index 94fee64..49f0f74 100644
> --- a/src/evaluate.c
> +++ b/src/evaluate.c
> @@ -534,7 +534,8 @@ static int expr_evaluate_binop(struct eval_ctx *ctx, struct expr **expr)
>  		return -1;
>  	right = op->right;
>  
> -	if (expr_basetype(left)->type != TYPE_INTEGER)
> +	if (expr_basetype(left)->type != TYPE_INTEGER &&
> +	    expr_basetype(left)->type != TYPE_STRING)
>  		return expr_binary_error(ctx, left, op,
>  					 "Binary operation (%s) is undefined "
>  					 "for %s types",
> -- 
> 1.7.10.4

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] scanner: rename address selector from 'eth' to 'ether'
  2014-01-14 11:30 ` [PATCH 3/3] scanner: rename address selector from 'eth' to 'ether' Pablo Neira Ayuso
@ 2014-01-14 12:23   ` Patrick McHardy
  0 siblings, 0 replies; 15+ messages in thread
From: Patrick McHardy @ 2014-01-14 12:23 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

On Tue, Jan 14, 2014 at 12:30:30PM +0100, Pablo Neira Ayuso wrote:
> eth is may easily ocur when using ifname mask. This could be also
> fixed by interpreting 'eth' as a simple string in the parser but
> I think this selector also looks more similar to what we use in
> tcpdump.

This is a very good idea, I always use ether by mistake anyway :)

> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  src/parser.y  |    4 ++--
>  src/scanner.l |    2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/parser.y b/src/parser.y
> index 26e71e3..461eb3f 100644
> --- a/src/parser.y
> +++ b/src/parser.y
> @@ -193,7 +193,7 @@ static void location_update(struct location *loc, struct location *rhs, int n)
>  
>  %token BRIDGE			"bridge"
>  
> -%token ETH			"eth"
> +%token ETHER			"ether"
>  %token SADDR			"saddr"
>  %token DADDR			"daddr"
>  %token TYPE			"type"
> @@ -1392,7 +1392,7 @@ payload_base_spec	:	LL_HDR		{ $$ = PAYLOAD_BASE_LL_HDR; }
>  			|	TRANSPORT_HDR	{ $$ = PAYLOAD_BASE_TRANSPORT_HDR; }
>  			;
>  
> -eth_hdr_expr		:	ETH	eth_hdr_field
> +eth_hdr_expr		:	ETHER	eth_hdr_field
>  			{
>  				$$ = payload_expr_alloc(&@$, &payload_eth, $2);
>  			}
> diff --git a/src/scanner.l b/src/scanner.l
> index 2d7ac88..a4db017 100644
> --- a/src/scanner.l
> +++ b/src/scanner.l
> @@ -278,7 +278,7 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
>  
>  "bridge"		{ return BRIDGE; }
>  
> -"eth"			{ return ETH; }
> +"ether"			{ return ETHER; }
>  "saddr"			{ return SADDR; }
>  "daddr"			{ return DADDR; }
>  "type"			{ return TYPE; }
> -- 
> 1.7.10.4

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/3] scanner: replace binary characters '&' '|' and '!' by their names
  2014-01-14 12:00   ` Pablo Neira Ayuso
@ 2014-01-14 12:24     ` Patrick McHardy
  0 siblings, 0 replies; 15+ messages in thread
From: Patrick McHardy @ 2014-01-14 12:24 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

On Tue, Jan 14, 2014 at 01:00:57PM +0100, Pablo Neira Ayuso wrote:
> On Tue, Jan 14, 2014 at 12:30:28PM +0100, Pablo Neira Ayuso wrote:
> > These symbol need to be escaped in bash and can lead to confusion,
> > so let's use their names instead which are still short, eg.
> > 
> >  nft add rule filter output meta mark and 0x3 == 0x1
> > 
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > ---
> >  src/scanner.l |    6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/scanner.l b/src/scanner.l
> > index cee6aa6..2d7ac88 100644
> > --- a/src/scanner.l
> > +++ b/src/scanner.l
> > @@ -200,9 +200,9 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
> >  "<<"			{ return LSHIFT; }
> >  ">>"			{ return RSHIFT; }
> >  "^"			{ return CARET; }
> 
> Extending this proposal:
> 
> For consistency, we can also rename the caret to 'xor'.
> 
> Regarding <<, we can use lshift, but that's a bit longer.
> 
> We also have >, >=, < and <=. We could use gt, ge, lt, le.

Agreed on all of those, but please also keep the short forms.

> > -"&"			{ return AMPERSAND; }
> > -"|"			{ return '|'; }
> > -"!"			{ return NOT; }
> > +"and"			{ return AMPERSAND; }
> > +"or"			{ return '|'; }
> > +"not"			{ return NOT; }
> >  "/"			{ return SLASH; }
> >  "-"			{ return DASH; }
> >  "*"			{ return ASTERISK; }
> > -- 
> > 1.7.10.4
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/3] evaluate: allow to use string with binary operations
  2014-01-14 12:22   ` Patrick McHardy
@ 2014-01-14 15:25     ` Pablo Neira Ayuso
  2014-01-14 15:49       ` Patrick McHardy
  0 siblings, 1 reply; 15+ messages in thread
From: Pablo Neira Ayuso @ 2014-01-14 15:25 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: netfilter-devel

On Tue, Jan 14, 2014 at 12:22:52PM +0000, Patrick McHardy wrote:
> On Tue, Jan 14, 2014 at 12:30:29PM +0100, Pablo Neira Ayuso wrote:
> > This allows us to match ifname masks, eg.
> > 
> > nft add rule filter output meta oifname and eth == eth counter
> > 
> > I've been investigating other possibility, such as adding
> > ofiname-mask, which requires several patches and transformations
> > to make it look binop tree, but I still think this looks like
> > a natural way (and simple, look at the patch, it's rather small)
> > to represent this in the nftables.
> 
> I was just going to suggest adding a shortcut for this since its exposing
> a lot of low-level detail. The transformation should be quite easy during
> evaluation, could you elaborate on the problems?

Not really a problem but a bit more specific code to handle this case.
I started writing support for this following several approaches, but
after looking at my patchset I thought this approach was smaller and
it's requiring way less specific code.

The fist of my patches here (the ones that I didn't send) replace all
NFT_META_* references in the parser by internal META_*, eg. META_MARK,
just to prepare the addition of META_IIFNAMEMASK and META_OIFNAMEMASK.
Then, the follow-up patch transforms the following expression that we
got from that looks like:

              relational
                /     \
               /       \
    meta oifnamemask  string

to a binary op expression. These also needs some specific code in the
delinearize path to transform the binop tree back to the expression
above.

Let me know if you have any better idea. Thanks.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/3] evaluate: allow to use string with binary operations
  2014-01-14 15:25     ` Pablo Neira Ayuso
@ 2014-01-14 15:49       ` Patrick McHardy
  2014-01-15  9:29         ` Pablo Neira Ayuso
  0 siblings, 1 reply; 15+ messages in thread
From: Patrick McHardy @ 2014-01-14 15:49 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

On Tue, Jan 14, 2014 at 04:25:32PM +0100, Pablo Neira Ayuso wrote:
> On Tue, Jan 14, 2014 at 12:22:52PM +0000, Patrick McHardy wrote:
> > On Tue, Jan 14, 2014 at 12:30:29PM +0100, Pablo Neira Ayuso wrote:
> > > This allows us to match ifname masks, eg.
> > > 
> > > nft add rule filter output meta oifname and eth == eth counter
> > > 
> > > I've been investigating other possibility, such as adding
> > > ofiname-mask, which requires several patches and transformations
> > > to make it look binop tree, but I still think this looks like
> > > a natural way (and simple, look at the patch, it's rather small)
> > > to represent this in the nftables.
> > 
> > I was just going to suggest adding a shortcut for this since its exposing
> > a lot of low-level detail. The transformation should be quite easy during
> > evaluation, could you elaborate on the problems?
> 
> Not really a problem but a bit more specific code to handle this case.
> I started writing support for this following several approaches, but
> after looking at my patchset I thought this approach was smaller and
> it's requiring way less specific code.
> 
> The fist of my patches here (the ones that I didn't send) replace all
> NFT_META_* references in the parser by internal META_*, eg. META_MARK,
> just to prepare the addition of META_IIFNAMEMASK and META_OIFNAMEMASK.
> Then, the follow-up patch transforms the following expression that we
> got from that looks like:
> 
>               relational
>                 /     \
>                /       \
>     meta oifnamemask  string
> 
> to a binary op expression. These also needs some specific code in the
> delinearize path to transform the binop tree back to the expression
> above.
> 
> Let me know if you have any better idea. Thanks.

Well, I think the easiest approach would be to add some code to
expr_evaluate_relational() for OP_EQ for convert the LHS of a
relational meta expression to LHS & RHS:

     relational (==)
    /               \
meta oifname       string

=>

        relational (==)
       /               \
    binop (&)        string
  /          \
meta oifname  string

The attached patch uses '*' as a trigger (and obviously won't work
because the '*' is also used in the mask, but you get the idea.
netlink_delinarize adjustments are missing, but it should be pretty
trivial to add the corresponding code to postprocessing of relational
expressions.


<cmdline>:0:0-33: Evaluate
filter output meta oifname "eth*"
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^


<cmdline>:1:15-33: Evaluate
filter output meta oifname "eth*"
              ^^^^^^^^^^^^^^^^^^^
meta oifname $eth*

<cmdline>:1:15-33: Evaluate
filter output meta oifname "eth*"
              ^^^^^^^^^^^^^^^^^^^
meta oifname $eth*

<cmdline>:1:15-26: Evaluate
filter output meta oifname "eth*"
              ^^^^^^^^^^^^
meta oifname

<cmdline>:1:28-33: Evaluate
filter output meta oifname "eth*"
                           ^^^^^^
$eth*

<cmdline>:1:28-33: Evaluate
filter output meta oifname "eth*"
                           ^^^^^^
"eth*"

<cmdline>:1:15-26: Evaluate
filter output meta oifname "eth*"
              ^^^^^^^^^^^^
meta oifname & "eth*"

<cmdline>:1:15-26: Evaluate
filter output meta oifname "eth*"
              ^^^^^^^^^^^^
meta oifname

<cmdline>:1:28-33: Evaluate
filter output meta oifname "eth*"
                           ^^^^^^
"eth*"

<cmdline>:0:0-33: Error: Could not process rule: Operation not permitted
filter output meta oifname "eth*"
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^


diff --git a/src/evaluate.c b/src/evaluate.c
index 4ca3294..9c659dc 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -547,7 +547,8 @@ static int expr_evaluate_binop(struct eval_ctx *ctx, struct expr **expr)
 		return -1;
 	right = op->right;
 
-	if (expr_basetype(left)->type != TYPE_INTEGER)
+	if (expr_basetype(left)->type != TYPE_INTEGER &&
+	    expr_basetype(left)->type != TYPE_STRING)
 		return expr_binary_error(ctx, left, op,
 					 "Binary operation (%s) is undefined "
 					 "for %s types",
@@ -936,6 +937,22 @@ static int expr_evaluate_relational(struct eval_ctx *ctx, struct expr **expr)
 		    left->ops->pctx_update)
 			left->ops->pctx_update(&ctx->pctx, rel);
 
+		if (left->ops->type == EXPR_META &&
+		    (left->meta.key == NFT_META_IIFNAME ||
+		     left->meta.key == NFT_META_OIFNAME)) {
+			unsigned int len = div_round_up(right->len, BITS_PER_BYTE);
+			char data[len + 1];
+
+			mpz_export_data(data, right->value, BYTEORDER_HOST_ENDIAN, len);
+			if (strchr(data, '*')) {
+				left = binop_expr_alloc(&left->location, OP_AND,
+							left, expr_clone(right));
+				if (expr_evaluate(ctx, &left) < 0)
+					return -1;
+				(*expr)->left = left;
+			}
+		}
+
 		if (left->ops->type == EXPR_CONCAT)
 			return 0;
 

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/3] evaluate: allow to use string with binary operations
  2014-01-14 15:49       ` Patrick McHardy
@ 2014-01-15  9:29         ` Pablo Neira Ayuso
  2014-01-15 15:58           ` Pablo Neira Ayuso
  0 siblings, 1 reply; 15+ messages in thread
From: Pablo Neira Ayuso @ 2014-01-15  9:29 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: netfilter-devel

On Tue, Jan 14, 2014 at 03:49:00PM +0000, Patrick McHardy wrote:
> On Tue, Jan 14, 2014 at 04:25:32PM +0100, Pablo Neira Ayuso wrote:
> > On Tue, Jan 14, 2014 at 12:22:52PM +0000, Patrick McHardy wrote:
> > > On Tue, Jan 14, 2014 at 12:30:29PM +0100, Pablo Neira Ayuso wrote:
> > > > This allows us to match ifname masks, eg.
> > > > 
> > > > nft add rule filter output meta oifname and eth == eth counter
> > > > 
> > > > I've been investigating other possibility, such as adding
> > > > ofiname-mask, which requires several patches and transformations
> > > > to make it look binop tree, but I still think this looks like
> > > > a natural way (and simple, look at the patch, it's rather small)
> > > > to represent this in the nftables.
> > > 
> > > I was just going to suggest adding a shortcut for this since its exposing
> > > a lot of low-level detail. The transformation should be quite easy during
> > > evaluation, could you elaborate on the problems?
> > 
> > Not really a problem but a bit more specific code to handle this case.
> > I started writing support for this following several approaches, but
> > after looking at my patchset I thought this approach was smaller and
> > it's requiring way less specific code.
> > 
> > The fist of my patches here (the ones that I didn't send) replace all
> > NFT_META_* references in the parser by internal META_*, eg. META_MARK,
> > just to prepare the addition of META_IIFNAMEMASK and META_OIFNAMEMASK.
> > Then, the follow-up patch transforms the following expression that we
> > got from that looks like:
> > 
> >               relational
> >                 /     \
> >                /       \
> >     meta oifnamemask  string
> > 
> > to a binary op expression. These also needs some specific code in the
> > delinearize path to transform the binop tree back to the expression
> > above.
> > 
> > Let me know if you have any better idea. Thanks.
> 
> Well, I think the easiest approach would be to add some code to
> expr_evaluate_relational() for OP_EQ for convert the LHS of a
> relational meta expression to LHS & RHS:
> 
>      relational (==)
>     /               \
> meta oifname       string
> 
> =>
> 
>         relational (==)
>        /               \
>     binop (&)        string
>   /          \
> meta oifname  string
> 
> The attached patch uses '*' as a trigger (and obviously won't work
> because the '*' is also used in the mask, but you get the idea.
> netlink_delinarize adjustments are missing, but it should be pretty
> trivial to add the corresponding code to postprocessing of relational
> expressions.

Oh yes, with that wildcard trick the thing is simplified. There was
some discuss on the use of '+' that seems to be possible to be used in
a device name. I guess '*' is safe as udev is using it in their rules.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/3] evaluate: allow to use string with binary operations
  2014-01-15  9:29         ` Pablo Neira Ayuso
@ 2014-01-15 15:58           ` Pablo Neira Ayuso
  2014-01-15 16:05             ` Patrick McHardy
  0 siblings, 1 reply; 15+ messages in thread
From: Pablo Neira Ayuso @ 2014-01-15 15:58 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: netfilter-devel

On Wed, Jan 15, 2014 at 10:29:43AM +0100, Pablo Neira Ayuso wrote:
> On Tue, Jan 14, 2014 at 03:49:00PM +0000, Patrick McHardy wrote:
> > Well, I think the easiest approach would be to add some code to
> > expr_evaluate_relational() for OP_EQ for convert the LHS of a
> > relational meta expression to LHS & RHS:
> > 
> >      relational (==)
> >     /               \
> > meta oifname       string
> > 
> > =>
> > 
> >         relational (==)
> >        /               \
> >     binop (&)        string
> >   /          \
> > meta oifname  string
> > 
> > The attached patch uses '*' as a trigger (and obviously won't work
> > because the '*' is also used in the mask, but you get the idea.
> > netlink_delinarize adjustments are missing, but it should be pretty
> > trivial to add the corresponding code to postprocessing of relational
> > expressions.
> 
> Oh yes, with that wildcard trick the thing is simplified. There was
> some discuss on the use of '+' that seems to be possible to be used in
> a device name. I guess '*' is safe as udev is using it in their rules.

It seems we need to include the '*' into the string rule in flex for
this. We already have a single '*' that is used in the parser for
prefixes, I think that may lead to ambiguity problems.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/3] evaluate: allow to use string with binary operations
  2014-01-15 15:58           ` Pablo Neira Ayuso
@ 2014-01-15 16:05             ` Patrick McHardy
  0 siblings, 0 replies; 15+ messages in thread
From: Patrick McHardy @ 2014-01-15 16:05 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

On Wed, Jan 15, 2014 at 04:58:49PM +0100, Pablo Neira Ayuso wrote:
> On Wed, Jan 15, 2014 at 10:29:43AM +0100, Pablo Neira Ayuso wrote:
> > On Tue, Jan 14, 2014 at 03:49:00PM +0000, Patrick McHardy wrote:
> > > Well, I think the easiest approach would be to add some code to
> > > expr_evaluate_relational() for OP_EQ for convert the LHS of a
> > > relational meta expression to LHS & RHS:
> > > 
> > >      relational (==)
> > >     /               \
> > > meta oifname       string
> > > 
> > > =>
> > > 
> > >         relational (==)
> > >        /               \
> > >     binop (&)        string
> > >   /          \
> > > meta oifname  string
> > > 
> > > The attached patch uses '*' as a trigger (and obviously won't work
> > > because the '*' is also used in the mask, but you get the idea.
> > > netlink_delinarize adjustments are missing, but it should be pretty
> > > trivial to add the corresponding code to postprocessing of relational
> > > expressions.
> > 
> > Oh yes, with that wildcard trick the thing is simplified. There was
> > some discuss on the use of '+' that seems to be possible to be used in
> > a device name. I guess '*' is safe as udev is using it in their rules.
> 
> It seems we need to include the '*' into the string rule in flex for
> this. We already have a single '*' that is used in the parser for
> prefixes, I think that may lead to ambiguity problems.

We of course could use something different, but I think it should be fine.

The '*' for prefixes can be reasonable expected to follow some whitespace,
while in this case it would always be the final character of a string.

This *seems* to work in some very limited testing.

diff --git a/src/scanner.l b/src/scanner.l
index 6ff8846..abc46e1 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -439,6 +439,11 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
 				return STRING;
 			}
 
+{string}\*		{
+				yylval->string = xstrdup(yytext);
+				return STRING;
+			}
+
 \\{newline}		{
 				reset_pos(yyget_extra(yyscanner), yylloc);
 			}

^ permalink raw reply related	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2014-01-15 16:05 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-14 11:30 [PATCH 0/3 nft] [RFC] more syntax changes Pablo Neira Ayuso
2014-01-14 11:30 ` [PATCH 1/3] scanner: replace binary characters '&' '|' and '!' by their names Pablo Neira Ayuso
2014-01-14 12:00   ` Pablo Neira Ayuso
2014-01-14 12:24     ` Patrick McHardy
2014-01-14 12:21   ` Patrick McHardy
2014-01-14 11:30 ` [PATCH 2/3] evaluate: allow to use string with binary operations Pablo Neira Ayuso
2014-01-14 12:22   ` Patrick McHardy
2014-01-14 15:25     ` Pablo Neira Ayuso
2014-01-14 15:49       ` Patrick McHardy
2014-01-15  9:29         ` Pablo Neira Ayuso
2014-01-15 15:58           ` Pablo Neira Ayuso
2014-01-15 16:05             ` Patrick McHardy
2014-01-14 11:30 ` [PATCH 3/3] scanner: rename address selector from 'eth' to 'ether' Pablo Neira Ayuso
2014-01-14 12:23   ` Patrick McHardy
2014-01-14 11:47 ` [PATCH 0/3 nft] [RFC] more syntax changes Arturo Borrero Gonzalez

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.