From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ferruh Yigit <ferruh.yigit@intel.com>
Subject: Re: Fwd: Memory Leak: PCAP TX to File
Date: Thu, 26 Oct 2017 17:24:01 -0700
Message-ID: <046bab26-6128-f220-34e9-d5806976e47e@intel.com>
References: <CAHzKxpb-d-naP+qJdG6DGa84z+DfcLSJkorH5K-j_+ebquOrDQ@mail.gmail.com>
 <CAHzKxpbykU5YfUbdkjt6s7N9a0d_4tpQJij0ek-yh-TNfVPqSA@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
To: Stefan Baranoff <sbaranoff@gmail.com>, dev@dpdk.org
Return-path: <dev-bounces@dpdk.org>
Received: from mga04.intel.com (mga04.intel.com [192.55.52.120])
 by dpdk.org (Postfix) with ESMTP id 1A08C1BB51
 for <dev@dpdk.org>; Fri, 27 Oct 2017 02:24:03 +0200 (CEST)
In-Reply-To: <CAHzKxpbykU5YfUbdkjt6s7N9a0d_4tpQJij0ek-yh-TNfVPqSA@mail.gmail.com>
Content-Language: en-US
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On 10/26/2017 10:44 AM, Stefan Baranoff wrote:
> All,
> 
> I think I've found a (very minor) memory leak in writing to a PCAP file. In
> drivers/net/pcap/rte_eth_pcap.c in open_single_tx_pcap around line 405 a
> pcap_t is allocated by pcap_open_dead but is never freed.

You are right.

> I see two obvious fixes:
>   1) Free the pcap_t immediately after either on pcap_dump_open success or
> failure
>   2) Set the pcap_t pointer internals->tx_queue[i]->tx->pcap so when the
> eth_dev_stop callback happens the pcap_t is freed.

Option 2 looks more proper, since eth_dev_stop() calls pcap_close() which frees
the pcap_t, so nothing special requires to cover the case,

But :),

open_single_tx_pcap() doesn't returns pcap_t back, so this function and
functions using this needs to be updated. I believe doesn't worth to this update
and as far as I can see freeing pcap_t immediately is safe,

So I would suggest lets go with option 1, it is simpler change.

Would you mind providing patch for this fix?

Thanks,
ferruh

> 
> I believe the first is safe but am not a libpcap guru. Can someone please
> confirm this?
> 
> 
> Thanks,
> Stefan
>