[dpdk-dev] [PATCH] Pass verbose flag to kernel module

[dpdk-dev] [PATCH] Pass verbose flag to kernel module

[Sergio Gonzalez Monroy]

---
 mk/rte.module.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mk/rte.module.mk b/mk/rte.module.mk
index c4ca3fd..bd3c596 100644
--- a/mk/rte.module.mk
+++ b/mk/rte.module.mk
@@ -78,7 +78,7 @@ build: _postbuild
 $(MODULE).ko: $(SRCS_LINKS)
 	@if [ ! -f $(notdir Makefile) ]; then ln -nfs $(SRCDIR)/Makefile . ; fi
 	@$(MAKE) -C $(RTE_KERNELDIR) M=$(CURDIR) O=$(RTE_KERNELDIR) \
-		CROSS_COMPILE=$(CROSS)
+		V=$(if $(V),1,0) CROSS_COMPILE=$(CROSS)
 
 # install module in $(RTE_OUTPUT)/kmod
 $(RTE_OUTPUT)/kmod/$(MODULE).ko: $(MODULE).ko
-- 
1.9.3

Re: [dpdk-dev] [PATCH] Pass verbose flag to kernel module

[Thomas Monjalon]

Hi Sergio,

2014-10-06 17:09, Sergio Gonzalez Monroy:
> --- a/mk/rte.module.mk
> +++ b/mk/rte.module.mk
> @@ -78,7 +78,7 @@ build: _postbuild
>  $(MODULE).ko: $(SRCS_LINKS)
>  	@if [ ! -f $(notdir Makefile) ]; then ln -nfs $(SRCDIR)/Makefile . ; fi
>  	@$(MAKE) -C $(RTE_KERNELDIR) M=$(CURDIR) O=$(RTE_KERNELDIR) \
> -		CROSS_COMPILE=$(CROSS)
> +		V=$(if $(V),1,0) CROSS_COMPILE=$(CROSS)

Please could you explain why it is needed?
The variable V should be inherited by the recursive make.
It's working without your patch in my test.

-- 
Thomas

Re: [dpdk-dev] [PATCH] Pass verbose flag to kernel module

[Sergio Gonzalez Monroy]

On Wed, Oct 08, 2014 at 07:05:32PM +0200, Thomas Monjalon wrote:
> Hi Sergio,
> 
> 2014-10-06 17:09, Sergio Gonzalez Monroy:
> > --- a/mk/rte.module.mk
> > +++ b/mk/rte.module.mk
> > @@ -78,7 +78,7 @@ build: _postbuild
> >  $(MODULE).ko: $(SRCS_LINKS)
> >  	@if [ ! -f $(notdir Makefile) ]; then ln -nfs $(SRCDIR)/Makefile . ; fi
> >  	@$(MAKE) -C $(RTE_KERNELDIR) M=$(CURDIR) O=$(RTE_KERNELDIR) \
> > -		CROSS_COMPILE=$(CROSS)
> > +		V=$(if $(V),1,0) CROSS_COMPILE=$(CROSS)
> 
> Please could you explain why it is needed?
> The variable V should be inherited by the recursive make.
> It's working without your patch in my test.
> 
Hi Thomas,

You are right. If you set V=1 in the commmand line it will inherit and pass it down.
In the curent framework, we do not force V to be 1, just to be defined (coud be V=y
or V=enable, etc).
This patch was just forcing the value to be 1 as it is the required value for the
kernel makefiles.

It is not a big deal and we could approach this by specifiying on the docs to be V=1
or any other way you think more appropiate?

Thanks,
Sergio

> -- 
> Thomas

Re: [dpdk-dev] [PATCH] Pass verbose flag to kernel module

[Thomas Monjalon]

Hi Sergio,

