diff mbox series

[v2,04/11] examples/l2fwd-crypto: fix build with pkg-config

Message ID 20201113122430.25354-5-david.marchand@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers show
Series Examples compilation fixes | expand

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

David Marchand Nov. 13, 2020, 12:24 p.m. UTC
Two issues fixed here.

First add the experimental flag.
Then fix a link issue with the crypto scheduler driver:
/usr/bin/ld: /tmp/cchr7aHA.o: in function `main':
main.c:(.text.startup+0x1673): undefined reference to
`rte_cryptodev_scheduler_workers_get'
collect2: error: ld returned 1 exit status

Fixes: e3bcb99a5e13 ("examples/l2fwd-crypto: limit number of sessions")
Fixes: 261bbff75e34 ("examples: use separate crypto session mempools")

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 examples/l2fwd-crypto/Makefile | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Bruce Richardson Nov. 13, 2020, 1:53 p.m. UTC | #1
On Fri, Nov 13, 2020 at 01:24:23PM +0100, David Marchand wrote:
> Two issues fixed here.
> 
> First add the experimental flag.
> Then fix a link issue with the crypto scheduler driver:
> /usr/bin/ld: /tmp/cchr7aHA.o: in function `main':
> main.c:(.text.startup+0x1673): undefined reference to
> `rte_cryptodev_scheduler_workers_get'
> collect2: error: ld returned 1 exit status
> 
> Fixes: e3bcb99a5e13 ("examples/l2fwd-crypto: limit number of sessions")
> Fixes: 261bbff75e34 ("examples: use separate crypto session mempools")
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  examples/l2fwd-crypto/Makefile | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/examples/l2fwd-crypto/Makefile b/examples/l2fwd-crypto/Makefile
> index 4953ee2b95..211a076699 100644
> --- a/examples/l2fwd-crypto/Makefile
> +++ b/examples/l2fwd-crypto/Makefile
> @@ -24,8 +24,14 @@ PKGCONF ?= pkg-config
>  PC_FILE := $(shell $(PKGCONF) --path libdpdk 2>/dev/null)
>  CFLAGS += -O3 $(shell $(PKGCONF) --cflags libdpdk)
>  LDFLAGS_SHARED = $(shell $(PKGCONF) --libs libdpdk)
> +NEED_CRYPTO_SCHEDULER = $(shell echo RTE_CRYPTO_SCHEDULER | $(CPP) $(CFLAGS) -P - | tail -1)
> +ifeq ($(NEED_CRYPTO_SCHEDULER), 1)

Sorry for the last-minute comment, but I wonder for this check if we can do
better by adding into each makefile something like:

CONFIG_DEFINES=$(shell $(CC) $(CFLAGS) -dM -E - < /dev/null)

Then we can easily do multiple checks for vars as needed using findstring,
e.g.

