diff mbox series

[v3,1/6] common/mlx5: consider local functions as internal

Message ID 20220224232511.3238707-2-michaelba@nvidia.com (mailing list archive)
State Accepted, archived
Delegated to: Raslan Darawsheh
Headers
Series mlx5: external RxQ support |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-testing warning apply patch failure

Commit Message

Michael Baum Feb. 24, 2022, 11:25 p.m. UTC 
The functions which are not explicitly marked as internal
were exported because the local catch-all rule was missing in the
version script.
After adding the missing rule, all local functions are hidden.
The function mlx5_get_device_guid is used in another library,
so it needs to be exported (as internal).

Because the local functions were exported as non-internal
in DPDK 21.11, any change in these functions would break the ABI.
An ABI exception is added for this library, considering that all
functions are either local or internal.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 devtools/libabigail.abignore               | 4 ++++
 drivers/common/mlx5/linux/mlx5_common_os.h | 1 +
 drivers/common/mlx5/version.map            | 3 +++
 3 files changed, 8 insertions(+)

Comments

Ferruh Yigit Feb. 25, 2022, 6:01 p.m. UTC | #1 
On 2/24/2022 11:25 PM, Michael Baum wrote:
> The functions which are not explicitly marked as internal
> were exported because the local catch-all rule was missing in the
> version script.
> After adding the missing rule, all local functions are hidden.
> The function mlx5_get_device_guid is used in another library,
> so it needs to be exported (as internal).
> 
> Because the local functions were exported as non-internal
> in DPDK 21.11, any change in these functions would break the ABI.
> An ABI exception is added for this library, considering that all
> functions are either local or internal.
> 

When a function is not listed explicitly in .map file, it shouldn't
be exported at all.

So I am not sure if this exception is required, did you get
warning for tool, or is this theoretical?

cc'ed David and Ray for comment.

> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> Acked-by: Matan Azrad <matan@nvidia.com>


<...>
Thomas Monjalon Feb. 25, 2022, 6:38 p.m. UTC | #2 
25/02/2022 19:01, Ferruh Yigit:
> On 2/24/2022 11:25 PM, Michael Baum wrote:
> > The functions which are not explicitly marked as internal
> > were exported because the local catch-all rule was missing in the
> > version script.
> > After adding the missing rule, all local functions are hidden.
> > The function mlx5_get_device_guid is used in another library,
> > so it needs to be exported (as internal).
> > 
> > Because the local functions were exported as non-internal
> > in DPDK 21.11, any change in these functions would break the ABI.
> > An ABI exception is added for this library, considering that all
> > functions are either local or internal.
> > 
> 
> When a function is not listed explicitly in .map file, it shouldn't
> be exported at all.

It seems we need local:* to achieve this behaviour.
Few other libs are missing it. I plan to send a patch for them.

> So I am not sure if this exception is required, did you get
> warning for tool, or is this theoretical?

It is not theoritical, you can check with objdump:
objdump -T build/lib/librte_common_mlx5.so | sed -rn 's,^[[:xdigit:]]* g *(D[^0]*)[^ ]* *,\1,p'

I did not check the ABI tool without the exception.
Ferruh Yigit Feb. 25, 2022, 7:13 p.m. UTC | #3 
On 2/25/2022 6:38 PM, Thomas Monjalon wrote:
> 25/02/2022 19:01, Ferruh Yigit:
>> On 2/24/2022 11:25 PM, Michael Baum wrote:
>>> The functions which are not explicitly marked as internal
>>> were exported because the local catch-all rule was missing in the
>>> version script.
>>> After adding the missing rule, all local functions are hidden.
>>> The function mlx5_get_device_guid is used in another library,
>>> so it needs to be exported (as internal).
>>>
>>> Because the local functions were exported as non-internal
>>> in DPDK 21.11, any change in these functions would break the ABI.
>>> An ABI exception is added for this library, considering that all
>>> functions are either local or internal.
>>>
>>
>> When a function is not listed explicitly in .map file, it shouldn't
>> be exported at all.
> 
> It seems we need local:* to achieve this behaviour.
> Few other libs are missing it. I plan to send a patch for them.
> 

+1 for this patch, thanks.

>> So I am not sure if this exception is required, did you get
>> warning for tool, or is this theoretical?
> 
> It is not theoritical, you can check with objdump:
> objdump -T build/lib/librte_common_mlx5.so | sed -rn 's,^[[:xdigit:]]* g *(D[^0]*)[^ ]* *,\1,p'
> 
> I did not check the ABI tool without the exception.
> 

Yes tool complains with change [1], I will proceed with original patch.

