netdev - Re: [PATCH net-next v3] net: add skb allocation flags to pskb

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] [day] [month] [year] [list]

Message-Id: <20140611.120914.1187135773039853303.davem@davemloft.net>
Date:	Wed, 11 Jun 2014 12:09:14 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	octavian.purdila@...el.com
Cc:	David.Laight@...lab.com, netdev@...r.kernel.org,
	christoph.paasch@...ouvain.be, alex.bluesman.smirnov@...il.com,
	dbaryshkov@...il.com, mareklindner@...mailbox.ch,
	sw@...onwunderlich.de, antonio@...hcoding.com, marcel@...tmann.org,
	gustavo@...ovan.org, johan.hedberg@...il.com,
	arvid.brodin@...en.se, kaber@...sh.net, pablo@...filter.org,
	kadlec@...ckhole.kfki.hu, lauro.venancio@...nbossa.org,
	aloisio.almeida@...nbossa.org, sameo@...ux.intel.com,
	jon.maloy@...csson.com, allan.stephens@...driver.com,
	andrew.hendry@...il.com, edumazet@...gle.com
Subject: Re: [PATCH net-next v3] net: add skb allocation flags to pskb_copy

From: Octavian Purdila <octavian.purdila@...el.com>
Date: Wed, 11 Jun 2014 12:59:26 +0300

> On Wed, Jun 11, 2014 at 12:22 PM, David Laight <David.Laight@...lab.com> wrote:
>> From: Octavian Purdila
>>> There are several instances where a pskb_copy or __pskb_copy is
>>> immediately followed by an skb_clone. Add a new parameter to allow the
>>> skb to be allocated from the fclone cache and thus speed up subsequent
>>> skb_clone calls.
>> ...
>>> @@ -2233,9 +2233,9 @@ static inline dma_addr_t skb_frag_dma_map(struct device *dev,
>>>  }
>>>
>>>  static inline struct sk_buff *pskb_copy(struct sk_buff *skb,
>>> -                                     gfp_t gfp_mask)
>>> +                                     gfp_t gfp_mask, bool fclone)
>>>  {
>>> -     return __pskb_copy(skb, skb_headroom(skb), gfp_mask);
>>> +     return __pskb_copy(skb, skb_headroom(skb), gfp_mask, fclone);
>> ...
>>
>> Why not add:
>> static inline struct sk_buff *pskb_copy_for_clone(struct sk_buff *skb,
>>                                         gfp_t gfp_mask)
>> {
>>         return __pskb_copy(skb, skb_headroom(skb), gfp_mask, true);
>> }
>>
>> So that only some of the call sites need changing.
>> The different function name is probably easier to grok than a Boolean
>> parameter.
>>
> 
> Hi David,
> 
> Since cloning a copy seems to be more frequent then one would thought,
> I think that explicitly adding the option will force people to think
> about allocating from the fclone cache.
> 
> But I don't care much either way, so if people think a new function is
> better I will rework the patch.

Actually you can do this in a way that makes the patch simpler.

Keep __pskb_copy()'s signature as is, and make it do something like:

	__pskb_copy_clone(skb, gfp_mask, false);

And then make the aforementioned pskb_copy_for_clone() which is:

	__pskb_copy_clone(skb, gfp_mask, true);

Then most of the call sites don't need to be touched at all.  You only
change the ones that want the new semantics.
--
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