|SUCCESS| [RFC] dts: add verify coverage for cryptodev testing

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

Commit: (SHA not available from patch context)
Author: Andrew Bailey <abailey at iol.unh.edu>

[RFC] dts: add verify coverage for cryptodev testing

Currently, next-DTS only covers throughput testing through the
dpdk-test-crypto application. This series adds coverage for the verify
option to next DTS to allow functional testing for various algorithms of
crypto devices and virtual devices.

---

> diff --git a/dts/tests/TestSuite_cryptodev_verify.py b/dts/tests/TestSuite_cryptodev_verify.py
> new file mode 100644
> index 0000000000..a3f797b225
> --- /dev/null
> +++ b/dts/tests/TestSuite_cryptodev_verify.py
> @@ -0,0 +1,228 @@

[ ... ]

> +class Config(BaseConfig):
> +    """Performance test metrics.
> +
> +    Attributes:
> +        delta_tolerance: The allowed tolerance below a given baseline.
> +        throughput_test_parameters: The test parameters to use in the test suite.
> +    """
> +
> +    delta_tolerance: float = 0.05
> +
> +    verify_test_parameters: dict[str, list[int]] = {
> +        "aesni_mb_vdev": config_list,
> +        "openssl_vdev": config_list,
> +        "sha1_hmac_buff_32": config_list,
> +    }

Does the docstring accurately describe this class? The docstring mentions
"Performance test metrics" and "throughput_test_parameters" but the class
is for verify testing and contains "verify_test_parameters".

[ ... ]

> +    def _verify_output(
> +        self,
> +        results: list[CryptodevResults],
> +        key: str,
> +    ) -> list[dict[str, int | float]]:
> +        result_list: list[dict[str, int | float]] = []
> +        if key not in self.verify_test_parameters:
> +            skip(f"{key} test not configured")
> +
> +        results_list = []
> +        passed = True
> +        for result in results:
> +            if getattr(result, "failed_enqueue") / TOTAL_OPS > self.delta_tolerance:
> +                passed = False
> +            if getattr(result, "failed_dequeue") / TOTAL_OPS > self.delta_tolerance:
> +                passed = False
> +            if getattr(result, "failed_ops") / TOTAL_OPS > self.delta_tolerance:
> +                passed = False

Does this code handle division by zero if TOTAL_OPS is zero?

Is the variable "result_list" declared but never used while "results_list"
is used instead? Should these be the same variable?

Can "passed" be set to False once and then incorrectly carry over to
subsequent iterations of the loop, making all remaining results appear as
failures?

> +            results_list.append(
> +                {
> +                    "Failed Enqueue": getattr(result, "failed_enqueue"),
> +                    "Failed Dequeue": getattr(result, "failed_dequeue"),
> +                    "Failed Operations": getattr(result, "failed_ops"),
> +                    "Delta Tolerance": self.delta_tolerance,
> +                    "Passed": passed,
> +                }
> +            )
> +        return result_list

Does this code return the wrong variable? The function returns
"result_list" which is initialized to an empty list but never populated,
while "results_list" contains the actual results.

> +
> +    @crypto_test
> +    def aesni_mb_vdev(self) -> None:

[ ... ]

> +        app = Cryptodev(
> +            vdevs=[VirtualDevice("crypto_aesni_mb0")],
> +            ptest=TestType.verify,
> +            test_file=TEST_FILE,
> +            test_name="sha1_hmac_buff_32",
> +            devtype=DeviceType.crypto_aesni_mb,
> +            optype=OperationType.cipher_then_auth,
> +            cipher_algo=CipherAlgorithm.aes_cbc,
> +            cipher_op=EncryptDecryptSwitch.encrypt,
> +            cipher_key_sz=32,
> +            cipher_iv_sz=16,
> +            auth_algo=AuthenticationAlgorithm.sha1_hmac,
> +            auth_op=AuthenticationOpMode.generate,
> +            auth_key_sz=64,
> +            digest_sz=12,
> +            burst_sz=32,
> +            total_ops=TOTAL_OPS,
> +            buffer_sz=self.buffer_sizes["aesni_mb_vdev"],
> +        )
> +        results = self._verify_output(app.run_app(), "aesni_mb_vdev")
> +        self._print_stats(results)
> +        for result in results:
> +            verify(result["passed"] == "PASS", "Gbps fell below delta tolerance")

Does this comparison make sense? The "passed" field is set to a boolean
value (True/False) in _verify_output, but here it's compared against the
string "PASS".

Does the error message make sense for a verify test? The message says
"Gbps fell below delta tolerance" but this is a verify test checking
failed operations, not a throughput test measuring Gbps.

> +
> +    @crypto_test
> +    def openssl_vdev(self) -> None:

[ ... ]

> +        app = Cryptodev(
> +            vdevs=[VirtualDevice("crypto_openssl0")],
> +            ptest=TestType.verify,
> +            test_file=TEST_FILE,
> +            test_name="aes_gcm_buff_32",
> +            devtype=DeviceType.crypto_openssl,
> +            optype=OperationType.aead,
> +            aead_algo=AeadAlgName.aes_gcm,
> +            aead_op=EncryptDecryptSwitch.encrypt,
> +            aead_key_sz=16,
> +            aead_aad_sz=16,
> +            aead_iv_sz=12,
> +            digest_sz=16,
> +            burst_sz=32,
> +            buffer_sz=self.buffer_sizes["aesni_mb"],

Does this code use the wrong key? The function is "openssl_vdev" and
checks "openssl_vdev" in verify_test_parameters, but accesses
self.buffer_sizes["aesni_mb"] instead of
self.buffer_sizes["openssl_vdev"].

[ ... ]

> +    @crypto_test
> +    def sha1_hmac_buff_32(self) -> None:
> +        """aes_cbc test.

Does the docstring match the function name? The function is named
"sha1_hmac_buff_32" but the docstring says "aes_cbc test".

[ ... ]

> +        app = Cryptodev(
> +            ptest=TestType.verify,
> +            test_file=TEST_FILE,
> +            test_name="sha1_hmac_buff_32",
> +            devtype=self.device_type,
> +            optype=OperationType.aead,
> +            cipher_algo=CipherAlgorithm.aes_cbc,
> +            cipher_op=EncryptDecryptSwitch.encrypt,
> +            cipher_key_sz=32,
> +            cipher_iv_sz=16,
> +            auth_algo=AuthenticationAlgorithm.sha1_hmac,
> +            auth_op=AuthenticationOpMode.generate,
> +            auth_key_sz=64,
> +            digest_sz=20,
> +            burst_sz=32,
> +            buffer_sz=self.buffer_sizes["sha1_hmac_buff_32"],
> +            total_ops=TOTAL_OPS,
> +        )

Does the optype configuration make sense? The code sets
optype=OperationType.aead but also configures both cipher_algo and
auth_algo separately, which suggests cipher_then_auth operation type
should be used instead.