From: Tal Shnaiderman <talshn@nvidia.com>
To: David Marchand <david.marchand@redhat.com>,
Ferruh Yigit <ferruh.yigit@intel.com>
Cc: NBU-Contact-Thomas Monjalon <thomas@monjalon.net>,
Raslan Darawsheh <rasland@nvidia.com>,
"dev@dpdk.org" <dev@dpdk.org>, Matan Azrad <matan@nvidia.com>,
Asaf Penso <asafp@nvidia.com>, Wisam Monther <wisamm@nvidia.com>,
"stable@dpdk.org" <stable@dpdk.org>,
Bruce Richardson <bruce.richardson@intel.com>
Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH] common/mlx5: fix missing __rte_internal tags in exported functions
Date: Mon, 12 Apr 2021 15:00:48 +0000 [thread overview]
Message-ID: <DM6PR12MB39455A9C02B724B32E9156B1A4709@DM6PR12MB3945.namprd12.prod.outlook.com> (raw)
In-Reply-To: <CAJFAV8ypBgtGk2zSU10seqhmRg4jz7XYs3Gm7=vVOZON51X9Ew@mail.gmail.com>
> 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.
next prev parent reply other threads:[~2021-04-12 15:00 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-11 12:21 [dpdk-stable] " 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 ` Ferruh Yigit
2021-04-12 13:06 ` Thomas Monjalon
2021-04-12 13:35 ` [dpdk-stable] [dpdk-dev] " Ferruh Yigit
2021-04-12 13:54 ` David Marchand
2021-04-12 15:00 ` Tal Shnaiderman [this message]
2021-04-12 15:12 ` Ferruh Yigit
2021-04-12 15:28 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DM6PR12MB39455A9C02B724B32E9156B1A4709@DM6PR12MB3945.namprd12.prod.outlook.com \
--to=talshn@nvidia.com \
--cc=asafp@nvidia.com \
--cc=bruce.richardson@intel.com \
--cc=david.marchand@redhat.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
--cc=matan@nvidia.com \
--cc=rasland@nvidia.com \
--cc=stable@dpdk.org \
--cc=thomas@monjalon.net \
--cc=wisamm@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).