All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH libnftnl] expr: imm: Fix immediate verdict comparison
@ 2016-08-25 14:56 Carlos Falgueras García
  2016-08-25 15:19 ` Pablo Neira Ayuso
  0 siblings, 1 reply; 5+ messages in thread
From: Carlos Falgueras García @ 2016-08-25 14:56 UTC (permalink / raw)
  To: netfilter-devel; +Cc: pablo

An immediate expression of type 'DATA_VERDICT' can have set a chain (jump
or goto), in this cases we must compare its 'union nftnl_data_reg' using
'DATA_CHAIN' flag instead of 'DATA_VERDICT'

Before this patch compare expressions "jump -> chain_a" and
"jump -> chain_b" returns they are equals.

Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
---
 src/expr/immediate.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/expr/immediate.c b/src/expr/immediate.c
index cb8a81b..b26fc8d 100644
--- a/src/expr/immediate.c
+++ b/src/expr/immediate.c
@@ -329,10 +329,16 @@ static bool nftnl_expr_immediate_cmp(const struct nftnl_expr *e1,
 
 	if (e1->flags & (1 << NFTNL_EXPR_IMM_DREG))
 		eq &= (i1->dreg == i2->dreg);
-	if (e1->flags & (1 << NFTNL_EXPR_IMM_VERDICT))
-		eq &= nftnl_data_reg_cmp(&i1->data, &i2->data, DATA_VERDICT);
-	else if (e1->flags & (1 << NFTNL_EXPR_IMM_DATA))
+	if (e1->flags & (1 << NFTNL_EXPR_IMM_VERDICT)) {
+		if (e1->flags & (1 << NFTNL_EXPR_IMM_CHAIN))
+			eq &= nftnl_data_reg_cmp(&i1->data, &i2->data,
+						 DATA_CHAIN);
+		else
+			eq &= nftnl_data_reg_cmp(&i1->data, &i2->data,
+						 DATA_VERDICT);
+	} else if (e1->flags & (1 << NFTNL_EXPR_IMM_DATA)) {
 		eq &= nftnl_data_reg_cmp(&i1->data, &i2->data, DATA_VALUE);
+	}
 
 	return eq;
 }
-- 
2.9.3


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

* Re: [PATCH libnftnl] expr: imm: Fix immediate verdict comparison
  2016-08-25 14:56 [PATCH libnftnl] expr: imm: Fix immediate verdict comparison Carlos Falgueras García
@ 2016-08-25 15:19 ` Pablo Neira Ayuso
  2016-08-25 15:27   ` Carlos Falgueras García
  2016-08-26 13:49   ` Carlos Falgueras García
  0 siblings, 2 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2016-08-25 15:19 UTC (permalink / raw)
  To: Carlos Falgueras García; +Cc: netfilter-devel

