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

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] 8+ messages in thread

* [dpdk-dev] [PATCH 1/2] doc: announce rte_dev_probe() API change
  2020-06-08 15:53 [dpdk-dev] [PATCH 0/2] rte_dev_probe() API change Maxime Coquelin
@ 2020-06-08 15:53 ` Maxime Coquelin
  2020-06-11  8:08   ` Gaëtan Rivet
  2020-06-08 15:53 ` [dpdk-dev] [PATCH v20.11 2/2] eal: improve device probing API Maxime Coquelin
  1 sibling, 1 reply; 8+ messages in thread
From: Maxime Coquelin @ 2020-06-08 15:53 UTC (permalink / raw)
  To: dev, david.marchand, wenzhuo.lu, beilei.xing, bernard.iremonger, grive
  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 0bee924255..47151eac0b 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -138,3 +138,8 @@ Deprecation Notices
   driver probe scheme. The legacy virtio support will be available through
   the existing VFIO/UIO based kernel driver scheme.
   More details at https://patches.dpdk.org/patch/69351/
+
+* eal: Change ``rte_dev_probe`` API to return a pointer on the probed
+  rte_device or NULL instead of 0 or an error code in DPDK v20.11. This
+  change will help calling application in avoiding to iterate the devices
+  list when willing to call rte_dev_remove() later.
-- 
2.26.2


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

* [dpdk-dev] [PATCH v20.11 2/2] eal: improve device probing API
  2020-06-08 15:53 [dpdk-dev] [PATCH 0/2] rte_dev_probe() API change Maxime Coquelin
  2020-06-08 15:53 ` [dpdk-dev] [PATCH 1/2] doc: announce " Maxime Coquelin
@ 2020-06-08 15:53 ` Maxime Coquelin
  2020-06-10 12:06   ` Gaëtan Rivet
  1 sibling, 1 reply; 8+ messages in thread
From: Maxime Coquelin @ 2020-06-08 15:53 UTC (permalink / raw)
  To: dev, david.marchand, wenzhuo.lu, beilei.xing, bernard.iremonger, grive
  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] 8+ messages in thread

* Re: [dpdk-dev] [PATCH v20.11 2/2] eal: improve device probing API
  2020-06-08 15:53 ` [dpdk-dev] [PATCH v20.11 2/2] eal: improve device probing API Maxime Coquelin
@ 2020-06-10 12:06   ` Gaëtan Rivet
  2020-06-10 17:08     ` Maxime Coquelin
  0 siblings, 1 reply; 8+ messages in thread
From: Gaëtan Rivet @ 2020-06-10 12:06 UTC (permalink / raw)
  To: Maxime Coquelin
  Cc: dev, david.marchand, wenzhuo.lu, beilei.xing, bernard.iremonger

Hello Maxime,

On 08/06/20 17:53 +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.
> 

I agree with the idea. I recall starting to do it on the legacy functions
(rte_eal_hotplug_*), but it was scrapped for API compat.

> 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;

Is is a problem to keep the ENOMSG through rte_errno?

>  		}
>  		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.

If rte_errno is set, mapping codes to meanings would be helpful here.

Acked-by: Gaetan Rivet <grive@u256.net>

-- 
Gaëtan

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

* Re: [dpdk-dev] [PATCH v20.11 2/2] eal: improve device probing API
  2020-06-10 12:06   ` Gaëtan Rivet
@ 2020-06-10 17:08     ` Maxime Coquelin
  2020-06-10 18:13       ` Gaëtan Rivet
  0 siblings, 1 reply; 8+ messages in thread
From: Maxime Coquelin @ 2020-06-10 17:08 UTC (permalink / raw)
  To: Gaëtan Rivet
  Cc: dev, david.marchand, wenzhuo.lu, beilei.xing, bernard.iremonger

Hi Gaëtan,

On 6/10/20 2:06 PM, Gaëtan Rivet wrote:
> Hello Maxime,
> 
> On 08/06/20 17:53 +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.
>>
> 
> I agree with the idea. I recall starting to do it on the legacy functions
> (rte_eal_hotplug_*), but it was scrapped for API compat.
> 
>> 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(-)
>>
...
>>  
>>  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.
> 
> If rte_errno is set, mapping codes to meanings would be helpful here.

Actually David made me the same comment before I post the patch.
I am fine with setting rte_errno. If we do that, I think we should have
fixed error code in rte_dev_probe() and not propagate error codes from
functions it calls. Otherwise it's likely the API doc will be outdated
quite rapidly.

What do you think?

> Acked-by: Gaetan Rivet <grive@u256.net>
> 

Great, could you also ack the deprecation notice, as this is the part
that needs to be merged into v20.08?

Thanks!
Maxime


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

* Re: [dpdk-dev] [PATCH v20.11 2/2] eal: improve device probing API
  2020-06-10 17:08     ` Maxime Coquelin
@ 2020-06-10 18:13       ` Gaëtan Rivet
  0 siblings, 0 replies; 8+ messages in thread
From: Gaëtan Rivet @ 2020-06-10 18:13 UTC (permalink / raw)
  To: Maxime Coquelin
  Cc: dev, david.marchand, wenzhuo.lu, beilei.xing, bernard.iremonger

On 10/06/20 19:08 +0200, Maxime Coquelin wrote:
> Hi Gaëtan,
> 
> On 6/10/20 2:06 PM, Gaëtan Rivet wrote:
> > Hello Maxime,
> > 
> > On 08/06/20 17:53 +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.
> >>
> > 
> > I agree with the idea. I recall starting to do it on the legacy functions
> > (rte_eal_hotplug_*), but it was scrapped for API compat.
> > 
> >> 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(-)
> >>
> ...
> >>  
> >>  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.
> > 
> > If rte_errno is set, mapping codes to meanings would be helpful here.
> 
> Actually David made me the same comment before I post the patch.
> I am fine with setting rte_errno. If we do that, I think we should have
> fixed error code in rte_dev_probe() and not propagate error codes from
> functions it calls. Otherwise it's likely the API doc will be outdated
> quite rapidly.
> 
> What do you think?
> 

