doc/compressdev: clarify that structs should be zeroed before use

Message ID 1566926764-31816-1-git-send-email-fiona.trahe@intel.com (mailing list archive)
State Rejected, archived
Headers
Series doc/compressdev: clarify that structs should be zeroed before use |

Checks

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

Commit Message

Fiona Trahe Aug. 27, 2019, 5:26 p.m. UTC
  Some structs used on the API are zeroed on creation by API calls,
(e.g. rte_comp_op), but a few are allocated in the application domain.
Clarify that the application should zero those to enable future
extensions without API breakage.

Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
---
 doc/guides/prog_guide/compressdev.rst |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)
  

Comments

Shally Verma Aug. 28, 2019, 4:37 a.m. UTC | #1
> -----Original Message-----
> From: Fiona Trahe <fiona.trahe@intel.com>
> Sent: Tuesday, August 27, 2019 10:56 PM
> To: dev@dpdk.org
> Cc: akhil.goyal@nxp.com; Ashish Gupta <ashishg@marvell.com>; Shally
> Verma <shallyv@marvell.com>; Fiona Trahe <fiona.trahe@intel.com>
> Subject: [PATCH] doc/compressdev: clarify that structs should be zeroed
> before use
> 
> Some structs used on the API are zeroed on creation by API calls, (e.g.
> rte_comp_op), but a few are allocated in the application domain.
> Clarify that the application should zero those to enable future extensions
> without API breakage.
> 
> Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
> ---
>  doc/guides/prog_guide/compressdev.rst |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/doc/guides/prog_guide/compressdev.rst
> b/doc/guides/prog_guide/compressdev.rst
> index a089db1..2a85eba 100644
> --- a/doc/guides/prog_guide/compressdev.rst
> +++ b/doc/guides/prog_guide/compressdev.rst
> @@ -76,6 +76,11 @@ The ``rte_compressdev_configure`` API is used to
> configure a compression device.
>  The ``rte_compressdev_config`` structure is used to pass the configuration
> parameters.
> 
> +The allocation of the ``rte_compressdev_config`` struct passed on the
> +API is in the application domain, so to allow future API extensions in
> +a backwardly compatible manner the application should zero this struct,
> +e.g. using sizeof(), before populating it. This allows the addition of new
> parameters to the struct with default value of zero indicating original
> behaviour.
> +
>  See *DPDK API Reference* for details.
> 
>  Configuration of Queue Pairs
> @@ -264,6 +269,11 @@ Compression transforms (``rte_comp_xform``) are
> the mechanism  to specify the details of the compression operation such as
> algorithm,  window size and checksum.
> 
> +The allocation of the ``rte_comp_xform`` struct passed on the API is in
> +the application domain, so to allow future API extensions in a
> +backwardly compatible manner the application should zero this struct,
> +e.g. using sizeof(), before populating it. This allows the addition of new
> parameters to the struct with default value of zero indicating original
> behaviour.
> +
[Shally] Though summary and description looks fine. Only thing to confirm is that this description is applicable only on xform and config structure? And 0s to all field is allowed values.
I have not gone back to refer to latest config and xform structure so take it just a pointer to check on these cases.

Thanks
Shally
>  Compression API Hash support
>  ----------------------------
> 
> --
> 1.7.0.7
  
Fiona Trahe Aug. 28, 2019, 9:04 a.m. UTC | #2
HI Shally,

> -----Original Message-----
> From: Shally Verma [mailto:shallyv@marvell.com]
> Sent: Wednesday, August 28, 2019 5:37 AM
> To: Trahe, Fiona <fiona.trahe@intel.com>; dev@dpdk.org
> Cc: akhil.goyal@nxp.com; Ashish Gupta <ashishg@marvell.com>
> Subject: RE: [PATCH] doc/compressdev: clarify that structs should be zeroed before use
> 
> 
> 
> > -----Original Message-----
> > From: Fiona Trahe <fiona.trahe@intel.com>
> > Sent: Tuesday, August 27, 2019 10:56 PM
> > To: dev@dpdk.org
> > Cc: akhil.goyal@nxp.com; Ashish Gupta <ashishg@marvell.com>; Shally
> > Verma <shallyv@marvell.com>; Fiona Trahe <fiona.trahe@intel.com>
> > Subject: [PATCH] doc/compressdev: clarify that structs should be zeroed
> > before use
> >
> > Some structs used on the API are zeroed on creation by API calls, (e.g.
> > rte_comp_op), but a few are allocated in the application domain.
> > Clarify that the application should zero those to enable future extensions
> > without API breakage.
> >
> > Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
> > ---
> >  doc/guides/prog_guide/compressdev.rst |   10 ++++++++++
> >  1 files changed, 10 insertions(+), 0 deletions(-)
> >
> > diff --git a/doc/guides/prog_guide/compressdev.rst
> > b/doc/guides/prog_guide/compressdev.rst
> > index a089db1..2a85eba 100644
> > --- a/doc/guides/prog_guide/compressdev.rst
> > +++ b/doc/guides/prog_guide/compressdev.rst
> > @@ -76,6 +76,11 @@ The ``rte_compressdev_configure`` API is used to
> > configure a compression device.
> >  The ``rte_compressdev_config`` structure is used to pass the configuration
> > parameters.
> >
> > +The allocation of the ``rte_compressdev_config`` struct passed on the
> > +API is in the application domain, so to allow future API extensions in
> > +a backwardly compatible manner the application should zero this struct,
> > +e.g. using sizeof(), before populating it. This allows the addition of new
> > parameters to the struct with default value of zero indicating original
> > behaviour.
> > +
> >  See *DPDK API Reference* for details.
> >
> >  Configuration of Queue Pairs
> > @@ -264,6 +269,11 @@ Compression transforms (``rte_comp_xform``) are
> > the mechanism  to specify the details of the compression operation such as
> > algorithm,  window size and checksum.
> >
> > +The allocation of the ``rte_comp_xform`` struct passed on the API is in
> > +the application domain, so to allow future API extensions in a
> > +backwardly compatible manner the application should zero this struct,
> > +e.g. using sizeof(), before populating it. This allows the addition of new
> > parameters to the struct with default value of zero indicating original
> > behaviour.
> > +
> [Shally] Though summary and description looks fine. Only thing to confirm is that this description is
> applicable only on xform and config structure? And 0s to all field is allowed values.
> I have not gone back to refer to latest config and xform structure so take it just a pointer to check on
> these cases.
[Fiona] I looked through all API calls and these were the only one I could see. The op, private_xform and stream are all handed under the API. But would appreciate a review in case I've missed something.

