|SUCCESS| common/qat: add asym crypto disable option

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Thu Apr 16 18:58:48 CEST 2026


Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/163121

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-04-16

66c8e5abcc2f1a3f9e2e1c0f9e5a2e3f9e5a2e3f
Author: Radu Nicolau <radu.nicolau at intel.com>

common/qat: add asym crypto disable option

The Asymmetric crypto QAT PMD can now be disabled independently.

Example:
meson setup build -Ddisable_drivers=crypto/qat/asym

> diff --git a/drivers/common/qat/meson.build b/drivers/common/qat/meson.build
> index 31e06f4376..7f12f2b09d 100644
> --- a/drivers/common/qat/meson.build
> +++ b/drivers/common/qat/meson.build

[ ... ]

> @@ -88,6 +97,7 @@ sources += files(
>  includes += include_directories(
>          'qat_adf',
>          qat_crypto_relpath,
> +        qat_crypto_asym_relpath,
>          qat_compress_relpath,

Does this code unconditionally add the qat_crypto_asym_relpath include
directory regardless of whether BUILD_QAT_ASYM is enabled? This could make
the qat_crypto_asym_relpath variable undefined when qat_crypto_asym is
false.

> @@ -108,9 +118,8 @@ cflags += no_shadow_cflag
>  
>  if qat_crypto
>      foreach f: ['qat_sym.c', 'qat_sym_session.c',
> -            'qat_asym.c', 'qat_crypto.c',
> +            'qat_crypto.c',
>              'dev/qat_sym_pmd_gen1.c',
> -            'dev/qat_asym_pmd_gen1.c',
>              'dev/qat_crypto_pmd_gen2.c',
>              'dev/qat_crypto_pmd_gen3.c',
>              'dev/qat_crypto_pmd_gen4.c',
> @@ -119,6 +128,14 @@ if qat_crypto
>          ]
>          sources += files(join_paths(qat_crypto_relpath, f))
>      endforeach
> +    cflags += '-DBUILD_QAT_SYM'
> +    if qat_crypto_asym
> +        foreach f: ['asym/qat_asym.c',
> +                    'dev/qat_asym_pmd_gen1.c',
> +            ]
> +            sources += files(join_paths(qat_crypto_relpath, f))
> +        endforeach
> +        cflags += '-DBUILD_QAT_ASYM'
> +    endif
>      deps += ['security']
> -    cflags += ['-DBUILD_QAT_SYM', '-DBUILD_QAT_ASYM']
>  endif

[ ... ]

> diff --git a/drivers/crypto/qat/qat_crypto.c b/drivers/crypto/qat/qat_crypto.c
> index 861679373b..daa36c5eed 100644
> --- a/drivers/crypto/qat/qat_crypto.c
> +++ b/drivers/crypto/qat/qat_crypto.c
> @@ -53,9 +53,11 @@ qat_cryptodev_info_get(struct rte_cryptodev *dev,
>  			qat_qps_per_service(qat_dev, service_type);
>  		info->feature_flags = dev->feature_flags;
>  		info->capabilities = qat_private->qat_dev_capabilities;
> +
> +#ifdef BUILD_QAT_ASYM
>  		if (service_type == QAT_SERVICE_ASYMMETRIC)
>  			info->driver_id = qat_asym_driver_id;
> -
> +#endif
>  		if (service_type == QAT_SERVICE_SYMMETRIC)
>  			info->driver_id = qat_sym_driver_id;

In qat_cryptodev_info_get(), if BUILD_QAT_ASYM is not defined and
service_type equals QAT_SERVICE_ASYMMETRIC, does this code fail to set
info->driver_id, leaving it uninitialized? Should there be validation or
error handling when service_type is QAT_SERVICE_ASYMMETRIC but
BUILD_QAT_ASYM is disabled?

>  		/* No limit of number of sessions */
> @@ -168,8 +170,10 @@ qat_cryptodev_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
>  	for (i = 0; i < qp->nb_descriptors; i++) {
>  		if (service_type == QAT_SERVICE_SYMMETRIC)
>  			qat_sym_init_op_cookie(qp->op_cookies[i]);
> +#ifdef BUILD_QAT_ASYM
>  		else
>  			qat_asym_init_op_cookie(qp->op_cookies[i]);
> +#endif
>  	}

In qat_cryptodev_qp_setup(), when BUILD_QAT_ASYM is undefined and
service_type is QAT_SERVICE_ASYMMETRIC, does this code fail to initialize
the op_cookies for the asymmetric service type? Does qp->op_cookies[i]
remain uninitialized in this case, potentially leading to use of
uninitialized memory later?


More information about the test-report mailing list