[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <54A6DA1F.7040206@cloudius-systems.com>
Date: Fri, 02 Jan 2015 19:49:19 +0200
From: Vlad Zolotarov <vladz@...udius-systems.com>
To: Alexander Duyck <alexander.duyck@...il.com>, netdev@...r.kernel.org
CC: gleb@...udius-systems.com, avi@...udius-systems.com,
jeffrey.t.kirsher@...el.com,
Don Skidmore <donald.c.skidmore@...el.com>,
"tantilov, Emil S" <emil.s.tantilov@...el.com>
Subject: Re: [PATCH net-next v1 2/5] ixgbevf: Add a RETA query code
On 01/01/15 20:09, Alexander Duyck wrote:
> On 12/31/2014 10:33 AM, Vlad Zolotarov wrote:
>> On 12/31/14 20:00, Alexander Duyck wrote:
>>> I suspect this code is badly broken as it doesn't take several things
>>> into account.
>>>
>>> First the PF redirection table can have values outside of the range
>>> supported by the VF. This is allowed as the VF can set how many bits of
>>> the redirection table it actually wants to use. This is controlled via
>>> the PSRTYPE register. So for example the PF can be running with 4
>>> queues, and the VF can run either in single queue or as just a pair of
>>> queues.
>>>
>>> Second you could compress this data much more tightly by taking
>>> advantage of the bit widths allowed. So for everything x540 and older
>>> they only use a 4 bit value per entry. That means you could
>>> theoretically stuff 8 entries per u32 instead of just 4.
>> Compression is nice but I think ethtool expects it in a certain
>> format: one entry per byte. And since this patch is targeting the
>> ethtool the output format should be as ethtool expects it to be and
>> this is what this patch does. However I agree that masking the
>> appropriate bits according to PSRTYPE is required. Good catch!
> The idea of compression comes into play when you consider there is
> significant latency trying to get messages across the mailbox. By
> reducing the number of messages needed to get the redirection table you
> should be able to significantly reduce the amount of time needed to
> fetch it. The job of compressing/expanding the values is actually
> pretty straight forward when you consider all that should be needed is a
> simple loop to perform some shift, and, and or operations.
We are talking about a super slow path here. So, regardless how slow it
is it should be done only once between boots. Therefore IMHO any code
complication is not justified here.
>
> - Alex
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists