[dpdk-dev] [PATCH v6 4/8] ethdev: Add port representor device flag
Doherty, Declan
declan.doherty at intel.com
Thu Mar 29 16:53:08 CEST 2018
On 29/03/2018 7:13 AM, Shahaf Shuler wrote:
> Wednesday, March 28, 2018 4:54 PM, Declan Doherty:
>> Subject: [dpdk-dev][PATCH v6 4/8] ethdev: Add port representor device flag
>>
>> Add new device flag to specify that ethdev port is a port representor.
>> Extend rte_eth_dev_info structure to expose device flags to user which
>> enable applications to discover if a port is a representor port.
>>
>> Signed-off-by: Declan Doherty <declan.doherty at intel.com>
>> ---
>> lib/librte_ether/rte_ethdev.c | 1 +
>> lib/librte_ether/rte_ethdev.h | 9 ++++++---
>> lib/librte_ether/rte_ethdev_representor.h | 3 +++
>> 3 files changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
>> index c719f84a3..163246433 100644
>> --- a/lib/librte_ether/rte_ethdev.c
>> +++ b/lib/librte_ether/rte_ethdev.c
>> @@ -2399,6 +2399,7 @@ rte_eth_dev_info_get(uint16_t port_id, struct
>> rte_eth_dev_info *dev_info)
>> dev_info->nb_rx_queues = dev->data->nb_rx_queues;
>> dev_info->nb_tx_queues = dev->data->nb_tx_queues;
>> dev_info->switch_id = dev->data->switch_id;
>> + dev_info->dev_flags = dev->data->dev_flags;
>> }
>>
>> int
>> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
>> index dced4fc41..226acc8b1 100644
>> --- a/lib/librte_ether/rte_ethdev.h
>> +++ b/lib/librte_ether/rte_ethdev.h
>> @@ -996,6 +996,7 @@ struct rte_eth_dev_info {
>> const char *driver_name; /**< Device Driver name. */
>> unsigned int if_index; /**< Index to bound host interface, or 0 if
>> none.
>> Use if_indextoname() to translate into an interface name. */
>> + uint32_t dev_flags; /**< Device flags */
>> uint32_t min_rx_bufsize; /**< Minimum size of RX buffer. */
>> uint32_t max_rx_pktlen; /**< Maximum configurable length of RX
>> pkt. */
>> uint16_t max_rx_queues; /**< Maximum number of RX queues. */
>> @@ -1229,11 +1230,13 @@ struct rte_eth_dev_owner { };
>>
>> /** Device supports link state interrupt */
>> -#define RTE_ETH_DEV_INTR_LSC 0x0002
>> +#define RTE_ETH_DEV_INTR_LSC 0x0002
>> /** Device is a bonded slave */
>> -#define RTE_ETH_DEV_BONDED_SLAVE 0x0004
>> +#define RTE_ETH_DEV_BONDED_SLAVE 0x0004
>> /** Device supports device removal interrupt */
>> -#define RTE_ETH_DEV_INTR_RMV 0x0008
>> +#define RTE_ETH_DEV_INTR_RMV 0x0008
>> +/** Device is port representor */
>> +#define RTE_ETH_DEV_REPRESENTOR 0x0010
>
> Maybe it is a good time to make some order here.
> I understand the decision to use flags instead of bit-field. It is better.
>
> However there is a mix here of device capabilities like : RTE_ETH_DEV_INTR_LSC and RTE_ETH_DEV_INTR_RMV
> And device attributes like : RTE_ETH_DEV_BONDED_SLAVE and RTE_ETH_DEV_REPRESENTOR.
> I don't think they belong together under the genetic name of dev_flags.
>
> Moreover, I am not sure the fact device is bonded slave should be exposed to the application. It should be internal to ethdev and its port iterators.
That's a good point on the bonded slave flag, I'll look at fixing that
for the next release. I don't think changing it should effect ABI but
I'll need to have a closer look.
Do you think that we should have a separate device attributes field,
which the representor flag is contained in.
>
> Finally I think representor port may need more info now (and in the future), for example the associated vf id.
> For that, I think it is better it to be exposed as a dedicated struct on device info.
I think a switch port id should suffice for that, for SR-IOV devices it
would map to the vf_id.
>
>>
>> /**
>> * @warning
>> diff --git a/lib/librte_ether/rte_ethdev_representor.h
>> b/lib/librte_ether/rte_ethdev_representor.h
>> index cbc1f2855..f3726d0ba 100644
>> --- a/lib/librte_ether/rte_ethdev_representor.h
>> +++ b/lib/librte_ether/rte_ethdev_representor.h
>> @@ -22,6 +22,9 @@ eth_dev_representor_port_init(struct rte_eth_dev
>> *ethdev, void *init_params)
>> /** representor inherits the switch id of it's base device */
>> ethdev->data->switch_id = base_ethdev->data->switch_id;
>>
>> + /** Set device flags to specify that device is a representor port */
>> + ethdev->data->dev_flags |= RTE_ETH_DEV_REPRESENTOR;
>
> Should be set in the PMD, not in ethdev layer
As in the previous patch this is just a generic port bus init function
which meets the simplest use case of representor port with a single
switch domain, a PMD doesn't need to use it but having it here saves
duplicating the same code across multiple PMD which are only supporting
the basic mode.
>
>> +
>> return 0;
>> }
>>
>> --
>> 2.14.3
>
More information about the dev
mailing list