linux-crypto.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Ondrej Mosnáček" <omosnacek@gmail.com>
To: Binoy Jayan <binoy.jayan@linaro.org>
Cc: Herbert Xu <herbert@gondor.apana.org.au>,
	linux-crypto@vger.kernel.org, dm-devel@redhat.com,
	Mike Snitzer <snitzer@redhat.com>,
	Milan Broz <gmazyland@gmail.com>,
	Mikulas Patocka <mpatocka@redhat.com>,
	Mark Brown <broonie@kernel.org>, Arnd Bergmann <arnd@arndb.de>
Subject: Re: [RFC PATCH 6/6] dm-crypt: Add bulk crypto processing support
Date: Tue, 17 Jan 2017 12:15:59 +0100	[thread overview]
Message-ID: <CAAUqJDuAKYbxNEEct7G+B8Rt4XpenXy2KCmBdfUuc5O-4UmShw@mail.gmail.com> (raw)
In-Reply-To: <CAHv-k_92j=C8AOzfFzyuiJO=NXWh=zP3=N8MqdyqSUUZf9Hzzw@mail.gmail.com>

Hi Binoy,

2017-01-16 9:37 GMT+01:00 Binoy Jayan <binoy.jayan@linaro.org>:
> The initial goal of our proposal was to process the encryption requests with the
> maximum possible block sizes with a hardware which has automated iv generation
> capabilities. But when it is done in software, and if the bulk
> requests are processed
> sequentially, one block at a time, the memory foot print could be
> reduced even if
> the bulk request exceeds a page. While your patch looks good, there
> are couple of
> drawbacks one of which is the maximum size of a bulk request is a page. This
> could limit the capability of the crypto hardware. If the whole bio is
> processed at
> once, which is what qualcomm's version of dm-req-crypt does, it achieves an even
> better performance.

I see... well, I added the limit only so that the async fallback
implementation can allocate multiple requests, so they can be
processed in parallel, as they would be in the current dm-crypt code.
I'm not really sure if that brings any benefit, but I guess if some HW
accelerator has multiple engines, then this allows distributing the
work among them. (I wonder how switching to the crypto API's IV
generation will affect the situation for drivers that can process
requests in parallel, but do not support the IV generators...)

I could remove the limit and switch the fallback to sequential
processing (or maybe even allocate the requests from a mempool, the
way dm-crypt does it now...), but after Herbert's feedback I'm
probably going to scrap this patchset anyway...

>> Note that if the 'keycount' parameter of the cipher specification is set to a
>> value other than 1, dm-crypt still sends only one sector in each request, since
>> in such case the neighboring sectors are encrypted with different keys.
>
> This could be avoided if the key management is done at the crypto layer.

Yes, but remember that the only reasonable use-case for using keycount
!= 1 is mounting loop-AES partitions (which is kind of a legacy
format, so there is not much point in making HW drivers for it). It is
an unfortunate consequence of Milan's decision to make keycount an
independent part of the cipher specification (instead of making it
specific for the LMK mode), that all the other IV modes are now
'polluted' with the requirement to support it.

I discussed with Milan the possibility of deprecating the keycount
parameter (i.e. allowing only value of 64 for LMK and 1 for all the
other IV modes) and then converting the IV modes to skciphers (or IV
generators, or some combination of both). This would significantly
simplify the key management and allow for better optimization
strategies. However, I don't know if such change would be accepted by
device-mapper maintainers, since it may break someone's unusual
dm-crypt configuration...

Cheers,
Ondrej

  reply	other threads:[~2017-01-17 11:16 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-12 12:59 [RFC PATCH 0/6] Add bulk skcipher requests to crypto API and dm-crypt Ondrej Mosnacek
2017-01-12 12:59 ` [RFC PATCH 1/6] crypto: skcipher - Add bulk request processing API Ondrej Mosnacek
2017-01-12 12:59 ` [RFC PATCH 2/6] crypto: skcipher - Add bulk request support to walk Ondrej Mosnacek
2017-01-12 12:59 ` [RFC PATCH 3/6] crypto: cryptd - Add skcipher bulk request support Ondrej Mosnacek
2017-01-12 12:59 ` [RFC PATCH 4/6] crypto: simd - Add " Ondrej Mosnacek
2017-01-12 12:59 ` [RFC PATCH 5/6] crypto: aesni-intel " Ondrej Mosnacek
2017-01-13  3:19   ` Eric Biggers
2017-01-13 11:27     ` Ondrej Mosnáček
2017-01-12 12:59 ` [RFC PATCH 6/6] dm-crypt: Add bulk crypto processing support Ondrej Mosnacek
2017-01-16  8:37   ` Binoy Jayan
2017-01-17 11:15     ` Ondrej Mosnáček [this message]
2017-01-13 10:41 ` [RFC PATCH 0/6] Add bulk skcipher requests to crypto API and dm-crypt Herbert Xu
2017-01-13 12:01   ` Ondrej Mosnáček
2017-01-13 14:29     ` Herbert Xu
2017-01-17 11:20       ` Ondrej Mosnáček
2017-01-18  4:48         ` Herbert Xu
2017-01-19 14:21           ` Ondrej Mosnáček
2017-01-23 13:04             ` Herbert Xu
2017-01-18 17:09     ` Binoy Jayan

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=CAAUqJDuAKYbxNEEct7G+B8Rt4XpenXy2KCmBdfUuc5O-4UmShw@mail.gmail.com \
    --to=omosnacek@gmail.com \
    --cc=arnd@arndb.de \
    --cc=binoy.jayan@linaro.org \
    --cc=broonie@kernel.org \
    --cc=dm-devel@redhat.com \
    --cc=gmazyland@gmail.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=linux-crypto@vger.kernel.org \
    --cc=mpatocka@redhat.com \
    --cc=snitzer@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).