Patchwork cryptodev kernel module recipe

login
register
mail settings
Submitter Yashpal Dutta
Date Oct. 18, 2012, 9:57 a.m.
Message ID <1350554279-8975-1-git-send-email-yashpal.dutta@freescale.com>
Download mbox | patch
Permalink /patch/38263/
State New
Headers show

Comments

Khem Raj - Oct. 18, 2012, 6:14 a.m.
On Oct 18, 2012, at 2:57 AM, Yashpal Dutta <yashpal.dutta@freescale.com> wrote:

> +-install:
> ++modules_install:
> + 	make -C $(KERNEL_DIR) SUBDIRS=`pwd` modules_install
> +-	@echo "Installing cryptodev.h in /usr/include/crypto ..."
> +-	@install -D crypto/cryptodev.h /usr/include/crypto/cryptodev.h
> ++	@echo "Installing cryptodev.h in $(PREFIX)/usr/include/crypto ..."
> ++	@install -D crypto/cryptodev.h $(PREFIX)/usr/include/crypto/cryptodev.h
> + 

why is it installing linux headers under /usr/include shouldn't they be under usr/include/linux ?
Khem Raj - Oct. 18, 2012, 6:35 a.m.
On Oct 17, 2012, at 11:27 PM, Dutta Yashpal-B05456 <B05456@freescale.com> wrote:

>> -----Original Message-----
>> From: Khem Raj [mailto:raj.khem@gmail.com]
>> Sent: Thursday, October 18, 2012 11:44 AM
>> To: Dutta Yashpal-B05456
>> Cc: openembedded-core@lists.openembedded.org
>> Subject: Re: [OE-core] [PATCH] cryptodev kernel module recipe
>> 
>> 
>> On Oct 18, 2012, at 2:57 AM, Yashpal Dutta <yashpal.dutta@freescale.com>
>> wrote:
>> 
>>> +-install:
>>> ++modules_install:
>>> + 	make -C $(KERNEL_DIR) SUBDIRS=`pwd` modules_install
>>> +-	@echo "Installing cryptodev.h in /usr/include/crypto ..."
>>> +-	@install -D crypto/cryptodev.h /usr/include/crypto/cryptodev.h
>>> ++	@echo "Installing cryptodev.h in $(PREFIX)/usr/include/crypto ..."
>>> ++	@install -D crypto/cryptodev.h
>> $(PREFIX)/usr/include/crypto/cryptodev.h
>>> +
>> 
>> why is it installing linux headers under /usr/include shouldn't they be
>> under usr/include/linux ?
> [Yash] That is how original cryptodev module makefile is installing the file for packages like openssl.
> Here is one instance where openssl is referring cryptodev.h:
> 
> crypto/engine/eng_cryptodev.c:#include <crypto/cryptodev.h>
> crypto/evp/openbsd_hw.c:#include <crypto/cryptodev.h>
> 

hmm ok.
Yashpal Dutta - Oct. 18, 2012, 9:57 a.m.
This is a /dev/crypto device driver, equivalent to those in OpenBSD or FreeBSD.
The main idea is to access of existing ciphers in kernel space from userspace,
thus enabling re-use of a hardware implementation of a cipher.

Signed-off-by: Yashpal Dutta <yashpal.dutta@freescale.com>
---
 meta/recipes-kernel/cryptodev/cryptodev_1.5.bb     |   18 +++++++++++++
 .../cryptodev/files/makefile_fixup.patch           |   26 ++++++++++++++++++++
 2 files changed, 44 insertions(+), 0 deletions(-)
 create mode 100644 meta/recipes-kernel/cryptodev/cryptodev_1.5.bb
 create mode 100644 meta/recipes-kernel/cryptodev/files/makefile_fixup.patch
Bruce Ashfield - Oct. 18, 2012, 1:33 p.m.
On Thu, Oct 18, 2012 at 5:57 AM, Yashpal Dutta
<yashpal.dutta@freescale.com> wrote:
> This is a /dev/crypto device driver, equivalent to those in OpenBSD or FreeBSD.
> The main idea is to access of existing ciphers in kernel space from userspace,
> thus enabling re-use of a hardware implementation of a cipher.

I always use OCF for an overlapping set of functionality. To make this external
module gracefully deal with a situation such as this, maybe a check for OCF
in the kernel config ?

The same thing could be said about having a kernel with this functionality
already integrated (I have several of those as well).

That's a more general question about what's the best way to gracefully deal
with out of tree modules detecting that they are being built against a kernel
that already has the functionality merged. The easy answer is that
it's something
the distro maintainer needs to know, and manage, and maybe that's the
final answer. But I'm more wondering about this with respect to
inter-operability
of layers, if something in a layer depends/rdepends on this module, it will be
pulled in, and won't that limit the mix/match/stacking of the various layers ?

I added Richard for comment on the above.

But that of course raises the question, why should this be in oe-core versus
something like OCF ? This is definitely simpler, but OCF has it's use cases and
advantages as well, that are an overlapping set of functionality.

I don't have all the answers, just plenty of questions :)

Cheers,

Bruce


