Re: [PATCH] crypto: qat - lower priority for skcipher and aead algorithms

[Eric Biggers <ebiggers@kernel.org>]

On Fri, Jun 13, 2025 at 12:01:50PM -0700, Eric Biggers wrote:
> On Fri, Jun 13, 2025 at 11:32:27AM +0100, Giovanni Cabiddu wrote:
> > Most kernel applications utilizing the crypto API operate synchronously
> > and on small buffer sizes, therefore do not benefit from QAT acceleration.
> > 
> > Reduce the priority of QAT implementations for both skcipher and aead
> > algorithms, allowing more suitable alternatives to be selected by default.
> > 
> > Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
> > Link: https://lore.kernel.org/all/20250613012357.GA3603104@google.com/
> > Cc: stable@vger.kernel.org
> > ---
> >  drivers/crypto/intel/qat/qat_common/qat_algs.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/crypto/intel/qat/qat_common/qat_algs.c b/drivers/crypto/intel/qat/qat_common/qat_algs.c
> > index 3c4bba4a8779..d69cc1e5e023 100644
> > --- a/drivers/crypto/intel/qat/qat_common/qat_algs.c
> > +++ b/drivers/crypto/intel/qat/qat_common/qat_algs.c
> > @@ -1277,7 +1277,7 @@ static struct aead_alg qat_aeads[] = { {
> >  	.base = {
> >  		.cra_name = "authenc(hmac(sha1),cbc(aes))",
> >  		.cra_driver_name = "qat_aes_cbc_hmac_sha1",
> > -		.cra_priority = 4001,
> > +		.cra_priority = 100,
> >  		.cra_flags = CRYPTO_ALG_ASYNC | CRYPTO_ALG_ALLOCATES_MEMORY,
> >  		.cra_blocksize = AES_BLOCK_SIZE,
> >  		.cra_ctxsize = sizeof(struct qat_alg_aead_ctx),
> > @@ -1294,7 +1294,7 @@ static struct aead_alg qat_aeads[] = { {
> >  	.base = {
> >  		.cra_name = "authenc(hmac(sha256),cbc(aes))",
> >  		.cra_driver_name = "qat_aes_cbc_hmac_sha256",
> > -		.cra_priority = 4001,
> > +		.cra_priority = 100,
> >  		.cra_flags = CRYPTO_ALG_ASYNC | CRYPTO_ALG_ALLOCATES_MEMORY,
> >  		.cra_blocksize = AES_BLOCK_SIZE,
> >  		.cra_ctxsize = sizeof(struct qat_alg_aead_ctx),
> > @@ -1311,7 +1311,7 @@ static struct aead_alg qat_aeads[] = { {
> >  	.base = {
> >  		.cra_name = "authenc(hmac(sha512),cbc(aes))",
> >  		.cra_driver_name = "qat_aes_cbc_hmac_sha512",
> > -		.cra_priority = 4001,
> > +		.cra_priority = 100,
> >  		.cra_flags = CRYPTO_ALG_ASYNC | CRYPTO_ALG_ALLOCATES_MEMORY,
> >  		.cra_blocksize = AES_BLOCK_SIZE,
> >  		.cra_ctxsize = sizeof(struct qat_alg_aead_ctx),
> > @@ -1329,7 +1329,7 @@ static struct aead_alg qat_aeads[] = { {
> >  static struct skcipher_alg qat_skciphers[] = { {
> >  	.base.cra_name = "cbc(aes)",
> >  	.base.cra_driver_name = "qat_aes_cbc",
> > -	.base.cra_priority = 4001,
> > +	.base.cra_priority = 100,
> >  	.base.cra_flags = CRYPTO_ALG_ASYNC | CRYPTO_ALG_ALLOCATES_MEMORY,
> >  	.base.cra_blocksize = AES_BLOCK_SIZE,
> >  	.base.cra_ctxsize = sizeof(struct qat_alg_skcipher_ctx),
> > @@ -1347,7 +1347,7 @@ static struct skcipher_alg qat_skciphers[] = { {
> >  }, {
> >  	.base.cra_name = "ctr(aes)",
> >  	.base.cra_driver_name = "qat_aes_ctr",
> > -	.base.cra_priority = 4001,
> > +	.base.cra_priority = 100,
> >  	.base.cra_flags = CRYPTO_ALG_ASYNC | CRYPTO_ALG_ALLOCATES_MEMORY,
> >  	.base.cra_blocksize = 1,
> >  	.base.cra_ctxsize = sizeof(struct qat_alg_skcipher_ctx),
> > @@ -1365,7 +1365,7 @@ static struct skcipher_alg qat_skciphers[] = { {
> >  }, {
> >  	.base.cra_name = "xts(aes)",
> >  	.base.cra_driver_name = "qat_aes_xts",
> > -	.base.cra_priority = 4001,
> > +	.base.cra_priority = 100,
> >  	.base.cra_flags = CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK |
> >  			  CRYPTO_ALG_ALLOCATES_MEMORY,
> >  	.base.cra_blocksize = AES_BLOCK_SIZE,
> > -- 
> > 2.49.0
> > 
> 
> Acked-by: Eric Biggers <ebiggers@kernel.org>
> 
> But, I think your commit message may be misleading:
> 
> > Most kernel applications utilizing the crypto API operate synchronously
> > and on small buffer sizes, therefore do not benefit from QAT acceleration.
> 
> That implies that QAT acceleration *would* be beneficial for kernel applications
> using asynchronous processing, large buffer sizes, or both.
> 
> But as far as I know, that hasn't been shown to be true either.  Those things
> would likely make the performance characteristics of the QAT driver less bad,
> but that doesn't necessarily mean it would become better than VAES.  VAES is
> already incredibly fast, and far easier to use.

Also, it seems that you're considering 4096 bytes to be a "small" buffer size,
which is kind of misleading too.  In dm-crypt for example 4096-byte sectors (the
recommended setting these days) are considered "large sectors", differentiating
them from the traditional 512-byte sectors.

- Eric