diff mbox series

bbdev: add missing experimental tags

Message ID 1543957065-20990-1-git-send-email-david.marchand@redhat.com (mailing list archive)
State Changes Requested, archived
Delegated to: akhil goyal
Headers
Series bbdev: add missing experimental tags |

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS

Commit Message

David Marchand Dec. 4, 2018, 8:57 p.m. UTC 
  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(-)

Comments

Neil Horman Dec. 5, 2018, 12:23 p.m. UTC | #1 
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>
Mokhtar, Amr Dec. 5, 2018, 4:46 p.m. UTC | #2 
> -----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 Dec. 14, 2018, 9:54 a.m. UTC | #3 
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. UTC | #4 
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. UTC | #5 
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
Mokhtar, Amr Dec. 14, 2018, 12:52 p.m. UTC | #6 
> -----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. UTC | #7 
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 Dec. 18, 2018, 10:43 a.m. UTC | #8 
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. UTC | #9 
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.
Mokhtar, Amr Dec. 19, 2018, 9:20 a.m. UTC | #10 
From: David Marchand [mailto:david.marchand@redhat.com]
Sent: Tuesday 18 December 2018 10:49
To: Akhil Goyal <akhil.goyal@nxp.com>; Mokhtar, Amr <amr.mokhtar@intel.com>
Cc: Neil Horman <nhorman@tuxdriver.com>; dev@dpdk.org; stable@dpdk.org; tredaelli@redhat.com; Yigit, Ferruh <ferruh.yigit@intel.com>
Subject: Re: [dpdk-dev] [PATCH] bbdev: add missing experimental tags


On Tue, Dec 18, 2018 at 11:43 AM Akhil Goyal <akhil.goyal@nxp.com<mailto: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 ?

Agree. I am updating the other patch with all updates.

Thanks.

--
David Marchand
diff mbox series

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