From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [PATCH v6 1/3] ether: support runtime queue setup
Date: Tue, 10 Apr 2018 15:59:03 +0200
Message-ID: <9581683.2joz8NotYG@xps>
References: <20180212045314.171616-1-qi.z.zhang@intel.com>
 <20180408024221.228450-1-qi.z.zhang@intel.com>
 <20180408024221.228450-2-qi.z.zhang@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7Bit
Cc: dev@dpdk.org, konstantin.ananyev@intel.com, beilei.xing@intel.com,
 jingjing.wu@intel.com, wenzhuo.lu@intel.com
To: Qi Zhang <qi.z.zhang@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from out5-smtp.messagingengine.com (out5-smtp.messagingengine.com
 [66.111.4.29]) by dpdk.org (Postfix) with ESMTP id 31C0B1B8D9
 for <dev@dpdk.org>; Tue, 10 Apr 2018 15:59:06 +0200 (CEST)
In-Reply-To: <20180408024221.228450-2-qi.z.zhang@intel.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

Hi,

Please replace ether and etherdev by ethdev (in title and text).

08/04/2018 04:42, Qi Zhang:
> The patch let etherdev driver expose the capability flag through
> rte_eth_dev_info_get when it support runtime queue configuraiton,

typo: configuration

> then base on the flag rte_eth_[rx|tx]_queue_setup could decide
> continue to setup the queue or just return fail when device already
> started.

Generally speaking, it is easier to read when broke in several sentences,
and starting with the problem statement.
Example:
"
It is not possible to setup a queue when the port is started
because of a check in ethdev layer.
New capability flags are added in order to relax this check
for devices which support queue setup in runtime.
The functions rte_eth_[rx|tx]_queue_setup will raise an error only
if the port is started and runtime setup of queue is not supported.
"
> 
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> ---
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -981,6 +981,11 @@ struct rte_eth_conf {
>   */
>  #define DEV_TX_OFFLOAD_SECURITY         0x00020000
>  
> +#define DEV_RUNTIME_RX_QUEUE_SETUP 0x00000001
> +/**< Deferred setup rx queue */
> +#define DEV_RUNTIME_TX_QUEUE_SETUP 0x00000002
> +/**< Deferred setup tx queue */

Please use RTE_ETH_ prefix.

>  /*
>   * If new Tx offload capabilities are defined, they also must be
>   * mentioned in rte_tx_offload_names in rte_ethdev.c file.
> @@ -1029,6 +1034,8 @@ struct rte_eth_dev_info {
>  	/** Configured number of rx/tx queues */
>  	uint16_t nb_rx_queues; /**< Number of RX queues. */
>  	uint16_t nb_tx_queues; /**< Number of TX queues. */
> +	uint64_t runtime_queue_setup_capa;
> +	/**< queues can be setup after dev_start (DEV_DEFERRED_). */

Why using uint64_t for that?
Maybe these flags can find another place, less specific.
What about a field for all setup capabilities? setup_capa?