>
> Signed-off-by: Yashpal Dutta <yashpal.dutta@freescale.com>
> ---
>  meta/recipes-kernel/cryptodev/cryptodev_1.5.bb     |   18 +++++++++++++
>  .../cryptodev/files/makefile_fixup.patch           |   26 ++++++++++++++++++++
>  2 files changed, 44 insertions(+), 0 deletions(-)
>  create mode 100644 meta/recipes-kernel/cryptodev/cryptodev_1.5.bb
>  create mode 100644 meta/recipes-kernel/cryptodev/files/makefile_fixup.patch
>
> diff --git a/meta/recipes-kernel/cryptodev/cryptodev_1.5.bb b/meta/recipes-kernel/cryptodev/cryptodev_1.5.bb
> new file mode 100644
> index 0000000..5125710
> --- /dev/null
> +++ b/meta/recipes-kernel/cryptodev/cryptodev_1.5.bb
> @@ -0,0 +1,18 @@
> +SECTION = "devel"
> +SUMMARY = "Linux Cryptodev KERNEL MODULE"
> +DESCRIPTION = "The Cryptodev package contains the kernel /dev/crypto module"
> +LICENSE = "GPLv2"
> +LIC_FILES_CHKSUM = "file://COPYING;md5=b234ee4d69f5fce4486a80fdaf4a4263"
> +
> +DEPENDS = "virtual/kernel"
> +
> +inherit module
> +
> +SRCREV = "1c24a0aa996630518d47826a2e3fea129ea094c7"
> +
> +SRC_URI = "git://repo.or.cz/cryptodev-linux.git;protocol=git \
> +         file://makefile_fixup.patch"
> +
> +EXTRA_OEMAKE='KERNEL_DIR="${STAGING_KERNEL_DIR}" PREFIX="${D}"'
> +
> +S = "${WORKDIR}/git"
> diff --git a/meta/recipes-kernel/cryptodev/files/makefile_fixup.patch b/meta/recipes-kernel/cryptodev/files/makefile_fixup.patch
> new file mode 100644
> index 0000000..323aacd
> --- /dev/null
> +++ b/meta/recipes-kernel/cryptodev/files/makefile_fixup.patch
> @@ -0,0 +1,26 @@
> +diff --git a/Makefile b/Makefile
> +index 2be8825..b36d68c 100644
> +--- a/Makefile
> ++++ b/Makefile
> +@@ -1,6 +1,7 @@
> + KBUILD_CFLAGS += -I$(src)
> + KERNEL_DIR = /lib/modules/$(shell uname -r)/build
> + VERSION = 1.5
> ++PREFIX =
> +
> + cryptodev-objs = ioctl.o main.o cryptlib.o authenc.o zc.o util.o
> +
> +@@ -12,10 +13,10 @@ build: version.h
> + version.h: Makefile
> +       @echo "#define VERSION \"$(VERSION)\"" > version.h
> +
> +-install:
> ++modules_install:
> +       make -C $(KERNEL_DIR) SUBDIRS=`pwd` modules_install
> +-      @echo "Installing cryptodev.h in /usr/include/crypto ..."
> +-      @install -D crypto/cryptodev.h /usr/include/crypto/cryptodev.h
> ++      @echo "Installing cryptodev.h in $(PREFIX)/usr/include/crypto ..."
> ++      @install -D crypto/cryptodev.h $(PREFIX)/usr/include/crypto/cryptodev.h
> +
> + clean:
> +       make -C $(KERNEL_DIR) SUBDIRS=`pwd` clean
> --
> 1.7.0.4
>
>
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Darren Hart - Oct. 18, 2012, 7:59 p.m.
On 10/18/2012 02:14 AM, Khem Raj wrote:
> 
> On Oct 18, 2012, at 2:57 AM, Yashpal Dutta <yashpal.dutta@freescale.com> wrote:
> 
>> +-install:
>> ++modules_install:
>> + 	make -C $(KERNEL_DIR) SUBDIRS=`pwd` modules_install
>> +-	@echo "Installing cryptodev.h in /usr/include/crypto ..."
>> +-	@install -D crypto/cryptodev.h /usr/include/crypto/cryptodev.h
>> ++	@echo "Installing cryptodev.h in $(PREFIX)/usr/include/crypto ..."
>> ++	@install -D crypto/cryptodev.h $(PREFIX)/usr/include/crypto/cryptodev.h
>> + 
> 
> why is it installing linux headers under /usr/include shouldn't they be under usr/include/linux ?

And shouldn't /usr/include be one of those ${*dir} variables?
McClintock Matthew-B29882 - Oct. 18, 2012, 8:06 p.m.
On Thu, Oct 18, 2012 at 2:59 PM, Darren Hart <dvhart@linux.intel.com> wrote:
>
>
> On 10/18/2012 02:14 AM, Khem Raj wrote:
>>
>> On Oct 18, 2012, at 2:57 AM, Yashpal Dutta <yashpal.dutta@freescale.com> wrote:
>>
>>> +-install:
>>> ++modules_install:
>>> +    make -C $(KERNEL_DIR) SUBDIRS=`pwd` modules_install
>>> +-   @echo "Installing cryptodev.h in /usr/include/crypto ..."
>>> +-   @install -D crypto/cryptodev.h /usr/include/crypto/cryptodev.h
>>> ++   @echo "Installing cryptodev.h in $(PREFIX)/usr/include/crypto ..."
>>> ++   @install -D crypto/cryptodev.h $(PREFIX)/usr/include/crypto/cryptodev.h
>>> +
>>
>> why is it installing linux headers under /usr/include shouldn't they be under usr/include/linux ?
>
> And shouldn't /usr/include be one of those ${*dir} variables?

That's a patch to a Makefile... not a recipe.

-M
Darren Hart - Oct. 18, 2012, 8:16 p.m.
On 10/18/2012 05:57 AM, Yashpal Dutta wrote:
> This is a /dev/crypto device driver, equivalent to those in OpenBSD or FreeBSD.
> The main idea is to access of existing ciphers in kernel space from userspace,
> thus enabling re-use of a hardware implementation of a cipher.
> 
> Signed-off-by: Yashpal Dutta <yashpal.dutta@freescale.com>
> ---
>  meta/recipes-kernel/cryptodev/cryptodev_1.5.bb     |   18 +++++++++++++
>  .../cryptodev/files/makefile_fixup.patch           |   26 ++++++++++++++++++++
>  2 files changed, 44 insertions(+), 0 deletions(-)
>  create mode 100644 meta/recipes-kernel/cryptodev/cryptodev_1.5.bb
>  create mode 100644 meta/recipes-kernel/cryptodev/files/makefile_fixup.patch
> 
> diff --git a/meta/recipes-kernel/cryptodev/cryptodev_1.5.bb b/meta/recipes-kernel/cryptodev/cryptodev_1.5.bb
> new file mode 100644
> index 0000000..5125710
> --- /dev/null
> +++ b/meta/recipes-kernel/cryptodev/cryptodev_1.5.bb
> @@ -0,0 +1,18 @@
> +SECTION = "devel"
> +SUMMARY = "Linux Cryptodev KERNEL MODULE"
> +DESCRIPTION = "The Cryptodev package contains the kernel /dev/crypto module"
> +LICENSE = "GPLv2"
> +LIC_FILES_CHKSUM = "file://COPYING;md5=b234ee4d69f5fce4486a80fdaf4a4263"
> +
> +DEPENDS = "virtual/kernel"