ifeq ($(findstring RTE_CRYPTO_SCHEDULER,$(CONFIG_DEFINES),)
$(info No crypto scheduler found)
else
...
endif

Whatever approach we use here, I'd like applicable across all makefiles for
consistency, and shelling out per-value seems wasteful. Pulling all macro
values also allows checks for architecture and instruction set levels too,
if so desired.

/Bruce

> +LDFLAGS_SHARED += -lrte_crypto_scheduler
> +endif
>  LDFLAGS_STATIC = $(shell $(PKGCONF) --static --libs libdpdk)
>  
> +CFLAGS += -DALLOW_EXPERIMENTAL_API
> +
>  build/$(APP)-shared: $(SRCS-y) Makefile $(PC_FILE) | build
>  	$(CC) $(CFLAGS) $(SRCS-y) -o $@ $(LDFLAGS) $(LDFLAGS_SHARED)
>  
> -- 
> 2.23.0
>
David Marchand Nov. 13, 2020, 2:15 p.m. UTC | #2
On Fri, Nov 13, 2020 at 2:53 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
> > +NEED_CRYPTO_SCHEDULER = $(shell echo RTE_CRYPTO_SCHEDULER | $(CPP) $(CFLAGS) -P - | tail -1)
> > +ifeq ($(NEED_CRYPTO_SCHEDULER), 1)
>
> Sorry for the last-minute comment, but I wonder for this check if we can do
> better by adding into each makefile something like:
>
> CONFIG_DEFINES=$(shell $(CC) $(CFLAGS) -dM -E - < /dev/null)
>
> Then we can easily do multiple checks for vars as needed using findstring,
> e.g.
>
> ifeq ($(findstring RTE_CRYPTO_SCHEDULER,$(CONFIG_DEFINES),)
> $(info No crypto scheduler found)
> else
> ...
> endif
>
> Whatever approach we use here, I'd like applicable across all makefiles for
> consistency, and shelling out per-value seems wasteful. Pulling all macro
> values also allows checks for architecture and instruction set levels too,
> if so desired.

I'll have a try..
David Marchand Nov. 13, 2020, 3:41 p.m. UTC | #3
On Fri, Nov 13, 2020 at 3:15 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> On Fri, Nov 13, 2020 at 2:53 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> > > +NEED_CRYPTO_SCHEDULER = $(shell echo RTE_CRYPTO_SCHEDULER | $(CPP) $(CFLAGS) -P - | tail -1)
> > > +ifeq ($(NEED_CRYPTO_SCHEDULER), 1)
> >
> > Sorry for the last-minute comment, but I wonder for this check if we can do
> > better by adding into each makefile something like:
> >
> > CONFIG_DEFINES=$(shell $(CC) $(CFLAGS) -dM -E - < /dev/null)
> >
> > Then we can easily do multiple checks for vars as needed using findstring,
> > e.g.
> >
> > ifeq ($(findstring RTE_CRYPTO_SCHEDULER,$(CONFIG_DEFINES),)
> > $(info No crypto scheduler found)
> > else
> > ...
> > endif
> >
> > Whatever approach we use here, I'd like applicable across all makefiles for
> > consistency, and shelling out per-value seems wasteful. Pulling all macro
> > values also allows checks for architecture and instruction set levels too,
> > if so desired.

--- a/examples/l2fwd-crypto/Makefile
+++ b/examples/l2fwd-crypto/Makefile
@@ -23,9 +23,15 @@ PKGCONF ?= pkg-config

 PC_FILE := $(shell $(PKGCONF) --path libdpdk 2>/dev/null)
 CFLAGS += -O3 $(shell $(PKGCONF) --cflags libdpdk)
+CONFIG_DEFINES = $(shell $(CC) $(CFLAGS) -dM -E - < /dev/null)
 LDFLAGS_SHARED = $(shell $(PKGCONF) --libs libdpdk)
+ifneq ($(findstring RTE_CRYPTO_SCHEDULER,$(CONFIG_DEFINES)),)
+LDFLAGS_SHARED += -lrte_crypto_scheduler
+endif
 LDFLAGS_STATIC = $(shell $(PKGCONF) --static --libs libdpdk)

+CFLAGS += -DALLOW_EXPERIMENTAL_API
+
 build/$(APP)-shared: $(SRCS-y) Makefile $(PC_FILE) | build
        $(CC) $(CFLAGS) $(SRCS-y) -o $@ $(LDFLAGS) $(LDFLAGS_SHARED)

I'll send a v3 later.
Bruce Richardson Nov. 13, 2020, 4:08 p.m. UTC | #4
On Fri, Nov 13, 2020 at 04:41:52PM +0100, David Marchand wrote:
> On Fri, Nov 13, 2020 at 3:15 PM David Marchand
> <david.marchand@redhat.com> wrote:
> >
> > On Fri, Nov 13, 2020 at 2:53 PM Bruce Richardson
> > <bruce.richardson@intel.com> wrote:
> > > > +NEED_CRYPTO_SCHEDULER = $(shell echo RTE_CRYPTO_SCHEDULER | $(CPP) $(CFLAGS) -P - | tail -1)
> > > > +ifeq ($(NEED_CRYPTO_SCHEDULER), 1)
> > >
> > > Sorry for the last-minute comment, but I wonder for this check if we can do
> > > better by adding into each makefile something like:
> > >
> > > CONFIG_DEFINES=$(shell $(CC) $(CFLAGS) -dM -E - < /dev/null)
> > >
> > > Then we can easily do multiple checks for vars as needed using findstring,
> > > e.g.
> > >
> > > ifeq ($(findstring RTE_CRYPTO_SCHEDULER,$(CONFIG_DEFINES),)
> > > $(info No crypto scheduler found)
> > > else
> > > ...
> > > endif
> > >
> > > Whatever approach we use here, I'd like applicable across all makefiles for
> > > consistency, and shelling out per-value seems wasteful. Pulling all macro
> > > values also allows checks for architecture and instruction set levels too,
> > > if so desired.
> 
> --- a/examples/l2fwd-crypto/Makefile
> +++ b/examples/l2fwd-crypto/Makefile
> @@ -23,9 +23,15 @@ PKGCONF ?= pkg-config
> 
>  PC_FILE := $(shell $(PKGCONF) --path libdpdk 2>/dev/null)
>  CFLAGS += -O3 $(shell $(PKGCONF) --cflags libdpdk)
> +CONFIG_DEFINES = $(shell $(CC) $(CFLAGS) -dM -E - < /dev/null)
>  LDFLAGS_SHARED = $(shell $(PKGCONF) --libs libdpdk)
> +ifneq ($(findstring RTE_CRYPTO_SCHEDULER,$(CONFIG_DEFINES)),)
> +LDFLAGS_SHARED += -lrte_crypto_scheduler
> +endif
>  LDFLAGS_STATIC = $(shell $(PKGCONF) --static --libs libdpdk)
> 

Can we perhaps keep the existing pkg-config query lines together in the
existing block, and just add the extra checks for this particular app
afterwards? Having done changes en-mass to the example makefiles a couple
of times, it was made a lot easier by having the exact same lines in each
as much as possible, allowing changes to be made by applying a single
patch-file to each makefile in turn.
diff mbox series

Patch

diff --git a/examples/l2fwd-crypto/Makefile b/examples/l2fwd-crypto/Makefile
index 4953ee2b95..211a076699 100644
--- a/examples/l2fwd-crypto/Makefile
+++ b/examples/l2fwd-crypto/Makefile
@@ -24,8 +24,14 @@  PKGCONF ?= pkg-config
 PC_FILE := $(shell $(PKGCONF) --path libdpdk 2>/dev/null)
 CFLAGS += -O3 $(shell $(PKGCONF) --cflags libdpdk)
 LDFLAGS_SHARED = $(shell $(PKGCONF) --libs libdpdk)
+NEED_CRYPTO_SCHEDULER = $(shell echo RTE_CRYPTO_SCHEDULER | $(CPP) $(CFLAGS) -P - | tail -1)
+ifeq ($(NEED_CRYPTO_SCHEDULER), 1)
+LDFLAGS_SHARED += -lrte_crypto_scheduler
+endif
 LDFLAGS_STATIC = $(shell $(PKGCONF) --static --libs libdpdk)
 
+CFLAGS += -DALLOW_EXPERIMENTAL_API
+
 build/$(APP)-shared: $(SRCS-y) Makefile $(PC_FILE) | build
 	$(CC) $(CFLAGS) $(SRCS-y) -o $@ $(LDFLAGS) $(LDFLAGS_SHARED)