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 A607DA04BC; Fri, 9 Oct 2020 20:36:55 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 83DF91B7C0; Fri, 9 Oct 2020 20:36:54 +0200 (CEST) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id DF8DF1B733 for ; Fri, 9 Oct 2020 20:36:52 +0200 (CEST) IronPort-SDR: TCdR0ffybMYkAeCe2dkHN9/Svl6JsH4wUmzOzD8F9pOa2zx3w7x8C68xjGm53IU7FLepMYqvss mQIfJzaOcNEg== X-IronPort-AV: E=McAfee;i="6000,8403,9769"; a="165585398" X-IronPort-AV: E=Sophos;i="5.77,355,1596524400"; d="scan'208";a="165585398" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Oct 2020 11:36:50 -0700 IronPort-SDR: RguNYE3zKdeOrTZkPW+bXLUDpo/SxGSTusvElf4Qq2pLLm040LVZO8jjFFIx40ZuuXEh7TvElM E5yDWzT5avcA== X-IronPort-AV: E=Sophos;i="5.77,355,1596524400"; d="scan'208";a="462284517" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.252.18.7]) ([10.252.18.7]) by orsmga004-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Oct 2020 11:36:49 -0700 To: oulijun , wenzhuo.lu@intel.com, beilei.xing@intel.com, adrien.mazarguil@6wind.com Cc: dev@dpdk.org, linuxarm@huawei.com References: <1599702678-11142-1-git-send-email-oulijun@huawei.com> <1600955105-53176-1-git-send-email-oulijun@huawei.com> <1600955105-53176-2-git-send-email-oulijun@huawei.com> <5ca8988c-741f-d9e2-3f53-2c97fe64ee5b@huawei.com> From: Ferruh Yigit Message-ID: <9c4bba47-cedc-ce6d-02f2-053b4fa8ad8c@intel.com> Date: Fri, 9 Oct 2020 19:36:46 +0100 MIME-Version: 1.0 In-Reply-To: <5ca8988c-741f-d9e2-3f53-2c97fe64ee5b@huawei.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v4] app/testpmd: fix the default RSS key configuration 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 10/9/2020 12:59 PM, oulijun wrote: > > > 在 2020/9/30 21:36, Ferruh Yigit 写道: >> On 9/24/2020 2:45 PM, Lijun Ou wrote: >>> It 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. >>> >>> Fixes: ac8d22de2394 ("ethdev: flatten RSS configuration in flow API") >>> Cc: stable@dpdk.org >>> >>> Signed-off-by: Lijun Ou >>> --- >>> V3->V4: >>> -fix checkpatch warning and shorter commit content. >>> >>> 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; >> >> Do you need this last assignment at all? >> 'rss_conf.rss_key' point to 'action_rss_data->key' >> 'action_rss_data->conf.key' already point to 'action_rss_data->key' >> so it just assigns same value back, no? >> > You need to call rte_eth_dev_rss_hash_conf_get to obtain the RSS hash key > configured in the hardware as the default key. > Currently, the testpmd uses a key that is different from that configured in the > hardware after the testpmd is started as the default key when users crearte a > RSS flow rule without specifying a RSS hash key. > I got this part, I am saying why you have the following assignment: "action_rss_data->conf.key = rss_conf.rss_key;" 'action_rss_data->conf.key' already points to 'action_rss_data->key' [1] later 'rss_conf.rss_key' pointer points 'action_rss_data->key' [2] This is used to fill the RSS key. At this stage both 'action_rss_data->conf.key' & 'rss_conf.rss_key' are point to exact same address. So what is the point of assigning 'action_rss_data->conf.key' to 'rss_conf.rss_key'? [1] *action_rss_data = (struct action_rss_data){ conf = (struct rte_flow_action_rss){ ... .key = action_rss_data->key, ... } ... } [2] rss_conf.rss_key = action_rss_data->key;