From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 2C12AA04C9; Mon, 14 Sep 2020 00:27:37 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id B02782C52; Mon, 14 Sep 2020 00:27:36 +0200 (CEST) Received: from wout2-smtp.messagingengine.com (wout2-smtp.messagingengine.com [64.147.123.25]) by dpdk.org (Postfix) with ESMTP id D5DF41DB9 for ; Mon, 14 Sep 2020 00:27:34 +0200 (CEST) Received: from compute7.internal (compute7.nyi.internal [10.202.2.47]) by mailout.west.internal (Postfix) with ESMTP id 845795E8; Sun, 13 Sep 2020 18:27:33 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute7.internal (MEProxy); Sun, 13 Sep 2020 18:27:34 -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=fm2; bh= k4e/PescJEwcabWhoZhI7iPutD4sKIJCvJB/5zpUkKY=; b=cU2j7e1uvw4TIJB/ nyBCcHE6QaUGA/ip4OHOKt2vu6tB7XQqQeTuS20Ig31RMvIVABmzU8uHkVSxT5Yc LJGyVvz8pBzqhekQTHLPm7+7MOUk4a7Hu43fpafyZc5bHmE6Hlsls4WHc2UarcUY nk/HeM+LjKhaDkgd1WAnQ5Wvz7Q0A2pQlA4fqTGltN/Tf6nGCZgB10GThZox8xDY 5TBsLM8p819LewlvbwNDVGVdrCJG150oZh0QMszfL1soJqopbHpU3zz5Um9ivoXd MM6GwXETnvDPyzhgc1AcKpqY8NFa3FB9x45EUdO3v0pqKshbtGKL/Dfm/D+9iy8T baSM/Q== 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=fm3; bh=k4e/PescJEwcabWhoZhI7iPutD4sKIJCvJB/5zpUk KY=; b=CaGrTJt7O7DFJQgA1U/26emwAtUS5r8Cxzpn+O6gcHc0Sl9f0DsdT87XY grIpPYtk/SGcmbrE/ApFfjHmPLl1ISj3r4xXAgsKunHjr/TdeOAzhFIjK7dmxe7Z XbbifdUfGvqDkB8BnExMPwucD8zk48OsbwVoAdJT27yZs/BvzQjCxVccRGimVqYS ovYbgHW67doH8rd7QquFp68P6OuKO5S3LPCzKi3ItBQydy9YvWL0vxY8ofCXCyQX aCvaibdvcvdOQJHfIThlhbOxrxVLTYLvkT3BAVT7GwAYdrSbtlrVLGz/1qWd7v03 FmQyCWEj7nxiWy1I0jowqXlfARN4A== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduiedrudeihedgtdelucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg ftrfgrthhtvghrnhepffdvffejueetleefieeludduuefgteejleevfeekjeefieegheet ffdvkeefgedunecuffhomhgrihhnpeguphgukhdrohhrghenucfkphepjeejrddufeegrd dvtdefrddukeegnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhf rhhomhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth 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 095343064674; Sun, 13 Sep 2020 18:27:31 -0400 (EDT) From: Thomas Monjalon To: Yunjian Wang Cc: dev@dpdk.org, ferruh.yigit@intel.com, jerry.lilijun@huawei.com, xudingke@huawei.com, keith.wiles@intel.com Date: Mon, 14 Sep 2020 00:27:30 +0200 Message-ID: <6549777.GFsRTjX4uZ@thomas> In-Reply-To: <2500283.dqYlU2TMh5@thomas> References: <9a94f272bc7c1e5650295ddf3e02fb19b15e7951.1598596042.git.wangyunjian@huawei.com> <2500283.dqYlU2TMh5@thomas> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH] net/tap: release port upon close 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" As you may notice, I have included a slightly modified version of this patch in my series in order to cover the full picture: https://patches.dpdk.org/patch/77566/ Feel free to continue improving your patch in this thread or the other, as you prefer, as long as the secondary process issue is adressed. Thanks 13/09/2020 11:16, Thomas Monjalon: > Hi, > > 28/08/2020 14:37, wangyunjian: > > @@ -1078,6 +1085,23 @@ tap_dev_close(struct rte_eth_dev *dev) > > + > > + /* mac_addrs must not be freed alone because part of dev_private */ > > + dev->data->mac_addrs = NULL; > > + > > + internals = dev->data->dev_private; > > + TAP_LOG(DEBUG, "Closing %s Ethernet device on numa %u", > > + tuntap_types[internals->type], rte_socket_id()); > > + > > + if (internals->ioctl_sock != -1) { > > + close(internals->ioctl_sock); > > + internals->ioctl_sock = -1; > > + } > > + rte_free(dev->process_private); > > + dev->process_private = NULL; > > + if (tap_devices_count == 1) > > + rte_mp_action_unregister(TAP_MP_KEY); > > + tap_devices_count--; > > } > [...] > > @@ -2446,12 +2466,11 @@ static int > > rte_pmd_tap_remove(struct rte_vdev_device *dev) > > { > > > > struct rte_eth_dev *eth_dev = NULL; > > > > - struct pmd_internals *internals; > > > > /* find the ethdev entry */ > > eth_dev = rte_eth_dev_allocated(rte_vdev_device_name(dev)); > > if (!eth_dev) > > > > - return -ENODEV; > > + return 0; > > > > /* mac_addrs must not be freed alone because part of dev_private */ > > eth_dev->data->mac_addrs = NULL; > > The line above can be removed because mac_addrs is not freed > in secondary process, and it is redundant with tap_dev_close(). > > > > > if (rte_eal_process_type() != RTE_PROC_PRIMARY) > > return rte_eth_dev_release_port(eth_dev); > > There is an inconsistency in secondary process > if tap_dev_close() is not called from .remove (unplug) > but can be called from .dev_close (rte_eth_dev_close). > > I think the process type check must be done inside tap_dev_close(), > so the process private resources can be freed. > > > > > tap_dev_close(eth_dev); > > > > This blank line can be removed in my opinion. > > > - internals = eth_dev->data->dev_private; > > - TAP_LOG(DEBUG, "Closing %s Ethernet device on numa %u", > > - tuntap_types[internals->type], rte_socket_id()); > > - > > - close(internals->ioctl_sock); > > - rte_free(eth_dev->process_private); > > - if (tap_devices_count == 1) > > - rte_mp_action_unregister(TAP_MP_KEY); > > - tap_devices_count--; > > rte_eth_dev_release_port(eth_dev); > > > > return 0;