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: <20150319.175107.1317700631160353748.davem@davemloft.net>
Date:	Thu, 19 Mar 2015 17:51:07 -0400 (EDT)
From:	David Miller <davem@...emloft.net>
To:	thomas.lendacky@....com
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCH net-next v2 10/10] amd-xgbe: Rework the Rx path SKB
 allocation

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.

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