[dpdk-dev] [PATCH 1/2] app/testpmd: fix copy of raw flow item (revisited)

[dpdk-dev] [PATCH 1/2] app/testpmd: fix copy of raw flow item (revisited)

[Adrien Mazarguil]

While previous fix with the same title does address the main issue, root
cause is that proper handling of spec/last/mask was overlooked in the
original patch.

Mask and last fields must be taken into account at all times.

Fixes: d0ad8648b1c5 ("app/testpmd: fix RSS flow action configuration")
Fixes: 67af7ecc52ec ("app/testpmd: fix copy of raw flow item")
Cc: stable@dpdk.org
Cc: Qi Zhang <qi.z.zhang@intel.com>

Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
---
 app/test-pmd/config.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index b07376af9..4520084b8 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1049,17 +1049,26 @@ flow_item_spec_copy(void *buf, const struct rte_flow_item *item,
 		    enum item_spec_type type)
 {
 	size_t size = 0;
-	const void *item_spec =
+	const void *data =
 		type == ITEM_SPEC ? item->spec :
 		type == ITEM_LAST ? item->last :
 		type == ITEM_MASK ? item->mask :
 		NULL;
 
-	if (!item_spec)
+	if (!item->spec || !data)
 		goto empty;
 	switch (item->type) {
 		union {
 			const struct rte_flow_item_raw *raw;
+		} spec;
+		union {
+			const struct rte_flow_item_raw *raw;
+		} last;
+		union {
+			const struct rte_flow_item_raw *raw;
+		} mask;
+		union {
+			const struct rte_flow_item_raw *raw;
 		} src;
 		union {
 			struct rte_flow_item_raw *raw;
@@ -1067,12 +1076,21 @@ flow_item_spec_copy(void *buf, const struct rte_flow_item *item,
 		size_t off;
 
 	case RTE_FLOW_ITEM_TYPE_RAW:
-		src.raw = item_spec;
+		spec.raw = item->spec;
+		last.raw = item->last ? item->last : item->spec;
+		mask.raw = item->mask ? item->mask : &rte_flow_item_raw_mask;
+		src.raw = data;
 		dst.raw = buf;
 		off = RTE_ALIGN_CEIL(sizeof(struct rte_flow_item_raw),
 				     sizeof(*src.raw->pattern));
-		size = off + ((const struct rte_flow_item_raw *)item->spec)->
-			length * sizeof(*src.raw->pattern);
+		if (type == ITEM_SPEC ||
+		    (type == ITEM_MASK &&
+		     ((spec.raw->length & mask.raw->length) >=
+		      (last.raw->length & mask.raw->length))))
+			size = spec.raw->length & mask.raw->length;
+		else
+			size = last.raw->length & mask.raw->length;
+		size = off + size * sizeof(*src.raw->pattern);
 		if (dst.raw) {
 			memcpy(dst.raw, src.raw, sizeof(*src.raw));
 			dst.raw->pattern = memcpy((uint8_t *)dst.raw + off,
@@ -1083,7 +1101,7 @@ flow_item_spec_copy(void *buf, const struct rte_flow_item *item,
 	default:
 		size = flow_item[item->type].size;
 		if (buf)
-			memcpy(buf, item_spec, size);
+			memcpy(buf, data, size);
 		break;
 	}
 empty:
-- 
2.11.0

[dpdk-dev] [PATCH 2/2] ethdev: fix shallow copy of flow API RAW item

[Adrien Mazarguil]

Like original commit mentioned below, this fix synchronizes flow rule copy
function with testpmd's own implementation following "app/testpmd: fix copy
of raw flow item (revisited)".

Fixes: d0ad8648b1c5 ("ethdev: fix shallow copy of flow API RSS action")
Cc: stable@dpdk.org
Cc: Qi Zhang <qi.z.zhang@intel.com>

Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
---
 lib/librte_ethdev/rte_flow.c | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index 7947529da..b2afba089 100644
--- a/lib/librte_ethdev/rte_flow.c
+++ b/lib/librte_ethdev/rte_flow.c
@@ -300,17 +300,26 @@ flow_item_spec_copy(void *buf, const struct rte_flow_item *item,
 		    enum item_spec_type type)
 {
 	size_t size = 0;
-	const void *item_spec =
+	const void *data =
 		type == ITEM_SPEC ? item->spec :
 		type == ITEM_LAST ? item->last :
 		type == ITEM_MASK ? item->mask :
 		NULL;
 
-	if (!item_spec)
+	if (!item->spec || !data)
 		goto empty;
 	switch (item->type) {
 		union {
 			const struct rte_flow_item_raw *raw;
+		} spec;
+		union {
+			const struct rte_flow_item_raw *raw;
+		} last;
+		union {
+			const struct rte_flow_item_raw *raw;
+		} mask;
+		union {
+			const struct rte_flow_item_raw *raw;
 		} src;
 		union {
 			struct rte_flow_item_raw *raw;
@@ -318,11 +327,21 @@ flow_item_spec_copy(void *buf, const struct rte_flow_item *item,
 		size_t off;
 
 	case RTE_FLOW_ITEM_TYPE_RAW:
-		src.raw = item_spec;
+		spec.raw = item->spec;
+		last.raw = item->last ? item->last : item->spec;
+		mask.raw = item->mask ? item->mask : &rte_flow_item_raw_mask;
+		src.raw = data;
 		dst.raw = buf;
 		off = RTE_ALIGN_CEIL(sizeof(struct rte_flow_item_raw),
 				     sizeof(*src.raw->pattern));
-		size = off + src.raw->length * sizeof(*src.raw->pattern);
+		if (type == ITEM_SPEC ||
+		    (type == ITEM_MASK &&
+		     ((spec.raw->length & mask.raw->length) >=
+		      (last.raw->length & mask.raw->length))))
+			size = spec.raw->length & mask.raw->length;
+		else
+			size = last.raw->length & mask.raw->length;
+		size = off + size * sizeof(*src.raw->pattern);
 		if (dst.raw) {
 			memcpy(dst.raw, src.raw, sizeof(*src.raw));
 			dst.raw->pattern = memcpy((uint8_t *)dst.raw + off,
@@ -333,7 +352,7 @@ flow_item_spec_copy(void *buf, const struct rte_flow_item *item,
 	default:
 		size = rte_flow_desc_item[item->type].size;
 		if (buf)
-			memcpy(buf, item_spec, size);
+			memcpy(buf, data, size);
 		break;
 	}
 empty:
-- 
2.11.0

Re: [dpdk-dev] [PATCH 2/2] ethdev: fix shallow copy of flow API RAW item

[Ferruh Yigit]

On 5/16/2018 4:41 PM, Adrien Mazarguil wrote:
> Like original commit mentioned below, this fix synchronizes flow rule copy
> function with testpmd's own implementation following "app/testpmd: fix copy
> of raw flow item (revisited)".
> 
> Fixes: d0ad8648b1c5 ("ethdev: fix shallow copy of flow API RSS action")
> Cc: stable@dpdk.org
> Cc: Qi Zhang <qi.z.zhang@intel.com>
> 
> Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>

Hi Thomas,

What do you suggest about this one?
Scope is limited to rte_flow but still many features are now relies on rte_flow,
what is your comment on getting this in rc5?


> ---
>  lib/librte_ethdev/rte_flow.c | 29 ++++++++++++++++++++++++-----
>  1 file changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
> index 7947529da..b2afba089 100644
> --- a/lib/librte_ethdev/rte_flow.c
> +++ b/lib/librte_ethdev/rte_flow.c
> @@ -300,17 +300,26 @@ flow_item_spec_copy(void *buf, const struct rte_flow_item *item,
>  		    enum item_spec_type type)
>  {
>  	size_t size = 0;
> -	const void *item_spec =
> +	const void *data =
>  		type == ITEM_SPEC ? item->spec :
>  		type == ITEM_LAST ? item->last :
>  		type == ITEM_MASK ? item->mask :
>  		NULL;
>  
> -	if (!item_spec)
> +	if (!item->spec || !data)
>  		goto empty;
>  	switch (item->type) {
>  		union {
>  			const struct rte_flow_item_raw *raw;
> +		} spec;
> +		union {
> +			const struct rte_flow_item_raw *raw;
> +		} last;
> +		union {
> +			const struct rte_flow_item_raw *raw;
> +		} mask;
> +		union {
> +			const struct rte_flow_item_raw *raw;
>  		} src;
>  		union {
>  			struct rte_flow_item_raw *raw;
> @@ -318,11 +327,21 @@ flow_item_spec_copy(void *buf, const struct rte_flow_item *item,
>  		size_t off;
>  
>  	case RTE_FLOW_ITEM_TYPE_RAW:
> -		src.raw = item_spec;
> +		spec.raw = item->spec;
> +		last.raw = item->last ? item->last : item->spec;
> +		mask.raw = item->mask ? item->mask : &rte_flow_item_raw_mask;
> +		src.raw = data;
>  		dst.raw = buf;
>  		off = RTE_ALIGN_CEIL(sizeof(struct rte_flow_item_raw),
>  				     sizeof(*src.raw->pattern));
> -		size = off + src.raw->length * sizeof(*src.raw->pattern);
> +		if (type == ITEM_SPEC ||
> +		    (type == ITEM_MASK &&
> +		     ((spec.raw->length & mask.raw->length) >=
> +		      (last.raw->length & mask.raw->length))))
> +			size = spec.raw->length & mask.raw->length;
> +		else
> +			size = last.raw->length & mask.raw->length;
> +		size = off + size * sizeof(*src.raw->pattern);
>  		if (dst.raw) {
>  			memcpy(dst.raw, src.raw, sizeof(*src.raw));
>  			dst.raw->pattern = memcpy((uint8_t *)dst.raw + off,
> @@ -333,7 +352,7 @@ flow_item_spec_copy(void *buf, const struct rte_flow_item *item,
>  	default:
>  		size = rte_flow_desc_item[item->type].size;
>  		if (buf)
> -			memcpy(buf, item_spec, size);
> +			memcpy(buf, data, size);
>  		break;
>  	}
>  empty:
> 

Re: [dpdk-dev] [PATCH 2/2] ethdev: fix shallow copy of flow API RAW item

[Thomas Monjalon]

18/05/2018 19:06, Ferruh Yigit:
> On 5/16/2018 4:41 PM, Adrien Mazarguil wrote:
> > Like original commit mentioned below, this fix synchronizes flow rule copy
> > function with testpmd's own implementation following "app/testpmd: fix copy
> > of raw flow item (revisited)".
> > 
> > Fixes: d0ad8648b1c5 ("ethdev: fix shallow copy of flow API RSS action")
> > Cc: stable@dpdk.org
> > Cc: Qi Zhang <qi.z.zhang@intel.com>
> > 
> > Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> 
> Hi Thomas,
> 
> What do you suggest about this one?
> Scope is limited to rte_flow but still many features are now relies on rte_flow,
> what is your comment on getting this in rc5?

We need to know exactly what is broken.
If nothing serious, it can wait 18.08.

Adrien, please can you describe the use case, the issue and the impact?

Re: [dpdk-dev] [PATCH 2/2] ethdev: fix shallow copy of flow API RAW item

[Adrien Mazarguil]

On Sat, May 19, 2018 at 04:25:15PM +0200, Thomas Monjalon wrote:
> 18/05/2018 19:06, Ferruh Yigit:
> > On 5/16/2018 4:41 PM, Adrien Mazarguil wrote:
> > > Like original commit mentioned below, this fix synchronizes flow rule copy
> > > function with testpmd's own implementation following "app/testpmd: fix copy
> > > of raw flow item (revisited)".
> > > 
> > > Fixes: d0ad8648b1c5 ("ethdev: fix shallow copy of flow API RSS action")
> > > Cc: stable@dpdk.org
> > > Cc: Qi Zhang <qi.z.zhang@intel.com>
> > > 
> > > Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > 
> > Hi Thomas,
> > 
> > What do you suggest about this one?
> > Scope is limited to rte_flow but still many features are now relies on rte_flow,
> > what is your comment on getting this in rc5?
> 
> We need to know exactly what is broken.
> If nothing serious, it can wait 18.08.
> 
> Adrien, please can you describe the use case, the issue and the impact?

A prior patch [1] (applied as "app/testpmd: fix copy of raw flow item"),
addresses a crash in testpmd's flow copy function.

The first patch of the present series [2] addresses remaining issues with
its behavior which is, in fact, what caused the original issue.

While both patches focus on testpmd, rte_flow also exposes its own public
copy function with the exact same code that breaks when encountering a RAW
pattern item. Primary users for this function are bonding and failsafe
PMDs. 

This patch therefore addresses both [1] and [2] at once for rte_flow_copy().

[1] "app/testpmd: fix invalid memory access"
    http://dpdk.org/ml/archives/dev/2018-May/100364.html

[2] "app/testpmd: fix copy of raw flow item (revisited)"
    http://dpdk.org/ml/archives/dev/2018-May/101994.html

-- 
Adrien Mazarguil
6WIND

Re: [dpdk-dev] [PATCH 2/2] ethdev: fix shallow copy of flow API RAW item

[Ferruh Yigit]

On 5/21/2018 9:24 AM, Adrien Mazarguil wrote:
> On Sat, May 19, 2018 at 04:25:15PM +0200, Thomas Monjalon wrote:
>> 18/05/2018 19:06, Ferruh Yigit:
>>> On 5/16/2018 4:41 PM, Adrien Mazarguil wrote:
>>>> Like original commit mentioned below, this fix synchronizes flow rule copy
>>>> function with testpmd's own implementation following "app/testpmd: fix copy
>>>> of raw flow item (revisited)".
>>>>
>>>> Fixes: d0ad8648b1c5 ("ethdev: fix shallow copy of flow API RSS action")
>>>> Cc: stable@dpdk.org
>>>> Cc: Qi Zhang <qi.z.zhang@intel.com>
>>>>
>>>> Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
>>>
>>> Hi Thomas,
>>>
>>> What do you suggest about this one?
>>> Scope is limited to rte_flow but still many features are now relies on rte_flow,
>>> what is your comment on getting this in rc5?
>>
>> We need to know exactly what is broken.
>> If nothing serious, it can wait 18.08.
>>
>> Adrien, please can you describe the use case, the issue and the impact?
> 
> A prior patch [1] (applied as "app/testpmd: fix copy of raw flow item"),
> addresses a crash in testpmd's flow copy function.
> 
> The first patch of the present series [2] addresses remaining issues with
> its behavior which is, in fact, what caused the original issue.
> 
> While both patches focus on testpmd, rte_flow also exposes its own public
> copy function with the exact same code that breaks when encountering a RAW
> pattern item. Primary users for this function are bonding and failsafe
> PMDs. 
> 
> This patch therefore addresses both [1] and [2] at once for rte_flow_copy().

Hi Adrien,

What is the effect of _not_ getting this patch, just trying to understand if
this is something to get for this release or postpone to next one.

Thanks,
ferruh

> 
> [1] "app/testpmd: fix invalid memory access"
>     http://dpdk.org/ml/archives/dev/2018-May/100364.html
> 
> [2] "app/testpmd: fix copy of raw flow item (revisited)"
>     http://dpdk.org/ml/archives/dev/2018-May/101994.html
> 

Re: [dpdk-dev] [PATCH 2/2] ethdev: fix shallow copy of flow API RAW item

[Adrien Mazarguil]

On Mon, May 21, 2018 at 11:44:33AM +0100, Ferruh Yigit wrote:
> On 5/21/2018 9:24 AM, Adrien Mazarguil wrote:
> > On Sat, May 19, 2018 at 04:25:15PM +0200, Thomas Monjalon wrote:
> >> 18/05/2018 19:06, Ferruh Yigit:
> >>> On 5/16/2018 4:41 PM, Adrien Mazarguil wrote:
> >>>> Like original commit mentioned below, this fix synchronizes flow rule copy
> >>>> function with testpmd's own implementation following "app/testpmd: fix copy
> >>>> of raw flow item (revisited)".
> >>>>
> >>>> Fixes: d0ad8648b1c5 ("ethdev: fix shallow copy of flow API RSS action")
> >>>> Cc: stable@dpdk.org
> >>>> Cc: Qi Zhang <qi.z.zhang@intel.com>
> >>>>
> >>>> Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> >>>
> >>> Hi Thomas,
> >>>
> >>> What do you suggest about this one?
> >>> Scope is limited to rte_flow but still many features are now relies on rte_flow,
> >>> what is your comment on getting this in rc5?
> >>
> >> We need to know exactly what is broken.
> >> If nothing serious, it can wait 18.08.
> >>
> >> Adrien, please can you describe the use case, the issue and the impact?
> > 
> > A prior patch [1] (applied as "app/testpmd: fix copy of raw flow item"),
> > addresses a crash in testpmd's flow copy function.
> > 
> > The first patch of the present series [2] addresses remaining issues with
> > its behavior which is, in fact, what caused the original issue.
> > 
> > While both patches focus on testpmd, rte_flow also exposes its own public
> > copy function with the exact same code that breaks when encountering a RAW
> > pattern item. Primary users for this function are bonding and failsafe
> > PMDs. 
> > 
> > This patch therefore addresses both [1] and [2] at once for rte_flow_copy().
> 
> Hi Adrien,
> 
> What is the effect of _not_ getting this patch, just trying to understand if
> this is something to get for this release or postpone to next one.

Hi Ferruh,

*Not* getting this patch means rte_flow_copy() crashes when user creates a
flow rule that involves the RAW pattern item on top of either bonding or
failsafe PMDs.

Ditto for any external application that relies on rte_flow_copy() combined
with the RAW pattern item.

I'll send v2 to provide a bit more info and fix the wrong commit ID on the
"Fixes" line.

> > [1] "app/testpmd: fix invalid memory access"
> >     http://dpdk.org/ml/archives/dev/2018-May/100364.html
> > 
> > [2] "app/testpmd: fix copy of raw flow item (revisited)"
> >     http://dpdk.org/ml/archives/dev/2018-May/101994.html

-- 
Adrien Mazarguil
6WIND

Re: [dpdk-dev] [PATCH 1/2] app/testpmd: fix copy of raw flow item (revisited)

[Iremonger, Bernard]

Hi Adrien,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Adrien Mazarguil
> Sent: Wednesday, May 16, 2018 4:42 PM
> To: Yigit, Ferruh <ferruh.yigit@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>
> Subject: [dpdk-dev] [PATCH 1/2] app/testpmd: fix copy of raw flow item
> (revisited)
> 
> While previous fix with the same title does address the main issue, root cause is
> that proper handling of spec/last/mask was overlooked in the original patch.
> 
> Mask and last fields must be taken into account at all times.
> 
> Fixes: d0ad8648b1c5 ("app/testpmd: fix RSS flow action configuration")
> Fixes: 67af7ecc52ec ("app/testpmd: fix copy of raw flow item")
> Cc: stable@dpdk.org
> Cc: Qi Zhang <qi.z.zhang@intel.com>
> 
> Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>

Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>

[dpdk-dev] [PATCH v2 1/2] app/testpmd: fix copy of raw flow item (revisited)

[Adrien Mazarguil]

While previous fix with the same title does address the main issue, root
cause is that proper handling of spec/last/mask was overlooked in the
original patch.

Mask and last fields must be taken into account at all times.

Fixes: d0ad8648b1c5 ("app/testpmd: fix RSS flow action configuration")
Fixes: 67af7ecc52ec ("app/testpmd: fix copy of raw flow item")
Cc: stable@dpdk.org
Cc: Qi Zhang <qi.z.zhang@intel.com>

Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
---
 app/test-pmd/config.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index b07376af9..4520084b8 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1049,17 +1049,26 @@ flow_item_spec_copy(void *buf, const struct rte_flow_item *item,
 		    enum item_spec_type type)
 {
 	size_t size = 0;
-	const void *item_spec =
+	const void *data =
 		type == ITEM_SPEC ? item->spec :
 		type == ITEM_LAST ? item->last :
 		type == ITEM_MASK ? item->mask :
 		NULL;
 
-	if (!item_spec)
+	if (!item->spec || !data)
 		goto empty;
 	switch (item->type) {
 		union {
 			const struct rte_flow_item_raw *raw;
+		} spec;
+		union {
+			const struct rte_flow_item_raw *raw;
+		} last;
+		union {
+			const struct rte_flow_item_raw *raw;
+		} mask;
+		union {
+			const struct rte_flow_item_raw *raw;
 		} src;
 		union {
 			struct rte_flow_item_raw *raw;
@@ -1067,12 +1076,21 @@ flow_item_spec_copy(void *buf, const struct rte_flow_item *item,
 		size_t off;
 
 	case RTE_FLOW_ITEM_TYPE_RAW:
-		src.raw = item_spec;
+		spec.raw = item->spec;
+		last.raw = item->last ? item->last : item->spec;
+		mask.raw = item->mask ? item->mask : &rte_flow_item_raw_mask;
+		src.raw = data;
 		dst.raw = buf;
 		off = RTE_ALIGN_CEIL(sizeof(struct rte_flow_item_raw),
 				     sizeof(*src.raw->pattern));
-		size = off + ((const struct rte_flow_item_raw *)item->spec)->
-			length * sizeof(*src.raw->pattern);
+		if (type == ITEM_SPEC ||
+		    (type == ITEM_MASK &&
+		     ((spec.raw->length & mask.raw->length) >=
+		      (last.raw->length & mask.raw->length))))
+			size = spec.raw->length & mask.raw->length;
+		else
+			size = last.raw->length & mask.raw->length;
+		size = off + size * sizeof(*src.raw->pattern);
 		if (dst.raw) {
 			memcpy(dst.raw, src.raw, sizeof(*src.raw));
 			dst.raw->pattern = memcpy((uint8_t *)dst.raw + off,
@@ -1083,7 +1101,7 @@ flow_item_spec_copy(void *buf, const struct rte_flow_item *item,
 	default:
 		size = flow_item[item->type].size;
 		if (buf)
-			memcpy(buf, item_spec, size);
+			memcpy(buf, data, size);
 		break;
 	}
 empty:
-- 
2.11.0

[dpdk-dev] [PATCH v2 2/2] ethdev: fix shallow copy of flow API RAW item

[Adrien Mazarguil]

Like original commit mentioned below, this fix synchronizes flow rule copy
function with testpmd's own implementation following "app/testpmd: fix copy
of raw flow item (revisited)".

It addresses a crash that occurs when feeding a RAW pattern item to
rte_flow_copy(). Besides external applications, two PMDs (bonding and
failsafe) rely on this function internally.

Note the scope of this patch is limited to the RAW pattern item and has no
impact on all others.

Fixes: 972bf3610611 ("ethdev: fix shallow copy of flow API RSS action")
Cc: stable@dpdk.org
Cc: Qi Zhang <qi.z.zhang@intel.com>

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

---

v2 changes:

- Fixed commit ID on "Fixes" line.
- Described the crash addressed by this patch so people do not have to
  derive this information from the referenced commits.
---
 lib/librte_ethdev/rte_flow.c | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index 7947529da..b2afba089 100644
--- a/lib/librte_ethdev/rte_flow.c
+++ b/lib/librte_ethdev/rte_flow.c
@@ -300,17 +300,26 @@ flow_item_spec_copy(void *buf, const struct rte_flow_item *item,
 		    enum item_spec_type type)
 {
 	size_t size = 0;
-	const void *item_spec =
+	const void *data =
 		type == ITEM_SPEC ? item->spec :
 		type == ITEM_LAST ? item->last :
 		type == ITEM_MASK ? item->mask :
 		NULL;
 
-	if (!item_spec)
+	if (!item->spec || !data)
 		goto empty;
 	switch (item->type) {
 		union {
 			const struct rte_flow_item_raw *raw;
+		} spec;
+		union {
+			const struct rte_flow_item_raw *raw;
+		} last;
+		union {
+			const struct rte_flow_item_raw *raw;
+		} mask;
+		union {
+			const struct rte_flow_item_raw *raw;
 		} src;
 		union {
 			struct rte_flow_item_raw *raw;
@@ -318,11 +327,21 @@ flow_item_spec_copy(void *buf, const struct rte_flow_item *item,
 		size_t off;
 
 	case RTE_FLOW_ITEM_TYPE_RAW:
-		src.raw = item_spec;
+		spec.raw = item->spec;
+		last.raw = item->last ? item->last : item->spec;
+		mask.raw = item->mask ? item->mask : &rte_flow_item_raw_mask;
+		src.raw = data;
 		dst.raw = buf;
 		off = RTE_ALIGN_CEIL(sizeof(struct rte_flow_item_raw),
 				     sizeof(*src.raw->pattern));
-		size = off + src.raw->length * sizeof(*src.raw->pattern);
+		if (type == ITEM_SPEC ||
+		    (type == ITEM_MASK &&
+		     ((spec.raw->length & mask.raw->length) >=
+		      (last.raw->length & mask.raw->length))))
+			size = spec.raw->length & mask.raw->length;
+		else
+			size = last.raw->length & mask.raw->length;
+		size = off + size * sizeof(*src.raw->pattern);
 		if (dst.raw) {
 			memcpy(dst.raw, src.raw, sizeof(*src.raw));
 			dst.raw->pattern = memcpy((uint8_t *)dst.raw + off,
@@ -333,7 +352,7 @@ flow_item_spec_copy(void *buf, const struct rte_flow_item *item,
 	default:
 		size = rte_flow_desc_item[item->type].size;
 		if (buf)
-			memcpy(buf, item_spec, size);
+			memcpy(buf, data, size);
 		break;
 	}
 empty:
-- 
2.11.0

Re: [dpdk-dev] [PATCH v2 1/2] app/testpmd: fix copy of raw flow item (revisited)

[Ferruh Yigit]

On 5/21/2018 12:44 PM, Adrien Mazarguil wrote:
> While previous fix with the same title does address the main issue, root
> cause is that proper handling of spec/last/mask was overlooked in the
> original patch.
> 
> Mask and last fields must be taken into account at all times.
> 
> Fixes: d0ad8648b1c5 ("app/testpmd: fix RSS flow action configuration")
> Fixes: 67af7ecc52ec ("app/testpmd: fix copy of raw flow item")
> Cc: stable@dpdk.org
> Cc: Qi Zhang <qi.z.zhang@intel.com>
> 
> Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>

Series applied to dpdk-next-net/master, thanks.