Re: [PATCH v6 2/9] block: Add encryption context to struct bio

Martin K. Petersen
2019-12-19T00:48:29Z

Eric,

> There's not really any such thing as "use the bio integrity plumbing".
> blk-integrity just does blk-integrity; it's not a plumbing layer that
> allows other features to be supported. Well, in theory we could
> refactor and rename all the hooks to "blk-extra" and make them
> delegate to either blk-integrity or blk-crypto, but I think that would
> be overkill.

I certainly don't expect your crypto stuff to plug in without any
modification to what we currently have. I'm just observing that the
existing plumbing is designed to have pluggable functions that let
filesystems attach additional information to bios on writes and process
additional attached information on reads. And the block layer already
handles slicing and dicing these attachments as the I/O traverses the
stack.

There's also other stuff that probably won't be directly applicable or
interesting for your use case. It just seems like identifying actual
commonalities and differences would be worthwhile.

Note that substantial changes to the integrity code would inevitably
lead to a lot of pain and suffering for me. So from that perspective I
am very happy if you leave it alone. From an architectural viewpoint,
however, it seems that there are more similarities than differences
between crypto and integrity. And we should avoid duplication where
possible. That's all.

> What we could do, though, is say that at most one of blk-crypto and
> blk-integrity can be used at once on a given bio, and put the
> bi_integrity and bi_crypt_context pointers in union. (That would
> require allocating a REQ_INLINECRYPT bit so that we can tell what the
> pointer points to.)

Absolutely. That's why it's a union. Putting your stuff there is a
prerequisite as far as I'm concerned. No need to grow the bio when the
two features are unlikely to coexist. We can revisit that later should
the need arise.

--
Martin K. Petersen Oracle Linux Engineering