Patchwork [dpdk-dev] bbdev: add missing experimental tags

login
register
mail settings
Submitter David Marchand
Date Dec. 4, 2018, 8:57 p.m.
Message ID <1543957065-20990-1-git-send-email-david.marchand@redhat.com>
Download mbox | patch
Permalink /patch/672407/
State New
Headers show

Comments

David Marchand - Dec. 4, 2018, 8:57 p.m.
Those two symbols are missing the experimental tag in the library
header.
Because of this, a user can try to call this symbol without being aware
this is an experimental api (neither compilation nor link warning).

Fixes: 4935e1e9f76e ("bbdev: introduce wireless base band device lib")
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/librte_bbdev/rte_bbdev_op.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Neil Horman - Dec. 5, 2018, 12:23 p.m.
On Tue, Dec 04, 2018 at 09:57:45PM +0100, David Marchand wrote:
> Those two symbols are missing the experimental tag in the library
> header.
> Because of this, a user can try to call this symbol without being aware
> this is an experimental api (neither compilation nor link warning).
> 
> Fixes: 4935e1e9f76e ("bbdev: introduce wireless base band device lib")
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  lib/librte_bbdev/rte_bbdev_op.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_bbdev/rte_bbdev_op.h b/lib/librte_bbdev/rte_bbdev_op.h
> index 83f62c2..c9200b5 100644
> --- a/lib/librte_bbdev/rte_bbdev_op.h
> +++ b/lib/librte_bbdev/rte_bbdev_op.h
> @@ -459,7 +459,7 @@ struct rte_bbdev_op_pool_private {
>   *   Operation type as string or NULL if op_type is invalid
>   *
>   */
> -const char*
> +__rte_experimental const char *
>  rte_bbdev_op_type_str(enum rte_bbdev_op_type op_type);
>  
>  /**
> @@ -482,7 +482,7 @@ struct rte_bbdev_op_pool_private {
>   *   - Pointer to a mempool on success,
>   *   - NULL pointer on failure.
>   */
> -struct rte_mempool *
> +__rte_experimental struct rte_mempool *
>  rte_bbdev_op_pool_create(const char *name, enum rte_bbdev_op_type type,
>  		unsigned int num_elements, unsigned int cache_size,
>  		int socket_id);
> -- 
> 1.8.3.1
> 
> 
Acked-by: Neil Horman <nhorman@tuxdriver.com>
Amr Mokhtar - Dec. 5, 2018, 4:46 p.m.
> -----Original Message-----
> From: David Marchand [mailto:david.marchand@redhat.com]
> Sent: Tuesday 4 December 2018 20:58
> To: dev@dpdk.org
> Cc: stable@dpdk.org; nhorman@tuxdriver.com; tredaelli@redhat.com;
> Yigit, Ferruh <ferruh.yigit@intel.com>; Mokhtar, Amr
> <amr.mokhtar@intel.com>
> Subject: [PATCH] bbdev: add missing experimental tags
> 
> Those two symbols are missing the experimental tag in the library
> header.
> Because of this, a user can try to call this symbol without being aware
> this is an experimental api (neither compilation nor link warning).
> 
> Fixes: 4935e1e9f76e ("bbdev: introduce wireless base band device lib")
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

Acked-by: Amr Mokhtar <amr.mokhtar@intel.com>
akhil.goyal@nxp.com - Dec. 14, 2018, 9:54 a.m.
Hi David,

On 12/5/2018 2:27 AM, David Marchand wrote:
> Those two symbols are missing the experimental tag in the library

> header.

> Because of this, a user can try to call this symbol without being aware

> this is an experimental api (neither compilation nor link warning).

>

> Fixes: 4935e1e9f76e ("bbdev: introduce wireless base band device lib")

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

> ---

>   lib/librte_bbdev/rte_bbdev_op.h | 4 ++--

>   1 file changed, 2 insertions(+), 2 deletions(-)

>

> diff --git a/lib/librte_bbdev/rte_bbdev_op.h b/lib/librte_bbdev/rte_bbdev_op.h

> index 83f62c2..c9200b5 100644

> --- a/lib/librte_bbdev/rte_bbdev_op.h

> +++ b/lib/librte_bbdev/rte_bbdev_op.h

> @@ -459,7 +459,7 @@ struct rte_bbdev_op_pool_private {

>    *   Operation type as string or NULL if op_type is invalid

>    *

>    */

> -const char*

> +__rte_experimental const char *

>   rte_bbdev_op_type_str(enum rte_bbdev_op_type op_type);

>   

>   /**

> @@ -482,7 +482,7 @@ struct rte_bbdev_op_pool_private {

>    *   - Pointer to a mempool on success,

>    *   - NULL pointer on failure.

>    */

> -struct rte_mempool *

> +__rte_experimental struct rte_mempool *

>   rte_bbdev_op_pool_create(const char *name, enum rte_bbdev_op_type type,

>   		unsigned int num_elements, unsigned int cache_size,

>   		int socket_id);

I can see that there are other APIs as well which are not marked as 
experimental like rte_bbdev_dec_op_alloc_bulk
rte_bbdev_dec_op_free_bulk, rte_bbdev_enqueue_enc_ops etc.

-Akhil
David Marchand - Dec. 14, 2018, 10 a.m.
On Fri, Dec 14, 2018 at 10:54 AM Akhil Goyal <akhil.goyal@nxp.com> wrote:

> Hi David,
>
> I can see that there are other APIs as well which are not marked as
> experimental like rte_bbdev_dec_op_alloc_bulk
> rte_bbdev_dec_op_free_bulk, rte_bbdev_enqueue_enc_ops etc.
>

Well, that's the problem with inlines...
I don't think we can detect these easily.
Neil Horman - Dec. 14, 2018, 12:35 p.m.
On Fri, Dec 14, 2018 at 11:00:17AM +0100, David Marchand wrote:
> On Fri, Dec 14, 2018 at 10:54 AM Akhil Goyal <akhil.goyal@nxp.com> wrote:
> 
> > Hi David,
> >
> > I can see that there are other APIs as well which are not marked as
> > experimental like rte_bbdev_dec_op_alloc_bulk
> > rte_bbdev_dec_op_free_bulk, rte_bbdev_enqueue_enc_ops etc.
> >
> 
> Well, that's the problem with inlines...
> I don't think we can detect these easily.
> 
We can, if the symbols get added to the version map as they should.  But (as you
note), because these functions are inlines, theres no error thrown for not
following that rule.

> 
> -- 
> David Marchand
Amr Mokhtar - Dec. 14, 2018, 12:52 p.m.
> -----Original Message-----
> From: Neil Horman [mailto:nhorman@tuxdriver.com]
> Sent: Friday 14 December 2018 12:36
> To: David Marchand <david.marchand@redhat.com>
> Cc: Akhil Goyal <akhil.goyal@nxp.com>; dev@dpdk.org; stable@dpdk.org;
> tredaelli@redhat.com; Yigit, Ferruh <ferruh.yigit@intel.com>; Mokhtar,
> Amr <amr.mokhtar@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] bbdev: add missing experimental tags
> 
> On Fri, Dec 14, 2018 at 11:00:17AM +0100, David Marchand wrote:
> > On Fri, Dec 14, 2018 at 10:54 AM Akhil Goyal <akhil.goyal@nxp.com>
> wrote:
> >
> > > Hi David,
> > >
> > > I can see that there are other APIs as well which are not marked as
> > > experimental like rte_bbdev_dec_op_alloc_bulk
> > > rte_bbdev_dec_op_free_bulk, rte_bbdev_enqueue_enc_ops etc.
> > >

It seems that all APIs defined in rte_bbdev_op.h are missing their
experimental tags.

> >
> > Well, that's the problem with inlines...
> > I don't think we can detect these easily.
> >
> We can, if the symbols get added to the version map as they should.  But
> (as you
> note), because these functions are inlines, theres no error thrown for not
> following that rule.
> 

Right, there are some APIs missing in the map file.
I am submitting a patch of those functions missing.

> >
> > --
> > David Marchand
David Marchand - Dec. 18, 2018, 10:37 a.m.
Hello Akhil,

On Fri, Dec 14, 2018 at 1:52 PM Mokhtar, Amr <amr.mokhtar@intel.com> wrote:

>
> > -----Original Message-----
> > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > Sent: Friday 14 December 2018 12:36
> > To: David Marchand <david.marchand@redhat.com>
> > Cc: Akhil Goyal <akhil.goyal@nxp.com>; dev@dpdk.org; stable@dpdk.org;
> > tredaelli@redhat.com; Yigit, Ferruh <ferruh.yigit@intel.com>; Mokhtar,
> > Amr <amr.mokhtar@intel.com>
> > Subject: Re: [dpdk-dev] [PATCH] bbdev: add missing experimental tags
> >
> > On Fri, Dec 14, 2018 at 11:00:17AM +0100, David Marchand wrote:
> > > On Fri, Dec 14, 2018 at 10:54 AM Akhil Goyal <akhil.goyal@nxp.com>
> > wrote:
> > >
> > > > Hi David,
> > > >
> > > > I can see that there are other APIs as well which are not marked as
> > > > experimental like rte_bbdev_dec_op_alloc_bulk
> > > > rte_bbdev_dec_op_free_bulk, rte_bbdev_enqueue_enc_ops etc.
> > > >
>
> It seems that all APIs defined in rte_bbdev_op.h are missing their
> experimental tags.
>
> > >
> > > Well, that's the problem with inlines...
> > > I don't think we can detect these easily.
> > >
> > We can, if the symbols get added to the version map as they should.  But
> > (as you
> > note), because these functions are inlines, theres no error thrown for
> not
> > following that rule.
> >
>
> Right, there are some APIs missing in the map file.
> I am submitting a patch of those functions missing.
>

Sorry, did not follow up on this.
My patch has been marked as "Changes Requested" in pw but Amr volunteered
to fix those issues.

How do we proceed, do you want a single fix ? or you can already pick mine.
akhil.goyal@nxp.com - Dec. 18, 2018, 10:43 a.m.
Hi David,

On 12/18/2018 4:07 PM, David Marchand wrote:
> Hello Akhil,

>

> On Fri, Dec 14, 2018 at 1:52 PM Mokhtar, Amr <amr.mokhtar@intel.com> wrote:

>

>>> -----Original Message-----

>>> From: Neil Horman [mailto:nhorman@tuxdriver.com]

>>> Sent: Friday 14 December 2018 12:36

>>> To: David Marchand <david.marchand@redhat.com>

>>> Cc: Akhil Goyal <akhil.goyal@nxp.com>; dev@dpdk.org; stable@dpdk.org;

>>> tredaelli@redhat.com; Yigit, Ferruh <ferruh.yigit@intel.com>; Mokhtar,

>>> Amr <amr.mokhtar@intel.com>

>>> Subject: Re: [dpdk-dev] [PATCH] bbdev: add missing experimental tags

>>>

>>> On Fri, Dec 14, 2018 at 11:00:17AM +0100, David Marchand wrote:

>>>> On Fri, Dec 14, 2018 at 10:54 AM Akhil Goyal <akhil.goyal@nxp.com>

>>> wrote:

>>>>> Hi David,

>>>>>

>>>>> I can see that there are other APIs as well which are not marked as

>>>>> experimental like rte_bbdev_dec_op_alloc_bulk

>>>>> rte_bbdev_dec_op_free_bulk, rte_bbdev_enqueue_enc_ops etc.

>>>>>

>> It seems that all APIs defined in rte_bbdev_op.h are missing their

>> experimental tags.

>>

>>>> Well, that's the problem with inlines...

>>>> I don't think we can detect these easily.

>>>>

>>> We can, if the symbols get added to the version map as they should.  But

>>> (as you

>>> note), because these functions are inlines, theres no error thrown for

>> not

>>> following that rule.

>>>

>> Right, there are some APIs missing in the map file.

>> I am submitting a patch of those functions missing.

>>

> Sorry, did not follow up on this.

> My patch has been marked as "Changes Requested" in pw but Amr volunteered

> to fix those issues.

>

> How do we proceed, do you want a single fix ? or you can already pick mine.

>

>

I believe a single patch should be enough to fix this, it can be either 
you or Amr. As of now, none of the patch is complete.

Thanks,
Akhil
David Marchand - Dec. 18, 2018, 10:48 a.m.
On Tue, Dec 18, 2018 at 11:43 AM Akhil Goyal <akhil.goyal@nxp.com> wrote:

> Hi David,
>
> On 12/18/2018 4:07 PM, David Marchand wrote:
> > Sorry, did not follow up on this.
> > My patch has been marked as "Changes Requested" in pw but Amr volunteered
> > to fix those issues.
> >
> > How do we proceed, do you want a single fix ? or you can already pick
> mine.
> >
> >
> I believe a single patch should be enough to fix this, it can be either
> you or Amr. As of now, none of the patch is complete.
>

Amr, you seem to have been a bit further than me.
Can you squash my bits in yours ?

Thanks.

Patch

diff --git a/lib/librte_bbdev/rte_bbdev_op.h b/lib/librte_bbdev/rte_bbdev_op.h
index 83f62c2..c9200b5 100644
--- a/lib/librte_bbdev/rte_bbdev_op.h
+++ b/lib/librte_bbdev/rte_bbdev_op.h
@@ -459,7 +459,7 @@  struct rte_bbdev_op_pool_private {
  *   Operation type as string or NULL if op_type is invalid
  *
  */
-const char*
+__rte_experimental const char *
 rte_bbdev_op_type_str(enum rte_bbdev_op_type op_type);
 
 /**
@@ -482,7 +482,7 @@  struct rte_bbdev_op_pool_private {
  *   - Pointer to a mempool on success,
  *   - NULL pointer on failure.
  */
-struct rte_mempool *
+__rte_experimental struct rte_mempool *
 rte_bbdev_op_pool_create(const char *name, enum rte_bbdev_op_type type,
 		unsigned int num_elements, unsigned int cache_size,
 		int socket_id);