* [dpdk-stable] [PATCH 1/2] app/testpmd: fix copy of raw flow item (revisited)
@ 2018-05-16 15:41 Adrien Mazarguil
2018-05-16 15:41 ` [dpdk-stable] [PATCH 2/2] ethdev: fix shallow copy of flow API RAW item Adrien Mazarguil
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Adrien Mazarguil @ 2018-05-16 15:41 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: dev, stable, Qi Zhang
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [dpdk-stable] [PATCH 2/2] ethdev: fix shallow copy of flow API RAW item
2018-05-16 15:41 [dpdk-stable] [PATCH 1/2] app/testpmd: fix copy of raw flow item (revisited) Adrien Mazarguil
@ 2018-05-16 15:41 ` Adrien Mazarguil
2018-05-18 17:06 ` Ferruh Yigit
2018-05-17 14:44 ` [dpdk-stable] [dpdk-dev] [PATCH 1/2] app/testpmd: fix copy of raw flow item (revisited) Iremonger, Bernard
2018-05-21 11:44 ` [dpdk-stable] [PATCH v2 " Adrien Mazarguil
2 siblings, 1 reply; 11+ messages in thread
From: Adrien Mazarguil @ 2018-05-16 15:41 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: dev, stable, Qi Zhang
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH 1/2] app/testpmd: fix copy of raw flow item (revisited)
2018-05-16 15:41 [dpdk-stable] [PATCH 1/2] app/testpmd: fix copy of raw flow item (revisited) Adrien Mazarguil
2018-05-16 15:41 ` [dpdk-stable] [PATCH 2/2] ethdev: fix shallow copy of flow API RAW item Adrien Mazarguil
@ 2018-05-17 14:44 ` Iremonger, Bernard
2018-05-21 11:44 ` [dpdk-stable] [PATCH v2 " Adrien Mazarguil
2 siblings, 0 replies; 11+ messages in thread
From: Iremonger, Bernard @ 2018-05-17 14:44 UTC (permalink / raw)
To: Adrien Mazarguil, Yigit, Ferruh; +Cc: dev, stable, Zhang, Qi Z
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>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [dpdk-stable] [PATCH 2/2] ethdev: fix shallow copy of flow API RAW item
2018-05-16 15:41 ` [dpdk-stable] [PATCH 2/2] ethdev: fix shallow copy of flow API RAW item Adrien Mazarguil
@ 2018-05-18 17:06 ` Ferruh Yigit
2018-05-19 14:25 ` Thomas Monjalon
0 siblings, 1 reply; 11+ messages in thread
From: Ferruh Yigit @ 2018-05-18 17:06 UTC (permalink / raw)
To: Adrien Mazarguil, Thomas Monjalon; +Cc: dev, stable, Qi Zhang
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:
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [dpdk-stable] [PATCH 2/2] ethdev: fix shallow copy of flow API RAW item
2018-05-18 17:06 ` Ferruh Yigit
@ 2018-05-19 14:25 ` Thomas Monjalon
2018-05-21 8:24 ` Adrien Mazarguil
0 siblings, 1 reply; 11+ messages in thread
From: Thomas Monjalon @ 2018-05-19 14:25 UTC (permalink / raw)
To: Adrien Mazarguil; +Cc: Ferruh Yigit, dev, stable, Qi Zhang
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?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [dpdk-stable] [PATCH 2/2] ethdev: fix shallow copy of flow API RAW item
2018-05-19 14:25 ` Thomas Monjalon
@ 2018-05-21 8:24 ` Adrien Mazarguil
2018-05-21 10:44 ` [dpdk-stable] [dpdk-dev] " Ferruh Yigit
0 siblings, 1 reply; 11+ messages in thread
From: Adrien Mazarguil @ 2018-05-21 8:24 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: Ferruh Yigit, dev, stable, Qi Zhang
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH 2/2] ethdev: fix shallow copy of flow API RAW item
2018-05-21 8:24 ` Adrien Mazarguil
@ 2018-05-21 10:44 ` Ferruh Yigit
2018-05-21 11:18 ` Adrien Mazarguil
0 siblings, 1 reply; 11+ messages in thread
From: Ferruh Yigit @ 2018-05-21 10:44 UTC (permalink / raw)
To: Adrien Mazarguil, Thomas Monjalon; +Cc: dev, stable, Qi Zhang
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
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH 2/2] ethdev: fix shallow copy of flow API RAW item
2018-05-21 10:44 ` [dpdk-stable] [dpdk-dev] " Ferruh Yigit
@ 2018-05-21 11:18 ` Adrien Mazarguil
0 siblings, 0 replies; 11+ messages in thread
From: Adrien Mazarguil @ 2018-05-21 11:18 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: Thomas Monjalon, dev, stable, Qi Zhang
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [dpdk-stable] [PATCH v2 1/2] app/testpmd: fix copy of raw flow item (revisited)
2018-05-16 15:41 [dpdk-stable] [PATCH 1/2] app/testpmd: fix copy of raw flow item (revisited) Adrien Mazarguil
2018-05-16 15:41 ` [dpdk-stable] [PATCH 2/2] ethdev: fix shallow copy of flow API RAW item Adrien Mazarguil
2018-05-17 14:44 ` [dpdk-stable] [dpdk-dev] [PATCH 1/2] app/testpmd: fix copy of raw flow item (revisited) Iremonger, Bernard
@ 2018-05-21 11:44 ` Adrien Mazarguil
2018-05-21 11:44 ` [dpdk-stable] [PATCH v2 2/2] ethdev: fix shallow copy of flow API RAW item Adrien Mazarguil
2018-05-21 12:44 ` [dpdk-stable] [PATCH v2 1/2] app/testpmd: fix copy of raw flow item (revisited) Ferruh Yigit
2 siblings, 2 replies; 11+ messages in thread
From: Adrien Mazarguil @ 2018-05-21 11:44 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: dev, stable, Qi Zhang
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [dpdk-stable] [PATCH v2 2/2] ethdev: fix shallow copy of flow API RAW item
2018-05-21 11:44 ` [dpdk-stable] [PATCH v2 " Adrien Mazarguil
@ 2018-05-21 11:44 ` Adrien Mazarguil
2018-05-21 12:44 ` [dpdk-stable] [PATCH v2 1/2] app/testpmd: fix copy of raw flow item (revisited) Ferruh Yigit
1 sibling, 0 replies; 11+ messages in thread
From: Adrien Mazarguil @ 2018-05-21 11:44 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: dev, stable, Qi Zhang
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [dpdk-stable] [PATCH v2 1/2] app/testpmd: fix copy of raw flow item (revisited)
2018-05-21 11:44 ` [dpdk-stable] [PATCH v2 " Adrien Mazarguil
2018-05-21 11:44 ` [dpdk-stable] [PATCH v2 2/2] ethdev: fix shallow copy of flow API RAW item Adrien Mazarguil
@ 2018-05-21 12:44 ` Ferruh Yigit
1 sibling, 0 replies; 11+ messages in thread
From: Ferruh Yigit @ 2018-05-21 12:44 UTC (permalink / raw)
To: Adrien Mazarguil; +Cc: dev, stable, Qi Zhang
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.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-05-21 12:44 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-16 15:41 [dpdk-stable] [PATCH 1/2] app/testpmd: fix copy of raw flow item (revisited) Adrien Mazarguil
2018-05-16 15:41 ` [dpdk-stable] [PATCH 2/2] ethdev: fix shallow copy of flow API RAW item Adrien Mazarguil
2018-05-18 17:06 ` Ferruh Yigit
2018-05-19 14:25 ` Thomas Monjalon
2018-05-21 8:24 ` Adrien Mazarguil
2018-05-21 10:44 ` [dpdk-stable] [dpdk-dev] " Ferruh Yigit
2018-05-21 11:18 ` Adrien Mazarguil
2018-05-17 14:44 ` [dpdk-stable] [dpdk-dev] [PATCH 1/2] app/testpmd: fix copy of raw flow item (revisited) Iremonger, Bernard
2018-05-21 11:44 ` [dpdk-stable] [PATCH v2 " Adrien Mazarguil
2018-05-21 11:44 ` [dpdk-stable] [PATCH v2 2/2] ethdev: fix shallow copy of flow API RAW item Adrien Mazarguil
2018-05-21 12:44 ` [dpdk-stable] [PATCH v2 1/2] app/testpmd: fix copy of raw flow item (revisited) Ferruh Yigit
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).