[dpdk-dev,v2] examples/ip_pipeline: fix freebsd build error

Message ID 20180410095132.130232-1-jasvinder.singh@intel.com (mailing list archive)
State Accepted, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Jasvinder Singh April 10, 2018, 9:51 a.m. UTC
  IP_Pipeline app is not supported in FreeBSD environment. Therefore,
skip it while building the sample apps on FreeBSD.

Fixes: 4bbf8e30aa5e ("examples/ip_pipeline: add CLI interface")
Fixes: 2f74ae28e23f ("examples/ip_pipeline: add tap object")

Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
---
 examples/ip_pipeline/Makefile | 8 ++++++++
 1 file changed, 8 insertions(+)
  

Comments

Thomas Monjalon April 10, 2018, 10:15 a.m. UTC | #1
10/04/2018 11:51, Jasvinder Singh:
> IP_Pipeline app is not supported in FreeBSD environment. Therefore,
> skip it while building the sample apps on FreeBSD.
[...]
> --- a/examples/ip_pipeline/Makefile
> +++ b/examples/ip_pipeline/Makefile
> @@ -67,6 +67,13 @@ RTE_TARGET ?= x86_64-native-linuxapp-gcc
>  
>  include $(RTE_SDK)/mk/rte.vars.mk
>  
> +ifneq ($(CONFIG_RTE_EXEC_ENV),"linuxapp")

You can use CONFIG_RTE_EXEC_ENV_LINUXAPP

> +$(info This application can only operate in a linuxapp environment, \
> +please change the definition of the RTE_TARGET environment variable)
> +all:
> +clean:
> +else


Copy of my previous comment:
"
It is not skipping the example, just throwing an error (which is needed).

You should add a test in examples/Makefile to really skip it.
"

In this v2, you replaced the error by an info message.
Error was better in my opinion.
  
Jasvinder Singh April 10, 2018, 10:25 a.m. UTC | #2
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Tuesday, April 10, 2018 11:15 AM
> To: Singh, Jasvinder <jasvinder.singh@intel.com>
> Cc: dev@dpdk.org; Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2] examples/ip_pipeline: fix freebsd build
> error
> 
> 10/04/2018 11:51, Jasvinder Singh:
> > IP_Pipeline app is not supported in FreeBSD environment. Therefore,
> > skip it while building the sample apps on FreeBSD.
> [...]
> > --- a/examples/ip_pipeline/Makefile
> > +++ b/examples/ip_pipeline/Makefile
> > @@ -67,6 +67,13 @@ RTE_TARGET ?= x86_64-native-linuxapp-gcc
> >
> >  include $(RTE_SDK)/mk/rte.vars.mk
> >
> > +ifneq ($(CONFIG_RTE_EXEC_ENV),"linuxapp")
> 
> You can use CONFIG_RTE_EXEC_ENV_LINUXAPP
> 
> > +$(info This application can only operate in a linuxapp environment, \
> > +please change the definition of the RTE_TARGET environment variable)
> > +all:
> > +clean:
> > +else
> 
> 
> Copy of my previous comment:
> "
> It is not skipping the example, just throwing an error (which is needed).
> 
> You should add a test in examples/Makefile to really skip it.
> "

In V2, we follow the same pattern as followed in other examples such as l3fwd-power, ethtool, etc.

If we put check in examples/Makefile, than how the situation is handled if someone tries to build ip_pipeline straight instead of building all examples. 

> In this v2, you replaced the error by an info message.
> Error was better in my opinion.
> 
>
  
Thomas Monjalon April 10, 2018, 12:19 p.m. UTC | #3
10/04/2018 12:25, Singh, Jasvinder:
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > 10/04/2018 11:51, Jasvinder Singh:
> > > IP_Pipeline app is not supported in FreeBSD environment. Therefore,
> > > skip it while building the sample apps on FreeBSD.
> > [...]
> > > --- a/examples/ip_pipeline/Makefile
> > > +++ b/examples/ip_pipeline/Makefile
> > > @@ -67,6 +67,13 @@ RTE_TARGET ?= x86_64-native-linuxapp-gcc
> > >
> > >  include $(RTE_SDK)/mk/rte.vars.mk
> > >
> > > +ifneq ($(CONFIG_RTE_EXEC_ENV),"linuxapp")
> > 
> > You can use CONFIG_RTE_EXEC_ENV_LINUXAPP
> > 
> > > +$(info This application can only operate in a linuxapp environment, \
> > > +please change the definition of the RTE_TARGET environment variable)
> > > +all:
> > > +clean:
> > > +else
> > 
> > 
> > Copy of my previous comment:
> > "
> > It is not skipping the example, just throwing an error (which is needed).
> > 
> > You should add a test in examples/Makefile to really skip it.
> > "
> 
> In V2, we follow the same pattern as followed in other examples such as l3fwd-power, ethtool, etc.

