DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] common/mlx5: fix missing __rte_internal tags in exported functions
@ 2021-04-11 12:21 Tal Shnaiderman
  2021-04-11 13:00 ` Matan Azrad
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Tal Shnaiderman @ 2021-04-11 12:21 UTC (permalink / raw)
  To: dev; +Cc: thomas, matan, rasland, asafp, wisamm, stable

Several functions introduced in the addition of the Windows support to
mlx5 were missing the __rte_internal tag although being exported.

Fixes: 1552fb287166 ("common/mlx5: add alloc/dealloc PD on Windows")
Fixes: 1969ee424405 ("common/mlx5: add UMEM reg/dereg functions on Windows")
Fixes: ba420719823c ("common/mlx5: add reg/dereg MR on Windows")
Cc: stable@dpdk.org

Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
---
 drivers/common/mlx5/linux/mlx5_common_os.h   | 4 ++++
 drivers/common/mlx5/windows/mlx5_common_os.h | 6 ++++++
 2 files changed, 10 insertions(+)

diff --git a/drivers/common/mlx5/linux/mlx5_common_os.h b/drivers/common/mlx5/linux/mlx5_common_os.h
index 63f070d9c4..d1c7e3dce0 100644
--- a/drivers/common/mlx5/linux/mlx5_common_os.h
+++ b/drivers/common/mlx5/linux/mlx5_common_os.h
@@ -203,24 +203,28 @@ mlx5_os_get_devx_uar_page_id(void *uar)
 #endif
 }
 
+__rte_internal
 static inline void *
 mlx5_os_alloc_pd(void *ctx)
 {
 	return mlx5_glue->alloc_pd(ctx);
 }
 
+__rte_internal
 static inline int
 mlx5_os_dealloc_pd(void *pd)
 {
 	return mlx5_glue->dealloc_pd(pd);
 }
 
+__rte_internal
 static inline void *
 mlx5_os_umem_reg(void *ctx, void *addr, size_t size, uint32_t access)
 {
 	return mlx5_glue->devx_umem_reg(ctx, addr, size, access);
 }
 
+__rte_internal
 static inline int
 mlx5_os_umem_dereg(void *pumem)
 {
diff --git a/drivers/common/mlx5/windows/mlx5_common_os.h b/drivers/common/mlx5/windows/mlx5_common_os.h
index e92533c4d3..3756e1959b 100644
--- a/drivers/common/mlx5/windows/mlx5_common_os.h
+++ b/drivers/common/mlx5/windows/mlx5_common_os.h
@@ -248,11 +248,17 @@ mlx5_os_devx_subscribe_devx_event(void *eventc,
 	return -ENOTSUP;
 }
 
+__rte_internal
 void *mlx5_os_alloc_pd(void *ctx);
+__rte_internal
 int mlx5_os_dealloc_pd(void *pd);
+__rte_internal
 void *mlx5_os_umem_reg(void *ctx, void *addr, size_t size, uint32_t access);
+__rte_internal
 int mlx5_os_umem_dereg(void *pumem);
+__rte_internal
 int mlx5_os_reg_mr(void *pd,
 		   void *addr, size_t length, struct mlx5_pmd_mr *pmd_mr);
+__rte_internal
 void mlx5_os_dereg_mr(struct mlx5_pmd_mr *pmd_mr);
 #endif /* RTE_PMD_MLX5_COMMON_OS_H_ */
-- 
2.16.1.windows.4


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH] common/mlx5: fix missing __rte_internal tags in exported functions
  2021-04-11 12:21 [dpdk-dev] [PATCH] common/mlx5: fix missing __rte_internal tags in exported functions Tal Shnaiderman
@ 2021-04-11 13:00 ` Matan Azrad
  2021-04-11 13:09 ` Wisam Monther
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 13+ messages in thread
From: Matan Azrad @ 2021-04-11 13:00 UTC (permalink / raw)
  To: Tal Shnaiderman, dev
  Cc: NBU-Contact-Thomas Monjalon, Raslan Darawsheh, Asaf Penso,
	Wisam Monther, stable



From: Tal Shnaiderman
> Several functions introduced in the addition of the Windows support to
> mlx5 were missing the __rte_internal tag although being exported.
> 
> Fixes: 1552fb287166 ("common/mlx5: add alloc/dealloc PD on Windows")
> Fixes: 1969ee424405 ("common/mlx5: add UMEM reg/dereg functions on
> Windows")
> Fixes: ba420719823c ("common/mlx5: add reg/dereg MR on Windows")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>


> ---
>  drivers/common/mlx5/linux/mlx5_common_os.h   | 4 ++++
>  drivers/common/mlx5/windows/mlx5_common_os.h | 6 ++++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/common/mlx5/linux/mlx5_common_os.h
> b/drivers/common/mlx5/linux/mlx5_common_os.h
> index 63f070d9c4..d1c7e3dce0 100644
> --- a/drivers/common/mlx5/linux/mlx5_common_os.h
> +++ b/drivers/common/mlx5/linux/mlx5_common_os.h
> @@ -203,24 +203,28 @@ mlx5_os_get_devx_uar_page_id(void *uar)  #endif  }
> 
> +__rte_internal
>  static inline void *
>  mlx5_os_alloc_pd(void *ctx)
>  {
>  	return mlx5_glue->alloc_pd(ctx);
>  }
> 
> +__rte_internal
>  static inline int
>  mlx5_os_dealloc_pd(void *pd)
>  {
>  	return mlx5_glue->dealloc_pd(pd);
>  }
> 
> +__rte_internal
>  static inline void *
>  mlx5_os_umem_reg(void *ctx, void *addr, size_t size, uint32_t access)  {
>  	return mlx5_glue->devx_umem_reg(ctx, addr, size, access);  }
> 
> +__rte_internal
>  static inline int
>  mlx5_os_umem_dereg(void *pumem)
>  {
> diff --git a/drivers/common/mlx5/windows/mlx5_common_os.h
> b/drivers/common/mlx5/windows/mlx5_common_os.h
> index e92533c4d3..3756e1959b 100644
> --- a/drivers/common/mlx5/windows/mlx5_common_os.h
> +++ b/drivers/common/mlx5/windows/mlx5_common_os.h
> @@ -248,11 +248,17 @@ mlx5_os_devx_subscribe_devx_event(void *eventc,
>  	return -ENOTSUP;
>  }
> 
> +__rte_internal
>  void *mlx5_os_alloc_pd(void *ctx);
> +__rte_internal
>  int mlx5_os_dealloc_pd(void *pd);
> +__rte_internal
>  void *mlx5_os_umem_reg(void *ctx, void *addr, size_t size, uint32_t access);
> +__rte_internal
>  int mlx5_os_umem_dereg(void *pumem);
> +__rte_internal
>  int mlx5_os_reg_mr(void *pd,
>  		   void *addr, size_t length, struct mlx5_pmd_mr *pmd_mr);
> +__rte_internal
>  void mlx5_os_dereg_mr(struct mlx5_pmd_mr *pmd_mr);  #endif /*
> RTE_PMD_MLX5_COMMON_OS_H_ */
> --
> 2.16.1.windows.4


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH] common/mlx5: fix missing __rte_internal tags in exported functions
  2021-04-11 12:21 [dpdk-dev] [PATCH] common/mlx5: fix missing __rte_internal tags in exported functions Tal Shnaiderman
  2021-04-11 13:00 ` Matan Azrad
