[v3] app/testpmd: fix the default RSS key configuration
Checks
Commit Message
When a user runs a flow create cmd to configure an RSS rule
with specifying the empty rss actions in testpmd, this mean
that the flow gets the default RSS functions from the valid
NIC default RSS hash key. However, the testpmd is not set
the default RSS key incorrectly when RSS key is not specified
in flow create cmd. the cmdline as flows:
1. first, startup testpmd:
testpmd> show port 0 rss-hash key
RSS functions:
all ipv4-frag ipv4-other ipv6-frag ipv6-other ip
RSS key:
6D5A56DA255B0EC24167253D43A38FB0D0CA2BCBAE7B30B477CB2DA38030F
20C6A42B73BBEAC01FA
2. create a rss rule
testpmd> flow create 0 ingress pattern eth / ipv4 / udp / end actions rss \
types ipv4-udp end queues end / end
3. show rss-hash key
testpmd> show port 0 rss-hash key
RSS functions:
all ipv4-udp udp
RSS key:
74657374706D6427732064656661756C74205253532068617368206B65792C206F
76657272696465
Now, it uses rte_eth_dev_rss_hash_conf_get to correctly the
default rss key. the cmdline and result as flows:
testpmd> show port 0 rss-hash key
RSS functions:
all ipv4-frag ipv4-other ipv6-frag ipv6-other ip
RSS key:
6D5A56DA255B0EC24167253D43A38FB0D0CA2BCBAE7B30B477CB2DA38030F20C
6A42B73BBEAC01FA
testpmd> flow create 0 ingress pattern eth / ipv4 / udp / end actions rss \
types ipv4-udp end queues end / end
testpmd> show port 0 rss-hash key
RSS functions:
all ipv4-udp udp
RSS key:
6D5A56DA255B0EC24167253D43A38FB0D0CA2BCBAE7B30B477CB2DA38030F
20C6A42B73BBEAC01FA
Fixes: ac8d22de2394 ("ethdev: flatten RSS configuration in flow API")
Cc: stable@dpdk.org
Signed-off-by: Lijun Ou <oulijun@huawei.com>
---
V2->V3:
-fix checkpatch warning.
V1->V2:
-fix the commit.
---
app/test-pmd/cmdline_flow.c | 8 ++++++++
1 file changed, 8 insertions(+)
Comments
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Lijun Ou
> Sent: Thursday, September 10, 2020 9:51 AM
> To: wenzhuo.lu@intel.com; beilei.xing@intel.com;
> adrien.mazarguil@6wind.com; ferruh.yigit@intel.com
> Cc: dev@dpdk.org; linuxarm@huawei.com
> Subject: [dpdk-dev] [PATCH v3] app/testpmd: fix the default RSS key
> configuration
Hi Lijun,
Please fix the coding style issues.
"Must be a reply to the first patch (--in-reply-to)."
>
> When a user runs a flow create cmd to configure an RSS rule
> with specifying the empty rss actions in testpmd, this mean
^^^ means
> that the flow gets the default RSS functions from the valid
> NIC default RSS hash key. However, the testpmd is not set
^^^ is set xxx incorrectly
> the default RSS key incorrectly when RSS key is not specified
> in flow create cmd.
Use the NIC valid default RSS key instead of the testpmd dummy RSS key in the flow configuration when the RSS key is not specified in the flow rule. If the NIC RSS key is invalid, it will use testpmd dummy RSS key as the default key.
Is that good to put it in this way? Because I think it is not a bug, your patch offers an approach to update the default testpmd RSS key.
I would also suggest making the commit message shorter and move the test result into the cover letter.
Because the checkepatch tool is not happy with the hex dumped below.
http://mails.dpdk.org/archives/test-report/2020-September/151395.html
Thanks,
Phil
> the cmdline as flows:
> 1. first, startup testpmd:
> testpmd> show port 0 rss-hash key
> RSS functions:
> all ipv4-frag ipv4-other ipv6-frag ipv6-other ip
> RSS key:
> 6D5A56DA255B0EC24167253D43A38FB0D0CA2BCBAE7B30B477CB2DA38030F
> 20C6A42B73BBEAC01FA
>
> 2. create a rss rule
> testpmd> flow create 0 ingress pattern eth / ipv4 / udp / end actions rss \
> types ipv4-udp end queues end / end
>
> 3. show rss-hash key
> testpmd> show port 0 rss-hash key
> RSS functions:
> all ipv4-udp udp
> RSS key:
> 74657374706D6427732064656661756C74205253532068617368206B65792C206F
> 76657272696465
>
> Now, it uses rte_eth_dev_rss_hash_conf_get to correctly the
> default rss key. the cmdline and result as flows:
> testpmd> show port 0 rss-hash key
> RSS functions:
> all ipv4-frag ipv4-other ipv6-frag ipv6-other ip
> RSS key:
> 6D5A56DA255B0EC24167253D43A38FB0D0CA2BCBAE7B30B477CB2DA38030F2
> 0C
> 6A42B73BBEAC01FA
> testpmd> flow create 0 ingress pattern eth / ipv4 / udp / end actions rss \
> types ipv4-udp end queues end / end
> testpmd> show port 0 rss-hash key
> RSS functions:
> all ipv4-udp udp
> RSS key:
> 6D5A56DA255B0EC24167253D43A38FB0D0CA2BCBAE7B30B477CB2DA38030F
> 20C6A42B73BBEAC01FA
>
> Fixes: ac8d22de2394 ("ethdev: flatten RSS configuration in flow API")
> Cc: stable@dpdk.org
>
> Signed-off-by: Lijun Ou <oulijun@huawei.com>
> ---
> V2->V3:
> -fix checkpatch warning.
>
> V1->V2:
> -fix the commit.
> ---
> app/test-pmd/cmdline_flow.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index 6263d30..e6648da 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -4312,6 +4312,7 @@ parse_vc_action_rss(struct context *ctx, const
> struct token *token,
> action_rss_data->queue[i] = i;
> if (!port_id_is_invalid(ctx->port, DISABLED_WARN) &&
> ctx->port != (portid_t)RTE_PORT_ALL) {
> + struct rte_eth_rss_conf rss_conf = {0};
> struct rte_eth_dev_info info;
> int ret2;
>
> @@ -4322,6 +4323,13 @@ parse_vc_action_rss(struct context *ctx, const
> struct token *token,
> action_rss_data->conf.key_len =
> RTE_MIN(sizeof(action_rss_data->key),
> info.hash_key_size);
> +
> + rss_conf.rss_key_len = sizeof(action_rss_data->key);
> + rss_conf.rss_key = action_rss_data->key;
> + ret2 = rte_eth_dev_rss_hash_conf_get(ctx->port,
> &rss_conf);
> + if (ret2 != 0)
> + return ret2;
> + action_rss_data->conf.key = rss_conf.rss_key;
> }
> action->conf = &action_rss_data->conf;
> return ret;
> --
> 2.7.4
在 2020/9/22 17:51, Phil Yang 写道:
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Lijun Ou
>> Sent: Thursday, September 10, 2020 9:51 AM
>> To: wenzhuo.lu@intel.com; beilei.xing@intel.com;
>> adrien.mazarguil@6wind.com; ferruh.yigit@intel.com
>> Cc: dev@dpdk.org; linuxarm@huawei.com
>> Subject: [dpdk-dev] [PATCH v3] app/testpmd: fix the default RSS key
>> configuration
>
> Hi Lijun,
>
> Please fix the coding style issues.
>
> "Must be a reply to the first patch (--in-reply-to)."
>
>
>>
>> When a user runs a flow create cmd to configure an RSS rule
>> with specifying the empty rss actions in testpmd, this mean
> ^^^ means
>> that the flow gets the default RSS functions from the valid
>> NIC default RSS hash key. However, the testpmd is not set
> ^^^ is set xxx incorrectly
>> the default RSS key incorrectly when RSS key is not specified
>> in flow create cmd.
>
> Use the NIC valid default RSS key instead of the testpmd dummy RSS key in the flow configuration when the RSS key is not specified in the flow rule. If the NIC RSS key is invalid, it will use testpmd dummy RSS key as the default key.
>
> Is that good to put it in this way? Because I think it is not a bug, your patch offers an approach to update the default testpmd RSS key.
>
Do you have any better advice? or don't use my approach? I think the
previous methods are easy to misunderstand.Can we use the NUL KEY
solution and fix the problem that the length is 0 and the RSS key is not
NULL ?
Thanks
Lijun Ou
> I would also suggest making the commit message shorter and move the test result into the cover letter.
> Because the checkepatch tool is not happy with the hex dumped below.
> http://mails.dpdk.org/archives/test-report/2020-September/151395.html
>
>
> Thanks,
> Phil
>
>> the cmdline as flows:
>> 1. first, startup testpmd:
>> testpmd> show port 0 rss-hash key
>> RSS functions:
>> all ipv4-frag ipv4-other ipv6-frag ipv6-other ip
>> RSS key:
>> 6D5A56DA255B0EC24167253D43A38FB0D0CA2BCBAE7B30B477CB2DA38030F
>> 20C6A42B73BBEAC01FA
>>
>> 2. create a rss rule
>> testpmd> flow create 0 ingress pattern eth / ipv4 / udp / end actions rss \
>> types ipv4-udp end queues end / end
>>
>> 3. show rss-hash key
>> testpmd> show port 0 rss-hash key
>> RSS functions:
>> all ipv4-udp udp
>> RSS key:
>> 74657374706D6427732064656661756C74205253532068617368206B65792C206F
>> 76657272696465
>>
>> Now, it uses rte_eth_dev_rss_hash_conf_get to correctly the
>> default rss key. the cmdline and result as flows:
>> testpmd> show port 0 rss-hash key
>> RSS functions:
>> all ipv4-frag ipv4-other ipv6-frag ipv6-other ip
>> RSS key:
>> 6D5A56DA255B0EC24167253D43A38FB0D0CA2BCBAE7B30B477CB2DA38030F2
>> 0C
>> 6A42B73BBEAC01FA
>> testpmd> flow create 0 ingress pattern eth / ipv4 / udp / end actions rss \
>> types ipv4-udp end queues end / end
>> testpmd> show port 0 rss-hash key
>> RSS functions:
>> all ipv4-udp udp
>> RSS key:
>> 6D5A56DA255B0EC24167253D43A38FB0D0CA2BCBAE7B30B477CB2DA38030F
>> 20C6A42B73BBEAC01FA
>>
>> Fixes: ac8d22de2394 ("ethdev: flatten RSS configuration in flow API")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Lijun Ou <oulijun@huawei.com>
>> ---
>> V2->V3:
>> -fix checkpatch warning.
>>
>> V1->V2:
>> -fix the commit.
>> ---
>> app/test-pmd/cmdline_flow.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
>> index 6263d30..e6648da 100644
>> --- a/app/test-pmd/cmdline_flow.c
>> +++ b/app/test-pmd/cmdline_flow.c
>> @@ -4312,6 +4312,7 @@ parse_vc_action_rss(struct context *ctx, const
>> struct token *token,
>> action_rss_data->queue[i] = i;
>> if (!port_id_is_invalid(ctx->port, DISABLED_WARN) &&
>> ctx->port != (portid_t)RTE_PORT_ALL) {
>> + struct rte_eth_rss_conf rss_conf = {0};
>> struct rte_eth_dev_info info;
>> int ret2;
>>
>> @@ -4322,6 +4323,13 @@ parse_vc_action_rss(struct context *ctx, const
>> struct token *token,
>> action_rss_data->conf.key_len =
>> RTE_MIN(sizeof(action_rss_data->key),
>> info.hash_key_size);
>> +
>> + rss_conf.rss_key_len = sizeof(action_rss_data->key);
>> + rss_conf.rss_key = action_rss_data->key;
>> + ret2 = rte_eth_dev_rss_hash_conf_get(ctx->port,
>> &rss_conf);
>> + if (ret2 != 0)
>> + return ret2;
>> + action_rss_data->conf.key = rss_conf.rss_key;
>> }
>> action->conf = &action_rss_data->conf;
>> return ret;
>> --
>> 2.7.4
>
> .
>
dev <dev-bounces@dpdk.org> On Behalf Of Lijun Ou writes:
> >> Subject: [dpdk-dev] [PATCH v3] app/testpmd: fix the default RSS key
> >> configuration
> >
> > Hi Lijun,
> >
> > Please fix the coding style issues.
> >
> > "Must be a reply to the first patch (--in-reply-to)."
> >
> >
> >>
> >> When a user runs a flow create cmd to configure an RSS rule
> >> with specifying the empty rss actions in testpmd, this mean
> > ^^^ means
> >> that the flow gets the default RSS functions from the valid
> >> NIC default RSS hash key. However, the testpmd is not set
> > ^^^ is set xxx incorrectly
> >> the default RSS key incorrectly when RSS key is not specified
> >> in flow create cmd.
> >
> > Use the NIC valid default RSS key instead of the testpmd dummy RSS key in
> the flow configuration when the RSS key is not specified in the flow rule. If
> the NIC RSS key is invalid, it will use testpmd dummy RSS key as the default
> key.
> >
> > Is that good to put it in this way? Because I think it is not a bug, your patch
> offers an approach to update the default testpmd RSS key.
> >
> Do you have any better advice? or don't use my approach? I think the
No, I think you misunderstood me. I agree with your proposal and your patch looks good to me.
My suggestion is to reword the commit message to highlight that you mare making testpmd use the valid NIC RSS key as the default flow RSS key in this patch.
In my perspective, if you don't specify any RSS key in your flow rule, it should allow any available RSS key work as the default key.
So use a dummy RSS key is correct as well.
> previous methods are easy to misunderstand.Can we use the NUL KEY
> solution and fix the problem that the length is 0 and the RSS key is not
> NULL ?
>
> Thanks
> Lijun Ou
> > I would also suggest making the commit message shorter and move the
> test result into the cover letter.
> > Because the checkepatch tool is not happy with the hex dumped below.
> > http://mails.dpdk.org/archives/test-report/2020-September/151395.html
> >
> >
> > Thanks,
> > Phil
> >
> >> the cmdline as flows:
> >> 1. first, startup testpmd:
> >> testpmd> show port 0 rss-hash key
> >> RSS functions:
> >> all ipv4-frag ipv4-other ipv6-frag ipv6-other ip
> >> RSS key:
> >>
> 6D5A56DA255B0EC24167253D43A38FB0D0CA2BCBAE7B30B477CB2DA38030F
> >> 20C6A42B73BBEAC01FA
> >>
> >> 2. create a rss rule
> >> testpmd> flow create 0 ingress pattern eth / ipv4 / udp / end actions rss \
> >> types ipv4-udp end queues end / end
> >>
> >> 3. show rss-hash key
> >> testpmd> show port 0 rss-hash key
> >> RSS functions:
> >> all ipv4-udp udp
> >> RSS key:
> >>
> 74657374706D6427732064656661756C74205253532068617368206B65792C206F
> >> 76657272696465
> >>
> >> Now, it uses rte_eth_dev_rss_hash_conf_get to correctly the
> >> default rss key. the cmdline and result as flows:
> >> testpmd> show port 0 rss-hash key
> >> RSS functions:
> >> all ipv4-frag ipv4-other ipv6-frag ipv6-other ip
> >> RSS key:
> >>
> 6D5A56DA255B0EC24167253D43A38FB0D0CA2BCBAE7B30B477CB2DA38030F2
> >> 0C
> >> 6A42B73BBEAC01FA
> >> testpmd> flow create 0 ingress pattern eth / ipv4 / udp / end actions rss \
> >> types ipv4-udp end queues end / end
> >> testpmd> show port 0 rss-hash key
> >> RSS functions:
> >> all ipv4-udp udp
> >> RSS key:
> >>
> 6D5A56DA255B0EC24167253D43A38FB0D0CA2BCBAE7B30B477CB2DA38030F
> >> 20C6A42B73BBEAC01FA
> >>
> >> Fixes: ac8d22de2394 ("ethdev: flatten RSS configuration in flow API")
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Lijun Ou <oulijun@huawei.com>
> >> ---
> >> V2->V3:
> >> -fix checkpatch warning.
> >>
> >> V1->V2:
> >> -fix the commit.
> >> ---
> >> app/test-pmd/cmdline_flow.c | 8 ++++++++
> >> 1 file changed, 8 insertions(+)
> >>
> >> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-
> pmd/cmdline_flow.c
> >> index 6263d30..e6648da 100644
> >> --- a/app/test-pmd/cmdline_flow.c
> >> +++ b/app/test-pmd/cmdline_flow.c
> >> @@ -4312,6 +4312,7 @@ parse_vc_action_rss(struct context *ctx, const
> >> struct token *token,
> >> action_rss_data->queue[i] = i;
> >> if (!port_id_is_invalid(ctx->port, DISABLED_WARN) &&
> >> ctx->port != (portid_t)RTE_PORT_ALL) {
> >> + struct rte_eth_rss_conf rss_conf = {0};
> >> struct rte_eth_dev_info info;
> >> int ret2;
> >>
> >> @@ -4322,6 +4323,13 @@ parse_vc_action_rss(struct context *ctx, const
> >> struct token *token,
> >> action_rss_data->conf.key_len =
> >> RTE_MIN(sizeof(action_rss_data->key),
> >> info.hash_key_size);
> >> +
> >> + rss_conf.rss_key_len = sizeof(action_rss_data->key);
> >> + rss_conf.rss_key = action_rss_data->key;
> >> + ret2 = rte_eth_dev_rss_hash_conf_get(ctx->port,
> >> &rss_conf);
> >> + if (ret2 != 0)
> >> + return ret2;
> >> + action_rss_data->conf.key = rss_conf.rss_key;
> >> }
> >> action->conf = &action_rss_data->conf;
> >> return ret;
> >> --
> >> 2.7.4
> >
> > .
> >
Consider the follow usage with testpmd:
1. first, startup testpmd:
testpmd> show port 0 rss-hash key
RSS functions:
all ipv4-frag ipv4-other ipv6-frag ipv6-other ip
RSS key:
6D5A56DA255B0EC24167253D43A38FB0D0CA2BCBAE7B30B477CB2DA38030F
20C6A42B73BBEAC01FA
2. create a rss rule
testpmd> flow create 0 ingress pattern eth / ipv4 / udp / end actions rss \
types ipv4-udp end queues end / end
3. show rss-hash key
testpmd> show port 0 rss-hash key
RSS functions:
all ipv4-udp udp
RSS key:
74657374706D6427732064656661756C74205253532068617368206B65792C206F
76657272696465
As a result, the step 3 with RSS key and the step 1 RSS key
is not the same. The patch[1] to solve the above problems.
Lijun Ou (1):
app/testpmd: fix the default RSS key configuration
app/test-pmd/cmdline_flow.c | 8 ++++++++
1 file changed, 8 insertions(+)
On 9/24/2020 2:45 PM, Lijun Ou wrote:
> Consider the follow usage with testpmd:
> 1. first, startup testpmd:
> testpmd> show port 0 rss-hash key
> RSS functions:
> all ipv4-frag ipv4-other ipv6-frag ipv6-other ip
> RSS key:
> 6D5A56DA255B0EC24167253D43A38FB0D0CA2BCBAE7B30B477CB2DA38030F
> 20C6A42B73BBEAC01FA
> 2. create a rss rule
> testpmd> flow create 0 ingress pattern eth / ipv4 / udp / end actions rss \
> types ipv4-udp end queues end / end
>
> 3. show rss-hash key
> testpmd> show port 0 rss-hash key
> RSS functions:
> all ipv4-udp udp
> RSS key:
> 74657374706D6427732064656661756C74205253532068617368206B65792C206F
> 76657272696465
>
> As a result, the step 3 with RSS key and the step 1 RSS key
> is not the same. The patch[1] to solve the above problems.
>
This is interesting, can you please provide above information in the commit log too?
Also can you please provide the details on why this happens, callstack can help?
Thanks,
ferruh
> Lijun Ou (1):
> app/testpmd: fix the default RSS key configuration
>
> app/test-pmd/cmdline_flow.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
在 2020/9/30 21:17, Ferruh Yigit 写道:
> On 9/24/2020 2:45 PM, Lijun Ou wrote:
>> Consider the follow usage with testpmd:
>> 1. first, startup testpmd:
>> testpmd> show port 0 rss-hash key
>> RSS functions:
>> all ipv4-frag ipv4-other ipv6-frag ipv6-other ip
>> RSS key:
>> 6D5A56DA255B0EC24167253D43A38FB0D0CA2BCBAE7B30B477CB2DA38030F
>> 20C6A42B73BBEAC01FA
>> 2. create a rss rule
>> testpmd> flow create 0 ingress pattern eth / ipv4 / udp / end actions
>> rss \
>> types ipv4-udp end queues end / end
>>
>> 3. show rss-hash key
>> testpmd> show port 0 rss-hash key
>> RSS functions:
>> all ipv4-udp udp
>> RSS key:
>> 74657374706D6427732064656661756C74205253532068617368206B65792C206F
>> 76657272696465
>>
>> As a result, the step 3 with RSS key and the step 1 RSS key
>> is not the same. The patch[1] to solve the above problems.
>>
>
> This is interesting, can you please provide above information in the
> commit log too?
>
Yes， I submitted detailed operation information in patch v3 as a commit,
and Yang suggested that the operation information be included in the
cover letter.
> Also can you please provide the details on why this happens, callstack
> can help?
>
When you start the testpmd, the pmd driver initializes the RSS
configuration. Generally, the recommended RSS hash key is used as the
default key in the driver. In addition, the default key is different
from the default RSS flow in testpmd without specifying RSS hash key.
So， if you do not specify the RSS key when creating an RSS rule, the
testpmd uses the default key as the default RSS key of the RSS rule.As a
result, you may mistakenly consider that the RSS key in use is the valid
default key of the NIC, actually, the key and the valid default key of
the NIC are two values.
> Thanks,
> ferruh
>
>
>> Lijun Ou (1):
>> app/testpmd: fix the default RSS key configuration
>>
>> app/test-pmd/cmdline_flow.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>
> .
>
On 10/9/2020 1:09 PM, oulijun wrote:
>
>
> 在 2020/9/30 21:17, Ferruh Yigit 写道:
>> On 9/24/2020 2:45 PM, Lijun Ou wrote:
>>> Consider the follow usage with testpmd:
>>> 1. first, startup testpmd:
>>> testpmd> show port 0 rss-hash key
>>> RSS functions:
>>> all ipv4-frag ipv4-other ipv6-frag ipv6-other ip
>>> RSS key:
>>> 6D5A56DA255B0EC24167253D43A38FB0D0CA2BCBAE7B30B477CB2DA38030F
>>> 20C6A42B73BBEAC01FA
>>> 2. create a rss rule
>>> testpmd> flow create 0 ingress pattern eth / ipv4 / udp / end actions rss \
>>> types ipv4-udp end queues end / end
>>>
>>> 3. show rss-hash key
>>> testpmd> show port 0 rss-hash key
>>> RSS functions:
>>> all ipv4-udp udp
>>> RSS key:
>>> 74657374706D6427732064656661756C74205253532068617368206B65792C206F
>>> 76657272696465
>>>
>>> As a result, the step 3 with RSS key and the step 1 RSS key
>>> is not the same. The patch[1] to solve the above problems.
>>>
>>
>> This is interesting, can you please provide above information in the commit
>> log too?
>>
> Yes， I submitted detailed operation information in patch v3 as a commit, and
> Yang suggested that the operation information be included in the cover letter.
>
OK, understood.
Only, commands helped me to understand the problem, it is easy to grasp the
issue with samples, so I thought it may help others later in if it is in the
commit log, since cover letter won't be visible in the git repo.
@Phil, will you be OK to have them in the commit log if the checkpatch warnings
fixed?
>> Also can you please provide the details on why this happens, callstack can help?
>>
> When you start the testpmd, the pmd driver initializes the RSS configuration.
> Generally, the recommended RSS hash key is used as the default key in the
> driver. In addition, the default key is different from the default RSS flow in
> testpmd without specifying RSS hash key.
> So， if you do not specify the RSS key when creating an RSS rule, the testpmd
> uses the default key as the default RSS key of the RSS rule.As a result, you may
> mistakenly consider that the RSS key in use is the valid default key of the NIC,
> actually, the key and the valid default key of the NIC are two values.
Above description looks good, can you include this to the commit log please?
>> Thanks,
>> ferruh
>>
>>
>>> Lijun Ou (1):
>>> app/testpmd: fix the default RSS key configuration
>>>
>>> app/test-pmd/cmdline_flow.c | 8 ++++++++
>>> 1 file changed, 8 insertions(+)
>>>
>>
>> .
>>
Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> Sent: Saturday, October 10, 2020 2:53 AM
> To: oulijun <oulijun@huawei.com>; wenzhuo.lu@intel.com;
> beilei.xing@intel.com; adrien.mazarguil@6wind.com; Phil Yang
> <Phil.Yang@arm.com>
> Cc: dev@dpdk.org; linuxarm@huawei.com
> Subject: Re: [PATCH v4] RSS key use with testpmd
>
> On 10/9/2020 1:09 PM, oulijun wrote:
> >
> >
> > 在 2020/9/30 21:17, Ferruh Yigit 写道:
> >> On 9/24/2020 2:45 PM, Lijun Ou wrote:
> >>> Consider the follow usage with testpmd:
> >>> 1. first, startup testpmd:
> >>> testpmd> show port 0 rss-hash key
> >>> RSS functions:
> >>> all ipv4-frag ipv4-other ipv6-frag ipv6-other ip
> >>> RSS key:
> >>>
> 6D5A56DA255B0EC24167253D43A38FB0D0CA2BCBAE7B30B477CB2DA38030F
> >>> 20C6A42B73BBEAC01FA
> >>> 2. create a rss rule
> >>> testpmd> flow create 0 ingress pattern eth / ipv4 / udp / end actions rss
> \
> >>> types ipv4-udp end queues end / end
> >>>
> >>> 3. show rss-hash key
> >>> testpmd> show port 0 rss-hash key
> >>> RSS functions:
> >>> all ipv4-udp udp
> >>> RSS key:
> >>>
> 74657374706D6427732064656661756C74205253532068617368206B65792C206F
> >>> 76657272696465
> >>>
> >>> As a result, the step 3 with RSS key and the step 1 RSS key
> >>> is not the same. The patch[1] to solve the above problems.
> >>>
> >>
> >> This is interesting, can you please provide above information in the
> commit
> >> log too?
> >>
> > Yes， I submitted detailed operation information in patch v3 as a commit,
> and
> > Yang suggested that the operation information be included in the cover
> letter.
> >
>
> OK, understood.
> Only, commands helped me to understand the problem, it is easy to grasp
> the
> issue with samples, so I thought it may help others later in if it is in the
> commit log, since cover letter won't be visible in the git repo.
>
> @Phil, will you be OK to have them in the commit log if the checkpatch
> warnings
> fixed?
Yeah. It is OK.
>
> >> Also can you please provide the details on why this happens, callstack can
> help?
> >>
> > When you start the testpmd, the pmd driver initializes the RSS
> configuration.
> > Generally, the recommended RSS hash key is used as the default key in the
> > driver. In addition, the default key is different from the default RSS flow in
> > testpmd without specifying RSS hash key.
> > So， if you do not specify the RSS key when creating an RSS rule, the
> testpmd
> > uses the default key as the default RSS key of the RSS rule.As a result, you
> may
> > mistakenly consider that the RSS key in use is the valid default key of the
> NIC,
> > actually, the key and the valid default key of the NIC are two values.
>
> Above description looks good, can you include this to the commit log please?
>
> >> Thanks,
> >> ferruh
> >>
> >>
> >>> Lijun Ou (1):
> >>> app/testpmd: fix the default RSS key configuration
> >>>
> >>> app/test-pmd/cmdline_flow.c | 8 ++++++++
> >>> 1 file changed, 8 insertions(+)
> >>>
> >>
> >> .
> >>
在 2020/10/10 2:52, Ferruh Yigit 写道:
> On 10/9/2020 1:09 PM, oulijun wrote:
>>
>>
>> 在 2020/9/30 21:17, Ferruh Yigit 写道:
>>> On 9/24/2020 2:45 PM, Lijun Ou wrote:
>>>> Consider the follow usage with testpmd:
>>>> 1. first, startup testpmd:
>>>> testpmd> show port 0 rss-hash key
>>>> RSS functions:
>>>> all ipv4-frag ipv4-other ipv6-frag ipv6-other ip
>>>> RSS key:
>>>> 6D5A56DA255B0EC24167253D43A38FB0D0CA2BCBAE7B30B477CB2DA38030F
>>>> 20C6A42B73BBEAC01FA
>>>> 2. create a rss rule
>>>> testpmd> flow create 0 ingress pattern eth / ipv4 / udp / end
>>>> actions rss \
>>>> types ipv4-udp end queues end / end
>>>>
>>>> 3. show rss-hash key
>>>> testpmd> show port 0 rss-hash key
>>>> RSS functions:
>>>> all ipv4-udp udp
>>>> RSS key:
>>>> 74657374706D6427732064656661756C74205253532068617368206B65792C206F
>>>> 76657272696465
>>>>
>>>> As a result, the step 3 with RSS key and the step 1 RSS key
>>>> is not the same. The patch[1] to solve the above problems.
>>>>
>>>
>>> This is interesting, can you please provide above information in the
>>> commit log too?
>>>
>> Yes， I submitted detailed operation information in patch v3 as a
>> commit, and Yang suggested that the operation information be included
>> in the cover letter.
> >
>
> OK, understood.
> Only, commands helped me to understand the problem, it is easy to grasp
> the issue with samples, so I thought it may help others later in if it
> is in the commit log, since cover letter won't be visible in the git repo.
>
> @Phil, will you be OK to have them in the commit log if the checkpatch
> warnings fixed?
>
>>> Also can you please provide the details on why this happens,
>>> callstack can help?
>>>
>> When you start the testpmd, the pmd driver initializes the RSS
>> configuration. Generally, the recommended RSS hash key is used as the
>> default key in the driver. In addition, the default key is different
>> from the default RSS flow in testpmd without specifying RSS hash key.
>> So， if you do not specify the RSS key when creating an RSS rule, the
>> testpmd uses the default key as the default RSS key of the RSS rule.As
>> a result, you may mistakenly consider that the RSS key in use is the
>> valid default key of the NIC, actually, the key and the valid default
>> key of the NIC are two values.
>
> Above description looks good, can you include this to the commit log
> please?
>
Hi，Ferruh
Your opinion is to put the following comit log in app/testpmd: fix
the default RSS key configuration？
Consider the follow usage with testpmd:
1. first, startup testpmd:
testpmd> show port 0 rss-hash key
RSS functions:
all ipv4-frag ipv4-other ipv6-frag ipv6-other ip
RSS key:
6D5A56DA255B0EC24167253D43A38FB0D0CA2BCBAE7B30B477CB2DA38030F
20C6A42B73BBEAC01FA
2. create a rss rule
testpmd> flow create 0 ingress pattern eth / ipv4 / udp / end actions rss \
types ipv4-udp end queues end / end
3. show rss-hash key
testpmd> show port 0 rss-hash key
RSS functions:
all ipv4-udp udp
RSS key:
74657374706D6427732064656661756C74205253532068617368206B65792C206F
76657272696465
>>> Thanks,
>>> ferruh
>>>
>>>
>>>> Lijun Ou (1):
>>>> app/testpmd: fix the default RSS key configuration
>>>>
>>>> app/test-pmd/cmdline_flow.c | 8 ++++++++
>>>> 1 file changed, 8 insertions(+)
>>>>
>>>
>>> .
>>>
>
> .
>
On 10/14/2020 7:15 AM, oulijun wrote:
>
>
> 在 2020/10/10 2:52, Ferruh Yigit 写道:
>> On 10/9/2020 1:09 PM, oulijun wrote:
>>>
>>>
>>> 在 2020/9/30 21:17, Ferruh Yigit 写道:
>>>> On 9/24/2020 2:45 PM, Lijun Ou wrote:
>>>>> Consider the follow usage with testpmd:
>>>>> 1. first, startup testpmd:
>>>>> testpmd> show port 0 rss-hash key
>>>>> RSS functions:
>>>>> all ipv4-frag ipv4-other ipv6-frag ipv6-other ip
>>>>> RSS key:
>>>>> 6D5A56DA255B0EC24167253D43A38FB0D0CA2BCBAE7B30B477CB2DA38030F
>>>>> 20C6A42B73BBEAC01FA
>>>>> 2. create a rss rule
>>>>> testpmd> flow create 0 ingress pattern eth / ipv4 / udp / end actions rss \
>>>>> types ipv4-udp end queues end / end
>>>>>
>>>>> 3. show rss-hash key
>>>>> testpmd> show port 0 rss-hash key
>>>>> RSS functions:
>>>>> all ipv4-udp udp
>>>>> RSS key:
>>>>> 74657374706D6427732064656661756C74205253532068617368206B65792C206F
>>>>> 76657272696465
>>>>>
>>>>> As a result, the step 3 with RSS key and the step 1 RSS key
>>>>> is not the same. The patch[1] to solve the above problems.
>>>>>
>>>>
>>>> This is interesting, can you please provide above information in the commit
>>>> log too?
>>>>
>>> Yes， I submitted detailed operation information in patch v3 as a commit, and
>>> Yang suggested that the operation information be included in the cover letter.
>> >
>>
>> OK, understood.
>> Only, commands helped me to understand the problem, it is easy to grasp the
>> issue with samples, so I thought it may help others later in if it is in the
>> commit log, since cover letter won't be visible in the git repo.
>>
>> @Phil, will you be OK to have them in the commit log if the checkpatch
>> warnings fixed?
>>
>>>> Also can you please provide the details on why this happens, callstack can
>>>> help?
>>>>
>>> When you start the testpmd, the pmd driver initializes the RSS configuration.
>>> Generally, the recommended RSS hash key is used as the default key in the
>>> driver. In addition, the default key is different from the default RSS flow
>>> in testpmd without specifying RSS hash key.
>>> So， if you do not specify the RSS key when creating an RSS rule, the testpmd
>>> uses the default key as the default RSS key of the RSS rule.As a result, you
>>> may mistakenly consider that the RSS key in use is the valid default key of
>>> the NIC, actually, the key and the valid default key of the NIC are two values.
>>
>> Above description looks good, can you include this to the commit log please?
>>
> Hi，Ferruh
> Your opinion is to put the following comit log in app/testpmd: fix the
> default RSS key configuration？
> Consider the follow usage with testpmd:
> 1. first, startup testpmd:
> testpmd> show port 0 rss-hash key
> RSS functions:
> all ipv4-frag ipv4-other ipv6-frag ipv6-other ip
> RSS key:
> 6D5A56DA255B0EC24167253D43A38FB0D0CA2BCBAE7B30B477CB2DA38030F
> 20C6A42B73BBEAC01FA
> 2. create a rss rule
> testpmd> flow create 0 ingress pattern eth / ipv4 / udp / end actions rss \
> types ipv4-udp end queues end / end
>
> 3. show rss-hash key
> testpmd> show port 0 rss-hash key
> RSS functions:
> all ipv4-udp udp
> RSS key:
> 74657374706D6427732064656661756C74205253532068617368206B65792C206F
> 76657272696465
>
Above description and this sample please? Thanks.
>>>> Thanks,
>>>> ferruh
>>>>
>>>>
>>>>> Lijun Ou (1):
>>>>> app/testpmd: fix the default RSS key configuration
>>>>>
>>>>> app/test-pmd/cmdline_flow.c | 8 ++++++++
>>>>> 1 file changed, 8 insertions(+)
>>>>>
>>>>
>>>> .
>>>>
>>
>> .
>>
@@ -4312,6 +4312,7 @@ parse_vc_action_rss(struct context *ctx, const struct token *token,
action_rss_data->queue[i] = i;
if (!port_id_is_invalid(ctx->port, DISABLED_WARN) &&
ctx->port != (portid_t)RTE_PORT_ALL) {
+ struct rte_eth_rss_conf rss_conf = {0};
struct rte_eth_dev_info info;
int ret2;
@@ -4322,6 +4323,13 @@ parse_vc_action_rss(struct context *ctx, const struct token *token,
action_rss_data->conf.key_len =
RTE_MIN(sizeof(action_rss_data->key),
info.hash_key_size);
+
+ rss_conf.rss_key_len = sizeof(action_rss_data->key);
+ rss_conf.rss_key = action_rss_data->key;
+ ret2 = rte_eth_dev_rss_hash_conf_get(ctx->port, &rss_conf);
+ if (ret2 != 0)
+ return ret2;
+ action_rss_data->conf.key = rss_conf.rss_key;
}
action->conf = &action_rss_data->conf;
return ret;