Well we're stuck with the classic errno limitations.

I agree with you, if we consider possible errors as part of a function
API, then we cannot recursively inherit this API from callees.

That being said, masking errors is not ok. If an error cannot be handled
by rte_dev_probe(), it should log an appropriate message and set
rte_errno to a value that is part of its API. If the error can be
handled, then errno should be reset to its original value preceding
rte_dev_probe() call.

Of course the EAL rarely does it, and even myself I probably rarely
respected this behavior, but it would be nice if we could all define a
common agreed-upon discipline in the EAL and stick to it. I think the
current state of error reporting in EAL is terrible for users downstream.

> > Acked-by: Gaetan Rivet <grive@u256.net>
> > 
> 
> Great, could you also ack the deprecation notice, as this is the part
> that needs to be merged into v20.08?
> 

I wanted to refresh myself with the latest rules about API breakage
before doing so but got context switched away :) . I will get back to it.

> Thanks!
> Maxime
> 

-- 
Gaëtan

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

* Re: [dpdk-dev] [PATCH 1/2] doc: announce rte_dev_probe() API change
  2020-06-08 15:53 ` [dpdk-dev] [PATCH 1/2] doc: announce " Maxime Coquelin
@ 2020-06-11  8:08   ` Gaëtan Rivet
  2020-06-25  7:50     ` Maxime Coquelin
  0 siblings, 1 reply; 8+ messages in thread
From: Gaëtan Rivet @ 2020-06-11  8:08 UTC (permalink / raw)
  To: Maxime Coquelin
  Cc: dev, david.marchand, wenzhuo.lu, beilei.xing, bernard.iremonger

On 08/06/20 17:53 +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>
> ---
>  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 0bee924255..47151eac0b 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -138,3 +138,8 @@ Deprecation Notices
>    driver probe scheme. The legacy virtio support will be available through
>    the existing VFIO/UIO based kernel driver scheme.
>    More details at https://patches.dpdk.org/patch/69351/

On the principle ok, formulation seems heavy though (but I'm not a
native speaker so ymmv...):

> +
> +* eal: Change ``rte_dev_probe`` API to return a pointer on the probed
> +  rte_device or NULL instead of 0 or an error code in DPDK v20.11.

The 'in DPDK v20.11' is confusing here (it could equally apply to first
or second part of the sentence). Given the context it's obvious, but
maybe:

Change ``rte_dev_probe`` API in DPDK v20.11 to return a pointer on ...

> +                                                                   This
> +  change will help calling application in avoiding to iterate the devices
> +  list when willing to call rte_dev_remove() later.

How about:

   This change will allow applications avoid iterating on devices after a
   probe to get access to the new rte_device.

> -- 
> 2.26.2
> 

-- 
Gaëtan

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

* Re: [dpdk-dev] [PATCH 1/2] doc: announce rte_dev_probe() API change
  2020-06-11  8:08   ` Gaëtan Rivet
@ 2020-06-25  7:50     ` Maxime Coquelin
  0 siblings, 0 replies; 8+ messages in thread
From: Maxime Coquelin @ 2020-06-25  7:50 UTC (permalink / raw)
  To: Gaëtan Rivet
  Cc: dev, david.marchand, wenzhuo.lu, beilei.xing, bernard.iremonger



On 6/11/20 10:08 AM, Gaëtan Rivet wrote:
> On 08/06/20 17:53 +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>
>> ---
>>  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 0bee924255..47151eac0b 100644
>> --- a/doc/guides/rel_notes/deprecation.rst
>> +++ b/doc/guides/rel_notes/deprecation.rst
>> @@ -138,3 +138,8 @@ Deprecation Notices
>>    driver probe scheme. The legacy virtio support will be available through
>>    the existing VFIO/UIO based kernel driver scheme.
>>    More details at https://patches.dpdk.org/patch/69351/
> 
> On the principle ok, formulation seems heavy though (but I'm not a
> native speaker so ymmv...):
> 
>> +
>> +* eal: Change ``rte_dev_probe`` API to return a pointer on the probed
>> +  rte_device or NULL instead of 0 or an error code in DPDK v20.11.
> 
> The 'in DPDK v20.11' is confusing here (it could equally apply to first
> or second part of the sentence). Given the context it's obvious, but
> maybe:
> 
> Change ``rte_dev_probe`` API in DPDK v20.11 to return a pointer on ...
> 
>> +                                                                   This
>> +  change will help calling application in avoiding to iterate the devices
>> +  list when willing to call rte_dev_remove() later.
> 
> How about:
> 
>    This change will allow applications avoid iterating on devices after a
>    probe to get access to the new rte_device.
> 

Good suggestions, I'll fix that.

Thanks,
Maxime


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

end of thread, other threads:[~2020-06-25  7:50 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-08 15:53 [dpdk-dev] [PATCH 0/2] rte_dev_probe() API change Maxime Coquelin
2020-06-08 15:53 ` [dpdk-dev] [PATCH 1/2] doc: announce " Maxime Coquelin
2020-06-11  8:08   ` Gaëtan Rivet
2020-06-25  7:50     ` Maxime Coquelin
2020-06-08 15:53 ` [dpdk-dev] [PATCH v20.11 2/2] eal: improve device probing API Maxime Coquelin
2020-06-10 12:06   ` Gaëtan Rivet
2020-06-10 17:08     ` Maxime Coquelin
2020-06-10 18:13       ` Gaëtan Rivet

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git