@ 2021-04-11 13:09 ` Wisam Monther
  2021-04-12 11:25 ` Raslan Darawsheh
  2021-04-12 15:28 ` [dpdk-dev] " Thomas Monjalon
  3 siblings, 0 replies; 13+ messages in thread
From: Wisam Monther @ 2021-04-11 13:09 UTC (permalink / raw)
  To: Tal Shnaiderman, dev
  Cc: NBU-Contact-Thomas Monjalon, Matan Azrad, Raslan Darawsheh,
	Asaf Penso, stable

Hi,

> -----Original Message-----
> From: Tal Shnaiderman <talshn@nvidia.com>
> Sent: Sunday, April 11, 2021 3:22 PM
> To: dev@dpdk.org
> Cc: NBU-Contact-Thomas Monjalon <thomas@monjalon.net>; Matan Azrad
> <matan@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>; Asaf Penso
> <asafp@nvidia.com>; Wisam Monther <wisamm@nvidia.com>;
> stable@dpdk.org
> Subject: [PATCH] common/mlx5: fix missing __rte_internal tags in exported
> functions
> 
> Several functions introduced in the addition of the Windows support to
> mlx5 were missing the __rte_internal tag although being exported.
> 
> Fixes: 1552fb287166 ("common/mlx5: add alloc/dealloc PD on Windows")
> Fixes: 1969ee424405 ("common/mlx5: add UMEM reg/dereg functions on
> Windows")
> Fixes: ba420719823c ("common/mlx5: add reg/dereg MR on Windows")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
Tested-by: Wisam Jaddo <wisamm@nvidia.com>


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH] common/mlx5: fix missing __rte_internal tags in exported functions
  2021-04-11 12:21 [dpdk-dev] [PATCH] common/mlx5: fix missing __rte_internal tags in exported functions Tal Shnaiderman
  2021-04-11 13:00 ` Matan Azrad
  2021-04-11 13:09 ` Wisam Monther
@ 2021-04-12 11:25 ` Raslan Darawsheh
  2021-04-12 12:26   ` Thomas Monjalon
  2021-04-12 12:59   ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
  2021-04-12 15:28 ` [dpdk-dev] " Thomas Monjalon
  3 siblings, 2 replies; 13+ messages in thread
