[dpdk-dev] doc: annouce ABI change for cryptodev ops structure

Message ID 1486726767-81742-1-git-send-email-roy.fan.zhang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel compilation success Compilation OK

Commit Message

Fan Zhang Feb. 10, 2017, 11:39 a.m. UTC
  Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
---
 doc/guides/rel_notes/deprecation.rst | 4 ++++
 1 file changed, 4 insertions(+)
  

Comments

Fiona Trahe Feb. 10, 2017, 1:59 p.m. UTC | #1
Hi Fan,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Fan Zhang
> Sent: Friday, February 10, 2017 11:39 AM
> To: dev@dpdk.org
> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Subject: [dpdk-dev] [PATCH] doc: annouce ABI change for cryptodev ops
> structure
> 
> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> ---
>  doc/guides/rel_notes/deprecation.rst | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> index 755dc65..564d93a 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -62,3 +62,7 @@ Deprecation Notices
>    PMDs that implement the latter.
>    Target release for removal of the legacy API will be defined once most
>    PMDs have switched to rte_flow.
> +
> +* ABI changes are planned for 17.05 in the ``rte_cryptodev_ops`` structure.
> +  The field ``cryptodev_configure_t`` function prototype will be added a
> +  parameter of a struct rte_cryptodev_config type pointer.
> --
> 2.7.4

Can you fix the grammar here please. I'm not sure what the change is?
  
Fan Zhang Feb. 13, 2017, 4:07 p.m. UTC | #2
Hi Fiona,

Sorry for my bad English, I will try to explain better here.

"cryptodev_configure_t" is a function prototype with only "rte_cryptodev *dev"
as sole parameter. Structure ``rte_cryptodev_ops`` holds one function pointer
"dev_configure" of it. 

The patch involves in the announcement of adding a parameter of 
"struct rte_cryptodev_config" pointer so the function prototype could look like:

typedef int (*cryptodev_configure_t)(struct rte_cryptodev *dev, struct rte_cryptodev_config *config);

Without this parameter, a specific crypto PMD may not have enough information to
configure itself. Which may not be big problem as other Cryptodevs as all configures
are done in rte_cryptodev_configure(), but it is important for the scheduler PMD as it
needs this parameter to configure all its slaves. Currently the user have to configure
every slave one by one.

The problem is, although I want to change an API of the function prototype "cryptodev_configure_t",
but in order to do that I have to break the ABI of structure "rte_cryptodev_ops". Any help on the grammar
for stating this nicer would be appreciated.

Best regards,
Fan




> -----Original Message-----
> From: Trahe, Fiona
> Sent: Friday, February 10, 2017 2:00 PM
> To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; dev@dpdk.org
> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Trahe, Fiona
> <fiona.trahe@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] doc: annouce ABI change for cryptodev ops
> structure
> 
> Hi Fan,
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Fan Zhang
> > Sent: Friday, February 10, 2017 11:39 AM
> > To: dev@dpdk.org
> > Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> > Subject: [dpdk-dev] [PATCH] doc: annouce ABI change for cryptodev ops
> > structure
> >
> > Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> > ---
> >  doc/guides/rel_notes/deprecation.rst | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/doc/guides/rel_notes/deprecation.rst
> > b/doc/guides/rel_notes/deprecation.rst
> > index 755dc65..564d93a 100644
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > @@ -62,3 +62,7 @@ Deprecation Notices
> >    PMDs that implement the latter.
> >    Target release for removal of the legacy API will be defined once most
> >    PMDs have switched to rte_flow.
> > +
> > +* ABI changes are planned for 17.05 in the ``rte_cryptodev_ops`` structure.
> > +  The field ``cryptodev_configure_t`` function prototype will be
> > +added a
> > +  parameter of a struct rte_cryptodev_config type pointer.
> > --
> > 2.7.4
> 
> Can you fix the grammar here please. I'm not sure what the change is?
  
Fiona Trahe Feb. 13, 2017, 5:34 p.m. UTC | #3
Thanks Fan, now it makes sense.

> -----Original Message-----
> From: Zhang, Roy Fan
> Sent: Monday, February 13, 2017 4:07 PM
> To: Trahe, Fiona <fiona.trahe@intel.com>; dev@dpdk.org
> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] doc: annouce ABI change for cryptodev ops
> structure
> 
> Hi Fiona,
> 
> Sorry for my bad English, I will try to explain better here.
> 
> "cryptodev_configure_t" is a function prototype with only "rte_cryptodev
> *dev"
> as sole parameter. Structure ``rte_cryptodev_ops`` holds one function pointer
> "dev_configure" of it.
> 
> The patch involves in the announcement of adding a parameter of
> "struct rte_cryptodev_config" pointer so the function prototype could look
> like:
> 
> typedef int (*cryptodev_configure_t)(struct rte_cryptodev *dev, struct
> rte_cryptodev_config *config);
> 
> Without this parameter, a specific crypto PMD may not have enough
> information to
> configure itself. Which may not be big problem as other Cryptodevs as all
> configures
> are done in rte_cryptodev_configure(), but it is important for the scheduler
> PMD as it
> needs this parameter to configure all its slaves. Currently the user have to
> configure
> every slave one by one.
> 
> The problem is, although I want to change an API of the function prototype
> "cryptodev_configure_t",
> but in order to do that I have to break the ABI of structure
> "rte_cryptodev_ops". Any help on the grammar
> for stating this nicer would be appreciated.
> 
> Best regards,
> Fan
> 
> 
> 
> 
> > -----Original Message-----
> > From: Trahe, Fiona
> > Sent: Friday, February 10, 2017 2:00 PM
> > To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; dev@dpdk.org
> > Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Trahe, Fiona
> > <fiona.trahe@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH] doc: annouce ABI change for cryptodev ops
> > structure
> >
> > Hi Fan,
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Fan Zhang
> > > Sent: Friday, February 10, 2017 11:39 AM
> > > To: dev@dpdk.org
> > > Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> > > Subject: [dpdk-dev] [PATCH] doc: annouce ABI change for cryptodev ops
> > > structure
> > >
> > > Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> > > ---
> > >  doc/guides/rel_notes/deprecation.rst | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/doc/guides/rel_notes/deprecation.rst
> > > b/doc/guides/rel_notes/deprecation.rst
> > > index 755dc65..564d93a 100644
> > > --- a/doc/guides/rel_notes/deprecation.rst
> > > +++ b/doc/guides/rel_notes/deprecation.rst
> > > @@ -62,3 +62,7 @@ Deprecation Notices
> > >    PMDs that implement the latter.
> > >    Target release for removal of the legacy API will be defined once most
> > >    PMDs have switched to rte_flow.
> > > +
> > > +* ABI changes are planned for 17.05 in the ``rte_cryptodev_ops``
> structure.
> > > +  The field ``cryptodev_configure_t`` function prototype will be
> > > +added a
> > > +  parameter of a struct rte_cryptodev_config type pointer.
> > > --
> > > 2.7.4
> >
> > Can you fix the grammar here please. I'm not sure what the change is?
  
