[dpdk-dev] kni: fix use of undefined comma variable in makefile

  On 5/30/2016 12:56 PM, Olivier Matz wrote:
> The $(comma) variable is not defined in this Makefile, nor in
> any included Makefile. Seen while doing a "make clean" on ubuntu:
> 
>   $ make clean
>   == Clean lib
>   == Clean lib/librte_compat
>   == Clean lib/librte_eal
>   == Clean lib/librte_eal/common
>   == Clean lib/librte_eal/linuxapp
>   == Clean lib/librte_eal/linuxapp/eal
>   == Clean lib/librte_eal/linuxapp/igb_uio
>   == Clean lib/librte_eal/linuxapp/kni
>   tr: missing operand after ‘.-’
>   Two strings must be given when translating.
>   Try 'tr --help' for more information.

I don't observe this error on Ubuntu.

Also did a quick check and $(comma) seems defined, but not sure exactly
where. What I test:

$ lsb_release -si
Ubuntu


$ git diff


$ make -C lib/librte_eal/linuxapp/kni test_comma
make: Entering directory '.../dpdk/lib/librte_eal/linuxapp/kni'
= comma: , =
make: Leaving directory '.../dpdk/lib/librte_eal/linuxapp/kni'
  

Hi Ferruh,

On 05/30/2016 05:49 PM, Ferruh Yigit wrote:
> On 5/30/2016 12:56 PM, Olivier Matz wrote:
>> The $(comma) variable is not defined in this Makefile, nor in
>> any included Makefile. Seen while doing a "make clean" on ubuntu:
>>
>>   $ make clean
>>   == Clean lib
>>   == Clean lib/librte_compat
>>   == Clean lib/librte_eal
>>   == Clean lib/librte_eal/common
>>   == Clean lib/librte_eal/linuxapp
>>   == Clean lib/librte_eal/linuxapp/eal
>>   == Clean lib/librte_eal/linuxapp/igb_uio
>>   == Clean lib/librte_eal/linuxapp/kni
>>   tr: missing operand after ‘.-’
>>   Two strings must be given when translating.
>>   Try 'tr --help' for more information.
> 
> I don't observe this error on Ubuntu.
> 
> Also did a quick check and $(comma) seems defined, but not sure exactly
> where. What I test:
> 
> $ lsb_release -si
> Ubuntu
> 
> 
> $ git diff
> diff --git a/lib/librte_eal/linuxapp/kni/Makefile
> b/lib/librte_eal/linuxapp/kni/Makefile
> index ac99d3f..dcad241 100644
> --- a/lib/librte_eal/linuxapp/kni/Makefile
> +++ b/lib/librte_eal/linuxapp/kni/Makefile
> @@ -90,4 +90,7 @@ SRCS-y += kni_net.c
>  SRCS-y += kni_ethtool.c
>  SRCS-$(CONFIG_RTE_KNI_VHOST) += kni_vhost.c
> 
> +test_comma:
> +       @echo "= comma: $(comma) ="
> +
> 
> 
> 
> $ make -C lib/librte_eal/linuxapp/kni test_comma
> make: Entering directory '.../dpdk/lib/librte_eal/linuxapp/kni'
> = comma: , =
> make: Leaving directory '.../dpdk/lib/librte_eal/linuxapp/kni'

Thanks for testing and review.
I added the following lines to the makefile (just after the include
rte.vars.mk):

	$(info ------------------)
	$(info comma=$(comma))
	$(info $(origin comma))

And it gives me:

	$ make clean
	== Clean lib/librte_eal/linuxapp/kni
	--------------------
	comma=,
	file
	make -C /lib/modules/3.13.0-79-generic/build
M=/home/user/dpdk.org/build/build/lib/librte_eal/linuxapp/kni
O=/lib/modules/3.13.0-79-generic/build clean
	make -C /usr/src/linux-headers-3.13.0-79-generic \
        KBUILD_SRC=/usr/src/linux-headers-3.13.0-79-generic \

KBUILD_EXTMOD="/home/user/dpdk.org/build/build/lib/librte_eal/linuxapp/kni"
-f /usr/src/linux-headers-3.13.0-79-generic/Makefile \
	        clean
	make -f /usr/src/linux-headers-3.13.0-79-generic/scripts/Makefile.clean
obj=/home/user/dpdk.org/build/build/lib/librte_eal/linuxapp/kni
	--------------------
	comma=
	undefined
	tr: missing operand after ‘.-’
	Two strings must be given when translating.
	Try 'tr --help' for more information.
	[...]

Actually this Makefile is used twice, and second time the $(comma)
variable is not defined.

