All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers@kernel.org>
To: Mike Snitzer <snitzer@redhat.com>, Jens Axboe <axboe@kernel.dk>
Cc: linux-block@vger.kernel.org,
	Satya Tangirala <satyaprateek2357@gmail.com>,
	dm-devel@redhat.com, linux-mmc@vger.kernel.org,
	linux-scsi@vger.kernel.org, Ulf Hansson <ulf.hansson@linaro.org>
Subject: Re: [PATCH v4 3/4] blk-crypto: rename blk_keyslot_manager to blk_crypto_profile
Date: Wed, 6 Oct 2021 12:19:48 -0700	[thread overview]
Message-ID: <YV321JFYV/u7pbsO@gmail.com> (raw)
In-Reply-To: <YV2kdHeS4GTXUdpi@redhat.com>

On Wed, Oct 06, 2021 at 09:28:20AM -0400, Mike Snitzer wrote:
> On Wed, Sep 29 2021 at 12:35P -0400,
> Eric Biggers <ebiggers@kernel.org> wrote:
> 
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > blk_keyslot_manager is misnamed because it doesn't necessarily manage
> > keyslots.  It actually does several different things:
> > 
> >   - Contains the crypto capabilities of the device.
> > 
> >   - Provides functions to control the inline encryption hardware.
> >     Originally these were just for programming/evicting keyslots;
> >     however, new functionality (hardware-wrapped keys) will require new
> >     functions here which are unrelated to keyslots.  Moreover,
> >     device-mapper devices already (ab)use "keyslot_evict" to pass key
> >     eviction requests to their underlying devices even though
> >     device-mapper devices don't have any keyslots themselves (so it
> >     really should be "evict_key", not "keyslot_evict").
> > 
> >   - Sometimes (but not always!) it manages keyslots.  Originally it
> >     always did, but device-mapper devices don't have keyslots
> >     themselves, so they use a "passthrough keyslot manager" which
> >     doesn't actually manage keyslots.  This hack works, but the
> >     terminology is unnatural.  Also, some hardware doesn't have keyslots
> >     and thus also uses a "passthrough keyslot manager" (support for such
> >     hardware is yet to be upstreamed, but it will happen eventually).
> > 
> > Let's stop having keyslot managers which don't actually manage keyslots.
> > Instead, rename blk_keyslot_manager to blk_crypto_profile.
> > 
> > This is a fairly big change, since for consistency it also has to update
> > keyslot manager-related function names, variable names, and comments --
> > not just the actual struct name.  However it's still a fairly
> > straightforward change, as it doesn't change any actual functionality.
> > 
> > Acked-by: Ulf Hansson <ulf.hansson@linaro.org> # For MMC
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> 
> Unfortunate how fiddley this change forced you to get but it looks
> like you've done a very solid job of cleaning it all up to be
> consistent.
> 
> Reviewed-by: Mike Snitzer <snitzer@redhat.com>
> 

Thanks for the reviews!  Yes, we should have done it this way originally which
would have saved some pain, but better late than never.

Jens, anything else you're waiting for before applying this series?  Note that
I'm not sure that Satya will leave any feedback, given that he's no longer
working for Google, so any kernel work he does is in his free time.

- Eric

WARNING: multiple messages have this Message-ID
From: Eric Biggers <ebiggers@kernel.org>
To: Mike Snitzer <snitzer@redhat.com>, Jens Axboe <axboe@kernel.dk>
Cc: Satya Tangirala <satyaprateek2357@gmail.com>,
	Ulf Hansson <ulf.hansson@linaro.org>,
	linux-scsi@vger.kernel.org, linux-mmc@vger.kernel.org,
	linux-block@vger.kernel.org, dm-devel@redhat.com
Subject: Re: [dm-devel] [PATCH v4 3/4] blk-crypto: rename blk_keyslot_manager to blk_crypto_profile
Date: Wed, 6 Oct 2021 12:19:48 -0700	[thread overview]
Message-ID: <YV321JFYV/u7pbsO@gmail.com> (raw)
In-Reply-To: <YV2kdHeS4GTXUdpi@redhat.com>

