[PATCH v1] dts: add verify argument to set forward mode

[PATCH v1] dts: add verify argument to set forward mode

[Dean Marx]

Add optional verify argument to the set_forward_mode
method in testpmd shell.

Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
---
 dts/framework/remote_session/testpmd_shell.py | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
index 43e9f56517..b8cdfc01b9 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -717,11 +717,12 @@ def set_forward_mode(self, mode: SimpleForwardingModes, verify: bool = True):
                 fails to update.
         """
         set_fwd_output = self.send_command(f"set fwd {mode.value}")
-        if f"Set {mode.value} packet forwarding mode" not in set_fwd_output:
-            self._logger.debug(f"Failed to set fwd mode to {mode.value}:\n{set_fwd_output}")
-            raise InteractiveCommandExecutionError(
-                f"Test pmd failed to set fwd mode to {mode.value}"
-            )
+        if verify:
+            if f"Set {mode.value} packet forwarding mode" not in set_fwd_output:
+                self._logger.debug(f"Failed to set fwd mode to {mode.value}:\n{set_fwd_output}")
+                raise InteractiveCommandExecutionError(
+                    f"Test pmd failed to set fwd mode to {mode.value}"
+                )
 
     def show_port_info_all(self) -> list[TestPmdPort]:
         """Returns the information of all the ports.
-- 
2.44.0

Re: [PATCH v1] dts: add verify argument to set forward mode

[Jeremy Spewock]

On Mon, Aug 12, 2024 at 10:23 AM Dean Marx <dmarx@iol.unh.edu> wrote:
>
> Add optional verify argument to the set_forward_mode
> method in testpmd shell.
>
> Signed-off-by: Dean Marx <dmarx@iol.unh.edu>

The patch all looks good to me, but it might be beneficial to add a
"fixes" tag as well as a "bugzilla ID" tag to the commit body. The
DPDK contribution guidelines explains how to format the tags [1] and
also which order they should appear in [2].

[1] https://doc.dpdk.org/guides/contributing/patches.html#patch-fix-related-issues
[2] https://doc.dpdk.org/guides/contributing/patches.html#tag-order

> 2.44.0
>

Re: [PATCH v1] dts: add verify argument to set forward mode

[Juraj Linkeš]

On 12. 8. 2024 16:23, Dean Marx wrote:
> Add optional verify argument to the set_forward_mode
> method in testpmd shell.
> 

This doesn't add the method, just fixes it's use. Don't forget to fix 
the subject as well. And also add the tags that Jeremy mentioned.

> Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
> ---
>   dts/framework/remote_session/testpmd_shell.py | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
> index 43e9f56517..b8cdfc01b9 100644
> --- a/dts/framework/remote_session/testpmd_shell.py
> +++ b/dts/framework/remote_session/testpmd_shell.py
> @@ -717,11 +717,12 @@ def set_forward_mode(self, mode: SimpleForwardingModes, verify: bool = True):
>                   fails to update.
>           """
>           set_fwd_output = self.send_command(f"set fwd {mode.value}")
> -        if f"Set {mode.value} packet forwarding mode" not in set_fwd_output:
> -            self._logger.debug(f"Failed to set fwd mode to {mode.value}:\n{set_fwd_output}")
> -            raise InteractiveCommandExecutionError(
> -                f"Test pmd failed to set fwd mode to {mode.value}"
> -            )
> +        if verify:
> +            if f"Set {mode.value} packet forwarding mode" not in set_fwd_output:
> +                self._logger.debug(f"Failed to set fwd mode to {mode.value}:\n{set_fwd_output}")
> +                raise InteractiveCommandExecutionError(
> +                    f"Test pmd failed to set fwd mode to {mode.value}"
> +                )
>   
>       def show_port_info_all(self) -> list[TestPmdPort]:
>           """Returns the information of all the ports.

[PATCH v2] dts: fix verify argument in set forward mode

[Dean Marx]

Fix optional verify argument in set_forward_mode
method in testpmd shell class.

Bugzilla ID: 1410
Fixes: fc0f7dc47ee3 ("dts: add testpmd shell params")

Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
---
 dts/framework/remote_session/testpmd_shell.py | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
index dc1bef431d..c43b8b7520 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -1567,11 +1567,12 @@ def set_forward_mode(self, mode: SimpleForwardingModes, verify: bool = True):
                 fails to update.
         """
         set_fwd_output = self.send_command(f"set fwd {mode.value}")
-        if f"Set {mode.value} packet forwarding mode" not in set_fwd_output:
-            self._logger.debug(f"Failed to set fwd mode to {mode.value}:\n{set_fwd_output}")
-            raise InteractiveCommandExecutionError(
-                f"Test pmd failed to set fwd mode to {mode.value}"
-            )
+        if verify:
+            if f"Set {mode.value} packet forwarding mode" not in set_fwd_output:
+                self._logger.debug(f"Failed to set fwd mode to {mode.value}:\n{set_fwd_output}")
+                raise InteractiveCommandExecutionError(
+                    f"Test pmd failed to set fwd mode to {mode.value}"
+                )
 
     def stop_all_ports(self, verify: bool = True) -> None:
         """Stops all the ports.
-- 
2.44.0

Re: [PATCH v2] dts: fix verify argument in set forward mode

[Patrick Robb]

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

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

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

Re: [PATCH v2] dts: fix verify argument in set forward mode

[Nicholas Pratte]

Reviewed-by: Nicholas Pratte <npratte@iol.unh.edu>

On Thu, Oct 10, 2024 at 2:42 PM Dean Marx <dmarx@iol.unh.edu> wrote:
>
> Fix optional verify argument in set_forward_mode
> method in testpmd shell class.
>
> Bugzilla ID: 1410
> Fixes: fc0f7dc47ee3 ("dts: add testpmd shell params")
>
> Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
> ---
>  dts/framework/remote_session/testpmd_shell.py | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
> index dc1bef431d..c43b8b7520 100644
> --- a/dts/framework/remote_session/testpmd_shell.py
> +++ b/dts/framework/remote_session/testpmd_shell.py
> @@ -1567,11 +1567,12 @@ def set_forward_mode(self, mode: SimpleForwardingModes, verify: bool = True):
>                  fails to update.
>          """
>          set_fwd_output = self.send_command(f"set fwd {mode.value}")
> -        if f"Set {mode.value} packet forwarding mode" not in set_fwd_output:
> -            self._logger.debug(f"Failed to set fwd mode to {mode.value}:\n{set_fwd_output}")
> -            raise InteractiveCommandExecutionError(
> -                f"Test pmd failed to set fwd mode to {mode.value}"
> -            )
> +        if verify:
> +            if f"Set {mode.value} packet forwarding mode" not in set_fwd_output:
> +                self._logger.debug(f"Failed to set fwd mode to {mode.value}:\n{set_fwd_output}")
> +                raise InteractiveCommandExecutionError(
> +                    f"Test pmd failed to set fwd mode to {mode.value}"
> +                )
>
>      def stop_all_ports(self, verify: bool = True) -> None:
>          """Stops all the ports.
> --
> 2.44.0
>

Re: [PATCH v2] dts: fix verify argument in set forward mode

[Patrick Robb]

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

Thanks Dean. I reworded the commit message to "Condition set_forward_mode
verify code on the verify argument, which was originally omitted" because I
think even with a tiny bugfix, we want to explain for the commit history
what the commit is doing, instead of a vague "this is a bugfix" type
message.

Thanks, applied.

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