It seems the $(comma) variable is defined in rte.cpuflags.mk when
including rte.vars.mk:

  mk/rte.vars.mk
    mk/target/generic/rte.vars.mk
      mk/rte.cpuflags.mk    (only if KERNELRELEASE is unset, which is
                             not the case for the second call)


Do you see the issue when you do a "make clean"?

Thanks,
Olivier
  

On 5/30/2016 5:21 PM, Olivier Matz wrote:
> Hi Ferruh,
> 
> On 05/30/2016 05:49 PM, Ferruh Yigit wrote:
>> On 5/30/2016 12:56 PM, Olivier Matz wrote:
>>> The $(comma) variable is not defined in this Makefile, nor in
>>> any included Makefile. Seen while doing a "make clean" on ubuntu:
>>>
>>>   $ make clean
>>>   == Clean lib
>>>   == Clean lib/librte_compat
>>>   == Clean lib/librte_eal
>>>   == Clean lib/librte_eal/common
>>>   == Clean lib/librte_eal/linuxapp
>>>   == Clean lib/librte_eal/linuxapp/eal
>>>   == Clean lib/librte_eal/linuxapp/igb_uio
>>>   == Clean lib/librte_eal/linuxapp/kni
>>>   tr: missing operand after ‘.-’
>>>   Two strings must be given when translating.
>>>   Try 'tr --help' for more information.
>>
>> I don't observe this error on Ubuntu.
>>
>> Also did a quick check and $(comma) seems defined, but not sure exactly
>> where. What I test:
>>
>> $ lsb_release -si
>> Ubuntu
>>
>>
>> $ git diff
>> diff --git a/lib/librte_eal/linuxapp/kni/Makefile
>> b/lib/librte_eal/linuxapp/kni/Makefile
>> index ac99d3f..dcad241 100644
>> --- a/lib/librte_eal/linuxapp/kni/Makefile
>> +++ b/lib/librte_eal/linuxapp/kni/Makefile
>> @@ -90,4 +90,7 @@ SRCS-y += kni_net.c
>>  SRCS-y += kni_ethtool.c
>>  SRCS-$(CONFIG_RTE_KNI_VHOST) += kni_vhost.c
>>
>> +test_comma:
>> +       @echo "= comma: $(comma) ="
>> +
>>
>>
>>
>> $ make -C lib/librte_eal/linuxapp/kni test_comma
>> make: Entering directory '.../dpdk/lib/librte_eal/linuxapp/kni'
>> = comma: , =
>> make: Leaving directory '.../dpdk/lib/librte_eal/linuxapp/kni'
> 
> Thanks for testing and review.
> I added the following lines to the makefile (just after the include
> rte.vars.mk):
> 
> 	$(info ------------------)
> 	$(info comma=$(comma))
> 	$(info $(origin comma))
> 
> And it gives me:
> 
> 	$ make clean
> 	== Clean lib/librte_eal/linuxapp/kni
> 	--------------------
> 	comma=,
> 	file
> 	make -C /lib/modules/3.13.0-79-generic/build
> M=/home/user/dpdk.org/build/build/lib/librte_eal/linuxapp/kni
> O=/lib/modules/3.13.0-79-generic/build clean
> 	make -C /usr/src/linux-headers-3.13.0-79-generic \
>         KBUILD_SRC=/usr/src/linux-headers-3.13.0-79-generic \
> 
> KBUILD_EXTMOD="/home/user/dpdk.org/build/build/lib/librte_eal/linuxapp/kni"
> -f /usr/src/linux-headers-3.13.0-79-generic/Makefile \
> 	        clean
> 	make -f /usr/src/linux-headers-3.13.0-79-generic/scripts/Makefile.clean
> obj=/home/user/dpdk.org/build/build/lib/librte_eal/linuxapp/kni
> 	--------------------
> 	comma=
> 	undefined
> 	tr: missing operand after ‘.-’
> 	Two strings must be given when translating.
> 	Try 'tr --help' for more information.
> 	[...]
> 
> Actually this Makefile is used twice, and second time the $(comma)
> variable is not defined.
> 
> It seems the $(comma) variable is defined in rte.cpuflags.mk when
> including rte.vars.mk:
> 
>   mk/rte.vars.mk
>     mk/target/generic/rte.vars.mk
>       mk/rte.cpuflags.mk    (only if KERNELRELEASE is unset, which is
>                              not the case for the second call)
> 
> 
> Do you see the issue when you do a "make clean"?
> 

No issue on "make clean".

I did same modification as you did, for me comma defined for both times,
a system variable can be triggering the behavior perhaps.

What I got with "make clean" is:

