From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [148.163.129.52]) by dpdk.org (Postfix) with ESMTP id C0FEA34F3 for ; Wed, 12 Sep 2018 17:44:54 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mx1-us1.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id DA231B40062; Wed, 12 Sep 2018 15:44:52 +0000 (UTC) Received: from [192.168.38.17] (91.220.146.112) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Wed, 12 Sep 2018 16:44:48 +0100 To: Thomas Monjalon CC: , References: <20180907233929.21950-1-thomas@monjalon.net> <2007089.03mgKk1tIn@xps> <402147ea-d88a-27a0-25a9-905c79fb5b89@solarflare.com> <3176257.grtejztXq6@xps> From: Andrew Rybchenko Message-ID: <69a4709e-c588-69a9-c916-15094eed5a6e@solarflare.com> Date: Wed, 12 Sep 2018 18:44:17 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <3176257.grtejztXq6@xps> Content-Language: en-US X-Originating-IP: [91.220.146.112] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-12.5.0.1300-8.5.1010-24088.003 X-TM-AS-Result: No-15.347300-8.000000-10 X-TMASE-MatchedRID: oHOSwQSJZWgOwH4pD14DsPHkpkyUphL9wuIWIvQEbW6vPoND+wakFlPK Q4g0ENBTz0kHebusWqG/OcTZGXKzdinPugGsN3p5bc297PAGtWbhKQh1LCmGBk+nXRcya9Nvq4N L3KtjQybxF3lBuZS3jYGwaIKd6VKF9rhTO8T5ktELPVZHwod7gPiH64jt3FfETmPQ6wyP0TowV1 2/6ktut+pwPHdvzh+MTZc1hGgdJWRl90qXRtPpVqMVgdN9w+TCcmsHQK7cMOdJurFUBN2cx3IxC LRqRqKfudHLZ4uowZpi/hfJByKMq31GcR5AeEs79AByWqqZEHw6QNs2WCY79eiMPdrmcZwr807t +VEWpeYgZIcq73TEpZJTzjwz+Gzox7hajJv6RUXIOn6NK8S1ayXdp9l6EkRZ52VTYrkmb1ssyVh q83/5Ul1AnRO85J+uFo5J0JpbFBvfX2sZfyOJoZ4CIKY/Hg3AtOt1ofVlaoLUHQeTVDUrIke0Gx /n9RcliiIrjNyRrTGKowa2iCkhfr3RMJ4u0pk5jySIokzxj/ZXGxSWEZv/l9PFNDPt5/s4aBqLO 3vOm6dZcTENjpZm6WZGvrPYcj1uV26yNZKOwVYfwV6sBPR0lg== X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--15.347300-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24088.003 X-MDID: 1536767093-Ah-zLSXOk_1l Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [RFC] ethdev: complete closing to free all resources 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, 12 Sep 2018 15:44:55 -0000 On 09/12/2018 05:57 PM, Thomas Monjalon wrote: > 10/09/2018 10:54, Andrew Rybchenko: >> On 09/10/2018 11:42 AM, Thomas Monjalon wrote: >>> 10/09/2018 10:03, Andrew Rybchenko: >>>> On 09/08/2018 02:39 AM, Thomas Monjalon wrote: >>>>> After closing a port, it cannot be restarted. >>>>> So there is no reason to not free all associated resources. >>>>> >>>>> The last step was done with rte_eth_dev_detach() which is deprecated. >>>>> Instead of removing the associated rte_device, the driver should check >>>>> if no more port (ethdev, cryptodev, etc) is still open for the device. >>>>> Then the device resources can be freed by the driver inside the >>>>> dev_close() driver callback operation. >>>>> >>>>> The last ethdev freeing (dev_private and final release), which were done >>>>> by rte_eth_dev_detach(), are now done at the end of rte_eth_dev_close(). >>>> For me, it sounds more logical to kill dev_close and keep detach. >>>> IMHO, dev_close is artificial and hardly useful. detach is a local pair >>>> to attach. >>> I don't get your point. >>> >>> In order to free a port, we need close + detach. >>> We can keep only one. >>> I choose close because: >>> 1) attach/detach are deprecated >>> 2) probe/close is a more obvious pair >>> 3) we need the driver to free the lower level resources >> Yes, I'm sorry I used bad terminology. >> We have probe/remove pair for both PCI and vdev drivers and I mean >> that remove is a better candidate to be kept (as a pair for probe which >> allocates all resources). > OK, yes probe/remove is the pair at EAL level. > But if we want to request removal at ethdev level, rte_eth_dev_close > is the function. > > Note that there is no function to request creation of an ethdev port. > Adding a new port is done only by the PMD during probing (rte_bus level). The overall picture is too vague. May be it is simply incomplete looking at the patch only. I understand that restructuring is required looking at rte_eth_dev_destroy() (which is used by i40e and ixgbe drivers only) and rte_eth_dev_pci_release() used by other PCI drivers. Right now it looks symmetric at least on drivers level which provides init callback to probe to do driver-specific job and uninit callback to remove to free driver-specific resources. I can't say if the patch is right or wrong direction since final design is unclear.