[dpdk-dev] kni: Use utsrelease.h to determine Ubuntu kernel version

  /proc/version_signature is the version for the host machine, but in
e.g., chroots, this does not need to match that DPDK is built for. Use
utsrelease.h from the kernel sources instead and fake the upload
version.

Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
Signed-off-by: Johan Faltstrom <johan.faltstrom@netinsight.net>
---
 lib/librte_eal/linuxapp/kni/Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
  

Hi guys

Could you help to review the code changes where you modified before?

Regards,
Helin

> -----Original Message-----
> From: Simon Kagstrom [mailto:simon.kagstrom@netinsight.net]
> Sent: Wednesday, May 27, 2015 7:45 PM
> To: dev@dpdk.org; Zhang, Helin
> Subject: [PATCH] kni: Use utsrelease.h to determine Ubuntu kernel version
> 
> /proc/version_signature is the version for the host machine, but in e.g., chroots,
> this does not need to match that DPDK is built for. Use utsrelease.h from the
> kernel sources instead and fake the upload version.
> 
> Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
> Signed-off-by: Johan Faltstrom <johan.faltstrom@netinsight.net>
> ---
>  lib/librte_eal/linuxapp/kni/Makefile | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_eal/linuxapp/kni/Makefile
> b/lib/librte_eal/linuxapp/kni/Makefile
> index fb673d9..ac99d3f 100644
> --- a/lib/librte_eal/linuxapp/kni/Makefile
> +++ b/lib/librte_eal/linuxapp/kni/Makefile
> @@ -44,10 +44,10 @@ MODULE_CFLAGS += -I$(RTE_OUTPUT)/include
> -I$(SRCDIR)/ethtool/ixgbe -I$(SRCDIR)/e  MODULE_CFLAGS += -include
> $(RTE_OUTPUT)/include/rte_config.h
>  MODULE_CFLAGS += -Wall -Werror
> 
> -ifeq ($(shell test -f /proc/version_signature && lsb_release -si
> 2>/dev/null),Ubuntu)
> +ifeq ($(shell lsb_release -si 2>/dev/null),Ubuntu)
>  MODULE_CFLAGS += -DUBUNTU_RELEASE_CODE=$(shell lsb_release -sr | tr -d .)
> -UBUNTU_KERNEL_CODE := $(shell cut -d' ' -f2 /proc/version_signature | \
> -                        cut -d'~' -f1 | cut -d- -f1,2 | tr .- $(comma))
> +UBUNTU_KERNEL_CODE := $(shell echo `grep UTS_RELEASE
> $(RTE_KERNELDIR)/include/generated/utsrelease.h \
> +	 | cut -d '"' -f2 | cut -d- -f1,2 | tr .- $(comma)`,1)
>  MODULE_CFLAGS +=
> -D"UBUNTU_KERNEL_CODE=UBUNTU_KERNEL_VERSION($(UBUNTU_KERNEL_C
> ODE))"
>  endif
> 
> --
> 1.9.1
  

This ugly and fast becoming unmaintainable and ridiculous.

KNI needs to get out of being a bunch of out of tree code chasing the
kernel API tail lights
and become something stable and submitted upstream.

On Wed, May 27, 2015 at 4:45 AM, Simon Kagstrom <
simon.kagstrom@netinsight.net> wrote:

> /proc/version_signature is the version for the host machine, but in
> e.g., chroots, this does not need to match that DPDK is built for. Use
> utsrelease.h from the kernel sources instead and fake the upload
> version.
>
> Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
> Signed-off-by: Johan Faltstrom <johan.faltstrom@netinsight.net>
> ---
>  lib/librte_eal/linuxapp/kni/Makefile | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/lib/librte_eal/linuxapp/kni/Makefile
> b/lib/librte_eal/linuxapp/kni/Makefile
> index fb673d9..ac99d3f 100644
> --- a/lib/librte_eal/linuxapp/kni/Makefile
> +++ b/lib/librte_eal/linuxapp/kni/Makefile
> @@ -44,10 +44,10 @@ MODULE_CFLAGS += -I$(RTE_OUTPUT)/include
> -I$(SRCDIR)/ethtool/ixgbe -I$(SRCDIR)/e
>  MODULE_CFLAGS += -include $(RTE_OUTPUT)/include/rte_config.h
>  MODULE_CFLAGS += -Wall -Werror
>
> -ifeq ($(shell test -f /proc/version_signature && lsb_release -si
> 2>/dev/null),Ubuntu)
> +ifeq ($(shell lsb_release -si 2>/dev/null),Ubuntu)
>  MODULE_CFLAGS += -DUBUNTU_RELEASE_CODE=$(shell lsb_release -sr | tr -d .)
> -UBUNTU_KERNEL_CODE := $(shell cut -d' ' -f2 /proc/version_signature | \
> -                        cut -d'~' -f1 | cut -d- -f1,2 | tr .- $(comma))
> +UBUNTU_KERNEL_CODE := $(shell echo `grep UTS_RELEASE
> $(RTE_KERNELDIR)/include/generated/utsrelease.h \
> +        | cut -d '"' -f2 | cut -d- -f1,2 | tr .- $(comma)`,1)
>  MODULE_CFLAGS +=
> -D"UBUNTU_KERNEL_CODE=UBUNTU_KERNEL_VERSION($(UBUNTU_KERNEL_CODE))"
>  endif
>
> --
> 1.9.1
>
>
  

