From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751514AbdK2FWy (ORCPT ); Wed, 29 Nov 2017 00:22:54 -0500 Received: from mx1.redhat.com ([209.132.183.28]:59136 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750713AbdK2FWx (ORCPT ); Wed, 29 Nov 2017 00:22:53 -0500 Date: Wed, 29 Nov 2017 13:44:22 +0800 From: Wei Xu To: Jason Wang Cc: virtualization@lists.linux-foundation.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, mjrosato@linux.vnet.ibm.com, mst@redhat.com Subject: Re: [PATCH net,stable] vhost: fix skb leak in handle_rx() 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-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <37f16bef-4a59-b895-c61b-e3f473617411@redhat.com> User-Agent: NeoMutt/20170113-14-7f1397-dirty (1.7.2) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Wed, 29 Nov 2017 05:22:53 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@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