Re: [PATCH net-next 3/3] net: allow to leave the buffer fragmented in skb_cow_data()

[Mathias Krause <mathias.krause@secunet.com>]

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