[v8,04/13] mempool/dpaa2: move internal symbols into INTERNAL section

Message ID 20200515094752.28490-5-hemant.agrawal@nxp.com (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series NXP DPAAx: move internal symbols to INTERNAL |

Checks

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

Commit Message

Hemant Agrawal May 15, 2020, 9:47 a.m. UTC
  This patch moves the internal symbols to INTERNAL sections
so that any change in them is not reported as ABI breakage.

Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
 devtools/libabigail.abignore                        | 8 ++++++++
 drivers/mempool/dpaa/rte_mempool_dpaa_version.map   | 6 ++++--
 drivers/mempool/dpaa2/dpaa2_hw_mempool.h            | 1 +
 drivers/mempool/dpaa2/rte_mempool_dpaa2_version.map | 9 +++++++--
 4 files changed, 20 insertions(+), 4 deletions(-)
  

Comments

Ray Kinsella May 19, 2020, 11:03 a.m. UTC | #1
On 15/05/2020 10:47, Hemant Agrawal wrote:
> This patch moves the internal symbols to INTERNAL sections
> so that any change in them is not reported as ABI breakage.
> 
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> ---
>  devtools/libabigail.abignore                        | 8 ++++++++
>  drivers/mempool/dpaa/rte_mempool_dpaa_version.map   | 6 ++++--
>  drivers/mempool/dpaa2/dpaa2_hw_mempool.h            | 1 +
>  drivers/mempool/dpaa2/rte_mempool_dpaa2_version.map | 9 +++++++--
>  4 files changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/devtools/libabigail.abignore b/devtools/libabigail.abignore
> index ab34302d0c..42f9469221 100644
> --- a/devtools/libabigail.abignore
> +++ b/devtools/libabigail.abignore
> @@ -55,3 +55,11 @@
>  	file_name_regexp = ^librte_bus_fslmc\.
>  [suppress_file]
>  	file_name_regexp = ^librte_bus_dpaa\.
> +[suppress_function]
> +	name = rte_dpaa2_mbuf_alloc_bulk
> +[suppress_variable]
> +	name_regexp = ^rte_dpaa_memsegs
> +[suppress_variable]
> +	name_regexp = ^rte_dpaa_bpid_info
> +[suppress_variable]
> +	name_regexp = ^rte_dpaa2_bpid_info

Is there a specific reason you are using name_regexp here.
There is only a single variable involved in each case - would "name" not work equally as well?

> diff --git a/drivers/mempool/dpaa/rte_mempool_dpaa_version.map b/drivers/mempool/dpaa/rte_mempool_dpaa_version.map
> index 9eebaf7ffd..89d7cf4957 100644
> --- a/drivers/mempool/dpaa/rte_mempool_dpaa_version.map
> +++ b/drivers/mempool/dpaa/rte_mempool_dpaa_version.map
> @@ -1,8 +1,10 @@
>  DPDK_20.0 {
> +	local: *;
> +};
> +
> +INTERNAL {
>  	global:
>  
>  	rte_dpaa_bpid_info;
>  	rte_dpaa_memsegs;
> -
> -	local: *;
>  };
> diff --git a/drivers/mempool/dpaa2/dpaa2_hw_mempool.h b/drivers/mempool/dpaa2/dpaa2_hw_mempool.h
> index fa0f2280d5..53fa1552d1 100644
> --- a/drivers/mempool/dpaa2/dpaa2_hw_mempool.h
> +++ b/drivers/mempool/dpaa2/dpaa2_hw_mempool.h
> @@ -61,6 +61,7 @@ struct dpaa2_bp_info {
>  
>  extern struct dpaa2_bp_info *rte_dpaa2_bpid_info;
>  
> +__rte_internal
>  int rte_dpaa2_mbuf_alloc_bulk(struct rte_mempool *pool,
>  		       void **obj_table, unsigned int count);
>  
> diff --git a/drivers/mempool/dpaa2/rte_mempool_dpaa2_version.map b/drivers/mempool/dpaa2/rte_mempool_dpaa2_version.map
> index cd4bc88273..686b024624 100644
> --- a/drivers/mempool/dpaa2/rte_mempool_dpaa2_version.map
> +++ b/drivers/mempool/dpaa2/rte_mempool_dpaa2_version.map
> @@ -1,10 +1,15 @@
>  DPDK_20.0 {
>  	global:
>  
> -	rte_dpaa2_bpid_info;
> -	rte_dpaa2_mbuf_alloc_bulk;
>  	rte_dpaa2_mbuf_from_buf_addr;
>  	rte_dpaa2_mbuf_pool_bpid;
>  
>  	local: *;
>  };
> +
> +INTERNAL {
> +	global:
> +
> +	rte_dpaa2_bpid_info;
> +	rte_dpaa2_mbuf_alloc_bulk;
> +};
>
  
Hemant Agrawal May 19, 2020, 11:16 a.m. UTC | #2
> On 15/05/2020 10:47, Hemant Agrawal wrote:
> > This patch moves the internal symbols to INTERNAL sections so that any
> > change in them is not reported as ABI breakage.
> >
> > Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> > ---
> >  devtools/libabigail.abignore                        | 8 ++++++++
> >  drivers/mempool/dpaa/rte_mempool_dpaa_version.map   | 6 ++++--
> >  drivers/mempool/dpaa2/dpaa2_hw_mempool.h            | 1 +
> >  drivers/mempool/dpaa2/rte_mempool_dpaa2_version.map | 9 +++++++--
> >  4 files changed, 20 insertions(+), 4 deletions(-)
> >
> > diff --git a/devtools/libabigail.abignore
> > b/devtools/libabigail.abignore index ab34302d0c..42f9469221 100644
> > --- a/devtools/libabigail.abignore
> > +++ b/devtools/libabigail.abignore
> > @@ -55,3 +55,11 @@
> >  	file_name_regexp = ^librte_bus_fslmc\.
> >  [suppress_file]
> >  	file_name_regexp = ^librte_bus_dpaa\.
> > +[suppress_function]
> > +	name = rte_dpaa2_mbuf_alloc_bulk
> > +[suppress_variable]
> > +	name_regexp = ^rte_dpaa_memsegs
> > +[suppress_variable]
> > +	name_regexp = ^rte_dpaa_bpid_info
> > +[suppress_variable]
> > +	name_regexp = ^rte_dpaa2_bpid_info
> 
> Is there a specific reason you are using name_regexp here.
> There is only a single variable involved in each case - would "name" not work
> equally as well?

[Hemant]  I remember getting some errors in case of variables. But now name is also working ok.
So, yes, name will also work in this case.
If I need to do a next version of this series, I will improve it. Is that ok for you?


> 
> > diff --git a/drivers/mempool/dpaa/rte_mempool_dpaa_version.map
> > b/drivers/mempool/dpaa/rte_mempool_dpaa_version.map
> > index 9eebaf7ffd..89d7cf4957 100644
> > --- a/drivers/mempool/dpaa/rte_mempool_dpaa_version.map
> > +++ b/drivers/mempool/dpaa/rte_mempool_dpaa_version.map
> > @@ -1,8 +1,10 @@
> >  DPDK_20.0 {
> > +	local: *;
> > +};
> > +
> > +INTERNAL {
> >  	global:
> >
> >  	rte_dpaa_bpid_info;
> >  	rte_dpaa_memsegs;
> > -
> > -	local: *;
> >  };
> > diff --git a/drivers/mempool/dpaa2/dpaa2_hw_mempool.h
> > b/drivers/mempool/dpaa2/dpaa2_hw_mempool.h
> > index fa0f2280d5..53fa1552d1 100644
> > --- a/drivers/mempool/dpaa2/dpaa2_hw_mempool.h
> > +++ b/drivers/mempool/dpaa2/dpaa2_hw_mempool.h
> > @@ -61,6 +61,7 @@ struct dpaa2_bp_info {
> >
> >  extern struct dpaa2_bp_info *rte_dpaa2_bpid_info;
> >
> > +__rte_internal
> >  int rte_dpaa2_mbuf_alloc_bulk(struct rte_mempool *pool,
> >  		       void **obj_table, unsigned int count);
> >
> > diff --git a/drivers/mempool/dpaa2/rte_mempool_dpaa2_version.map
> > b/drivers/mempool/dpaa2/rte_mempool_dpaa2_version.map
> > index cd4bc88273..686b024624 100644
> > --- a/drivers/mempool/dpaa2/rte_mempool_dpaa2_version.map
> > +++ b/drivers/mempool/dpaa2/rte_mempool_dpaa2_version.map
> > @@ -1,10 +1,15 @@
> >  DPDK_20.0 {
> >  	global:
> >
> > -	rte_dpaa2_bpid_info;
> > -	rte_dpaa2_mbuf_alloc_bulk;
> >  	rte_dpaa2_mbuf_from_buf_addr;
> >  	rte_dpaa2_mbuf_pool_bpid;
> >
> >  	local: *;
> >  };
> > +
> > +INTERNAL {
> > +	global:
> > +
> > +	rte_dpaa2_bpid_info;
> > +	rte_dpaa2_mbuf_alloc_bulk;
> > +};
> >
  
Ray Kinsella May 19, 2020, 11:30 a.m. UTC | #3
On 19/05/2020 12:16, Hemant Agrawal wrote:
>  
>> On 15/05/2020 10:47, Hemant Agrawal wrote:
>>> This patch moves the internal symbols to INTERNAL sections so that any
>>> change in them is not reported as ABI breakage.
>>>
>>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>>> ---
>>>  devtools/libabigail.abignore                        | 8 ++++++++
>>>  drivers/mempool/dpaa/rte_mempool_dpaa_version.map   | 6 ++++--
>>>  drivers/mempool/dpaa2/dpaa2_hw_mempool.h            | 1 +
>>>  drivers/mempool/dpaa2/rte_mempool_dpaa2_version.map | 9 +++++++--
>>>  4 files changed, 20 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/devtools/libabigail.abignore
>>> b/devtools/libabigail.abignore index ab34302d0c..42f9469221 100644
>>> --- a/devtools/libabigail.abignore
>>> +++ b/devtools/libabigail.abignore
>>> @@ -55,3 +55,11 @@
>>>  	file_name_regexp = ^librte_bus_fslmc\.
>>>  [suppress_file]
>>>  	file_name_regexp = ^librte_bus_dpaa\.
>>> +[suppress_function]
>>> +	name = rte_dpaa2_mbuf_alloc_bulk
>>> +[suppress_variable]
>>> +	name_regexp = ^rte_dpaa_memsegs
>>> +[suppress_variable]
>>> +	name_regexp = ^rte_dpaa_bpid_info
>>> +[suppress_variable]
>>> +	name_regexp = ^rte_dpaa2_bpid_info
>>
>> Is there a specific reason you are using name_regexp here.
>> There is only a single variable involved in each case - would "name" not work
>> equally as well?
> 
> [Hemant]  I remember getting some errors in case of variables. But now name is also working ok.
> So, yes, name will also work in this case.
> If I need to do a next version of this series, I will improve it. Is that ok for you?

yes - that is perfect, I will give one more look over then. 

> 
> 
>>
>>> diff --git a/drivers/mempool/dpaa/rte_mempool_dpaa_version.map
>>> b/drivers/mempool/dpaa/rte_mempool_dpaa_version.map
>>> index 9eebaf7ffd..89d7cf4957 100644
>>> --- a/drivers/mempool/dpaa/rte_mempool_dpaa_version.map
>>> +++ b/drivers/mempool/dpaa/rte_mempool_dpaa_version.map
>>> @@ -1,8 +1,10 @@
>>>  DPDK_20.0 {
>>> +	local: *;
>>> +};
>>> +
>>> +INTERNAL {
>>>  	global:
>>>
>>>  	rte_dpaa_bpid_info;
>>>  	rte_dpaa_memsegs;
>>> -
>>> -	local: *;
>>>  };
>>> diff --git a/drivers/mempool/dpaa2/dpaa2_hw_mempool.h
>>> b/drivers/mempool/dpaa2/dpaa2_hw_mempool.h
>>> index fa0f2280d5..53fa1552d1 100644
>>> --- a/drivers/mempool/dpaa2/dpaa2_hw_mempool.h
>>> +++ b/drivers/mempool/dpaa2/dpaa2_hw_mempool.h
>>> @@ -61,6 +61,7 @@ struct dpaa2_bp_info {
>>>
>>>  extern struct dpaa2_bp_info *rte_dpaa2_bpid_info;
>>>
>>> +__rte_internal
>>>  int rte_dpaa2_mbuf_alloc_bulk(struct rte_mempool *pool,
>>>  		       void **obj_table, unsigned int count);
>>>
>>> diff --git a/drivers/mempool/dpaa2/rte_mempool_dpaa2_version.map
>>> b/drivers/mempool/dpaa2/rte_mempool_dpaa2_version.map
>>> index cd4bc88273..686b024624 100644
>>> --- a/drivers/mempool/dpaa2/rte_mempool_dpaa2_version.map
>>> +++ b/drivers/mempool/dpaa2/rte_mempool_dpaa2_version.map
>>> @@ -1,10 +1,15 @@
>>>  DPDK_20.0 {
>>>  	global:
>>>
>>> -	rte_dpaa2_bpid_info;
>>> -	rte_dpaa2_mbuf_alloc_bulk;
>>>  	rte_dpaa2_mbuf_from_buf_addr;
>>>  	rte_dpaa2_mbuf_pool_bpid;
>>>
>>>  	local: *;
>>>  };
>>> +
>>> +INTERNAL {
>>> +	global:
>>> +
>>> +	rte_dpaa2_bpid_info;
>>> +	rte_dpaa2_mbuf_alloc_bulk;
>>> +};
>>>
  

Patch

diff --git a/devtools/libabigail.abignore b/devtools/libabigail.abignore
index ab34302d0c..42f9469221 100644
--- a/devtools/libabigail.abignore
+++ b/devtools/libabigail.abignore
@@ -55,3 +55,11 @@ 
 	file_name_regexp = ^librte_bus_fslmc\.
 [suppress_file]
 	file_name_regexp = ^librte_bus_dpaa\.
+[suppress_function]
+	name = rte_dpaa2_mbuf_alloc_bulk
+[suppress_variable]
+	name_regexp = ^rte_dpaa_memsegs
+[suppress_variable]
+	name_regexp = ^rte_dpaa_bpid_info
+[suppress_variable]
+	name_regexp = ^rte_dpaa2_bpid_info
diff --git a/drivers/mempool/dpaa/rte_mempool_dpaa_version.map b/drivers/mempool/dpaa/rte_mempool_dpaa_version.map
index 9eebaf7ffd..89d7cf4957 100644
--- a/drivers/mempool/dpaa/rte_mempool_dpaa_version.map
+++ b/drivers/mempool/dpaa/rte_mempool_dpaa_version.map
@@ -1,8 +1,10 @@ 
 DPDK_20.0 {
+	local: *;
+};
+
+INTERNAL {
 	global:
 
 	rte_dpaa_bpid_info;
 	rte_dpaa_memsegs;
-
-	local: *;
 };
diff --git a/drivers/mempool/dpaa2/dpaa2_hw_mempool.h b/drivers/mempool/dpaa2/dpaa2_hw_mempool.h
index fa0f2280d5..53fa1552d1 100644
--- a/drivers/mempool/dpaa2/dpaa2_hw_mempool.h
+++ b/drivers/mempool/dpaa2/dpaa2_hw_mempool.h
@@ -61,6 +61,7 @@  struct dpaa2_bp_info {
 
 extern struct dpaa2_bp_info *rte_dpaa2_bpid_info;
 
+__rte_internal
 int rte_dpaa2_mbuf_alloc_bulk(struct rte_mempool *pool,
 		       void **obj_table, unsigned int count);
 
diff --git a/drivers/mempool/dpaa2/rte_mempool_dpaa2_version.map b/drivers/mempool/dpaa2/rte_mempool_dpaa2_version.map
index cd4bc88273..686b024624 100644
--- a/drivers/mempool/dpaa2/rte_mempool_dpaa2_version.map
+++ b/drivers/mempool/dpaa2/rte_mempool_dpaa2_version.map
@@ -1,10 +1,15 @@ 
 DPDK_20.0 {
 	global:
 
-	rte_dpaa2_bpid_info;
-	rte_dpaa2_mbuf_alloc_bulk;
 	rte_dpaa2_mbuf_from_buf_addr;
 	rte_dpaa2_mbuf_pool_bpid;
 
 	local: *;
 };
+
+INTERNAL {
+	global:
+
+	rte_dpaa2_bpid_info;
+	rte_dpaa2_mbuf_alloc_bulk;
+};