在 2020/9/22 17:51, Phil Yang 写道:
-----Original Message-----
From: dev <dev-boun...@dpdk.org> On Behalf Of Lijun Ou
Sent: Thursday, September 10, 2020 9:51 AM
To: wenzhuo...@intel.com; beilei.x...@intel.com;
adrien.mazarg...@6wind.com; ferruh.yi...@intel.com
Cc: dev@dpdk.org; linux...@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: sta...@dpdk.org

Signed-off-by: Lijun Ou <ouli...@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

.

Reply via email to