[dpdk-dev] [PATCH 0/3] export hotplug capability

[dpdk-dev] [PATCH 0/3] export hotplug capability

[David Marchand]

This little patchset makes it possible for an application to know if a port
supports hotplug.


-- 
David Marchand

Maxime Leroy (3):
  ethdev: rework value return by rte_eth_dev_is_detachable
  ethdev: export rte_eth_dev_is_detachable function
  ethdev: prevent segfaults in rte_eth_dev_is_detachable

 lib/librte_ether/rte_ethdev.c          | 15 +++++++++------
 lib/librte_ether/rte_ethdev.h          | 12 ++++++++++++
 lib/librte_ether/rte_ether_version.map |  7 +++++++
 3 files changed, 28 insertions(+), 6 deletions(-)

-- 
1.9.1

[dpdk-dev] [PATCH 1/3] ethdev: rework value return by rte_eth_dev_is_detachable

[David Marchand]

From: Maxime Leroy <maxime.leroy@6wind.com>

The rte_eth_dev_is_detachable return 0 when the device is detachable.
It not, it returns a negative value or 1.

This patch modifies this function to return 1 when the device is detachable
and 0 when is not.

Signed-off-by: Maxime Leroy <maxime.leroy@6wind.com>
Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_ether/rte_ethdev.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index f593f6e..396667c 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -507,7 +507,7 @@ rte_eth_dev_is_detachable(uint8_t port_id)
 
 	if (!rte_eth_dev_is_valid_port(port_id)) {
 		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
-		return -EINVAL;
+		return 0;
 	}
 
 	if (rte_eth_devices[port_id].dev_type == RTE_ETH_DEV_PCI) {
@@ -518,12 +518,12 @@ rte_eth_dev_is_detachable(uint8_t port_id)
 			break;
 		case RTE_KDRV_VFIO:
 		default:
-			return -ENOTSUP;
+			return 0;
 		}
 	}
 
 	drv_flags = rte_eth_devices[port_id].driver->pci_drv.drv_flags;
-	return !(drv_flags & RTE_PCI_DRV_DETACHABLE);
+	return drv_flags & RTE_PCI_DRV_DETACHABLE;
 }
 
 /* attach the new physical device, then store port_id of the device */
@@ -571,7 +571,7 @@ rte_eth_dev_detach_pdev(uint8_t port_id, struct rte_pci_addr *addr)
 		goto err;
 
 	/* check whether the driver supports detach feature, or not */
-	if (rte_eth_dev_is_detachable(port_id))
+	if (!rte_eth_dev_is_detachable(port_id))
 		goto err;
 
 	/* get pci address by port id */
@@ -648,7 +648,7 @@ rte_eth_dev_detach_vdev(uint8_t port_id, char *vdevname)
 		goto err;
 
 	/* check whether the driver supports detach feature, or not */
-	if (rte_eth_dev_is_detachable(port_id))
+	if (!rte_eth_dev_is_detachable(port_id))
 		goto err;
 
 	/* get device name by port id */
-- 
1.9.1

[dpdk-dev] [PATCH 2/3] ethdev: export rte_eth_dev_is_detachable function

[David Marchand]

From: Maxime Leroy <maxime.leroy@6wind.com>

It can be useful for application to know if a port can be detached or not.

Signed-off-by: Maxime Leroy <maxime.leroy@6wind.com>
Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_ether/rte_ethdev.c          |  2 +-
 lib/librte_ether/rte_ethdev.h          | 12 ++++++++++++
 lib/librte_ether/rte_ether_version.map |  7 +++++++
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 396667c..7fa5717 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -500,7 +500,7 @@ rte_eth_dev_get_name_by_port(uint8_t port_id, char *name)
 	return 0;
 }
 