From: Raslan Darawsheh @ 2021-04-12 11:25 UTC (permalink / raw)
  To: Tal Shnaiderman, dev
  Cc: NBU-Contact-Thomas Monjalon, Matan Azrad, Asaf Penso,
	Wisam Monther, stable

Hi,

> -----Original Message-----
> From: Tal Shnaiderman <talshn@nvidia.com>
> Sent: Sunday, April 11, 2021 3:22 PM
> To: dev@dpdk.org
> Cc: NBU-Contact-Thomas Monjalon <thomas@monjalon.net>; Matan Azrad
> <matan@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>; Asaf Penso
> <asafp@nvidia.com>; Wisam Monther <wisamm@nvidia.com>;
> stable@dpdk.org
> Subject: [PATCH] common/mlx5: fix missing __rte_internal tags in exported
> functions
> 
Removed __ from the commit title to fix wrong headline format issue.

> Several functions introduced in the addition of the Windows support to
> mlx5 were missing the __rte_internal tag although being exported.
> 
> Fixes: 1552fb287166 ("common/mlx5: add alloc/dealloc PD on Windows")
> Fixes: 1969ee424405 ("common/mlx5: add UMEM reg/dereg functions on
> Windows")
> Fixes: ba420719823c ("common/mlx5: add reg/dereg MR on Windows")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>

Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH] common/mlx5: fix missing __rte_internal tags in exported functions
  2021-04-12 11:25 ` Raslan Darawsheh
@ 2021-04-12 12:26   ` Thomas Monjalon
  2021-04-12 12:30     ` Raslan Darawsheh
  2021-04-12 12:59   ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
  1 sibling, 1 reply; 13+ messages in thread
From: Thomas Monjalon @ 2021-04-12 12:26 UTC (permalink / raw)
  To: Tal Shnaiderman, Raslan Darawsheh
  Cc: dev, Matan Azrad, Asaf Penso, Wisam Monther, stable

12/04/2021 13:25, Raslan Darawsheh:
> Removed __ from the commit title to fix wrong headline format issue.

Removing __ makes it wrong: rte_internal does not exist.
The idea of discouraging the use of underscore is to avoid
naming bits and bytes in the title.
Here it should be: "add missing internal tags to some functions"

> > Several functions introduced in the addition of the Windows support to
> > mlx5 were missing the __rte_internal tag although being exported.




^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH] common/mlx5: fix missing __rte_internal tags in exported functions
  2021-04-12 12:26   ` Thomas Monjalon
