Re: [dpdk-dev] [PATCH v6 1/3] ether: support runtime queue setup

[Thomas Monjalon <thomas@monjalon.net>]

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?