DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v2 0/2] rte_dev_probe() API change
@ 2020-06-25  8:04 Maxime Coquelin
  2020-06-25  8:04 ` [dpdk-dev] [PATCH v2 1/2] doc: announce " Maxime Coquelin
  2020-06-25  8:04 ` [dpdk-dev] [PATCH v2 (v20.11) 2/2] eal: improve device probing API Maxime Coquelin
  0 siblings, 2 replies; 13+ messages in thread
From: Maxime Coquelin @ 2020-06-25  8:04 UTC (permalink / raw)
  To: dev, grive, david.marchand, wenzhuo.lu, beilei.xing, bernard.iremonger
  Cc: Maxime Coquelin

This series proposes a change in rte_dev_probe() API
for DPDK v20.11. First patch is the deprecation notice
to be applied in v20.11


Changes since v1:
-----------------
- Rebase on top of master
- Reword deprecation notice (Gaëtan)

Maxime Coquelin (2):
  doc: announce rte_dev_probe() API change
  eal: improve device probing API

 app/test-pmd/testpmd.c                 |  2 +-
 doc/guides/rel_notes/deprecation.rst   |  5 +++++
 drivers/net/failsafe/failsafe.c        |  5 +++--
 lib/librte_eal/common/eal_common_dev.c | 18 ++++++++++--------
 lib/librte_eal/include/rte_dev.h       |  4 ++--
 5 files changed, 21 insertions(+), 13 deletions(-)

-- 
2.26.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [dpdk-dev] [PATCH v2 1/2] doc: announce rte_dev_probe() API change
  2020-06-25  8:04 [dpdk-dev] [PATCH v2 0/2] rte_dev_probe() API change Maxime Coquelin
@ 2020-06-25  8:04 ` Maxime Coquelin
  2020-07-31  7:20   ` Maxime Coquelin
                     ` (2 more replies)
  2020-06-25  8:04 ` [dpdk-dev] [PATCH v2 (v20.11) 2/2] eal: improve device probing API Maxime Coquelin
  1 sibling, 3 replies; 13+ messages in thread
From: Maxime Coquelin @ 2020-06-25  8:04 UTC (permalink / raw)
  To: dev, grive, david.marchand, wenzhuo.lu, beilei.xing, bernard.iremonger
  Cc: Maxime Coquelin

In order to simplify the use of rte_dev_probe() and
rte_dev_remove() by applications, rte_dev_probe() will
return a reference on the rte_device stating DPDK v20.11.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 doc/guides/rel_notes/deprecation.rst | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index d1034f60fd..8661a673b5 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -130,3 +130,8 @@ Deprecation Notices
   Python 2 support will be completely removed in 20.11.
   In 20.08, explicit deprecation warnings will be displayed when running
   scripts with Python 2.
+
+* eal: Change ``rte_dev_probe`` API in DPDK v20.11 to return a pointer on the
+  probed rte_device or NULL instead of 0 or an error code. This change will
+  allow applications avoid iterating on devices after a probe to get access to
+  the new rte_device.
-- 
2.26.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [dpdk-dev] [PATCH v2 (v20.11) 2/2] eal: improve device probing API
  2020-06-25  8:04 [dpdk-dev] [PATCH v2 0/2] rte_dev_probe() API change Maxime Coquelin
  2020-06-25  8:04 ` [dpdk-dev] [PATCH v2 1/2] doc: announce " Maxime Coquelin
@ 2020-06-25  8:04 ` Maxime Coquelin
  2020-06-29  9:57   ` Gaëtan Rivet
  2020-08-05 23:33   ` Thomas Monjalon
  1 sibling, 2 replies; 13+ messages in thread
From: Maxime Coquelin @ 2020-06-25  8:04 UTC (permalink / raw)
  To: dev, grive, david.marchand, wenzhuo.lu, beilei.xing, bernard.iremonger
  Cc: Maxime Coquelin

This patch makes rte_dev_probe() to return the
rte_device pointer on success and NULL on error
instead of returning 0 on success and negative
value on error.

