From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by dpdk.org (Postfix) with ESMTP id CC9C07D34 for ; Wed, 17 Oct 2018 10:22:32 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 7194A21F56; Wed, 17 Oct 2018 04:22:32 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Wed, 17 Oct 2018 04:22:32 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=mesmtp; bh=eY4JkpNSIPGF2nw9fPrOhyQhSUq6hpU4kNY8yImdsEM=; b=puAFgQv2X3W4 gJi0wAdxQnEPaPDeM9xttJhg3hVUXzx11kOUarKTNl+7lmXWE4CHg+xmr2g8jkwq OP16+Eix40I4ams+1d4RHnUXJubKiOYHVvGEcrHIn5+jTd5swdiVyIk9SKteTNeV WuXq/tY+9xg1y/FT24uHy46cb2FRNyk= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; bh=eY4JkpNSIPGF2nw9fPrOhyQhSUq6hpU4kNY8yImds EM=; b=b22fUT2xRp60mkTlr/uuKGY5V5VpHmNwB++Z9gBHl/DHC2QeS87ORaJ9A MMEfCZw9VjQ7qkvxE3bQm/WKwAu/iOmEdz7Ux/QcurpI6WYylGKEHLjLPckWdQaN dBdJFDu+EnbOrvySXMaHNvjEHExtkJmnXAeCSMe3H1ZidE7AIgRTydCBIJw/n+ds 0fTifL3bwJylJsC6YdGwn4wiPYOeMh+FNlizL/tFs+IFbzDIFb1EpsIBAHqyONKH 2jBxW7ezy9fBcObPSvGPjCHqlnmqbbN43xlnIjcxHDSNQtmPr09nZNNVniylLQL/ 8ivNUVZfhnU2tRQiylOc1Ljod0EUA== X-ME-Sender: X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 435C3E4071; Wed, 17 Oct 2018 04:22:31 -0400 (EDT) From: Thomas Monjalon To: Andrew Rybchenko Cc: ferruh.yigit@intel.com, dev@dpdk.org, ophirmu@mellanox.com Date: Wed, 17 Oct 2018 10:22:34 +0200 Message-ID: <17711677.o5lTiYJ96P@xps> In-Reply-To: <7fcb80d5-7abd-3034-f271-bb87fea72de4@solarflare.com> References: <20180907233929.21950-1-thomas@monjalon.net> <20181017015450.15783-3-thomas@monjalon.net> <7fcb80d5-7abd-3034-f271-bb87fea72de4@solarflare.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v3 2/4] ethdev: free all common data when releasing port X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 17 Oct 2018 08:22:33 -0000 17/10/2018 09:13, Andrew Rybchenko: > On 10/17/18 4:54 AM, Thomas Monjalon wrote: > > This is a clean-up of common ethdev data freeing. > > All data freeing are moved to rte_eth_dev_release_port() > > and done only in case of primary process. > > > > It is probably fixing some memory leaks for PMDs which were > > not freeing all data. > > > > Signed-off-by: Thomas Monjalon > > [...] > > > diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h > > index ca31b5777..a1398a80c 100644 > > --- a/lib/librte_ethdev/rte_ethdev_driver.h > > +++ b/lib/librte_ethdev/rte_ethdev_driver.h > > @@ -58,7 +58,16 @@ struct rte_eth_dev *rte_eth_dev_attach_secondary(const char *name); > > > > /** > > * @internal > > - * Release the specified ethdev port. > > + * Notify RTE_ETH_EVENT_DESTROY and release the specified ethdev port. > > + * > > + * The following data fields will be freed: > > + * - rx_queues > > + * - tx_queues > > I think rx_queues and tx_queues from should not be mentioned here. > They are managed by ethdev and there is no options here. > > > + * - mac_addrs > > + * - hash_mac_addrs > > + * - dev_private > > + * If one of these fields should not be freed, > > + * it must be reset to NULL by the caller. > > I'm afraid nobody will find it here. Let's add > @see rte_eth_dev_release_port() > to these members description in rte_ethdev_core.h > struct rte_eth_dev_data. > > Also I think "by the caller" is misleading here. > Let's highlight that it is PMD responsibility, i.e. > "by the PMD" may be with > "typically in dev_close method implementation". OK, thanks for the feedback.