netdev - Re: [PATCH][RFC] network splice receive

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: <20070612112950.GA16477@2ka.mipt.ru>
Date:	Tue, 12 Jun 2007 15:29:51 +0400
From:	Evgeniy Polyakov <johnpol@....mipt.ru>
To:	Jens Axboe <jens.axboe@...cle.com>
Cc:	David Miller <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH][RFC] network splice receive

On Sat, Jun 09, 2007 at 08:36:09AM +0200, Jens Axboe (jens.axboe@...cle.com) wrote:
> On Fri, Jun 08 2007, Evgeniy Polyakov wrote:
> > On Fri, Jun 08, 2007 at 06:57:25PM +0400, Evgeniy Polyakov (johnpol@....mipt.ru) wrote:
> > > I will try some things for the nearest 30-60 minutes, and then will move to
> > > canoe trip until thuesday, so will not be able to work on this idea.
> > 
> > Ok, replacing in fs/splice.c every page_cache_release() with
> > static void splice_page_release(struct page *p)
> > {
> > 	if (!PageSlab(p))
> > 		page_cache_release(p);
> > }
> 
> Ehm, I don't see why that should be necessary. Except in
> splice_to_pipe(), I have considered that we need to pass in a release
> function if mapping fails at some point. But it's probably best to do
> that in the caller, since they have the knowledge of how to release the
> pages.
> 
> The rest of the PageSlab() tests are bogus.

I had a crashdump, where page was released via splice_to_pipe() indeed,
I did not investigate if it is possible to release provided page in
other places. I think if in future there will other slab usage cases
except networking receiving, that might be useful, but as is it is not
needed.

> > and putting cloned skb into private field instead of 
> > original on in spd_fill_page() ends up without kernel hung.
> 
> Why? Seems pointless to allocate a clone just to hold on to the skb, a
> reference should be equally good. I would not be opposed to doing it
> this way, I just don't see what a clone buys us as compared to just
> holding that reference to the skb.

Receiving code does not expect shared skbs - too many fields are changed
with assumptions that it is a private copy.

> > I'm not sure it is correct, that page can be released in fs/splice.c
> > without calling any callback from network code, when network data is
> > being processed.
> 
> Please explain!

I had a crashdump, where page was attempted to be released in
fs/splice.c:splice_to_pipe(), I do not have details handy, but the best
solution would be to provide a release callback and use that instead of
page_cache_release().

-- 
	Evgeniy Polyakov
-
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