From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 7E337A0C41; Wed, 15 Sep 2021 11:16:25 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id F31BA4003F; Wed, 15 Sep 2021 11:16:24 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 00E064003C; Wed, 15 Sep 2021 11:16:22 +0200 (CEST) Received: from [192.168.38.17] (aros.oktetlabs.ru [192.168.38.17]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id 69F1E7F4FE; Wed, 15 Sep 2021 12:16:22 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru 69F1E7F4FE DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1631697382; bh=WchoAwgMs541UbczsNFNYc98+FkorHYs0paE9WhCjqE=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=o1kO0NUcTcQ1g/tU3pDFNXY2x0ejTdsSl8ymo1ezrrBgI07gmhP1nQG2nOHl+BY+N BpOTB8rFThOzp69ukzMH/Juf7EOOaw6/3Ih9AckTB1kBXmCY/NNnR5gM35z/FjUcte qkenIUhPmaQDQl3yF2YmaBpmyFLUVrUnKVPNjMAw= To: Maxime Coquelin , Chenbo Xia Cc: dev@dpdk.org, Ivan Ilchenko , stable@dpdk.org References: <20210818141329.2842686-1-andrew.rybchenko@oktetlabs.ru> <20210818141329.2842686-2-andrew.rybchenko@oktetlabs.ru> <8983d9c0-0f7c-6feb-07fd-d40300e446c6@redhat.com> From: Andrew Rybchenko Organization: OKTET Labs Message-ID: <3af06e9b-48e2-e732-4607-1cdd1e5478df@oktetlabs.ru> Date: Wed, 15 Sep 2021 12:16:22 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: <8983d9c0-0f7c-6feb-07fd-d40300e446c6@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 2/2] net/virtio: fix Tx completed mbufs leak on device stop X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 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 9/13/21 6:41 PM, Maxime Coquelin wrote: > > > On 8/18/21 4:13 PM, Andrew Rybchenko wrote: >> From: Ivan Ilchenko >> >> Free Tx completed mbufs on device stop. Not completed Tx mbufs cannot be >> freed since they are still in use. >> >> Fixes: c1f86306a02 ("virtio: add new driver") >> Cc: stable@dpdk.org >> >> Signed-off-by: Ivan Ilchenko >> Signed-off-by: Andrew Rybchenko >> --- >> drivers/net/virtio/virtio_ethdev.c | 30 ++++++++++++++++++++++++++++++ >> 1 file changed, 30 insertions(+) >> >> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c >> index e58085a2c9..ed3fefee7c 100644 >> --- a/drivers/net/virtio/virtio_ethdev.c >> +++ b/drivers/net/virtio/virtio_ethdev.c >> @@ -2393,6 +2393,34 @@ static void virtio_dev_free_mbufs(struct rte_eth_dev *dev) >> PMD_INIT_LOG(DEBUG, "%d mbufs freed", mbuf_num); >> } >> >> +static void >> +virtio_tx_completed_cleanup(struct rte_eth_dev *dev) >> +{ >> + struct virtio_hw *hw = dev->data->dev_private; >> + struct virtqueue *vq; >> + int qidx; >> + void (*xmit_cleanup)(struct virtqueue *vq, uint16_t nb_used); >> + >> + if (virtio_with_packed_queue(hw)) { >> + if (hw->use_vec_tx) >> + xmit_cleanup = &virtio_xmit_cleanup_inorder_packed; >> + else if (virtio_with_feature(hw, VIRTIO_F_IN_ORDER)) >> + xmit_cleanup = &virtio_xmit_cleanup_inorder_packed; >> + else >> + xmit_cleanup = &virtio_xmit_cleanup_normal_packed; >> + } else { >> + if (hw->use_inorder_tx) >> + xmit_cleanup = &virtio_xmit_cleanup_inorder; >> + else >> + xmit_cleanup = &virtio_xmit_cleanup; >> + } >> + >> + for (qidx = 0; qidx < hw->max_queue_pairs; qidx++) { >> + vq = hw->vqs[2 * qidx + VTNET_SQ_TQ_QUEUE_IDX]; > > Maybe add a check to ensure that vq is non-NULL since it is dereferenced > later without checking. Good idea, I'll send v2. > >> + xmit_cleanup(vq, virtqueue_nused(vq)); >> + } >> +} >> + >> /* >> * Stop device: disable interrupt and mark link down >> */ >> @@ -2411,6 +2439,8 @@ virtio_dev_stop(struct rte_eth_dev *dev) >> goto out_unlock; >> hw->started = 0; >> >> + virtio_tx_completed_cleanup(dev); >> + >> if (intr_conf->lsc || intr_conf->rxq) { >> virtio_intr_disable(dev); >> >>