Thread (8 messages) 8 messages, 4 authors, 2021-01-05

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

From: Ferruh Yigit <hidden>
Date: 2021-01-05 14:10:29

On 1/5/2021 12:52 PM, Wang, Haiyue wrote:
quoted
-----Original Message-----
From: Ferruh Yigit <redacted>
Sent: Tuesday, January 5, 2021 18:30
To: Simon Ellmann <redacted>; Wang, Haiyue <redacted>; Guo, Jia
[off-list ref]
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:
quoted
On 1/4/21 4:56 PM, Ferruh Yigit wrote:
quoted
On 12/18/2020 2:34 AM, Wang, Haiyue wrote:
quoted
quoted
-----Original Message-----
From: Simon Ellmann <redacted>
Sent: Friday, December 18, 2020 01:15
To: Guo, Jia <redacted>; Wang, Haiyue <redacted>
Cc: dev@dpdk.org; Simon Ellmann <redacted>
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
quoted
Signed-off-by: Simon Ellmann <redacted>
---
   drivers/net/ixgbe/base/ixgbe_vf.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
Good catch, thanks!

Acked-by: Haiyue Wang <redacted>
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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help