The goal is to avoid that the calling application
iterates the devices list afterwards to retrieve
the pointer. Retrieving the pointer is required
for calling rte_dev_remove() later.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 app/test-pmd/testpmd.c                 |  2 +-
 drivers/net/failsafe/failsafe.c        |  5 +++--
 lib/librte_eal/common/eal_common_dev.c | 18 ++++++++++--------
 lib/librte_eal/include/rte_dev.h       |  4 ++--
 4 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 4989d22ca8..f777f449a8 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2764,7 +2764,7 @@ attach_port(char *identifier)
 		return;
 	}
 
-	if (rte_dev_probe(identifier) < 0) {
+	if (rte_dev_probe(identifier) == NULL) {
 		TESTPMD_LOG(ERR, "Failed to attach port %s\n", identifier);
 		return;
 	}
diff --git a/drivers/net/failsafe/failsafe.c b/drivers/net/failsafe/failsafe.c
index 72362f35de..e32effdef2 100644
--- a/drivers/net/failsafe/failsafe.c
+++ b/drivers/net/failsafe/failsafe.c
@@ -341,6 +341,7 @@ rte_pmd_failsafe_probe(struct rte_vdev_device *vdev)
 	struct rte_eth_dev *eth_dev;
 	struct sub_device  *sdev;
 	struct rte_devargs devargs;
+	struct rte_device *dev;
 	uint8_t i;
 	int ret;
 
@@ -378,8 +379,8 @@ rte_pmd_failsafe_probe(struct rte_vdev_device *vdev)
 				continue;
 			}
 			if (!devargs_already_listed(&devargs)) {
-				ret = rte_dev_probe(devargs.name);
-				if (ret < 0) {
+				dev = rte_dev_probe(devargs.name);
+				if (dev == NULL) {
 					ERROR("Failed to probe devargs %s",
 					      devargs.name);
 					continue;
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index 9e4f09d83e..72baae2e48 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -120,7 +120,9 @@ rte_eal_hotplug_add(const char *busname, const char *devname,
 	if (ret != 0)
 		return ret;
 
-	ret = rte_dev_probe(devargs);
+	if (rte_dev_probe(devargs) == NULL)
+		ret = -1;
+
 	free(devargs);
 
 	return ret;
@@ -192,7 +194,7 @@ local_dev_probe(const char *devargs, struct rte_device **new_dev)
 	return ret;
 }
 
-int
+struct rte_device *
 rte_dev_probe(const char *devargs)
 {
 	struct eal_dev_mp_req req;
@@ -212,12 +214,12 @@ rte_dev_probe(const char *devargs)
 		if (ret != 0) {
 			RTE_LOG(ERR, EAL,
 				"Failed to send hotplug request to primary\n");
-			return -ENOMSG;
+			return NULL;
 		}
 		if (req.result != 0)
 			RTE_LOG(ERR, EAL,
 				"Failed to hotplug add device\n");
-		return req.result;
+		return NULL;
 	}
 
 	/* attach a shared device from primary start from here: */
@@ -236,7 +238,7 @@ rte_dev_probe(const char *devargs)
 		 * process.
 		 */
 		if (ret != -EEXIST)
-			return ret;
+			return dev;
 	}
 
 	/* primary send attach sync request to secondary. */
@@ -261,11 +263,11 @@ rte_dev_probe(const char *devargs)
 
 		/* for -EEXIST, we don't need to rollback. */
 		if (ret == -EEXIST)
-			return ret;
+			return dev;
 		goto rollback;
 	}
 
-	return 0;
+	return dev;
 
 rollback:
 	req.t = EAL_DEV_REQ_TYPE_ATTACH_ROLLBACK;
@@ -282,7 +284,7 @@ rte_dev_probe(const char *devargs)
 			"Failed to rollback device attach on primary."
 			"Devices in secondary may not sync with primary\n");
 
-	return ret;
+	return NULL;
 }
 
 int
