netdev - Re: [PATCH] virtio_net: indicate oom when addbuf returns failure

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: <4C10B4760200004800097CA9@sinclair.provo.novell.com>
Date:	Thu, 10 Jun 2010 09:46:30 -0600
From:	"Bruce Rogers" <brogers@...ell.com>
To:	"Michael S. Tsirkin" <mst@...hat.com>,
	"Rusty Russell" <rusty@...tcorp.com.au>
Cc:	"Herbert Xu" <herbert@...dor.hengli.com.au>, <stable@...nel.org>,
	<virtualization@...ts.linux-foundation.org>,
	<netdev@...r.kernel.org>
Subject: Re: [PATCH] virtio_net: indicate oom when addbuf returns
 failure

 >>> On 6/6/2010 at 02:13 PM, "Michael S. Tsirkin" <mst@...hat.com> wrote: 
> On Fri, Jun 04, 2010 at 10:28:56AM +0930, Rusty Russell wrote:
>> This patch is a subset of an already upstream patch, but this portion
>> is useful in earlier releases.
>> 
>> Please consider for the 2.6.32 and 2.6.33 stable trees.
>> 
>> If the add_buf operation fails, indicate failure to the caller.
>> 
>> Signed-off-by: Bruce Rogers <brogers@...ell.com>
>> Signed-off-by: Rusty Russell <rusty@...tcorp.com.au>
> 
> Actually this code looks strange:
> Note that add_buf inicates out of memory
> condition with a positive return value, and ring full
> (which is not an error!) with -ENOSPC.
> 
> So it seems that this patch (and upstream code) will fill
> the ring and then end up setting oom = true and rescheduling the work
> forever.  And I suspect I actually saw this at some point
> on one of my systems: observed BW would drop
> with high CPU usage until reboot.
> Can't reproduce it now anymore ..
> 

Thanks for looking into this.

We've decided not to use this patch, since it is not a part of the
solution we need. The upstream patch from whence it came at first glance seemed useful, but is problematic as you point out.

We've retested without that patch and are still getting good results.

Bruce

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