@ 2021-04-12 12:30     ` Raslan Darawsheh
  0 siblings, 0 replies; 13+ messages in thread
From: Raslan Darawsheh @ 2021-04-12 12:30 UTC (permalink / raw)
  To: NBU-Contact-Thomas Monjalon, Tal Shnaiderman
  Cc: dev, Matan Azrad, Asaf Penso, Wisam Monther, stable

Hi,

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Monday, April 12, 2021 3:26 PM
> To: Tal Shnaiderman <talshn@nvidia.com>; Raslan Darawsheh
> <rasland@nvidia.com>
> Cc: dev@dpdk.org; Matan Azrad <matan@nvidia.com>; Asaf Penso
> <asafp@nvidia.com>; Wisam Monther <wisamm@nvidia.com>;
> stable@dpdk.org
> Subject: Re: [PATCH] common/mlx5: fix missing __rte_internal tags in
> exported functions
> 
> 12/04/2021 13:25, Raslan Darawsheh:
> > Removed __ from the commit title to fix wrong headline format issue.
> 
> Removing __ makes it wrong: rte_internal does not exist.
> The idea of discouraging the use of underscore is to avoid
> naming bits and bytes in the title.
> Here it should be: "add missing internal tags to some functions"
My suggestion was to make it like this:
"fix missing rte internal tags in exported functions"

But, I find your suggestion even better, so I'd change it to that

> 
> > > Several functions introduced in the addition of the Windows support to
> > > mlx5 were missing the __rte_internal tag although being exported.
> 
> 
Kindest regards
Raslan Darawsheh

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH] common/mlx5: fix missing __rte_internal tags in exported functions
  2021-04-12 11:25 ` Raslan Darawsheh
  2021-04-12 12:26   ` Thomas Monjalon
@ 2021-04-12 12:59   ` Ferruh Yigit
  2021-04-12 13:06     ` Thomas Monjalon
  1 sibling, 1 reply; 13+ messages in thread
From: Ferruh Yigit @ 2021-04-12 12:59 UTC (permalink / raw)
  To: Raslan Darawsheh, Tal Shnaiderman, dev, NBU-Contact-Thomas Monjalon
  Cc: Matan Azrad, Asaf Penso, Wisam Monther, stable

On 4/12/2021 12:25 PM, Raslan Darawsheh wrote:
> Hi,
> 
>> -----Original Message-----
>> From: Tal Shnaiderman <talshn@nvidia.com>
>> Sent: Sunday, April 11, 2021 3:22 PM
>> To: dev@dpdk.org
>> Cc: NBU-Contact-Thomas Monjalon <thomas@monjalon.net>; Matan Azrad
>> <matan@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>; Asaf Penso
>> <asafp@nvidia.com>; Wisam Monther <wisamm@nvidia.com>;
>> stable@dpdk.org
>> Subject: [PATCH] common/mlx5: fix missing __rte_internal tags in exported
>> functions
>>
> Removed __ from the commit title to fix wrong headline format issue.
> 
>> Several functions introduced in the addition of the Windows support to
>> mlx5 were missing the __rte_internal tag although being exported.
>>
>> Fixes: 1552fb287166 ("common/mlx5: add alloc/dealloc PD on Windows")
>> Fixes: 1969ee424405 ("common/mlx5: add UMEM reg/dereg functions on
>> Windows")
>> Fixes: ba420719823c ("common/mlx5: add reg/dereg MR on Windows")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
> 
> Patch applied to next-net-mlx,
> 

Can we merge this directly to main repo?
Since debug build is broken without it.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH] common/mlx5: fix missing __rte_internal tags in exported functions
  2021-04-12 12:59   ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
@ 2021-04-12 13:06     ` Thomas Monjalon
  2021-04-12 13:35       ` Ferruh Yigit
  0 siblings, 1 reply; 13+ messages in thread
From: Thomas Monjalon @ 2021-04-12 13:06 UTC (permalink / raw)
  To: Raslan Darawsheh, Tal Shnaiderman, dev, Ferruh Yigit
  Cc: Matan Azrad, Asaf Penso, Wisam Monther, stable

12/04/2021 14:59, Ferruh Yigit:
> On 4/12/2021 12:25 PM, Raslan Darawsheh wrote:
> > Hi,
> > 
> > From: Tal Shnaiderman <talshn@nvidia.com>
> >>
> > Removed __ from the commit title to fix wrong headline format issue.
> > 
> >> Several functions introduced in the addition of the Windows support to
> >> mlx5 were missing the __rte_internal tag although being exported.
> >>
> >> Fixes: 1552fb287166 ("common/mlx5: add alloc/dealloc PD on Windows")
> >> Fixes: 1969ee424405 ("common/mlx5: add UMEM reg/dereg functions on
> >> Windows")
> >> Fixes: ba420719823c ("common/mlx5: add reg/dereg MR on Windows")
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
> > 
> > Patch applied to next-net-mlx,
> > 
> 
> Can we merge this directly to main repo?
> Since debug build is broken without it.

Which debug option?
It is broken since when?



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH] common/mlx5: fix missing __rte_internal tags in exported functions
  2021-04-12 13:06     ` Thomas Monjalon
