[PATCH v2 libnetfilter_conntrack] zero value handling of mark and zone

netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH v2 libnetfilter_conntrack] zero value handling of mark and zone
@ 2014-06-17 12:04 Ken-ichirou MATSUZAWA
  2014-06-17 16:40 ` Florian Westphal
  2014-06-19 15:16 ` Florian Westphal
  0 siblings, 2 replies; 7+ messages in thread
From: Ken-ichirou MATSUZAWA @ 2014-06-17 12:04 UTC (permalink / raw)
  To: The netfilter developer mailinglist; +Cc: Florian Westphal

This patch enables comparison of 0 value with mark and zone since
both CTA_MARK and CTA_ZONE are not set in case of its value is 0.
These changes has been done in cmp_meta() and its own cmp function
as Florian pointed out.

This enables `conntrack -L --zone 0' to work expctedly too.

Signed-off-by: Ken-ichirou MATSUZAWA <chamas@h4.dion.ne.jp>
---
 src/conntrack/compare.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/src/conntrack/compare.c b/src/conntrack/compare.c
index f4a194a..384050e 100644
--- a/src/conntrack/compare.c
+++ b/src/conntrack/compare.c
@@ -291,7 +291,10 @@ cmp_mark(const struct nf_conntrack *ct1,
 	 const struct nf_conntrack *ct2,
 	 unsigned int flags)
 {
-	return (ct1->mark == ct2->mark);
+	return (flags & NFCT_CMP_MASK &&
+		!test_bit(ATTR_MARK, ct1->head.set)) ||
+		nfct_get_attr_u32(ct1, ATTR_MARK)
+		== nfct_get_attr_u32(ct2, ATTR_MARK);
 }
 
 static int 
@@ -357,7 +360,10 @@ cmp_zone(const struct nf_conntrack *ct1,
 	 const struct nf_conntrack *ct2,
 	 unsigned int flags)
 {
-	return (ct1->zone == ct2->zone);
+	return (flags & NFCT_CMP_MASK &&
+		!test_bit(ATTR_ZONE, ct1->head.set)) ||
+		nfct_get_attr_u16(ct1, ATTR_ZONE)
+		== nfct_get_attr_u16(ct2, ATTR_ZONE);
 }
 
 static int
@@ -421,7 +427,7 @@ static int cmp_meta(const struct nf_conntrack *ct1,
 {
 	if (!__cmp(ATTR_ID, ct1, ct2, flags, cmp_id))
 		return 0;
-	if (!__cmp(ATTR_MARK, ct1, ct2, flags, cmp_mark))
+	if (!cmp_mark(ct1, ct2, flags))
 		return 0;
 	if (!__cmp(ATTR_TIMEOUT, ct1, ct2, flags, cmp_timeout))
 		return 0;
@@ -433,7 +439,7 @@ static int cmp_meta(const struct nf_conntrack *ct1,
 		return 0;
 	if (!__cmp(ATTR_DCCP_STATE, ct1, ct2, flags, cmp_dccp_state))
 		return 0;
-	if (!__cmp(ATTR_ZONE, ct1, ct2, flags, cmp_zone))
+	if (!cmp_zone(ct1, ct2, flags))
 		return 0;
 	if (!__cmp(ATTR_SECCTX, ct1, ct2, flags, cmp_secctx))
 		return 0;
-- 
1.9.1



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

* Re: [PATCH v2 libnetfilter_conntrack] zero value handling of mark and zone
  2014-06-17 12:04 [PATCH v2 libnetfilter_conntrack] zero value handling of mark and zone Ken-ichirou MATSUZAWA
@ 2014-06-17 16:40 ` Florian Westphal
  2014-06-19 15:16 ` Florian Westphal
  1 sibling, 0 replies; 7+ messages in thread
From: Florian Westphal @ 2014-06-17 16:40 UTC (permalink / raw)
  To: Ken-ichirou MATSUZAWA
  Cc: The netfilter developer mailinglist, Florian Westphal

Ken-ichirou MATSUZAWA <chamaken@gmail.com> wrote:
> This patch enables comparison of 0 value with mark and zone since
> both CTA_MARK and CTA_ZONE are not set in case of its value is 0.
> These changes has been done in cmp_meta() and its own cmp function
> as Florian pointed out.

Looks good to me.

I'll apply this soon if there are no other objections.

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

* Re: [PATCH v2 libnetfilter_conntrack] zero value handling of mark and zone
  2014-06-17 12:04 [PATCH v2 libnetfilter_conntrack] zero value handling of mark and zone Ken-ichirou MATSUZAWA
  2014-06-17 16:40 ` Florian Westphal
