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 130E6A0C47; Tue, 14 Sep 2021 09:20:42 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5E0C6410DC; Tue, 14 Sep 2021 09:20:41 +0200 (CEST) Received: from mail-wr1-f53.google.com (mail-wr1-f53.google.com [209.85.221.53]) by mails.dpdk.org (Postfix) with ESMTP id E13444003C for ; Tue, 14 Sep 2021 09:20:39 +0200 (CEST) Received: by mail-wr1-f53.google.com with SMTP id g16so18562545wrb.3 for ; Tue, 14 Sep 2021 00:20:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to; bh=qUcJpxwBaaIMCCFuq/g1JONHTj6OuNefGikeyFVihJc=; b=Q7dhoiuRIdqes0fFCAnQaxwHfEpuPfTXKE024VgH/2A9g4atavV/s2BKGkYN4r2ReL R3aKBluSMWlld3tPLLaUUdRB3fyjXHDH+PSFGqVImmzrf6bOWPDRfnJK6aDze+bLIlOd 41bZTxOnDfnDztOLa9p/NQ5E14U8NX76hkWS2WvvfytfxQeHaVv8BAlLzAf8wnVKcOsk yVUHnENyTNz/x2YUHQiK4ix3fRICwEHRjyXLgkdIieuhAm/l3PINqRatu7u2j831eDaJ AJxisS2eqKrT0+giNcg9RRwWz/0jcaYjV3BIE13fbgr46jvVTX45yx7hAZil+Y2RqHbx PHZA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to; bh=qUcJpxwBaaIMCCFuq/g1JONHTj6OuNefGikeyFVihJc=; b=oXRA8Y6XdI0a3kePUAIkEMayJDKr+iXotqbQssk17vJCh1vp9q0CPO/gBFRWnlJWLC kpFySU/Emq87F7LfKPDGKyVoI3vVbmblhpdVHeDc7Mmiis7P1NblmidWjJoWsDzWo2Fo g3AuSAnPM/A3tZFCHr3w9bLEqIl8W/xx/Vp2seCVIOh0fnceXsFTDLXDBpQj39z8Irz2 TgN1K8CXH27cR+oj3RZqjCkkuCqKwSg6PlybLcqSnDj1HddVZ+7br6FF0q93gINLTwZh znPivfvkoJDj2/BRAB9cEj1muLS40O71pr2PNNrZ9hErxSjlcDgvsKo96dGJTyKyNsx0 xaQQ== X-Gm-Message-State: AOAM5312M86mhJ3RB7A1eRjjgVwWJYXi2YahUtkDzl2ngaWCfENa0ycm evKxDOjNgC9HaOtlTYVj5Lfn X-Google-Smtp-Source: ABdhPJykpKz4Fz06SQA3vLO5DeGQI0oZIFyktRh7RazW78HQrJYYRZRCgdbtN2VJUihKZI5WlgP7aA== X-Received: by 2002:adf:f683:: with SMTP id v3mr13086721wrp.423.1631604039552; Tue, 14 Sep 2021 00:20:39 -0700 (PDT) Received: from acdc.dev.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id y6sm2085723wrp.46.2021.09.14.00.20.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 14 Sep 2021 00:20:38 -0700 (PDT) Date: Tue, 14 Sep 2021 09:20:14 +0200 From: =?iso-8859-1?Q?N=E9lio?= Laranjeiro To: Maxime Coquelin , ferruh.yigit@intel.com Cc: Andrew Rybchenko , dev@dpdk.org, chenbo.xia@intel.com, amorenoz@redhat.com, david.marchand@redhat.com, michaelba@nvidia.com, viacheslavo@nvidia.com, stable@dpdk.org, Shahaf Shuler Message-ID: References: <20210910091734.7023-1-maxime.coquelin@redhat.com> <20210910091734.7023-3-maxime.coquelin@redhat.com> <7548b2d2-fd23-4a1a-59d7-b3424912afcc@redhat.com> <85036fee-62f0-5d61-b451-65ee428ee2bc@oktetlabs.ru> <1daeb2f2-2122-c546-3eb4-f0d7fba035f0@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1daeb2f2-2122-c546-3eb4-f0d7fba035f0@redhat.com> Subject: Re: [dpdk-dev] [PATCH 2/3] app/testpmd: fix RSS hash type update 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 Sender: "dev" +Shahaf, Hi Maxime, On Mon, Sep 13, 2021 at 11:41:04AM +0200, Maxime Coquelin wrote: > Hi Nélio, > > On 9/10/21 4:16 PM, Nélio Laranjeiro wrote: > > On Fri, Sep 10, 2021 at 01:06:53PM +0300, Andrew Rybchenko wrote: > >> On 9/10/21 12:57 PM, Maxime Coquelin wrote: > >>> > >>> > >>> On 9/10/21 11:51 AM, Andrew Rybchenko wrote: > >>>> On 9/10/21 12:17 PM, Maxime Coquelin wrote: > >>>>> port_rss_hash_key_update() initializes rss_conf with the > >>>>> RSS hash type and key provided by the user, but it calls > >>>>> rte_eth_dev_rss_hash_conf_get() before calling > >>>>> rte_eth_dev_rss_hash_update(), which overides the parsed > >>>>> config with current NIC's config. > >>>>> > >>>>> While the RSS key value is set again after, this is not > >>>>> the case of the key length and the type of hash. > >>>>> > >>>>> There is no need to read the RSS config from the NIC, let's > >>>>> just try to set the user defined one. > >>>>> > >>>>> Fixes: 8205e241b2b0 ("app/testpmd: add missing type to RSS hash commands") > >>>>> Cc: stable@dpdk.org > >>>>> Cc: nelio.laranjeiro@6wind.com > >>>>> > >>>>> Signed-off-by: Maxime Coquelin > >>>>> --- > >>>>> app/test-pmd/config.c | 8 ++------ > >>>>> 1 file changed, 2 insertions(+), 6 deletions(-) > >>>>> > >>>>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c > >>>>> index 31d8ba1b91..451bda53b1 100644 > >>>>> --- a/app/test-pmd/config.c > >>>>> +++ b/app/test-pmd/config.c > >>>>> @@ -2853,18 +2853,14 @@ port_rss_hash_key_update(portid_t port_id, char rss_type[], uint8_t *hash_key, > >>>>> int diag; > >>>>> unsigned int i; > >>>>> > >>>>> - rss_conf.rss_key = NULL; > >>>>> + rss_conf.rss_key = hash_key; > >>>>> rss_conf.rss_key_len = hash_key_len; > >>>>> 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; > >>>>> } > >>>>> - diag = rte_eth_dev_rss_hash_conf_get(port_id, &rss_conf); > >>>>> - if (diag == 0) { > >>>>> - rss_conf.rss_key = hash_key; > >>>>> - diag = rte_eth_dev_rss_hash_update(port_id, &rss_conf); > >>>>> - } > >>>>> + diag = rte_eth_dev_rss_hash_update(port_id, &rss_conf); > >>>> > >>>> I'm not 100% sure, but I'd say the intent above could be > >>>> to update key only as the function name says. I.e. keep > >>>> rss_hf as is. That could be the reason to get first. > > > > True, > > > >>> I think that was the intial purpose of the command, but patch > >>> 8205e241b2b0 added setting the hash type as mandatory. There are > >>> no other command to configure the hash type from testpmd AFAICT. > > > > Also for the same initial purpose, some NIC have an hash key per > > protocol, by default it uses the same key for all of them but it can be > > configured individually making for example key0 for all protocols expect > > IPv4 which uses key1. > > Thanks for the info, I have looked at most drivers but didn't found one > that support this feature, could you give some pointer? Well, I have done the modification at that time for MLX5 PMD, since I left DPDK in 2018 I don't know if they still support such configuration from this API or if they fully moved to rte_flow. > Given how the drivers implément the callback, do you agree with the fix, > or do you have something else in mind? I cannot answer if this get() is mandatory, this predates my arrival on DPDK (original commit written in 2014), looking at DPDK state on commit f79959ea1504 ("app/testpmd: allow to configure RSS hash key"). Maybe someone from Intel can help, eventually you can contact PMD maintainers to review this patch. Regards, Nélio > Thanks, > Maxime > > >>> Also, even without 8205e241b2b0, the function was broken because the > >>> key length was overiden. > >> > >> I see, many thanks for explanations. > > > -- Nélio Laranjeiro 6WIND