From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wei Xu Subject: Re: [PATCH net,stable] vhost: fix skb leak in handle_rx() Date: Wed, 29 Nov 2017 13:44:22 +0800 Message-ID: <20171129054422.qmhjvc2tn3tn5gm7@Wei-Dev> References: <1511889436-12876-1-git-send-email-wexu@redhat.com> <9b234fdd-baa3-f919-6c72-f6768265f1ca@redhat.com> <37f16bef-4a59-b895-c61b-e3f473617411@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: virtualization@lists.linux-foundation.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, mjrosato@linux.vnet.ibm.com, mst@redhat.com To: Jason Wang Return-path: Content-Disposition: inline In-Reply-To: <37f16bef-4a59-b895-c61b-e3f473617411@redhat.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On Wed, Nov 29, 2017 at 01:06:28PM +0800, Jason Wang wrote: > > > On 2017年11月29日 09:53, Jason Wang wrote: > > > > > > On 2017å¹´11月29æ—¥ 01:17, wexu@redhat.com wrote: > > > From: Wei Xu > > > > > > Matthew found a roughly 40% tcp throughput regression with commit > > > c67df11f(vhost_net: try batch dequing from skb array) as discussed > > > in the following thread: > > > https://www.mail-archive.com/netdev@vger.kernel.org/msg187936.html > > > > > > Eventually we figured out that it was a skb leak in handle_rx() > > > when sending packets to the VM. This usually happens when a guest > > > can not drain out vq as fast as vhost fills in, afterwards it sets > > > off the traffic jam and leaks skb(s) which occurs as no headcount > > > to send on the vq from vhost side. > > > > > > This can be avoided by making sure we have got enough headcount > > > before actually consuming a skb from the batched rx array while > > > transmitting, which is simply done by deferring it a moment later > > > in this patch. > > > > > > Signed-off-by: Wei Xu > > > --- > > >   drivers/vhost/net.c | 4 ++-- > > >   1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c > > > index 8d626d7..e76535e 100644 > > > --- a/drivers/vhost/net.c > > > +++ b/drivers/vhost/net.c > > > @@ -778,8 +778,6 @@ static void handle_rx(struct vhost_net *net) > > >           /* On error, stop handling until the next kick. */ > > >           if (unlikely(headcount < 0)) > > >               goto out; > > > -        if (nvq->rx_array) > > > -            msg.msg_control = vhost_net_buf_consume(&nvq->rxq); > > >           /* On overrun, truncate and discard */ > > >           if (unlikely(headcount > UIO_MAXIOV)) { > > > > You need do msg.msg_control = vhost_net_buf_consume() here too, > > otherwise we may still get it leaked. > > > > Thanks > > Not a leak actually, but the packet won't be consumed and we will hit > UIO_MAXIOV forever in this case. I see, thanks, will make a v2. > > Thanks