common/mlx5: fix missing __rte_internal tags in exported functions
Checks
Commit Message
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(+)
Comments
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
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>
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
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.
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
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.
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?
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")
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.
> 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.
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.
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>
@@ -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)
{
@@ -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_ */