@ 2014-06-19 15:16 ` Florian Westphal
  2014-06-23 10:12   ` [PATCH v3 0/2 " Ken-ichirou MATSUZAWA
  1 sibling, 1 reply; 7+ messages in thread
From: Florian Westphal @ 2014-06-19 15:16 UTC (permalink / raw)
  To: Ken-ichirou MATSUZAWA
  Cc: The netfilter developer mailinglist, Florian Westphal

Ken-ichirou MATSUZAWA <chamaken@gmail.com> wrote:
> This patch enables comparison of 0 value with mark and zone since
> both CTA_MARK and CTA_ZONE are not set in case of its value is 0.
> These changes has been done in cmp_meta() and its own cmp function
> as Florian pointed out.
> 
> This enables `conntrack -L --zone 0' to work expctedly too.
> 
> Signed-off-by: Ken-ichirou MATSUZAWA <chamas@h4.dion.ne.jp>
> ---
>  src/conntrack/compare.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/src/conntrack/compare.c b/src/conntrack/compare.c
> index f4a194a..384050e 100644
> --- a/src/conntrack/compare.c
> +++ b/src/conntrack/compare.c
> @@ -291,7 +291,10 @@ cmp_mark(const struct nf_conntrack *ct1,
>  	 const struct nf_conntrack *ct2,
>  	 unsigned int flags)
>  {
> -	return (ct1->mark == ct2->mark);
> +	return (flags & NFCT_CMP_MASK &&
> +		!test_bit(ATTR_MARK, ct1->head.set)) ||
> +		nfct_get_attr_u32(ct1, ATTR_MARK)
> +		== nfct_get_attr_u32(ct2, ATTR_MARK);
>  }

I spoke too soon.

This is problematic and will most likely cause regression.

Ex.
ct1: attr is not set
ct2: attr is set to 42

Current behaviour:
- With STRICT, they are not equal
- Without, they are qual

Now, even without STRICT, they are considered
to not match since 0 != 42.  But without STRICT, they should
be equal, since default behaviour is to only compare an
attribute if it is set in both conntrack objects.

I've updated the regression test suite in qa/test_api
with your reported bug case:

http://git.netfilter.org/libnetfilter_conntrack/commit/?id=169b1a3f37a70018aa402d90ba564ad01cb3a4cd

IOW, 'make -C qa test_api && qa/test_api' will fail
due to assertion failure in line 275.

I am afraid you will need to come up with a common
handler similar to __cmp() to deals with this, and
then use that handler for MARK and ZONE attributes.

With your new patch, the test suite would hopefully succeed
again.

Also, feel free to update/mangle the test cases if needed,
it will help us to make sure we do not accidentally alter
standing library behaviour.

Best Regards,
Florian

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

* [PATCH v3 0/2 libnetfilter_conntrack] zero value handling of mark and zone
  2014-06-19 15:16 ` Florian Westphal
@ 2014-06-23 10:12   ` Ken-ichirou MATSUZAWA
  2014-06-23 10:15     ` [PATCH v3 1/2 libnetfilter_conntrack] qa: update cmp ATTR_ZONE size " Ken-ichirou MATSUZAWA
  2014-06-23 10:18     ` [PATCH v3 2/2 libnetfilter_conntrack] conntrack: zero value handling of " Ken-ichirou MATSUZAWA
  0 siblings, 2 replies; 7+ messages in thread
From: Ken-ichirou MATSUZAWA @ 2014-06-23 10:12 UTC (permalink / raw)
  To: Florian Westphal; +Cc: The netfilter developer mailinglist

On Thu, Jun 19, 2014 at 05:16:11PM +0200, Florian Westphal wrote:
> I've updated the regression test suite in qa/test_api
> with your reported bug case:
> 
> http://git.netfilter.org/libnetfilter_conntrack/commit/?id=169b1a3f37a70018aa402d90ba564ad01cb3a4cd

Thanks to you, things we should do has made obvious:

    assert(test_cmp_attr32(ATTR_ZONE, true, false, 0, 0, NFCT_CMP_STRICT) == 1);
    assert(test_cmp_attr32(ATTR_ZONE, false, true, 0, 0, NFCT_CMP_STRICT) == 1);
    assert(test_cmp_attr32(ATTR_ZONE, true, false, 0, 0, NFCT_CMP_MASK) == 1);

and I worked, but it's dirty-looking. Is there more sophiscated way?

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

* [PATCH v3 1/2 libnetfilter_conntrack] qa: update cmp ATTR_ZONE size mark and zone
  2014-06-23 10:12   ` [PATCH v3 0/2 " Ken-ichirou MATSUZAWA
@ 2014-06-23 10:15     ` Ken-ichirou MATSUZAWA
  2014-06-24 15:23       ` Florian Westphal
  2014-06-23 10:18     ` [PATCH v3 2/2 libnetfilter_conntrack] conntrack: zero value handling of " Ken-ichirou MATSUZAWA
  1 sibling, 1 reply; 7+ messages in thread
From: Ken-ichirou MATSUZAWA @ 2014-06-23 10:15 UTC (permalink / raw)
  To: Florian Westphal; +Cc: The netfilter developer mailinglist

---
 qa/test_api.c | 102 ++++++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 78 insertions(+), 24 deletions(-)

diff --git a/qa/test_api.c b/qa/test_api.c
index 91b3dbf..1335b23 100644
--- a/qa/test_api.c
+++ b/qa/test_api.c
@@ -243,17 +243,17 @@ static int test_nfct_cmp_api_single(struct nf_conntrack *ct1,
 	return 0;
 }
 
-static int test_cmp_attr32(int attr, bool at1, bool at2,
-			   uint32_t v1, uint32_t v2, unsigned int flags)
+static int test_cmp_attr16(int attr, bool at1, bool at2,
+			   uint16_t v1, uint16_t v2, unsigned int flags)
 {
 	struct nf_conntrack *ct1 = nfct_new();
 	struct nf_conntrack *ct2 = nfct_new();
 	int ret;
 
 	if (at1)
-		nfct_set_attr_u32(ct1, attr, v1);
+		nfct_set_attr_u16(ct1, attr, v1);
 	if (at2)
-		nfct_set_attr_u32(ct2, attr, v2);
+		nfct_set_attr_u16(ct2, attr, v2);
 
 	ret = nfct_cmp(ct1, ct2, NFCT_CMP_ALL | flags);
 
@@ -264,26 +264,80 @@ static int test_cmp_attr32(int attr, bool at1, bool at2,
 }
 static void test_nfct_cmp_attr(int attr)
 {
-	assert(test_cmp_attr32(ATTR_ZONE, false, false, 0, 0, 0) == 1);
-	assert(test_cmp_attr32(ATTR_ZONE, true, true, 0, 0, 0) == 1);
-	assert(test_cmp_attr32(ATTR_ZONE, true, true, 1, 1, 0) == 1);
-
-	/* This compare should be true */
-	assert(test_cmp_attr32(ATTR_ZONE, false, true, 0, 1, 0) == 1);
-
-	assert(test_cmp_attr32(ATTR_ZONE, true, true, 1, 1, NFCT_CMP_STRICT) == 1);
-
-	assert(test_cmp_attr32(ATTR_ZONE, true, false, 0, 0, NFCT_CMP_STRICT) == 1);
-	assert(test_cmp_attr32(ATTR_ZONE, false, true, 0, 0, NFCT_CMP_STRICT) == 1);
-
-	assert(test_cmp_attr32(ATTR_ZONE, false, true, 0, 1, NFCT_CMP_STRICT) == 0);
-
-	assert(test_cmp_attr32(ATTR_ZONE, false, true, 0, 1, NFCT_CMP_MASK) == 1);
-	assert(test_cmp_attr32(ATTR_ZONE, true, true, 0, 1, NFCT_CMP_MASK) == 0);
-
-	assert(test_cmp_attr32(ATTR_ZONE, true, false, 0, 0, NFCT_CMP_MASK) == 1);
-
-	assert(test_cmp_attr32(ATTR_ZONE, true, false, 1, 0, NFCT_CMP_MASK) == 0);
+	unsigned int flags = 0;
+
+	/* 0000, 1000, 1100, 0010, 1010... */
+	/*		       attr       at1    at2    v1	v2 */
+	assert(test_cmp_attr16(ATTR_ZONE, false, false,	0,	0,	flags) == 1);
+	assert(test_cmp_attr16(ATTR_ZONE, true,	 false,	0,	0,	flags) == 1);
+	assert(test_cmp_attr16(ATTR_ZONE, false, true,	0,	0,	flags) == 1);
+	assert(test_cmp_attr16(ATTR_ZONE, true,	 true,	0,	0,	flags) == 1);
+	assert(test_cmp_attr16(ATTR_ZONE, false, false,	1,	0,	flags) == 1); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, true,  false,	1,	0,	flags) == 1);
+	assert(test_cmp_attr16(ATTR_ZONE, false, true,	1,	0,	flags) == 1); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, true,  true,	1,	0,	flags) == 0);
+	assert(test_cmp_attr16(ATTR_ZONE, false, false,	0,	1,	flags) == 1); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, true,  false,	0,	1,	flags) == 1); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, false, true,	0,	1,	flags) == 1);
+	assert(test_cmp_attr16(ATTR_ZONE, true,  true,	0,	1,	flags) == 0);
+	assert(test_cmp_attr16(ATTR_ZONE, false, false,	1,	1,	flags) == 1); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, true,  false,	1,	1,	flags) == 1); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, false, true,	1,	1,	flags) == 1); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, true,  true,	1,	1,	flags) == 1);
+
+	flags = NFCT_CMP_STRICT;
+	assert(test_cmp_attr16(ATTR_ZONE, false, false,	0,	0,	flags) == 1);
+	assert(test_cmp_attr16(ATTR_ZONE, true,	 false,	0,	0,	flags) == 0);
+	assert(test_cmp_attr16(ATTR_ZONE, false, true,	0,	0,	flags) == 0);
+	assert(test_cmp_attr16(ATTR_ZONE, true,	 true,	0,	0,	flags) == 1);
+	assert(test_cmp_attr16(ATTR_ZONE, false, false,	1,	0,	flags) == 1); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, true,  false,	1,	0,	flags) == 0);
+	assert(test_cmp_attr16(ATTR_ZONE, false, true,	1,	0,	flags) == 0); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, true,  true,	1,	0,	flags) == 0);
+	assert(test_cmp_attr16(ATTR_ZONE, false, false,	0,	1,	flags) == 1); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, true,  false,	0,	1,	flags) == 0); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, false, true,	0,	1,	flags) == 0);
+	assert(test_cmp_attr16(ATTR_ZONE, true,  true,	0,	1,	flags) == 0);
+	assert(test_cmp_attr16(ATTR_ZONE, false, false,	1,	1,	flags) == 1); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, true,  false,	1,	1,	flags) == 0); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, false, true,	1,	1,	flags) == 0); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, true,  true,	1,	1,	flags) == 1);
+
+	flags = NFCT_CMP_MASK;
+	assert(test_cmp_attr16(ATTR_ZONE, false, false,	0,	0,	flags) == 1);
+	assert(test_cmp_attr16(ATTR_ZONE, true,	 false,	0,	0,	flags) == 0);
+	assert(test_cmp_attr16(ATTR_ZONE, false, true,	0,	0,	flags) == 1);
+	assert(test_cmp_attr16(ATTR_ZONE, true,	 true,	0,	0,	flags) == 1);
+	assert(test_cmp_attr16(ATTR_ZONE, false, false,	1,	0,	flags) == 1); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, true,  false,	1,	0,	flags) == 0);
+	assert(test_cmp_attr16(ATTR_ZONE, false, true,	1,	0,	flags) == 1); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, true,  true,	1,	0,	flags) == 0);
+	assert(test_cmp_attr16(ATTR_ZONE, false, false,	0,	1,	flags) == 1); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, true,  false,	0,	1,	flags) == 0); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, false, true,	0,	1,	flags) == 1);
+	assert(test_cmp_attr16(ATTR_ZONE, true,  true,	0,	1,	flags) == 0);
+	assert(test_cmp_attr16(ATTR_ZONE, false, false,	1,	1,	flags) == 1); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, true,  false,	1,	1,	flags) == 0); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, false, true,	1,	1,	flags) == 1); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, true,  true,	1,	1,	flags) == 1);
+
+	flags = NFCT_CMP_STRICT|NFCT_CMP_MASK;
+	assert(test_cmp_attr16(ATTR_ZONE, false, false,	0,	0,	flags) == 1);
+	assert(test_cmp_attr16(ATTR_ZONE, true,	 false,	0,	0,	flags) == 0);
+	assert(test_cmp_attr16(ATTR_ZONE, false, true,	0,	0,	flags) == 0);
+	assert(test_cmp_attr16(ATTR_ZONE, true,	 true,	0,	0,	flags) == 1);
+	assert(test_cmp_attr16(ATTR_ZONE, false, false,	1,	0,	flags) == 1); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, true,  false,	1,	0,	flags) == 0);
+	assert(test_cmp_attr16(ATTR_ZONE, false, true,	1,	0,	flags) == 0); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, true,  true,	1,	0,	flags) == 0);
+	assert(test_cmp_attr16(ATTR_ZONE, false, false,	0,	1,	flags) == 1); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, true,  false,	0,	1,	flags) == 0); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, false, true,	0,	1,	flags) == 0);
+	assert(test_cmp_attr16(ATTR_ZONE, true,  true,	0,	1,	flags) == 0);
+	assert(test_cmp_attr16(ATTR_ZONE, false, false,	1,	1,	flags) == 1); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, true,  false,	1,	1,	flags) == 0); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, false, true,	1,	1,	flags) == 0); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, true,  true,	1,	1,	flags) == 1);
 }
 
 static void test_nfct_cmp_api(struct nf_conntrack *ct1, struct nf_conntrack *ct2)
-- 
1.9.1


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

* Re: [PATCH v3 1/2 libnetfilter_conntrack] qa: update cmp ATTR_ZONE size mark and zone
  2014-06-23 10:15     ` [PATCH v3 1/2 libnetfilter_conntrack] qa: update cmp ATTR_ZONE size " Ken-ichirou MATSUZAWA
@ 2014-06-24 15:23       ` Florian Westphal
  0 siblings, 0 replies; 7+ messages in thread
From: Florian Westphal @ 2014-06-24 15:23 UTC (permalink / raw)
  To: Ken-ichirou MATSUZAWA
  Cc: Florian Westphal, The netfilter developer mailinglist

Ken-ichirou MATSUZAWA <chamaken@gmail.com> wrote:
> ---
>  qa/test_api.c | 102 ++++++++++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 78 insertions(+), 24 deletions(-)

[ ATTR_ZONE test cases with all flag combinations ]

Thanks, this should indeed cover all the possible combinations of
attr states and compare flags.

I've merged this patch with the test_api change from patch 2/2 and
pushed the result.

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

* [PATCH v3 2/2 libnetfilter_conntrack] conntrack: zero value handling of mark and zone
  2014-06-23 10:12   ` [PATCH v3 0/2 " Ken-ichirou MATSUZAWA
  2014-06-23 10:15     ` [PATCH v3 1/2 libnetfilter_conntrack] qa: update cmp ATTR_ZONE size " Ken-ichirou MATSUZAWA
@ 2014-06-23 10:18     ` Ken-ichirou MATSUZAWA
  1 sibling, 0 replies; 7+ messages in thread
From: Ken-ichirou MATSUZAWA @ 2014-06-23 10:18 UTC (permalink / raw)
  To: Florian Westphal; +Cc: The netfilter developer mailinglist

This patch enables comparison of 0 value with mark and zone since both CTA_MARK
and CTA_ZONE are not set in case of its value is 0. This patch has just passed
qa test which Florian wrote, I guess there is more sophiscated way that settle
cmp_mark() and cmp_zone().

Signed-off-by: Ken-ichirou MATSUZAWA <chamas@h4.dion.ne.jp>
---
 qa/test_api.c           | 20 ++++++++++----------
 src/conntrack/compare.c | 44 ++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 50 insertions(+), 14 deletions(-)

diff --git a/qa/test_api.c b/qa/test_api.c
index 1335b23..c8faeda 100644
--- a/qa/test_api.c
+++ b/qa/test_api.c
@@ -287,15 +287,15 @@ static void test_nfct_cmp_attr(int attr)
 
 	flags = NFCT_CMP_STRICT;
 	assert(test_cmp_attr16(ATTR_ZONE, false, false,	0,	0,	flags) == 1);
-	assert(test_cmp_attr16(ATTR_ZONE, true,	 false,	0,	0,	flags) == 0);
-	assert(test_cmp_attr16(ATTR_ZONE, false, true,	0,	0,	flags) == 0);
+	assert(test_cmp_attr16(ATTR_ZONE, true,	 false,	0,	0,	flags) == 1);
+	assert(test_cmp_attr16(ATTR_ZONE, false, true,	0,	0,	flags) == 1);
 	assert(test_cmp_attr16(ATTR_ZONE, true,	 true,	0,	0,	flags) == 1);
 	assert(test_cmp_attr16(ATTR_ZONE, false, false,	1,	0,	flags) == 1); /* verbose */
 	assert(test_cmp_attr16(ATTR_ZONE, true,  false,	1,	0,	flags) == 0);
-	assert(test_cmp_attr16(ATTR_ZONE, false, true,	1,	0,	flags) == 0); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, false, true,	1,	0,	flags) == 1); /* verbose */
 	assert(test_cmp_attr16(ATTR_ZONE, true,  true,	1,	0,	flags) == 0);
 	assert(test_cmp_attr16(ATTR_ZONE, false, false,	0,	1,	flags) == 1); /* verbose */
-	assert(test_cmp_attr16(ATTR_ZONE, true,  false,	0,	1,	flags) == 0); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, true,  false,	0,	1,	flags) == 1); /* verbose */
 	assert(test_cmp_attr16(ATTR_ZONE, false, true,	0,	1,	flags) == 0);
 	assert(test_cmp_attr16(ATTR_ZONE, true,  true,	0,	1,	flags) == 0);
 	assert(test_cmp_attr16(ATTR_ZONE, false, false,	1,	1,	flags) == 1); /* verbose */
