On Wed, Sep 23, 2026 at 12:34 AM Eric Biggers <[email protected]> wrote: > > On Tue, Sep 22, 2026 at 04:49:45PM +0300, Itai Handler wrote: > > Storage is not the use case - crypto offload is. > > > > Where the cipher is a hardware engine driven over DMA, the per-request > > cost is a descriptor setup and a round trip, and that cost dominates. > > Making the request sixteen times larger amortises it. With the in-tree > > qce driver on an arm64 64k-page board, plain dm-crypt over a ramdisk, > > MB/s: > > That isn't a real use case, though. Using the QCE driver is *much* > slower than just using the encryption on the CPU, even on long messages. > That's been established in many previous discussions and is now even > admitted by the maintainers of the QCE driver. The QCE driver even > (incredibly) uses more CPU time than just doing the crypto on the CPU, > due to all its driver, scheduling, and IRQ overhead. The pending > patches to add BAM locking will make it even slower. > > The fix is to not use QCE. (And also ensure that > CONFIG_CRYPTO_AES_ARM64_CE_BLK=y is set, to get the ARMv8 CE accelerated > code. But even on legacy CPUs without ARMv8 CE, QCE is still slower.) > > Unsurprisingly, the numbers in this patch's cover letter show this as > well, with much higher throughput reported for the CPU-based encryption. > > (Also note that QCE is currently marked as BROKEN upstream.) > > I really do not think dm-crypt should accept changes to further > accommodate obsolete and problematic external crypto engines like this.
Thanks for the detailed feedback. I agree that QCE is not a good example to use as the main motivation for this change if its driver overhead makes it slower, and even more so if the driver consumes more CPU time than the software implementation. I will therefore not argue that QCE itself justifies changing dm-crypt. However, I don't think this necessarily establishes that the generic dm-crypt change is unnecessary. One point regarding the suggested ARMv8 Crypto Extensions implementation: PAGE_SIZE and the presence of ARMv8 Crypto Extensions are independent properties. In particular, an ARM64 system can use 64 KiB pages without having ARMv8 Crypto Extensions. Therefore, the fact that AES-CE is faster than QCE on CPUs which support it does not apply to all systems where the proposed change would be possible. There is also a more general reason why hardware crypto offload can be useful even when its raw throughput is lower than CPU-based encryption. Offloading encryption can leave CPU cycles available for the application, filesystem, networking, or other work. Of course, this only makes sense when the accelerator and its driver have reasonable overhead; I agree that QCE is a poor example if its driver overhead outweighs this benefit. My main question with the patch is therefore independent of QCE: Is there a reason for dm-crypt itself to impose a fixed 4096-byte encryption-unit limit, rather than allowing an encryption unit up to the size that its existing I/O conversion path can safely handle? The patch does not make larger sectors the default, and it does not remove the kernel-side validation. The default remains 512 bytes, and a larger sector must be explicitly requested. The proposed upper bound is the smaller of PAGE_SIZE and BLK_MAX_BLOCK_SIZE, so configurations that cannot represent such an I/O unit are still rejected. The motivation for using PAGE_SIZE as the upper bound is also not tied to any particular crypto accelerator. The current dm-crypt conversion path processes the encryption unit through the bio/scatterlist representation, and the relevant bio vector cannot represent more than PAGE_SIZE in a single entry. Consequently, PAGE_SIZE is already a natural upper bound for the current implementation. The performance measurements with QCE were intended to demonstrate the kind of workload where reducing the number of crypto submissions can matter, rather than to claim that QCE is itself a desirable accelerator. I agree that I should find a better hardware-accelerator example before using those measurements as evidence for the patch. I also don't intend this change to imply that larger encryption sectors are appropriate for LUKS or other portable on-disk formats. That is a separate userspace/on-disk-format consideration. The kernel would still enforce the limits of the dm-crypt implementation itself. So I would frame the patch more narrowly: this is an attempt to remove an apparently arbitrary 4096-byte limitation from dm-crypt and allow larger encryption units on systems where PAGE_SIZE and the block-layer limits permit them, without changing the default behavior. If there is a specific correctness, security, or architectural reason why dm-crypt should retain 4096 bytes as an absolute limit even on systems with larger PAGE_SIZE, I would be very interested to understand it. Otherwise, I think the question is better evaluated independently of whether QCE is a suitable accelerator. Thanks, Itai
