On 03/26/15 17:59, Alexander Duyck wrote:

On 03/26/2015 08:10 AM, Vlad Zolotarov wrote:

quoted

On 03/26/15 16:40, Alexander Duyck wrote:

quoted

On 03/26/2015 02:35 AM, Vlad Zolotarov wrote:

quoted

On 03/25/15 23:04, Alexander Duyck wrote:

quoted

On 03/25/2015 01:17 PM, Vlad Zolotarov wrote:

quoted

On 03/25/15 20:35, Tantilov, Emil S wrote:

quoted

quoted

-----Original Message-----
From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
Sent: Wednesday, March 25, 2015 2:28 AM
Subject: Re: [PATCH net-next v6 4/7] ixgbevf: Add a RETA query 
code

<snip>

quoted

quoted

quoted

quoted

quoted

The user should actually not query the indirection table and a 
hash key too often. And if he/she does - it should be his/her 
problem.
However, if like with the ixgbevf_set_num_queues() u insist on 
your way of doing this (on caching the indirection table and hash 
key) - then please let me know and I will add it. Because, 
frankly, I care about the PF part of this series much more than 
for the VF part... ;)

I would say you don't need to cache it, but for 82599 and x540 
there isn't any need to store more than 3 bits per entry, 384b, or 
12 DWORDs for the entire RETA of the VF since the hardware can 
support at most 8 queues w/ SR-IOV. Then you only need one message 
instead of 3 which will reduce quite a bit of the complication 
with all of this.

Also it might make more sense to start working on displaying this 
on the PF before you start trying to do this on the VF. As far as 
I know ixgbe still doesn't have this functionality and it would 
make much more sense to enable that first on ixgbe before you 
start trying to find a way to feed the data to the VF.

Let's agree on the next steps:

1. I'll reduce the series scope to 82599 and x540 devices.

For mailbox implementation that is okay, my advice for the x550 
would be to just read the registers.  They should already be defined 
in the driver so it is just a matter of reversing the process for 
writing the RETA.  You should have a solid grasp of that once you 
implement the solution for the PF.

The problem is that I have no means to validate the solution - I have 
only x540 devices (nor specs). I'd prefer not to work in darkness and 
let u, guys, do it...

If it helps the specs for the x540 can be found at 
http://www.intel.com/content/www/us/en/network-adapters/10-gigabit-network-adapters/ethernet-x540-datasheet.html. 
Section 8 has a full list of the register definitions.

All the code you need should already be stored in ixgbe_setup_reta and 
ixgbe_setup_vfreta.  If you do it right you shouldn't even need to 
read the hardware.  Odds are you could generate it in software and 
that is what you would be accessing on the PF. 

I have no problems implementing the PF x550 part. I would need the 
proper spec to implement the VF part though. And since I can't have it 
now I'd rather not to it at the moment... ;)


Sure. This is what I have already implemented. The same is for RSS hash 
key.

After all, if at some point you reset the hardware you would need to 
restore it from software wouldn't you, and since the table is static 
for now you should be able to calculate the value without need of 
reading a register.  If at some point in the future you can change the 
table then it means a copy has to be maintained in kernel memory and 
at that point you could use that for the data you would be sending to 
the VF.

Right.

quoted

quoted

quoted

4. I won't implement the caching of the indirection and RSS hash key
   query results in the VF level.

The idea with caching is to keep the number of messages across the 
mailbox to a minimum.  Compressing it down to 3 bits per entry will 
help some, but I am not sure if that is enough to completely 
mitigate the need for caching.  For now though the caching isn't a 
must-have, and it won't be needed for x550 since it can access the 
table directly.  The main thing I would recommend keeping an eye on 
would be how long it will take to complete the messages necessary to 
get the ethtool info.  If there are any scenarios where things take 
more than 1 second then I would say we are taking too long and we 
would have to look into caching because we want to avoid holding the 
rtnl lock for any extended period of time.

I've described the limitations the caching approach imposes above. I 
suggest we follow your approach here - solve the problems when they 
arrive. ;) I haven't succeeded to make it take as long as even near 
one second.

quoted

My advice is get the PF patches in first, then we can look at trying 
to get feature parity on the VF.

"feature parity on the VF"?

Get "ethtool -x/X" working on the PF, and then get it working on the 
VF.  That way it makes for very simple verification as you can inspect 
that the PF and VF tables are consistent.  As I said if nothing else 
you can probably take a look at the igb driver to see how this was 
done on the 1Gbps Intel hardware.

If memory serves me well I've written the ethtool -x|-X implementation 
for bnx2x once so I have a general idea of what there should be. ;)
Making ethtool -X work is a bit more sensitive since a simplistic 
implementation like u have in igb may lead to a momentary out-of-order 
packet arrival in the context of the same stream - the packet that 
arrives later on wire may be served earlier since there may still be 
unhandled packets on the "old" RSS queue when it arrives. This is unless 
updating the RSS indirection table registers ensures the fast path 
queues flushing.

The proper solution should ensure the fast path queues draining before 
updating the indirection table. In any case, I'd prefer not to implement 
the -X option and concentrate on -x option only.

quoted

I'll send the patches later today. I'll incorporate the new PF code 
into the existing series.

You might want to break things out into a couple series of patches. Do 
the PF driver first just to make sure you have an understanding of 
what you expect, think of it as a proof of concept if nothing else for 
how you want the VF to function, then submit the VF series as a 
separate patch set and include the mailbox API changes.

Sound reasonable.

- Alex