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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <550B48AC.5030007@amd.com>
Date:	Thu, 19 Mar 2015 17:07:40 -0500
From:	Tom Lendacky <thomas.lendacky@....com>
To:	David Miller <davem@...emloft.net>
CC:	<netdev@...r.kernel.org>
Subject: Re: [PATCH net-next v2 10/10] amd-xgbe: Rework the Rx path SKB allocation

On 03/19/2015 04:51 PM, David Miller wrote:
> From: Tom Lendacky <thomas.lendacky@....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@....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@....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.
>
--
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