> >
> > -ifeq ($(shell test -f /proc/version_signature && lsb_release -si
> > 2>/dev/null),Ubuntu)
> > +ifeq ($(shell lsb_release -si 2>/dev/null),Ubuntu)
> >  MODULE_CFLAGS += -DUBUNTU_RELEASE_CODE=$(shell lsb_release -sr | tr -
> d .)
> > -UBUNTU_KERNEL_CODE := $(shell cut -d' ' -f2 /proc/version_signature | \
> > -                        cut -d'~' -f1 | cut -d- -f1,2 | tr .- $(comma))
> > +UBUNTU_KERNEL_CODE := $(shell echo `grep UTS_RELEASE
> > $(RTE_KERNELDIR)/include/generated/utsrelease.h \
> > +	 | cut -d '"' -f2 | cut -d- -f1,2 | tr .- $(comma)`,1)
> >  MODULE_CFLAGS +=
> > -
> D"UBUNTU_KERNEL_CODE=UBUNTU_KERNEL_VERSION($(UBUNTU_KERNEL_
> C
> > ODE))"
> >  endif
> >
> > --
> > 1.9.1
Hi,

It is fine for me if it do the job and does not break build on other OS (also other 
Ubuntu versions especially 12.04 if we still support it).
Please only check if UTS_RELEASE is available on all Ubuntu versions DPDK support.

Pawel
  

On 2015-05-28 12:05, Wodkowski, PawelX wrote:
>>>
>>> -UBUNTU_KERNEL_CODE := $(shell cut -d' ' -f2 /proc/version_signature | \
>>> -                        cut -d'~' -f1 | cut -d- -f1,2 | tr .- $(comma))
>>> +UBUNTU_KERNEL_CODE := $(shell echo `grep UTS_RELEASE
>>> $(RTE_KERNELDIR)/include/generated/utsrelease.h \
>>> +	 | cut -d '"' -f2 | cut -d- -f1,2 | tr .- $(comma)`,1)
> 
> It is fine for me if it do the job and does not break build on other OS (also other 
> Ubuntu versions especially 12.04 if we still support it).
> Please only check if UTS_RELEASE is available on all Ubuntu versions DPDK support.

From some digging, it appears it entered the kernel tree in 2006 and
moved to include/generated/ in 2009 so I guess that should be fine for
DPDK builds?

// Simon
  

> -----Original Message-----
> From: Simon Kågström [mailto:simon.kagstrom@netinsight.net]
> Sent: Thursday, May 28, 2015 12:37 PM
> To: Wodkowski, PawelX; Zhang, Helin; Alexander Guy; Julien Cretin; Buriez,
> Patrice
> Cc: dev@dpdk.org
> Subject: Re: [PATCH] kni: Use utsrelease.h to determine Ubuntu kernel version
> 
> On 2015-05-28 12:05, Wodkowski, PawelX wrote:
> >>>
> >>> -UBUNTU_KERNEL_CODE := $(shell cut -d' ' -f2 /proc/version_signature | \
> >>> -                        cut -d'~' -f1 | cut -d- -f1,2 | tr .- $(comma))
> >>> +UBUNTU_KERNEL_CODE := $(shell echo `grep UTS_RELEASE
> >>> $(RTE_KERNELDIR)/include/generated/utsrelease.h \
> >>> +	 | cut -d '"' -f2 | cut -d- -f1,2 | tr .- $(comma)`,1)
> >
> > It is fine for me if it do the job and does not break build on other OS (also other
> > Ubuntu versions especially 12.04 if we still support it).
> > Please only check if UTS_RELEASE is available on all Ubuntu versions DPDK
> support.
> 
> From some digging, it appears it entered the kernel tree in 2006 and
> moved to include/generated/ in 2009 so I guess that should be fine for
> DPDK builds?
> 
> // Simon

I also think that it is OK but I also think should check by building you (o ask
someone to do it for you)  on those systems not by theory :)
  

Hi all,
Please forgive top reply and bottom disclaimer.
Not sure anyway that this email will reach the mailing list, since I did not subscribe to it.

I am worried about the removal of: cut -d'~' -f1
It was introduced by Pawel in commit 35170c52d0ae33dc30e69bcf681e5a17168bf11e
http://dpdk.org/browse/dpdk/commit/lib/librte_eal/linuxapp/kni/Makefile?id=35170c52d0ae33dc30e69bcf681e5a17168bf11e
in order to fix the parsing of: 3.11.0-15.25~precise1-generic
Not sure what utsrelease.h would contain in this specific case, but removal of ~precise1-generic is broken with this recent patch.

Regards,
Patrice

