[dpdk-dev] [PATCH v2 1/6] ethdev: added switch_domain and representor port flag

[Mohammad Abdul Awal]

On 20/11/2017 19:57, Ferruh Yigit wrote:
> On 11/17/2017 6:42 AM, Mohammad Abdul Awal wrote:
>> switch_domain attribute has been added to specify that a rte_eth_dev
>> instance belongs to a switch domain in the software switch.
>>
>> RTE_ETH_DEV_REPRESENTOR_PORT has been defined to specify that a
>> rte_eth_dev instance is a representor device.
>>
>> Signed-off-by: Mohammad Abdul Awal <mohammad.abdul.awal at intel.com>
>> Signed-off-by: Declan Doherty <declan.doherty at intel.com>
>> ---
>>   lib/librte_ether/rte_ethdev.h | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
>> index 341c2d6..7e82c70 100644
>> --- a/lib/librte_ether/rte_ethdev.h
>> +++ b/lib/librte_ether/rte_ethdev.h
>> @@ -1047,6 +1047,7 @@ 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. */
>> +	uint16_t switch_domain; /**< Switch domain which port belongs to. */
> What to put or not into rte_eth_dev_info is not that clear, but recent
> description from Andrew [1] makes sense to me: using more device related static
> information in dev_info instead of dynamic data.
>
> There is an option to create a new API for this, what do you think?
>
> [1]
> http://dpdk.org/ml/archives/dev/2017-November/081868.html
>
>>   };
>>   
>>   /**
>> @@ -1810,6 +1811,7 @@ struct rte_eth_dev_data {
>>   	int numa_node;  /**< NUMA node connection */
>>   	struct rte_vlan_filter_conf vlan_filter_conf;
>>   	/**< VLAN filter configuration. */
>> +	uint16_t switch_domain; /**< Switch domain which port belongs to. */
> Can you please describe why "switch_domain" used for? In patchset only ixgbe
> uses it, and that is only for assign to dev_info, it is not clear why this is
> needed.

We kept it keeping in mind for future use, specially for switchdev API 
and OVS support. But for this patch it is not necessary to be here. We 
will remove it from here now. New patch will be submitted if needed in 
future.

>>   };
>>   
>>   /** Device supports link state interrupt */
>> @@ -1818,6 +1820,8 @@ struct rte_eth_dev_data {
>>   #define RTE_ETH_DEV_BONDED_SLAVE 0x0004
>>   /** Device supports device removal interrupt */
>>   #define RTE_ETH_DEV_INTR_RMV     0x0008
>> +/** Device is a port representor */
>> +#define RTE_ETH_DEV_REPRESENTOR_PORT 0x0010
> As far as I can see this is not a generic device option, but just a way to mark
> a specific virtual PMD. There must be a better way to do this, I think this flag
> is not good idea.
Thanks Ferruh. We will also remove this flag option. Without this flag 
option, we can still identify the PMD as a port representor type or not 
by looking at the device name.

>>   
>>   /**
>>    * @internal
>>
>