[PATCH 0/2] fix flow rules copy functions

[PATCH 0/2] fix flow rules copy functions

[Matan Azrad]

some issue seen when trying to test failsafe plug out and in. 

Matan Azrad (2):
  app/testpmd: fix flow rule copy functions
  ethdev: fix flow rule copy functions

 app/test-pmd/config.c       | 16 ++++++++++------
 lib/librte_ether/rte_flow.c | 16 ++++++++++------
 2 files changed, 20 insertions(+), 12 deletions(-)

-- 
1.8.3.1

[PATCH 1/2] app/testpmd: fix flow rule copy functions

[Matan Azrad]

The corrupted code checks only RAW flow item type special case for
returning its size but doesn't deal with any other flow item type
and returns 0 for all the others.

This bug leaves the flow descriptor empty for non RAW types.

The fix takes the correct size to any regular types from appropriate
array.

The same issue, with a similar fix, is in flow action size method which deals only with
RSS special type.

Fixes: 99457e8fb8ce ("app/testpmd: implement basic support for flow API")
Cc: stable@dpdk.org

Signed-off-by: Matan Azrad <matan@mellanox.com>
---
 app/test-pmd/config.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index ee6644d..0c7c514 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -954,8 +954,10 @@ struct rss_type_info {
 flow_item_spec_size(const struct rte_flow_item *item,
 		    size_t *size, size_t *pad)
 {
-	if (!item->spec)
+	if (!item->spec){
+		*size = 0;
 		goto empty;
+	}
 	switch (item->type) {
 		union {
 			const struct rte_flow_item_raw *raw;
@@ -967,10 +969,10 @@ struct rss_type_info {
 			spec.raw->length * sizeof(*spec.raw->pattern);
 		break;
 	default:
-empty:
-		*size = 0;
+		*size = flow_item[item->type].size;
 		break;
 	}
+empty:
 	*pad = RTE_ALIGN_CEIL(*size, sizeof(double)) - *size;
 }
 
@@ -1005,8 +1007,10 @@ struct rss_type_info {
 flow_action_conf_size(const struct rte_flow_action *action,
 		      size_t *size, size_t *pad)
 {
-	if (!action->conf)
+	if (!action->conf){
+		*size = 0;
 		goto empty;
+	}
 	switch (action->type) {
 		union {
 			const struct rte_flow_action_rss *rss;
@@ -1018,10 +1022,10 @@ struct rss_type_info {
 			conf.rss->num * sizeof(*conf.rss->queue);
 		break;
 	default:
-empty:
-		*size = 0;
+		*size = flow_action[action->type].size;
 		break;
 	}
+empty:
 	*pad = RTE_ALIGN_CEIL(*size, sizeof(double)) - *size;
 }
 
-- 
1.8.3.1

[PATCH 2/2] ethdev: fix flow rule copy functions

[Matan Azrad]

The corrupted code checks only RAW flow item type special case for
returning its size but doesn't deal with any other flow item type
and returns 0 for all the others.

This bug leaves the flow descriptor empty for non RAW types.

The fix takes the correct size to any regular types from appropriate
array.

The same issue, with a similar fix, is in flow action size method
which deals only with RSS special type.

This bug was already present in the original code taken from testpmd.

Fixes: bae73ebad91c ("ethdev: add flow rule copy function")

Signed-off-by: Matan Azrad <matan@mellanox.com>
---
 lib/librte_ether/rte_flow.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/lib/librte_ether/rte_flow.c b/lib/librte_ether/rte_flow.c
index 884e4f6..00a07a1 100644
--- a/lib/librte_ether/rte_flow.c
+++ b/lib/librte_ether/rte_flow.c
@@ -248,8 +248,10 @@ struct rte_flow *
 flow_item_spec_size(const struct rte_flow_item *item,
 		    size_t *size, size_t *pad)
 {
-	if (!item->spec)
+	if (!item->spec){
+		*size = 0;
 		goto empty;
+	}
 	switch (item->type) {
 		union {
 			const struct rte_flow_item_raw *raw;
@@ -262,10 +264,10 @@ struct rte_flow *
 			spec.raw->length * sizeof(*spec.raw->pattern);
 		break;
 	default:
-empty:
-		*size = 0;
+		*size = rte_flow_desc_item[item->type].size;
 		break;
 	}
+empty:
 	*pad = RTE_ALIGN_CEIL(*size, sizeof(double)) - *size;
 }
 
@@ -274,8 +276,10 @@ struct rte_flow *
 flow_action_conf_size(const struct rte_flow_action *action,
 		      size_t *size, size_t *pad)
 {
-	if (!action->conf)
+	if (!action->conf){
+		*size = 0;
 		goto empty;
+	}
 	switch (action->type) {
 		union {
 			const struct rte_flow_action_rss *rss;
@@ -288,10 +292,10 @@ struct rte_flow *
 			conf.rss->num * sizeof(*conf.rss->queue);
 		break;
 	default:
-empty:
-		*size = 0;
+		*size = rte_flow_desc_action[action->type].size;
 		break;
 	}
+empty:
 	*pad = RTE_ALIGN_CEIL(*size, sizeof(double)) - *size;
 }
 
-- 
1.8.3.1

Re: [PATCH 0/2] fix flow rules copy functions

[Adrien Mazarguil]

On Mon, Jul 24, 2017 at 10:10:20AM +0300, Matan Azrad wrote:
> some issue seen when trying to test failsafe plug out and in. 
> 
> Matan Azrad (2):
>   app/testpmd: fix flow rule copy functions
>   ethdev: fix flow rule copy functions
> 
>  app/test-pmd/config.c       | 16 ++++++++++------
>  lib/librte_ether/rte_flow.c | 16 ++++++++++------
>  2 files changed, 20 insertions(+), 12 deletions(-)

Thanks!

I think the original code didn't have these functions and used the
flow_item[x].size value directly, which worked fine until I added support
for the RAW item and the RSS action. Both had to be handled through special
functions which I messed up.

Unfortunately I didn't catch it in testpmd because it doesn't do anything
with the copied data unlike the fail-safe PMD.

Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>

-- 
Adrien Mazarguil
6WIND

[PATCH v2 0/2] fix flow rules copy functions

[Matan Azrad]

some issue seen when trying to test failsafe plug out and in.

The V2 fixes some checkpatch issues. 

Matan Azrad (2):
  app/testpmd: fix flow rule copy functions
  ethdev: fix flow rule copy functions

 app/test-pmd/config.c       | 16 ++++++++++------
 lib/librte_ether/rte_flow.c | 16 ++++++++++------
 2 files changed, 20 insertions(+), 12 deletions(-)

-- 
1.8.3.1

[PATCH v2 1/2] app/testpmd: fix flow rule copy functions

[Matan Azrad]

The corrupted code checks only RAW flow item type special case for
returning its size but doesn't deal with any other flow item type
and returns 0 for all the others.

This bug leaves the flow descriptor empty for non RAW types.

The fix takes the correct size to any regular types from appropriate
array.

The same issue, with a similar fix, is in flow action size method which
deals only with RSS special type.

Fixes: 99457e8fb8ce ("app/testpmd: implement basic support for flow API")
Cc: stable@dpdk.org

Signed-off-by: Matan Azrad <matan@mellanox.com>
---
 app/test-pmd/config.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index ee6644d..3ae3e1c 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -954,8 +954,10 @@ struct rss_type_info {
 flow_item_spec_size(const struct rte_flow_item *item,
 		    size_t *size, size_t *pad)
 {
-	if (!item->spec)
+	if (!item->spec) {
+		*size = 0;
 		goto empty;
+	}
 	switch (item->type) {
 		union {
 			const struct rte_flow_item_raw *raw;
@@ -967,10 +969,10 @@ struct rss_type_info {
 			spec.raw->length * sizeof(*spec.raw->pattern);
 		break;
 	default:
-empty:
-		*size = 0;
+		*size = flow_item[item->type].size;
 		break;
 	}
+empty:
 	*pad = RTE_ALIGN_CEIL(*size, sizeof(double)) - *size;
 }
 
@@ -1005,8 +1007,10 @@ struct rss_type_info {
 flow_action_conf_size(const struct rte_flow_action *action,
 		      size_t *size, size_t *pad)
 {
-	if (!action->conf)
+	if (!action->conf) {
+		*size = 0;
 		goto empty;
+	}
 	switch (action->type) {
 		union {
 			const struct rte_flow_action_rss *rss;
@@ -1018,10 +1022,10 @@ struct rss_type_info {
 			conf.rss->num * sizeof(*conf.rss->queue);
 		break;
 	default:
-empty:
-		*size = 0;
+		*size = flow_action[action->type].size;
 		break;
 	}
+empty:
 	*pad = RTE_ALIGN_CEIL(*size, sizeof(double)) - *size;
 }
 
-- 
1.8.3.1

[PATCH v2 2/2] ethdev: fix flow rule copy functions

[Matan Azrad]

The corrupted code checks only RAW flow item type special case for
returning its size but doesn't deal with any other flow item type
and returns 0 for all the others.

This bug leaves the flow descriptor empty for non RAW types.

The fix takes the correct size to any regular types from appropriate
array.

The same issue, with a similar fix, is in flow action size method
which deals only with RSS special type.

This bug was already present in the original code taken from testpmd.

Fixes: bae73ebad91c ("ethdev: add flow rule copy function")

Signed-off-by: Matan Azrad <matan@mellanox.com>
---
 lib/librte_ether/rte_flow.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/lib/librte_ether/rte_flow.c b/lib/librte_ether/rte_flow.c
index 884e4f6..2001fbb 100644
--- a/lib/librte_ether/rte_flow.c
+++ b/lib/librte_ether/rte_flow.c
@@ -248,8 +248,10 @@ struct rte_flow *
 flow_item_spec_size(const struct rte_flow_item *item,
 		    size_t *size, size_t *pad)
 {
-	if (!item->spec)
+	if (!item->spec) {
+		*size = 0;
 		goto empty;
+	}
 	switch (item->type) {
 		union {
 			const struct rte_flow_item_raw *raw;
@@ -262,10 +264,10 @@ struct rte_flow *
 			spec.raw->length * sizeof(*spec.raw->pattern);
 		break;
 	default:
-empty:
-		*size = 0;
+		*size = rte_flow_desc_item[item->type].size;
 		break;
 	}
+empty:
 	*pad = RTE_ALIGN_CEIL(*size, sizeof(double)) - *size;
 }
 
@@ -274,8 +276,10 @@ struct rte_flow *
 flow_action_conf_size(const struct rte_flow_action *action,
 		      size_t *size, size_t *pad)
 {
-	if (!action->conf)
+	if (!action->conf) {
+		*size = 0;
 		goto empty;
+	}
 	switch (action->type) {
 		union {
 			const struct rte_flow_action_rss *rss;
@@ -288,10 +292,10 @@ struct rte_flow *
 			conf.rss->num * sizeof(*conf.rss->queue);
 		break;
 	default:
-empty:
-		*size = 0;
+		*size = rte_flow_desc_action[action->type].size;
 		break;
 	}
+empty:
 	*pad = RTE_ALIGN_CEIL(*size, sizeof(double)) - *size;
 }
 
-- 
1.8.3.1

Re: [PATCH v2 0/2] fix flow rules copy functions

[Thomas Monjalon]

24/07/2017 15:47, Matan Azrad:
> some issue seen when trying to test failsafe plug out and in.
> 
> The V2 fixes some checkpatch issues. 
> 
> Matan Azrad (2):
>   app/testpmd: fix flow rule copy functions
>   ethdev: fix flow rule copy functions

Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>

Applied, thanks