[dpdk-dev] [PATCH v2] driver/net/pcap fix: fd leak bug

[dpdk-dev] [PATCH v2] driver/net/pcap fix: fd leak bug

[ZhangTengfei]

pcap fd was opend when vdev probed,
but not closed when vdev removed.
This bug appears in dpdk-pdump

Signed-off-by: ZhangTengfei <zypscode@outlook.com>
---
 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)
-- 
2.26.2

Re: [dpdk-dev] [PATCH v2] driver/net/pcap fix: fd leak bug

[Ferruh Yigit]

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 <zypscode@outlook.com>

Would you mind using the sign-off as:
"Tengfei Zhang <zypscode@outlook.com>

> ---
>   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;
          }