Re: [PATCH net-next v2 10/10] amd-xgbe: Rework the Rx path SKB allocation

[Tom Lendacky <thomas.lendacky@amd.com>]

On 03/19/2015 04:51 PM, David Miller wrote:
> From: Tom Lendacky <thomas.lendacky@amd.com>
> Date: Thu, 19 Mar 2015 15:54:24 -0500
>
>> On 03/19/2015 03:20 PM, David Miller wrote:
>>> From: Tom Lendacky <thomas.lendacky@amd.com>
>>> Date: Thu, 19 Mar 2015 15:09:08 -0500
>>>
>>>> When the driver creates an SKB it currently only copies the header
>>>> buffer data (which can be just the header if split header processing
>>>> succeeded or header plus data if split header processing did not
>>>> succeed) into the SKB. The receive buffer data is always added as a
>>>> frag, even if it could fit in the SKB. As part of SKB creation, inline
>>>> the receive buffer data if it will fit in the the SKB, otherwise add
>>>> it
>>>> as a frag during SKB creation.
>>>>
>>>> Also, Update the code to trigger off of the first/last descriptor
>>>> indicators and remove the incomplete indicator.
>>>>
>>>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>>>
>>> I do not understand the motivation for this, could you explain?
>>>
>>> The less copying you do the better, just having the headers in the
>>> linear area is the most optimal situation, and have all the data
>>> in page frag references.
>>>
>>
>> I was trying to make the Rx path more logical from a first / last
>> descriptor point of view.  If it's the first descriptor allocate the
>> SKB, otherwise add the data as a frag. Compared to the current code:
>> check for null skb pointer, allocate the SKB and if there's data left
>> add it as a frag.
>>
>> I could keep the first / last descriptor methodology and in the
>> xgbe_create_skb routine avoid the second copy and just always add the
>> other buffer as a frag. That will eliminate the extra copying. Would
>> that be ok or would you prefer that I just drop this patch?
>
> The point is, the data might not even be touched by the cpu so copying
> it into the linear SKB data area could be a complete waste of cpu
> cycles.

I understood that point, sorry if I wasn't clear.  I'd would remove the
copying of the data and just always add it as a frag in xgbe_create_skb
routine so that only the headers are in the linear area.  What I'd like
to do though is keep the overall changes of how I determine when to
call the xgbe_create_skb routine so that it appears a bit cleaner,
more logical.  The net effect is that the behavior of the code would
remain the same (headers in the linear area, data as frags), but I feel
it reads better and is easier to understand.

Thanks,
Tom

>
> Only the headers will be touched by the cpu in the packet processing
> paths.
>
> And when we copy the packet data into userspace, that might even occur
> on another cpu, so the data will just thrash between the individual
> cpu's caches.
>