This DEPENDS in inherited from the module.bbclass, no need to duplicate

> +
> +inherit module
> +
> +SRCREV = "1c24a0aa996630518d47826a2e3fea129ea094c7"
> +
> +SRC_URI = "git://repo.or.cz/cryptodev-linux.git;protocol=git \
> +	  file://makefile_fixup.patch"

Tabs to indent, spaces to align. Spaces here please.

> +
> +EXTRA_OEMAKE='KERNEL_DIR="${STAGING_KERNEL_DIR}" PREFIX="${D}"'

modules.bbclass already sets KERNEL_PATH and KERNEL_SRC, perhaps you
could use one of those?

If you just use ${includedir} in your install I believe you can skip
PREFIX here and get the /usr part right there.

> +
> +S = "${WORKDIR}/git"
> diff --git a/meta/recipes-kernel/cryptodev/files/makefile_fixup.patch b/meta/recipes-kernel/cryptodev/files/makefile_fixup.patch
> new file mode 100644
> index 0000000..323aacd
> --- /dev/null
> +++ b/meta/recipes-kernel/cryptodev/files/makefile_fixup.patch
> @@ -0,0 +1,26 @@
> +diff --git a/Makefile b/Makefile
> +index 2be8825..b36d68c 100644
> +--- a/Makefile
> ++++ b/Makefile
> +@@ -1,6 +1,7 @@
> + KBUILD_CFLAGS += -I$(src)
> + KERNEL_DIR = /lib/modules/$(shell uname -r)/build
> + VERSION = 1.5
> ++PREFIX =
> + 
> + cryptodev-objs = ioctl.o main.o cryptlib.o authenc.o zc.o util.o
> + 
> +@@ -12,10 +13,10 @@ build: version.h
> + version.h: Makefile
> + 	@echo "#define VERSION \"$(VERSION)\"" > version.h
> + 
> +-install:
> ++modules_install:
> + 	make -C $(KERNEL_DIR) SUBDIRS=`pwd` modules_install

Current kernels recommend using M= instead of SUBDIRS=:

SRC := $(shell pwd)
make -C $(KERNEL_SRC) M=$(SRC) modules_install

See the hello-mod example in meta-skeleton/recipes-kernel/hello-mod for
a minimal example.

> +-	@echo "Installing cryptodev.h in /usr/include/crypto ..."
> +-	@install -D crypto/cryptodev.h /usr/include/crypto/cryptodev.h
> ++	@echo "Installing cryptodev.h in $(PREFIX)/usr/include/crypto ..."
> ++	@install -D crypto/cryptodev.h $(PREFIX)/usr/include/crypto/cryptodev.h

Use ${includedir} here

> + 
> + clean:
> + 	make -C $(KERNEL_DIR) SUBDIRS=`pwd` clean
> 

M=
Darren Hart - Oct. 18, 2012, 8:21 p.m.
On 10/18/2012 04:06 PM, McClintock Matthew-B29882 wrote:
> On Thu, Oct 18, 2012 at 2:59 PM, Darren Hart <dvhart@linux.intel.com> wrote:
>>
>>
>> On 10/18/2012 02:14 AM, Khem Raj wrote:
>>>
>>> On Oct 18, 2012, at 2:57 AM, Yashpal Dutta <yashpal.dutta@freescale.com> wrote:
>>>
>>>> +-install:
>>>> ++modules_install:
>>>> +    make -C $(KERNEL_DIR) SUBDIRS=`pwd` modules_install
>>>> +-   @echo "Installing cryptodev.h in /usr/include/crypto ..."
>>>> +-   @install -D crypto/cryptodev.h /usr/include/crypto/cryptodev.h
>>>> ++   @echo "Installing cryptodev.h in $(PREFIX)/usr/include/crypto ..."
>>>> ++   @install -D crypto/cryptodev.h $(PREFIX)/usr/include/crypto/cryptodev.h
>>>> +
>>>
>>> why is it installing linux headers under /usr/include shouldn't they be under usr/include/linux ?
>>
>> And shouldn't /usr/include be one of those ${*dir} variables?
> 
> That's a patch to a Makefile... not a recipe.

Duh.... hrm. I'm not sure how we deal with this typically.
McClintock Matthew-B29882 - Oct. 18, 2012, 8:33 p.m.
On Thu, Oct 18, 2012 at 3:16 PM, Darren Hart <dvhart@linux.intel.com> wrote:
>
>
> On 10/18/2012 05:57 AM, Yashpal Dutta wrote:
>> This is a /dev/crypto device driver, equivalent to those in OpenBSD or FreeBSD.
>> The main idea is to access of existing ciphers in kernel space from userspace,
>> thus enabling re-use of a hardware implementation of a cipher.
>>
>> Signed-off-by: Yashpal Dutta <yashpal.dutta@freescale.com>
>> ---
>>  meta/recipes-kernel/cryptodev/cryptodev_1.5.bb     |   18 +++++++++++++
>>  .../cryptodev/files/makefile_fixup.patch           |   26 ++++++++++++++++++++
>>  2 files changed, 44 insertions(+), 0 deletions(-)
>>  create mode 100644 meta/recipes-kernel/cryptodev/cryptodev_1.5.bb
>>  create mode 100644 meta/recipes-kernel/cryptodev/files/makefile_fixup.patch
>>
>> diff --git a/meta/recipes-kernel/cryptodev/cryptodev_1.5.bb b/meta/recipes-kernel/cryptodev/cryptodev_1.5.bb
>> new file mode 100644
>> index 0000000..5125710
>> --- /dev/null
>> +++ b/meta/recipes-kernel/cryptodev/cryptodev_1.5.bb
>> @@ -0,0 +1,18 @@
>> +SECTION = "devel"
>> +SUMMARY = "Linux Cryptodev KERNEL MODULE"
>> +DESCRIPTION = "The Cryptodev package contains the kernel /dev/crypto module"
>> +LICENSE = "GPLv2"
>> +LIC_FILES_CHKSUM = "file://COPYING;md5=b234ee4d69f5fce4486a80fdaf4a4263"
>> +
>> +DEPENDS = "virtual/kernel"
>
> This DEPENDS in inherited from the module.bbclass, no need to duplicate
>
>> +
>> +inherit module
>> +
>> +SRCREV = "1c24a0aa996630518d47826a2e3fea129ea094c7"
>> +
>> +SRC_URI = "git://repo.or.cz/cryptodev-linux.git;protocol=git \
>> +       file://makefile_fixup.patch"
>
> Tabs to indent, spaces to align. Spaces here please.
>
>> +
>> +EXTRA_OEMAKE='KERNEL_DIR="${STAGING_KERNEL_DIR}" PREFIX="${D}"'
>
> modules.bbclass already sets KERNEL_PATH and KERNEL_SRC, perhaps you
> could use one of those?

