Re: [PATCH 01/04] Adding cipher mode context information to crypto_tfm

[Fruhwirth Clemens <clemens@endorphin.org>]

[-- Attachment #1: Type: text/plain, Size: 1781 bytes --]

On Wed, 2005-02-02 at 17:46 -0500, James Morris wrote:
> On Sun, 30 Jan 2005, Fruhwirth Clemens wrote:
> > James, please test it against ipsec. I'm confident, that everything will
> > work as expected and we can proceed to merge padlock-multiblock against this
> > scatterwalker, so please Andrew, merge after a successful test of James.
> 
> This code tests ok with IPSec, and delivers some good performance
> improvements.  e.g. FTP transfers over transport mode AES over GigE sped
> up with this patch applied on one side of the connection by 20% for send
> and 15% for receive.

Fine, nice to hear that!

> There are quite a few coding style and minor issues to be fixed (per 
> below), and the code should probably then be tested in the -mm tree for a 
> while (2.6.11 is too soon for mainline merge).
> 
> > +static int ecb_process_gw(void *_priv, int nsg, void **buf) 

> What does _gw mean?

generic walker.. can be removed, if you like.

> > +struct cbc_process_priv {
> > +	struct crypto_tfm *tfm;
> > +	int enc;
> > +	cryptfn_t *crfn;
> > +	u8 *curIV;
> > +	u8 *nextIV;
> > +};
> 
> No caps please, I suggest curr_iv and next_iv.

Ack, cipher.c is underscore style. But my LRW private helper lib
gfmulseq.c is going to stay lowerCamelCase. I hope that's ok for
everyone. If not, the one concerned should post a reformat patch.

> > +		r = pf(priv, nsl, dispatch_list);
> > +		if(unlikely(r < 0))
> > +			goto out;
> 
> Not sure if the unlikely() is justified here, given that this is not a 
> fast path.  I guess it doesn't do any harm.

I suspected unlikely to be a hint for the compiler to do better jump
prediction and speculations. Remove?

-- 
Fruhwirth Clemens <clemens@endorphin.org>  http://clemens.endorphin.org

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]