diff mbox

[dpdk-dev,v8,05/14] ethdev: Add rte_eth_dev_free to free specified device

Message ID 1424060073-23484-6-git-send-email-mukawa@igel.co.jp (mailing list archive)
State Superseded, archived
Headers

Commit Message

Tetsuya Mukawa Feb. 16, 2015, 4:14 a.m. UTC 
This patch adds rte_eth_dev_free(). The function is used for changing an
attached status of the device that has specified name.

v6:
- Use rte_eth_dev structure as the paramter of rte_eth_dev_free().
v4:
- Add parameter checking.

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

Comments

Iremonger, Bernard Feb. 16, 2015, 4:20 p.m. UTC | #1 
> -----Original Message-----
> From: Tetsuya Mukawa [mailto:mukawa@igel.co.jp]
> Sent: Monday, February 16, 2015 4:14 AM
> To: dev@dpdk.org
> Cc: Qiu, Michael; Iremonger, Bernard; Tetsuya Mukawa
> Subject: [PATCH v8 05/14] ethdev: Add rte_eth_dev_free to free specified device
> 
> This patch adds rte_eth_dev_free(). The function is used for changing an attached status of the device
> that has specified name.
> 
> v6:
> - Use rte_eth_dev structure as the paramter of rte_eth_dev_free().
> v4:
> - Add parameter checking.
> 
> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>

Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>
Thomas Monjalon Feb. 17, 2015, 12:46 a.m. UTC | #2 
2015-02-16 13:14, Tetsuya Mukawa:
> This patch adds rte_eth_dev_free(). The function is used for changing an
> attached status of the device that has specified name.
> 
> v6:
> - Use rte_eth_dev structure as the paramter of rte_eth_dev_free().
> v4:
> - Add parameter checking.
> 
> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> ---
>  lib/librte_ether/rte_ethdev.c | 11 +++++++++++
>  lib/librte_ether/rte_ethdev.h | 14 ++++++++++++++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index a79fa5b..2463d18 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -260,6 +260,17 @@ rte_eth_dev_allocate(const char *name)
>  	return eth_dev;
>  }
>  
> +int
> +rte_eth_dev_free(struct rte_eth_dev *eth_dev)
> +{
> +	if (eth_dev == NULL)
> +		return -EINVAL;
> +
> +	eth_dev->attached = 0;
> +	nb_ports--;
> +	return 0;
> +}

This function is strange. I would imagine it calling the free (uninit) function
of the driver.

[...]
> +/**
> + * Function for internal use by dummy drivers primarily, e.g. ring-based
> + * driver.
> + * Free the specified ethdev.
> + *
> + * @param eth_dev
> + * The *eth_dev* pointer is the address of the *rte_eth_dev* structure
> + *   associated with the matching device and which have been [automatically]
> + *   allocated in the *rte_eth_devices* array.
> + * @return
> + *   - 0 on success, negative on error
> + */
> +int rte_eth_dev_free(struct rte_eth_dev *eth_dev);
> +
>  struct eth_driver;
>  /**
>   * @internal
>
Tetsuya Mukawa Feb. 17, 2015, 6:15 a.m. UTC | #3 
On 2015/02/17 9:46, Thomas Monjalon wrote:
> 2015-02-16 13:14, Tetsuya Mukawa:
>> This patch adds rte_eth_dev_free(). The function is used for changing an
>> attached status of the device that has specified name.
>>
>> v6:
>> - Use rte_eth_dev structure as the paramter of rte_eth_dev_free().
>> v4:
>> - Add parameter checking.
>>
>> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
>> ---
>>  lib/librte_ether/rte_ethdev.c | 11 +++++++++++
>>  lib/librte_ether/rte_ethdev.h | 14 ++++++++++++++
>>  2 files changed, 25 insertions(+)
>>
>> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
>> index a79fa5b..2463d18 100644
>> --- a/lib/librte_ether/rte_ethdev.c
>> +++ b/lib/librte_ether/rte_ethdev.c
>> @@ -260,6 +260,17 @@ rte_eth_dev_allocate(const char *name)
>>  	return eth_dev;
>>  }
>>  
>> +int
>> +rte_eth_dev_free(struct rte_eth_dev *eth_dev)
>> +{
>> +	if (eth_dev == NULL)
>> +		return -EINVAL;
>> +
>> +	eth_dev->attached = 0;
>> +	nb_ports--;
>> +	return 0;
>> +}
> This function is strange. I would imagine it calling the free (uninit) function
> of the driver.

How about changing the name like below?
rte_eth_dev_release_port()

> [...]
>> +/**
>> + * Function for internal use by dummy drivers primarily, e.g. ring-based
>> + * driver.
>> + * Free the specified ethdev.
>> + *
>> + * @param eth_dev
>> + * The *eth_dev* pointer is the address of the *rte_eth_dev* structure
>> + *   associated with the matching device and which have been [automatically]
>> + *   allocated in the *rte_eth_devices* array.
>> + * @return
>> + *   - 0 on success, negative on error
>> + */
>> +int rte_eth_dev_free(struct rte_eth_dev *eth_dev);
>> +
>>  struct eth_driver;
>>  /**
>>   * @internal
>>
>
diff mbox

Patch

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index a79fa5b..2463d18 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -260,6 +260,17 @@  rte_eth_dev_allocate(const char *name)
 	return eth_dev;
 }
 
+int
+rte_eth_dev_free(struct rte_eth_dev *eth_dev)
+{
+	if (eth_dev == NULL)
+		return -EINVAL;
+
+	eth_dev->attached = 0;
+	nb_ports--;
+	return 0;
+}
+
 static int
 rte_eth_dev_init(struct rte_pci_driver *pci_drv,
 		 struct rte_pci_device *pci_dev)
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index ca101f5..fbe7ac1 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1627,6 +1627,20 @@  extern uint8_t rte_eth_dev_count(void);
  */
 struct rte_eth_dev *rte_eth_dev_allocate(const char *name);
 
+/**
+ * Function for internal use by dummy drivers primarily, e.g. ring-based
+ * driver.
+ * Free the specified ethdev.
+ *
+ * @param eth_dev
+ * The *eth_dev* pointer is the address of the *rte_eth_dev* structure
+ *   associated with the matching device and which have been [automatically]
+ *   allocated in the *rte_eth_devices* array.
+ * @return
+ *   - 0 on success, negative on error
+ */
+int rte_eth_dev_free(struct rte_eth_dev *eth_dev);
+
 struct eth_driver;
 /**
  * @internal