[dpdk-dev] [PATCH v3 1/4] compat: Add infrastructure to support symbol versioning
Neil Horman
nhorman at tuxdriver.com
Wed Jan 14 21:29:50 CET 2015
On Wed, Jan 14, 2015 at 04:25:19PM +0100, Thomas Monjalon wrote:
> Hi Neil,
>
> 2014-12-23 10:51, Neil Horman:
> > Add initial pass header files to support symbol versioning.
>
> [...]
>
> > +# Copyright(c) 2010-2014 Neil Horman <nhorman at tuxdriver.com>
>
> Why these dates?
>
Because I copied the Makefile from librte_acl, and modified the name but not the
dates.
> > +# All rights reserved.
>
> I think this line is not required anymore:
> http://en.wikipedia.org/wiki/All_rights_reserved
>
Hmm, apparently so. However, since it exists in every other copyright notice in
the tree, I'd just as soon keep this language consistent, and make a tree wide
change in a separate patch if the consensus is to do so.
> [...]
>
> > +#ifndef _RTE_COMPAT_H_
> > +#define _RTE_COMPAT_H_
>
> Why using underscores?
> I think it's reserved:
> http://en.wikipedia.org/wiki/Include_guard#Use_of_.23include_guards
>
Its reserved for the implementation, and must not be used by a user using the
header file. Its ok, and is common practice. See every other symlinked header
file in the DPDK.
> > +#define SA(x) #x
>
> It should be prefixed. But it's better to use RTE_STR.
>
very well
> > +#ifdef RTE_BUILD_SHARED_LIB
> > +
> > +/*
> > + * Provides backwards compatibility when updating exported functions.
> > + * When a symol is exported from a library to provide an API, it also provides a
> > + * calling convention (ABI) that is embodied in its name, return type,
> > + * arguments, etc. On occasion that function may need to change to accomodate
> > + * new functionality, behavior, etc. When that occurs, it is desireable to
> > + * allow for backwards compatibility for a time with older binaries that are
> > + * dynamically linked to the dpdk. to support that the __vsym and
>
> Should be "To support that," with uppercase and comma.
>
yup
> > + * VERSION_SYMBOL macros are created. They, in conjunction with the
> > + * <library>_version.map file for a given library allow for multiple versions of
> > + * a symbol to exist in a shared library so that older binaries need not be
> > + * immediately recompiled. Their use is outlined in the following example:
> > + * Assumptions: DPDK 1.(X) contains a function int foo(char *string)
> > + * DPDK 1.(X+1) needs to change foo to be int foo(int index)
> > + *
> > + * To accomplish this:
> > + * 1) Edit lib/<library>/library_version.map to add a DPDK_1.(X+1) node, in which
> > + * foo is exported as a global symbol.
> > + *
> > + * 2) rename the existing function int foo(char *string) to
> > + * int __vsym foo_v18(char *string)
> > + *
> > + * 3) Add this macro immediately below the function
> > + * VERSION_SYMBOL(foo, _v18, 1.8);
> > + *
> > + * 4) Implement a new version of foo.
> > + * char foo(int value, int otherval) { ...}
> > + *
> > + * 5) Mark the newest version as the default version
> > + * BIND_DEFAULT_SYMBOL(foo, 1.9);
> > + *
> > + */
>
> Thanks for this good tutorial.
>
> > +#define VERSION_SYMBOL(b, e, v) __asm__(".symver " SA(b) SA(e) ", "SA(b)"@DPDK_"SA(v))
> > +#define BASE_SYMBOL(b, n) __asm__(".symver " SA(n) ", "SA(b)"@")
> > +#define BIND_DEFAULT_SYMBOL(b, v) __asm__(".symver " SA(b) ", "SA(b)"@@DPDK_"SA(v))
> > +#define __vsym __attribute__((used))
>
> OK. It would be simpler to read if b, e, v and n were formally defined in a comment.
>
> > +#else
> [...]
> > +/*
> > + * RTE_BUILD_SHARED_LIB
> > + */
>
> This type of comment is strange. It makes me think that we are in the case
> RTE_BUILD_SHARED_LIB=y
>
> > +#endif
>
> [...]
>
> > +
> > +CPU_LDFLAGS += --version-script=$(EXPORT_MAP)
>
> Why this variable name? VERSION_SCRIPT or VERSION_MAP seems more appropriate.
>
> > +
> > endif
> >
> > +
>
> Why this newline?
>
> > _BUILD = $(LIB)
>
> --
> Thomas
>
More information about the dev
mailing list