diff --git a/lib/librte_eal/include/rte_dev.h b/lib/librte_eal/include/rte_dev.h
index c8d985fb5c..9cf7c7fd71 100644
--- a/lib/librte_eal/include/rte_dev.h
+++ b/lib/librte_eal/include/rte_dev.h
@@ -148,9 +148,9 @@ int rte_eal_hotplug_add(const char *busname, const char *devname,
  * @param devargs
  *   Device arguments including bus, class and driver properties.
  * @return
- *   0 on success, negative on error.
+ *   Generic device pointer on success, NULL on error.
  */
-int rte_dev_probe(const char *devargs);
+struct rte_device *rte_dev_probe(const char *devargs);
 
 /**
  * Hotplug remove a given device from a specific bus.
-- 
2.26.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH v2 (v20.11) 2/2] eal: improve device probing API
  2020-06-25  8:04 ` [dpdk-dev] [PATCH v2 (v20.11) 2/2] eal: improve device probing API Maxime Coquelin
@ 2020-06-29  9:57   ` Gaëtan Rivet
  2020-08-05 23:33   ` Thomas Monjalon
  1 sibling, 0 replies; 13+ messages in thread
From: Gaëtan Rivet @ 2020-06-29  9:57 UTC (permalink / raw)
  To: Maxime Coquelin
  Cc: dev, david.marchand, wenzhuo.lu, beilei.xing, bernard.iremonger

On 25/06/20 10:04 +0200, Maxime Coquelin wrote:
> This patch makes rte_dev_probe() to return the
> rte_device pointer on success and NULL on error
> instead of returning 0 on success and negative
> value on error.
> 
> The goal is to avoid that the calling application
> iterates the devices list afterwards to retrieve
> the pointer. Retrieving the pointer is required
> for calling rte_dev_remove() later.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  app/test-pmd/testpmd.c                 |  2 +-
>  drivers/net/failsafe/failsafe.c        |  5 +++--
>  lib/librte_eal/common/eal_common_dev.c | 18 ++++++++++--------
>  lib/librte_eal/include/rte_dev.h       |  4 ++--
>  4 files changed, 16 insertions(+), 13 deletions(-)
> 

[...]

> diff --git a/lib/librte_eal/include/rte_dev.h b/lib/librte_eal/include/rte_dev.h
> index c8d985fb5c..9cf7c7fd71 100644
> --- a/lib/librte_eal/include/rte_dev.h
> +++ b/lib/librte_eal/include/rte_dev.h
> @@ -148,9 +148,9 @@ int rte_eal_hotplug_add(const char *busname, const char *devname,
>   * @param devargs
>   *   Device arguments including bus, class and driver properties.
>   * @return
> - *   0 on success, negative on error.
> + *   Generic device pointer on success, NULL on error.

Hello Maxime,

Do you plan on a v3 with rte_errno support in the new API or do you
prefer to keep it this way after all?

-- 
Gaëtan

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/2] doc: announce rte_dev_probe() API change
  2020-06-25  8:04 ` [dpdk-dev] [PATCH v2 1/2] doc: announce " Maxime Coquelin
@ 2020-07-31  7:20   ` Maxime Coquelin
  2020-07-31  9:26   ` Bruce Richardson
  2020-08-04  8:12   ` David Marchand
  2 siblings, 0 replies; 13+ messages in thread
From: Maxime Coquelin @ 2020-07-31  7:20 UTC (permalink / raw)
  To: dev, grive, david.marchand, wenzhuo.lu, beilei.xing,
	bernard.iremonger, Thomas Monjalon

Hi Thomas, Davis,

On 6/25/20 10:04 AM, Maxime Coquelin wrote:
> In order to simplify the use of rte_dev_probe() and
> rte_dev_remove() by applications, rte_dev_probe() will
> return a reference on the rte_device stating DPDK v20.11.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  doc/guides/rel_notes/deprecation.rst | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index d1034f60fd..8661a673b5 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -130,3 +130,8 @@ Deprecation Notices
>    Python 2 support will be completely removed in 20.11.
>    In 20.08, explicit deprecation warnings will be displayed when running
>    scripts with Python 2.
> +
> +* eal: Change ``rte_dev_probe`` API in DPDK v20.11 to return a pointer on the
> +  probed rte_device or NULL instead of 0 or an error code. This change will
> +  allow applications avoid iterating on devices after a probe to get access to
> +  the new rte_device.
> 

Any chance this patch goes into -rc4?
Patch 2 is for v20.11, but we would need the deprecation notice to be in
v20.08.

Thanks,
Maxime


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/2] doc: announce rte_dev_probe() API change
  2020-06-25  8:04 ` [dpdk-dev] [PATCH v2 1/2] doc: announce " Maxime Coquelin
  2020-07-31  7:20   ` Maxime Coquelin
@ 2020-07-31  9:26   ` Bruce Richardson
  2020-07-31  9:44     ` Gaëtan Rivet
  2020-08-04  8:12   ` David Marchand
  2 siblings, 1 reply; 13+ messages in thread
From: Bruce Richardson @ 2020-07-31  9:26 UTC (permalink / raw)
  To: Maxime Coquelin
  Cc: dev, grive, david.marchand, wenzhuo.lu, beilei.xing, bernard.iremonger

On Thu, Jun 25, 2020 at 10:04:29AM +0200, Maxime Coquelin wrote:
> In order to simplify the use of rte_dev_probe() and
> rte_dev_remove() by applications, rte_dev_probe() will
> return a reference on the rte_device stating DPDK v20.11.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/2] doc: announce rte_dev_probe() API change
  2020-07-31  9:26   ` Bruce Richardson
@ 2020-07-31  9:44     ` Gaëtan Rivet
  2020-07-31  9:47       ` Maxime Coquelin
  0 siblings, 1 reply; 13+ messages in thread
From: Gaëtan Rivet @ 2020-07-31  9:44 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: Maxime Coquelin, dev, david.marchand, wenzhuo.lu, beilei.xing,
	bernard.iremonger

On 31/07/20 10:26 +0100, Bruce Richardson wrote:
> On Thu, Jun 25, 2020 at 10:04:29AM +0200, Maxime Coquelin wrote:
> > In order to simplify the use of rte_dev_probe() and
> > rte_dev_remove() by applications, rte_dev_probe() will
> > return a reference on the rte_device stating DPDK v20.11.
> > 
> > Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> > ---
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>

I don't remember if I acked the reworded version, in anycase
Acked-by: Gaetan Rivet <grive@u256.net>

-- 
Gaëtan

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/2] doc: announce rte_dev_probe() API change
  2020-07-31  9:44     ` Gaëtan Rivet
@ 2020-07-31  9:47       ` Maxime Coquelin
  0 siblings, 0 replies; 13+ messages in thread
From: Maxime Coquelin @ 2020-07-31  9:47 UTC (permalink / raw)
  To: Gaëtan Rivet, Bruce Richardson
  Cc: dev, david.marchand, wenzhuo.lu, beilei.xing, bernard.iremonger



On 7/31/20 11:44 AM, Gaëtan Rivet wrote:
> On 31/07/20 10:26 +0100, Bruce Richardson wrote:
>> On Thu, Jun 25, 2020 at 10:04:29AM +0200, Maxime Coquelin wrote:
>>> In order to simplify the use of rte_dev_probe() and
>>> rte_dev_remove() by applications, rte_dev_probe() will
>>> return a reference on the rte_device stating DPDK v20.11.

s/stating/starting/

>>>
>>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>> ---
>> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> I don't remember if I acked the reworded version, in anycase
> Acked-by: Gaetan Rivet <grive@u256.net>
> 

No you didn't, but I didn't replied to your comment on patch 2,
so you are forgiven :)

Thanks!
Maxime


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/2] doc: announce rte_dev_probe() API change
  2020-06-25  8:04 ` [dpdk-dev] [PATCH v2 1/2] doc: announce " Maxime Coquelin
  2020-07-31  7:20   ` Maxime Coquelin
  2020-07-31  9:26   ` Bruce Richardson
@ 2020-08-04  8:12   ` David Marchand
  2020-08-04  8:53     ` Jerin Jacob
  2 siblings, 1 reply; 13+ messages in thread
From: David Marchand @ 2020-08-04  8:12 UTC (permalink / raw)
  To: Maxime Coquelin
  Cc: dev, Gaetan Rivet, Wenzhuo Lu, Beilei Xing, Iremonger, Bernard

On Thu, Jun 25, 2020 at 10:04 AM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
>
> In order to simplify the use of rte_dev_probe() and
> rte_dev_remove() by applications, rte_dev_probe() will
> return a reference on the rte_device stating DPDK v20.11.
>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Acked-by: David Marchand <david.marchand@redhat.com>


-- 
David Marchand


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/2] doc: announce rte_dev_probe() API change
  2020-08-04  8:12   ` David Marchand
@ 2020-08-04  8:53     ` Jerin Jacob
  0 siblings, 0 replies; 13+ messages in thread
From: Jerin Jacob @ 2020-08-04  8:53 UTC (permalink / raw)
  To: David Marchand
  Cc: Maxime Coquelin, dev, Gaetan Rivet, Wenzhuo Lu, Beilei Xing,
	Iremonger, Bernard

On Tue, Aug 4, 2020 at 1:42 PM David Marchand <david.marchand@redhat.com> wrote:
>
> On Thu, Jun 25, 2020 at 10:04 AM Maxime Coquelin
> <maxime.coquelin@redhat.com> wrote:
> >
> > In order to simplify the use of rte_dev_probe() and
> > rte_dev_remove() by applications, rte_dev_probe() will
> > return a reference on the rte_device stating DPDK v20.11.
> >
> > Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>
> Acked-by: David Marchand <david.marchand@redhat.com>

Acked-by: Jerin Jacob <jerinj@marvell.com>


>
>
> --
> David Marchand
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH v2 (v20.11) 2/2] eal: improve device probing API
  2020-06-25  8:04 ` [dpdk-dev] [PATCH v2 (v20.11) 2/2] eal: improve device probing API Maxime Coquelin
  2020-06-29  9:57   ` Gaëtan Rivet
@ 2020-08-05 23:33   ` Thomas Monjalon
  2020-08-06 10:29     ` Maxime Coquelin
  1 sibling, 1 reply; 13+ messages in thread
From: Thomas Monjalon @ 2020-08-05 23:33 UTC (permalink / raw)
  To: Maxime Coquelin
  Cc: dev, grive, david.marchand, wenzhuo.lu, beilei.xing, bernard.iremonger

25/06/2020 10:04, Maxime Coquelin:
> This patch makes rte_dev_probe() to return the
> rte_device pointer on success and NULL on error
> instead of returning 0 on success and negative
> value on error.
> 
> The goal is to avoid that the calling application
> iterates the devices list afterwards to retrieve
> the pointer. Retrieving the pointer is required
> for calling rte_dev_remove() later.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
> --- a/lib/librte_eal/include/rte_dev.h
> +++ b/lib/librte_eal/include/rte_dev.h
> @@ -148,9 +148,9 @@ int rte_eal_hotplug_add(const char *busname, const char *devname,
>   * @param devargs
>   *   Device arguments including bus, class and driver properties.
>   * @return
> - *   0 on success, negative on error.
> + *   Generic device pointer on success, NULL on error.
>   */
> -int rte_dev_probe(const char *devargs);
> +struct rte_device *rte_dev_probe(const char *devargs);

Sorry for not catching it earlier, I think this change is against
the idea of having a generic devargs syntax.
One string could identify multiple devices.
And a successful probe does not mean there is a new rte_device
(can be an update, allowing more ports on the same device).



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH v2 (v20.11) 2/2] eal: improve device probing API
  2020-08-05 23:33   ` Thomas Monjalon
@ 2020-08-06 10:29     ` Maxime Coquelin
  2020-08-06 11:43       ` Thomas Monjalon
  0 siblings, 1 reply; 13+ messages in thread
From: Maxime Coquelin @ 2020-08-06 10:29 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: dev, grive, david.marchand, wenzhuo.lu, beilei.xing, bernard.iremonger



On 8/6/20 1:33 AM, Thomas Monjalon wrote:
> 25/06/2020 10:04, Maxime Coquelin:
>> This patch makes rte_dev_probe() to return the
>> rte_device pointer on success and NULL on error
>> instead of returning 0 on success and negative
>> value on error.
>>
>> The goal is to avoid that the calling application
>> iterates the devices list afterwards to retrieve
>> the pointer. Retrieving the pointer is required
>> for calling rte_dev_remove() later.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>> --- a/lib/librte_eal/include/rte_dev.h
>> +++ b/lib/librte_eal/include/rte_dev.h
>> @@ -148,9 +148,9 @@ int rte_eal_hotplug_add(const char *busname, const char *devname,
>>   * @param devargs
>>   *   Device arguments including bus, class and driver properties.
>>   * @return
>> - *   0 on success, negative on error.
>> + *   Generic device pointer on success, NULL on error.
>>   */
>> -int rte_dev_probe(const char *devargs);
>> +struct rte_device *rte_dev_probe(const char *devargs);
> 
> Sorry for not catching it earlier, I think this change is against
> the idea of having a generic devargs syntax.
> One string could identify multiple devices.

That sounds fragile to me. What if one of the multiple devices fails to
probe? Do the other ones are going to be removed?

> And a successful probe does not mean there is a new rte_device
> (can be an update, allowing more ports on the same device).

This should be done by a separate API in my opinion, ports may be seen
as a sub-function of the device.

But anyway, I am fine with dropping it. It is not blocking vDPA probing,
just making it more cumbersome.

Regards,
Maxime




^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH v2 (v20.11) 2/2] eal: improve device probing API
  2020-08-06 10:29     ` Maxime Coquelin
