From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (xvm-189-124.dc0.ghst.net [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 4A8FBA09FF; Tue, 5 Jan 2021 15:10:28 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 386B216086C; Tue, 5 Jan 2021 15:10:28 +0100 (CET) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by mails.dpdk.org (Postfix) with ESMTP id B7E1516086B for ; Tue, 5 Jan 2021 15:10:26 +0100 (CET) IronPort-SDR: K0HPPS7FRmTe+GPA6rEvMiPG6ANKHEd5LbDkx/360umWVVR8xLs+cSHI7VD6niiraZ4Xf2cgc9 uaeWsIfGOuxg== X-IronPort-AV: E=McAfee;i="6000,8403,9854"; a="177210267" X-IronPort-AV: E=Sophos;i="5.78,477,1599548400"; d="scan'208";a="177210267" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Jan 2021 06:10:25 -0800 IronPort-SDR: faCMgR+/RfAHsYlETXAL3xX5+88e/nK2PXmkzfd4/7yIubpsq+G1+XfX0OcXxSQwFVlFj7QntJ WYPaSJTIV00Q== X-IronPort-AV: E=Sophos;i="5.78,477,1599548400"; d="scan'208";a="421786384" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.236.177]) ([10.213.236.177]) by orsmga001-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Jan 2021 06:10:24 -0800 To: "Wang, Haiyue" , Simon Ellmann , "Guo, Jia" Cc: "dev@dpdk.org" References: <20201217171452.24914-1-simon.ellmann@tum.de> <9b039080-a3a6-ca58-f0a7-2af4ce327585@tum.de> From: Ferruh Yigit Message-ID: <83f1dabd-ec60-5bac-0d56-4f3eedbe42c3@intel.com> Date: Tue, 5 Jan 2021 14:10:20 +0000 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: clear registers of all queues on VF reset 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" On 1/5/2021 12:52 PM, Wang, Haiyue wrote: >> -----Original Message----- >> From: Ferruh Yigit >> Sent: Tuesday, January 5, 2021 18:30 >> To: Simon Ellmann ; Wang, Haiyue ; Guo, Jia >> >> Cc: dev@dpdk.org >> Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: clear registers of all queues on VF reset >> >> On 1/5/2021 9:02 AM, Simon Ellmann wrote: >>> On 1/4/21 4:56 PM, Ferruh Yigit wrote: >>> >>>> On 12/18/2020 2:34 AM, Wang, Haiyue wrote: >>>>>> -----Original Message----- >>>>>> From: Simon Ellmann >>>>>> Sent: Friday, December 18, 2020 01:15 >>>>>> To: Guo, Jia ; Wang, Haiyue >>>>>> Cc: dev@dpdk.org; Simon Ellmann >>>>>> Subject: [PATCH] net/ixgbe: clear registers of all queues on VF reset >>>>>> >>>>>> ixgbe devices support up to 8 Rx and Tx queues per virtual function. >>>>>> Currently, the registers of only seven queues are set to default when >>>>>> resetting a VF. >>>>>> >>>>> >>>>> Fixes: d17d0b7a2407 ("ixgbe/base: reset VF registers") >>>>> Cc: stable@dpdk.org >>>>> >>>>>> Signed-off-by: Simon Ellmann >>>>>> --- >>>>>> drivers/net/ixgbe/base/ixgbe_vf.c | 2 +- >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>> >>>>> >>>>> Good catch, thanks! >>>>> >>>>> Acked-by: Haiyue Wang >>>>> >>>> >>>> This seems a very long lived defect, I am just suspicious if there was a >>>> reason to limit queue number to 7. >>>> >>>> >>>> Simon, >>>> >>>> How did you find the defect? And did you test/verify it with the update? >>>> (assuming you are using all 8 queues for the VF) >>> >>> Hi Ferruh, >>> >>> I was implementing ixgbevf for ixy.rs (https://ixy.rs/) by reading the code in >>> DPDK and Linux. While doing that I noticed that DPDK was only resetting 7 queues >>> which looked like a off-by-one error to me. I would have expected a comment if >>> this behaviour was intentional. I haven't checked the update. >>> >> >> Most probably you are right, but when I tried to test this, the HW I have >> doesn't let me set more than 4 queues for the VF, so I was a little suspicious >> about hardcoded 8 value. >> >> What do you think using 'hw->mac.max_rx_queues' instead? Which seems used a few >> other places to walk the queues? >> >> Also can you please point the equivalent code in the Linux driver? As far as I >> can see it also uses 'adapter->num_rx_queues', but I wonder if is there any >> hardcoded value there. >> > > I found the original detail log: > > ---- > ixgbevf: Clear VF registers we are required to configure anyway after VFLR > > This function resets just about any register that could "possibly leak" > any information from one VM instance to another. The registers set it > is returning to initial values is the same as those we are required > to reconfigure after a VFLR anyway. So this shouldn't affect any > driver that is doing what it is suppose to. > > ---- > > Also, the datasheet says it is '0 ... 7' > > Receive DMA Registers, > > 0x01000 + 0x40*n, n=0...7 VFRDBAL ... > > So it should be safe to uses '8' to reset the VF hardware BAR0 information. > The 'hw->mac.max_rx_queues' is about the software resource allocation in PF. > Thanks Haiuye for the clarification, so I will proceed with the patch.