A kind of counter example is:
	ifeq ($(CONFIG_RTE_ARCH_X86_64),y)
	DIRS-y += performance-thread
	endif

But I get your point that it is done this way in several examples.

> If we put check in examples/Makefile, than how the situation is handled if someone tries to build ip_pipeline straight instead of building all examples.

My thought is that if you try to build explicitly this example on FreeBSD,
you should get an error.

> > In this v2, you replaced the error by an info message.
> > Error was better in my opinion.

I will get this v2 and not bother you more about that.
All examples could be fixed at once. Not sure it is worth though,
given this is the legacy build system.
  
Bruce Richardson April 10, 2018, 12:39 p.m. UTC | #4
On Tue, Apr 10, 2018 at 02:19:38PM +0200, Thomas Monjalon wrote:
> 10/04/2018 12:25, Singh, Jasvinder:
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > 10/04/2018 11:51, Jasvinder Singh:
> > > > IP_Pipeline app is not supported in FreeBSD environment. Therefore,
> > > > skip it while building the sample apps on FreeBSD.
> > > [...]
> > > > --- a/examples/ip_pipeline/Makefile +++
> > > > b/examples/ip_pipeline/Makefile @@ -67,6 +67,13 @@ RTE_TARGET ?=
> > > > x86_64-native-linuxapp-gcc
> > > >
> > > >  include $(RTE_SDK)/mk/rte.vars.mk
> > > >
> > > > +ifneq ($(CONFIG_RTE_EXEC_ENV),"linuxapp")
> > > 
> > > You can use CONFIG_RTE_EXEC_ENV_LINUXAPP
> > > 
> > > > +$(info This application can only operate in a linuxapp
> > > > environment, \ +please change the definition of the RTE_TARGET
> > > > environment variable) +all: +clean: +else
> > > 
> > > 
> > > Copy of my previous comment: " It is not skipping the example, just
> > > throwing an error (which is needed).
> > > 
> > > You should add a test in examples/Makefile to really skip it.  "
> > 
> > In V2, we follow the same pattern as followed in other examples such as
> > l3fwd-power, ethtool, etc.
> 
> A kind of counter example is: ifeq ($(CONFIG_RTE_ARCH_X86_64),y) DIRS-y
> += performance-thread endif
> 
> But I get your point that it is done this way in several examples.
> 
> > If we put check in examples/Makefile, than how the situation is handled
> > if someone tries to build ip_pipeline straight instead of building all
> > examples.
> 
> My thought is that if you try to build explicitly this example on
> FreeBSD, you should get an error.
> 
> > > In this v2, you replaced the error by an info message.  Error was
> > > better in my opinion.
> 
> I will get this v2 and not bother you more about that.  All examples
> could be fixed at once. Not sure it is worth though, given this is the
> legacy build system.
> 

Well, we still need a make-based solution for the example apps, because if
we using meson & ninja for the main DPDK build, we still need to provide a
makefile for each sample app. That makefile should build each app
standalone using the pkg-config info to find DPDK libs. It also needs to
signal to the user when they are trying to compile the app on an
unsupported platform.

The alternative, I suppose, is not to have unsupported examples shipped for
different platforms, but I think that is not as good a solution.

/Bruce
  
Thomas Monjalon April 10, 2018, 12:40 p.m. UTC | #5
10/04/2018 11:51, Jasvinder Singh:
> IP_Pipeline app is not supported in FreeBSD environment. Therefore,
> skip it while building the sample apps on FreeBSD.
> 
> Fixes: 4bbf8e30aa5e ("examples/ip_pipeline: add CLI interface")
> Fixes: 2f74ae28e23f ("examples/ip_pipeline: add tap object")
> 
> Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>

Applied, thanks
  

Patch

diff --git a/examples/ip_pipeline/Makefile b/examples/ip_pipeline/Makefile
index c936d1e..11d2b35 100644
--- a/examples/ip_pipeline/Makefile
+++ b/examples/ip_pipeline/Makefile
@@ -67,6 +67,13 @@  RTE_TARGET ?= x86_64-native-linuxapp-gcc
 
 include $(RTE_SDK)/mk/rte.vars.mk
 
+ifneq ($(CONFIG_RTE_EXEC_ENV),"linuxapp")
+$(info This application can only operate in a linuxapp environment, \
+please change the definition of the RTE_TARGET environment variable)
+all:
+clean:
+else
+
 INC += $(sort $(wildcard *.h))
 
 SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) := $(SRCS-y)
@@ -79,3 +86,4 @@  CFLAGS += $(WERROR_FLAGS)
 include $(RTE_SDK)/mk/rte.extapp.mk
 
 endif
+endif