cryptodev Makefile does not use these it uses KERNEL_DIR in it's
Makefile for whatever reason. Getting an upstream project to change is
more difficult.

> If you just use ${includedir} in your install I believe you can skip
> PREFIX here and get the /usr part right there.

This is more changes to the upstream Makefile....

>> +
>> +S = "${WORKDIR}/git"
>> diff --git a/meta/recipes-kernel/cryptodev/files/makefile_fixup.patch b/meta/recipes-kernel/cryptodev/files/makefile_fixup.patch
>> new file mode 100644
>> index 0000000..323aacd
>> --- /dev/null
>> +++ b/meta/recipes-kernel/cryptodev/files/makefile_fixup.patch
>> @@ -0,0 +1,26 @@
>> +diff --git a/Makefile b/Makefile
>> +index 2be8825..b36d68c 100644
>> +--- a/Makefile
>> ++++ b/Makefile
>> +@@ -1,6 +1,7 @@
>> + KBUILD_CFLAGS += -I$(src)
>> + KERNEL_DIR = /lib/modules/$(shell uname -r)/build
>> + VERSION = 1.5
>> ++PREFIX =
>> +
>> + cryptodev-objs = ioctl.o main.o cryptlib.o authenc.o zc.o util.o
>> +
>> +@@ -12,10 +13,10 @@ build: version.h
>> + version.h: Makefile
>> +     @echo "#define VERSION \"$(VERSION)\"" > version.h
>> +
>> +-install:
>> ++modules_install:
>> +     make -C $(KERNEL_DIR) SUBDIRS=`pwd` modules_install
>
> Current kernels recommend using M= instead of SUBDIRS=:
>
> SRC := $(shell pwd)
> make -C $(KERNEL_SRC) M=$(SRC) modules_install
>
> See the hello-mod example in meta-skeleton/recipes-kernel/hello-mod for
> a minimal example.
>
>> +-    @echo "Installing cryptodev.h in /usr/include/crypto ..."
>> +-    @install -D crypto/cryptodev.h /usr/include/crypto/cryptodev.h
>> ++    @echo "Installing cryptodev.h in $(PREFIX)/usr/include/crypto ..."
>> ++    @install -D crypto/cryptodev.h $(PREFIX)/usr/include/crypto/cryptodev.h
>
> Use ${includedir} here

You can't use this bitbake variable in a Makefile... or am I missing
something? We are making small changes the Makefile for things like
prefix so we can install to locations other than the hosts
/usr/include which is hard coded.

-M

>
>> +
>> + clean:
>> +     make -C $(KERNEL_DIR) SUBDIRS=`pwd` clean
>>
>
> M=

Is changing upstream cryptodev is one thing (e.g. SUBDIR= vs. M=), how
far do you want a patch to a Makefile to change the way a project
builds?

-M

>
> --
> Darren Hart
> Intel Open Source Technology Center
> Yocto Project - Technical Lead - Linux Kernel
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Darren Hart - Oct. 18, 2012, 8:38 p.m.
On 10/18/2012 04:33 PM, McClintock Matthew-B29882 wrote:
> On Thu, Oct 18, 2012 at 3:16 PM, Darren Hart <dvhart@linux.intel.com> wrote:
>>
>>
>> On 10/18/2012 05:57 AM, Yashpal Dutta wrote:
>>> This is a /dev/crypto device driver, equivalent to those in OpenBSD or FreeBSD.
>>> The main idea is to access of existing ciphers in kernel space from userspace,
>>> thus enabling re-use of a hardware implementation of a cipher.
>>>
>>> Signed-off-by: Yashpal Dutta <yashpal.dutta@freescale.com>
>>> ---
>>>  meta/recipes-kernel/cryptodev/cryptodev_1.5.bb     |   18 +++++++++++++
>>>  .../cryptodev/files/makefile_fixup.patch           |   26 ++++++++++++++++++++
>>>  2 files changed, 44 insertions(+), 0 deletions(-)
>>>  create mode 100644 meta/recipes-kernel/cryptodev/cryptodev_1.5.bb
>>>  create mode 100644 meta/recipes-kernel/cryptodev/files/makefile_fixup.patch
>>>
>>> diff --git a/meta/recipes-kernel/cryptodev/cryptodev_1.5.bb b/meta/recipes-kernel/cryptodev/cryptodev_1.5.bb
>>> new file mode 100644
>>> index 0000000..5125710
>>> --- /dev/null
>>> +++ b/meta/recipes-kernel/cryptodev/cryptodev_1.5.bb
>>> @@ -0,0 +1,18 @@
>>> +SECTION = "devel"
>>> +SUMMARY = "Linux Cryptodev KERNEL MODULE"
>>> +DESCRIPTION = "The Cryptodev package contains the kernel /dev/crypto module"
>>> +LICENSE = "GPLv2"
>>> +LIC_FILES_CHKSUM = "file://COPYING;md5=b234ee4d69f5fce4486a80fdaf4a4263"
>>> +
>>> +DEPENDS = "virtual/kernel"
>>
>> This DEPENDS in inherited from the module.bbclass, no need to duplicate
>>
>>> +
>>> +inherit module
>>> +
>>> +SRCREV = "1c24a0aa996630518d47826a2e3fea129ea094c7"
>>> +
>>> +SRC_URI = "git://repo.or.cz/cryptodev-linux.git;protocol=git \
>>> +       file://makefile_fixup.patch"
>>
>> Tabs to indent, spaces to align. Spaces here please.
>>
>>> +
>>> +EXTRA_OEMAKE='KERNEL_DIR="${STAGING_KERNEL_DIR}" PREFIX="${D}"'
>>
>> modules.bbclass already sets KERNEL_PATH and KERNEL_SRC, perhaps you
>> could use one of those?
> 
> cryptodev Makefile does not use these it uses KERNEL_DIR in it's
> Makefile for whatever reason. Getting an upstream project to change is
> more difficult.

