From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <thomas@monjalon.net>
Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com
 [66.111.4.25]) by dpdk.org (Postfix) with ESMTP id 162A41BE44
 for <dev@dpdk.org>; Tue,  3 Jul 2018 11:21:31 +0200 (CEST)
Received: from compute1.internal (compute1.nyi.internal [10.202.2.41])
 by mailout.nyi.internal (Postfix) with ESMTP id A0EDA21F1B;
 Tue,  3 Jul 2018 05:21:30 -0400 (EDT)
Received: from mailfrontend2 ([10.202.2.163])
 by compute1.internal (MEProxy); Tue, 03 Jul 2018 05:21:30 -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=StQDoOO03pmzPZyqThHXPng4dY
 sNGw6fn8cAEtftMG8=; b=J4vGnQolx7vUYVQAAEUKX5SNhQXf9L0mx8hUrKZce8
 lQzznWOVlVizn2Zw3DKNlbeqtOjI7vV3aoPc3dfzUp6AtYkBG0OuoyIm9IRTJtyv
 XxWdPXvQipWj4Haa/4it+GH0joCrkN8TPPdmbd6jVLvpAeTvwW9C3l/Rr+H9Wvm0
 8=
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=StQDoO
 O03pmzPZyqThHXPng4dYsNGw6fn8cAEtftMG8=; b=j7jschrc+6epltVXIGgH3i
 QPw5m1+L7ZYXHFuHn3shCF0iLbQiT6KhPQMeNxqCXx9UAxY5487Z2njBZqDUuoJ5
 I2YyDMUJbflbqFcPHOr/HFAgaSx/RoReecOv5437W2cU4vUV7cOkUgLfjsOeUetH
 N6+jkPhLPxvhAi3aSsOhgiCzZiJ4IV5Zeqew8150MQQ7443ynlhW9sL7PpE1rKsr
 HIUiaHlGyIFy9Ct9FX3YpofH4Dzt/71FVzVGXOdZNZXSGNn1jgUdASTXYXENe13S
 Q49tV7DnusePD7BsGNvzvUpDdfG0mg8AZwRcCPmxexiL4LxI0Tjw09jLof0HkA5Q
 ==
X-ME-Proxy: <xmx:GkA7W1ccgd-qpkGtxYF52orRKquh4wHqXu3-Skx6ofEOVD6oQcQHxQ>
 <xmx:GkA7W-zGGaFnHd8YpmqwJVS6n9jWQgkRNreSxl9DBVEUDW8_4FUHew>
 <xmx:GkA7W2XDhvUQJAwPp1IxrmMWMLCa_D9yxKSPWwdX90sS8tfUBiRqAA>
 <xmx:GkA7W2SqqOviaeCvzGGiteq9H7iHv44OwP8A5mTM-COBZre6jL6CJQ>
 <xmx:GkA7W1dyaKJS8T3hYJ4LDewqKQyq7V5g4XXIYssRHl7d1bRC1uFoag>
 <xmx:GkA7W0PQgaI0cKAc-nbNnZE_c5u-Clk6ho4j9C_FHaVMLEDBvt0lGQ>
X-ME-Sender: <xms:GkA7W04gy-20oUmxRC783jsE7_6OJqIclQ1moIq-fd077Vj038b0Ew>
Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184])
 by mail.messagingengine.com (Postfix) with ESMTPA id 5E58510255;
 Tue,  3 Jul 2018 05:21:29 -0400 (EDT)
From: Thomas Monjalon <thomas@monjalon.net>
To: Qi Zhang <qi.z.zhang@intel.com>
Cc: dev@dpdk.org, anatoly.burakov@intel.com, konstantin.ananyev@intel.com,
 bruce.richardson@intel.com, ferruh.yigit@intel.com,
 benjamin.h.shelton@intel.com, narender.vangati@intel.com
Date: Tue, 03 Jul 2018 11:21:27 +0200
Message-ID: <1911216.OZSudRO5zr@xps>
In-Reply-To: <20180702054450.29269-2-qi.z.zhang@intel.com>
References: <20180607123849.14439-1-qi.z.zhang@intel.com>
 <20180702054450.29269-1-qi.z.zhang@intel.com>
 <20180702054450.29269-2-qi.z.zhang@intel.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Subject: Re: [dpdk-dev] [PATCH v8 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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Tue, 03 Jul 2018 09:21:31 -0000

Hi,

02/07/2018 07:44, Qi Zhang:
> --- a/lib/librte_ethdev/rte_ethdev_driver.h
> +++ b/lib/librte_ethdev/rte_ethdev_driver.h
> @@ -70,6 +70,19 @@ int rte_eth_dev_release_port(struct rte_eth_dev *eth_dev);
>  
>  /**
>   * @internal
> + * Release the specified ethdev port in local process, only set to ethdev
> + * state to unused, but not reset share data since it assume other process
> + * is still using it, typically it is called by secondary process.

Please check grammar in doxygen comments, and do not make long sentences.

"only set to ethdev state to unused" -> "only set ethdev state to unused"
"share" -> "shared"
"it assume" -> "it assumes"

Please split sentences with a dot and an uppercase.
I expect all patches of the series must be reviewed for english wording.

> + *
> + * @param eth_dev
> + * The *eth_dev* pointer is the address of the *rte_eth_dev* structure.

This comment is useless.
You can just say "Device to be detached".

> + * @return
> + *   - 0 on success, negative on error
> + */
> +int rte_eth_dev_release_port_private(struct rte_eth_dev *eth_dev);


[...]
> --- a/lib/librte_ethdev/rte_ethdev_pci.h
> +++ b/lib/librte_ethdev/rte_ethdev_pci.h
> @@ -197,6 +197,9 @@ rte_eth_dev_pci_generic_remove(struct rte_pci_device *pci_dev,
> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> +		return rte_eth_dev_release_port_private(eth_dev);

This is a behaviour change.
It means a PCI device cannot be globally detached directly by a secondary.
It must be justified by a comment in the code.

Please explain.