-static int
+int
 rte_eth_dev_is_detachable(uint8_t port_id)
 {
 	uint32_t drv_flags;
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 8a8c82b..160edd4 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1558,6 +1558,18 @@ int rte_eth_dev_attach(const char *devargs, uint8_t *port_id);
  */
 int rte_eth_dev_detach(uint8_t port_id, char *devname);
 
+/**
+ * Check if an Ethernet device specified by port identifer is detachable.
+ *
+ * @param port_id
+ *   The port identifier of the device to check if is detachable
+ *
+ * @return
+ *  1 if device is detachable, else 0
+ */
+int
+rte_eth_dev_is_detachable(uint8_t port_id);
+
 struct eth_driver;
 /**
  * @internal
diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map
index 8345a6c..7f872c2 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -127,3 +127,10 @@ DPDK_2.1 {
 	rte_eth_timesync_read_tx_timestamp;
 
 } DPDK_2.0;
+
+DPDK_2.2 {
+	global:
+
+	rte_eth_dev_is_detachable;
+
+} DPDK_2.1;
-- 
1.9.1

[dpdk-dev] [PATCH 3/3] ethdev: prevent segfaults in rte_eth_dev_is_detachable

[David Marchand]

From: Maxime Leroy <maxime.leroy@6wind.com>

Some drivers like virtual ones don't specify any driver pointer in the
structure rte_eth_dev.

To prevent segfault, we should check if this pointer is NULL before
dereferencing it.

Signed-off-by: Maxime Leroy <maxime.leroy@6wind.com>
Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_ether/rte_ethdev.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 7fa5717..750e4b5 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -522,6 +522,9 @@ rte_eth_dev_is_detachable(uint8_t port_id)
 		}
 	}
 
+	if (rte_eth_devices[port_id].driver == NULL)
+		return 0;
+
 	drv_flags = rte_eth_devices[port_id].driver->pci_drv.drv_flags;
 	return drv_flags & RTE_PCI_DRV_DETACHABLE;
 }
-- 
1.9.1

Re: [dpdk-dev] [PATCH 3/3] ethdev: prevent segfaults in rte_eth_dev_is_detachable

[Thomas Monjalon]

2015-10-29 09:55, David Marchand:
> From: Maxime Leroy <maxime.leroy@6wind.com>
> 
> Some drivers like virtual ones don't specify any driver pointer in the
> structure rte_eth_dev.
> 
> To prevent segfault, we should check if this pointer is NULL before
> dereferencing it.
> 
> Signed-off-by: Maxime Leroy <maxime.leroy@6wind.com>
> Signed-off-by: David Marchand <david.marchand@6wind.com>

It is not clear what the problem is.
And the driver fields should probably be removed or changed as it too tigthly
related to the PCI driver.

Re: [dpdk-dev] [PATCH 3/3] ethdev: prevent segfaults in rte_eth_dev_is_detachable

[David Marchand]

On Tue, Nov 3, 2015 at 12:10 PM, Thomas Monjalon <thomas.monjalon@6wind.com>
wrote:

> 2015-10-29 09:55, David Marchand:
> > From: Maxime Leroy <maxime.leroy@6wind.com>
> >
> > Some drivers like virtual ones don't specify any driver pointer in the
> > structure rte_eth_dev.
> >
> > To prevent segfault, we should check if this pointer is NULL before
> > dereferencing it.
> >
> > Signed-off-by: Maxime Leroy <maxime.leroy@6wind.com>
> > Signed-off-by: David Marchand <david.marchand@6wind.com>
>
> It is not clear what the problem is.
> And the driver fields should probably be removed or changed as it too
> tigthly
> related to the PCI driver.
>

It does look odd to me as well, quickly went through head, and did not see
a case where this could happen.
Maxime is not available at the moment, so I think I will just drop this
patch.

How about this patchset rebased on head with just the first two patches ?


-- 
David Marchand

Re: [dpdk-dev] [PATCH 3/3] ethdev: prevent segfaults in rte_eth_dev_is_detachable

[Thomas Monjalon]

2015-11-03 12:19, David Marchand:
> On Tue, Nov 3, 2015 at 12:10 PM, Thomas Monjalon <thomas.monjalon@6wind.com>
> wrote:
> 
> > 2015-10-29 09:55, David Marchand:
> > > From: Maxime Leroy <maxime.leroy@6wind.com>
> > >
> > > Some drivers like virtual ones don't specify any driver pointer in the
> > > structure rte_eth_dev.
> > >
> > > To prevent segfault, we should check if this pointer is NULL before
> > > dereferencing it.
> > >
> > > Signed-off-by: Maxime Leroy <maxime.leroy@6wind.com>
> > > Signed-off-by: David Marchand <david.marchand@6wind.com>
> >
> > It is not clear what the problem is.
> > And the driver fields should probably be removed or changed as it too
> > tigthly
> > related to the PCI driver.
> >
> 
> It does look odd to me as well, quickly went through head, and did not see
> a case where this could happen.
> Maxime is not available at the moment, so I think I will just drop this
> patch.
> 
> How about this patchset rebased on head with just the first two patches ?

Perfect, thanks

[dpdk-dev] [PATCH v2 0/2] export hotplug capability

[David Marchand]

This little patchset makes it possible for an application to know if a port
supports hotplug.

Changes since v1:
- rebased on HEAD
- dropped patch 3


-- 
David Marchand

Maxime Leroy (2):
  ethdev: rework value return by rte_eth_dev_is_detachable
  ethdev: export rte_eth_dev_is_detachable function

 lib/librte_ether/rte_ethdev.c          | 12 ++++++------
 lib/librte_ether/rte_ethdev.h          | 12 ++++++++++++
 lib/librte_ether/rte_ether_version.map |  1 +
 3 files changed, 19 insertions(+), 6 deletions(-)

-- 
1.9.1

[dpdk-dev] [PATCH v2 1/2] ethdev: rework value return by rte_eth_dev_is_detachable

[David Marchand]

From: Maxime Leroy <maxime.leroy@6wind.com>

The rte_eth_dev_is_detachable return 0 when the device is detachable.
It not, this one return a negative value or 1.
Such behavior is not logical.

This patch modifies this function to return 1 when the device is detachable
and 0 when is not.

Signed-off-by: Maxime Leroy <maxime.leroy@6wind.com>
Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_ether/rte_ethdev.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 073ffe9..ec1b632 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -510,7 +510,7 @@ rte_eth_dev_is_detachable(uint8_t port_id)
 
 	if (!rte_eth_dev_is_valid_port(port_id)) {
 		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
-		return -EINVAL;
+		return 0;
 	}
 
 	if (rte_eth_devices[port_id].dev_type == RTE_ETH_DEV_PCI) {
@@ -521,12 +521,12 @@ rte_eth_dev_is_detachable(uint8_t port_id)
 			break;
 		case RTE_KDRV_VFIO:
 		default:
-			return -ENOTSUP;
+			return 0;
 		}
 	}
 
 	drv_flags = rte_eth_devices[port_id].driver->pci_drv.drv_flags;
-	return !(drv_flags & RTE_PCI_DRV_DETACHABLE);
+	return drv_flags & RTE_PCI_DRV_DETACHABLE;
 }
 
 /* attach the new physical device, then store port_id of the device */
@@ -574,7 +574,7 @@ rte_eth_dev_detach_pdev(uint8_t port_id, struct rte_pci_addr *addr)
 		goto err;
 
 	/* check whether the driver supports detach feature, or not */
-	if (rte_eth_dev_is_detachable(port_id))
+	if (!rte_eth_dev_is_detachable(port_id))
 		goto err;
 
 	/* get pci address by port id */
@@ -651,7 +651,7 @@ rte_eth_dev_detach_vdev(uint8_t port_id, char *vdevname)
 		goto err;
 
 	/* check whether the driver supports detach feature, or not */
-	if (rte_eth_dev_is_detachable(port_id))
+	if (!rte_eth_dev_is_detachable(port_id))
 		goto err;
 
 	/* get device name by port id */
-- 
1.9.1

[dpdk-dev] [PATCH v2 2/2] ethdev: export rte_eth_dev_is_detachable function

[David Marchand]

From: Maxime Leroy <maxime.leroy@6wind.com>

It can be useful for application to know if a port can be detached or not.

Signed-off-by: Maxime Leroy <maxime.leroy@6wind.com>
Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_ether/rte_ethdev.c          |  2 +-
 lib/librte_ether/rte_ethdev.h          | 12 ++++++++++++
 lib/librte_ether/rte_ether_version.map |  1 +
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index ec1b632..373d29b 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -503,7 +503,7 @@ rte_eth_dev_get_name_by_port(uint8_t port_id, char *name)
 	return 0;
 }
 