@@ -305,7 +305,7 @@ static void test_nfct_cmp_attr(int attr)
 
 	flags = NFCT_CMP_MASK;
 	assert(test_cmp_attr16(ATTR_ZONE, false, false,	0,	0,	flags) == 1);
-	assert(test_cmp_attr16(ATTR_ZONE, true,	 false,	0,	0,	flags) == 0);
+	assert(test_cmp_attr16(ATTR_ZONE, true,	 false,	0,	0,	flags) == 1);
 	assert(test_cmp_attr16(ATTR_ZONE, false, true,	0,	0,	flags) == 1);
 	assert(test_cmp_attr16(ATTR_ZONE, true,	 true,	0,	0,	flags) == 1);
 	assert(test_cmp_attr16(ATTR_ZONE, false, false,	1,	0,	flags) == 1); /* verbose */
@@ -313,7 +313,7 @@ static void test_nfct_cmp_attr(int attr)
 	assert(test_cmp_attr16(ATTR_ZONE, false, true,	1,	0,	flags) == 1); /* verbose */
 	assert(test_cmp_attr16(ATTR_ZONE, true,  true,	1,	0,	flags) == 0);
 	assert(test_cmp_attr16(ATTR_ZONE, false, false,	0,	1,	flags) == 1); /* verbose */
