[PATCH] CRYPTO: Simplify code using ARRAY_SIZE() macro.

[PATCH] CRYPTO: Simplify code using ARRAY_SIZE() macro.

[Robert P. J. Day]

Signed-off-by: Robert P. J. Day <rpjday@crashcourse.ca>

---

diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c
index 81f3f95..c4c018d 100644
--- a/drivers/crypto/hifn_795x.c
+++ b/drivers/crypto/hifn_795x.c
@@ -894,7 +894,7 @@ static int hifn_enable_crypto(struct hifn_device *dev)
 	char *offtbl = NULL;
 	int i;

-	for (i = 0; i < sizeof(pci2id)/sizeof(pci2id[0]); i++) {
+	for (i = 0; i < ARRAY_SIZE(pci2id)); i++) {
 		if (pci2id[i].pci_vendor == dev->pdev->vendor &&
 				pci2id[i].pci_prod == dev->pdev->device) {
 			offtbl = pci2id[i].card_id;

========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry:
    Have classroom, will lecture.

http://crashcourse.ca                          Waterloo, Ontario, CANADA
========================================================================

Re: [PATCH] CRYPTO: Simplify code using ARRAY_SIZE() macro.

[Herbert Xu]

On Sun, May 25, 2008 at 11:32:45AM +0000, Robert P. J. Day wrote:
> 
> Signed-off-by: Robert P. J. Day <rpjday@crashcourse.ca>
> 
> ---
> 
> diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c
> index 81f3f95..c4c018d 100644
> --- a/drivers/crypto/hifn_795x.c
> +++ b/drivers/crypto/hifn_795x.c
> @@ -894,7 +894,7 @@ static int hifn_enable_crypto(struct hifn_device *dev)
>  	char *offtbl = NULL;
>  	int i;
> 
> -	for (i = 0; i < sizeof(pci2id)/sizeof(pci2id[0]); i++) {
> +	for (i = 0; i < ARRAY_SIZE(pci2id)); i++) {

You've got an extra parenthesis here.  You should be building the
patches that you send, no matter how trivial they might look!

I've fixed it for you this time.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] CRYPTO: Simplify code using ARRAY_SIZE() macro.

[Robert P. J. Day]

On Mon, 26 May 2008, Herbert Xu wrote:

> On Sun, May 25, 2008 at 11:32:45AM +0000, Robert P. J. Day wrote:
> >
> > Signed-off-by: Robert P. J. Day <rpjday@crashcourse.ca>
> >
> > ---
> >
> > diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c
> > index 81f3f95..c4c018d 100644
> > --- a/drivers/crypto/hifn_795x.c
> > +++ b/drivers/crypto/hifn_795x.c
> > @@ -894,7 +894,7 @@ static int hifn_enable_crypto(struct hifn_device *dev)
> >  	char *offtbl = NULL;
> >  	int i;
> >
> > -	for (i = 0; i < sizeof(pci2id)/sizeof(pci2id[0]); i++) {
> > +	for (i = 0; i < ARRAY_SIZE(pci2id)); i++) {
>
> You've got an extra parenthesis here.  You should be building the
> patches that you send, no matter how trivial they might look!
>
> I've fixed it for you this time.

yes, my apologies.  thanks.

rday
--
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry:
    Have classroom, will lecture.

http://crashcourse.ca                          Waterloo, Ontario, CANADA
========================================================================

IPSec ESP Authenc Offload

[Loc Ho]

Hi Herbert,

For authenc hardware offload outbound, we need to know the whole ESP
header length - IP header + UDP header + ESP header + IV. I am thinking
adding a field in struct aead_givcrypt_request as below:

/**
 *      struct aead_givcrypt_request - AEAD request with IV generation
 *      @seq: Sequence number for IV generation
 *      @giv: Space for generated IV
 *      @areq: The AEAD request itself
 *      @hl: ESP total header length
 */
struct aead_givcrypt_request {
        u64 seq;
        u8 *giv;
        u16 hl;		<===== new ESP total header including IV

        struct aead_request areq;
};


aead_givcrypt_set_giv(req, esph->enc_data,
		      XFRM_SKB_CB(skb)->seq.output,
		      x->props.header_len);

Might be able to use skb_network_offset(skb) instead
x->props.header_len. And of course change function aead_givcrypt_set_giv
to set field hl.

Any other suggest as to how a hardware driver can retrieve this
information besides passing the data packet?

-Loc

Re: IPSec ESP Authenc Offload

[Herbert Xu]

On Tue, May 27, 2008 at 11:29:22AM -0700, Loc Ho wrote:
> 
> For authenc hardware offload outbound, we need to know the whole ESP
> header length - IP header + UDP header + ESP header + IV. I am thinking
> adding a field in struct aead_givcrypt_request as below:

Could you please let me know why it needs this information?
Is it doing ESP offload in addition to crypt/hash offload?

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

RE: IPSec ESP Authenc Offload

[Loc Ho]

Hi,

With IPSec ESP Authenc, it is expected that the selected driver
generates "IV" as well as encrypts the data. Our 'hardware' (available
currently), can only handle either no header processing or header
processing (from ESP to IV processing but not individual field
processing). 

For no header processing, we will have to do a lot more work in software
- create a context SA for each requested operation, copy from the
initial context SA, after the operation completed, retrieve the update
IV from context SA, and then write it back to the packet.

For header processing, tell hardware to skip IP header (- ESP header -
IV), write SPI, SEQ, and IV. (This is all handled by hardware with the
exception of compute the skipped length of IP header.) It does write the
SPI and SEQ again but it is handled by the hardware and with the same
value as software. Alternatinely, we can parse the IP header for the IP
header length but this information is already available in IPSec statck,
would not work with UDP encapsulation, and would be cleaner.

-Loc 

-----Original Message-----
From: linux-crypto-owner@vger.kernel.org
[mailto:linux-crypto-owner@vger.kernel.org] On Behalf Of Herbert Xu
Sent: Tuesday, May 27, 2008 11:35 PM
To: Loc Ho
Cc: linux-crypto@vger.kernel.org
Subject: Re: IPSec ESP Authenc Offload

On Tue, May 27, 2008 at 11:29:22AM -0700, Loc Ho wrote:
> 
> For authenc hardware offload outbound, we need to know the whole ESP 
> header length - IP header + UDP header + ESP header + IV. I am 
> thinking adding a field in struct aead_givcrypt_request as below:

Could you please let me know why it needs this information?
Is it doing ESP offload in addition to crypt/hash offload?

Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au> Home Page:
http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto"
in the body of a message to majordomo@vger.kernel.org More majordomo
info at  http://vger.kernel.org/majordomo-info.html

Re: IPSec ESP Authenc Offload

[Herbert Xu]

On Wed, May 28, 2008 at 09:42:47AM -0700, Loc Ho wrote:
> Hi,
> 
> With IPSec ESP Authenc, it is expected that the selected driver
> generates "IV" as well as encrypts the data. Our 'hardware' (available
> currently), can only handle either no header processing or header
> processing (from ESP to IV processing but not individual field
> processing). 
> 
> For no header processing, we will have to do a lot more work in software
> - create a context SA for each requested operation, copy from the
> initial context SA, after the operation completed, retrieve the update
> IV from context SA, and then write it back to the packet.

Do you still need to do this if we used a software-generated IV?

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

RE: IPSec ESP Authenc Offload

[Loc Ho]

Hi,

It doesn't help if it is generated by software. The driver still needs a
context SA for each operation. In addition, the driver will have to
increment seq (or load from request) and load SEQ and IV into each
context SA. It is much cleaner if our driver knows the whole header
length. Even if the hardware rewrites the SPI and SEQ again, it is all
handled by hardware offload and will not be a problem for IPSEC ESP.

-Loc


-----Original Message-----
From: Herbert Xu [mailto:herbert@gondor.apana.org.au] 
Sent: Wednesday, May 28, 2008 3:23 PM
To: Loc Ho
Cc: linux-crypto@vger.kernel.org
Subject: Re: IPSec ESP Authenc Offload

On Wed, May 28, 2008 at 09:42:47AM -0700, Loc Ho wrote:
> Hi,
> 
> With IPSec ESP Authenc, it is expected that the selected driver 
> generates "IV" as well as encrypts the data. Our 'hardware' (available

> currently), can only handle either no header processing or header 
> processing (from ESP to IV processing but not individual field 
> processing).
> 
> For no header processing, we will have to do a lot more work in 
> software
> - create a context SA for each requested operation, copy from the 
> initial context SA, after the operation completed, retrieve the update

> IV from context SA, and then write it back to the packet.

Do you still need to do this if we used a software-generated IV?

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au> Home Page:
http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: IPSec ESP Authenc Offload

[Herbert Xu]

On Wed, May 28, 2008 at 04:02:11PM -0700, Loc Ho wrote:
> 
> It doesn't help if it is generated by software. The driver still needs a
> context SA for each operation. In addition, the driver will have to
> increment seq (or load from request) and load SEQ and IV into each
> context SA. It is much cleaner if our driver knows the whole header
> length. Even if the hardware rewrites the SPI and SEQ again, it is all
> handled by hardware offload and will not be a problem for IPSEC ESP.

I'm happy to add support for ESP offload.  However, I don't think
we should add it onto the AEAD interface.  We should instead create
an ESP interface that specifically does this.

I still think that you can use the existing interface though and
just throw away the ESP work since that's trivial anyway.  Having
a context SA is not a problem since each tfm corresponds to a single
SA and you can just store the context in its context area.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

RE: IPSec ESP Authenc Offload

[Loc Ho]

Hi,

See inline...


-----Original Message-----
From: Herbert Xu [mailto:herbert@gondor.apana.org.au] 
Sent: Thursday, May 29, 2008 12:02 AM
To: Loc Ho
Cc: linux-crypto@vger.kernel.org
Subject: Re: IPSec ESP Authenc Offload

On Wed, May 28, 2008 at 04:02:11PM -0700, Loc Ho wrote:
>> 
>> It doesn't help if it is generated by software. The driver still
needs 
>> a context SA for each operation. In addition, the driver will have to

>> increment seq (or load from request) and load SEQ and IV into each 
>> context SA. It is much cleaner if our driver knows the whole header 
>> length. Even if the hardware rewrites the SPI and SEQ again, it is
all 
>> handled by hardware offload and will not be a problem for IPSEC ESP.
>
>I'm happy to add support for ESP offload.  However, I don't think we
should add it onto the 
>AEAD interface.  We should instead create an ESP interface that
specifically does this.
>

[Loc Ho]
I agree if we are doing complete ESP offload. Right now, we just want to
handle 'authenc' offload and not hanving to deal with complete offload.

>I still think that you can use the existing interface though and just
throw away the ESP 
>work since that's trivial anyway.  Having a context SA is not a problem
since each tfm
>corresponds to a single SA and you can just store the context in its
context area.
>

[Loc Ho]
I would like to use the existent interface and it does work. Except, it
will require extra works such as 1) another context SA per an operation
(this is in addition to the context SA with the tfm), 2) as there are
now multiple context SA's, they need to be in sync, and 3) copy fields
from one context SA to another. We can not use the same context SA for
per operation for following reasons:
	1. For hardware generated IV, we need to store the IV in per
context SA per operation so that we can write back into the packet by
software.
	2. For software generated IV, we need to store the IV in per
context SA per operation for the hardware to access for crypto
algorithm. 

Alternative, we can parse the IP header if this information is not
available from the request 'struct skcipher_givcrypt_request'. This
would avoid per context SA per operation. But this is extra work as it
is already available in IPSec ESP.

-Loc

Re: IPSec ESP Authenc Offload

[Herbert Xu]

On Thu, May 29, 2008 at 10:44:47AM -0700, Loc Ho wrote:
> 
> [Loc Ho]
> I would like to use the existent interface and it does work. Except, it
> will require extra works such as 1) another context SA per an operation
> (this is in addition to the context SA with the tfm), 2) as there are
> now multiple context SA's, they need to be in sync, and 3) copy fields
> from one context SA to another. We can not use the same context SA for
> per operation for following reasons:

Is it possible to have a look at your hardware spec?

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt