All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Herbert Xu <herbert@gondor.apana.org.au>
To: "Horia Geantă" <horia.geanta@freescale.com>
Cc: ruchika.gupta@freescale.com, cristian.stoica@freescale.com,
	NiteshNarayanLal@freescale.com, jinyanjiang@gmail.com,
	Tudor Ambarus <tudor.ambarus@freescale.com>,
	Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
	"Leonidas S. Barbosa" <leosilva@linux.vnet.ibm.com>,
	Fionnuala Gunter <fin@linux.vnet.ibm.com>
Subject: Re: [v2 PATCH 6/8] crypto: caam - Convert GCM to new AEAD interface
Date: Thu, 18 Jun 2015 14:17:34 +0800	[thread overview]
Message-ID: <20150618061734.GA28418@gondor.apana.org.au> (raw)
In-Reply-To: <5581A826.7060907@freescale.com>

On Wed, Jun 17, 2015 at 08:02:30PM +0300, Horia Geantă wrote:
> >  
> > -#define DESC_MAX_USED_BYTES		(DESC_RFC4543_GIVENC_LEN + \
> > -					 CAAM_MAX_KEY_SIZE)
> > -#define DESC_MAX_USED_LEN		(DESC_MAX_USED_BYTES / CAAM_CMD_SZ)
> > +#define DESC_MAX_USED_LEN		(CAAM_DESC_BYTES_MAX - DESC_JOB_IO_LEN)
> 
> This is going to increase the size of caam_ctx struct, but I agree
> previous approach was error-prone.

The problem with the previous code is that it doesn't take into
account the size of the inline key should the key fit into the
64 entries.

However, it appears that I incorrectly removed DESC_MAX_USED_BYTES
and thus made it 4 times bigger than necessary.  I'll fix that up.

> > +	/* skip assoc data */
> > +	append_seq_fifo_store(desc, 0, FIFOST_TYPE_SKIP | FIFOLDST_VLF);
> 
> This wasn't previously needed. I assume it's related to your comment:
> "This series converts various GCM implementations to the new AEAD
> interface.  The main changes [...] both src/dst now contain space at the
> head equal to assoclen, but only src has the actual AD."

Right.  The new interface always includes assoclen bytes in both
src and dst SG lists.  req->assoc is gone.

> > +static void init_gcm_job(struct aead_request *req,
> > +			 struct aead_edesc *edesc,
> > +			 bool all_contig, bool encrypt)
> > +{
> > +	struct crypto_aead *aead = crypto_aead_reqtfm(req);
> > +	struct caam_ctx *ctx = crypto_aead_ctx(aead);
> > +	unsigned int ivsize = crypto_aead_ivsize(aead);
> > +	u32 *desc = edesc->hw_desc;
> > +	bool generic_gcm = (ivsize == 12);
> > +	unsigned int last;
> > +
> > +	init_aead_job(req, edesc, all_contig, encrypt);
> > +
> > +	/* BUG This should not be specific to generic GCM. */
> 
> AFAICT, for non-generic GCM uses (RFC4106, RFC4543), cryptlen and/or
> assoclen are always > 0. That's why the descriptors do not address these
> cases.

Of course.  But with the algif_aead interface you need to at least
ensure that you don't crash or do something silly should the user
give you such an input.  So my question is what happens when it is
zero? Does the hardware simply emit an error and recover, or does it
hang/lock up/do something worse?
 
> Now that GCM is handled separately, is_gcm logic should be removed from
> all old_aead_* functions.

I haven't touched the old_aead_* path at all because there are more
conversions to come.  Once it's all done we can kill all of the
old_aead_* functions.

> > -		sg_to_sec4_sg_last(req->src,
> > -				   src_nents,
> > -				   edesc->sec4_sg +
> > -				   sec4_sg_index, 0);
> > +		sg_to_sec4_sg(req->src, src_nents,
> > +			      edesc->sec4_sg + sec4_sg_index, 0);
> 
> Need to mark end of input S/G, use sg_to_sec4_sg_last() instead.

Thanks I'll fix that up too.

BTW does this actually work on your hardware now?

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

  reply	other threads:[~2015-06-18  6:17 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-16  5:45 [v2 PATCH 0/8] crypto: aead - Convert gcm to new interface Herbert Xu
2015-06-16  5:54 ` [v2 PATCH 1/8] crypto: testmgr - Disable rfc4543 test Herbert Xu
2015-06-16  5:54 ` [v2 PATCH 2/8] crypto: gcm - Convert to new AEAD interface Herbert Xu
2015-06-16  5:54 ` [v2 PATCH 3/8] crypto: testmgr - Update rfc4543 test vectors Herbert Xu
2015-06-16  5:54 ` [v2 PATCH 4/8] crypto: nx - Convert GCM to new AEAD interface Herbert Xu
2015-06-16  5:54 ` [v2 PATCH 5/8] crypto: caam - Handle errors in dma_map_sg_chained Herbert Xu
2015-06-16  5:54 ` [v2 PATCH 6/8] crypto: caam - Convert GCM to new AEAD interface Herbert Xu
2015-06-17 17:02   ` Horia Geantă
2015-06-18  6:17     ` Herbert Xu [this message]
2015-06-18  6:25       ` [PATCH 1/2] crypto: caam - Reintroduce DESC_MAX_USED_BYTES Herbert Xu
2015-06-18  6:25       ` [PATCH 2/2] crypto: caam - Set last bit on src SG list Herbert Xu
2015-06-18 11:18       ` [v2 PATCH 6/8] crypto: caam - Convert GCM to new AEAD interface Horia Geantă
2015-06-19  0:05         ` Herbert Xu
2015-06-16  5:54 ` [v2 PATCH 7/8] Revert "crypto: testmgr - Disable rfc4543 test" Herbert Xu
2015-06-16  5:54 ` [v2 PATCH 8/8] crypto: testmgr - Add mcgrew test vectors for rfc4106 Herbert Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150618061734.GA28418@gondor.apana.org.au \
    --to=herbert@gondor.apana.org.au \
    --cc=NiteshNarayanLal@freescale.com \
    --cc=cristian.stoica@freescale.com \
    --cc=fin@linux.vnet.ibm.com \
    --cc=horia.geanta@freescale.com \
    --cc=jinyanjiang@gmail.com \
    --cc=leosilva@linux.vnet.ibm.com \
    --cc=linux-crypto@vger.kernel.org \
    --cc=ruchika.gupta@freescale.com \
    --cc=tudor.ambarus@freescale.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.