[dpdk-dev] [PATCH 1/2] buildtools: fix experimental symbols listing

[dpdk-dev] [PATCH 1/2] buildtools: fix experimental symbols listing

[David Marchand]

The map-list-symbol.sh script displays the filename, section and symbol
names of map files.

Example:
$ buildtools/map-list-symbol.sh -S EXPERIMENTAL \
    lib/librte_ethdev/rte_ethdev_version.map |grep rte_mtr_create
lib/librte_ethdev/rte_ethdev_version.map EXPERIMENTAL rte_mtr_create

The experimental symbol check should only consider the symbol name.

Fixes: 3290ac14eb94 ("buildtools: detect discrepancies for experimental symbols")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 buildtools/check-experimental-syms.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/buildtools/check-experimental-syms.sh b/buildtools/check-experimental-syms.sh
index 145dd70ebf..abebb89f12 100755
--- a/buildtools/check-experimental-syms.sh
+++ b/buildtools/check-experimental-syms.sh
@@ -23,7 +23,7 @@ trap 'rm -f "$DUMPFILE"' EXIT
 objdump -t $OBJFILE >$DUMPFILE
 
 ret=0
-for SYM in `$LIST_SYMBOL -S EXPERIMENTAL $MAPFILE`
+for SYM in `$LIST_SYMBOL -S EXPERIMENTAL $MAPFILE |cut -d ' ' -f 3`
 do
 	if grep -q "\.text.*$SYM$" $DUMPFILE &&
 		! grep -q "\.text\.experimental.*$SYM$" $DUMPFILE
-- 
2.23.0

[dpdk-dev] [PATCH 2/2] buildtools: fix build with coverage enabled

[David Marchand]

A compiler can reuse a variable name and prefix it when instrumenting
with coverage.

Example:
$ make defconfig T=x86_64-native-linux-gcc O=master
$ make EXTRA_CFLAGS='--coverage' O=master
[...]
    CC rte_flow.o
 rte_flow_dynf_metadata_offs is not flagged as experimental but is listed
 in version map
 Please add __rte_experimental to the definition of
 rte_flow_dynf_metadata_offs

$ objdump -t master/build/lib/librte_ethdev/rte_flow.o |grep _offs$
0000000000000000 l     F .text.startup	000000000000000a
  _GLOBAL__sub_I_65535_0_rte_flow_dynf_metadata_offs
0000000000000620 g     O .data	0000000000000004
  rte_flow_dynf_metadata_offs

Protect against this by adding a space character in the pattern.

Fixes: a4bcd61de82d ("buildtools: add script to check experimental API exports")
Cc: stable@dpdk.org

Reported-by: Andrew Rybchenko <arybchenko@solarflare.com>
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 buildtools/check-experimental-syms.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/buildtools/check-experimental-syms.sh b/buildtools/check-experimental-syms.sh
index abebb89f12..f3603e5bac 100755
--- a/buildtools/check-experimental-syms.sh
+++ b/buildtools/check-experimental-syms.sh
@@ -25,8 +25,8 @@ objdump -t $OBJFILE >$DUMPFILE
 ret=0
 for SYM in `$LIST_SYMBOL -S EXPERIMENTAL $MAPFILE |cut -d ' ' -f 3`
 do
-	if grep -q "\.text.*$SYM$" $DUMPFILE &&
-		! grep -q "\.text\.experimental.*$SYM$" $DUMPFILE
+	if grep -q "\.text.*[[:space:]]$SYM$" $DUMPFILE &&
+		! grep -q "\.text\.experimental.*[[:space:]]$SYM$" $DUMPFILE
 	then
 		cat >&2 <<- END_OF_MESSAGE
 		$SYM is not flagged as experimental
-- 
2.23.0

Re: [dpdk-dev] [PATCH 2/2] buildtools: fix build with coverage enabled

[Andrew Rybchenko]

On 11/25/19 11:10 AM, David Marchand wrote:
> A compiler can reuse a variable name and prefix it when instrumenting
> with coverage.
> 
> Example:
> $ make defconfig T=x86_64-native-linux-gcc O=master
> $ make EXTRA_CFLAGS='--coverage' O=master
> [...]
>     CC rte_flow.o
>  rte_flow_dynf_metadata_offs is not flagged as experimental but is listed
>  in version map
>  Please add __rte_experimental to the definition of
>  rte_flow_dynf_metadata_offs
> 
> $ objdump -t master/build/lib/librte_ethdev/rte_flow.o |grep _offs$
> 0000000000000000 l     F .text.startup	000000000000000a
>   _GLOBAL__sub_I_65535_0_rte_flow_dynf_metadata_offs
> 0000000000000620 g     O .data	0000000000000004
>   rte_flow_dynf_metadata_offs
> 
> Protect against this by adding a space character in the pattern.
> 
> Fixes: a4bcd61de82d ("buildtools: add script to check experimental API exports")
> Cc: stable@dpdk.org
> 
> Reported-by: Andrew Rybchenko <arybchenko@solarflare.com>
> Signed-off-by: David Marchand <david.marchand@redhat.com>

David,

thanks a lot, it solves build problem.

It does not solve experimental symbol inconsistency problem in
markup and map file, but it is a separate less critical
problem.

Tested-by: Andrew Rybchenko <arybchenko@solarflare.com>

Re: [dpdk-dev] [PATCH 2/2] buildtools: fix build with coverage enabled

[David Marchand]

On Mon, Nov 25, 2019 at 9:29 AM Andrew Rybchenko
<arybchenko@solarflare.com> wrote:
>
> On 11/25/19 11:10 AM, David Marchand wrote:
> > A compiler can reuse a variable name and prefix it when instrumenting
> > with coverage.
> >
> > Example:
> > $ make defconfig T=x86_64-native-linux-gcc O=master
> > $ make EXTRA_CFLAGS='--coverage' O=master
> > [...]
> >     CC rte_flow.o
> >  rte_flow_dynf_metadata_offs is not flagged as experimental but is listed
> >  in version map
> >  Please add __rte_experimental to the definition of
> >  rte_flow_dynf_metadata_offs
> >
> > $ objdump -t master/build/lib/librte_ethdev/rte_flow.o |grep _offs$
> > 0000000000000000 l     F .text.startup        000000000000000a
> >   _GLOBAL__sub_I_65535_0_rte_flow_dynf_metadata_offs
> > 0000000000000620 g     O .data        0000000000000004
> >   rte_flow_dynf_metadata_offs
> >
> > Protect against this by adding a space character in the pattern.
> >
> > Fixes: a4bcd61de82d ("buildtools: add script to check experimental API exports")
> > Cc: stable@dpdk.org
> >
> > Reported-by: Andrew Rybchenko <arybchenko@solarflare.com>
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
>
> David,
>
> thanks a lot, it solves build problem.
>
> It does not solve experimental symbol inconsistency problem in
> markup and map file, but it is a separate less critical
> problem.

We have lived with this situation on experimental variables.
So yes, the priority is to fix compilation.

I will see if you can do/need something about the variables.


Thanks for the tests Andrew.


-- 
David Marchand

Re: [dpdk-dev] [PATCH 1/2] buildtools: fix experimental symbols listing

[Andrew Rybchenko]

On 11/25/19 11:10 AM, David Marchand wrote:
> The map-list-symbol.sh script displays the filename, section and symbol
> names of map files.
>
> Example:
> $ buildtools/map-list-symbol.sh -S EXPERIMENTAL \
>     lib/librte_ethdev/rte_ethdev_version.map |grep rte_mtr_create
> lib/librte_ethdev/rte_ethdev_version.map EXPERIMENTAL rte_mtr_create
>
> The experimental symbol check should only consider the symbol name.
>
> Fixes: 3290ac14eb94 ("buildtools: detect discrepancies for experimental symbols")
> Cc: stable@dpdk.org
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Tested-by: Andrew Rybchenko <arybchenko@solarflare.com>

Re: [dpdk-dev] [PATCH 1/2] buildtools: fix experimental symbols listing

[Neil Horman]

On Mon, Nov 25, 2019 at 09:10:06AM +0100, David Marchand wrote:
> The map-list-symbol.sh script displays the filename, section and symbol
> names of map files.
> 
> Example:
> $ buildtools/map-list-symbol.sh -S EXPERIMENTAL \
>     lib/librte_ethdev/rte_ethdev_version.map |grep rte_mtr_create
> lib/librte_ethdev/rte_ethdev_version.map EXPERIMENTAL rte_mtr_create
> 
> The experimental symbol check should only consider the symbol name.
> 
> Fixes: 3290ac14eb94 ("buildtools: detect discrepancies for experimental symbols")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  buildtools/check-experimental-syms.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/buildtools/check-experimental-syms.sh b/buildtools/check-experimental-syms.sh
> index 145dd70ebf..abebb89f12 100755
> --- a/buildtools/check-experimental-syms.sh
> +++ b/buildtools/check-experimental-syms.sh
> @@ -23,7 +23,7 @@ trap 'rm -f "$DUMPFILE"' EXIT
>  objdump -t $OBJFILE >$DUMPFILE
>  
>  ret=0
> -for SYM in `$LIST_SYMBOL -S EXPERIMENTAL $MAPFILE`
> +for SYM in `$LIST_SYMBOL -S EXPERIMENTAL $MAPFILE |cut -d ' ' -f 3`
>  do
>  	if grep -q "\.text.*$SYM$" $DUMPFILE &&
>  		! grep -q "\.text\.experimental.*$SYM$" $DUMPFILE
> -- 
> 2.23.0
> 
> 
Series
Acked-by: Neil Horman <nhorman@tuxdriver.com>

Re: [dpdk-dev] [PATCH 1/2] buildtools: fix experimental symbols listing

[David Marchand]

On Mon, Nov 25, 2019 at 2:26 PM Neil Horman <nhorman@tuxdriver.com> wrote:
>
> On Mon, Nov 25, 2019 at 09:10:06AM +0100, David Marchand wrote:
> > The map-list-symbol.sh script displays the filename, section and symbol
> > names of map files.
> >
> > Example:
> > $ buildtools/map-list-symbol.sh -S EXPERIMENTAL \
> >     lib/librte_ethdev/rte_ethdev_version.map |grep rte_mtr_create
> > lib/librte_ethdev/rte_ethdev_version.map EXPERIMENTAL rte_mtr_create
> >
> > The experimental symbol check should only consider the symbol name.
> >
> > Fixes: 3290ac14eb94 ("buildtools: detect discrepancies for experimental symbols")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> On 11/25/19 11:10 AM, David Marchand wrote:
> The map-list-symbol.sh script displays the filename, section and symbol
> names of map files.
>
> Example:
> $ buildtools/map-list-symbol.sh -S EXPERIMENTAL \
>     lib/librte_ethdev/rte_ethdev_version.map |grep rte_mtr_create
> lib/librte_ethdev/rte_ethdev_version.map EXPERIMENTAL rte_mtr_create
>
> The experimental symbol check should only consider the symbol name.
>
> Fixes: 3290ac14eb94 ("buildtools: detect discrepancies for experimental symbols")
> Cc: stable@dpdk.org
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Tested-by: Andrew Rybchenko <arybchenko@solarflare.com>

> Acked-by: Neil Horman <nhorman@tuxdriver.com>

Series applied.


--
David Marchand