From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com [66.111.4.25]) by dpdk.org (Postfix) with ESMTP id 21C7F1BE02 for ; Wed, 4 Jul 2018 23:42:02 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 8D07421B8B; Wed, 4 Jul 2018 17:42:01 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Wed, 04 Jul 2018 17:42:01 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:content-transfer-encoding:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-sender :x-me-sender:x-sasl-enc; s=mesmtp; bh=oc8Kwf0wQ5VOvJphMIAbUp/TL3 c8cmSxMfoH68zzL5M=; b=B5nK1dwJUkam2MrHJOQMxA624SNNnEWn3uEN5pW6aE 5Eds6zeyZtJQJnIgSp79/87bM5C8Bw00FDVx16yNOOX+dyBHtUE1xTPN7Ic+nTWT JkU2p/6Z1y2r59kDmiV+qHv3IWkFoyflWQHdkn7Tdtyhex14/tDMGQ/ZuttsRLEw A= 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-sender:x-me-sender:x-sasl-enc; s=fm3; bh=oc8Kwf 0wQ5VOvJphMIAbUp/TL3c8cmSxMfoH68zzL5M=; b=lefv0E8kIDlefREyjHmKw8 V73UFUVULXWz2dz9XsNBGIQHThUu/oNFKElafi6vecQfafbLe22o7pBKFV0Q3CSe U1eVYvTUz3P/wA+7YkyLSBBvJrXTajB9xJKshH6F6Pxnx+whLW4p4P3VH0jaqsNA 9d1l1/xcpnSNGrstNOBPebIfHqFL5LtBfmVjEhhLErOTfFvLptJ0a7Uti5MkRUmh SqYEzaks3p2xy36Iw5jkGltj4TPdD4EmKQzYNixcPabgXtoMYJKrrW0qMTFfsmsp BUVnz5A/4VMg3cKai/KolUaIohDf9i7fPBimm8zxp7Y7rD8/JnYMhcpl+6RzaOWQ == X-ME-Proxy: X-ME-Sender: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id EF5CA10276; Wed, 4 Jul 2018 17:41:59 -0400 (EDT) From: Thomas Monjalon To: "Zhang, Qi Z" Cc: dev@dpdk.org, "Burakov, Anatoly" , "Ananyev, Konstantin" , "Richardson, Bruce" , "Yigit, Ferruh" , "Shelton, Benjamin H" , "Vangati, Narender" , arybchenko@solarflare.com Date: Wed, 04 Jul 2018 23:41:58 +0200 Message-ID: <6105228.vaBHSlUH0d@xps> In-Reply-To: <039ED4275CED7440929022BC67E706115324B015@shsmsx102.ccr.corp.intel.com> References: <20180607123849.14439-1-qi.z.zhang@intel.com> <4367091.6QlV0rL9d1@xps> <039ED4275CED7440929022BC67E706115324B015@shsmsx102.ccr.corp.intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v8 04/19] ethdev: introduce device lock 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, 04 Jul 2018 21:42:02 -0000 04/07/2018 12:49, Zhang, Qi Z: > From: Thomas Monjalon [mailto:thomas@monjalon.net] > > 04/07/2018 03:47, Zhang, Qi Z: > > > From: Thomas Monjalon [mailto:thomas@monjalon.net] > > > > 03/07/2018 17:08, Zhang, Qi Z: > > > > > From: Thomas Monjalon [mailto:thomas@monjalon.net] > > > > > > 02/07/2018 07:44, Qi Zhang: > > > > > > > Introduce API rte_eth_dev_lock and rte_eth_dev_unlock to let > > > > > > > application lock or unlock on specific ethdev, a locked device > > > > > > > can't be detached, this help applicaiton to prevent unexpected > > > > > > > device detaching, especially in multi-process envrionment. > > > > > > > > > > > > Trying to understand: a process of an application could try to > > > > > > detach a port while another process is against this decision. > > > > > > Why an application needs to be protected against itself? > > > > > > > > > > I think we can regard this as a help function, it help application > > > > > to simplified > > > > the situation when one process want to detach a device while another > > > > one is still using it. > > > > > Application can register a callback which can do to necessary > > > > > clean up (like > > > > stop traffic, release memory ...) before device be detached. > > > > > > > > Yes I agree such hook can be a good idea. [...] > > > > After all, it is just a pre-detach hook. > > > > > > > Wait, how is it different of RTE_ETH_EVENT_DESTROY callback? > > > > Perhaps we just need to improve the handling of the DESTROY event? > > > > > > I have thought about this before. > > > Not like RTE_ETH_EVENT_DESTROY and other event hook, the hook here > > need to give feedback, pass or fail will impact the following behavior, this > > make it special, so I separate it from all exist rte_eth_event_type handle > > mechanism. > > > > Look at _rte_eth_dev_callback_process, there is a "ret_param". > > OK, that should work. > > > > > The alternative solution is > > > we just introduce a new event type like RTE_ETH_EVENT_PRE_DETACH and > > > reuse all exist API > > rte_eth_dev_callback_register/rte_eth_dev_callback_unregister. > > > > I don't think we need a new event. > > Let's try to use RTE_ETH_EVENT_DESTROY. > > The problem is RTE_ETH_EVENT_DESTROY is used in rte_eth_dev_release_port already. > And in PMD, rte_eth_dev_release_port is called after dev_uninit, that mean its too late to reject a detach You're right. It's a real mess currently. The right order should be to remove ethdev ports before removing the underlying EAL device. But it's strangely not the case. We need to separate things. The function rte_eth_dev_close can be used to remove an ethdev port if we add a call to rte_eth_dev_release_port. So we could call rte_eth_dev_close in PMD remove functions. Is "close" a good time to ask confirmation to the application? Or should we ask confirmation a step before, on "stop"? > So , do you mean we can remove _rte_eth_dev_callback_process(RTE_ETH_EVENT_DESTROPY) in rte_eth_dev_release_port I would say we need RTE_ETH_EVENT_DESTROY to notify that the port is really destroyed. Maybe the right thing to do is to add a new event RTE_ETH_EVENT_CLOSE_REQUEST or something else. Note that we already have 2 removal events in ethdev: - RTE_ETH_EVENT_INTR_RMV when the port cannot be used anymore - RTE_ETH_EVENT_DESTROY when the port is going to be deleted > And where is right place to call _rte_eth_dev_callback_process(RTE_ETH_EVENT_DESTROY)? > If can't be called in rte_eth_dev_detach, because if device is removed by rte_eal_hotplug_remove, it will be skipped. No, rte_eth_dev_detach and rte_eal_hotplug_remove are 2 different things. One is a mix of ethdev and EAL (and should be deprecated), the other one is for the underlying device at EAL level. > probably we need to call this at the beginning of each PMD driver->remove?, that means, we need to change all PMD drivers? Yes, we can call rte_eth_dev_stop and rte_eth_dev_close at the beginning of PMD remove. Note that there is already a helper rte_eth_dev_destroy called in some PMD to achieve the removal, but curiously, it doesn't call stop and close functions.