Re: [PATCH v4 1/4] eal: add macro to embed information in binaries

[Andre Muezerie <andremue@linux.microsoft.com>]

On Wed, Jun 04, 2025 at 03:13:55PM +0100, Bruce Richardson wrote:
> On Wed, Apr 16, 2025 at 02:44:55PM -0700, Andre Muezerie wrote:
> > DPDK uses GCC attribute "used" through macro __rte_used to indicate
> > that a variable not referenced in the code should be assumed being
> > used and therefore not be optimized away. This technique is used to embed
> > information in the binaries, by having crafted information stored in
> > them.
> > 
> > MSVC offers similar functionality, but it differs significantly: MSVC
> > requires a pragma to be used to send a command to the linker telling it
> > explicitly the name of the symbol that should be included (even if not
> > referenced). As a side-effect, variables called out to be included cannot
> > be static, otherwise their symbols are not "seen" by the linker. This
> > restriction requires some DPDK code to be refactored.
> > 
> > To assimilate these requirements/restrictions, macro RTE_INCLUDE is
> > introduced in this patch.
> > 
> > Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> > ---
> >  lib/eal/common/eal_common_options.c | 2 +-
> >  lib/eal/include/rte_common.h        | 6 ++++++
> >  2 files changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c
> > index b6fff7ec05..8d115a5bd7 100644
> > --- a/lib/eal/common/eal_common_options.c
> > +++ b/lib/eal/common/eal_common_options.c
> > @@ -135,7 +135,7 @@ static const char *default_solib_dir = RTE_EAL_PMD_PATH;
> >   * Note: PLEASE DO NOT ALTER THIS without making a corresponding
> >   * change to usertools/dpdk-pmdinfo.py
> >   */
> > -static const char dpdk_solib_path[] __rte_used =
> > +RTE_INCLUDE(const char, dpdk_solib_path)[] =
> >  "DPDK_PLUGIN_PATH=" RTE_EAL_PMD_PATH;
> >  
> >  TAILQ_HEAD(device_option_list, device_option);
> > diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
> > index a6085dce27..4c5a3b668f 100644
> > --- a/lib/eal/include/rte_common.h
> > +++ b/lib/eal/include/rte_common.h
> > @@ -231,6 +231,12 @@ typedef uint16_t unaligned_uint16_t;
> >  /**
> >   * Force symbol to be generated even if it appears to be unused.
> >   */
> > +#ifdef RTE_TOOLCHAIN_MSVC
> > +#define RTE_INCLUDE(type, name) __pragma(comment(linker, "/include:" RTE_STR(name))) type name
> > +#else
> > +#define RTE_INCLUDE(type, name) __attribute__((used)) type name
> 
> Any reason we shouldn't also have "static" here, for the non-MSVC case?
> 

The keyword "static" can't be used here with MSVC. It is nice to have the same source code
for MSVC and non-MSVC, and I don't see a good reason to have "static"
here just for the non-MSVC case. To some extent it is even somewhat contradictory to
have "static" and "__attribute__((used))" in the same line (hide it here but expose it there).

Let me know if you see a strong reason to have "static" just for the non-MSVC case.

> As a general question, is this needed for any/many tasks other than putting
> in place strings? Rather than having a general include macro,
> I'm just wondering if we are better doing specific macros for the driver
> string, and then other-string use-cases.

There might be other use cases for RTE_INCLUDE, but for now it has only been used
to put in place strings in drivers and common code
(like lib\eal\common\eal_common_options.c).

In most cases RTE_INCLUDE is not being called directly - it's called by other macros
like the one below:

#define RTE_PMD_REGISTER_KMOD_DEP(name, str) \
RTE_INCLUDE(const char, DRV_EXP_TAG(name, kmod_dep_export))[] = str

That being said, I don't see strong reason to prevent people from calling it directly.

> 
> /Bruce