@ 2021-04-12 13:35       ` Ferruh Yigit
  2021-04-12 13:54         ` David Marchand
  0 siblings, 1 reply; 13+ messages in thread
From: Ferruh Yigit @ 2021-04-12 13:35 UTC (permalink / raw)
  To: Thomas Monjalon, Raslan Darawsheh, Tal Shnaiderman, dev
  Cc: Matan Azrad, Asaf Penso, Wisam Monther, stable, David Marchand,
	Bruce Richardson

On 4/12/2021 2:06 PM, Thomas Monjalon wrote:
> 12/04/2021 14:59, Ferruh Yigit:
>> On 4/12/2021 12:25 PM, Raslan Darawsheh wrote:
>>> Hi,
>>>
>>> From: Tal Shnaiderman <talshn@nvidia.com>
>>>>
>>> Removed __ from the commit title to fix wrong headline format issue.
>>>
>>>> Several functions introduced in the addition of the Windows support to
>>>> mlx5 were missing the __rte_internal tag although being exported.
>>>>
>>>> Fixes: 1552fb287166 ("common/mlx5: add alloc/dealloc PD on Windows")
>>>> Fixes: 1969ee424405 ("common/mlx5: add UMEM reg/dereg functions on
>>>> Windows")
>>>> Fixes: ba420719823c ("common/mlx5: add reg/dereg MR on Windows")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
>>>
>>> Patch applied to next-net-mlx,
>>>
>>
>> Can we merge this directly to main repo?
>> Since debug build is broken without it.
> 
> Which debug option?
> It is broken since when?
> 
> 

"meson --buildtype=debug build && ninja -C build" is broken [1], I thought that 
is why this patch is done at first place.
Some checks are done now only in the debug mode, since checks are reduced to 
developer mode [2].

It become visible when .def file removed [3], since that is when symbols are 
added to the .map file.


[1]
FAILED: drivers/rte_common_mlx5.sym_chk
.../meson --internal exe --capture drivers/rte_common_mlx5.sym_chk -- 
.../buildtools/check-symbols.sh .../drivers/common/mlx5/version.map 
drivers/librte_common_mlx5.a
mlx5_os_umem_reg is not flagged as internal
but is listed in version map
Please add __rte_internal to the definition of mlx5_os_umem_reg
mlx5_os_umem_dereg is not flagged as internal
but is listed in version map
Please add __rte_internal to the definition of mlx5_os_umem_dereg

[2]
Commit 720dfda4551e ("build: limit symbol checks to developer mode")

[3]
Commit 56ea803e878e ("build: remove Windows export symbol list")

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH] common/mlx5: fix missing __rte_internal tags in exported functions
  2021-04-12 13:35       ` Ferruh Yigit
@ 2021-04-12 13:54         ` David Marchand
  2021-04-12 15:00           ` Tal Shnaiderman
  2021-04-12 15:12           ` Ferruh Yigit
  0 siblings, 2 replies; 13+ messages in thread
From: David Marchand @ 2021-04-12 13:54 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: Thomas Monjalon, Raslan Darawsheh, Tal Shnaiderman, dev,
	Matan Azrad, Asaf Penso, Wisam Monther, stable, Bruce Richardson

On Mon, Apr 12, 2021 at 3:35 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> On 4/12/2021 2:06 PM, Thomas Monjalon wrote:
> > 12/04/2021 14:59, Ferruh Yigit:
> >> On 4/12/2021 12:25 PM, Raslan Darawsheh wrote:
> >>> Hi,
> >>>
> >>> From: Tal Shnaiderman <talshn@nvidia.com>
> >>>>
> >>> Removed __ from the commit title to fix wrong headline format issue.
> >>>
> >>>> Several functions introduced in the addition of the Windows support to
> >>>> mlx5 were missing the __rte_internal tag although being exported.
> >>>>
> >>>> Fixes: 1552fb287166 ("common/mlx5: add alloc/dealloc PD on Windows")
> >>>> Fixes: 1969ee424405 ("common/mlx5: add UMEM reg/dereg functions on
> >>>> Windows")
> >>>> Fixes: ba420719823c ("common/mlx5: add reg/dereg MR on Windows")
> >>>> Cc: stable@dpdk.org
> >>>>
> >>>> Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
> >>>
> >>> Patch applied to next-net-mlx,
> >>>
> >>
> >> Can we merge this directly to main repo?
> >> Since debug build is broken without it.
> >
> > Which debug option?
> > It is broken since when?
> >
> >
>
> "meson --buildtype=debug build && ninja -C build" is broken [1], I thought that
> is why this patch is done at first place.
> Some checks are done now only in the debug mode, since checks are reduced to
> developer mode [2].

I don't think [2] has something to do with it.

The symbols fixed here are inlined.
In release (and debugoptimized) modes, I can see no trace of them in the .o

$ nm build/drivers/librte_common_mlx5.a |grep umem_reg
                 U mlx5dv_devx_umem_reg
0000000000000410 t mlx5_glue_devx_umem_reg

While in debug mode:
$ nm build.debug/drivers/librte_common_mlx5.a |grep umem_reg
00000000000000b9 t mlx5_os_umem_reg
                 ^^^^^^^^^^^^^^^^^^
                 U mlx5dv_devx_umem_reg
0000000000001578 t mlx5_glue_devx_umem_reg


The symbol check then catches mlx5_os_umem_reg wrt the version.map and
complains mlx5_os_umem_reg is not tagged internal.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH] common/mlx5: fix missing __rte_internal tags in exported functions
  2021-04-12 13:54         ` David Marchand
@ 2021-04-12 15:00           ` Tal Shnaiderman
  2021-04-12 15:12           ` Ferruh Yigit
  1 sibling, 0 replies; 13+ messages in thread
From: Tal Shnaiderman @ 2021-04-12 15:00 UTC (permalink / raw)
  To: David Marchand, Ferruh Yigit
  Cc: NBU-Contact-Thomas Monjalon, Raslan Darawsheh, dev, Matan Azrad,
	Asaf Penso, Wisam Monther, stable, Bruce Richardson

> Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH] common/mlx5: fix missing
> __rte_internal tags in exported functions
> 
> External email: Use caution opening links or attachments
> 
> 
> On Mon, Apr 12, 2021 at 3:35 PM Ferruh Yigit <ferruh.yigit@intel.com>
> wrote:
> >
> > On 4/12/2021 2:06 PM, Thomas Monjalon wrote:
> > > 12/04/2021 14:59, Ferruh Yigit:
> > >> On 4/12/2021 12:25 PM, Raslan Darawsheh wrote:
> > >>> Hi,
> > >>>
> > >>> From: Tal Shnaiderman <talshn@nvidia.com>
> > >>>>
> > >>> Removed __ from the commit title to fix wrong headline format issue.
> > >>>
> > >>>> Several functions introduced in the addition of the Windows
> > >>>> support to
> > >>>> mlx5 were missing the __rte_internal tag although being exported.
> > >>>>
> > >>>> Fixes: 1552fb287166 ("common/mlx5: add alloc/dealloc PD on
> > >>>> Windows")
> > >>>> Fixes: 1969ee424405 ("common/mlx5: add UMEM reg/dereg
> functions
> > >>>> on
> > >>>> Windows")
> > >>>> Fixes: ba420719823c ("common/mlx5: add reg/dereg MR on
> Windows")
> > >>>> Cc: stable@dpdk.org
> > >>>>
> > >>>> Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
> > >>>
> > >>> Patch applied to next-net-mlx,
> > >>>
> > >>
> > >> Can we merge this directly to main repo?
> > >> Since debug build is broken without it.
> > >
> > > Which debug option?
> > > It is broken since when?
> > >
> > >
> >
> > "meson --buildtype=debug build && ninja -C build" is broken [1], I
> > thought that is why this patch is done at first place.
> > Some checks are done now only in the debug mode, since checks are
> > reduced to developer mode [2].
> 
> I don't think [2] has something to do with it.
> 
> The symbols fixed here are inlined.
> In release (and debugoptimized) modes, I can see no trace of them in the .o
> 
> $ nm build/drivers/librte_common_mlx5.a |grep umem_reg
>                  U mlx5dv_devx_umem_reg
> 0000000000000410 t mlx5_glue_devx_umem_reg
> 
> While in debug mode:
> $ nm build.debug/drivers/librte_common_mlx5.a |grep umem_reg
> 00000000000000b9 t mlx5_os_umem_reg
>                  ^^^^^^^^^^^^^^^^^^
>                  U mlx5dv_devx_umem_reg
> 0000000000001578 t mlx5_glue_devx_umem_reg
> 
> 
> The symbol check then catches mlx5_os_umem_reg wrt the version.map
> and complains mlx5_os_umem_reg is not tagged internal.
> 
> 
> --
> David Marchand

Right, the issue was in observed from commit 56ea803e878e ("build: remove Windows export symbol list") and this patch fixes it, apologies for not stating it in the commit log.

Since rte_common_mlx5_exports.def was merged to version.map several functions which needed export only for Windows were brought in, e.g. mlx5_os_umem_reg, in Linux it is implemented in the .h file but has a .c implementation on Windows so it is needed to be in version.map.

Windows doesn't run symbol check in debug so up until the file merge it wasn't caught.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH] common/mlx5: fix missing __rte_internal tags in exported functions
  2021-04-12 13:54         ` David Marchand
  2021-04-12 15:00           ` Tal Shnaiderman
@ 2021-04-12 15:12           ` Ferruh Yigit
  1 sibling, 0 replies; 13+ messages in thread