2014-10-09 10:37, Sergio Gonzalez Monroy:
> On Wed, Oct 08, 2014 at 07:05:32PM +0200, Thomas Monjalon wrote:
> > Hi Sergio,
> > 
> > 2014-10-06 17:09, Sergio Gonzalez Monroy:
> > > --- a/mk/rte.module.mk
> > > +++ b/mk/rte.module.mk
> > > @@ -78,7 +78,7 @@ build: _postbuild
> > >  $(MODULE).ko: $(SRCS_LINKS)
> > >  	@if [ ! -f $(notdir Makefile) ]; then ln -nfs $(SRCDIR)/Makefile . ; fi
> > >  	@$(MAKE) -C $(RTE_KERNELDIR) M=$(CURDIR) O=$(RTE_KERNELDIR) \
> > > -		CROSS_COMPILE=$(CROSS)
> > > +		V=$(if $(V),1,0) CROSS_COMPILE=$(CROSS)
> > 
> > Please could you explain why it is needed?
> > The variable V should be inherited by the recursive make.
> > It's working without your patch in my test.
> 
> You are right. If you set V=1 in the commmand line it will inherit and pass it down.
> In the curent framework, we do not force V to be 1, just to be defined (coud be V=y
> or V=enable, etc).
> This patch was just forcing the value to be 1 as it is the required value for the
> kernel makefiles.
> 
> It is not a big deal and we could approach this by specifiying on the docs to be V=1
> or any other way you think more appropiate?

Oh OK, I didn't imagine passing other value to V ;)
It's better to fix makefile than doc. So I'm OK with this patch.

-- 
Thomas

Re: [dpdk-dev] [PATCH] Pass verbose flag to kernel module

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Sergio Gonzalez
> Monroy
> Sent: Monday, October 06, 2014 5:09 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] Pass verbose flag to kernel module
> 
> ---
>  mk/rte.module.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mk/rte.module.mk b/mk/rte.module.mk
> index c4ca3fd..bd3c596 100644
> --- a/mk/rte.module.mk
> +++ b/mk/rte.module.mk
> @@ -78,7 +78,7 @@ build: _postbuild
>  $(MODULE).ko: $(SRCS_LINKS)
>  	@if [ ! -f $(notdir Makefile) ]; then ln -nfs $(SRCDIR)/Makefile . ; fi
>  	@$(MAKE) -C $(RTE_KERNELDIR) M=$(CURDIR)
> O=$(RTE_KERNELDIR) \
> -		CROSS_COMPILE=$(CROSS)
> +		V=$(if $(V),1,0) CROSS_COMPILE=$(CROSS)
> 
>  # install module in $(RTE_OUTPUT)/kmod
>  $(RTE_OUTPUT)/kmod/$(MODULE).ko: $(MODULE).ko
> --
> 1.9.3

Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

Re: [dpdk-dev] [PATCH] Pass verbose flag to kernel module

[Thomas Monjalon]

Sergio,

I'd like to see a v2 patch with your explanations in commit log
and with a Signed-off-by.

> > -		CROSS_COMPILE=$(CROSS)
> > +		V=$(if $(V),1,0) CROSS_COMPILE=$(CROSS)

Minor nits:
- paren is not needed for one letter variable
- V option should go to the end to keep things sorted by importance

-               CROSS_COMPILE=$(CROSS)
+               CROSS_COMPILE=$(CROSS) V=$(if $V,1,0)

> Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

Please Pablo, do not give your ack if there is no Signed-off-by.
It's mandatory.

Thanks
-- 
Thomas

Re: [dpdk-dev] [PATCH] Pass verbose flag to kernel module

[Gonzalez Monroy, Sergio]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, October 15, 2014 1:05 PM
> To: De Lara Guarch, Pablo; Gonzalez Monroy, Sergio
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] Pass verbose flag to kernel module
> 
> Sergio,
> 
> I'd like to see a v2 patch with your explanations in commit log and with a
> Signed-off-by.
> 
My mistake, I will be more careful to check proper comments and signed-off.
V2 on the way.

> > > -		CROSS_COMPILE=$(CROSS)
> > > +		V=$(if $(V),1,0) CROSS_COMPILE=$(CROSS)
> 
> Minor nits:
> - paren is not needed for one letter variable


I don't mind doing this but a quick grep on the current code
will show more single letters with parenthesis than without.
I was just trying to be consistent with current style.

Thanks,
Sergio

> - V option should go to the end to keep things sorted by importance
> 
> -               CROSS_COMPILE=$(CROSS)
> +               CROSS_COMPILE=$(CROSS) V=$(if $V,1,0)
> 
> > Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> 
> Please Pablo, do not give your ack if there is no Signed-off-by.
> It's mandatory.
> 
> Thanks
> --
> Thomas

[dpdk-dev] [PATCH] Set V=1 when building kernel modules

[Sergio Gonzalez Monroy]

Linux kernel build system requires V=1 to enable verbose output, but
current DPDK framework just check if V is defined.

