From: Ferruh Yigit <ferruh.yigit@xilinx.com>
To: "lihuisong (C)" <lihuisong@huawei.com>,
Xiaoyun Li <xiaoyun.li@intel.com>,
Aman Singh <aman.deep.singh@intel.com>,
Yuying Zhang <yuying.zhang@intel.com>,
Helin Zhang <helin.zhang@intel.com>,
Jingjing Wu <jingjing.wu@intel.com>,
Thomas Monjalon <thomas.monjalon@6wind.com>
Cc: <dev@dpdk.org>, Thomas Monjalon <thomas@monjalon.net>,
Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
<stable@dpdk.org>
Subject: Re: [PATCH 1/3] app/testpmd: fix displaying RSS info
Date: Mon, 30 May 2022 11:43:03 +0100 [thread overview]
Message-ID: <242f8a69-81d9-1795-fcf3-94358dae590d@xilinx.com> (raw)
In-Reply-To: <3110523f-72e6-20be-4510-be579722ffbd@huawei.com>
On 5/27/2022 3:30 AM, lihuisong (C) wrote:
> CAUTION: This message has originated from an External Source. Please use
> proper judgment and caution when opening attachments, clicking links, or
> responding to this email.
>
>
> 在 2022/5/26 1:37, Ferruh Yigit 写道:
>> When supported RSS offload flow types are printed via 'show port info #'
>> command, flow names are get from flow type array which is wrong and
>> causing some RSS flow types not being displayed.
>>
>> Instead RSS flow type array should be used. Also helper functions added
>> and existing code updated to use helpers.
>>
>> Fixes: b12964f621dc ("ethdev: unification of RSS offload types")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@xilinx.com>
>> ---
>> Cc: helin.zhang@intel.com
>>
>> Note:
>> In ethdev, flow type macros 'RTE_ETH_FLOW_*' and RSS type macros
>> 'RTE_ETH_RSS_*' are related, buy they seems diverged a little, may need
>> to check that too.
>> ---
>> app/test-pmd/config.c | 92 ++++++++++++++++++++++++++++++-------------
>> 1 file changed, 64 insertions(+), 28 deletions(-)
>>
>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>> index 72d2606d19d5..c353d224ef06 100644
>> --- a/app/test-pmd/config.c
>> +++ b/app/test-pmd/config.c
>> @@ -147,6 +147,32 @@ const struct rss_type_info rss_type_table[] = {
>> { NULL, 0 },
>> };
>>
>> +static const char *
>> +rsstype_to_str(uint64_t rss_type)
>> +{
>> + int i;
>> +
>> + for (i = 0; rss_type_table[i].str != NULL; i++) {
>> + if (rss_type_table[i].rss_type == rss_type)
>> + return rss_type_table[i].str;
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +static uint64_t
>> +str_to_rsstype(const char *str)
>> +{
>> + int i;
>> +
>> + for (i = 0; rss_type_table[i].str != NULL; i++) {
>> + if (!strcmp(rss_type_table[i].str, str))
>> + return rss_type_table[i].rss_type;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> static const struct {
>> enum rte_eth_fec_mode mode;
>> const char *name;
>> @@ -779,19 +805,21 @@ port_infos_display(portid_t port_id)
>> if (!dev_info.flow_type_rss_offloads)
>> printf("No RSS offload flow type is supported.\n");
>> else {
>> + uint64_t rss_types = dev_info.flow_type_rss_offloads;
>> uint16_t i;
>> - char *p;
>>
>> printf("Supported RSS offload flow types:\n");
>> - for (i = RTE_ETH_FLOW_UNKNOWN + 1;
>> - i < sizeof(dev_info.flow_type_rss_offloads) *
>> CHAR_BIT; i++) {
>> - if (!(dev_info.flow_type_rss_offloads & (1ULL <<
>> i)))
>> - continue;
>> - p = flowtype_to_str(i);
>> - if (p)
>> - printf(" %s\n", p);
>> - else
>> - printf(" user defined %d\n", i);
>> + for (i = 0; rss_types != 0; i++) {
>> + if (rss_types & 1) {
>> + uint64_t rss_type = 1ULL << i;
>> + const char *p = rsstype_to_str(rss_type);
>> +
>> + if (p)
>> + printf(" %s\n", p);
>> + else
>> + printf(" user defined
>> 0x%"PRIx64"\n", rss_type);
>> + }
>> + rss_types >>= 1;
>> }
>> }
>>
>> @@ -1547,6 +1575,7 @@ port_flow_complain(struct rte_flow_error *error)
>> static void
>> rss_config_display(struct rte_flow_action_rss *rss_conf)
>> {
>> + uint64_t rss_types;
>> uint8_t i;
>>
>> if (rss_conf == NULL) {
>> @@ -1582,16 +1611,23 @@ rss_config_display(struct rte_flow_action_rss
>> *rss_conf)
>> }
>>
>> printf(" types:\n");
>> - if (rss_conf->types == 0) {
>> + rss_types = rss_conf->types;
>> + if (rss_types == 0) {
>> printf(" none\n");
>> return;
>> }
>> - for (i = 0; rss_type_table[i].str; i++) {
>> - if ((rss_conf->types &
>> - rss_type_table[i].rss_type) ==
>> - rss_type_table[i].rss_type &&
>> - rss_type_table[i].rss_type != 0)
>> - printf(" %s\n", rss_type_table[i].str);
>> +
>> + for (i = 0; rss_types != 0; i++) {
>> + if (rss_types & 1) {
>> + uint64_t rss_type = 1ULL << i;
>> + const char *p = rsstype_to_str(rss_type);
> It seems that we can't use one bit to get rss type name.
> Because part of name in rss_type_table[] consist of multiple bits.
> Maybe it's better to use the original method to display RSS type name.
>
Right, it doesn't cover 'all' case, but thinking twice how useful 'all'
case is, it doesn't really cover all options, and for user it is hard to
know which functions are set when it displays 'all'.
What about to remove 'all' item from 'rss_type_table[]'?
> On the other hand, RSS type name are printed in many places as this
> patch modified.
> It is recommended that you encapsulate a funcion to display RSS type.
Yes but each are formatting slightly different, so to let various
formatting possible, common function returns string instead of printing
them. As 3/3 of this set does some formatting.
>> +
>> + if (p)
>> + printf(" %s\n", p);
>> + else
>> + printf(" user defined 0x%"PRIx64"\n",
>> rss_type);
>> + }
>> + rss_types >>= 1;
>> }
>> }
>>
>> @@ -3823,11 +3859,16 @@ port_rss_hash_conf_show(portid_t port_id, int
>> show_rss_key)
>> return;
>> }
>> printf("RSS functions:\n ");
>> - for (i = 0; rss_type_table[i].str; i++) {
>> - if (rss_type_table[i].rss_type == 0)
>> - continue;
>> - if ((rss_hf & rss_type_table[i].rss_type) ==
>> rss_type_table[i].rss_type)
>> - printf("%s ", rss_type_table[i].str);
>> + for (i = 0; rss_hf != 0; i++) {
>> + if (rss_hf & 1) {
>> + uint64_t rss_type = 1ULL << i;
>> + const char *p = rsstype_to_str(rss_type);
>> + if (p)
>> + printf("%s ", p);
>> + else
>> + printf("0x%"PRIx64" ", rss_type);
>> + }
>> + rss_hf >>= 1;
>> }
>> printf("\n");
>> if (!show_rss_key)
>> @@ -3844,15 +3885,10 @@ port_rss_hash_key_update(portid_t port_id,
>> char rss_type[], uint8_t *hash_key,
>> {
>> struct rte_eth_rss_conf rss_conf;
>> int diag;
>> - unsigned int i;
>>
>> rss_conf.rss_key = NULL;
>> rss_conf.rss_key_len = 0;
>> - rss_conf.rss_hf = 0;
>> - for (i = 0; rss_type_table[i].str; i++) {
>> - if (!strcmp(rss_type_table[i].str, rss_type))
>> - rss_conf.rss_hf = rss_type_table[i].rss_type;
>> - }
>> + rss_conf.rss_hf = str_to_rsstype(rss_type);
>> diag = rte_eth_dev_rss_hash_conf_get(port_id, &rss_conf);
>> if (diag == 0) {
>> rss_conf.rss_key = hash_key;
next prev parent reply other threads:[~2022-05-30 10:43 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-25 17:37 Ferruh Yigit
2022-05-26 3:52 ` lihuisong (C)
2022-05-26 8:56 ` Ferruh Yigit
2022-05-27 2:48 ` lihuisong (C)
2022-05-27 2:30 ` lihuisong (C)
2022-05-30 10:43 ` Ferruh Yigit [this message]
2022-05-30 12:32 ` lihuisong (C)
2022-05-30 13:02 ` Ferruh Yigit
2022-05-31 2:07 ` lihuisong (C)
2022-05-31 16:35 ` Andrew Rybchenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=242f8a69-81d9-1795-fcf3-94358dae590d@xilinx.com \
--to=ferruh.yigit@xilinx.com \
--cc=aman.deep.singh@intel.com \
--cc=andrew.rybchenko@oktetlabs.ru \
--cc=dev@dpdk.org \
--cc=helin.zhang@intel.com \
--cc=jingjing.wu@intel.com \
--cc=lihuisong@huawei.com \
--cc=stable@dpdk.org \
--cc=thomas.monjalon@6wind.com \
--cc=thomas@monjalon.net \
--cc=xiaoyun.li@intel.com \
--cc=yuying.zhang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).