From: Ferruh Yigit @ 2021-04-12 15:12 UTC (permalink / raw)
  To: David Marchand
  Cc: Thomas Monjalon, Raslan Darawsheh, Tal Shnaiderman, dev,
	Matan Azrad, Asaf Penso, Wisam Monther, stable, Bruce Richardson

On 4/12/2021 2:54 PM, David Marchand wrote:
> On Mon, Apr 12, 2021 at 3:35 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>
>> On 4/12/2021 2:06 PM, Thomas Monjalon wrote:
>>> 12/04/2021 14:59, Ferruh Yigit:
>>>> On 4/12/2021 12:25 PM, Raslan Darawsheh wrote:
>>>>> Hi,
>>>>>
>>>>> From: Tal Shnaiderman <talshn@nvidia.com>
>>>>>>
>>>>> Removed __ from the commit title to fix wrong headline format issue.
>>>>>
>>>>>> Several functions introduced in the addition of the Windows support to
>>>>>> mlx5 were missing the __rte_internal tag although being exported.
>>>>>>
>>>>>> Fixes: 1552fb287166 ("common/mlx5: add alloc/dealloc PD on Windows")
>>>>>> Fixes: 1969ee424405 ("common/mlx5: add UMEM reg/dereg functions on
>>>>>> Windows")
>>>>>> Fixes: ba420719823c ("common/mlx5: add reg/dereg MR on Windows")
>>>>>> Cc: stable@dpdk.org
>>>>>>
>>>>>> Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
>>>>>
>>>>> Patch applied to next-net-mlx,
>>>>>
>>>>
>>>> Can we merge this directly to main repo?
>>>> Since debug build is broken without it.
>>>
>>> Which debug option?
>>> It is broken since when?
>>>
>>>
>>
>> "meson --buildtype=debug build && ninja -C build" is broken [1], I thought that
>> is why this patch is done at first place.
>> Some checks are done now only in the debug mode, since checks are reduced to
>> developer mode [2].
> 
> I don't think [2] has something to do with it.
> 
> The symbols fixed here are inlined.
> In release (and debugoptimized) modes, I can see no trace of them in the .o
> 
> $ nm build/drivers/librte_common_mlx5.a |grep umem_reg
>                   U mlx5dv_devx_umem_reg
> 0000000000000410 t mlx5_glue_devx_umem_reg
> 
> While in debug mode:
> $ nm build.debug/drivers/librte_common_mlx5.a |grep umem_reg
> 00000000000000b9 t mlx5_os_umem_reg
>                   ^^^^^^^^^^^^^^^^^^
>                   U mlx5dv_devx_umem_reg
> 0000000000001578 t mlx5_glue_devx_umem_reg
> 
> 
> The symbol check then catches mlx5_os_umem_reg wrt the version.map and
> complains mlx5_os_umem_reg is not tagged internal.
> 
> 

Got it, if so this is not related to [2] as you said, but related to the debug 
mode build, thanks for clarification.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH] common/mlx5: fix missing __rte_internal tags in exported functions
  2021-04-11 12:21 [dpdk-dev] [PATCH] common/mlx5: fix missing __rte_internal tags in exported functions Tal Shnaiderman
                   ` (2 preceding siblings ...)
  2021-04-12 11:25 ` Raslan Darawsheh
