From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752846AbeEUPBj (ORCPT ); Mon, 21 May 2018 11:01:39 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:57238 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751722AbeEUPBh (ORCPT ); Mon, 21 May 2018 11:01:37 -0400 Date: Mon, 21 May 2018 18:01:36 +0300 From: "Michael S. Tsirkin" To: Jason Wang Cc: virtualization@lists.linux-foundation.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, John Fastabend Subject: Re: [PATCH net 4/4] virito-net: fix leaking page for gso packet during mergeable XDP Message-ID: <20180521175919-mutt-send-email-mst@kernel.org> References: <1526891706-18516-1-git-send-email-jasowang@redhat.com> <1526891706-18516-5-git-send-email-jasowang@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1526891706-18516-5-git-send-email-jasowang@redhat.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, May 21, 2018 at 04:35:06PM +0800, Jason Wang wrote: > We need to drop refcnt to xdp_page if we see a gso packet. Otherwise > it will be leaked. Fixing this by moving the check of gso packet above > the linearizing logic. > > Cc: John Fastabend > Fixes: 72979a6c3590 ("virtio_net: xdp, add slowpath case for non contiguous buffers") > Signed-off-by: Jason Wang typo in subject > --- > drivers/net/virtio_net.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 165a922..f8db809 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -707,6 +707,14 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, > void *data; > u32 act; > > + /* Transient failure which in theory could occur if > + * in-flight packets from before XDP was enabled reach > + * the receive path after XDP is loaded. In practice I > + * was not able to create this condition. BTW we should probably drop the last sentence. It says in theory, should be enough. > + */ > + if (unlikely(hdr->hdr.gso_type)) > + goto err_xdp; > + > /* This happens when rx buffer size is underestimated > * or headroom is not enough because of the buffer > * was refilled before XDP is set. This should only > @@ -728,14 +736,6 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, > xdp_page = page; > } > > - /* Transient failure which in theory could occur if > - * in-flight packets from before XDP was enabled reach > - * the receive path after XDP is loaded. In practice I > - * was not able to create this condition. > - */ > - if (unlikely(hdr->hdr.gso_type)) > - goto err_xdp; > - > /* Allow consuming headroom but reserve enough space to push > * the descriptor on if we get an XDP_TX return code. > */ > -- > 2.7.4 From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PATCH net 4/4] virito-net: fix leaking page for gso packet during mergeable XDP Date: Mon, 21 May 2018 18:01:36 +0300 Message-ID: <20180521175919-mutt-send-email-mst@kernel.org> References: <1526891706-18516-1-git-send-email-jasowang@redhat.com> <1526891706-18516-5-git-send-email-jasowang@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org, John Fastabend , linux-kernel@vger.kernel.org, virtualization@lists.linux-foundation.org To: Jason Wang Return-path: Content-Disposition: inline In-Reply-To: <1526891706-18516-5-git-send-email-jasowang@redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: virtualization-bounces@lists.linux-foundation.org Errors-To: virtualization-bounces@lists.linux-foundation.org List-Id: netdev.vger.kernel.org On Mon, May 21, 2018 at 04:35:06PM +0800, Jason Wang wrote: > We need to drop refcnt to xdp_page if we see a gso packet. Otherwise > it will be leaked. Fixing this by moving the check of gso packet above > the linearizing logic. > > Cc: John Fastabend > Fixes: 72979a6c3590 ("virtio_net: xdp, add slowpath case for non contiguous buffers") > Signed-off-by: Jason Wang typo in subject > --- > drivers/net/virtio_net.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 165a922..f8db809 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -707,6 +707,14 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, > void *data; > u32 act; > > + /* Transient failure which in theory could occur if > + * in-flight packets from before XDP was enabled reach > + * the receive path after XDP is loaded. In practice I > + * was not able to create this condition. BTW we should probably drop the last sentence. It says in theory, should be enough. > + */ > + if (unlikely(hdr->hdr.gso_type)) > + goto err_xdp; > + > /* This happens when rx buffer size is underestimated > * or headroom is not enough because of the buffer > * was refilled before XDP is set. This should only > @@ -728,14 +736,6 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, > xdp_page = page; > } > > - /* Transient failure which in theory could occur if > - * in-flight packets from before XDP was enabled reach > - * the receive path after XDP is loaded. In practice I > - * was not able to create this condition. > - */ > - if (unlikely(hdr->hdr.gso_type)) > - goto err_xdp; > - > /* Allow consuming headroom but reserve enough space to push > * the descriptor on if we get an XDP_TX return code. > */ > -- > 2.7.4