-	assert(test_cmp_attr16(ATTR_ZONE, true,  false,	0,	1,	flags) == 0); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, true,  false,	0,	1,	flags) == 1); /* verbose */
 	assert(test_cmp_attr16(ATTR_ZONE, false, true,	0,	1,	flags) == 1);
 	assert(test_cmp_attr16(ATTR_ZONE, true,  true,	0,	1,	flags) == 0);
 	assert(test_cmp_attr16(ATTR_ZONE, false, false,	1,	1,	flags) == 1); /* verbose */
@@ -323,15 +323,15 @@ static void test_nfct_cmp_attr(int attr)
 
 	flags = NFCT_CMP_STRICT|NFCT_CMP_MASK;
 	assert(test_cmp_attr16(ATTR_ZONE, false, false,	0,	0,	flags) == 1);
-	assert(test_cmp_attr16(ATTR_ZONE, true,	 false,	0,	0,	flags) == 0);
-	assert(test_cmp_attr16(ATTR_ZONE, false, true,	0,	0,	flags) == 0);
+	assert(test_cmp_attr16(ATTR_ZONE, true,	 false,	0,	0,	flags) == 1);
+	assert(test_cmp_attr16(ATTR_ZONE, false, true,	0,	0,	flags) == 1);
 	assert(test_cmp_attr16(ATTR_ZONE, true,	 true,	0,	0,	flags) == 1);
 	assert(test_cmp_attr16(ATTR_ZONE, false, false,	1,	0,	flags) == 1); /* verbose */
 	assert(test_cmp_attr16(ATTR_ZONE, true,  false,	1,	0,	flags) == 0);