On Wed, Oct 06, 2021 at 09:28:20AM -0400, Mike Snitzer wrote:
> On Wed, Sep 29 2021 at 12:35P -0400,
> Eric Biggers <ebiggers@kernel.org> wrote:
> 
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > blk_keyslot_manager is misnamed because it doesn't necessarily manage
> > keyslots.  It actually does several different things:
> > 
> >   - Contains the crypto capabilities of the device.
> > 
> >   - Provides functions to control the inline encryption hardware.
> >     Originally these were just for programming/evicting keyslots;
> >     however, new functionality (hardware-wrapped keys) will require new
> >     functions here which are unrelated to keyslots.  Moreover,
> >     device-mapper devices already (ab)use "keyslot_evict" to pass key
> >     eviction requests to their underlying devices even though
> >     device-mapper devices don't have any keyslots themselves (so it
> >     really should be "evict_key", not "keyslot_evict").
> > 
> >   - Sometimes (but not always!) it manages keyslots.  Originally it
> >     always did, but device-mapper devices don't have keyslots
> >     themselves, so they use a "passthrough keyslot manager" which
> >     doesn't actually manage keyslots.  This hack works, but the
> >     terminology is unnatural.  Also, some hardware doesn't have keyslots
> >     and thus also uses a "passthrough keyslot manager" (support for such
> >     hardware is yet to be upstreamed, but it will happen eventually).
> > 
> > Let's stop having keyslot managers which don't actually manage keyslots.
> > Instead, rename blk_keyslot_manager to blk_crypto_profile.
> > 
> > This is a fairly big change, since for consistency it also has to update
> > keyslot manager-related function names, variable names, and comments --
> > not just the actual struct name.  However it's still a fairly
> > straightforward change, as it doesn't change any actual functionality.
> > 
> > Acked-by: Ulf Hansson <ulf.hansson@linaro.org> # For MMC
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> 
> Unfortunate how fiddley this change forced you to get but it looks
> like you've done a very solid job of cleaning it all up to be
> consistent.
> 
> Reviewed-by: Mike Snitzer <snitzer@redhat.com>
> 

Thanks for the reviews!  Yes, we should have done it this way originally which
would have saved some pain, but better late than never.

Jens, anything else you're waiting for before applying this series?  Note that
I'm not sure that Satya will leave any feedback, given that he's no longer
working for Google, so any kernel work he does is in his free time.

- Eric

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


  reply	other threads:[~2021-10-06 19:19 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-29 16:35 [PATCH v4 0/4] blk-crypto cleanups Eric Biggers
2021-09-29 16:35 ` [dm-devel] " Eric Biggers
2021-09-29 16:35 ` [PATCH v4 1/4] blk-crypto-fallback: properly prefix function and struct names Eric Biggers
2021-09-29 16:35   ` [dm-devel] " Eric Biggers
2021-10-06 13:24   ` Mike Snitzer
2021-10-06 13:24     ` [dm-devel] " Mike Snitzer
2021-09-29 16:35 ` [PATCH v4 2/4] blk-crypto: rename keyslot-manager files to blk-crypto-profile Eric Biggers
2021-09-29 16:35   ` [dm-devel] " Eric Biggers
2021-10-06 13:25   ` Mike Snitzer
2021-10-06 13:25     ` [dm-devel] " Mike Snitzer
2021-09-29 16:35 ` [PATCH v4 3/4] blk-crypto: rename blk_keyslot_manager to blk_crypto_profile Eric Biggers
2021-09-29 16:35   ` [dm-devel] " Eric Biggers
2021-10-06 13:28   ` Mike Snitzer
2021-10-06 13:28     ` [dm-devel] " Mike Snitzer
2021-10-06 19:19     ` Eric Biggers [this message]
2021-10-06 19:19       ` Eric Biggers
2021-09-29 16:36 ` [PATCH v4 4/4] blk-crypto: update inline encryption documentation Eric Biggers
2021-09-29 16:36   ` [dm-devel] " Eric Biggers
2021-10-06 13:29   ` Mike Snitzer
2021-10-06 13:29     ` [dm-devel] " Mike Snitzer
2021-10-05 17:26 ` [PATCH v4 0/4] blk-crypto cleanups Eric Biggers
2021-10-05 17:26   ` [dm-devel] " Eric Biggers

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=YV321JFYV/u7pbsO@gmail.com \
    --to=ebiggers@kernel.org \
    --cc=axboe@kernel.dk \
    --cc=dm-devel@redhat.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=satyaprateek2357@gmail.com \
    --cc=snitzer@redhat.com \
    --cc=ulf.hansson@linaro.org \
    --subject='Re: [PATCH v4 3/4] blk-crypto: rename blk_keyslot_manager to blk_crypto_profile' \
    /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

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.