Re: [PATCH v2 3/3] dts: add flow validation

[Patrick Robb <probb@iol.unh.edu>]

[-- Attachment #1: Type: text/plain, Size: 3541 bytes --]

On Wed, May 21, 2025 at 3:26 PM Dean Marx <dmarx@iol.unh.edu> wrote:

> Add a method for validating flow rules to the testpmd shell class.
> Implement test case skipping for flow rules that do not pass
> validation.
>
> Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
> ---
>  dts/framework/remote_session/testpmd_shell.py | 15 ++++
>  dts/framework/test_run.py                     |  3 +
>  dts/framework/test_suite.py                   | 21 ++++-
>  dts/tests/TestSuite_flow.py                   | 90 +++++++++++++------
>  4 files changed, 102 insertions(+), 27 deletions(-)
>
> diff --git a/dts/framework/remote_session/testpmd_shell.py
> b/dts/framework/remote_session/testpmd_shell.py
> index bd5f2659bd..c2c7ade60a 100644
> --- a/dts/framework/remote_session/testpmd_shell.py
> +++ b/dts/framework/remote_session/testpmd_shell.py
> @@ -1961,6 +1961,21 @@ def flow_create(self, flow_rule: FlowRule, port_id:
> int) -> int:
>              self._logger.debug(f"Failed to create flow
> rule:\n{flow_output}")
>              raise InteractiveCommandExecutionError(f"Failed to create
> flow rule:\n{flow_output}")
>
> +    def flow_validate(self, flow_rule: FlowRule, port_id: int) -> bool:
> +        """Validates a flow rule in the testpmd session.
> +
> +        Args:
> +            flow_rule: :class:`FlowRule` object used for validating
> testpmd flow rule.
> +            port_id: Integer representing the port to use.
> +
> +        Returns:
> +            Boolean representing whether rule is valid or not.
> +        """
> +        flow_output = self.send_command(f"flow validate {port_id}
> {flow_rule}")
> +        if "Flow rule validated" in flow_output:
> +            return True
> +        return False
> +
>      def flow_delete(self, flow_id: int, port_id: int, verify: bool =
> True) -> None:
>          """Deletes the specified flow rule from the testpmd session.
>
> diff --git a/dts/framework/test_run.py b/dts/framework/test_run.py
> index 0fdc57ea9c..cf4af19e8f 100644
> --- a/dts/framework/test_run.py
> +++ b/dts/framework/test_run.py
> @@ -644,6 +644,9 @@ def next(self) -> State | None:
>                  return self
>
>              self.result.update(Result.FAIL, e)
> +        except SkippedTestException as e:
> +            self.logger.info(f"Test case execution SKIPPED: {e}")
> +            self.result.update(Result.SKIP, e)
>          else:
>              self.result.update(Result.PASS)
>              self.logger.info(f"{self.description.capitalize()} PASSED.")
> diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
> index e07c327b77..34e9eb54ea 100644
> --- a/dts/framework/test_suite.py
> +++ b/dts/framework/test_suite.py
> @@ -39,7 +39,7 @@
>      PacketFilteringConfig,
>  )
>
> -from .exception import ConfigurationError, InternalError,
> TestCaseVerifyError
> +from .exception import ConfigurationError, InternalError,
> SkippedTestException, TestCaseVerifyError
>  from .logger import DTSLogger, get_dts_logger
>  from .utils import get_packet_summaries, to_pascal_case
>
> @@ -411,6 +411,25 @@ def _fail_test_case_verify(self, failure_description:
> str) -> None:
>              self._logger.debug(command_res.command)
>          raise TestCaseVerifyError(failure_description)
>
> +    def verify_skip(self, condition: bool, skip_reason: str) -> None:
>

Maybe rename to "verify_else_skip" so it is more descriptive? Up to you.


>
>

Reviewed-by: Patrick Robb <probb@iol.unh.edu>

[-- Attachment #2: Type: text/html, Size: 4745 bytes --]