From: "Martin K. Petersen" <martin.petersen@oracle.com> To: Eric Biggers <ebiggers@kernel.org> Cc: "Martin K. Petersen" <martin.petersen@oracle.com>, "Darrick J. Wong" <darrick.wong@oracle.com>, Satya Tangirala <satyat@google.com>, linux-block@vger.kernel.org, linux-scsi@vger.kernel.org, linux-fscrypt@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, Barani Muthukumaran <bmuthuku@qti.qualcomm.com>, Kuohong Wang <kuohong.wang@mediatek.com>, Kim Boojin <boojin.kim@samsung.com> Subject: Re: [PATCH v6 2/9] block: Add encryption context to struct bio Date: Mon, 06 Jan 2020 23:35:46 -0500 [thread overview] Message-ID: <yq136cr3mu5.fsf@oracle.com> (raw) In-Reply-To: <20191220035237.GB718@sol.localdomain> (Eric Biggers's message of "Thu, 19 Dec 2019 19:52:37 -0800") Eric, > However, the nature of the per-bio information is very different. > Most of the complexity in blk-integrity is around managing of a > separate integrity scatterlist for each bio, alongside the regular > data scatterlist. > That's not something we need or want for inline encryption. For each > bio we just need a key, algorithm, data unit number, and data unit > size. Since the data unit number (IV) is automatically incremented > for each sector and the encryption is length-preserving, there's no > per-sector data. Fair enough. I just wanted to make sure that you guys had actually looked at the integrity stuff and determined it wasn't a good fit. > There are some ways the two features could be supported simultaneously > without using more space, like making the pointer point to a linked > list of tagged structs, or making the struct contain both a > bio_crypt_ctx and bio_integrity_payload (or whichever combination is > enabled in kconfig). We have previously discussed having a facility in which you could chain several different things (with different prep/endio functions) off a bio. Similar to how we allow arbitrary stacking of block_devices. That was actually my main interest in terms of opening the integrity can of worms in this thread. Trying to find out which pieces of the plumbing, if any, could potentially be made generic and feature-independent. The copy offload efforts, which are now again picking up momentum, also need to hang things off of the bio. That's the original reason the integrity field became a union, fwiw. > So if people really aren't willing to accept the extra 8 bytes per bio > even behind a kconfig option, my vote is we that we put > bi_crypt_context in the union with bi_integrity, and add a flag > REQ_INLINECRYPT (like REQ_INTEGRITY) that indicates that the > bi_crypt_context member of the union is valid. Agreed. > We'd also need some error-handling to prevent the two features from > actually being used together. It looks like there are several cases > to consider. One of them is what happens if bio_crypt_set_ctx() is > called when blk-integrity verification or generation is enabled for > the disk. The integrity profile is only attached if the device driver identifies a discovered device as capable. At least in the short term no device should indicate simultaneous support for DIX and your crypto interface. Not saying that sanity checks shouldn't exist. But I think both of these features fall into things that are registered at device discovery time so we shouldn't need to clutter the I/O hot path with mutual exclusivity checks. -- Martin K. Petersen Oracle Linux Engineering
WARNING: multiple messages have this Message-ID (diff)
From: "Martin K. Petersen" <martin.petersen@oracle.com> To: Eric Biggers <ebiggers@kernel.org> Cc: "Martin K. Petersen" <martin.petersen@oracle.com>, linux-scsi@vger.kernel.org, "Darrick J. Wong" <darrick.wong@oracle.com>, Kuohong Wang <kuohong.wang@mediatek.com>, Kim Boojin <boojin.kim@samsung.com>, Barani Muthukumaran <bmuthuku@qti.qualcomm.com>, Satya Tangirala <satyat@google.com>, linux-block@vger.kernel.org, linux-fscrypt@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net Subject: Re: [f2fs-dev] [PATCH v6 2/9] block: Add encryption context to struct bio Date: Mon, 06 Jan 2020 23:35:46 -0500 [thread overview] Message-ID: <yq136cr3mu5.fsf@oracle.com> (raw) In-Reply-To: <20191220035237.GB718@sol.localdomain> (Eric Biggers's message of "Thu, 19 Dec 2019 19:52:37 -0800") Eric, > However, the nature of the per-bio information is very different. > Most of the complexity in blk-integrity is around managing of a > separate integrity scatterlist for each bio, alongside the regular > data scatterlist. > That's not something we need or want for inline encryption. For each > bio we just need a key, algorithm, data unit number, and data unit > size. Since the data unit number (IV) is automatically incremented > for each sector and the encryption is length-preserving, there's no > per-sector data. Fair enough. I just wanted to make sure that you guys had actually looked at the integrity stuff and determined it wasn't a good fit. > There are some ways the two features could be supported simultaneously > without using more space, like making the pointer point to a linked > list of tagged structs, or making the struct contain both a > bio_crypt_ctx and bio_integrity_payload (or whichever combination is > enabled in kconfig). We have previously discussed having a facility in which you could chain several different things (with different prep/endio functions) off a bio. Similar to how we allow arbitrary stacking of block_devices. That was actually my main interest in terms of opening the integrity can of worms in this thread. Trying to find out which pieces of the plumbing, if any, could potentially be made generic and feature-independent. The copy offload efforts, which are now again picking up momentum, also need to hang things off of the bio. That's the original reason the integrity field became a union, fwiw. > So if people really aren't willing to accept the extra 8 bytes per bio > even behind a kconfig option, my vote is we that we put > bi_crypt_context in the union with bi_integrity, and add a flag > REQ_INLINECRYPT (like REQ_INTEGRITY) that indicates that the > bi_crypt_context member of the union is valid. Agreed. > We'd also need some error-handling to prevent the two features from > actually being used together. It looks like there are several cases > to consider. One of them is what happens if bio_crypt_set_ctx() is > called when blk-integrity verification or generation is enabled for > the disk. The integrity profile is only attached if the device driver identifies a discovered device as capable. At least in the short term no device should indicate simultaneous support for DIX and your crypto interface. Not saying that sanity checks shouldn't exist. But I think both of these features fall into things that are registered at device discovery time so we shouldn't need to clutter the I/O hot path with mutual exclusivity checks. -- Martin K. Petersen Oracle Linux Engineering _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
next prev parent reply other threads:[~2020-01-07 4:36 UTC|newest] Thread overview: 112+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-12-18 14:51 [PATCH v6 0/9] Inline Encryption Support Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-18 14:51 ` [PATCH v6 1/9] block: Keyslot Manager for Inline Encryption Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-18 20:13 ` Eric Biggers 2019-12-18 20:13 ` [f2fs-dev] " Eric Biggers 2020-01-17 9:10 ` Christoph Hellwig 2020-01-17 9:10 ` [f2fs-dev] " Christoph Hellwig 2019-12-18 14:51 ` [PATCH v6 2/9] block: Add encryption context to struct bio Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-18 21:10 ` Eric Biggers 2019-12-18 21:10 ` [f2fs-dev] " Eric Biggers 2019-12-18 21:21 ` Darrick J. Wong 2019-12-18 21:21 ` [f2fs-dev] " Darrick J. Wong 2019-12-18 21:25 ` Martin K. Petersen 2019-12-18 21:25 ` [f2fs-dev] " Martin K. Petersen 2019-12-18 22:27 ` Eric Biggers 2019-12-18 22:27 ` [f2fs-dev] " Eric Biggers 2019-12-19 0:47 ` Martin K. Petersen 2019-12-19 0:47 ` [f2fs-dev] " Martin K. Petersen 2019-12-20 3:52 ` Eric Biggers 2019-12-20 3:52 ` [f2fs-dev] " Eric Biggers 2020-01-07 4:35 ` Martin K. Petersen [this message] 2020-01-07 4:35 ` Martin K. Petersen 2020-01-08 14:07 ` Christoph Hellwig 2020-01-08 14:07 ` [f2fs-dev] " Christoph Hellwig 2020-01-08 17:26 ` Eric Biggers 2020-01-08 17:26 ` [f2fs-dev] " Eric Biggers 2020-01-17 8:32 ` Christoph Hellwig 2020-01-17 8:32 ` [f2fs-dev] " Christoph Hellwig 2020-01-18 5:11 ` Eric Biggers 2020-01-18 5:11 ` [f2fs-dev] " Eric Biggers 2020-01-21 22:05 ` Satya Tangirala 2020-01-21 22:05 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-01-09 3:40 ` Martin K. Petersen 2020-01-09 3:40 ` [f2fs-dev] " Martin K. Petersen 2020-01-14 21:24 ` Eric Biggers 2020-01-14 21:24 ` [f2fs-dev] " Eric Biggers 2019-12-18 14:51 ` [PATCH v6 3/9] block: blk-crypto for Inline Encryption Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-20 3:14 ` Eric Biggers 2019-12-20 3:14 ` [f2fs-dev] " Eric Biggers 2019-12-20 5:10 ` Eric Biggers 2019-12-20 5:10 ` [f2fs-dev] " Eric Biggers 2020-01-14 21:22 ` Eric Biggers 2020-01-14 21:22 ` [f2fs-dev] " Eric Biggers 2019-12-18 14:51 ` [PATCH v6 4/9] scsi: ufs: UFS driver v2.1 spec crypto additions Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-01-17 12:31 ` Christoph Hellwig 2020-01-17 12:31 ` [f2fs-dev] " Christoph Hellwig 2019-12-18 14:51 ` [PATCH v6 5/9] scsi: ufs: UFS crypto API Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-20 4:48 ` Eric Biggers 2019-12-20 4:48 ` [f2fs-dev] " Eric Biggers 2020-01-14 21:16 ` Eric Biggers 2020-01-14 21:16 ` [f2fs-dev] " Eric Biggers 2020-01-17 13:51 ` Christoph Hellwig 2020-01-17 13:51 ` [f2fs-dev] " Christoph Hellwig 2019-12-18 14:51 ` [PATCH v6 6/9] scsi: ufs: Add inline encryption support to UFS Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-20 5:44 ` Eric Biggers 2019-12-20 5:44 ` [f2fs-dev] " Eric Biggers 2020-01-17 13:58 ` Christoph Hellwig 2020-01-17 13:58 ` [f2fs-dev] " Christoph Hellwig 2020-01-18 5:27 ` Eric Biggers 2020-01-18 5:27 ` [f2fs-dev] " Eric Biggers 2020-02-05 18:07 ` Christoph Hellwig 2020-02-05 18:07 ` [f2fs-dev] " Christoph Hellwig 2020-01-18 3:58 ` Eric Biggers 2020-01-18 3:58 ` [f2fs-dev] " Eric Biggers 2020-02-05 20:47 ` Eric Biggers 2020-02-05 20:47 ` [f2fs-dev] " Eric Biggers 2019-12-18 14:51 ` [PATCH v6 7/9] fscrypt: add inline encryption support Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-01-14 21:12 ` Eric Biggers 2020-01-14 21:12 ` [f2fs-dev] " Eric Biggers 2019-12-18 14:51 ` [PATCH v6 8/9] f2fs: " Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-20 4:23 ` Eric Biggers 2019-12-20 4:23 ` [f2fs-dev] " Eric Biggers 2019-12-18 14:51 ` [PATCH v6 9/9] ext4: " Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-19 0:12 ` Eric Biggers 2019-12-19 0:12 ` [f2fs-dev] " Eric Biggers 2019-12-19 0:31 ` Satya Tangirala 2019-12-19 0:31 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-22 0:16 ` Eric Biggers 2019-12-22 0:16 ` [f2fs-dev] " Eric Biggers 2020-01-08 14:05 ` [PATCH v6 0/9] Inline Encryption Support Christoph Hellwig 2020-01-08 14:05 ` [f2fs-dev] " Christoph Hellwig 2020-01-08 18:43 ` Satya Tangirala 2020-01-08 18:43 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-01-17 8:52 ` Christoph Hellwig 2020-01-17 8:52 ` [f2fs-dev] " Christoph Hellwig 2020-02-01 0:53 ` Satya Tangirala 2020-02-01 0:53 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-02-03 9:15 ` Christoph Hellwig 2020-02-03 9:15 ` [f2fs-dev] " Christoph Hellwig 2020-02-04 3:39 ` Satya Tangirala 2020-02-04 3:39 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-02-04 14:58 ` Christoph Hellwig 2020-02-04 14:58 ` [f2fs-dev] " Christoph Hellwig 2020-02-04 21:21 ` Eric Biggers 2020-02-04 21:21 ` [f2fs-dev] " Eric Biggers 2020-02-05 7:36 ` Eric Biggers 2020-02-05 7:36 ` [f2fs-dev] " Eric Biggers 2020-02-05 18:05 ` Christoph Hellwig 2020-02-05 18:05 ` [f2fs-dev] " Christoph Hellwig 2020-02-21 12:30 ` Satya Tangirala 2020-02-21 12:30 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-02-21 14:20 ` Christoph Hellwig 2020-02-21 14:20 ` [f2fs-dev] " Christoph Hellwig
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=yq136cr3mu5.fsf@oracle.com \ --to=martin.petersen@oracle.com \ --cc=bmuthuku@qti.qualcomm.com \ --cc=boojin.kim@samsung.com \ --cc=darrick.wong@oracle.com \ --cc=ebiggers@kernel.org \ --cc=kuohong.wang@mediatek.com \ --cc=linux-block@vger.kernel.org \ --cc=linux-f2fs-devel@lists.sourceforge.net \ --cc=linux-fscrypt@vger.kernel.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-scsi@vger.kernel.org \ --cc=satyat@google.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.