netdev - Re: [PATCH 10/13] net: add paged frag destructor to skb_fill_page

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: <CAHXqBF+TJrBvEo+fd4tfXxDucEx5QaCgfBWPQ2B5UPjYQUAUgg@mail.gmail.com>
Date:	Fri, 22 Jul 2011 23:44:05 +0200
From:	Michał Mirosław <mirqus@...il.com>
To:	Ian Campbell <Ian.Campbell@...citrix.com>
Cc:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-nfs@...r.kernel.org" <linux-nfs@...r.kernel.org>,
	"David S. Miller" <davem@...emloft.net>,
	"James E.J. Bottomley" <JBottomley@...allels.com>,
	Alexey Kuznetsov <kuznet@....inr.ac.ru>,
	"Pekka Savola (ipv6)" <pekkas@...core.fi>,
	James Morris <jmorris@...ei.org>,
	Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
	Patrick McHardy <kaber@...sh.net>,
	"linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
	"linux-s390@...r.kernel.org" <linux-s390@...r.kernel.org>,
	"linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
	"devel@...n-fcoe.org" <devel@...n-fcoe.org>
Subject: Re: [PATCH 10/13] net: add paged frag destructor to skb_fill_page_desc()

W dniu 22 lipca 2011 23:07 użytkownik Ian Campbell
<Ian.Campbell@...citrix.com> napisał:
> On Fri, 2011-07-22 at 20:58 +0100, Michał Mirosław wrote:
>> 2011/7/22 Ian Campbell <ian.campbell@...rix.com>:
>> > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
>> > index 9818fe2..faee8d3 100644
>> > --- a/include/linux/skbuff.h
>> > +++ b/include/linux/skbuff.h
>> > @@ -1134,12 +1134,14 @@ static inline int skb_pagelen(const struct sk_buff *skb)
>> >  * Does not take any additional reference on the fragment.
>> >  */
>> >  static inline void __skb_fill_page_desc(struct sk_buff *skb, int i,
>> > -                                       struct page *page, int off, int size)
>> > +                                       struct page *page,
>> > +                                       struct skb_frag_destructor *destroy,
>> > +                                       int off, int size)
>> >  {
>> >        skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
>> >
>> >        frag->page.p              = page;
>> > -       frag->page.destructor     = NULL;
>> > +       frag->page.destructor     = destroy;
>> >        frag->page_offset         = off;
>> >        frag->size                = size;
>> >  }
>>
>> You could just rename this function to e.g.
>> __skb_fill_fragile_page_desc() (or whatever name) add an inline
>> wrapper calling it with destroy == NULL. This will avoid touching all
>> those drivers which won't ever need this functionality.
>
> I could call this variant __skb_frag_init (which I think better fits
> into the pattern of the new functions) and leave the existing
> __skb_fill_page_desc as a compat wrapper if that's preferred but I was
> trying to avoid duplicating up constructors just for different sets of
> defaults.

It's just Huffman coding: since most users need destroy = NULL, it's
good to have a wrapper for this case as it will then take less time to
write and understand the code (you won't need to think what is this
NULL for in all those places).

Best Regards,
Michał Mirosław
--
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