[-- Attachment #1: Type: text/plain, Size: 1475 bytes --]

On Thu, Aug 25, 2016 at 04:56:58PM +0200, Carlos Falgueras García wrote:
> An immediate expression of type 'DATA_VERDICT' can have set a chain (jump
> or goto), in this cases we must compare its 'union nftnl_data_reg' using
> 'DATA_CHAIN' flag instead of 'DATA_VERDICT'
> 
> Before this patch compare expressions "jump -> chain_a" and
> "jump -> chain_b" returns they are equals.
> 
> Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
> ---
>  src/expr/immediate.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/src/expr/immediate.c b/src/expr/immediate.c
> index cb8a81b..b26fc8d 100644
> --- a/src/expr/immediate.c
> +++ b/src/expr/immediate.c
> @@ -329,10 +329,16 @@ static bool nftnl_expr_immediate_cmp(const struct nftnl_expr *e1,
>  
>  	if (e1->flags & (1 << NFTNL_EXPR_IMM_DREG))
>  		eq &= (i1->dreg == i2->dreg);
> -	if (e1->flags & (1 << NFTNL_EXPR_IMM_VERDICT))
> -		eq &= nftnl_data_reg_cmp(&i1->data, &i2->data, DATA_VERDICT);
> -	else if (e1->flags & (1 << NFTNL_EXPR_IMM_DATA))
> +	if (e1->flags & (1 << NFTNL_EXPR_IMM_VERDICT)) {
> +		if (e1->flags & (1 << NFTNL_EXPR_IMM_CHAIN))
> +			eq &= nftnl_data_reg_cmp(&i1->data, &i2->data,
> +						 DATA_CHAIN);
> +		else
> +			eq &= nftnl_data_reg_cmp(&i1->data, &i2->data,
> +						 DATA_VERDICT);

Probably better with this patch below? You don't need to split the
lines.

No need to resend, just review and confirm this change is OK and I'll
apply this here.

[-- Attachment #2: y.patch --]
[-- Type: text/x-diff, Size: 1827 bytes --]

>From 521b6b1a7cc99445cf983403add23f373836d1a8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carlos=20Falgueras=20Garc=C3=ADa?= <carlosfg@riseup.net>
Date: Thu, 25 Aug 2016 16:56:58 +0200
Subject: [PATCH] expr: imm: Fix immediate verdict comparison
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

An immediate expression of type 'DATA_VERDICT' can have set a chain (jump
or goto), in this cases we must compare its 'union nftnl_data_reg' using
'DATA_CHAIN' flag instead of 'DATA_VERDICT'

Before this patch compare expressions "jump -> chain_a" and
"jump -> chain_b" returns they are equals.

Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/expr/immediate.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/expr/immediate.c b/src/expr/immediate.c
index cb8a81b..8636738 100644
--- a/src/expr/immediate.c
+++ b/src/expr/immediate.c
@@ -326,13 +326,20 @@ static bool nftnl_expr_immediate_cmp(const struct nftnl_expr *e1,
 	struct nftnl_expr_immediate *i1 = nftnl_expr_data(e1);
 	struct nftnl_expr_immediate *i2 = nftnl_expr_data(e2);
 	bool eq = true;
+	int type;
 
 	if (e1->flags & (1 << NFTNL_EXPR_IMM_DREG))
 		eq &= (i1->dreg == i2->dreg);
-	if (e1->flags & (1 << NFTNL_EXPR_IMM_VERDICT))
-		eq &= nftnl_data_reg_cmp(&i1->data, &i2->data, DATA_VERDICT);
-	else if (e1->flags & (1 << NFTNL_EXPR_IMM_DATA))
+	if (e1->flags & (1 << NFTNL_EXPR_IMM_VERDICT)) {
+		if (e1->flags & (1 << NFTNL_EXPR_IMM_CHAIN))
+			type = DATA_CHAIN;
+		else
+			type = DATA_VERDICT;
+
+		eq &= nftnl_data_reg_cmp(&i1->data, &i2->data, type);
+	} else if (e1->flags & (1 << NFTNL_EXPR_IMM_DATA)) {
 		eq &= nftnl_data_reg_cmp(&i1->data, &i2->data, DATA_VALUE);
+	}
 
 	return eq;
 }
-- 
2.1.4


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

* Re: [PATCH libnftnl] expr: imm: Fix immediate verdict comparison
  2016-08-25 15:19 ` Pablo Neira Ayuso
@ 2016-08-25 15:27   ` Carlos Falgueras García
  2016-08-26 13:49   ` Carlos Falgueras García
  1 sibling, 0 replies; 5+ messages in thread
From: Carlos Falgueras García @ 2016-08-25 15:27 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

On 08/25/2016 05:19 PM, Pablo Neira Ayuso wrote:
> On Thu, Aug 25, 2016 at 04:56:58PM +0200, Carlos Falgueras García wrote:
>> An immediate expression of type 'DATA_VERDICT' can have set a chain (jump
>> or goto), in this cases we must compare its 'union nftnl_data_reg' using
>> 'DATA_CHAIN' flag instead of 'DATA_VERDICT'
>>
>> Before this patch compare expressions "jump -> chain_a" and
>> "jump -> chain_b" returns they are equals.
>>
>> Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
>> ---
>>  src/expr/immediate.c | 12 +++++++++---
>>  1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/expr/immediate.c b/src/expr/immediate.c
>> index cb8a81b..b26fc8d 100644
>> --- a/src/expr/immediate.c
>> +++ b/src/expr/immediate.c
>> @@ -329,10 +329,16 @@ static bool nftnl_expr_immediate_cmp(const struct nftnl_expr *e1,
>>
>>  	if (e1->flags & (1 << NFTNL_EXPR_IMM_DREG))
>>  		eq &= (i1->dreg == i2->dreg);
>> -	if (e1->flags & (1 << NFTNL_EXPR_IMM_VERDICT))
>> -		eq &= nftnl_data_reg_cmp(&i1->data, &i2->data, DATA_VERDICT);
>> -	else if (e1->flags & (1 << NFTNL_EXPR_IMM_DATA))
>> +	if (e1->flags & (1 << NFTNL_EXPR_IMM_VERDICT)) {
>> +		if (e1->flags & (1 << NFTNL_EXPR_IMM_CHAIN))
>> +			eq &= nftnl_data_reg_cmp(&i1->data, &i2->data,
>> +						 DATA_CHAIN);
>> +		else
>> +			eq &= nftnl_data_reg_cmp(&i1->data, &i2->data,
>> +						 DATA_VERDICT);
>
> Probably better with this patch below? You don't need to split the
> lines.
>
> No need to resend, just review and confirm this change is OK and I'll
> apply this here.

Yes, thanks Pablo. It is equivalent but more clean. Maybe DATA_VALUE can be consolidated too?

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

* [PATCH libnftnl] expr: imm: Fix immediate verdict comparison
  2016-08-25 15:19 ` Pablo Neira Ayuso
  2016-08-25 15:27   ` Carlos Falgueras García
@ 2016-08-26 13:49   ` Carlos Falgueras García
  2016-08-26 17:26     ` Pablo Neira Ayuso
  1 sibling, 1 reply; 5+ messages in thread
From: Carlos Falgueras García @ 2016-08-26 13:49 UTC (permalink / raw)
  To: netfilter-devel; +Cc: pablo

An immediate expression of type 'DATA_VERDICT' can have set a chain (jump
or goto), in this cases we must compare its 'union nftnl_data_reg' using
'DATA_CHAIN' flag instead of 'DATA_VERDICT'

Before this patch compare expressions "jump -> chain_a" and
"jump -> chain_b" returns they are equals.

Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
---
V2: Chosen a clearest code structure
 src/expr/immediate.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/expr/immediate.c b/src/expr/immediate.c
index cb8a81b..41fd9c4 100644
--- a/src/expr/immediate.c
+++ b/src/expr/immediate.c
@@ -326,13 +326,19 @@ static bool nftnl_expr_immediate_cmp(const struct nftnl_expr *e1,
 	struct nftnl_expr_immediate *i1 = nftnl_expr_data(e1);
 	struct nftnl_expr_immediate *i2 = nftnl_expr_data(e2);
 	bool eq = true;
+	int type = DATA_NONE;
 
 	if (e1->flags & (1 << NFTNL_EXPR_IMM_DREG))
 		eq &= (i1->dreg == i2->dreg);
 	if (e1->flags & (1 << NFTNL_EXPR_IMM_VERDICT))
-		eq &= nftnl_data_reg_cmp(&i1->data, &i2->data, DATA_VERDICT);
+		if (e1->flags & (1 << NFTNL_EXPR_IMM_CHAIN))
+			type = DATA_CHAIN;
+		else
+			type = DATA_VERDICT;
 	else if (e1->flags & (1 << NFTNL_EXPR_IMM_DATA))
-		eq &= nftnl_data_reg_cmp(&i1->data, &i2->data, DATA_VALUE);
+			type = DATA_VALUE;
+	if (type != DATA_NONE)
+		eq &= nftnl_data_reg_cmp(&i1->data, &i2->data, type);
 
 	return eq;
 }
-- 
2.9.3


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

* Re: [PATCH libnftnl] expr: imm: Fix immediate verdict comparison
  2016-08-26 13:49   ` Carlos Falgueras García
@ 2016-08-26 17:26     ` Pablo Neira Ayuso
  0 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2016-08-26 17:26 UTC (permalink / raw)
  To: Carlos Falgueras García; +Cc: netfilter-devel

On Fri, Aug 26, 2016 at 03:49:22PM +0200, Carlos Falgueras García wrote:
> An immediate expression of type 'DATA_VERDICT' can have set a chain (jump
> or goto), in this cases we must compare its 'union nftnl_data_reg' using
> 'DATA_CHAIN' flag instead of 'DATA_VERDICT'
> 
> Before this patch compare expressions "jump -> chain_a" and
> "jump -> chain_b" returns they are equals.

Applied, thanks Carlos.

> Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
> ---
> V2: Chosen a clearest code structure

Please, place v2 also in your patch subject, ie.

        [PATCH libnftnl,v2] expr: imm: Fix immediate verdict comparison

Thanks.

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

end of thread, other threads:[~2016-08-26 17:26 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-25 14:56 [PATCH libnftnl] expr: imm: Fix immediate verdict comparison Carlos Falgueras García
2016-08-25 15:19 ` Pablo Neira Ayuso
2016-08-25 15:27   ` Carlos Falgueras García
2016-08-26 13:49   ` Carlos Falgueras García
2016-08-26 17:26     ` Pablo Neira Ayuso

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.