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 81048A0A02 for ; Mon, 17 May 2021 14:09:58 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 482A340041; Mon, 17 May 2021 14:09:58 +0200 (CEST) Received: from szxga05-in.huawei.com (szxga05-in.huawei.com [45.249.212.191]) by mails.dpdk.org (Postfix) with ESMTP id 4CD6A40041 for ; Mon, 17 May 2021 14:09:55 +0200 (CEST) Received: from dggems704-chm.china.huawei.com (unknown [172.30.72.59]) by szxga05-in.huawei.com (SkyGuard) with ESMTP id 4FkHth6MqlzmVR8; Mon, 17 May 2021 20:07:08 +0800 (CST) Received: from dggeme756-chm.china.huawei.com (10.3.19.102) by dggems704-chm.china.huawei.com (10.3.19.181) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.2176.2; Mon, 17 May 2021 20:09:53 +0800 Received: from [10.67.103.128] (10.67.103.128) by dggeme756-chm.china.huawei.com (10.3.19.102) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2176.2; Mon, 17 May 2021 20:09:52 +0800 To: "Xueming(Steven) Li" , "stable@dpdk.org" CC: "ferruh.yigit@intel.com" References: <1621069606-27683-1-git-send-email-humin29@huawei.com> From: "Min Hu (Connor)" Message-ID: Date: Mon, 17 May 2021 20:09:52 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.3.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="gbk"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.103.128] X-ClientProxiedBy: dggems703-chm.china.huawei.com (10.3.19.180) To dggeme756-chm.china.huawei.com (10.3.19.102) X-CFilter-Loop: Reflected Subject: Re: [dpdk-stable] [PATCH 20.11] net/hns3: fix mbuf leakage X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" Hi, Xueming, One question: This patch has been merged to stable 20.11 as you said. But why I cannot get the info when I print"git log". It only shows like that: b1e71cf version: 20.11.1 fa27a3c version: 20.11.1-rc1 52517bf net/hns3: fix stats flip overflow 2f6bb5f net/hns3: fix query order of link status and link info 668db47 net/mlx5: fix Tx queue size created with DevX 80fd7cc usertools: fix binding built-in kernel driver ef01e05 app/testpmd: fix help of metering commands ..... The reson I asked the question is that there exits other patch to backport, which relies on the previous patch which has merged. Hope for your reply. ÔÚ 2021/5/17 14:56, Xueming(Steven) Li дµÀ: > > >> -----Original Message----- >> From: Min Hu (Connor) >> Sent: Saturday, May 15, 2021 5:07 PM >> To: stable@dpdk.org >> Cc: ferruh.yigit@intel.com; Xueming(Steven) Li >> Subject: [PATCH 20.11] net/hns3: fix mbuf leakage >> >> From: Huisong Li >> >> [ upstream commit fdfde7a4a0f8be8f79c82ee91da9041acd64a798 ] >> >> The mbufs of rx queue will be allocated in "hns3_do_start" function. >> But these mbufs are not released when "hns3_dev_start" executes failed. >> >> Fixes: c4ae39b2cfc5 ("net/hns3: fix Rx interrupt after reset") >> Cc: stable@dpdk.org >> >> Signed-off-by: Huisong Li >> Signed-off-by: Lijun Ou >> --- >> drivers/net/hns3/hns3_ethdev.c | 44 ++++++++++++++++++++++++--------------- >> drivers/net/hns3/hns3_ethdev_vf.c | 43 +++++++++++++++++++++++--------------- >> 2 files changed, 53 insertions(+), 34 deletions(-) >> >> diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c index ba7d6e3..123d2bf 100644 >> --- a/drivers/net/hns3/hns3_ethdev.c >> +++ b/drivers/net/hns3/hns3_ethdev.c >> @@ -101,6 +101,7 @@ static int hns3_remove_mc_addr(struct hns3_hw *hw, >> struct rte_ether_addr *mac_addr); static int hns3_restore_fec(struct hns3_hw *hw); static int >> hns3_query_dev_fec_info(struct hns3_hw *hw); >> +static int hns3_do_stop(struct hns3_adapter *hns); >> >> static void >> hns3_pf_disable_irq0(struct hns3_hw *hw) @@ -4943,11 +4944,8 @@ hns3_dev_start(struct rte_eth_dev *dev) >> return ret; >> } >> ret = hns3_map_rx_interrupt(dev); >> - if (ret) { >> - hw->adapter_state = HNS3_NIC_CONFIGURED; >> - rte_spinlock_unlock(&hw->lock); >> - return ret; >> - } >> + if (ret) >> + goto map_rx_inter_err; >> >> /* >> * There are three register used to control the status of a TQP @@ -4961,19 +4959,12 @@ hns3_dev_start(struct rte_eth_dev >> *dev) >> * status of queue in the dpdk framework. >> */ >> ret = hns3_start_all_txqs(dev); >> - if (ret) { >> - hw->adapter_state = HNS3_NIC_CONFIGURED; >> - rte_spinlock_unlock(&hw->lock); >> - return ret; >> - } >> + if (ret) >> + goto map_rx_inter_err; >> >> ret = hns3_start_all_rxqs(dev); >> - if (ret) { >> - hns3_stop_all_txqs(dev); >> - hw->adapter_state = HNS3_NIC_CONFIGURED; >> - rte_spinlock_unlock(&hw->lock); >> - return ret; >> - } >> + if (ret) >> + goto start_all_rxqs_fail; >> >> hw->adapter_state = HNS3_NIC_STARTED; >> rte_spinlock_unlock(&hw->lock); >> @@ -4996,6 +4987,15 @@ hns3_dev_start(struct rte_eth_dev *dev) >> >> hns3_info(hw, "hns3 dev start successful!"); >> return 0; >> + >> +start_all_rxqs_fail: >> + hns3_stop_all_txqs(dev); >> +map_rx_inter_err: >> + (void)hns3_do_stop(hns); >> + hw->adapter_state = HNS3_NIC_CONFIGURED; >> + rte_spinlock_unlock(&hw->lock); >> + >> + return ret; >> } >> >> static int >> @@ -5004,6 +5004,17 @@ hns3_do_stop(struct hns3_adapter *hns) >> struct hns3_hw *hw = &hns->hw; >> int ret; >> >> + /* >> + * The "hns3_do_stop" function will also be called by .stop_service to >> + * prepare reset. At the time of global or IMP reset, the command cannot >> + * be sent to stop the tx/rx queues. The mbuf in Tx/Rx queues may be >> + * accessed during the reset process. So the mbuf can not be released >> + * during reset and is required to be released after the reset is >> + * completed. >> + */ >> + if (rte_atomic16_read(&hw->reset.resetting) == 0) >> + hns3_dev_release_mbufs(hns); >> + >> ret = hns3_cfg_mac_mode(hw, false); >> if (ret) >> return ret; >> @@ -5080,7 +5091,6 @@ hns3_dev_stop(struct rte_eth_dev *dev) >> hns3_stop_tqps(hw); >> hns3_do_stop(hns); >> hns3_unmap_rx_interrupt(dev); >> - hns3_dev_release_mbufs(hns); >> hw->adapter_state = HNS3_NIC_CONFIGURED; >> } >> hns3_rx_scattered_reset(dev); >> diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c >> index 9c84740..52ad825 100644 >> --- a/drivers/net/hns3/hns3_ethdev_vf.c >> +++ b/drivers/net/hns3/hns3_ethdev_vf.c >> @@ -1912,6 +1912,17 @@ hns3vf_do_stop(struct hns3_adapter *hns) >> >> hw->mac.link_status = ETH_LINK_DOWN; >> >> + /* >> + * The "hns3vf_do_stop" function will also be called by .stop_service to >> + * prepare reset. At the time of global or IMP reset, the command cannot >> + * be sent to stop the tx/rx queues. The mbuf in Tx/Rx queues may be >> + * accessed during the reset process. So the mbuf can not be released >> + * during reset and is required to be released after the reset is >> + * completed. >> + */ >> + if (rte_atomic16_read(&hw->reset.resetting) == 0) >> + hns3_dev_release_mbufs(hns); >> + >> if (rte_atomic16_read(&hw->reset.disable_cmd) == 0) { >> hns3vf_configure_mac_addr(hns, true); >> ret = hns3_reset_all_tqps(hns); >> @@ -1981,7 +1992,6 @@ hns3vf_dev_stop(struct rte_eth_dev *dev) >> hns3_stop_tqps(hw); >> hns3vf_do_stop(hns); >> hns3vf_unmap_rx_interrupt(dev); >> - hns3_dev_release_mbufs(hns); >> hw->adapter_state = HNS3_NIC_CONFIGURED; >> } >> hns3_rx_scattered_reset(dev); >> @@ -2223,11 +2233,8 @@ hns3vf_dev_start(struct rte_eth_dev *dev) >> return ret; >> } >> ret = hns3vf_map_rx_interrupt(dev); >> - if (ret) { >> - hw->adapter_state = HNS3_NIC_CONFIGURED; >> - rte_spinlock_unlock(&hw->lock); >> - return ret; >> - } >> + if (ret) >> + goto map_rx_inter_err; >> >> /* >> * There are three register used to control the status of a TQP @@ -2241,19 +2248,12 @@ hns3vf_dev_start(struct >> rte_eth_dev *dev) >> * status of queue in the dpdk framework. >> */ >> ret = hns3_start_all_txqs(dev); >> - if (ret) { >> - hw->adapter_state = HNS3_NIC_CONFIGURED; >> - rte_spinlock_unlock(&hw->lock); >> - return ret; >> - } >> + if (ret) >> + goto map_rx_inter_err; >> >> ret = hns3_start_all_rxqs(dev); >> - if (ret) { >> - hns3_stop_all_txqs(dev); >> - hw->adapter_state = HNS3_NIC_CONFIGURED; >> - rte_spinlock_unlock(&hw->lock); >> - return ret; >> - } >> + if (ret) >> + goto start_all_rxqs_fail; >> >> hw->adapter_state = HNS3_NIC_STARTED; >> rte_spinlock_unlock(&hw->lock); >> @@ -2275,6 +2275,15 @@ hns3vf_dev_start(struct rte_eth_dev *dev) >> hns3_start_tqps(hw); >> >> return ret; >> + >> +start_all_rxqs_fail: >> + hns3_stop_all_txqs(dev); >> +map_rx_inter_err: >> + (void)hns3vf_do_stop(hns); >> + hw->adapter_state = HNS3_NIC_CONFIGURED; >> + rte_spinlock_unlock(&hw->lock); >> + >> + return ret; >> } >> >> static bool >> -- >> 2.7.4 > > Hi Min, > > Thanks for backporting, merged. > > Best Regards, > Xueming > . >