-	assert(test_cmp_attr16(ATTR_ZONE, false, true,	1,	0,	flags) == 0); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, false, true,	1,	0,	flags) == 1); /* verbose */
 	assert(test_cmp_attr16(ATTR_ZONE, true,  true,	1,	0,	flags) == 0);
 	assert(test_cmp_attr16(ATTR_ZONE, false, false,	0,	1,	flags) == 1); /* verbose */
-	assert(test_cmp_attr16(ATTR_ZONE, true,  false,	0,	1,	flags) == 0); /* verbose */
+	assert(test_cmp_attr16(ATTR_ZONE, true,  false,	0,	1,	flags) == 1); /* verbose */
 	assert(test_cmp_attr16(ATTR_ZONE, false, true,	0,	1,	flags) == 0);
 	assert(test_cmp_attr16(ATTR_ZONE, true,  true,	0,	1,	flags) == 0);
 	assert(test_cmp_attr16(ATTR_ZONE, false, false,	1,	1,	flags) == 1); /* verbose */
diff --git a/src/conntrack/compare.c b/src/conntrack/compare.c
index f4a194a..a3da81b 100644
--- a/src/conntrack/compare.c
+++ b/src/conntrack/compare.c
@@ -291,7 +291,25 @@ cmp_mark(const struct nf_conntrack *ct1,
 	 const struct nf_conntrack *ct2,
 	 unsigned int flags)
 {
-	return (ct1->mark == ct2->mark);
+	int a = test_bit(ATTR_MARK, ct1->head.set);
+	int b = test_bit(ATTR_MARK, ct2->head.set);
+	if (a & b) {
+		return (ct1->mark == ct2->mark);
+	} else if (!a && !b) {
+		return 1;
+	}
+
+	switch (flags & (NFCT_CMP_STRICT | NFCT_CMP_MASK)) {
+	case NFCT_CMP_MASK:
+		if (!a)
+			return 1;
+	case NFCT_CMP_STRICT | NFCT_CMP_MASK:
+	case NFCT_CMP_STRICT:
+		return nfct_get_attr_u32(ct1, ATTR_MARK)
+			== nfct_get_attr_u32(ct2, ATTR_MARK);
+	default:
+		return 1;
+	}
 }
 
 static int 
