* Re: [dpdk-dev] [PATCH v2] kni: Use utsrelease.h to determine Ubuntu kernel version
2015-08-20 6:51 [dpdk-dev] [PATCH v2] kni: Use utsrelease.h to determine Ubuntu kernel version Simon Kagstrom
@ 2015-10-13 12:50 ` Simon Kagstrom
2015-10-13 13:06 ` Tom Ghyselinck
2015-11-04 7:53 ` Simon Kågström
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Simon Kagstrom @ 2015-10-13 12:50 UTC (permalink / raw)
To: dev, olivier.matz
Ping?
// Simon
On Thu, 20 Aug 2015 08:51:06 +0200
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 necessarily match that DPDK is built
> for. DPDK will then build for the wrong kernel version - that of the
> server, and not that installed in the (build) chroot.
>
> The patch uses utsrelease.h from the kernel sources instead and fakes
> the upload version.
>
> Tested on a server with Ubuntu 12.04, building in a chroot for Ubuntu
> 14.04.
>
> Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
> Signed-off-by: Johan Faltstrom <johan.faltstrom@netinsight.net>
> ---
> ChangeLog:
>
> v2: Improve description and motivation for the patch.
>
> 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
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [dpdk-dev] [PATCH v2] kni: Use utsrelease.h to determine Ubuntu kernel version
2015-10-13 12:50 ` Simon Kagstrom
@ 2015-10-13 13:06 ` Tom Ghyselinck
0 siblings, 0 replies; 10+ messages in thread
From: Tom Ghyselinck @ 2015-10-13 13:06 UTC (permalink / raw)
To: dev
Hi Simon,
I'm looking forward to this patch since we also build the DPDK for
kernel versions which differ from the host kernel.
IMHO, the check for "Ubuntu" also needs change since this is also a
"host system" check instead of "target system" check.
I.e. A user may want to build for Ubuntu on a non-Ubuntu system and
vice versa.
With best regards,
Tom.
--
Tom Ghyselinck <tom.ghyselinck@excentis.com>
Excentis N.V.
On di, 2015-10-13 at 14:50 +0200, Simon Kagstrom wrote:
> Ping?
>
> // Simon
>
> On Thu, 20 Aug 2015 08:51:06 +0200
> 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 necessarily match that DPDK is built
> > for. DPDK will then build for the wrong kernel version - that of
> > the
> > server, and not that installed in the (build) chroot.
> >
> > The patch uses utsrelease.h from the kernel sources instead and
> > fakes
> > the upload version.
> >
> > Tested on a server with Ubuntu 12.04, building in a chroot for
> > Ubuntu
> > 14.04.
> >
> > Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
> > Signed-off-by: Johan Faltstrom <johan.faltstrom@netinsight.net>
> > ---
> > ChangeLog:
> >
> > v2: Improve description and motivation for the patch.
> >
> > 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
> >
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-dev] [PATCH v2] kni: Use utsrelease.h to determine Ubuntu kernel version
2015-08-20 6:51 [dpdk-dev] [PATCH v2] kni: Use utsrelease.h to determine Ubuntu kernel version Simon Kagstrom
2015-10-13 12:50 ` Simon Kagstrom
@ 2015-11-04 7:53 ` Simon Kågström
2015-11-04 8:30 ` Zhang, Helin
2015-11-04 10:35 ` Thomas Monjalon
3 siblings, 0 replies; 10+ messages in thread
From: Simon Kågström @ 2015-11-04 7:53 UTC (permalink / raw)
To: helin.zhang, thomas.monjalon, dev; +Cc: patrice.buriez, pawelx.wodkowski
Ping?
(Also including Stephen, Patrice and Pawel which has had comments on an
earlier iteration of this patch).
// Simon
On 2015-08-20 08:51, Simon Kagstrom wrote:
> /proc/version_signature is the version for the host machine, but in
> e.g., chroots, this does not necessarily match that DPDK is built
> for. DPDK will then build for the wrong kernel version - that of the
> server, and not that installed in the (build) chroot.
>
> The patch uses utsrelease.h from the kernel sources instead and fakes
> the upload version.
>
> Tested on a server with Ubuntu 12.04, building in a chroot for Ubuntu
> 14.04.
>
> Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
> Signed-off-by: Johan Faltstrom <johan.faltstrom@netinsight.net>
> ---
> ChangeLog:
>
> v2: Improve description and motivation for the patch.
>
> 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
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-dev] [PATCH v2] kni: Use utsrelease.h to determine Ubuntu kernel version
2015-08-20 6:51 [dpdk-dev] [PATCH v2] kni: Use utsrelease.h to determine Ubuntu kernel version Simon Kagstrom
2015-10-13 12:50 ` Simon Kagstrom
2015-11-04 7:53 ` Simon Kågström
@ 2015-11-04 8:30 ` Zhang, Helin
2015-11-19 9:08 ` Thomas Monjalon
2015-11-04 10:35 ` Thomas Monjalon
3 siblings, 1 reply; 10+ messages in thread
From: Zhang, Helin @ 2015-11-04 8:30 UTC (permalink / raw)
To: Simon Kagstrom, thomas.monjalon; +Cc: dev
> -----Original Message-----
> From: Simon Kagstrom [mailto:simon.kagstrom@netinsight.net]
> Sent: Thursday, August 20, 2015 2:51 PM
> To: Zhang, Helin; thomas.monjalon@6wind.com; dev@dpdk.org
> Subject: [PATCH v2] 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 necessarily match that DPDK is built for. DPDK will then build for the
> wrong kernel version - that of the server, and not that installed in the (build)
> chroot.
>
> The patch uses utsrelease.h from the kernel sources instead and fakes the
> upload version.
>
> Tested on a server with Ubuntu 12.04, building in a chroot for Ubuntu 14.04.
>
> Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
> Signed-off-by: Johan Faltstrom <johan.faltstrom@netinsight.net>
Acked-by: Helin Zhang <helin.zhang@intel.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-dev] [PATCH v2] kni: Use utsrelease.h to determine Ubuntu kernel version
2015-11-04 8:30 ` Zhang, Helin
@ 2015-11-19 9:08 ` Thomas Monjalon
0 siblings, 0 replies; 10+ messages in thread
From: Thomas Monjalon @ 2015-11-19 9:08 UTC (permalink / raw)
To: Simon Kagstrom; +Cc: dev
> > /proc/version_signature is the version for the host machine, but in e.g., chroots,
> > this does not necessarily match that DPDK is built for. DPDK will then build for the
> > wrong kernel version - that of the server, and not that installed in the (build)
> > chroot.
> >
> > The patch uses utsrelease.h from the kernel sources instead and fakes the
> > upload version.
> >
> > Tested on a server with Ubuntu 12.04, building in a chroot for Ubuntu 14.04.
> >
> > Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
> > Signed-off-by: Johan Faltstrom <johan.faltstrom@netinsight.net>
> Acked-by: Helin Zhang <helin.zhang@intel.com>
It doesn't fix every cases but Ubuntu chroot in Ubuntu, so
Applied, thanks
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-dev] [PATCH v2] kni: Use utsrelease.h to determine Ubuntu kernel version
2015-08-20 6:51 [dpdk-dev] [PATCH v2] kni: Use utsrelease.h to determine Ubuntu kernel version Simon Kagstrom
` (2 preceding siblings ...)
2015-11-04 8:30 ` Zhang, Helin
@ 2015-11-04 10:35 ` Thomas Monjalon
2015-11-04 11:29 ` Simon Kågström
3 siblings, 1 reply; 10+ messages in thread
From: Thomas Monjalon @ 2015-11-04 10:35 UTC (permalink / raw)
To: Simon Kagstrom; +Cc: dev
2015-08-20 08:51, Simon Kagstrom:
> /proc/version_signature is the version for the host machine, but in
> e.g., chroots, this does not necessarily match that DPDK is built
> for. DPDK will then build for the wrong kernel version - that of the
> server, and not that installed in the (build) chroot.
>
> The patch uses utsrelease.h from the kernel sources instead and fakes
> the upload version.
>
> Tested on a server with Ubuntu 12.04, building in a chroot for Ubuntu
> 14.04.
[...]
> --- 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
Yes we must check RTE_KERNELDIR instead of the running kernel.
But it is still checking lsb_release for the running system.
It seems not consistent.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [dpdk-dev] [PATCH v2] kni: Use utsrelease.h to determine Ubuntu kernel version
2015-11-04 10:35 ` Thomas Monjalon
@ 2015-11-04 11:29 ` Simon Kågström
2015-11-04 18:21 ` Stephen Hemminger
0 siblings, 1 reply; 10+ messages in thread
From: Simon Kågström @ 2015-11-04 11:29 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev
On 2015-11-04 11:35, Thomas Monjalon wrote:
> 2015-08-20 08:51, Simon Kagstrom:
>> -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
>
> Yes we must check RTE_KERNELDIR instead of the running kernel.
> But it is still checking lsb_release for the running system.
> It seems not consistent.
I don't think so: the case the patch addresses is where the running
kernel and rootfs doesn't match, like in a chroot environment.
So lsb_release will come from the chroot, as it should, but without the
patch, the kernel version will not come from the installed kernel
headers in the chroot, but the running kernel - which might even not be
Ubuntu.
// Simon
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-dev] [PATCH v2] kni: Use utsrelease.h to determine Ubuntu kernel version
2015-11-04 11:29 ` Simon Kågström
@ 2015-11-04 18:21 ` Stephen Hemminger
2015-11-05 8:20 ` Simon Kågström
0 siblings, 1 reply; 10+ messages in thread
From: Stephen Hemminger @ 2015-11-04 18:21 UTC (permalink / raw)
To: Simon Kågström; +Cc: dev
On Wed, 4 Nov 2015 12:29:01 +0100
Simon Kågström <simon.kagstrom@netinsight.net> wrote:
> On 2015-11-04 11:35, Thomas Monjalon wrote:
> > 2015-08-20 08:51, Simon Kagstrom:
> >> -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
> >
> > Yes we must check RTE_KERNELDIR instead of the running kernel.
> > But it is still checking lsb_release for the running system.
> > It seems not consistent.
>
> I don't think so: the case the patch addresses is where the running
> kernel and rootfs doesn't match, like in a chroot environment.
>
> So lsb_release will come from the chroot, as it should, but without the
> patch, the kernel version will not come from the installed kernel
> headers in the chroot, but the running kernel - which might even not be
> Ubuntu.
>
> // Simon
>
The danger here is starting to assume the build machine is the same as the
running image. Using /proc to determine runtime environment is wrong.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-dev] [PATCH v2] kni: Use utsrelease.h to determine Ubuntu kernel version
2015-11-04 18:21 ` Stephen Hemminger
@ 2015-11-05 8:20 ` Simon Kågström
0 siblings, 0 replies; 10+ messages in thread
From: Simon Kågström @ 2015-11-05 8:20 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev
On 2015-11-04 19:21, Stephen Hemminger wrote:
> On Wed, 4 Nov 2015 12:29:01 +0100
> Simon Kågström <simon.kagstrom@netinsight.net> wrote:
>
>> On 2015-11-04 11:35, Thomas Monjalon wrote:
>>> 2015-08-20 08:51, Simon Kagstrom:
>>>> -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
>>>
>> So lsb_release will come from the chroot, as it should, but without the
>> patch, the kernel version will not come from the installed kernel
>> headers in the chroot, but the running kernel - which might even not be
>> Ubuntu.
>
> The danger here is starting to assume the build machine is the same as the
> running image. Using /proc to determine runtime environment is wrong.
Exactly, and our build breaks because of this without the patch. So the
patch removes the check in /proc and instead takes the kernel version
from the kernel headers.
// Simon
^ permalink raw reply [flat|nested] 10+ messages in thread