[1]
29 Removed functions:

   [D] 'function int mlx5_auxiliary_get_pci_str(const rte_auxiliary_device*, char*, size_t)'    {mlx5_auxiliary_get_pci_str}
   [D] 'function void mlx5_common_auxiliary_init()'    {mlx5_common_auxiliary_init}
   [D] 'function int mlx5_common_dev_dma_map(rte_device*, void*, uint64_t, size_t)'    {mlx5_common_dev_dma_map}
   [D] 'function int mlx5_common_dev_dma_unmap(rte_device*, void*, uint64_t, size_t)'    {mlx5_common_dev_dma_unmap}
   [D] 'function int mlx5_common_dev_probe(rte_device*)'    {mlx5_common_dev_probe}
   [D] 'function int mlx5_common_dev_remove(rte_device*)'    {mlx5_common_dev_remove}
   [D] 'function void mlx5_common_driver_on_register_pci(mlx5_class_driver*)'    {mlx5_common_driver_on_register_pci}
   [D] 'function void mlx5_common_pci_init()'    {mlx5_common_pci_init}
   [D] 'function mlx5_mr* mlx5_create_mr_ext(void*, uintptr_t, size_t, int, mlx5_reg_mr_t)'    {mlx5_create_mr_ext}
   [D] 'function bool mlx5_dev_pci_match(const mlx5_class_driver*, const rte_device*)'    {mlx5_dev_pci_match}
   [D] 'function int mlx5_dev_to_pci_str(const rte_device*, char*, size_t)'    {mlx5_dev_to_pci_str}
   [D] 'function void mlx5_free_mr_by_addr(mlx5_mr_share_cache*, const char*, void*, size_t)'    {mlx5_free_mr_by_addr}
   [D] 'function ibv_device* mlx5_get_aux_ibv_device(const rte_auxiliary_device*)'    {mlx5_get_aux_ibv_device}
   [D] 'function void mlx5_glue_constructor()'    {mlx5_glue_constructor}
   [D] 'function void mlx5_malloc_mem_select(uint32_t)'    {mlx5_malloc_mem_select}
   [D] 'function void mlx5_mr_btree_dump(mlx5_mr_btree*)'    {mlx5_mr_btree_dump}
   [D] 'function int mlx5_mr_create_cache(mlx5_mr_share_cache*, int)'    {mlx5_mr_create_cache}
   [D] 'function void mlx5_mr_free(mlx5_mr*, mlx5_dereg_mr_t)'    {mlx5_mr_free}
   [D] 'function int mlx5_mr_insert_cache(mlx5_mr_share_cache*, mlx5_mr*)'    {mlx5_mr_insert_cache}
   [D] 'function mlx5_mr* mlx5_mr_lookup_list(mlx5_mr_share_cache*, mr_cache_entry*, uintptr_t)'    {mlx5_mr_lookup_list}
   [D] 'function void mlx5_mr_rebuild_cache(mlx5_mr_share_cache*)'    {mlx5_mr_rebuild_cache}
   [D] 'function void mlx5_mr_release_cache(mlx5_mr_share_cache*)'    {mlx5_mr_release_cache}
   [D] 'function int mlx5_nl_devlink_family_id_get(int)'    {mlx5_nl_devlink_family_id_get}
   [D] 'function int mlx5_nl_enable_roce_get(int, int, const char*, int*)'    {mlx5_nl_enable_roce_get}
   [D] 'function int mlx5_nl_enable_roce_set(int, int, const char*, int)'    {mlx5_nl_enable_roce_set}
   [D] 'function int mlx5_os_open_device(mlx5_common_device*, uint32_t)'    {mlx5_os_open_device}
   [D] 'function int mlx5_os_pd_create(mlx5_common_device*)'    {mlx5_os_pd_create}
   [D] 'function void mlx5_os_set_reg_mr_cb(mlx5_reg_mr_t*, mlx5_dereg_mr_t*)'    {mlx5_os_set_reg_mr_cb}
   [D] 'function void mlx5_set_context_attr(rte_device*, ibv_context*)'    {mlx5_set_context_attr}

2 Removed variables:

   [D] 'uint32_t atomic_sn'    {atomic_sn}
   [D] 'int mlx5_common_logtype'    {mlx5_common_logtype}

1 Removed function symbol not referenced by debug info:

   [D] mlx5_mr_dump_cache
diff mbox series

Patch

diff --git a/devtools/libabigail.abignore b/devtools/libabigail.abignore
index ef0602975a..78d57497e6 100644
--- a/devtools/libabigail.abignore
+++ b/devtools/libabigail.abignore
@@ -20,3 +20,7 @@ 
 ; Ignore changes to rte_crypto_asym_op, asymmetric crypto API is experimental
 [suppress_type]
         name = rte_crypto_asym_op
+
+; Ignore changes in common mlx5 driver, should be all internal
+[suppress_file]
+        soname_regexp = ^librte_common_mlx5\.
\ No newline at end of file
diff --git a/drivers/common/mlx5/linux/mlx5_common_os.h b/drivers/common/mlx5/linux/mlx5_common_os.h
index 83066e752d..edf356a30a 100644
--- a/drivers/common/mlx5/linux/mlx5_common_os.h
+++ b/drivers/common/mlx5/linux/mlx5_common_os.h
@@ -300,6 +300,7 @@  mlx5_set_context_attr(struct rte_device *dev, struct ibv_context *ctx);
  *  0 if OFED doesn't support.
  *  >0 if success.
  */
+__rte_internal
 int
 mlx5_get_device_guid(const struct rte_pci_addr *dev, uint8_t *guid, size_t len);
 
diff --git a/drivers/common/mlx5/version.map b/drivers/common/mlx5/version.map
index 1c6153c576..cb20a7d893 100644
--- a/drivers/common/mlx5/version.map
+++ b/drivers/common/mlx5/version.map
@@ -80,6 +80,7 @@  INTERNAL {
 
 	mlx5_free;
 
+	mlx5_get_device_guid; # WINDOWS_NO_EXPORT
 	mlx5_get_ifname_sysfs; # WINDOWS_NO_EXPORT
 	mlx5_get_pci_addr; # WINDOWS_NO_EXPORT
 
@@ -149,4 +150,6 @@  INTERNAL {
 	mlx5_mp_req_mempool_reg;
 	mlx5_mr_mempool2mr_bh;
 	mlx5_mr_mempool_populate_cache;
+
+	local: *;
 };