[dpdk-dev] doc: announce API change for virtual device initialization

Message ID 1467643848-14780-1-git-send-email-pablo.de.lara.guarch@intel.com (mailing list archive)
State Rejected, archived
Headers

Commit Message

De Lara Guarch, Pablo July 4, 2016, 2:50 p.m. UTC
  In order to create a virtual device, user needs to call
rte_eal_vdev_init generally, but this function returns 0
on success or negative number if error. Instead, something
more useful would be to return the port or device id of the
device created, so the user can call rte_eal_vdev_init
function and use the device straight away, using the id returned
by the function.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 doc/guides/rel_notes/deprecation.rst | 5 +++++
 1 file changed, 5 insertions(+)
  

Comments

Doherty, Declan July 5, 2016, 10:44 a.m. UTC | #1
On 04/07/16 15:50, Pablo de Lara wrote:
> In order to create a virtual device, user needs to call
> rte_eal_vdev_init generally, but this function returns 0
> on success or negative number if error. Instead, something
> more useful would be to return the port or device id of the
> device created, so the user can call rte_eal_vdev_init
> function and use the device straight away, using the id returned
> by the function.
>
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.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 f502f86..60cc7c7 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -41,3 +41,8 @@ Deprecation Notices
>  * The mempool functions for single/multi producer/consumer are deprecated and
>    will be removed in 16.11.
>    It is replaced by rte_mempool_generic_get/put functions.
> +
> +* The rte_eal_vdev_init function will be changed in 16.11 to return
> +  the port/device id of the device created, instead of 0, when it has been
> +  initialized successfully, so user can use the returned value straight away
> +  to call all the device functions that require that parameter.
>

Acked-by: Declan Doherty <declan.doherty@intel.com>
  
Ferruh Yigit July 5, 2016, 1:04 p.m. UTC | #2
On 7/4/2016 3:50 PM, Pablo de Lara wrote:
> In order to create a virtual device, user needs to call
> rte_eal_vdev_init generally, but this function returns 0
> on success or negative number if error. Instead, something
> more useful would be to return the port or device id of the
> device created, so the user can call rte_eal_vdev_init
> function and use the device straight away, using the id returned
> by the function.
> 
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.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 f502f86..60cc7c7 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -41,3 +41,8 @@ Deprecation Notices
>  * The mempool functions for single/multi producer/consumer are deprecated and
>    will be removed in 16.11.
>    It is replaced by rte_mempool_generic_get/put functions.
> +
> +* The rte_eal_vdev_init function will be changed in 16.11 to return
> +  the port/device id of the device created, instead of 0, when it has been
> +  initialized successfully, so user can use the returned value straight away
> +  to call all the device functions that require that parameter.
> 

Hi Pablo,

There is another API rte_eth_dev_attach(), which returns port_id, and is
a common wrapper both for pdev and vdev.
So with proper devargs this API calls rte_eal_vdev_init() and returns
port_id (set in argument) without any extra side effect.

I just would like remind it in case that works for you, since what
rte_eth_dev_attach() does sounds like what is targeted with this API change.

Thanks,
ferruh
  
Thomas Monjalon July 5, 2016, 1:38 p.m. UTC | #3
2016-07-05 14:04, Ferruh Yigit:
> On 7/4/2016 3:50 PM, Pablo de Lara wrote:
> > +* The rte_eal_vdev_init function will be changed in 16.11 to return
> > +  the port/device id of the device created, instead of 0, when it has been
> > +  initialized successfully, so user can use the returned value straight away
> > +  to call all the device functions that require that parameter.
> 
> There is another API rte_eth_dev_attach(), which returns port_id, and is
> a common wrapper both for pdev and vdev.
> So with proper devargs this API calls rte_eal_vdev_init() and returns
> port_id (set in argument) without any extra side effect.

I think rte_eth_dev_attach() should be removed from ethdev.
Hotplugging a device belongs to device management, i.e. EAL.
  