I think this is the second reference to KERNEL_DIR in an external module,
perhaps module.bbclass should add that to it's list of predefined names for
the STAGING_KERNEL_DIR.

> 
> Is changing upstream cryptodev is one thing (e.g. SUBDIR= vs. M=), how
> far do you want a patch to a Makefile to change the way a project
> builds?

Yeah, you're correct about the Makefile changes. I got carried away
during a rapid review.
McClintock Matthew-B29882 - Oct. 19, 2012, 1:47 a.m.
On Thu, Oct 18, 2012 at 3:38 PM, Darren Hart <dvhart@linux.intel.com> wrote:
>>>> +EXTRA_OEMAKE='KERNEL_DIR="${STAGING_KERNEL_DIR}" PREFIX="${D}"'
>>>
>>> modules.bbclass already sets KERNEL_PATH and KERNEL_SRC, perhaps you
>>> could use one of those?
>>
>> cryptodev Makefile does not use these it uses KERNEL_DIR in it's
>> Makefile for whatever reason. Getting an upstream project to change is
>> more difficult.
>
> I think this is the second reference to KERNEL_DIR in an external module,
> perhaps module.bbclass should add that to it's list of predefined names for
> the STAGING_KERNEL_DIR.

Fine with me, but not sure how its worth it quite yet...
Andreas Oberritter - Oct. 19, 2012, 9:43 a.m.
On 19.10.2012 03:47, McClintock Matthew-B29882 wrote:
> On Thu, Oct 18, 2012 at 3:38 PM, Darren Hart <dvhart@linux.intel.com> wrote:
>>>>> +EXTRA_OEMAKE='KERNEL_DIR="${STAGING_KERNEL_DIR}" PREFIX="${D}"'
>>>>
>>>> modules.bbclass already sets KERNEL_PATH and KERNEL_SRC, perhaps you
>>>> could use one of those?
>>>
>>> cryptodev Makefile does not use these it uses KERNEL_DIR in it's
>>> Makefile for whatever reason. Getting an upstream project to change is
>>> more difficult.
>>
>> I think this is the second reference to KERNEL_DIR in an external module,
>> perhaps module.bbclass should add that to it's list of predefined names for
>> the STAGING_KERNEL_DIR.
> 
> Fine with me, but not sure how its worth it quite yet...

I don't think we should add more privately defined variables to
module.bbclass.

I'd rather use something like the following as the default, which should
work for all recipes where ${S} points to the directory containing the
top-level Kbuild file and is independent of those variables:

do_compile() {
        unset CFLAGS CPPFLAGS CXXFLAGS LDFLAGS
        oe_runmake -C "${STAGING_KERNEL_DIR}" SUBDIRS="${S}" modules
}
do_install() {
        unset CFLAGS CPPFLAGS CXXFLAGS LDFLAGS
        oe_runmake -C "${STAGING_KERNEL_DIR}" SUBDIRS="${S}"
DEPMOD="echo" INSTALL_MOD_PATH="${D}" modules_install
}

That's not an option for cryptodev, though, because unlike most recipes
inheriting module.bbclass, cryptodev not only installs modules, but also
header files.

Regards,
Andreas
Khem Raj - Oct. 19, 2012, 2:45 p.m.
On Fri, Oct 19, 2012 at 2:43 AM, Andreas Oberritter
<obi@opendreambox.org> wrote:
> That's not an option for cryptodev, though, because unlike most recipes
> inheriting module.bbclass, cryptodev not only installs modules, but also
> header files.

for such exceptions you can always use do_install_append and do the necessary
McClintock Matthew-B29882 - Oct. 30, 2012, 6:50 p.m.
On Thu, Oct 18, 2012 at 8:33 AM, Bruce Ashfield
<bruce.ashfield@gmail.com> wrote:
> On Thu, Oct 18, 2012 at 5:57 AM, Yashpal Dutta
> <yashpal.dutta@freescale.com> wrote:
>> This is a /dev/crypto device driver, equivalent to those in OpenBSD or FreeBSD.
>> The main idea is to access of existing ciphers in kernel space from userspace,
>> thus enabling re-use of a hardware implementation of a cipher.
>
> I always use OCF for an overlapping set of functionality. To make this external
> module gracefully deal with a situation such as this, maybe a check for OCF
> in the kernel config ?
>
> The same thing could be said about having a kernel with this functionality
> already integrated (I have several of those as well).
>
> That's a more general question about what's the best way to gracefully deal
> with out of tree modules detecting that they are being built against a kernel
> that already has the functionality merged. The easy answer is that
> it's something
> the distro maintainer needs to know, and manage, and maybe that's the
> final answer. But I'm more wondering about this with respect to
> inter-operability
> of layers, if something in a layer depends/rdepends on this module, it will be
> pulled in, and won't that limit the mix/match/stacking of the various layers ?
>
> I added Richard for comment on the above.
>
> But that of course raises the question, why should this be in oe-core versus
> something like OCF ? This is definitely simpler, but OCF has it's use cases and
> advantages as well, that are an overlapping set of functionality.
>
> I don't have all the answers, just plenty of questions :)

I'm not overly familiar with OCF. Does this just RCONFLICT with ocf-linux?

-M

