netdev - Re: [RFC PATCH 03/13] xen-netback: implement TX persistent grants

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: <20150602145359.GP19403@zion.uk.xensource.com>
Date:	Tue, 2 Jun 2015 15:53:59 +0100
From:	Wei Liu <wei.liu2@...rix.com>
To:	Joao Martins <Joao.Martins@...lab.eu>
CC:	Wei Liu <wei.liu2@...rix.com>,
	"xen-devel@...ts.xenproject.org" <xen-devel@...ts.xenproject.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"ian.campbell@...rix.com" <ian.campbell@...rix.com>,
	"david.vrabel@...rix.com" <david.vrabel@...rix.com>,
	"boris.ostrovsky@...cle.com" <boris.ostrovsky@...cle.com>,
	"konrad.wilk@...cle.com" <konrad.wilk@...cle.com>
Subject: Re: [RFC PATCH 03/13] xen-netback: implement TX persistent grants

On Fri, May 22, 2015 at 10:24:39AM +0000, Joao Martins wrote:
> 
> On 19 May 2015, at 17:23, Wei Liu <wei.liu2@...rix.com> wrote:
> > On Tue, May 12, 2015 at 07:18:27PM +0200, Joao Martins wrote:
> >> Introduces persistent grants for TX path which follows similar code path
> >> as the grant mapping.
> >> 
> >> It starts by checking if there's a persistent grant available for header
> >> and frags grefs and if so setting it in tx_pgrants. If no persistent grant
> >> is found in the tree for the header it will resort to grant copy (but
> >> preparing the map ops and add them laster). For the frags it will use the
> >                                     ^
> >                                     later
> > 
> >> tree page pool, and in case of no pages it fallbacks to grant map/unmap
> >> using mmap_pages. When skb destructor callback gets called we release the
> >> slot and persistent grant within the callback to avoid waking up the
> >> dealloc thread. As long as there are no unmaps to done the dealloc thread
> >> will remain inactive.
> >> 
> > 
> > This scheme looks complicated. Can we just only use one
> > scheme at a time? What's the rationale for using this combined scheme?
> > Maybe you're thinking about using a max_grants < ring_size to save
> > memory?
> 
> Yes, my purpose was to allow a max_grants < ring_size to save amount of
> memory mapped. I did a bulk transfer test with iperf and the max amount of
> grants in tree was <160 TX gnts, without affecting the max performance;
> tough using pktgen fills the tree completely.
> The second reason is to handle the case for a (malicious?) frontend providing
> more grefs than the max allowed in which I would fallback to grant map/unmap.
> 

This is indeed a valid concern. The only method is to expires oldest
grant when that happens -- but this is just complexity in another place,
not really simplifying anything.

> > 
> > Only skim the patch. I will do detailed reviews after we're sure this is
> > the right way to go.
> > 
[...]
> > 
> > Under what circumstance can we retrieve a already in use persistent
> > grant? You seem to suggest this is a bug in RX case.
> 
> A guest could share try to share the same mapped page in multiple frags,
> in which case I fallback to map/unmap. I think this is a limitation in
> the way we manage the persistent gnts where we can only have a single
> reference of a persistent grant inflight.
> 

How much harder would it be to ref-count inflight grants? Would that
simplify or perplex things? I'm just asking, not suggesting you should
choose ref-counting over current scheme.

In principle I favour simple code path over optimisation for every
possible corner case.

Wei.
--
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