From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id D016AA0540; Mon, 30 May 2022 15:02:23 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 6DED540694; Mon, 30 May 2022 15:02:23 +0200 (CEST) Received: from NAM12-DM6-obe.outbound.protection.outlook.com (mail-dm6nam12on2062.outbound.protection.outlook.com [40.107.243.62]) by mails.dpdk.org (Postfix) with ESMTP id 6ABA2400D6; Mon, 30 May 2022 15:02:20 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=d3CT8IfSOnJMNC2Ow4myAjHlmeiMpJ0rI7HHUf3L/3CGZbNhND3fcWiEDBOPR2sc4sdCKlyPeF2KL65hki7SgMmRxsk/niCggQGLI9kGNOR7UAYAv6T4s1WEbm/6LcmX3tvYStPH8mtyee4Xwz0AKtUBi1StJB8Xj+66sDK2z3sUYsfqmIRdD1wKfj+Ba2S6plviRNSLEUYF/ZhraGUQ7SSOTUweWM2O5iswujrMgf1VUs4QEbUjd3d3kElPgULsx9p9c2ag3UYjnnG2Nr5AR310WdHisQP8xMAuC+txikF2HDNoI+apIBDarMlw+7qwTpv7xHD1Yp0toNr0xK6eZw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=6G5xucrkmsIkAnkQFSMQMO3eOKb/0x9kLeiZku3PKLY=; b=RYB/8MtFr7bMdxYW3Bt/GNWZqggcf2YN5lXwJka6ntKjK1UiX7uhpj29v0v/APgm6jjgWGb2BJl3qJpX0VdlwImdFNgBW9apteXGfY3w9I+JxAdqV6UI+Vyt5HOF/rUFhLEs0otfwezTKQ3ZglS4jNmLlcyzLZk4we69OGwE/SEw/roXZ+0vKI/DYKqI6XPl1S2zfgMzQFjtfIvRfa8PHPKntDnqszdI+jTKPavyUcknDoMf/AhTXBNax/D6Y9fFtr0MIMmtJG7BCHsEYu2kxlhxGo+uLOTytnTjURiK7u0XGsO/UIJ7SnagkF5mjAd9xNbGNdX79JJRMrosrejDhA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 149.199.80.198) smtp.rcpttodomain=huawei.com smtp.mailfrom=xilinx.com; dmarc=pass (p=none sp=none pct=100) action=none header.from=xilinx.com; dkim=none (message not signed); arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=xilinx.onmicrosoft.com; s=selector2-xilinx-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=6G5xucrkmsIkAnkQFSMQMO3eOKb/0x9kLeiZku3PKLY=; b=PrppprM/m8dL5jlsGuMWc8RKTLq4uYkyok8vnprm5FbxrfnsdKBqsmq4tvgtULSPNy4Ku0BNladnFhzl3sgUttCUjJjcFc9AsVPd8gFb9C3V3TQH8lNhuT00UIBGJX/AlurTHCT8rg7MuWMkAklnESBo6eXC1ptO3AfX0wv5OQ4= Received: from DS7PR06CA0016.namprd06.prod.outlook.com (2603:10b6:8:2a::29) by SJ0PR02MB8816.namprd02.prod.outlook.com (2603:10b6:a03:3de::10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5293.13; Mon, 30 May 2022 13:02:16 +0000 Received: from DM3NAM02FT032.eop-nam02.prod.protection.outlook.com (2603:10b6:8:2a:cafe::6e) by DS7PR06CA0016.outlook.office365.com (2603:10b6:8:2a::29) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5293.13 via Frontend Transport; Mon, 30 May 2022 13:02:16 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 149.199.80.198) smtp.mailfrom=xilinx.com; dkim=none (message not signed) header.d=none;dmarc=pass action=none header.from=xilinx.com; Received-SPF: Pass (protection.outlook.com: domain of xilinx.com designates 149.199.80.198 as permitted sender) receiver=protection.outlook.com; client-ip=149.199.80.198; helo=xir-pvapexch01.xlnx.xilinx.com; pr=C Received: from xir-pvapexch01.xlnx.xilinx.com (149.199.80.198) by DM3NAM02FT032.mail.protection.outlook.com (10.13.5.65) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.20.5293.13 via Frontend Transport; Mon, 30 May 2022 13:02:16 +0000 Received: from xir-pvapexch02.xlnx.xilinx.com (172.21.17.17) by xir-pvapexch01.xlnx.xilinx.com (172.21.17.15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2176.14; Mon, 30 May 2022 14:02:13 +0100 Received: from smtp.xilinx.com (172.21.105.198) by xir-pvapexch02.xlnx.xilinx.com (172.21.17.17) with Microsoft SMTP Server id 15.1.2176.14 via Frontend Transport; Mon, 30 May 2022 14:02:13 +0100 Envelope-to: lihuisong@huawei.com, xiaoyun.li@intel.com, aman.deep.singh@intel.com, yuying.zhang@intel.com, helin.zhang@intel.com, jingjing.wu@intel.com, thomas.monjalon@6wind.com, dev@dpdk.org, thomas@monjalon.net, andrew.rybchenko@oktetlabs.ru, stable@dpdk.org Received: from [10.71.118.132] (port=63888) by smtp.xilinx.com with esmtp (Exim 4.90) (envelope-from ) id 1nvf2C-0001i2-SO; Mon, 30 May 2022 14:02:12 +0100 Message-ID: Date: Mon, 30 May 2022 14:02:12 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 Subject: Re: [PATCH 1/3] app/testpmd: fix displaying RSS info Content-Language: en-US To: "lihuisong (C)" , Xiaoyun Li , Aman Singh , Yuying Zhang , Helin Zhang , Jingjing Wu , Thomas Monjalon CC: , Thomas Monjalon , Andrew Rybchenko , References: <20220525173736.3394787-1-ferruh.yigit@xilinx.com> <3110523f-72e6-20be-4510-be579722ffbd@huawei.com> <242f8a69-81d9-1795-fcf3-94358dae590d@xilinx.com> From: Ferruh Yigit In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-EOPAttributedMessage: 0 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: c59abcc9-eab5-4736-09d3-08da423c9f44 X-MS-TrafficTypeDiagnostic: SJ0PR02MB8816:EE_ X-Microsoft-Antispam-PRVS: X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: ikQPaXyDInJv56W5ndgRMLfXgnuJ3gvZuAE/Me+gHVYaocPhofVBfh8rHJXPVxrHKTvFQSvBiPWmxKPeFpCrTnarQ0K14K9UFbW7NqZ7sotecTydjFA3HnRHpwqjx0mjaR9Or50lcER6dtLTSCeRUwpLFCcdkszLu9E5Plgoe4lylwzw5TUGXJD7kZ99ptPbOJ+Vm4kx+TdWLNUzAX79dmEGsAMWzGaD9fansG8f1vt1zvalmEbOahquUhr+tYpB5jjP3zEm7TiXZKu3WLWz9xlIp/ME1VI5bI9XEdEOsoZEsqKLt1jk+vnyTPMOd0VthK67CxumIGRF6DCMOKNVfr4pOr4UyDcXp4Pz+iVJUS5Es2VLByo7DKDZo0tDKjGjALLCGnCqzii0Qe31njL7FOTx5oR7tvJepcLWsiipiXIPh6yOW93jbrSn/uSS3ZVoBeq5jg7Ep0k/cowIASoE5x7TWNwulnab1TAovSxMUCews+aNm4YB/mbcYG/XfJW/+OrG+4K/z3e91z5Z0tEVPGMtLaGmwRRIAYju+m/qGO6SstGgCP5OhBPwxEDT6XnqMwRj9dHfoKHkQnm0QktDHJNcX3Y9FlR0eufh5yLL3LQ9vjA/7W/kNzhnFsc0+hgwwFRy2MKq+T7if+/fEZb+HVU0FEOIw0zmPq32pwHARey1aOvhhBWJg/FZGXiFdpoHTq2lp9fw10iMbt/UdhdzAkm8ArhfgG+u7DtRnkWB7a4W0IRA9yWyKGQbPnsXFWxDHtYsFhfOFeKw0zrX9I+aiXOVvRYGf30eD6wllCbRIh0svNwoHnlOb7ADI3SJ3uYhTGwkFMembSerGSNQB7Ssgg== X-Forefront-Antispam-Report: CIP:149.199.80.198; CTRY:IE; LANG:en; SCL:1; SRV:; IPV:CAL; SFV:NSPM; H:xir-pvapexch01.xlnx.xilinx.com; PTR:unknown-80-198.xilinx.com; CAT:NONE; SFS:(13230001)(4636009)(40470700004)(46966006)(36840700001)(186003)(426003)(8936002)(8676002)(31696002)(47076005)(356005)(2906002)(7636003)(5660300002)(7416002)(44832011)(2616005)(82310400005)(36860700001)(26005)(110136005)(316002)(54906003)(966005)(508600001)(83380400001)(9786002)(36756003)(4326008)(70206006)(70586007)(31686004)(40460700003)(336012)(53546011)(50156003)(21314003)(43740500002); DIR:OUT; SFP:1101; X-OriginatorOrg: xilinx.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 May 2022 13:02:16.0416 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: c59abcc9-eab5-4736-09d3-08da423c9f44 X-MS-Exchange-CrossTenant-Id: 657af505-d5df-48d0-8300-c31994686c5c X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=657af505-d5df-48d0-8300-c31994686c5c; Ip=[149.199.80.198]; Helo=[xir-pvapexch01.xlnx.xilinx.com] X-MS-Exchange-CrossTenant-AuthSource: DM3NAM02FT032.eop-nam02.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR02MB8816 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 5/30/2022 1:32 PM, 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/30 18:43, Ferruh Yigit 写道: >> 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 >>>> --- >>>> 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); > Maybe we need to add a mapping table between RSS offload and name for > 'flow_type_rss_offloads'. >>>> +                     } >>>> +                     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[]'? > It seems that remove 'all' item isn't the way to resolve this issue. > There are other iterms in 'rss_type_table[], such as, 'ip', 'udp', > 'tcp' and so on. The 'rss_type_table[]' should primarily serve > the "show port rss xxx" and "port confg rss xxx" commands. > They all need these. > You are right, it is not just 'all'. >> >>> 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. > > We can unify this display if we add the mapping table mentioned above. > If it can be unified, agree that this simplifies the code [2]. Both above two changes already exists in your set [1], if you don't mind to adapt 2/3 & 3/3 of this set into yours, we can continue with your set, is this OK for you? [1] https://patches.dpdk.org/project/dpdk/list/?series=22735 [2] In your 'show_rss_types()', it can get additional parameter for formatting, like after how many items to break to next line, this can cover existing formatting options, but not sure how elegant a solution it is. >> >>>> + >>>> +                     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; >> >> .