From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 12868A00C5; Wed, 6 May 2020 18:37:52 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 526311DA21; Wed, 6 May 2020 18:37:51 +0200 (CEST) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 7C3DF1D6BC for ; Wed, 6 May 2020 18:37:49 +0200 (CEST) IronPort-SDR: NZnVRH3ilXc0LF+lsLEIb3riClH1ERqhqUFQyoYdvS31y2Td0XS6l/VjBDugzAL3gK1cijI2aF m8kBeWPVx9Rg== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 May 2020 09:37:48 -0700 IronPort-SDR: 9TFoV6y7wn0ushu/xSH7bPi0ehiOTtfN7LWFR3excLvqMrSNi71GFJHIyeZYs/f3s1FNrFeSC0 LjPhQIwB+/aQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.73,360,1583222400"; d="scan'208";a="461497292" Received: from fyigit-mobl.ger.corp.intel.com (HELO [10.213.197.179]) ([10.213.197.179]) by fmsmga005.fm.intel.com with ESMTP; 06 May 2020 09:37:45 -0700 To: Xiaoyu Min Cc: Wenzhuo Lu , Jingjing Wu , Bernard Iremonger , John McNamara , Marko Kovacevic , dev@dpdk.org References: <127483b3dbf4840a80c6177a6230d1f3e8f40fb2.1588251701.git.jackmin@mellanox.com> <8a562aef-6d7b-4713-7fb9-e4da12d6bf56@intel.com> <20200506025247.psjk3deaitbwa6pd@mellanox.com> <20200506160405.muh4g4lagbocqirj@mellanox.com> From: Ferruh Yigit Autocrypt: addr=ferruh.yigit@intel.com; prefer-encrypt=mutual; keydata= mQINBFXZCFABEADCujshBOAaqPZpwShdkzkyGpJ15lmxiSr3jVMqOtQS/sB3FYLT0/d3+bvy qbL9YnlbPyRvZfnP3pXiKwkRoR1RJwEo2BOf6hxdzTmLRtGtwWzI9MwrUPj6n/ldiD58VAGQ +iR1I/z9UBUN/ZMksElA2D7Jgg7vZ78iKwNnd+vLBD6I61kVrZ45Vjo3r+pPOByUBXOUlxp9 GWEKKIrJ4eogqkVNSixN16VYK7xR+5OUkBYUO+sE6etSxCr7BahMPKxH+XPlZZjKrxciaWQb +dElz3Ab4Opl+ZT/bK2huX+W+NJBEBVzjTkhjSTjcyRdxvS1gwWRuXqAml/sh+KQjPV1PPHF YK5LcqLkle+OKTCa82OvUb7cr+ALxATIZXQkgmn+zFT8UzSS3aiBBohg3BtbTIWy51jNlYdy ezUZ4UxKSsFuUTPt+JjHQBvF7WKbmNGS3fCid5Iag4tWOfZoqiCNzxApkVugltxoc6rG2TyX CmI2rP0mQ0GOsGXA3+3c1MCdQFzdIn/5tLBZyKy4F54UFo35eOX8/g7OaE+xrgY/4bZjpxC1 1pd66AAtKb3aNXpHvIfkVV6NYloo52H+FUE5ZDPNCGD0/btFGPWmWRmkPybzColTy7fmPaGz cBcEEqHK4T0aY4UJmE7Ylvg255Kz7s6wGZe6IR3N0cKNv++O7QARAQABtCVGZXJydWggWWln aXQgPGZlcnJ1aC55aWdpdEBpbnRlbC5jb20+iQJsBBMBCgBWAhsDAh4BAheABQsJCAcDBRUK CQgLBRYCAwEABQkKqZZ8FiEE0jZTh0IuwoTjmYHH+TPrQ98TYR8FAl6ha3sXGHZrczovL2tl eXMub3BlbnBncC5vcmcACgkQ+TPrQ98TYR8uLA//QwltuFliUWe60xwmu9sY38c1DXvX67wk UryQ1WijVdIoj4H8cf/s2KtyIBjc89R254KMEfJDao/LrXqJ69KyGKXFhFPlF3VmFLsN4XiT PSfxkx8s6kHVaB3O183p4xAqnnl/ql8nJ5ph9HuwdL8CyO5/7dC/MjZ/mc4NGq5O9zk3YRGO lvdZAp5HW9VKW4iynvy7rl3tKyEqaAE62MbGyfJDH3C/nV/4+mPc8Av5rRH2hV+DBQourwuC ci6noiDP6GCNQqTh1FHYvXaN4GPMHD9DX6LtT8Fc5mL/V9i9kEVikPohlI0WJqhE+vQHFzR2 1q5nznE+pweYsBi3LXIMYpmha9oJh03dJOdKAEhkfBr6n8BWkWQMMiwfdzg20JX0o7a/iF8H 4dshBs+dXdIKzPfJhMjHxLDFNPNH8zRQkB02JceY9ESEah3wAbzTwz+e/9qQ5OyDTQjKkVOo cxC2U7CqeNt0JZi0tmuzIWrfxjAUulVhBmnceqyMOzGpSCQIkvalb6+eXsC9V1DZ4zsHZ2Mx Hi+7pCksdraXUhKdg5bOVCt8XFmx1MX4AoV3GWy6mZ4eMMvJN2hjXcrreQgG25BdCdcxKgqp e9cMbCtF+RZax8U6LkAWueJJ1QXrav1Jk5SnG8/5xANQoBQKGz+yFiWcgEs9Tpxth15o2v59 gXK5Ag0EV9ZMvgEQAKc0Db17xNqtSwEvmfp4tkddwW9XA0tWWKtY4KUdd/jijYqc3fDD54ES YpV8QWj0xK4YM0dLxnDU2IYxjEshSB1TqAatVWz9WtBYvzalsyTqMKP3w34FciuL7orXP4Ai bPtrHuIXWQOBECcVZTTOdZYGAzaYzxiAONzF9eTiwIqe9/oaOjTwTLnOarHt16QApTYQSnxD UQljeNvKYt1lZE/gAUUxNLWsYyTT+22/vU0GDUahsJxs1+f1yEr+OGrFiEAmqrzpF0lCS3f/ 3HVTU6rS9cK3glVUeaTF4+1SK5ZNO35piVQCwphmxa+dwTG/DvvHYCtgOZorTJ+OHfvCnSVj sM4kcXGjJPy3JZmUtyL9UxEbYlrffGPQI3gLXIGD5AN5XdAXFCjjaID/KR1c9RHd7Oaw0Pdc q9UtMLgM1vdX8RlDuMGPrj5sQrRVbgYHfVU/TQCk1C9KhzOwg4Ap2T3tE1umY/DqrXQgsgH7 1PXFucVjOyHMYXXugLT8YQ0gcBPHy9mZqw5mgOI5lCl6d4uCcUT0l/OEtPG/rA1lxz8ctdFB VOQOxCvwRG2QCgcJ/UTn5vlivul+cThi6ERPvjqjblLncQtRg8izj2qgmwQkvfj+h7Ex88bI 8iWtu5+I3K3LmNz/UxHBSWEmUnkg4fJlRr7oItHsZ0ia6wWQ8lQnABEBAAGJAjwEGAEKACYC GwwWIQTSNlOHQi7ChOOZgcf5M+tD3xNhHwUCXqFrngUJCKxSYAAKCRD5M+tD3xNhH3YWD/9b cUiWaHJasX+OpiuZ1Li5GG3m9aw4lR/k2lET0UPRer2Jy1JsL+uqzdkxGvPqzFTBXgx/6Byz EMa2mt6R9BCyR286s3lxVS5Bgr5JGB3EkpPcoJT3A7QOYMV95jBiiJTy78Qdzi5LrIu4tW6H o0MWUjpjdbR01cnj6EagKrDx9kAsqQTfvz4ff5JIFyKSKEHQMaz1YGHyCWhsTwqONhs0G7V2 0taQS1bGiaWND0dIBJ/u0pU998XZhmMzn765H+/MqXsyDXwoHv1rcaX/kcZIcN3sLUVcbdxA WHXOktGTQemQfEpCNuf2jeeJlp8sHmAQmV3dLS1R49h0q7hH4qOPEIvXjQebJGs5W7s2vxbA 5u5nLujmMkkfg1XHsds0u7Zdp2n200VC4GQf8vsUp6CSMgjedHeF9zKv1W4lYXpHp576ZV7T GgsEsvveAE1xvHnpV9d7ZehPuZfYlP4qgo2iutA1c0AXZLn5LPcDBgZ+KQZTzm05RU1gkx7n gL9CdTzVrYFy7Y5R+TrE9HFUnsaXaGsJwOB/emByGPQEKrupz8CZFi9pkqPuAPwjN6Wonokv ChAewHXPUadcJmCTj78Oeg9uXR6yjpxyFjx3vdijQIYgi5TEGpeTQBymLANOYxYWYOjXk+ae dYuOYKR9nbPv+2zK9pwwQ2NXbUBystaGyQ== Message-ID: <4332b96e-d883-1529-4cf9-15d112589ca2@intel.com> Date: Wed, 6 May 2020 17:37:44 +0100 MIME-Version: 1.0 In-Reply-To: <20200506160405.muh4g4lagbocqirj@mellanox.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v2] app/testpmd: add cmdline option to set Rx mq mode X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 5/6/2020 5:04 PM, Xiaoyu Min wrote: > On Wed, 20-05-06, 10:36, Ferruh Yigit wrote: >> On 5/6/2020 3:52 AM, Xiaoyu Min wrote: >>> On Tue, 20-05-05, 17:06, Ferruh Yigit wrote: >>>> On 4/30/2020 2:07 PM, Xiaoyu Min wrote: >>>>> One new cmdline option `--rx-mq-mode` is added in order to have the >>>>> possibility to check whether PMD handle the mq mode correctly or not. >>>>> >>>>> The reason is some NICs need to do different settings based on different >>>>> RX mq mode, i.e RSS or not. >>>>> >>>>> With this support in testpmd, the above scenario can be tested easily. >>>>> >>>>> Signed-off-by: Xiaoyu Min >>>>> --- >>>>> app/test-pmd/parameters.c | 12 ++++++++++++ >>>>> app/test-pmd/testpmd.c | 17 ++++++++++++++--- >>>>> app/test-pmd/testpmd.h | 3 +++ >>>>> doc/guides/rel_notes/release_20_05.rst | 4 ++++ >>>>> doc/guides/testpmd_app_ug/run_app.rst | 7 +++++++ >>>>> 5 files changed, 40 insertions(+), 3 deletions(-) >>>>> >>>>> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c >>>>> index 30c1753c32..a9dd58e96b 100644 >>>>> --- a/app/test-pmd/parameters.c >>>>> +++ b/app/test-pmd/parameters.c >>>>> @@ -212,6 +212,7 @@ usage(char* progname) >>>>> printf(" --noisy-lkup-num-writes=N: do N random reads and writes per packet\n"); >>>>> printf(" --no-iova-contig: mempool memory can be IOVA non contiguous. " >>>>> "valid only with --mp-alloc=anon\n"); >>>>> + printf(" --rx-mq-mode=0xX: hexadecimal bitmask of RX mq mode\n"); >>>> >>>> Do you think does it worth to say the bitmask is for modes that can be enabled, >>>> to remove need to look the code, not sure. >>> So you mean something like: >>> + printf(" --rx-mq-mode=0xX: hexadecimal bitmask of RX mq mode can be enabled\n"); >>> Is it right? >> >> Yes. >> >>> >>>> >>>>> } >>>>> >>>>> #ifdef RTE_LIBRTE_CMDLINE >>>>> @@ -670,6 +671,7 @@ launch_args_parse(int argc, char** argv) >>>>> { "noisy-lkup-num-reads", 1, 0, 0 }, >>>>> { "noisy-lkup-num-reads-writes", 1, 0, 0 }, >>>>> { "no-iova-contig", 0, 0, 0 }, >>>>> + { "rx-mq-mode", 1, 0, 0 }, >>>>> { 0, 0, 0, 0 }, >>>>> }; >>>>> >>>>> @@ -1363,6 +1365,16 @@ launch_args_parse(int argc, char** argv) >>>>> } >>>>> if (!strcmp(lgopts[opt_idx].name, "no-iova-contig")) >>>>> mempool_flags = MEMPOOL_F_NO_IOVA_CONTIG; >>>>> + >>>>> + if (!strcmp(lgopts[opt_idx].name, "rx-mq-mode")) { >>>>> + char *end = NULL; >>>>> + n = strtoul(optarg, &end, 16); >>>>> + if (n >= 0) >>>>> + rx_mq_mode = (enum rte_eth_rx_mq_mode)n; >>>> >>>> Should we check if the provided value, 'n', is not out of the enum range? >>> OK, I'll add the check. >>> >>>> >>>>> + else >>>>> + rte_exit(EXIT_FAILURE, >>>>> + "rx-mq-mode must be >= 0\n"); >>>>> + } >>>>> break; >>>>> case 'h': >>>>> usage(argv[0]); >>>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c >>>>> index 99bacddbfd..9536d6ee27 100644 >>>>> --- a/app/test-pmd/testpmd.c >>>>> +++ b/app/test-pmd/testpmd.c >>>>> @@ -482,6 +482,11 @@ uint8_t bitrate_enabled; >>>>> struct gro_status gro_ports[RTE_MAX_ETHPORTS]; >>>>> uint8_t gro_flush_cycles = GRO_DEFAULT_FLUSH_CYCLES; >>>>> >>>>> +/* >>>>> + * RX mq mode value set in the commandline >>>> >>>> This is not the "RX mq mode value", the above help string seems more accurate, >>>> "hexadecimal bitmask of RX mq mode". Can you please update here? >>> Sure, I'll update. >>> >>>> >>>>> + */ >>>>> +enum rte_eth_rx_mq_mode rx_mq_mode = ETH_MQ_RX_VMDQ_DCB_RSS; >>>>> + >>>>> /* Forward function declarations */ >>>>> static void setup_attached_port(portid_t pi); >>>>> static void map_port_queue_stats_mapping_registers(portid_t pi, >>>>> @@ -3337,7 +3342,9 @@ init_port_config(void) >>>>> >>>>> if (port->dcb_flag == 0) { >>>>> if( port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0) >>>>> - port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_RSS; >>>>> + port->dev_conf.rxmode.mq_mode = >>>>> + (enum rte_eth_rx_mq_mode) >>>> >>>> Do we need this enum type cast? >>> Yes, we need this cast otherwise the coverity will complain about enum type >>> mixed with other type. >> >> Here all variables looks an enum, 'mq_mode' type is enum, 'rx_mq_mode' is enum >> and 'ETH_MQ_RX_RSS' is enum item. With which type it is mixed with? >> >> I didn't run coverity for it, if it is giving warning OK to keep them, but can >> you please double check with coverity scan? >> > As you asked, I double checked with coverity by removing the enum cast. The coverity > has the below warning [1] but this time the warning is supressed by filter(I guess > it's newly added). It seems coverity doesn't allow the bitwise operators(?) > Anyhow I prefer to keep the cast this time since it's already there. > What do you think? OK, thanks for re-checking. > > [1]: > [2020-05-06T10:31:01.899Z] Detected 1 defect occurrence that passes the filter criteria. > [2020-05-06T10:31:01.899Z] There are 267 suppressed defects due to filters. > [2020-05-06T10:31:01.900Z] > [2020-05-06T10:31:01.900Z] app/test-pmd/testpmd.c:3345: CID 154009 (#1 of 1): > [2020-05-06T10:31:01.901Z] Type: Parse warning (PW.MIXED_ENUM_TYPE) > [2020-05-06T10:31:01.901Z] Classification: Unclassified > [2020-05-06T10:31:01.901Z] Severity: Unspecified > [2020-05-06T10:31:01.902Z] Action: Undecided > [2020-05-06T10:31:01.902Z] Owner: Unassigned > [2020-05-06T10:31:01.903Z] Defect only exists locally. > [2020-05-06T10:31:01.903Z] app/test-pmd/testpmd.c:3345: > [2020-05-06T10:31:01.903Z] 1. mixed_enum_type: enumerated type mixed with another type >>> >>>> >>>>> + (rx_mq_mode & ETH_MQ_RX_RSS); >>>>> else >>>>> port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_NONE; >>>>> } >>>>> @@ -3438,7 +3445,9 @@ get_eth_dcb_conf(portid_t pid, struct rte_eth_conf *eth_conf, >>>>> } >>>>> >>>>> /* set DCB mode of RX and TX of multiple queues */ >>>>> - eth_conf->rxmode.mq_mode = ETH_MQ_RX_VMDQ_DCB; >>>>> + eth_conf->rxmode.mq_mode = >>>>> + (enum rte_eth_rx_mq_mode) >>>>> + (rx_mq_mode & ETH_MQ_RX_VMDQ_DCB); >>>>> eth_conf->txmode.mq_mode = ETH_MQ_TX_VMDQ_DCB; >>>>> } else { >>>>> struct rte_eth_dcb_rx_conf *rx_conf = >>>>> @@ -3458,7 +3467,9 @@ get_eth_dcb_conf(portid_t pid, struct rte_eth_conf *eth_conf, >>>>> tx_conf->dcb_tc[i] = i % num_tcs; >>>>> } >>>>> >>>>> - eth_conf->rxmode.mq_mode = ETH_MQ_RX_DCB_RSS; >>>>> + eth_conf->rxmode.mq_mode = >>>>> + (enum rte_eth_rx_mq_mode) >>>>> + (rx_mq_mode & ETH_MQ_RX_DCB_RSS); >>>>> eth_conf->rx_adv_conf.rss_conf = rss_conf; >>>>> eth_conf->txmode.mq_mode = ETH_MQ_TX_DCB; >>>>> } >>>>> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h >>>>> index 7ff4c5dba3..32bb324c94 100644 >>>>> --- a/app/test-pmd/testpmd.h >>>>> +++ b/app/test-pmd/testpmd.h >>>>> @@ -602,6 +602,9 @@ struct mplsoudp_decap_conf { >>>>> }; >>>>> extern struct mplsoudp_decap_conf mplsoudp_decap_conf; >>>>> >>>>> +/* RX mq mode parameter. */ >>>> >>>> The variable name gives as much context as the comment, may it be dropped? >>> OK, I'll dorp this comment. >>> >>>> >>>>> +extern enum rte_eth_rx_mq_mode rx_mq_mode; >>>>> + >>>>> static inline unsigned int >>>>> lcore_num(void) >>>>> { >>>>> diff --git a/doc/guides/rel_notes/release_20_05.rst b/doc/guides/rel_notes/release_20_05.rst >>>>> index b124c3f287..8026afb451 100644 >>>>> --- a/doc/guides/rel_notes/release_20_05.rst >>>>> +++ b/doc/guides/rel_notes/release_20_05.rst >>>>> @@ -212,6 +212,10 @@ New Features >>>>> * Added IPsec inbound load-distribution support for ipsec-secgw application >>>>> using NIC load distribution feature(Flow Director). >>>>> >>>>> +* **Updated testpmd application.** >>>>> + >>>>> + * Added a new cmdline option ``--rx-mq-mode`` which can be used to test PMD's >>>>> + behaviour on handling Rx mq mode. >>>>> >>>>> Removed Items >>>>> ------------- >>>>> diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst >>>>> index 727ef52b8f..4f46299e68 100644 >>>>> --- a/doc/guides/testpmd_app_ug/run_app.rst >>>>> +++ b/doc/guides/testpmd_app_ug/run_app.rst >>>>> @@ -481,3 +481,10 @@ The command line options are: >>>>> >>>>> Enable to create mempool which is not IOVA contiguous. Valid only with --mp-alloc=anon. >>>>> The default value is 0. >>>>> + >>>>> +* ``--rx-mq-mode`` >>>>> + >>>>> + Set the hexadecimal bitmask of RX queue mq mode. >>>> >>>> It is good to expand the 'mq' at least in the documentation, and I guess it is >>>> "multi queue". >>> OK, I'll expand it. >>> >>> Thank you >>> -Jack >>>> >>>>> + The default value is 0x7:: >>>>> + >>>>> + ETH_MQ_RX_RSS_FLAG | ETH_MQ_RX_DCB_FLAG | ETH_MQ_RX_VMDQ_FLAG >>>>> >>>> >>