...
== Clean lib/librte_eal/linuxapp/kni
------------
comma=,
file
make -C /lib/modules/4.4.0-22-generic/build
M=/home/ferruhy/development/dpdk/build/build/lib/librte_eal/linuxapp/kni
O=/lib/modules/4.4.0-22-generic/build clean
make -C /usr/src/linux-headers-4.4.0-22-generic
KBUILD_SRC=/usr/src/linux-headers-4.4.0-22-generic \
-f /usr/src/linux-headers-4.4.0-22-generic/Makefile clean
make -f /usr/src/linux-headers-4.4.0-22-generic/scripts/Makefile.clean
obj=/home/ferruhy/development/dpdk/build/build/lib/librte_eal/linuxapp/kni
------------
comma=,
file
  rm -rf
/home/ferruhy/development/dpdk/build/build/lib/librte_eal/linuxapp/kni/.tmp_versions
  rm -f
/home/ferruhy/development/dpdk/build/build/lib/librte_eal/linuxapp/kni/Module.symvers
== Clean lib/librte_eal/linuxapp/xen_dom0
...
  

On 5/30/2016 5:46 PM, Ferruh Yigit wrote:
> On 5/30/2016 5:21 PM, Olivier Matz wrote:
>> Hi Ferruh,
>>
>> On 05/30/2016 05:49 PM, Ferruh Yigit wrote:
>>> On 5/30/2016 12:56 PM, Olivier Matz wrote:
>>>> The $(comma) variable is not defined in this Makefile, nor in
>>>> any included Makefile. Seen while doing a "make clean" on ubuntu:
>>>>
>>>>   $ make clean
>>>>   == Clean lib
>>>>   == Clean lib/librte_compat
>>>>   == Clean lib/librte_eal
>>>>   == Clean lib/librte_eal/common
>>>>   == Clean lib/librte_eal/linuxapp
>>>>   == Clean lib/librte_eal/linuxapp/eal
>>>>   == Clean lib/librte_eal/linuxapp/igb_uio
>>>>   == Clean lib/librte_eal/linuxapp/kni
>>>>   tr: missing operand after ‘.-’
>>>>   Two strings must be given when translating.
>>>>   Try 'tr --help' for more information.
>>>
>>> I don't observe this error on Ubuntu.
>>>
>>> Also did a quick check and $(comma) seems defined, but not sure exactly
>>> where. What I test:
>>>
>>> $ lsb_release -si
>>> Ubuntu
>>>
>>>
>>> $ git diff
>>> diff --git a/lib/librte_eal/linuxapp/kni/Makefile
>>> b/lib/librte_eal/linuxapp/kni/Makefile
>>> index ac99d3f..dcad241 100644
>>> --- a/lib/librte_eal/linuxapp/kni/Makefile
>>> +++ b/lib/librte_eal/linuxapp/kni/Makefile
>>> @@ -90,4 +90,7 @@ SRCS-y += kni_net.c
>>>  SRCS-y += kni_ethtool.c
>>>  SRCS-$(CONFIG_RTE_KNI_VHOST) += kni_vhost.c
>>>
>>> +test_comma:
>>> +       @echo "= comma: $(comma) ="
>>> +
>>>
>>>
>>>
>>> $ make -C lib/librte_eal/linuxapp/kni test_comma
>>> make: Entering directory '.../dpdk/lib/librte_eal/linuxapp/kni'
>>> = comma: , =
>>> make: Leaving directory '.../dpdk/lib/librte_eal/linuxapp/kni'
>>
>> Thanks for testing and review.
>> I added the following lines to the makefile (just after the include
>> rte.vars.mk):
>>
>> 	$(info ------------------)
>> 	$(info comma=$(comma))
>> 	$(info $(origin comma))
>>
>> And it gives me:
>>
>> 	$ make clean
>> 	== Clean lib/librte_eal/linuxapp/kni
>> 	--------------------
>> 	comma=,
>> 	file
>> 	make -C /lib/modules/3.13.0-79-generic/build
>> M=/home/user/dpdk.org/build/build/lib/librte_eal/linuxapp/kni
>> O=/lib/modules/3.13.0-79-generic/build clean
>> 	make -C /usr/src/linux-headers-3.13.0-79-generic \
>>         KBUILD_SRC=/usr/src/linux-headers-3.13.0-79-generic \
>>
>> KBUILD_EXTMOD="/home/user/dpdk.org/build/build/lib/librte_eal/linuxapp/kni"
>> -f /usr/src/linux-headers-3.13.0-79-generic/Makefile \
>> 	        clean
>> 	make -f /usr/src/linux-headers-3.13.0-79-generic/scripts/Makefile.clean
>> obj=/home/user/dpdk.org/build/build/lib/librte_eal/linuxapp/kni
>> 	--------------------
>> 	comma=
>> 	undefined
>> 	tr: missing operand after ‘.-’
>> 	Two strings must be given when translating.
>> 	Try 'tr --help' for more information.
>> 	[...]
>>
>> Actually this Makefile is used twice, and second time the $(comma)
>> variable is not defined.
>>
>> It seems the $(comma) variable is defined in rte.cpuflags.mk when
>> including rte.vars.mk:
>>
>>   mk/rte.vars.mk
>>     mk/target/generic/rte.vars.mk
>>       mk/rte.cpuflags.mk    (only if KERNELRELEASE is unset, which is
>>                              not the case for the second call)
>>
>>
>> Do you see the issue when you do a "make clean"?
>>
> 
> No issue on "make clean".
> 
> I did same modification as you did, for me comma defined for both times,
> a system variable can be triggering the behavior perhaps.
> 
> What I got with "make clean" is:
> 
> ...
> == Clean lib/librte_eal/linuxapp/kni
> ------------
> comma=,
> file
> make -C /lib/modules/4.4.0-22-generic/build
> M=/home/ferruhy/development/dpdk/build/build/lib/librte_eal/linuxapp/kni
> O=/lib/modules/4.4.0-22-generic/build clean
> make -C /usr/src/linux-headers-4.4.0-22-generic
> KBUILD_SRC=/usr/src/linux-headers-4.4.0-22-generic \
> -f /usr/src/linux-headers-4.4.0-22-generic/Makefile clean
> make -f /usr/src/linux-headers-4.4.0-22-generic/scripts/Makefile.clean
> obj=/home/ferruhy/development/dpdk/build/build/lib/librte_eal/linuxapp/kni
> ------------
> comma=,
> file
>   rm -rf
> /home/ferruhy/development/dpdk/build/build/lib/librte_eal/linuxapp/kni/.tmp_versions
>   rm -f
> /home/ferruhy/development/dpdk/build/build/lib/librte_eal/linuxapp/kni/Module.symvers
> == Clean lib/librte_eal/linuxapp/xen_dom0
> ...
> 

