From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [67.231.154.164]) by dpdk.org (Postfix) with ESMTP id 8212A1B5CE for ; Thu, 12 Jul 2018 11:49:47 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1-us3.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id 09229980058; Thu, 12 Jul 2018 09:49:46 +0000 (UTC) Received: from [192.168.1.16] (85.187.13.33) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1044.25; Thu, 12 Jul 2018 10:49:38 +0100 To: "Zhang, Qi Z" , "thomas@monjalon.net" , "Burakov, Anatoly" CC: "Ananyev, Konstantin" , "dev@dpdk.org" , "Richardson, Bruce" , "Yigit, Ferruh" , "Shelton, Benjamin H" , "Vangati, Narender" References: <20180607123849.14439-1-qi.z.zhang@intel.com> <20180711030917.181098-1-qi.z.zhang@intel.com> <20180711030917.181098-2-qi.z.zhang@intel.com> <0a0c7b4a-3f7d-8b5e-3e76-8b631edd1997@solarflare.com> <039ED4275CED7440929022BC67E7061153258BAE@SHSMSX103.ccr.corp.intel.com> <3a434539-2d48-2539-b970-6823add74d89@solarflare.com> <039ED4275CED7440929022BC67E7061153259350@SHSMSX103.ccr.corp.intel.com> From: Andrew Rybchenko Message-ID: <84538b68-95d3-9624-25bb-cc8271f70b9f@solarflare.com> Date: Thu, 12 Jul 2018 12:49:32 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <039ED4275CED7440929022BC67E7061153259350@SHSMSX103.ccr.corp.intel.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US X-Originating-IP: [85.187.13.33] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-11.0.0.1191-8.100.1062-23962.003 X-TM-AS-Result: No--14.702300-0.000000-31 X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-MDID: 1531388986-D8ijOVxIjo-o Subject: Re: [dpdk-dev] [PATCH v11 01/19] ethdev: add function to release port in local 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: Thu, 12 Jul 2018 09:49:47 -0000 On 12.07.2018 03:23, Zhang, Qi Z wrote: > >> -----Original Message----- >> From: Andrew Rybchenko [mailto:arybchenko@solarflare.com] >> Sent: Thursday, July 12, 2018 12:05 AM >> To: Zhang, Qi Z ; thomas@monjalon.net; Burakov, >> Anatoly >> Cc: Ananyev, Konstantin ; dev@dpdk.org; >> Richardson, Bruce ; Yigit, Ferruh >> ; Shelton, Benjamin H >> ; Vangati, Narender >> >> Subject: Re: [dpdk-dev] [PATCH v11 01/19] ethdev: add function to release port >> in local process >> >> On 11.07.2018 15:30, Zhang, Qi Z wrote: >>>> -----Original Message----- >>>> From: Andrew Rybchenko [mailto:arybchenko@solarflare.com] >>>> Sent: Wednesday, July 11, 2018 5:27 PM >>>> To: Zhang, Qi Z ; thomas@monjalon.net; Burakov, >>>> Anatoly >>>> Cc: Ananyev, Konstantin ; dev@dpdk.org; >>>> Richardson, Bruce ; Yigit, Ferruh >>>> ; Shelton, Benjamin H >>>> ; Vangati, Narender >>>> >>>> Subject: Re: [dpdk-dev] [PATCH v11 01/19] ethdev: add function to >>>> release port in local process >>>> >>>> On 11.07.2018 06:08, Qi Zhang wrote: >>>>> Add driver API rte_eth_release_port_private to support the case when >>>>> an ethdev need to be detached on a secondary process. >>>>> Local state is set to unused and shared data will not be reset so >>>>> the primary process can still use it. >>>>> >>>>> Signed-off-by: Qi Zhang >>>>> Reviewed-by: Andrew Rybchenko >>>>> Acked-by: Remy Horton >>>>> --- >>> <...> >>>>> + /** >>>>> + * PCI device can only be globally detached directly by a >>>>> + * primary process. In secondary process, we only need to >>>>> + * release port. >>>>> + */ >>>>> + if (rte_eal_process_type() != RTE_PROC_PRIMARY) >>>>> + return rte_eth_dev_release_port_private(eth_dev); >>>> I've realized that some uninit functions which will not be called >>>> anymore in secondary processes have check for process type and >>>> handling of secondary process case. It makes code inconsistent and should >> be fixed. >>> Good point, I did a scan and check all the places that >> rte_eth_dev_pci_generic_remove be involved. >>> I found only sfc driver (sfc_eth_dev_unit) will call some cleanup on >> secondary process as below. >> >> The patch makes impossible dev_uninit to be executed for secondary process >> for all cases if rte_eth_dev_pci_generic_remove() is used. However, many >> drivers still check for process type. Yes, sfc does cleanup, but some drivers >> return -EPERM, some return 0. In fact it does not matter. It leaves dead code >> which is really confusing. > OK, l can do a cleanup in a separate patchset if this one will be merged. For now, I'd like to revoke my Reviewed-by. I'll review once again when complete solution is suggested.