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

[Ferruh Yigit]

On 4/10/2018 2:59 PM, Thomas Monjalon wrote:
> 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 at intel.com>
>> Acked-by: Konstantin Ananyev <konstantin.ananyev at 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?

I was about to make similar comment, why not start a more generic capabilities
variable [1].
And make flag values more generic like: "DEV_CAPA_RUNTIME_RX_QUEUE_SETUP" etc ..


[1]
Previously a few times mentioned that there is no way for application to get
device capabilities dynamically, that is true, with offload capabilities flag
this has been solved for offloading but still remaining as a generic issue.