De Lara Guarch, Pablo July 5, 2016, 2:55 p.m. UTC | #4
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Tuesday, July 05, 2016 2:39 PM
> To: Yigit, Ferruh
> Cc: dev@dpdk.org; De Lara Guarch, Pablo; Mcnamara, John
> Subject: Re: [dpdk-dev] [PATCH] doc: announce API change for virtual device
> initialization
> 
> 2016-07-05 14:04, Ferruh Yigit:
> > On 7/4/2016 3:50 PM, Pablo de Lara wrote:
> > > +* The rte_eal_vdev_init function will be changed in 16.11 to return
> > > +  the port/device id of the device created, instead of 0, when it has been
> > > +  initialized successfully, so user can use the returned value straight away
> > > +  to call all the device functions that require that parameter.
> >
> > There is another API rte_eth_dev_attach(), which returns port_id, and is
> > a common wrapper both for pdev and vdev.
> > So with proper devargs this API calls rte_eal_vdev_init() and returns
> > port_id (set in argument) without any extra side effect.
> 
> I think rte_eth_dev_attach() should be removed from ethdev.
> Hotplugging a device belongs to device management, i.e. EAL.

Even though rte_eth_dev_attach returns the port id, I think that rte_eal_vdev_init
can return the port id anyway (and then, we can remove the call to rte_eth_dev_get_port_by_name
 in rte_eth_dev_attach).
  
De Lara Guarch, Pablo July 6, 2016, 10:59 a.m. UTC | #5
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of De Lara Guarch,
> Pablo
> Sent: Tuesday, July 05, 2016 3:55 PM
> To: Thomas Monjalon; Yigit, Ferruh
> Cc: dev@dpdk.org; Mcnamara, John
> Subject: Re: [dpdk-dev] [PATCH] doc: announce API change for virtual device
> initialization
> 
> 
> 
> > -----Original Message-----
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > Sent: Tuesday, July 05, 2016 2:39 PM
> > To: Yigit, Ferruh
> > Cc: dev@dpdk.org; De Lara Guarch, Pablo; Mcnamara, John
> > Subject: Re: [dpdk-dev] [PATCH] doc: announce API change for virtual device
> > initialization
> >
> > 2016-07-05 14:04, Ferruh Yigit:
> > > On 7/4/2016 3:50 PM, Pablo de Lara wrote:
> > > > +* The rte_eal_vdev_init function will be changed in 16.11 to return
> > > > +  the port/device id of the device created, instead of 0, when it has
> been
> > > > +  initialized successfully, so user can use the returned value straight
> away
> > > > +  to call all the device functions that require that parameter.
> > >
> > > There is another API rte_eth_dev_attach(), which returns port_id, and is
> > > a common wrapper both for pdev and vdev.
> > > So with proper devargs this API calls rte_eal_vdev_init() and returns
> > > port_id (set in argument) without any extra side effect.
> >
> > I think rte_eth_dev_attach() should be removed from ethdev.
> > Hotplugging a device belongs to device management, i.e. EAL.
> 
> Even though rte_eth_dev_attach returns the port id, I think that
> rte_eal_vdev_init
> can return the port id anyway (and then, we can remove the call to
> rte_eth_dev_get_port_by_name
>  in rte_eth_dev_attach).

Also, that function is only applicable to Ethernet devices,
so other devices like Crypto devices cannot be used.
If we want to use that function for all devices, we should move it to eal,
modify it slightly and rename it, and therefore, we will need
a deprecation notice as well.
  
David Marchand July 28, 2016, 12:18 p.m. UTC | #6
Hello Pablo,

On Mon, Jul 4, 2016 at 4:50 PM, Pablo de Lara
<pablo.de.lara.guarch@intel.com> wrote:
> In order to create a virtual device, user needs to call
> rte_eal_vdev_init generally, but this function returns 0
> on success or negative number if error. Instead, something
> more useful would be to return the port or device id of the
> device created, so the user can call rte_eal_vdev_init
> function and use the device straight away, using the id returned
> by the function.
>
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.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 f502f86..60cc7c7 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -41,3 +41,8 @@ Deprecation Notices
>  * The mempool functions for single/multi producer/consumer are deprecated and
>    will be removed in 16.11.
>    It is replaced by rte_mempool_generic_get/put functions.
> +
> +* The rte_eal_vdev_init function will be changed in 16.11 to return
> +  the port/device id of the device created, instead of 0, when it has been
> +  initialized successfully, so user can use the returned value straight away
> +  to call all the device functions that require that parameter.
> --
> 2.5.0
>

