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 529ABA054F; Tue, 2 Mar 2021 12:00:13 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 278244014E; Tue, 2 Mar 2021 12:00:13 +0100 (CET) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by mails.dpdk.org (Postfix) with ESMTP id CEF4040142 for ; Tue, 2 Mar 2021 12:00:11 +0100 (CET) IronPort-SDR: Ywms80J+drntSRBOMpWnzS+4Ie2lkQ3GULiJq0t2W7TaprGMhxuwJQCVeHufPMGGaow7vUA475 cxQ0Z526lCLA== X-IronPort-AV: E=McAfee;i="6000,8403,9910"; a="174407785" X-IronPort-AV: E=Sophos;i="5.81,216,1610438400"; d="scan'208";a="174407785" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Mar 2021 03:00:10 -0800 IronPort-SDR: 1pVsS7rODcjHDHVNXcRVHTRkgDA7/JAdI5NwMv/BO06WhxFp34l/D2KMwdXhZrJ8/rOzgFzUBc pR/COiaXFn5g== X-IronPort-AV: E=Sophos;i="5.81,216,1610438400"; d="scan'208";a="406662574" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.252.162]) ([10.213.252.162]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Mar 2021 03:00:09 -0800 To: ZhangTengfei Cc: dev@dpdk.org References: From: Ferruh Yigit X-User: ferruhy Message-ID: Date: Tue, 2 Mar 2021 11:00:06 +0000 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v2] driver/net/pcap fix: fd leak bug 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 3/1/2021 3:30 PM, ZhangTengfei wrote: > pcap fd was opend when vdev probed, > but not closed when vdev removed. > This bug appears in dpdk-pdump The patch title should have "net/pcap: " as module name instead of "driver/net/pcap: ", suggested title: net/pcap: fix fd leak on uninit Also needs fixes line, I guess following can be used: Fixes: c956caa6eabf ("pcap: support port hotplug") Cc: stable@dpdk.org > > Signed-off-by: ZhangTengfei Would you mind using the sign-off as: "Tengfei Zhang > --- > drivers/net/pcap/rte_eth_pcap.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c > index 90f5d75ea..396de29de 100644 > --- a/drivers/net/pcap/rte_eth_pcap.c > +++ b/drivers/net/pcap/rte_eth_pcap.c > @@ -755,6 +755,8 @@ eth_dev_close(struct rte_eth_dev *dev) > PMD_LOG(INFO, "Closing pcap ethdev on NUMA socket %d", > rte_socket_id()); > > + eth_dev_stop(dev); > + > rte_free(dev->process_private); > > if (rte_eal_process_type() != RTE_PROC_PRIMARY) > Now since 'eth_dev_stop()' can be called multiple times with this change, 'pcap_close()' is causing a crash when it gets NULL pointer, following check is required, can you please send a new version with it? --- a/drivers/net/pcap/rte_eth_pcap.c +++ b/drivers/net/pcap/rte_eth_pcap.c @@ -682,9 +682,11 @@ eth_dev_stop(struct rte_eth_dev *dev) /* Special iface case. Single pcap is open and shared between tx/rx. */ if (internals->single_iface) { queue_missed_stat_on_stop_update(dev, 0); - pcap_close(pp->tx_pcap[0]); - pp->tx_pcap[0] = NULL; - pp->rx_pcap[0] = NULL; + if (pp->tx_pcap[0] != NULL) { + pcap_close(pp->tx_pcap[0]); + pp->tx_pcap[0] = NULL; + pp->rx_pcap[0] = NULL; + } goto status_down; }