Re: [PATCH v3 07/28] ethdev: Add functions to know which port is attached or detached

Hi Michael,

(2014/12/09 23:39), Qiu, Michael wrote:

On 2014/12/9 14:32, Tetsuya Mukawa wrote:

quoted

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-AlSX/UN32fvPDbFq/vQRIQ@public.gmane.org>
---
 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

quoted

+	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:

quoted

On 2014/12/9 14:32, Tetsuya Mukawa wrote:

quoted

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-AlSX/UN32fvPDbFq/vQRIQ@public.gmane.org>
---
 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.

quoted

Thanks,
Michael

quoted

+	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