-----Original Message-----
From: Wodkowski, PawelX 
Sent: Thursday, May 28, 2015 12:48 PM
To: Simon Kågström; Zhang, Helin; Alexander Guy; Julien Cretin; Buriez, Patrice
Cc: dev@dpdk.org
Subject: RE: [PATCH] kni: Use utsrelease.h to determine Ubuntu kernel version


> -----Original Message-----
> From: Simon Kågström [mailto:simon.kagstrom@netinsight.net]
> Sent: Thursday, May 28, 2015 12:37 PM
> To: Wodkowski, PawelX; Zhang, Helin; Alexander Guy; Julien Cretin; 
> Buriez, Patrice
> Cc: dev@dpdk.org
> Subject: Re: [PATCH] kni: Use utsrelease.h to determine Ubuntu kernel 
> version
> 
> On 2015-05-28 12:05, Wodkowski, PawelX wrote:
> >>>
> >>> -UBUNTU_KERNEL_CODE := $(shell cut -d' ' -f2 /proc/version_signature | \
> >>> -                        cut -d'~' -f1 | cut -d- -f1,2 | tr .- $(comma))
> >>> +UBUNTU_KERNEL_CODE := $(shell echo `grep UTS_RELEASE
> >>> $(RTE_KERNELDIR)/include/generated/utsrelease.h \
> >>> +	 | cut -d '"' -f2 | cut -d- -f1,2 | tr .- $(comma)`,1)
> >
> > It is fine for me if it do the job and does not break build on other 
> > OS (also other Ubuntu versions especially 12.04 if we still support it).
> > Please only check if UTS_RELEASE is available on all Ubuntu versions 
> > DPDK
> support.
> 
> From some digging, it appears it entered the kernel tree in 2006 and 
> moved to include/generated/ in 2009 so I guess that should be fine for 
> DPDK builds?
> 
> // Simon

I also think that it is OK but I also think should check by building you (o ask someone to do it for you)  on those systems not by theory :)

--
Pawel


Intel Corporation NV/SA
Kings Square, Veldkant 31
2550 Kontich
RPM (Bruxelles) 0415.497.718. 
Citibank, Brussels, account 570/1031255/09

This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
  

> -----Original Message-----
> From: Buriez, Patrice
> Sent: Thursday, May 28, 2015 1:07 PM
> To: Wodkowski, PawelX; Simon Kågström; Zhang, Helin; Alexander Guy; Julien
> Cretin
> Cc: dev@dpdk.org
> Subject: RE: [PATCH] kni: Use utsrelease.h to determine Ubuntu kernel version
> 
> Hi all,
> Please forgive top reply and bottom disclaimer.
> Not sure anyway that this email will reach the mailing list, since I did not
> subscribe to it.
> 
> I am worried about the removal of: cut -d'~' -f1
> It was introduced by Pawel in commit

You are absolutely right. That is why I asked to check with documentation
and to verify  with real build on all supported Ubuntu versions. :)
  

On 2015-05-28 12:48, Wodkowski, PawelX wrote:
>>> Please only check if UTS_RELEASE is available on all Ubuntu versions DPDK
>> support.
>>
>> From some digging, it appears it entered the kernel tree in 2006 and
>> moved to include/generated/ in 2009 so I guess that should be fine for
>> DPDK builds?
> 
> I also think that it is OK but I also think should check by building you (o ask
> someone to do it for you)  on those systems not by theory :)

Well, I think this is one of the main motivations from something like
what Thomas F. Herbert proposed in another thread recently,

  DPDK: Proposal for a patch patch-test integration tree

basically, a continuous-integration-type of system should test-build
(and probably test) any prospective patch to see that it builds for
various targets. In my view, this would be a perfect match for
github+travis-ci.


Anyway, I'll see if I can dig up an older Ubuntu to build on, unless
someone else steps up and tests the patch. (My issue to start with was
that the build fails on a 14.04 chroot on a 12.04 host, but I only have
access to the chroot there).

// Simon
  

2015-05-27 13:45, Simon Kagstrom:
> /proc/version_signature is the version for the host machine, but in
> e.g., chroots, this does not need to match that DPDK is built for. Use
> utsrelease.h from the kernel sources instead and fake the upload
> version.

Sorry, I don't really understand the problem.
Do you mean that /proc/version_signature is not readable from the chroot?
Or do you mean that the /proc in the chroot is the host one and it
doesn't match the OS installed in the chroot?

Please precise which case you try to solve and which tests you did
with which Ubuntu versions?
  

2015-06-16 23:09, Thomas Monjalon:
> 2015-05-27 13:45, Simon Kagstrom:
> > /proc/version_signature is the version for the host machine, but in
> > e.g., chroots, this does not need to match that DPDK is built for. Use
> > utsrelease.h from the kernel sources instead and fake the upload
> > version.
> 
> Sorry, I don't really understand the problem.
> Do you mean that /proc/version_signature is not readable from the chroot?
> Or do you mean that the /proc in the chroot is the host one and it
> doesn't match the OS installed in the chroot?
> 
> Please precise which case you try to solve and which tests you did
> with which Ubuntu versions?

The issue is still not clearly described.
Closed in patchwork.