[dpdk-dev,v3,07/28] ethdev: Add functions to know which port is attached or detached

The patch adds rte_eth_dev_save() and rte_eth_dev_get_changed_port().
rte_eth_dev_save() is used for saving current rte_eth_dev structures.
rte_eth_dev_get_changed_port() receives the rte_eth_dev structures, then
compare these with current values to know which port is actually
attached or detached.

Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
---
 lib/librte_ether/rte_ethdev.c | 21 +++++++++++++++++++++
 lib/librte_ether/rte_ethdev.h | 21 +++++++++++++++++++++
 2 files changed, 42 insertions(+)
  

On 2014/12/9 14:32, Tetsuya Mukawa wrote:
> The patch adds rte_eth_dev_save() and rte_eth_dev_get_changed_port().
> rte_eth_dev_save() is used for saving current rte_eth_dev structures.
> rte_eth_dev_get_changed_port() receives the rte_eth_dev structures, then
> compare these with current values to know which port is actually
> attached or detached.
>
> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> ---
>  lib/librte_ether/rte_ethdev.c | 21 +++++++++++++++++++++
>  lib/librte_ether/rte_ethdev.h | 21 +++++++++++++++++++++
>  2 files changed, 42 insertions(+)
>
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index 51697e1..6a3700e 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -416,6 +416,27 @@ rte_eth_dev_count(void)
>  	return (nb_ports);
>  }
>  
> +void
> +rte_eth_dev_save(struct rte_eth_dev *devs)
> +{
> +	if (devs == NULL)
> +		return;
> +
> +	/* save current rte_eth_devices */
> +	memcpy(devs, rte_eth_devices,
> +			sizeof(struct rte_eth_dev) * RTE_MAX_ETHPORTS);
> +}
> +
> +int
> +rte_eth_dev_get_changed_port(struct rte_eth_dev *devs, uint8_t *port_id)
> +{
> +	/* check which port was attached or detached */
> +	for (*port_id = 0; *port_id < RTE_MAX_ETHPORTS; (*port_id)++, devs++)
> +		if (rte_eth_devices[*port_id].attached ^ devs->attached)
> +			return 0;

Can we have more than one port changed?
If so, your logic should do little modify.

Thanks,
Michael
> +	return 1;
> +}
> +
>  static int
>  rte_eth_dev_rx_queue_config(struct rte_eth_dev *dev, uint16_t nb_queues)
>  {
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index b329e11..03c8850 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -1642,6 +1642,27 @@ extern struct rte_eth_dev rte_eth_devices[];
>  extern uint8_t rte_eth_dev_count(void);
>  
>  /**
> + * Function for internal use by port hotplug functions.
> + * Copies current ethdev structures to the specified pointer.
> + *
> + * @param	devs	The pointer to the ethdev structures
> + */
> +extern void rte_eth_dev_save(struct rte_eth_dev *devs);
> +
> +/**
> + * Function for internal use by port hotplug functions.
> + * Compare the specified ethdev structures with currents. Then
> + * if there is a port which status is changed, fill the specified pointer
> + * with the port id of that port.
> + * @param	devs	The pointer to the ethdev structures
> + * @param	port_id	The pointer to the port id
> + * @return
> + *   - 0 on success, negative on error
> + */
> +extern int rte_eth_dev_get_changed_port(
> +		struct rte_eth_dev *devs, uint8_t *port_id);
> +
> +/**
>   * Function for internal use by dummy drivers primarily, e.g. ring-based
>   * driver.
>   * Allocates a new ethdev slot for an ethernet device and returns the pointer
  

Hi Michael,

(2014/12/09 23:39), Qiu, Michael wrote:
> On 2014/12/9 14:32, Tetsuya Mukawa wrote:
>> The patch adds rte_eth_dev_save() and rte_eth_dev_get_changed_port().
>> rte_eth_dev_save() is used for saving current rte_eth_dev structures.
>> rte_eth_dev_get_changed_port() receives the rte_eth_dev structures, then
>> compare these with current values to know which port is actually
>> attached or detached.
>>
>> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
>> ---
>>  lib/librte_ether/rte_ethdev.c | 21 +++++++++++++++++++++
>>  lib/librte_ether/rte_ethdev.h | 21 +++++++++++++++++++++
>>  2 files changed, 42 insertions(+)
>>
>> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
>> index 51697e1..6a3700e 100644
>> --- a/lib/librte_ether/rte_ethdev.c
>> +++ b/lib/librte_ether/rte_ethdev.c
>> @@ -416,6 +416,27 @@ rte_eth_dev_count(void)
>>  	return (nb_ports);
>>  }
>>  
>> +void
>> +rte_eth_dev_save(struct rte_eth_dev *devs)
>> +{
>> +	if (devs == NULL)
>> +		return;
>> +
>> +	/* save current rte_eth_devices */
>> +	memcpy(devs, rte_eth_devices,
>> +			sizeof(struct rte_eth_dev) * RTE_MAX_ETHPORTS);
>> +}
>> +
>> +int
>> +rte_eth_dev_get_changed_port(struct rte_eth_dev *devs, uint8_t *port_id)
>> +{
>> +	/* check which port was attached or detached */
>> +	for (*port_id = 0; *port_id < RTE_MAX_ETHPORTS; (*port_id)++, devs++)
>> +		if (rte_eth_devices[*port_id].attached ^ devs->attached)
>> +			return 0;
> Can we have more than one port changed?
> If so, your logic should do little modify.

The port hotplug APIs cannot attach or detach multiple port at the same
time.
And the APIs are not thread safe. DPDK application should have lock
properly if multiple threads call the APIs.
Because of this, we don't need to take care of such a case.

> Thanks,
> Michael
>> +	return 1;
>> +}
>> +
>>  static int
>>  rte_eth_dev_rx_queue_config(struct rte_eth_dev *dev, uint16_t nb_queues)
>>  {
>> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
>> index b329e11..03c8850 100644
>> --- a/lib/librte_ether/rte_ethdev.h
>> +++ b/lib/librte_ether/rte_ethdev.h
>> @@ -1642,6 +1642,27 @@ extern struct rte_eth_dev rte_eth_devices[];
>>  extern uint8_t rte_eth_dev_count(void);
>>  
>>  /**
>> + * Function for internal use by port hotplug functions.
>> + * Copies current ethdev structures to the specified pointer.
>> + *
>> + * @param	devs	The pointer to the ethdev structures
>> + */
>> +extern void rte_eth_dev_save(struct rte_eth_dev *devs);
>> +
>> +/**
>> + * Function for internal use by port hotplug functions.
>> + * Compare the specified ethdev structures with currents. Then
>> + * if there is a port which status is changed, fill the specified pointer
>> + * with the port id of that port.
>> + * @param	devs	The pointer to the ethdev structures
>> + * @param	port_id	The pointer to the port id
>> + * @return
>> + *   - 0 on success, negative on error
>> + */
>> +extern int rte_eth_dev_get_changed_port(
>> +		struct rte_eth_dev *devs, uint8_t *port_id);
>> +
>> +/**
>>   * Function for internal use by dummy drivers primarily, e.g. ring-based
>>   * driver.
>>   * Allocates a new ethdev slot for an ethernet device and returns the pointer
  

On 12/11/2014 11:12 AM, Tetsuya Mukawa wrote:
> Hi Michael,
>
> (2014/12/09 23:39), Qiu, Michael wrote:
>> On 2014/12/9 14:32, Tetsuya Mukawa wrote:
>>> The patch adds rte_eth_dev_save() and rte_eth_dev_get_changed_port().
>>> rte_eth_dev_save() is used for saving current rte_eth_dev structures.
>>> rte_eth_dev_get_changed_port() receives the rte_eth_dev structures, then
>>> compare these with current values to know which port is actually
>>> attached or detached.
>>>
>>> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
>>> ---
>>>  lib/librte_ether/rte_ethdev.c | 21 +++++++++++++++++++++
>>>  lib/librte_ether/rte_ethdev.h | 21 +++++++++++++++++++++
>>>  2 files changed, 42 insertions(+)
>>>
>>> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
>>> index 51697e1..6a3700e 100644
>>> --- a/lib/librte_ether/rte_ethdev.c
>>> +++ b/lib/librte_ether/rte_ethdev.c
>>> @@ -416,6 +416,27 @@ rte_eth_dev_count(void)
>>>  	return (nb_ports);
>>>  }
>>>  
>>> +void
>>> +rte_eth_dev_save(struct rte_eth_dev *devs)
>>> +{
>>> +	if (devs == NULL)
>>> +		return;
>>> +
>>> +	/* save current rte_eth_devices */
>>> +	memcpy(devs, rte_eth_devices,
>>> +			sizeof(struct rte_eth_dev) * RTE_MAX_ETHPORTS);
>>> +}
>>> +
>>> +int
>>> +rte_eth_dev_get_changed_port(struct rte_eth_dev *devs, uint8_t *port_id)
>>> +{
>>> +	/* check which port was attached or detached */
>>> +	for (*port_id = 0; *port_id < RTE_MAX_ETHPORTS; (*port_id)++, devs++)
>>> +		if (rte_eth_devices[*port_id].attached ^ devs->attached)
>>> +			return 0;
>> Can we have more than one port changed?
>> If so, your logic should do little modify.
> The port hotplug APIs cannot attach or detach multiple port at the same
> time.

What I mean is can we first detach one port, then another? If it legal
to do this, here will always return the port with the min port_id.

Thanks,
Michael
> And the APIs are not thread safe. DPDK application should have lock
> properly if multiple threads call the APIs.
> Because of this, we don't need to take care of such a case.
>
>> Thanks,
>> Michael
>>> +	return 1;
>>> +}
>>> +
>>>  static int
>>>  rte_eth_dev_rx_queue_config(struct rte_eth_dev *dev, uint16_t nb_queues)
>>>  {
>>> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
>>> index b329e11..03c8850 100644
>>> --- a/lib/librte_ether/rte_ethdev.h
>>> +++ b/lib/librte_ether/rte_ethdev.h
>>> @@ -1642,6 +1642,27 @@ extern struct rte_eth_dev rte_eth_devices[];
>>>  extern uint8_t rte_eth_dev_count(void);
>>>  
>>>  /**
>>> + * Function for internal use by port hotplug functions.
>>> + * Copies current ethdev structures to the specified pointer.
>>> + *
>>> + * @param	devs	The pointer to the ethdev structures
>>> + */
>>> +extern void rte_eth_dev_save(struct rte_eth_dev *devs);
>>> +
>>> +/**
>>> + * Function for internal use by port hotplug functions.
>>> + * Compare the specified ethdev structures with currents. Then
>>> + * if there is a port which status is changed, fill the specified pointer
>>> + * with the port id of that port.
>>> + * @param	devs	The pointer to the ethdev structures
>>> + * @param	port_id	The pointer to the port id
>>> + * @return
>>> + *   - 0 on success, negative on error
>>> + */
>>> +extern int rte_eth_dev_get_changed_port(
>>> +		struct rte_eth_dev *devs, uint8_t *port_id);
>>> +
>>> +/**
>>>   * Function for internal use by dummy drivers primarily, e.g. ring-based
>>>   * driver.
>>>   * Allocates a new ethdev slot for an ethernet device and returns the pointer
>
>
  

(2014/12/11 12:35), Qiu, Michael wrote:
> On 12/11/2014 11:12 AM, Tetsuya Mukawa wrote:
>> Hi Michael,
>>
>> (2014/12/09 23:39), Qiu, Michael wrote:
>>> On 2014/12/9 14:32, Tetsuya Mukawa wrote:
>>>> The patch adds rte_eth_dev_save() and rte_eth_dev_get_changed_port().
>>>> rte_eth_dev_save() is used for saving current rte_eth_dev structures.
>>>> rte_eth_dev_get_changed_port() receives the rte_eth_dev structures, then
>>>> compare these with current values to know which port is actually
>>>> attached or detached.
>>>>
>>>> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
>>>> ---
>>>>  lib/librte_ether/rte_ethdev.c | 21 +++++++++++++++++++++
>>>>  lib/librte_ether/rte_ethdev.h | 21 +++++++++++++++++++++
>>>>  2 files changed, 42 insertions(+)
>>>>
>>>> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
>>>> index 51697e1..6a3700e 100644
>>>> --- a/lib/librte_ether/rte_ethdev.c
>>>> +++ b/lib/librte_ether/rte_ethdev.c
>>>> @@ -416,6 +416,27 @@ rte_eth_dev_count(void)
>>>>  	return (nb_ports);
>>>>  }
>>>>  
>>>> +void
>>>> +rte_eth_dev_save(struct rte_eth_dev *devs)
>>>> +{
>>>> +	if (devs == NULL)
>>>> +		return;
>>>> +
>>>> +	/* save current rte_eth_devices */
>>>> +	memcpy(devs, rte_eth_devices,
>>>> +			sizeof(struct rte_eth_dev) * RTE_MAX_ETHPORTS);
>>>> +}
>>>> +
>>>> +int
>>>> +rte_eth_dev_get_changed_port(struct rte_eth_dev *devs, uint8_t *port_id)
>>>> +{
>>>> +	/* check which port was attached or detached */
>>>> +	for (*port_id = 0; *port_id < RTE_MAX_ETHPORTS; (*port_id)++, devs++)
>>>> +		if (rte_eth_devices[*port_id].attached ^ devs->attached)
>>>> +			return 0;
>>> Can we have more than one port changed?
>>> If so, your logic should do little modify.
>> The port hotplug APIs cannot attach or detach multiple port at the same
>> time.
> What I mean is can we first detach one port, then another? If it legal
> to do this, here will always return the port with the min port_id.

It's not legal.
Above 2 functions are implemented to be called by rte_eal_dev_attach().
When rte_eal_dev_attach() is called, rte_eth_dev_save() and
rte_eth_dev_get_changed_port() will be invoked continuously in the function.
But while executing the 2 functions, only one port status will be
changed, because rte_eal_dev_attach() and rte_eal_dev_detach() should be
called one by one.

Thanks,
Tetsuya

>
> Thanks,
> Michael
>> And the APIs are not thread safe. DPDK application should have lock
>> properly if multiple threads call the APIs.
>> Because of this, we don't need to take care of such a case.
>>
>>> Thanks,
>>> Michael
>>>> +	return 1;
>>>> +}
>>>> +
>>>>  static int
>>>>  rte_eth_dev_rx_queue_config(struct rte_eth_dev *dev, uint16_t nb_queues)
>>>>  {
>>>> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
>>>> index b329e11..03c8850 100644
>>>> --- a/lib/librte_ether/rte_ethdev.h
>>>> +++ b/lib/librte_ether/rte_ethdev.h
>>>> @@ -1642,6 +1642,27 @@ extern struct rte_eth_dev rte_eth_devices[];
>>>>  extern uint8_t rte_eth_dev_count(void);
>>>>  
>>>>  /**
>>>> + * Function for internal use by port hotplug functions.
>>>> + * Copies current ethdev structures to the specified pointer.
>>>> + *
>>>> + * @param	devs	The pointer to the ethdev structures
>>>> + */
>>>> +extern void rte_eth_dev_save(struct rte_eth_dev *devs);
>>>> +
>>>> +/**
>>>> + * Function for internal use by port hotplug functions.
>>>> + * Compare the specified ethdev structures with currents. Then
>>>> + * if there is a port which status is changed, fill the specified pointer
>>>> + * with the port id of that port.
>>>> + * @param	devs	The pointer to the ethdev structures
>>>> + * @param	port_id	The pointer to the port id
>>>> + * @return
>>>> + *   - 0 on success, negative on error
>>>> + */
>>>> +extern int rte_eth_dev_get_changed_port(
>>>> +		struct rte_eth_dev *devs, uint8_t *port_id);
>>>> +
>>>> +/**
>>>>   * Function for internal use by dummy drivers primarily, e.g. ring-based
>>>>   * driver.
>>>>   * Allocates a new ethdev slot for an ethernet device and returns the pointer
>>