|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