@ 2020-08-06 11:43       ` Thomas Monjalon
  0 siblings, 0 replies; 13+ messages in thread
From: Thomas Monjalon @ 2020-08-06 11:43 UTC (permalink / raw)
  To: Maxime Coquelin
  Cc: dev, grive, david.marchand, wenzhuo.lu, beilei.xing,
	bernard.iremonger, ferruh.yigit, akhil.goyal

06/08/2020 12:29, Maxime Coquelin:
> On 8/6/20 1:33 AM, Thomas Monjalon wrote:
> > 25/06/2020 10:04, Maxime Coquelin:
> >> This patch makes rte_dev_probe() to return the
> >> rte_device pointer on success and NULL on error
> >> instead of returning 0 on success and negative
> >> value on error.
> >>
> >> The goal is to avoid that the calling application
> >> iterates the devices list afterwards to retrieve
> >> the pointer. Retrieving the pointer is required
> >> for calling rte_dev_remove() later.
> >>
> >> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> >> ---
> >> --- a/lib/librte_eal/include/rte_dev.h
> >> +++ b/lib/librte_eal/include/rte_dev.h
> >> @@ -148,9 +148,9 @@ int rte_eal_hotplug_add(const char *busname, const char *devname,
> >>   * @param devargs
> >>   *   Device arguments including bus, class and driver properties.
> >>   * @return
> >> - *   0 on success, negative on error.
> >> + *   Generic device pointer on success, NULL on error.
> >>   */
> >> -int rte_dev_probe(const char *devargs);
> >> +struct rte_device *rte_dev_probe(const char *devargs);
> > 
> > Sorry for not catching it earlier, I think this change is against
> > the idea of having a generic devargs syntax.
> > One string could identify multiple devices.
> 
> That sounds fragile to me. What if one of the multiple devices fails to
> probe? Do the other ones are going to be removed?

No, what is correctly probed should not be rolled back in my opinion.

> > And a successful probe does not mean there is a new rte_device
> > (can be an update, allowing more ports on the same device).
> 
> This should be done by a separate API in my opinion, ports may be seen
> as a sub-function of the device.
> 
> But anyway, I am fine with dropping it. It is not blocking vDPA probing,
> just making it more cumbersome.

Apps should not iterate on devices.
vDPA should have an event mechanism like in ethdev:
a callback with RTE_ETH_EVENT_NEW is called for each new port probed.




^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-08-06 11:43 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-25  8:04 [dpdk-dev] [PATCH v2 0/2] rte_dev_probe() API change Maxime Coquelin
2020-06-25  8:04 ` [dpdk-dev] [PATCH v2 1/2] doc: announce " Maxime Coquelin
2020-07-31  7:20   ` Maxime Coquelin
2020-07-31  9:26   ` Bruce Richardson
2020-07-31  9:44     ` Gaëtan Rivet
2020-07-31  9:47       ` Maxime Coquelin
2020-08-04  8:12   ` David Marchand
2020-08-04  8:53     ` Jerin Jacob
2020-06-25  8:04 ` [dpdk-dev] [PATCH v2 (v20.11) 2/2] eal: improve device probing API Maxime Coquelin
2020-06-29  9:57   ` Gaëtan Rivet
2020-08-05 23:33   ` Thomas Monjalon
2020-08-06 10:29     ` Maxime Coquelin
2020-08-06 11:43       ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).