>
> Cheers,
>
> Bruce
>
>
>>
>> Signed-off-by: Yashpal Dutta <yashpal.dutta@freescale.com>
>> ---
>>  meta/recipes-kernel/cryptodev/cryptodev_1.5.bb     |   18 +++++++++++++
>>  .../cryptodev/files/makefile_fixup.patch           |   26 ++++++++++++++++++++
>>  2 files changed, 44 insertions(+), 0 deletions(-)
>>  create mode 100644 meta/recipes-kernel/cryptodev/cryptodev_1.5.bb
>>  create mode 100644 meta/recipes-kernel/cryptodev/files/makefile_fixup.patch
>>
>> diff --git a/meta/recipes-kernel/cryptodev/cryptodev_1.5.bb b/meta/recipes-kernel/cryptodev/cryptodev_1.5.bb
>> new file mode 100644
>> index 0000000..5125710
>> --- /dev/null
>> +++ b/meta/recipes-kernel/cryptodev/cryptodev_1.5.bb
>> @@ -0,0 +1,18 @@
>> +SECTION = "devel"
>> +SUMMARY = "Linux Cryptodev KERNEL MODULE"
>> +DESCRIPTION = "The Cryptodev package contains the kernel /dev/crypto module"
>> +LICENSE = "GPLv2"
>> +LIC_FILES_CHKSUM = "file://COPYING;md5=b234ee4d69f5fce4486a80fdaf4a4263"
>> +
>> +DEPENDS = "virtual/kernel"
>> +
>> +inherit module
>> +
>> +SRCREV = "1c24a0aa996630518d47826a2e3fea129ea094c7"
>> +
>> +SRC_URI = "git://repo.or.cz/cryptodev-linux.git;protocol=git \
>> +         file://makefile_fixup.patch"
>> +
>> +EXTRA_OEMAKE='KERNEL_DIR="${STAGING_KERNEL_DIR}" PREFIX="${D}"'
>> +
>> +S = "${WORKDIR}/git"
>> diff --git a/meta/recipes-kernel/cryptodev/files/makefile_fixup.patch b/meta/recipes-kernel/cryptodev/files/makefile_fixup.patch
>> new file mode 100644
>> index 0000000..323aacd
>> --- /dev/null
>> +++ b/meta/recipes-kernel/cryptodev/files/makefile_fixup.patch
>> @@ -0,0 +1,26 @@
>> +diff --git a/Makefile b/Makefile
>> +index 2be8825..b36d68c 100644
>> +--- a/Makefile
>> ++++ b/Makefile
>> +@@ -1,6 +1,7 @@
>> + KBUILD_CFLAGS += -I$(src)
>> + KERNEL_DIR = /lib/modules/$(shell uname -r)/build
>> + VERSION = 1.5
>> ++PREFIX =
>> +
>> + cryptodev-objs = ioctl.o main.o cryptlib.o authenc.o zc.o util.o
>> +
>> +@@ -12,10 +13,10 @@ build: version.h
>> + version.h: Makefile
>> +       @echo "#define VERSION \"$(VERSION)\"" > version.h
>> +
>> +-install:
>> ++modules_install:
>> +       make -C $(KERNEL_DIR) SUBDIRS=`pwd` modules_install
>> +-      @echo "Installing cryptodev.h in /usr/include/crypto ..."
>> +-      @install -D crypto/cryptodev.h /usr/include/crypto/cryptodev.h
>> ++      @echo "Installing cryptodev.h in $(PREFIX)/usr/include/crypto ..."
>> ++      @install -D crypto/cryptodev.h $(PREFIX)/usr/include/crypto/cryptodev.h
>> +
>> + clean:
>> +       make -C $(KERNEL_DIR) SUBDIRS=`pwd` clean
>> --
>> 1.7.0.4
>>
>>
>>
>> _______________________________________________
>> Openembedded-core mailing list
>> Openembedded-core@lists.openembedded.org
>> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>
>
>
> --
> "Thou shalt not follow the NULL pointer, for chaos and madness await
> thee at its end"
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
McClintock Matthew-B29882 - Oct. 30, 2012, 6:53 p.m.
On Thu, Oct 18, 2012 at 3:16 PM, Darren Hart <dvhart@linux.intel.com> wrote:
>> +++ b/meta/recipes-kernel/cryptodev/cryptodev_1.5.bb
>> @@ -0,0 +1,18 @@
>> +SECTION = "devel"
>> +SUMMARY = "Linux Cryptodev KERNEL MODULE"
>> +DESCRIPTION = "The Cryptodev package contains the kernel /dev/crypto module"
>> +LICENSE = "GPLv2"
>> +LIC_FILES_CHKSUM = "file://COPYING;md5=b234ee4d69f5fce4486a80fdaf4a4263"
>> +
>> +DEPENDS = "virtual/kernel"
>
> This DEPENDS in inherited from the module.bbclass, no need to duplicate
>
>> +
>> +inherit module
>> +
>> +SRCREV = "1c24a0aa996630518d47826a2e3fea129ea094c7"
>> +
>> +SRC_URI = "git://repo.or.cz/cryptodev-linux.git;protocol=git \
>> +       file://makefile_fixup.patch"
>
> Tabs to indent, spaces to align. Spaces here please.

Yashpal,

Can you address these two comments and submit a v2 of the patch if required.

-M
Bruce Ashfield - Oct. 31, 2012, 5:11 p.m.
On Tue, Oct 30, 2012 at 2:50 PM, McClintock Matthew-B29882 <
B29882@freescale.com> wrote:

> On Thu, Oct 18, 2012 at 8:33 AM, Bruce Ashfield
> <bruce.ashfield@gmail.com> wrote:
> > On Thu, Oct 18, 2012 at 5:57 AM, Yashpal Dutta
> > <yashpal.dutta@freescale.com> wrote:
> >> This is a /dev/crypto device driver, equivalent to those in OpenBSD or
> FreeBSD.
> >> The main idea is to access of existing ciphers in kernel space from
> userspace,
> >> thus enabling re-use of a hardware implementation of a cipher.
> >
> > I always use OCF for an overlapping set of functionality. To make this
> external
> > module gracefully deal with a situation such as this, maybe a check for
> OCF
> > in the kernel config ?
> >
> > The same thing could be said about having a kernel with this
> functionality
> > already integrated (I have several of those as well).
> >
> > That's a more general question about what's the best way to gracefully
> deal
> > with out of tree modules detecting that they are being built against a
> kernel
> > that already has the functionality merged. The easy answer is that
> > it's something
> > the distro maintainer needs to know, and manage, and maybe that's the
> > final answer. But I'm more wondering about this with respect to
> > inter-operability
> > of layers, if something in a layer depends/rdepends on this module, it
> will be
> > pulled in, and won't that limit the mix/match/stacking of the various
> layers ?
> >
> > I added Richard for comment on the above.
> >
> > But that of course raises the question, why should this be in oe-core
> versus
> > something like OCF ? This is definitely simpler, but OCF has it's use
> cases and
> > advantages as well, that are an overlapping set of functionality.
> >
> > I don't have all the answers, just plenty of questions :)
>
> I'm not overly familiar with OCF. Does this just RCONFLICT with ocf-linux?
>