-static int
+int
 rte_eth_dev_is_detachable(uint8_t port_id)
 {
 	uint32_t drv_flags;
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 7cf4af8..350733b 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1640,6 +1640,18 @@ int rte_eth_dev_attach(const char *devargs, uint8_t *port_id);
  */
 int rte_eth_dev_detach(uint8_t port_id, char *devname);
 
+/**
+ * Check if an Ethernet device specified by port identifer is detachable.
+ *
+ * @param port_id
+ *   The port identifier of the device to check if is detachable
+ *
+ * @return
+ *  1 if device is detachable, else 0
+ */
+int
+rte_eth_dev_is_detachable(uint8_t port_id);
+
 struct eth_driver;
 /**
  * @internal
diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map
index 7b04e95..40c2a02 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -132,6 +132,7 @@ DPDK_2.2 {
 	global:
 
 	rte_eth_dev_get_dcb_info;
+	rte_eth_dev_is_detachable;
 	rte_eth_rx_queue_info_get;
 	rte_eth_tx_queue_info_get;
 
-- 
1.9.1

Re: [dpdk-dev] [PATCH v2 2/2] ethdev: export rte_eth_dev_is_detachable function

[Thomas Monjalon]

2015-11-03 12:35, David Marchand:
> From: Maxime Leroy <maxime.leroy@6wind.com>
> 
> It can be useful for application to know if a port can be detached or not.
> 
> Signed-off-by: Maxime Leroy <maxime.leroy@6wind.com>
> Signed-off-by: David Marchand <david.marchand@6wind.com>
> ---
>  lib/librte_ether/rte_ethdev.c          |  2 +-
>  lib/librte_ether/rte_ethdev.h          | 12 ++++++++++++
>  lib/librte_ether/rte_ether_version.map |  1 +
>  3 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index ec1b632..373d29b 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -503,7 +503,7 @@ rte_eth_dev_get_name_by_port(uint8_t port_id, char *name)
>  	return 0;
>  }
>  
> -static int
> +int
>  rte_eth_dev_is_detachable(uint8_t port_id)
>  {
>  	uint32_t drv_flags;
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index 7cf4af8..350733b 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -1640,6 +1640,18 @@ int rte_eth_dev_attach(const char *devargs, uint8_t *port_id);
>   */
>  int rte_eth_dev_detach(uint8_t port_id, char *devname);
>  
> +/**
> + * Check if an Ethernet device specified by port identifer is detachable.
> + *
> + * @param port_id
> + *   The port identifier of the device to check if is detachable
> + *
> + * @return
> + *  1 if device is detachable, else 0
> + */
> +int
> +rte_eth_dev_is_detachable(uint8_t port_id);

After more thoughts, we do not need such function to query a capability.
The Bernard's patch add a bit-field to expose such capabilities.
So this patchset is rejected, sorry.

Re: [dpdk-dev] [PATCH v2 2/2] ethdev: export rte_eth_dev_is_detachable function

[David Marchand]

On Tue, Nov 3, 2015 at 2:50 PM, Thomas Monjalon <thomas.monjalon@6wind.com>
wrote:

> After more thoughts, we do not need such function to query a capability.
> The Bernard's patch add a bit-field to expose such capabilities.
> So this patchset is rejected, sorry.
>

As discussed offline, as long as those capabilities do the job, I am fine
with it.
Thanks.

-- 
David Marchand