@@ -357,7 +375,25 @@ cmp_zone(const struct nf_conntrack *ct1,
 	 const struct nf_conntrack *ct2,
 	 unsigned int flags)
 {
-	return (ct1->zone == ct2->zone);
+	int a = test_bit(ATTR_ZONE, ct1->head.set);
+	int b = test_bit(ATTR_ZONE, ct2->head.set);
+	if (a & b) {
+		return (ct1->zone == ct2->zone);
+	} else if (!a && !b) {
+		return 1;
+	}
+
+	switch (flags & (NFCT_CMP_STRICT | NFCT_CMP_MASK)) {
+	case NFCT_CMP_MASK:
+		if (!a)
+			return 1;
+	case NFCT_CMP_STRICT | NFCT_CMP_MASK:
+	case NFCT_CMP_STRICT:
+		return nfct_get_attr_u16(ct1, ATTR_ZONE)
+			== nfct_get_attr_u16(ct2, ATTR_ZONE);
+	default:
+		return 1;
+	}
 }
 
 static int
@@ -421,7 +457,7 @@ static int cmp_meta(const struct nf_conntrack *ct1,
 {
 	if (!__cmp(ATTR_ID, ct1, ct2, flags, cmp_id))
 		return 0;
-	if (!__cmp(ATTR_MARK, ct1, ct2, flags, cmp_mark))
+	if (!cmp_mark(ct1, ct2, flags))
 		return 0;
 	if (!__cmp(ATTR_TIMEOUT, ct1, ct2, flags, cmp_timeout))
 		return 0;
@@ -433,7 +469,7 @@ static int cmp_meta(const struct nf_conntrack *ct1,
 		return 0;
 	if (!__cmp(ATTR_DCCP_STATE, ct1, ct2, flags, cmp_dccp_state))
 		return 0;
-	if (!__cmp(ATTR_ZONE, ct1, ct2, flags, cmp_zone))
+	if (!cmp_zone(ct1, ct2, flags))
 		return 0;
 	if (!__cmp(ATTR_SECCTX, ct1, ct2, flags, cmp_secctx))
 		return 0;
-- 
1.9.1


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

end of thread, other threads:[~2014-06-24 15:23 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-17 12:04 [PATCH v2 libnetfilter_conntrack] zero value handling of mark and zone Ken-ichirou MATSUZAWA
2014-06-17 16:40 ` Florian Westphal
2014-06-19 15:16 ` Florian Westphal
2014-06-23 10:12   ` [PATCH v3 0/2 " Ken-ichirou MATSUZAWA
2014-06-23 10:15     ` [PATCH v3 1/2 libnetfilter_conntrack] qa: update cmp ATTR_ZONE size " Ken-ichirou MATSUZAWA
2014-06-24 15:23       ` Florian Westphal
2014-06-23 10:18     ` [PATCH v3 2/2 libnetfilter_conntrack] conntrack: zero value handling of " Ken-ichirou MATSUZAWA

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).