[dpdk-dev] [PATCH] net/ixgbe: clear registers of all queues on VF reset

[Wang, Haiyue]

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit at intel.com>
> Sent: Tuesday, January 5, 2021 18:30
> To: Simon Ellmann <simon.ellmann at tum.de>; Wang, Haiyue <haiyue.wang at intel.com>; Guo, Jia
> <jia.guo at intel.com>
> Cc: dev at 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 <simon.ellmann at tum.de>
> >>>> Sent: Friday, December 18, 2020 01:15
> >>>> To: Guo, Jia <jia.guo at intel.com>; Wang, Haiyue <haiyue.wang at intel.com>
> >>>> Cc: dev at dpdk.org; Simon Ellmann <simon.ellmann at tum.de>
> >>>> 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 at dpdk.org
> >>>
> >>>> Signed-off-by: Simon Ellmann <simon.ellmann at tum.de>
> >>>> ---
> >>>>   drivers/net/ixgbe/base/ixgbe_vf.c | 2 +-
> >>>>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>
> >>> Good catch, thanks!
> >>>
> >>> Acked-by: Haiyue Wang <haiyue.wang at intel.com>
> >>>
> >>
> >> 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.