netdev - RE: [PATCH net v2 1/3] xen-netback: remove pointless clause from if statement

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: <9AAE0902D5BC7E449B7C8E4E778ABCD029C992@AMSPEX01CL01.citrite.net>
Date:	Thu, 27 Mar 2014 13:54:46 +0000
From:	Paul Durrant <Paul.Durrant@...rix.com>
To:	Sander Eikelenboom <linux@...elenboom.it>
CC:	"xen-devel@...ts.xen.org" <xen-devel@...ts.xen.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	Ian Campbell <Ian.Campbell@...rix.com>,
	Wei Liu <wei.liu2@...rix.com>
Subject: RE: [PATCH net v2 1/3] xen-netback: remove pointless clause from if
 statement

> -----Original Message-----
> From: Sander Eikelenboom [mailto:linux@...elenboom.it]
> Sent: 27 March 2014 13:46
> To: Paul Durrant
> Cc: xen-devel@...ts.xen.org; netdev@...r.kernel.org; Ian Campbell; Wei Liu
> Subject: Re: [PATCH net v2 1/3] xen-netback: remove pointless clause from if
> statement
> 
> 
> Thursday, March 27, 2014, 1:56:11 PM, you wrote:
> 
> > This patch removes a test in start_new_rx_buffer() that checks whether
> > a copy operation is less than MAX_BUFFER_OFFSET in length, since
> > MAX_BUFFER_OFFSET is defined to be PAGE_SIZE and the only caller of
> > start_new_rx_buffer() already limits copy operations to PAGE_SIZE or less.
> 
> > Signed-off-by: Paul Durrant <paul.durrant@...rix.com>
> > Cc: Ian Campbell <ian.campbell@...rix.com>
> > Cc: Wei Liu <wei.liu2@...rix.com>
> > Cc: Sander Eikelenboom <linux@...elenboom.it>
> > ---
> 
> > v2:
> >  - Add BUG_ON() as suggested by Ian Campbell
> 
> >  drivers/net/xen-netback/netback.c |    4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> > diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-
> netback/netback.c
> > index 438d0c0..72314c7 100644
> > --- a/drivers/net/xen-netback/netback.c
> > +++ b/drivers/net/xen-netback/netback.c
> > @@ -192,8 +192,8 @@ static bool start_new_rx_buffer(int offset,
> unsigned long size, int head)
> >          * into multiple copies tend to give large frags their
> >          * own buffers as before.
> >          */
> > -       if ((offset + size > MAX_BUFFER_OFFSET) &&
> > -           (size <= MAX_BUFFER_OFFSET) && offset && !head)
> > +       BUG_ON(size > MAX_BUFFER_OFFSET);
> > +       if ((offset + size > MAX_BUFFER_OFFSET) && offset && !head)
> >                 return true;
> >
> >         return false;
> 
> Hi Paul,
> 
> Unfortunately .. no good ..
> 
> With these patches (v2) applied to 3.14-rc8 it all seems to work well,
> until i do my test case .. it still chokes and now effectively permanently stalls
> network traffic to that guest.
> 
> No error messages or anything in either xl dmesg or dmesg on the host .. and
> nothing in dmesg in the guest either.
> 
> But in the guest the TX bytes ifconfig reports for eth0 still increase but RX
> bytes does nothing, so it seems only the RX path is effected)
> 

But you're not getting ring overflow, right? So that suggests this series is working and you're now hitting another problem? I don't see how these patches could directly cause the new behaviour you're seeing.

  Paul

> So it now seems i now have the situation which you described in the commit
> message from "ca2f09f2b2c6c25047cfc545d057c4edfcfe561c",
> "Without this patch I can trivially stall netback permanently by just doing a
> large guest to guest file copy between two Windows Server 2008R2 VMs on a
> single host."
> 
> --
> Sander

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