Error is related to the kernel version, in second call "comma"
definition comes from kernel makefiles.

Following commit causes comma to be defined:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=371fdc77af44f4cb32475fd499e1d912ccc30890

So this is valid issue for old kernels, and using "," directly looks OK.

> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Acked-by Ferruh Yigit <ferruh.yigit@intel.com>
  

Hi Ferruh,

On 05/31/2016 03:35 PM, Ferruh Yigit wrote:
> Error is related to the kernel version, in second call "comma"
> definition comes from kernel makefiles.
> 
> Following commit causes comma to be defined:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=371fdc77af44f4cb32475fd499e1d912ccc30890
> 
> So this is valid issue for old kernels, and using "," directly looks OK.
> 
>> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> Acked-by Ferruh Yigit <ferruh.yigit@intel.com>

Thanks for investigating the issue!
Olivier
  

2016-05-31 14:35, Ferruh Yigit:
> On 5/30/2016 5:46 PM, Ferruh Yigit wrote:
> > On 5/30/2016 5:21 PM, Olivier Matz wrote:
> >> Hi Ferruh,
> >>
> >> On 05/30/2016 05:49 PM, Ferruh Yigit wrote:
> >>> On 5/30/2016 12:56 PM, Olivier Matz wrote:
> >>>> The $(comma) variable is not defined in this Makefile, nor in
> >>>> any included Makefile. Seen while doing a "make clean" on ubuntu:
> >>>>
> >>>>   $ make clean
> >>>>   == Clean lib
> >>>>   == Clean lib/librte_compat
> >>>>   == Clean lib/librte_eal
> >>>>   == Clean lib/librte_eal/common
> >>>>   == Clean lib/librte_eal/linuxapp
> >>>>   == Clean lib/librte_eal/linuxapp/eal
> >>>>   == Clean lib/librte_eal/linuxapp/igb_uio
> >>>>   == Clean lib/librte_eal/linuxapp/kni
> >>>>   tr: missing operand after ‘.-’
> >>>>   Two strings must be given when translating.
> >>>>   Try 'tr --help' for more information.
> 
> Error is related to the kernel version, in second call "comma"
> definition comes from kernel makefiles.
> 
> Following commit causes comma to be defined:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=371fdc77af44f4cb32475fd499e1d912ccc30890
> 
> So this is valid issue for old kernels, and using "," directly looks OK.
> 
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> Acked-by Ferruh Yigit <ferruh.yigit@intel.com>

Applied, thanks