Fix: force V=1 when building Linux kernel modules if verbose output is
enabled.

Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
---
 mk/rte.module.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mk/rte.module.mk b/mk/rte.module.mk
index c4ca3fd..34a2ba9 100644
--- a/mk/rte.module.mk
+++ b/mk/rte.module.mk
@@ -78,7 +78,7 @@ build: _postbuild
 $(MODULE).ko: $(SRCS_LINKS)
 	@if [ ! -f $(notdir Makefile) ]; then ln -nfs $(SRCDIR)/Makefile . ; fi
 	@$(MAKE) -C $(RTE_KERNELDIR) M=$(CURDIR) O=$(RTE_KERNELDIR) \
-		CROSS_COMPILE=$(CROSS)
+		CROSS_COMPILE=$(CROSS) V=$(if $V,1,0) 
 
 # install module in $(RTE_OUTPUT)/kmod
 $(RTE_OUTPUT)/kmod/$(MODULE).ko: $(MODULE).ko
-- 
1.9.3

[dpdk-dev] [PATCH v2] Set V=1 when building kernel modules

[Sergio Gonzalez Monroy]

Linux kernel build system requires V=1 to enable verbose output, but
current DPDK framework just check if V is defined.

Fix: force V=1 when building Linux kernel modules if verbose output is
enabled.

Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
---
 mk/rte.module.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mk/rte.module.mk b/mk/rte.module.mk
index c4ca3fd..34a2ba9 100644
--- a/mk/rte.module.mk
+++ b/mk/rte.module.mk
@@ -78,7 +78,7 @@ build: _postbuild
 $(MODULE).ko: $(SRCS_LINKS)
 	@if [ ! -f $(notdir Makefile) ]; then ln -nfs $(SRCDIR)/Makefile . ; fi
 	@$(MAKE) -C $(RTE_KERNELDIR) M=$(CURDIR) O=$(RTE_KERNELDIR) \
-		CROSS_COMPILE=$(CROSS)
+		CROSS_COMPILE=$(CROSS) V=$(if $V,1,0) 
 
 # install module in $(RTE_OUTPUT)/kmod
 $(RTE_OUTPUT)/kmod/$(MODULE).ko: $(MODULE).ko
-- 
1.9.3

Re: [dpdk-dev] [PATCH] Set V=1 when building kernel modules

[Gonzalez Monroy, Sergio]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Sergio Gonzalez
> Monroy
> Sent: Wednesday, October 15, 2014 2:08 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] Set V=1 when building kernel modules
> 
Self-NACK.
Please ignore as I forgot to add v2 prefix.

Sergio

> Linux kernel build system requires V=1 to enable verbose output, but current
> DPDK framework just check if V is defined.
> 
> Fix: force V=1 when building Linux kernel modules if verbose output is
> enabled.
> 
> Signed-off-by: Sergio Gonzalez Monroy
> <sergio.gonzalez.monroy@intel.com>
> ---
>  mk/rte.module.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mk/rte.module.mk b/mk/rte.module.mk index c4ca3fd..34a2ba9
> 100644
> --- a/mk/rte.module.mk
> +++ b/mk/rte.module.mk
> @@ -78,7 +78,7 @@ build: _postbuild
>  $(MODULE).ko: $(SRCS_LINKS)
>  	@if [ ! -f $(notdir Makefile) ]; then ln -nfs $(SRCDIR)/Makefile . ; fi
>  	@$(MAKE) -C $(RTE_KERNELDIR) M=$(CURDIR)
> O=$(RTE_KERNELDIR) \
> -		CROSS_COMPILE=$(CROSS)
> +		CROSS_COMPILE=$(CROSS) V=$(if $V,1,0)
> 
>  # install module in $(RTE_OUTPUT)/kmod
>  $(RTE_OUTPUT)/kmod/$(MODULE).ko: $(MODULE).ko
> --
> 1.9.3

Re: [dpdk-dev] [PATCH v2] Set V=1 when building kernel modules

[Thomas Monjalon]

2014-10-15 14:08, Sergio Gonzalez Monroy:
> Linux kernel build system requires V=1 to enable verbose output, but
> current DPDK framework just check if V is defined.
> 
> Fix: force V=1 when building Linux kernel modules if verbose output is
> enabled.
> 
> Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>

Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Applied

Thanks
-- 
Thomas