@ 2021-04-12 15:28 ` Thomas Monjalon
  3 siblings, 0 replies; 13+ messages in thread
From: Thomas Monjalon @ 2021-04-12 15:28 UTC (permalink / raw)
  To: Tal Shnaiderman
  Cc: dev, matan, rasland, asafp, wisamm, stable, ferruh.yigit, david.marchand

11/04/2021 14:21, Tal Shnaiderman:
> Several functions introduced in the addition of the Windows support to
> mlx5 were missing the __rte_internal tag although being exported.
> 
> Fixes: 1552fb287166 ("common/mlx5: add alloc/dealloc PD on Windows")
> Fixes: 1969ee424405 ("common/mlx5: add UMEM reg/dereg functions on Windows")
> Fixes: ba420719823c ("common/mlx5: add reg/dereg MR on Windows")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>

It is fixing Linux debug build (note: should be added to CI).

Applied in main tree with this message, thanks:

common/mlx5: add missing internal tags

Several functions introduced in the addition of the Windows support
to mlx5 were missing the __rte_internal tag.
This miss is better revealed when symbols became exported on Linux too,
and it is caught by the symbol checker with --buildtype=debug.

Fixes: 1552fb287166 ("common/mlx5: add alloc/dealloc PD on Windows")
Fixes: 1969ee424405 ("common/mlx5: add UMEM reg/dereg functions on Windows")
Fixes: ba420719823c ("common/mlx5: add reg/dereg MR on Windows")
Fixes: 56ea803e878e ("build: remove Windows export symbol list")
Cc: stable@dpdk.org

Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>



^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-04-12 15:29 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-11 12:21 [dpdk-dev] [PATCH] common/mlx5: fix missing __rte_internal tags in exported functions Tal Shnaiderman
2021-04-11 13:00 ` Matan Azrad
2021-04-11 13:09 ` Wisam Monther
2021-04-12 11:25 ` Raslan Darawsheh
2021-04-12 12:26   ` Thomas Monjalon
2021-04-12 12:30     ` Raslan Darawsheh
2021-04-12 12:59   ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
2021-04-12 13:06     ` Thomas Monjalon
2021-04-12 13:35       ` Ferruh Yigit
2021-04-12 13:54         ` David Marchand
2021-04-12 15:00           ` Tal Shnaiderman
2021-04-12 15:12           ` Ferruh Yigit
2021-04-12 15:28 ` [dpdk-dev] " Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).