2008-08-08 18:31:59

by Shasi Pulijala

[permalink] [raw]
Subject: OpenSSL patch to support Linux CryptoAPI.


Hi,

OpenSSL patch to support Linux CryptoAPI via CryptoDev interface. This patch depends on OCF patch to OpenSSL. After applying OCF patch to OpenSSL, apply this patch which will replace the lower interface into the Linux kernel.

-Shasi


Attachments:
0001-Cryptodev-OpenSSL-Patch.txt (15.85 kB)
0001-Cryptodev-OpenSSL-Patch.txt

2008-08-08 21:09:37

by Evgeniy Polyakov

[permalink] [raw]
Subject: Re: OpenSSL patch to support Linux CryptoAPI.

Hi.

On Fri, Aug 08, 2008 at 11:31:58AM -0700, Shasi Pulijala ([email protected]) wrote:
> struct crypt_op {
> - u_int32_t ses;
> - u_int16_t op; /* i.e. COP_ENCRYPT */
> #define COP_NONE 0
> #define COP_ENCRYPT 1
> #define COP_DECRYPT 2
> - u_int16_t flags;
> -#define COP_F_BATCH 0x0008 /* Batch op if possible */
> - u_int len;
> - caddr_t src, dst; /* become iov[] inside kernel */
> - caddr_t mac; /* must be big enough for chosen MAC */
> - caddr_t iv;
> + __u16 op; /* i.e. COP_ENCRYPT */
> + __u16 flags;
> + __u16 iv_size;
> + __u16 assoc_size;
> + __u32 src_size;
> + caddr_t src_data;
> + caddr_t dst_data;
> + __u8 data[0]; /* must be big enough for chosen MAC */
> };

If above caddr_t is what I thought (i.e. a pointer or long type),
there is no way it can be correct. It is _NOT_ allowed to put
variable sized members into structures shared between kernel
and userspace.

--
Evgeniy Polyakov

2008-08-08 22:24:26

by Loc Ho

[permalink] [raw]
Subject: RE: OpenSSL patch to support Linux CryptoAPI.

Hi,

This sound like the only solution to passing more than one pointers from user space is via custom system call - such as (new) crypto_op(...). Am I correct?

-Loc

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Evgeniy Polyakov
Sent: Friday, August 08, 2008 2:10 PM
To: Shasi Pulijala
Cc: [email protected]; Loc Ho
Subject: Re: OpenSSL patch to support Linux CryptoAPI.

Hi.

On Fri, Aug 08, 2008 at 11:31:58AM -0700, Shasi Pulijala ([email protected]) wrote:
> struct crypt_op {
> - u_int32_t ses;
> - u_int16_t op; /* i.e. COP_ENCRYPT */
> #define COP_NONE 0
> #define COP_ENCRYPT 1
> #define COP_DECRYPT 2
> - u_int16_t flags;
> -#define COP_F_BATCH 0x0008 /* Batch op if possible */
> - u_int len;
> - caddr_t src, dst; /* become iov[] inside kernel */
> - caddr_t mac; /* must be big enough for chosen MAC */
> - caddr_t iv;
> + __u16 op; /* i.e. COP_ENCRYPT */
> + __u16 flags;
> + __u16 iv_size;
> + __u16 assoc_size;
> + __u32 src_size;
> + caddr_t src_data;
> + caddr_t dst_data;
> + __u8 data[0]; /* must be big enough for chosen MAC */
> };

If above caddr_t is what I thought (i.e. a pointer or long type),
there is no way it can be correct. It is _NOT_ allowed to put
variable sized members into structures shared between kernel
and userspace.

--
Evgeniy Polyakov

2008-08-09 06:29:27

by Evgeniy Polyakov

[permalink] [raw]
Subject: Re: OpenSSL patch to support Linux CryptoAPI.

Hi.

On Fri, Aug 08, 2008 at 03:24:20PM -0700, Loc Ho ([email protected]) wrote:
> This sound like the only solution to passing more than one pointers from user space is via custom system call - such as (new) crypto_op(...). Am I correct?

No, it is not a right solution. Linux has so ugly compat layer to fix
this problems, that it is much better not to look at it before the
dinner.

You do not need to pass multiple pointers, but instead a multiple data.
You can do that via single area provided to the kernel and multiple size
fields, where each one corresponds to the size of the contiguous sectors
in the provided data.

--
Evgeniy Polyakov

2008-08-11 18:53:47

by Loc Ho

[permalink] [raw]
Subject: RE: OpenSSL patch to support Linux CryptoAPI.

Hi,

>
>You do not need to pass multiple pointers, but instead a multiple data.
>You can do that via single area provided to the kernel and multiple size fields, where each
> one corresponds to the size of the contiguous sectors in the provided data.

[Loc Ho]
It sounds like the solution is to format the data (parameter values, src, dst) into a single buffer. This will require memory copy of the source and destination data as follow:
1. User space formating the user space buffer that includes:
1a) size parameter fields (this is required regardless)
2b) parameter data such as IV, adata, and etc (this is required regardless)
3c) copy the source data from application buffer into this single buffer (this is extra memcpy)
2. Kernel operation of the single user buffer (this is required regardless)
3. User space copy the destination data buffer into its appropriate memory pointer
3a) For hash, it is just the hash
3b) For crypto, it is the entire buffer (= to source length)
3c) For aead, it is the entire buffer + aead ICV

As you can see, there is two extra memcpy's. There is noticeable performance on our SoC (AMCC 4xx) which memory copy is performed.

I was thinking. What do you think about passing multiple data pointer using writev (vector write)? And possible a similar idea for AIO.

-Loc



2008-08-11 19:53:16

by Evgeniy Polyakov

[permalink] [raw]
Subject: Re: OpenSSL patch to support Linux CryptoAPI.

Hi.

On Mon, Aug 11, 2008 at 11:53:41AM -0700, Loc Ho ([email protected]) wrote:
> >You do not need to pass multiple pointers, but instead a multiple data.
> >You can do that via single area provided to the kernel and multiple size fields, where each
> > one corresponds to the size of the contiguous sectors in the provided data.
>
> [Loc Ho]
> It sounds like the solution is to format the data (parameter values, src, dst) into a single buffer. This will require memory copy of the source and destination data as follow:
> 1. User space formating the user space buffer that includes:
> 1a) size parameter fields (this is required regardless)
> 2b) parameter data such as IV, adata, and etc (this is required regardless)
> 3c) copy the source data from application buffer into this single buffer (this is extra memcpy)
> 2. Kernel operation of the single user buffer (this is required regardless)
> 3. User space copy the destination data buffer into its appropriate memory pointer
> 3a) For hash, it is just the hash
> 3b) For crypto, it is the entire buffer (= to source length)
> 3c) For aead, it is the entire buffer + aead ICV
>
> As you can see, there is two extra memcpy's. There is noticeable performance on our SoC (AMCC 4xx) which memory copy is performed.

It will be buried in noise compared to crypto processing time, but you
still can try to optimize it.

> I was thinking. What do you think about passing multiple data pointer using writev (vector write)? And possible a similar idea for AIO.

Yes, that's a good approach.

Another one is to use a dedicated syscall.
It is up to you as developer decide, since all of them have advantages.

--
Evgeniy Polyakov