I missed this yesterday, sorry about that. gmail just left this in the
thread and
not in my inbox .. but anyway.

I only see parts ocf in oe-core, but maybe I'm just not understanding what
the
recipe is doing (are you seeing more) ? i.e. ocf-linux is buried under the
open-ssl recipe, and really looks to be just providing headers.

I'm doing some builds now to learn more .. which I just did .. and from
what I
can see, it is just the headers, which isn't all that useful without the
kernel
underpinning.

OCF does definitely accelerate openssl when properly used in both the
kernel and
userspace, but I'm not seeing the full offload kernel framework anywhere.

I wonder if anyone actually uses it :)

But yes AFAIC, if you had a full OCF, you need these to conflict, since
they'll both
try and enable/provide cryptodev.

Bruce


>
> -M
>
> >
> > Cheers,
> >
> > Bruce
> >
> >
> >>
> >> Signed-off-by: Yashpal Dutta <yashpal.dutta@freescale.com>
> >> ---
> >>  meta/recipes-kernel/cryptodev/cryptodev_1.5.bb     |   18
> +++++++++++++
> >>  .../cryptodev/files/makefile_fixup.patch           |   26
> ++++++++++++++++++++
> >>  2 files changed, 44 insertions(+), 0 deletions(-)
> >>  create mode 100644 meta/recipes-kernel/cryptodev/cryptodev_1.5.bb
> >>  create mode 100644
> meta/recipes-kernel/cryptodev/files/makefile_fixup.patch
> >>
> >> diff --git a/meta/recipes-kernel/cryptodev/cryptodev_1.5.bbb/meta/recipes-kernel/cryptodev/
> cryptodev_1.5.bb
> >> new file mode 100644
> >> index 0000000..5125710
> >> --- /dev/null
> >> +++ b/meta/recipes-kernel/cryptodev/cryptodev_1.5.bb
> >> @@ -0,0 +1,18 @@
> >> +SECTION = "devel"
> >> +SUMMARY = "Linux Cryptodev KERNEL MODULE"
> >> +DESCRIPTION = "The Cryptodev package contains the kernel /dev/crypto
> module"
> >> +LICENSE = "GPLv2"
> >> +LIC_FILES_CHKSUM =
> "file://COPYING;md5=b234ee4d69f5fce4486a80fdaf4a4263"
> >> +
> >> +DEPENDS = "virtual/kernel"
> >> +
> >> +inherit module
> >> +
> >> +SRCREV = "1c24a0aa996630518d47826a2e3fea129ea094c7"
> >> +
> >> +SRC_URI = "git://repo.or.cz/cryptodev-linux.git;protocol=git \
> >> +         file://makefile_fixup.patch"
> >> +
> >> +EXTRA_OEMAKE='KERNEL_DIR="${STAGING_KERNEL_DIR}" PREFIX="${D}"'
> >> +
> >> +S = "${WORKDIR}/git"
> >> diff --git a/meta/recipes-kernel/cryptodev/files/makefile_fixup.patch
> b/meta/recipes-kernel/cryptodev/files/makefile_fixup.patch
> >> new file mode 100644
> >> index 0000000..323aacd
> >> --- /dev/null
> >> +++ b/meta/recipes-kernel/cryptodev/files/makefile_fixup.patch
> >> @@ -0,0 +1,26 @@
> >> +diff --git a/Makefile b/Makefile
> >> +index 2be8825..b36d68c 100644
> >> +--- a/Makefile
> >> ++++ b/Makefile
> >> +@@ -1,6 +1,7 @@
> >> + KBUILD_CFLAGS += -I$(src)
> >> + KERNEL_DIR = /lib/modules/$(shell uname -r)/build
> >> + VERSION = 1.5
> >> ++PREFIX =
> >> +
> >> + cryptodev-objs = ioctl.o main.o cryptlib.o authenc.o zc.o util.o
> >> +
> >> +@@ -12,10 +13,10 @@ build: version.h
> >> + version.h: Makefile
> >> +       @echo "#define VERSION \"$(VERSION)\"" > version.h
> >> +
> >> +-install:
> >> ++modules_install:
> >> +       make -C $(KERNEL_DIR) SUBDIRS=`pwd` modules_install
> >> +-      @echo "Installing cryptodev.h in /usr/include/crypto ..."
> >> +-      @install -D crypto/cryptodev.h /usr/include/crypto/cryptodev.h
> >> ++      @echo "Installing cryptodev.h in $(PREFIX)/usr/include/crypto
> ..."
> >> ++      @install -D crypto/cryptodev.h
> $(PREFIX)/usr/include/crypto/cryptodev.h
> >> +
> >> + clean:
> >> +       make -C $(KERNEL_DIR) SUBDIRS=`pwd` clean
> >> --
> >> 1.7.0.4
> >>
> >>
> >>
> >> _______________________________________________
> >> Openembedded-core mailing list
> >> Openembedded-core@lists.openembedded.org
> >> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
> >
> >
> >
> > --
> > "Thou shalt not follow the NULL pointer, for chaos and madness await
> > thee at its end"
> >
> > _______________________________________________
> > Openembedded-core mailing list
> > Openembedded-core@lists.openembedded.org
> > http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>
McClintock Matthew-B29882 - Oct. 31, 2012, 7:21 p.m.
On Wed, Oct 31, 2012 at 12:11 PM, Bruce Ashfield
<bruce.ashfield@gmail.com> wrote:
>
> On Tue, Oct 30, 2012 at 2:50 PM, McClintock Matthew-B29882
> <B29882@freescale.com> wrote:
>>
>> On Thu, Oct 18, 2012 at 8:33 AM, Bruce Ashfield
>> <bruce.ashfield@gmail.com> wrote:
>> > On Thu, Oct 18, 2012 at 5:57 AM, Yashpal Dutta
>> > <yashpal.dutta@freescale.com> wrote:
>> >> This is a /dev/crypto device driver, equivalent to those in OpenBSD or
>> >> FreeBSD.
>> >> The main idea is to access of existing ciphers in kernel space from
>> >> userspace,
>> >> thus enabling re-use of a hardware implementation of a cipher.
>> >
>> > I always use OCF for an overlapping set of functionality. To make this
>> > external
>> > module gracefully deal with a situation such as this, maybe a check for
>> > OCF
>> > in the kernel config ?
>> >
>> > The same thing could be said about having a kernel with this
>> > functionality
>> > already integrated (I have several of those as well).
>> >
>> > That's a more general question about what's the best way to gracefully
>> > deal
>> > with out of tree modules detecting that they are being built against a
>> > kernel
>> > that already has the functionality merged. The easy answer is that
>> > it's something
>> > the distro maintainer needs to know, and manage, and maybe that's the
>> > final answer. But I'm more wondering about this with respect to
>> > inter-operability
>> > of layers, if something in a layer depends/rdepends on this module, it
>> > will be
>> > pulled in, and won't that limit the mix/match/stacking of the various
>> > layers ?
>> >
>> > I added Richard for comment on the above.
>> >
>> > But that of course raises the question, why should this be in oe-core
>> > versus
>> > something like OCF ? This is definitely simpler, but OCF has it's use
>> > cases and
>> > advantages as well, that are an overlapping set of functionality.
>> >
>> > I don't have all the answers, just plenty of questions :)
>>
>> I'm not overly familiar with OCF. Does this just RCONFLICT with ocf-linux?
>
>
> I missed this yesterday, sorry about that. gmail just left this in the
> thread and
> not in my inbox .. but anyway.
>
> I only see parts ocf in oe-core, but maybe I'm just not understanding what
> the
> recipe is doing (are you seeing more) ? i.e. ocf-linux is buried under the
> open-ssl recipe, and really looks to be just providing headers.
>
> I'm doing some builds now to learn more .. which I just did .. and from what
> I
> can see, it is just the headers, which isn't all that useful without the
> kernel
> underpinning.

