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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <5373BD20.8030800@citrix.com>
Date:	Wed, 14 May 2014 19:59:44 +0100
From:	Zoltan Kiss <zoltan.kiss@...rix.com>
To:	Ian Campbell <Ian.Campbell@...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

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!

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