|SUCCESS| [v1,2/2] dts: add latency coverage for cryptodev testing

[dpdklab at iol.unh.edu]

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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-05-13

I'll review this patch against the DPDK coding standards and contribution guidelines.

sha: 20260513152715.133381-1-abailey at iol.unh.edu
Author: Andrew Bailey <abailey at iol.unh.edu>
Subject: [PATCH v1 1/2] dts: update parsing for cryptodev latency

This patch updates the regex for parsing cryptodev latency output to 
properly capture the burst size attribute for latency results.

> diff --git a/dts/api/cryptodev/__init__.py b/dts/api/cryptodev/__init__.py
> index a4fafc3713..15cffcb409 100644
> --- a/dts/api/cryptodev/__init__.py
> +++ b/dts/api/cryptodev/__init__.py
> @@ -117,7 +117,6 @@ def run_app(self, num_vfs: int = 1) -> list[CryptodevResults]:
>                      f"dependencies missing for virtual device {self._app_params['vdevs'][0].name}"
>                  )
>              raise e
> -

Does this blank line removal follow the project's formatting standards? 
This appears to be an unrelated whitespace change. According to coding 
style guidelines, there should be a blank line after a block statement 
like try/except before starting new logic.

>          regex = r"^\s+\d+.*$"
>          parser_options = re.MULTILINE
>          parser: type[CryptodevResults]
> @@ -126,7 +125,7 @@ def run_app(self, num_vfs: int = 1) -> list[CryptodevResults]:
>              case TestType.throughput:
>                  parser = ThroughputResults
>              case TestType.latency:
> -                regex = r"total operations:.*time[^\n]*"
> +                regex = r"total operations:.*?time[^\n]*"
>                  parser_options |= re.DOTALL
>                  parser = LatencyResults

> diff --git a/dts/api/cryptodev/types.py b/dts/api/cryptodev/types.py
> index df73a86fa4..7b4fd17cb6 100644
> --- a/dts/api/cryptodev/types.py
> +++ b/dts/api/cryptodev/types.py
> @@ -65,11 +65,11 @@ class LatencyResults(CryptodevResults):
>  
>      #: buffer size ran with app
>      buffer_size: int = field(
> -        metadata=TextParser.find_int(r"Buf(?:.*\n\s+\d+\s+)?(?:fer size:\s+)?(\d+)"),
> +        metadata=TextParser.find_int(r"Buffer size:\s+(\d+)"),
>      )
>      #: burst size ran with app
>      burst_size: int = field(
> -        metadata=TextParser.find_int(rf"Burst(?:.*\n\s+\d+\s+){2}?(?: size:\s+)?(\d+)"),
> +        metadata=TextParser.find_int(r"Burst size:\s+(\d+)"),
>      )
>      #: total operations ran
>      total_ops: int = field(metadata=TextParser.find_int(r"total operations:\s+(\d+)"))


---

sha: 20260513152715.133381-2-abailey at iol.unh.edu
Author: Andrew Bailey <abailey at iol.unh.edu>
Subject: [PATCH v1 2/2] dts: add latency coverage for cryptodev testing

This patch adds a test suite to include latency testing for crypto 
devices in DTS.

[ ... ]

> diff --git a/dts/tests/TestSuite_cryptodev_latency.py b/dts/tests/TestSuite_cryptodev_latency.py
> new file mode 100644
> index 0000000000..54edd4e0a6
> --- /dev/null
> +++ b/dts/tests/TestSuite_cryptodev_latency.py

[ ... ]

> +    def _verify_latency(
> +        self,
> +        results: list[CryptodevResults],
> +        key: str,
> +    ) -> list[dict[str, int | float | str]]:
> +        result_list: list[dict[str, int | float | str]] = []
> +
> +        for result in results:
> +            # get the corresponding baseline for the current buffer size
> +            parameters: dict[str, int | float | str] = list(
> +                filter(
> +                    lambda x: x["buff_size"] == result.buffer_size,
> +                    self.latency_test_parameters[key],
> +                )
> +            )[0]

Does this code handle the case when filter() returns an empty list? The 
[0] index access could raise an IndexError if no matching baseline is 
found for result.buffer_size.

[ ... ]

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

[ ... ]

> +        results = self._verify_latency(app.run_app(), "kasmui_vdev")

Does this typo in the key string "kasmui_vdev" cause _verify_latency() 
to raise a KeyError? The test method is named kasumi_vdev but passes 
"kasmui_vdev" as the key parameter.

[ ... ]

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

[ ... ]

> +            buffer_sz=self.buffer_sizes["open_ssl_vdev"],
> +        )
> +        results = self._verify_latency(app.run_app(), "open_ssl_vdev")

Does this code use the wrong buffer_size key? The method is snow3g_vdev 
but it's using self.buffer_sizes["open_ssl_vdev"] instead of 
self.buffer_sizes["snow3g_vdev"].