ocf-linux recipe does infact appear to be just headers. As far as what
it's used for I'm not even sure. I'll ask the crypto guys to chime in
here.

> OCF does definitely accelerate openssl when properly used in both the kernel
> and
> userspace, but I'm not seeing the full offload kernel framework anywhere.

Can't comment what others do, it would be ideal if we got all users
talking here though.

> I wonder if anyone actually uses it :)

Good question, it was added by Saul so maybe he can chime in here.

> But yes AFAIC, if you had a full OCF, you need these to conflict, since
> they'll both
> try and enable/provide cryptodev.

Yashpal,

You should to add

RCONFLICTS_${PN} = "ocf-linux"

and/or maybe

RCONFLICTS_${PN}-dev = "ocf-linux-dev"

to your v2 of the patch.

-M

Patch

diff --git a/meta/recipes-kernel/cryptodev/cryptodev_1.5.bb b/meta/recipes-kernel/cryptodev/cryptodev_1.5.bb
new file mode 100644
index 0000000..5125710
--- /dev/null
+++ b/meta/recipes-kernel/cryptodev/cryptodev_1.5.bb
@@ -0,0 +1,18 @@ 
+SECTION = "devel"
+SUMMARY = "Linux Cryptodev KERNEL MODULE"
+DESCRIPTION = "The Cryptodev package contains the kernel /dev/crypto module"
+LICENSE = "GPLv2"
+LIC_FILES_CHKSUM = "file://COPYING;md5=b234ee4d69f5fce4486a80fdaf4a4263"
+
+DEPENDS = "virtual/kernel"
+
+inherit module
+
+SRCREV = "1c24a0aa996630518d47826a2e3fea129ea094c7"
+
+SRC_URI = "git://repo.or.cz/cryptodev-linux.git;protocol=git \
+	  file://makefile_fixup.patch"
+
+EXTRA_OEMAKE='KERNEL_DIR="${STAGING_KERNEL_DIR}" PREFIX="${D}"'
+
+S = "${WORKDIR}/git"
diff --git a/meta/recipes-kernel/cryptodev/files/makefile_fixup.patch b/meta/recipes-kernel/cryptodev/files/makefile_fixup.patch
new file mode 100644
index 0000000..323aacd
--- /dev/null
+++ b/meta/recipes-kernel/cryptodev/files/makefile_fixup.patch
@@ -0,0 +1,26 @@ 
+diff --git a/Makefile b/Makefile
+index 2be8825..b36d68c 100644
+--- a/Makefile
++++ b/Makefile
+@@ -1,6 +1,7 @@
+ KBUILD_CFLAGS += -I$(src)
+ KERNEL_DIR = /lib/modules/$(shell uname -r)/build
+ VERSION = 1.5
++PREFIX =
+ 
+ cryptodev-objs = ioctl.o main.o cryptlib.o authenc.o zc.o util.o
+ 
+@@ -12,10 +13,10 @@ build: version.h
+ version.h: Makefile
+ 	@echo "#define VERSION \"$(VERSION)\"" > version.h
+ 
+-install:
++modules_install:
+ 	make -C $(KERNEL_DIR) SUBDIRS=`pwd` modules_install
+-	@echo "Installing cryptodev.h in /usr/include/crypto ..."
+-	@install -D crypto/cryptodev.h /usr/include/crypto/cryptodev.h
++	@echo "Installing cryptodev.h in $(PREFIX)/usr/include/crypto ..."
++	@install -D crypto/cryptodev.h $(PREFIX)/usr/include/crypto/cryptodev.h
+ 
+ clean:
+ 	make -C $(KERNEL_DIR) SUBDIRS=`pwd` clean