* [dpdk-dev] [PATCH] app/testpmd: fix RSS key
@ 2021-01-18 8:59 Zhang,Alvin
2021-01-19 19:14 ` Ferruh Yigit
2021-01-21 9:41 ` [dpdk-dev] [PATCH v2] " Zhang,Alvin
0 siblings, 2 replies; 6+ messages in thread
From: Zhang,Alvin @ 2021-01-18 8:59 UTC (permalink / raw)
To: ferruh.yigit; +Cc: dev, Alvin Zhang, stable
From: Alvin Zhang <alvinx.zhang@intel.com>
Since the patch '1848b117' has set the value of key in 'struct
rte_flow_action_rss' to NULL, the PMD cannot get the RSS key now.
This patch sets offset and size of the key pointer as the first
parameter of the token 'key' and copies the start address of the
'HEX' data to the location specified by the first parameter of
the token.
Fixes: 1848b117cca1 ("app/testpmd: fix RSS key for flow API RSS rule")
Cc: stable@dpdk.org
Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
---
app/test-pmd/cmdline_flow.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 585cab9..6eb46d3 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -3403,7 +3403,10 @@ static int comp_set_sample_index(struct context *, const struct token *,
.name = "key",
.help = "RSS hash key",
.next = NEXT(action_rss, NEXT_ENTRY(HEX)),
- .args = ARGS(ARGS_ENTRY_ARB(0, 0),
+ .args = ARGS(ARGS_ENTRY_ARB
+ (offsetof(struct action_rss_data, conf) +
+ offsetof(struct rte_flow_action_rss, key),
+ sizeof(((struct rte_flow_action_rss *)0)->key)),
ARGS_ENTRY_ARB
(offsetof(struct action_rss_data, conf) +
offsetof(struct rte_flow_action_rss, key_len),
@@ -6495,19 +6498,18 @@ static int comp_set_sample_index(struct context *, const struct token *,
if (ctx->objmask)
memset((uint8_t *)ctx->objmask + arg_data->offset,
0xff, hexlen);
+
/* Save address if requested. */
if (arg_addr->size) {
- memcpy((uint8_t *)ctx->object + arg_addr->offset,
- (void *[]){
- (uint8_t *)ctx->object + arg_data->offset
- },
- arg_addr->size);
+ if (arg_addr->size < sizeof(void *))
+ goto error;
+
+ *(void **)((uint8_t *)ctx->object + arg_addr->offset) =
+ (uint8_t *)ctx->object + arg_data->offset;
+
if (ctx->objmask)
- memcpy((uint8_t *)ctx->objmask + arg_addr->offset,
- (void *[]){
- (uint8_t *)ctx->objmask + arg_data->offset
- },
- arg_addr->size);
+ *(void **)((uint8_t *)ctx->objmask + arg_addr->offset) =
+ (uint8_t *)ctx->objmask + arg_data->offset;
}
return len;
error:
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [dpdk-dev] [PATCH] app/testpmd: fix RSS key
2021-01-18 8:59 [dpdk-dev] [PATCH] app/testpmd: fix RSS key Zhang,Alvin
@ 2021-01-19 19:14 ` Ferruh Yigit
2021-01-21 9:35 ` Zhang, AlvinX
2021-01-21 9:41 ` [dpdk-dev] [PATCH v2] " Zhang,Alvin
1 sibling, 1 reply; 6+ messages in thread
From: Ferruh Yigit @ 2021-01-19 19:14 UTC (permalink / raw)
To: Zhang,Alvin; +Cc: dev, stable
On 1/18/2021 8:59 AM, Zhang,Alvin wrote:
> From: Alvin Zhang <alvinx.zhang@intel.com>
>
> Since the patch '1848b117' has set the value of key in 'struct
> rte_flow_action_rss' to NULL, the PMD cannot get the RSS key now.
>
> This patch sets offset and size of the key pointer as the first
> parameter of the token 'key' and copies the start address of the
> 'HEX' data to the location specified by the first parameter of
> the token.
>
Can you please put the rte_flow command that enables reproducing this defect, it
may help in the future?
> Fixes: 1848b117cca1 ("app/testpmd: fix RSS key for flow API RSS rule")
> Cc: stable@dpdk.org
>
> Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
> ---
> app/test-pmd/cmdline_flow.c | 24 +++++++++++++-----------
> 1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index 585cab9..6eb46d3 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -3403,7 +3403,10 @@ static int comp_set_sample_index(struct context *, const struct token *,
> .name = "key",
> .help = "RSS hash key",
> .next = NEXT(action_rss, NEXT_ENTRY(HEX)),
> - .args = ARGS(ARGS_ENTRY_ARB(0, 0),
> + .args = ARGS(ARGS_ENTRY_ARB
> + (offsetof(struct action_rss_data, conf) +
> + offsetof(struct rte_flow_action_rss, key),
> + sizeof(((struct rte_flow_action_rss *)0)->key)),
+1, it is required to write the address, and I confirm this enables getting
'key' value to the PMD.
> ARGS_ENTRY_ARB
> (offsetof(struct action_rss_data, conf) +
> offsetof(struct rte_flow_action_rss, key_len),
> @@ -6495,19 +6498,18 @@ static int comp_set_sample_index(struct context *, const struct token *,
> if (ctx->objmask)
> memset((uint8_t *)ctx->objmask + arg_data->offset,
> 0xff, hexlen);
> +
> /* Save address if requested. */
> if (arg_addr->size) {
> - memcpy((uint8_t *)ctx->object + arg_addr->offset,
> - (void *[]){
> - (uint8_t *)ctx->object + arg_data->offset
> - },
> - arg_addr->size);
> + if (arg_addr->size < sizeof(void *))
> + goto error;
Above two lines seems only actual change in this function, others are
refactoring the assignment.
1) why this check required, I think we can ignore it.
2) What do you think to remove the refactoring to reduce the change to the
actual fix?
> +
> + *(void **)((uint8_t *)ctx->object + arg_addr->offset) =
> + (uint8_t *)ctx->object + arg_data->offset;
> +
> if (ctx->objmask)
> - memcpy((uint8_t *)ctx->objmask + arg_addr->offset,
> - (void *[]){
> - (uint8_t *)ctx->objmask + arg_data->offset
> - },
> - arg_addr->size);
> + *(void **)((uint8_t *)ctx->objmask + arg_addr->offset) =
> + (uint8_t *)ctx->objmask + arg_data->offset;
> }
> return len;
> error:
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [dpdk-dev] [PATCH] app/testpmd: fix RSS key
2021-01-19 19:14 ` Ferruh Yigit
@ 2021-01-21 9:35 ` Zhang, AlvinX
0 siblings, 0 replies; 6+ messages in thread
From: Zhang, AlvinX @ 2021-01-21 9:35 UTC (permalink / raw)
To: Yigit, Ferruh; +Cc: dev, stable
Hi Ferruh,
Thanks for your review.
I will modify the patch in V2.
Alvin
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Wednesday, January 20, 2021 3:15 AM
> To: Zhang, AlvinX <alvinx.zhang@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: Re: [PATCH] app/testpmd: fix RSS key
>
> On 1/18/2021 8:59 AM, Zhang,Alvin wrote:
> > From: Alvin Zhang <alvinx.zhang@intel.com>
> >
> > Since the patch '1848b117' has set the value of key in 'struct
> > rte_flow_action_rss' to NULL, the PMD cannot get the RSS key now.
> >
> > This patch sets offset and size of the key pointer as the first
> > parameter of the token 'key' and copies the start address of the 'HEX'
> > data to the location specified by the first parameter of the token.
> >
>
> Can you please put the rte_flow command that enables reproducing this defect,
> it may help in the future?
>
> > Fixes: 1848b117cca1 ("app/testpmd: fix RSS key for flow API RSS rule")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
> > ---
> > app/test-pmd/cmdline_flow.c | 24 +++++++++++++-----------
> > 1 file changed, 13 insertions(+), 11 deletions(-)
> >
> > diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> > index 585cab9..6eb46d3 100644
> > --- a/app/test-pmd/cmdline_flow.c
> > +++ b/app/test-pmd/cmdline_flow.c
> > @@ -3403,7 +3403,10 @@ static int comp_set_sample_index(struct context
> *, const struct token *,
> > .name = "key",
> > .help = "RSS hash key",
> > .next = NEXT(action_rss, NEXT_ENTRY(HEX)),
> > - .args = ARGS(ARGS_ENTRY_ARB(0, 0),
> > + .args = ARGS(ARGS_ENTRY_ARB
> > + (offsetof(struct action_rss_data, conf) +
> > + offsetof(struct rte_flow_action_rss, key),
> > + sizeof(((struct rte_flow_action_rss *)0)->key)),
>
>
> +1, it is required to write the address, and I confirm this enables
> +getting
> 'key' value to the PMD.
>
> > ARGS_ENTRY_ARB
> > (offsetof(struct action_rss_data, conf) +
> > offsetof(struct rte_flow_action_rss, key_len), @@
> -6495,19
> > +6498,18 @@ static int comp_set_sample_index(struct context *, const
> struct token *,
> > if (ctx->objmask)
> > memset((uint8_t *)ctx->objmask + arg_data->offset,
> > 0xff, hexlen);
> > +
> > /* Save address if requested. */
> > if (arg_addr->size) {
> > - memcpy((uint8_t *)ctx->object + arg_addr->offset,
> > - (void *[]){
> > - (uint8_t *)ctx->object + arg_data->offset
> > - },
> > - arg_addr->size);
> > + if (arg_addr->size < sizeof(void *))
> > + goto error;
>
> Above two lines seems only actual change in this function, others are refactoring
> the assignment.
>
> 1) why this check required, I think we can ignore it.
> 2) What do you think to remove the refactoring to reduce the change to the
> actual fix?
>
> > +
> > + *(void **)((uint8_t *)ctx->object + arg_addr->offset) =
> > + (uint8_t *)ctx->object + arg_data->offset;
> > +
> > if (ctx->objmask)
> > - memcpy((uint8_t *)ctx->objmask + arg_addr->offset,
> > - (void *[]){
> > - (uint8_t *)ctx->objmask + arg_data->offset
> > - },
> > - arg_addr->size);
> > + *(void **)((uint8_t *)ctx->objmask + arg_addr->offset) =
> > + (uint8_t *)ctx->objmask + arg_data->offset;
> > }
> > return len;
> > error:
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* [dpdk-dev] [PATCH v2] app/testpmd: fix RSS key
2021-01-18 8:59 [dpdk-dev] [PATCH] app/testpmd: fix RSS key Zhang,Alvin
2021-01-19 19:14 ` Ferruh Yigit
@ 2021-01-21 9:41 ` Zhang,Alvin
2021-01-22 5:43 ` Zhou, JunX W
1 sibling, 1 reply; 6+ messages in thread
From: Zhang,Alvin @ 2021-01-21 9:41 UTC (permalink / raw)
To: ferruh.yigit; +Cc: dev, Alvin Zhang, stable
From: Alvin Zhang <alvinx.zhang@intel.com>
Since the patch '1848b117' has initialized the variable 'key' in
'struct rte_flow_action_rss' with 'NULL', the PMD cannot get the
RSS key now. Details as bellow:
testpmd> flow create 0 ingress pattern eth / ipv4 / end actions
rss types ipv4-other end key
1234567890123456789012345678901234567890FFFFFFFFFFFF123
4567890123456789012345678901234567890FFFFFFFFFFFF
queues end / end
Flow rule #1 created
testpmd> show port 0 rss-hash key
RSS functions:
all ipv4-other ip
RSS key:
4439796BB54C5023B675EA5B124F9F30B8A2C03DDFDC4D02A08C9B3
34AF64A4C05C6FA343958D8557D99583AE138C92E81150366
This patch sets offset and size of the 'key' variable as the first
parameter of the token 'key'. Later, the address of the RSS key will
be copied to 'key' variable.
Fixes: 1848b117cca1 ("app/testpmd: fix RSS key for flow API RSS rule")
Cc: stable@dpdk.org
Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
---
app/test-pmd/cmdline_flow.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 0618611..067e120 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -3541,7 +3541,10 @@ static int comp_set_modify_field_id(struct context *, const struct token *,
.name = "key",
.help = "RSS hash key",
.next = NEXT(action_rss, NEXT_ENTRY(HEX)),
- .args = ARGS(ARGS_ENTRY_ARB(0, 0),
+ .args = ARGS(ARGS_ENTRY_ARB
+ (offsetof(struct action_rss_data, conf) +
+ offsetof(struct rte_flow_action_rss, key),
+ sizeof(((struct rte_flow_action_rss *)0)->key)),
ARGS_ENTRY_ARB
(offsetof(struct action_rss_data, conf) +
offsetof(struct rte_flow_action_rss, key_len),
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [dpdk-dev] [PATCH v2] app/testpmd: fix RSS key
2021-01-21 9:41 ` [dpdk-dev] [PATCH v2] " Zhang,Alvin
@ 2021-01-22 5:43 ` Zhou, JunX W
2021-01-22 14:58 ` Ferruh Yigit
0 siblings, 1 reply; 6+ messages in thread
From: Zhou, JunX W @ 2021-01-22 5:43 UTC (permalink / raw)
To: Zhang, AlvinX, Yigit, Ferruh; +Cc: dev, Zhang, AlvinX, stable
Tested-by: Zhou, Jun <junx.w.zhou@intel.com>
-----Original Message-----
From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhang,Alvin
Sent: Thursday, January 21, 2021 5:42 PM
To: Yigit, Ferruh <ferruh.yigit@intel.com>
Cc: dev@dpdk.org; Zhang, AlvinX <alvinx.zhang@intel.com>; stable@dpdk.org
Subject: [dpdk-dev] [PATCH v2] app/testpmd: fix RSS key
From: Alvin Zhang <alvinx.zhang@intel.com>
Since the patch '1848b117' has initialized the variable 'key' in 'struct rte_flow_action_rss' with 'NULL', the PMD cannot get the RSS key now. Details as bellow:
testpmd> flow create 0 ingress pattern eth / ipv4 / end actions
rss types ipv4-other end key
1234567890123456789012345678901234567890FFFFFFFFFFFF123
4567890123456789012345678901234567890FFFFFFFFFFFF
queues end / end
Flow rule #1 created
testpmd> show port 0 rss-hash key
RSS functions:
all ipv4-other ip
RSS key:
4439796BB54C5023B675EA5B124F9F30B8A2C03DDFDC4D02A08C9B3
34AF64A4C05C6FA343958D8557D99583AE138C92E81150366
This patch sets offset and size of the 'key' variable as the first parameter of the token 'key'. Later, the address of the RSS key will be copied to 'key' variable.
Fixes: 1848b117cca1 ("app/testpmd: fix RSS key for flow API RSS rule")
Cc: stable@dpdk.org
Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
---
app/test-pmd/cmdline_flow.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c index 0618611..067e120 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -3541,7 +3541,10 @@ static int comp_set_modify_field_id(struct context *, const struct token *,
.name = "key",
.help = "RSS hash key",
.next = NEXT(action_rss, NEXT_ENTRY(HEX)),
- .args = ARGS(ARGS_ENTRY_ARB(0, 0),
+ .args = ARGS(ARGS_ENTRY_ARB
+ (offsetof(struct action_rss_data, conf) +
+ offsetof(struct rte_flow_action_rss, key),
+ sizeof(((struct rte_flow_action_rss *)0)->key)),
ARGS_ENTRY_ARB
(offsetof(struct action_rss_data, conf) +
offsetof(struct rte_flow_action_rss, key_len),
--
1.8.3.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [dpdk-dev] [PATCH v2] app/testpmd: fix RSS key
2021-01-22 5:43 ` Zhou, JunX W
@ 2021-01-22 14:58 ` Ferruh Yigit
0 siblings, 0 replies; 6+ messages in thread
From: Ferruh Yigit @ 2021-01-22 14:58 UTC (permalink / raw)
To: Zhou, JunX W, Zhang, AlvinX; +Cc: dev, stable
On 1/22/2021 5:43 AM, Zhou, JunX W wrote:
<...>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhang,Alvin
> Sent: Thursday, January 21, 2021 5:42 PM
> To: Yigit, Ferruh <ferruh.yigit@intel.com>
> Cc: dev@dpdk.org; Zhang, AlvinX <alvinx.zhang@intel.com>; stable@dpdk.org
> Subject: [dpdk-dev] [PATCH v2] app/testpmd: fix RSS key
>
> From: Alvin Zhang <alvinx.zhang@intel.com>
>
> Since the patch '1848b117' has initialized the variable 'key' in 'struct rte_flow_action_rss' with 'NULL', the PMD cannot get the RSS key now. Details as bellow:
>
> testpmd> flow create 0 ingress pattern eth / ipv4 / end actions
> rss types ipv4-other end key
> 1234567890123456789012345678901234567890FFFFFFFFFFFF123
> 4567890123456789012345678901234567890FFFFFFFFFFFF
> queues end / end
> Flow rule #1 created
> testpmd> show port 0 rss-hash key
> RSS functions:
> all ipv4-other ip
> RSS key:
> 4439796BB54C5023B675EA5B124F9F30B8A2C03DDFDC4D02A08C9B3
> 34AF64A4C05C6FA343958D8557D99583AE138C92E81150366
>
> This patch sets offset and size of the 'key' variable as the first parameter of the token 'key'. Later, the address of the RSS key will be copied to 'key' variable.
>
> Fixes: 1848b117cca1 ("app/testpmd: fix RSS key for flow API RSS rule")
> Cc: stable@dpdk.org
>
> Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
>
> Tested-by: Zhou, Jun <junx.w.zhou@intel.com>
>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
Applied to dpdk-next-net/main, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-01-22 14:58 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-18 8:59 [dpdk-dev] [PATCH] app/testpmd: fix RSS key Zhang,Alvin
2021-01-19 19:14 ` Ferruh Yigit
2021-01-21 9:35 ` Zhang, AlvinX
2021-01-21 9:41 ` [dpdk-dev] [PATCH v2] " Zhang,Alvin
2021-01-22 5:43 ` Zhou, JunX W
2021-01-22 14:58 ` Ferruh Yigit
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.