netdev - Re: [PATCH net-next 3/3] net: allow to leave the buffer fragmented in skb_cow

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: <527B5586.3090401@secunet.com>
Date:	Thu, 07 Nov 2013 09:55:34 +0100
From:	Mathias Krause <mathias.krause@...unet.com>
To:	Herbert Xu <herbert@...dor.apana.org.au>
CC:	"David S. Miller" <davem@...emloft.net>,
	Steffen Klassert <steffen.klassert@...unet.com>,
	Dmitry Tarnyagin <dmitry.tarnyagin@...kless.no>,
	netdev@...r.kernel.org
Subject: Re: [PATCH net-next 3/3] net: allow to leave the buffer fragmented
 in skb_cow_data()

On 06.11.2013 13:48, Herbert Xu wrote:
> On Wed, Nov 06, 2013 at 01:42:03PM +0100, Mathias Krause wrote:
>> Well, skb_cow_data() will only copy, i.e. call __pskb_pull_tail(), in
>> case the skb is either cloned or fragmented. As you already said it
>> won't be cloned in your case. Does it contain fragments, i.e. is
>> skb_shinfo(skb)->nr_frags != 0? If not, we won't copy with the current
>> code either.
> 
> Whenever we say page it means nr_frags != 0.  So currently as
> long as we have pages in our skb we will copy.  With your patch
> we will no longer copy in the case where we have pages but the
> skb isn't cloned.  In fact that is the whole point of your patch.

Indeed. I want to avoid the costly memcpy() on the CPU serving the NIC
interrupt, as that is a bottleneck in my setup. The packet processing --
encrypting/decrypting of ESP packets -- gets mostly parallelized via
pcrypt, so that's fine. But the initial network processing, i.e. getting
to pcrypt, is what's throttling the throughput currently. (RPS only
partly solves this problem as for the ESP receive path most traffic ends
up on the same flow).

>> Can you please explain why this would be needed? I still don't get the
>> reasoning behind "pages are considered not writable at the moment even
>> if they are anonymous".
> 
> As I said you don't know where the page in the skb came from.  It
> may point to read-only memory or memory that's shared with another
> task that isn't expecting things to change underneath it.
> 
> It may well turn out to most if not all cases of pages are safe to
> be written to if skb_cloned == 0.  However, we'd need to do a full
> audit of every source of page frags to be sure.  For example, you'd
> need to look at net drivers and splice.

Ah, okay. Now that makes sense. I'll look into it.

Thanks,
Mathias
--
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