This is a layer violation.
EAL does not know a thing about "ports".

This information should come from the crypto framework and so an api
in crypto framework is the right place, not EAL.

This is a NACK for me.
  
De Lara Guarch, Pablo July 28, 2016, 3:47 p.m. UTC | #7
> -----Original Message-----

> From: David Marchand [mailto:david.marchand@6wind.com]

> Sent: Thursday, July 28, 2016 5:18 AM

> To: De Lara Guarch, Pablo

> Cc: dev@dpdk.org; Mcnamara, John

> Subject: Re: [dpdk-dev] [PATCH] doc: announce API change for virtual device

> initialization

> 

> Hello Pablo,

> 

> On Mon, Jul 4, 2016 at 4:50 PM, Pablo de Lara

> <pablo.de.lara.guarch@intel.com> wrote:

> > In order to create a virtual device, user needs to call

> > rte_eal_vdev_init generally, but this function returns 0

> > on success or negative number if error. Instead, something

> > more useful would be to return the port or device id of the

> > device created, so the user can call rte_eal_vdev_init

> > function and use the device straight away, using the id returned

> > by the function.

> >

> > Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.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 f502f86..60cc7c7 100644

> > --- a/doc/guides/rel_notes/deprecation.rst

> > +++ b/doc/guides/rel_notes/deprecation.rst

> > @@ -41,3 +41,8 @@ Deprecation Notices

> >  * The mempool functions for single/multi producer/consumer are

> deprecated and

> >    will be removed in 16.11.

> >    It is replaced by rte_mempool_generic_get/put functions.

> > +

> > +* The rte_eal_vdev_init function will be changed in 16.11 to return

> > +  the port/device id of the device created, instead of 0, when it has been

> > +  initialized successfully, so user can use the returned value straight away

> > +  to call all the device functions that require that parameter.

> > --

> > 2.5.0

> >

> 

> This is a layer violation.

> EAL does not know a thing about "ports".

> 

> This information should come from the crypto framework and so an api

> in crypto framework is the right place, not EAL.

> 

> This is a NACK for me.


Fair enough. So you mean to use rte_eth_dev_attach in ethdev library and
a similar function in cryptodev library?

Thanks,
Pablo
> 

> 

> --

> David Marchand
  
David Marchand July 28, 2016, 3:56 p.m. UTC | #8
On Thu, Jul 28, 2016 at 5:47 PM, De Lara Guarch, Pablo
<pablo.de.lara.guarch@intel.com> wrote:
>> This is a layer violation.
>> EAL does not know a thing about "ports".
>>
>> This information should come from the crypto framework and so an api
>> in crypto framework is the right place, not EAL.
>>
>> This is a NACK for me.
>
> Fair enough. So you mean to use rte_eth_dev_attach in ethdev library and
> a similar function in cryptodev library?

Yes, and/or wait that the rework in ethdev/eal hotplug has taken place
and mirror this in crypto.
  
Andriy Berestovskyy July 29, 2016, 5:35 p.m. UTC | #9
Hey folks,

> On 28 Jul 2016, at 17:47, De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com> wrote:
> Fair enough. So you mean to use rte_eth_dev_attach in ethdev library and
> a similar function in cryptodev library?

There is a rte_eth_dev_get_port_by_name() which gets the port id right after the rte_eal_vdev_init() call. You might consider the same for the crypto...

Regards,
Andriy
  

Patch

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index f502f86..60cc7c7 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -41,3 +41,8 @@  Deprecation Notices
 * The mempool functions for single/multi producer/consumer are deprecated and
   will be removed in 16.11.
   It is replaced by rte_mempool_generic_get/put functions.
+
+* The rte_eal_vdev_init function will be changed in 16.11 to return
+  the port/device id of the device created, instead of 0, when it has been
+  initialized successfully, so user can use the returned value straight away
+  to call all the device functions that require that parameter.