On Fri, May 15, 2020 at 07:42:24AM -0700, Christoph Hellwig wrote: > On Fri, May 15, 2020 at 12:25:40PM +0000, Satya Tangirala wrote: > > One of the nice things about the current design is that regardless of what > > request queue an FS sends an encrypted bio to, blk-crypto will be able to handle > > the encryption (whether by using hardware inline encryption, or using the > > blk-crypto-fallback). The FS itself does not need to worry about what the > > request queue is. > > True. Which just makes me despise that design with the pointless > fallback even more.. The fallback is actually really useful. First, for testing: it allows all the filesystem code that uses inline crypto to be tested using gce-xfstests and kvm-xfstests, so that it's covered by the usual ext4 and f2fs regression testing and it's much easier to develop patches for. It also allowed us to enable the inlinecrypt mount option in Cuttlefish, which is the virtual Android device used to test the Android common kernels. So, it gets the kernel test platform as similar to a real Android device as possible. Ideally we'd implement virtualized inline encryption as you suggested. But these platforms use a mix of VMM's (QEMU, GCE, and crosvm) and storage types (virtio-blk, virtio-scsi, and maybe others; none of these even have an inline encryption standard defined yet). So it's not currently feasible. Second, it creates a clean design where users can just use blk-crypto, and not have to implement a second encryption implementation. For example, I'd eventually like to switch fscrypt over to just use blk-crypto. That would remove the duplicate code that you're concerned about. It would also make it much easier to implement direct I/O support in fscrypt which is something that people have been requesting for a long time. The reason the fscrypt conversion isn't yet part of the patchset is just that I consider it super important that we don't cause any regressions in fscrypt and that it doesn't use inline encryption hardware by default. So it's not quite time to switch over for real yet, especially while the current patches are still pending upstream. But I think it will come eventually, especially if we see that most Linux distros are enabling CONFIG_BLK_INLINE_ENCRYPTION anyway. The inlinecrypt mount option will thten start controlling whether blk-crypto is allowed to to use real hardware or not, not whether blk-crypto is used or not. Also, in the coming months we're planning to implement filesystem metadata encryption that is properly integrated with the fscrypt key derivation so that file contents don't have to be encrypted twice (as would be the case with dm-crypt + fscrypt). That's going to involve adding lots of encryption hooks to code in ext4, f2fs, and places like fs/buffer.c. blk-crypto-fallback is super helpful for this, since it will allow us to simply call fscrypt_set_bio_crypt_ctx() everywhere, and not have to both do that *and* implement a second case where we do all the crypto work scheduling, bounce page allocation, crypto API calls, etc. at the filesystem level. - Eric
