netdev - Re: [PATCH net v3] xen-netback: Fix grant ref resolution in RX path

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1400142556.30687.1.camel@kazak.uk.xensource.com>
Date:	Thu, 15 May 2014 09:29:16 +0100
From:	Ian Campbell <Ian.Campbell@...rix.com>
To:	Zoltan Kiss <zoltan.kiss@...rix.com>
CC:	<xen-devel@...ts.xenproject.org>, <wei.liu2@...rix.com>,
	<linux@...elenboom.it>, <paul.durrant@...rix.com>,
	<netdev@...r.kernel.org>, <david.vrabel@...rix.com>,
	<davem@...emloft.net>
Subject: Re: [PATCH net v3] xen-netback: Fix grant ref resolution in RX path

On Wed, 2014-05-14 at 19:59 +0100, Zoltan Kiss wrote:
> All the comments are acked and applied, expect this:
> 
> On 14/05/14 17:40, Ian Campbell wrote:
> > On Wed, 2014-05-14 at 13:08 +0100, Zoltan Kiss wrote:
> 
> >
> >> +		/* This variable also signals whether foreign_gref has a real
> >> +		 * value or not.
> >> +		 */
> >> +		struct xenvif *foreign_vif = NULL;
> >> +
> >> +		if ((skb_shinfo(skb)->tx_flags & SKBTX_DEV_ZEROCOPY) &&
> >> +			(ubuf->callback == &xenvif_zerocopy_callback)) {
> >> +			const struct ubuf_info *const startpoint = ubuf;
> >> +
> >> +			/* Ideally ubuf points to the chain element which
> >> +			 * belongs to this frag. Or if frags were removed from
> >> +			 * the beginning, then shortly before it.
> >> +			 */
> >> +			ubuf = xenvif_find_gref(skb, i, ubuf);
> >> +
> >> +			/* Try again from the beginning of the list, if we
> >> +			 * haven't tried from there. This only makes sense in
> >> +			 * the unlikely event of reordering the original frags.
> >> +			 * For injected local pages it's an unnecessary second
> >> +			 * run.
> >
> > A second run is unfortunate. Can we also pass startpoint to
> > xenvif_find_gref as an end value and have it only search that far?
> That would help, but it would just increase the complexity here, and I 
> think this is absolutely a non fast-path case. But the first search is, 
> passing a new parameter and doing another comparison can consume a few 
> cycles.
> We don't know about any scenarios where frag reordering or injection of 
> local frags can actually happen, this is just about closing out 
> theoretical possibilities. Let's optimize it when it actually needed!

OK.

Perhaps consider adding a stat to count the occurrences of this path?
(not necessarily right now)

Ian.


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