> Thanks
> Shally
> >  Compression API Hash support
> >  ----------------------------
> >
> > --
> > 1.7.0.7
  
Fiona Trahe Aug. 28, 2019, 10:09 a.m. UTC | #3
Hi all,
 
Thanks to feedback from Harry, I've realised that I was only considering applications re-compiled
against the latest DPDK, but without any code rework to use the new feature.  
I hadn't considered pre-compiled applications running against shared libs.
This solution is not sufficient to handle those cases. 
There are many ways an API may change, this was just one small attempt to
minimise the impact of possible future changes.
So will nack this - we'll just deal with them through the usual
deprecation notices, wrappers, etc if and when they arise.

> > > -----Original Message-----
> > > From: Fiona Trahe <fiona.trahe@intel.com>
> > > Sent: Tuesday, August 27, 2019 10:56 PM
> > > To: dev@dpdk.org
> > > Cc: akhil.goyal@nxp.com; Ashish Gupta <ashishg@marvell.com>; Shally
> > > Verma <shallyv@marvell.com>; Fiona Trahe <fiona.trahe@intel.com>
> > > Subject: [PATCH] doc/compressdev: clarify that structs should be zeroed
> > > before use
> > >
> > > Some structs used on the API are zeroed on creation by API calls, (e.g.
> > > rte_comp_op), but a few are allocated in the application domain.
> > > Clarify that the application should zero those to enable future extensions
> > > without API breakage.
> > >
> > > Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
> > > ---
> > > 
 doc/guides/prog_guide/compressdev.rst |   10 ++++++++++
> > >  1 files changed, 10 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/doc/guides/prog_guide/compressdev.rst
> > > b/doc/guides/prog_guide/compressdev.rst
> > > index a089db1..2a85eba 100644
> > > --- a/doc/guides/prog_guide/compressdev.rst
> > > +++ b/doc/guides/prog_guide/compressdev.rst
> > > @@ -76,6 +76,11 @@ The ``rte_compressdev_configure`` API is used to
> > > configure a compression device.
> > >  The ``rte_compressdev_config`` structure is used to pass the configuration
> > > parameters.
> > >
> > > +The allocation of the ``rte_compressdev_config`` struct passed on the
> > > +API is in the application domain, so to allow future API extensions in
> > > +a backwardly compatible manner the application should zero this struct,
> > > +e.g. using sizeof(), before populating it. This allows the addition of new
> > > parameters to the struct with default value of zero indicating original
> > > behaviour.
> > > +
> > >  See *DPDK API Reference* for details.
> > >
> > >  Configuration of Queue Pairs
> > > @@ -264,6 +269,11 @@ Compression transforms (``rte_comp_xform``) are
> > > the mechanism  to specify the details of the compression operation such as
> > > algorithm,  window size and checksum.
> > >
> > > +The allocation of the ``rte_comp_xform`` struct passed on the API is in
> > > +the application domain, so to allow future API extensions in a
> > > +backwardly compatible manner the application should zero this struct,
> > > +e.g. using sizeof(), before populating it. This allows the addition of new
> > > parameters to the struct with default value of zero indicating original
> > > behaviour.
> > > +
> > [Shally] Though summary and description looks fine. Only thing to confirm is that this description is
> > applicable only on xform and config structure? And 0s to all field is allowed values.
> > I have not gone back to refer to latest config and xform structure so take it just a pointer to check on
> > these cases.
> [Fiona] I looked through all API calls and these were the only one I could see. The op, private_xform and
> stream are all handed under the API. But would appreciate a review in case I've missed something.

Nack
  

Patch

diff --git a/doc/guides/prog_guide/compressdev.rst b/doc/guides/prog_guide/compressdev.rst
index a089db1..2a85eba 100644
--- a/doc/guides/prog_guide/compressdev.rst
+++ b/doc/guides/prog_guide/compressdev.rst
@@ -76,6 +76,11 @@  The ``rte_compressdev_configure`` API is used to configure a compression device.
 The ``rte_compressdev_config`` structure is used to pass the configuration
 parameters.
 
+The allocation of the ``rte_compressdev_config`` struct passed on the API is in the application
+domain, so to allow future API extensions in a backwardly compatible manner the application
+should zero this struct, e.g. using sizeof(), before populating it. This allows the addition
+of new parameters to the struct with default value of zero indicating original behaviour.
+
 See *DPDK API Reference* for details.
 
 Configuration of Queue Pairs
@@ -264,6 +269,11 @@  Compression transforms (``rte_comp_xform``) are the mechanism
 to specify the details of the compression operation such as algorithm,
 window size and checksum.
 
+The allocation of the ``rte_comp_xform`` struct passed on the API is in the application
+domain, so to allow future API extensions in a backwardly compatible manner the application
+should zero this struct, e.g. using sizeof(), before populating it. This allows the addition
+of new parameters to the struct with default value of zero indicating original behaviour.
+
 Compression API Hash support
 ----------------------------