Hemant Agrawal Feb. 14, 2017, 12:21 a.m. UTC | #4
On 2/10/2017 7:59 AM, Trahe, Fiona wrote:
> Hi Fan,
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Fan Zhang
>> Sent: Friday, February 10, 2017 11:39 AM
>> To: dev@dpdk.org
>> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
>> Subject: [dpdk-dev] [PATCH] doc: annouce ABI change for cryptodev ops
>> structure
>>
>> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
>> ---
>>  doc/guides/rel_notes/deprecation.rst | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/doc/guides/rel_notes/deprecation.rst
>> b/doc/guides/rel_notes/deprecation.rst
>> index 755dc65..564d93a 100644
>> --- a/doc/guides/rel_notes/deprecation.rst
>> +++ b/doc/guides/rel_notes/deprecation.rst
>> @@ -62,3 +62,7 @@ Deprecation Notices
>>    PMDs that implement the latter.
>>    Target release for removal of the legacy API will be defined once most
>>    PMDs have switched to rte_flow.
>> +
>> +* ABI changes are planned for 17.05 in the ``rte_cryptodev_ops`` structure.
>> +  The field ``cryptodev_configure_t`` function prototype will be added a
>> +  parameter of a struct rte_cryptodev_config type pointer.
>> --
>> 2.7.4
>
> Can you fix the grammar here please. I'm not sure what the change is?
>
I also find it hard to understand it first. Not perfect, but I tried to 
reword it.

A new parameter ``struct rte_cryptodev_config *config`` will be added to 
the ``cryptodev_configure_t`` function pointer field.
  
Hemant Agrawal Feb. 14, 2017, 5:11 a.m. UTC | #5
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Hemant Agrawal
> Sent: Monday, February 13, 2017 6:21 PM
> To: Trahe, Fiona <fiona.trahe@intel.com>; Zhang, Roy Fan
> <roy.fan.zhang@intel.com>; dev@dpdk.org
> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] doc: annouce ABI change for cryptodev ops
> structure
> 
> On 2/10/2017 7:59 AM, Trahe, Fiona wrote:
> > Hi Fan,
> >
> >> -----Original Message-----
> >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Fan Zhang
> >> Sent: Friday, February 10, 2017 11:39 AM
> >> To: dev@dpdk.org
> >> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> >> Subject: [dpdk-dev] [PATCH] doc: annouce ABI change for cryptodev ops
> >> structure
> >>
> >> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> >> ---
> >>  doc/guides/rel_notes/deprecation.rst | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >> diff --git a/doc/guides/rel_notes/deprecation.rst
> >> b/doc/guides/rel_notes/deprecation.rst
> >> index 755dc65..564d93a 100644
> >> --- a/doc/guides/rel_notes/deprecation.rst
> >> +++ b/doc/guides/rel_notes/deprecation.rst
> >> @@ -62,3 +62,7 @@ Deprecation Notices
> >>    PMDs that implement the latter.
> >>    Target release for removal of the legacy API will be defined once most
> >>    PMDs have switched to rte_flow.
> >> +
> >> +* ABI changes are planned for 17.05 in the ``rte_cryptodev_ops``
> structure.
> >> +  The field ``cryptodev_configure_t`` function prototype will be
> >> +added a
> >> +  parameter of a struct rte_cryptodev_config type pointer.
> >> --
> >> 2.7.4
> >
> > Can you fix the grammar here please. I'm not sure what the change is?
> >
> I also find it hard to understand it first. Not perfect, but I tried to reword it.
> 
> A new parameter ``struct rte_cryptodev_config *config`` will be added to the
> ``cryptodev_configure_t`` function pointer field.
> 

In any case,
Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>
  

Patch

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 755dc65..564d93a 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -62,3 +62,7 @@  Deprecation Notices
   PMDs that implement the latter.
   Target release for removal of the legacy API will be defined once most
   PMDs have switched to rte_flow.
+
+* ABI changes are planned for 17.05 in the ``rte_cryptodev_ops`` structure.
+  The field ``cryptodev_configure_t`` function prototype will be added a
+  parameter of a struct rte_cryptodev_config type pointer.