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 E57035B3E for ; Wed, 17 Oct 2018 11:27:45 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 78D4F21FC5; Wed, 17 Oct 2018 05:27:45 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Wed, 17 Oct 2018 05:27:45 -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=XfNafsL5j7mSyCeTfwmp/BdahaRwJCz99Ucjj3tIUDg=; b=HMZYOGFmdMqG G+Pm3PUJVE9Sa1APgiTBoacGKsGt0xXsrA+2WnY2Y0XSRmwPox574BA5YfIewlqt I9hRqrlGnDg+L5nNze5Mv9NiGncspVqkJXbRphdciNKHlV6JPAbVKM1PXGW8i78q j1gbhLDwYpdodqi0xBO88ODqFQ12izY= 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=XfNafsL5j7mSyCeTfwmp/BdahaRwJCz99Ucjj3tIU Dg=; b=sDyeXmRQbyoDzPGufZLbsRiZ4LZSMoi8iEF9osIe2k3Ca5jeYg58Vjtpn /8j8zwxxJ6f1i0b33gCigZRd8cKGLNdntVVSOWXQ0ygsdLtOO4u+OouITtkTbvQS zsUaZ4Hj/IdBpcpNBUJgmx48zc7VIQSFd9HDpVu0mIuP8JJwySK3jYIcBeTCtaR3 GDQiOPZSmK4CGqMYVk6glDUDQC0MfJ0m+3YR4foeB45WZHHSr5pLr7oCBX9jJ32e IOq1FT6RcnPa6ksw9/NIEj1BNajLR+UBw56ydxqNHOf2x0V+xqyXprhJ4A5T8Cbx BXtAfkw4i4rmke3uWn7GDoDvo9Mog== 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 75601E4119; Wed, 17 Oct 2018 05:27:44 -0400 (EDT) From: Thomas Monjalon To: Andrew Rybchenko Cc: ferruh.yigit@intel.com, dev@dpdk.org, ophirmu@mellanox.com Date: Wed, 17 Oct 2018 11:27:47 +0200 Message-ID: <4266738.Sgcdpvvvgs@xps> In-Reply-To: References: <20180907233929.21950-1-thomas@monjalon.net> <20181017015450.15783-4-thomas@monjalon.net> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v3 3/4] ethdev: remove release function for secondary process 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 09:27:46 -0000 17/10/2018 09:25, Andrew Rybchenko: > On 10/17/18 4:54 AM, Thomas Monjalon wrote: > > After previous changes, the function rte_eth_dev_release_port() > > can be used for primary or secondary process as well. > > The only difference with rte_eth_dev_release_port_secondary() > > is the shared lock used in rte_eth_dev_release_port(). > > > > The function rte_eth_dev_release_port_secondary() was recently > > added in 18.11 cycle. > > > > Signed-off-by: Thomas Monjalon > > I really like it. Few notes below. > > > diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c > > index b77283ae7..c9bf15d75 100644 > > --- a/drivers/net/null/rte_eth_null.c > > +++ b/drivers/net/null/rte_eth_null.c > > @@ -679,9 +679,6 @@ rte_pmd_null_remove(struct rte_vdev_device *dev) > > if (eth_dev == NULL) > > return -1; > > > > - if (rte_eal_process_type() != RTE_PROC_PRIMARY) > > - return rte_eth_dev_release_port_secondary(eth_dev); > > - > > /* mac_addrs must not be freed alone because part of dev_private */ > > eth_dev->data->mac_addrs = NULL; > > I think it should be done in primary process only. > > > rte_eth_dev_release_port(eth_dev); > > diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c > > index 1790a8064..bfb5d710d 100644 > > --- a/drivers/net/pcap/rte_eth_pcap.c > > +++ b/drivers/net/pcap/rte_eth_pcap.c > > @@ -1228,9 +1228,6 @@ pmd_pcap_remove(struct rte_vdev_device *dev) > > if (eth_dev == NULL) > > return -1; > > > > - if (rte_eal_process_type() != RTE_PROC_PRIMARY) > > - return rte_eth_dev_release_port_secondary(eth_dev); > > - > > internals = eth_dev->data->dev_private; > > if (internals != NULL && internals->phy_mac == 0) > > /* not dynamically allocated, must not be freed */ > > Like above I think that assignment (which follows) should be done > in the primary process only. I am not sure it has a real impact, because all processes are removing the device which should be already stopped. But OK, I will avoid changing shared data in secondary process.