From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 46522A0471 for ; Wed, 19 Jun 2019 14:34:52 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 3563C1C2C2; Wed, 19 Jun 2019 14:34:51 +0200 (CEST) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id E21DE1C291; Wed, 19 Jun 2019 14:34:49 +0200 (CEST) Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 09FE7B2DCE; Wed, 19 Jun 2019 12:34:44 +0000 (UTC) Received: from [10.36.112.46] (ovpn-112-46.ams2.redhat.com [10.36.112.46]) by smtp.corp.redhat.com (Postfix) with ESMTPS id E17091001E61; Wed, 19 Jun 2019 12:34:41 +0000 (UTC) To: Tiwei Bie , zhihong.wang@intel.com, dev@dpdk.org Cc: stable@dpdk.org References: <20190618074159.3322-1-tiwei.bie@intel.com> <20190618074159.3322-2-tiwei.bie@intel.com> From: Maxime Coquelin Message-ID: Date: Wed, 19 Jun 2019 14:34:40 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <20190618074159.3322-2-tiwei.bie@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Wed, 19 Jun 2019 12:34:44 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH 1/4] net/virtio: fix memory leak in in-order Rx X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 6/18/19 9:41 AM, Tiwei Bie wrote: > When there is no enough segments for a packet in in-order > mergeable Rx path, we should free the whole mbuf chain instead > of just the last segment. I would write instead: " we should free the whole mbuf chain instead of just recycling the last segment. " Because what was done before the patch it to refill the VQ with the last segment. With your patch, the full chain is freed, then the refill is done afterwards. Do you agree? Other than that: Reviewed-by: Maxime Coquelin Thanks! Maxime > > Fixes: e5f456a98d3c ("net/virtio: support in-order Rx and Tx") > Cc: stable@dpdk.org > > Signed-off-by: Tiwei Bie > --- > drivers/net/virtio/virtio_rxtx.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c > index 1f1178467..bdb3a2f18 100644 > --- a/drivers/net/virtio/virtio_rxtx.c > +++ b/drivers/net/virtio/virtio_rxtx.c > @@ -1555,7 +1555,7 @@ virtio_recv_pkts_inorder(void *rx_queue, > } else { > PMD_RX_LOG(ERR, > "No enough segments for packet."); > - virtio_discard_rxbuf_inorder(vq, prev); > + rte_pktmbuf_free(rx_pkts[nb_rx]); > rxvq->stats.errors++; > break; > } >