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

[Fruhwirth Clemens <clemens@endorphin.org>]

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

On Tue, 2005-01-25 at 10:52 -0500, James Morris wrote:
> On Mon, 24 Jan 2005, Andrew Morton wrote:
> 
> > These patches clash badly with Michael Ludvig's work:
> > 
> > ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.11-rc2/2.6.11-rc2-mm1/broken-out/cryptoapi-prepare-for-processing-multiple-buffers-at.patch
> > ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.11-rc2/2.6.11-rc2-mm1/broken-out/cryptoapi-update-padlock-to-process-multiple-blocks-at.patch
> > 
> > so someone's going to have to rework things.  Ordinarily Michael would go
> > first due to test coverage.
> > 
> > James, your call please.  Also, please advise on the suitability of
> > Michael's patches for a 2.6.11 merge.
>
> Perhaps temporarily drop the multible block changes above until we get the
> generic scatterwalk code in and a cleaned up design to handle cipher mode
> offload.

Andrew, do you agree with James on dropping this patches temporarily?
I'm running into a mess with patches for patches, and I'd be easier for
me to have my scatterwalk code in -mm to build on.

James, anything new on ipsec testing? Is there something else missing
for a "GO" from your side for scatterwalk generic?

I'm almost finished with my port of Michaels multiblock extensions, but
I run into a few single problems.

First, I'd set the bytes, a multiblock call can digest, to 4096, page
size. Why? Because, the scatterwalk code, even James original
implementation, will trigger heavy memcpy because the needscratch check
will always return true for page boundary crossing sections.

ATM max_nbytes isn't set to 4096, but to ((size_t)-1), the maximum value
of size_t. This is algorithm specific and set in padlock implementation.
(My port will drop these changes). But setting it to 4096 causes another
problem: the last fragment of a run might be shorter than 4096, but the
scatterwalk code (James and mine) wasn't designed to
change the stepsize/blocksize dynamically. Therefore, Michaels addition
to crypt(..) will wrongly process the whole last 4096 block, trashing
all data remaining data. That's not likely to break things, but the
behavior is certainly wrong.

So a lot of slippery details here. My advise is, drop Michaels patches
for now, merge scatterwalker and add an ability to change the stepsize
dynamically in the run. Then I will finish my port and post it.

If we can agree on this "agenda", I'll shift my focus to scatterwalker
testing.

-- 
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 --]