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 E4C065F2C for ; Wed, 17 Oct 2018 09:31:07 +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-us4.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id 17284B41E0F; Wed, 17 Oct 2018 07:26:00 +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, 17 Oct 2018 08:25:55 +0100 To: Thomas Monjalon , CC: , References: <20180907233929.21950-1-thomas@monjalon.net> <20181017015450.15783-1-thomas@monjalon.net> <20181017015450.15783-4-thomas@monjalon.net> From: Andrew Rybchenko Message-ID: Date: Wed, 17 Oct 2018 10:25:14 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.0 MIME-Version: 1.0 In-Reply-To: <20181017015450.15783-4-thomas@monjalon.net> Content-Language: en-GB 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-24160.003 X-TM-AS-Result: No-13.482600-8.000000-10 X-TMASE-MatchedRID: HXSqh3WYKfsOwH4pD14DsPHkpkyUphL9gRykyfrH1xnfTFKBkQVCsmOr KvsUT0pA5s1otBC8xHlKhBUJyxjhBuG1W8jesiDJFYJUGv4DL3w38FNTll9lElfXgfL55invidc YzIGzqIakRgEg7nrRyuutPg6r34pTgiIO7Sf/7rE00dkxYNMRt4n4DdeD/uLNw7+XQ3Lk9nkahn O4XreYZseWUHEpxPJFHcN4x+ltYL3oqGQsGM83sAcbMHjYNxGhhZApJAdFDDabKItl61J/ycnjL TA/UDoAoTCA5Efyn8AiEOZmeUqhz/P/AZVBOaCoviHrV6agm1Z3HztLx5fSpgOceLdbVCEiuAiY uUJqBm/1Z8v+FnYS5LlHhpdapwpSUFbgXAICjVLjhtyIAnzrZxyFdNnda6Rv X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--13.482600-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24160.003 X-MDID: 1539761467-iw8xpjz4PV6q 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] [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 07:31:08 -0000 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.