[RFC,v1,1/2] dts: add additional vlan configuration to testpmd shell class

Message ID 20240805171246.18580-2-npratte@iol.unh.edu (mailing list archive)
State New
Delegated to: Juraj Linkeš
Headers
Series dts: Ethertype ethdev api test suite |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation warning apply patch failure
ci/iol-testing warning apply patch failure

Commit Message

Nicholas Pratte Aug. 5, 2024, 5:12 p.m. UTC
The ethertype test suite requires many different internal runtime
vlan offload options to test ethertype configuration. The following patch
 adds a new RxVlanOffloadOptions class that contains all the different
vlan configuration options available within testpmd. Additionally, an
extra method has beena added to adjust both the inner and outer tpids of
ingressing and egressing packets.

Bugzilla ID: 1505

Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
 dts/framework/remote_session/testpmd_shell.py | 55 +++++++++++++++++++
 1 file changed, 55 insertions(+)
  

Comments

Jeremy Spewock Aug. 8, 2024, 9:40 p.m. UTC | #1
Hey Nick,

Patch looks pretty good to me, just a few minor comments below.

On Mon, Aug 5, 2024 at 1:13 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
>
> The ethertype test suite requires many different internal runtime
> vlan offload options to test ethertype configuration. The following patch
>  adds a new RxVlanOffloadOptions class that contains all the different
> vlan configuration options available within testpmd. Additionally, an
> extra method has beena added to adjust both the inner and outer tpids of

Extra "a" here at the end of "been."

> ingressing and egressing packets.
>
> Bugzilla ID: 1505
>
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
> ---
<snip>
> +    def set_vlan_tpid(self, port_id: int, tpid: int, inner_id: bool) -> None:

Is this one of those things that you can't really verify from testpmd
output? That makes sense if it is, but we should probably note why
this method is different in the doc-string.

> +        """Set ethertype tpid values using the ethdev api.
> +
> +        Args:
> +            port_id: the ID of the port the tpid is being changed on.

"the" should be capitalized here.

> +            tpid: The tpid value being changed on the port.
> +            inner_id: If :data:`True`, set the inner tpid to the specified value. If
> +                :data:`False`, change the outer tpid to the specified value.
> +
> +        Raises:
> +            InteractiveCommandExecutionError: If either `port_id` or `tpid` value is invalid.
> +        """
> +        if tpid < 0 or tpid > 65535:
> +            raise InteractiveCommandExecutionError("Invalid TPID value given.")
> +        output = self.send_command(
> +            f'vlan set {"inner" if inner_id else "outer"} tpid {tpid} {port_id}'
> +        )
> +        if output.startswith("Invalid port"):
> +            raise InteractiveCommandExecutionError(f"Invalid port ID {port_id} given.")
> +
<snip>
>
> +    def set_vlan_offload_option(
> +        self, port: int, offload_option: VLANOffloadFlag, on: bool, verify: bool = True
> +    ) -> None:
> +        """Enable extended vlans on the specified port.
> +
> +        Args:
> +            port: The port number to use, should be within 0-32.
> +            on: If :data:`True`, extended vlan turn on, otherwise it turns off.
> +            offload_options: The rx vlan offload option to be set.
> +            verify: If :data:`True`, the output of the command and show port info
> +                is scanned to verify that vlan stripping was enabled on the specified port.
> +                If not, it is considered an error.
> +
> +        Raises:
> +            InteractiveCommandExecutionError: If `verify` is :data:`True` and stripping
> +                fails to update.
> +        """
> +        offload_output = self.send_command(
> +            f"vlan set {str(offload_option).lower()} {'on' if on else 'off'} {port}"

Because flags can be combinations of multiple values, this might not
work if you passed in a flag that had STRIP and FILTER turned on at
the same time, for example. I think the string version would come out
to be something like STRIP|FILTER which definitely wouldn't be what
you want. Unfortunately, I think the only way around that though is to
have conditional values for each option and then checking to see if
the flag contains that option. Something like

if VLANOffloadFlag.STRIP in options:
    self.send_command(vlan set strip on 0)

Obviously with proper string formatting. Although, another thing you
might be able to do is if you can iterate through the members of the
VLANOffloadFlag class, you could do something more like:

for offload in VLANOffloadFlag.__members__:
    if offload in options:
        self.send_command(f"vlan set {offload.name.lower()} on 0")

And that would be a shorter way that covers you for any combination of
offloads. Could be fancy and make it a one-liner too if you wanted to,
but that's up to you.

> +        )
> +        if verify:
> +            if on ^ (str(offload_option) in str(self.show_port_info(port_id=port).vlan_offload)):

Is there a reason you can't directly compare the flags? The different
possible combinations of options will all have unique values, so I
think you should just be able to do `offload_option ==
self.show_port_info(port_id=port).vlan_offload)`



> +                self._logger.debug(
> +                    f"""Failed to set {offload_option} {'on' if on else 'off'} port {port}:
> +                        \n{offload_output}
> +                    """
> +                )
> +                raise InteractiveCommandExecutionError(
> +                    f"Testpmd failed to set {offload_option} {'on' if on else 'off'} port {port}."
> +                )
> +
>      def port_stop_all(self, verify: bool = True) -> None:
>          """Stop all ports.
>
> --
> 2.44.0
>
  

Patch

diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
index 288be9a085..268e34f0eb 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -70,6 +70,10 @@  class VLANOffloadFlag(Flag):
     #:
     QINQ_STRIP = auto()
 
+    def __str__(self):
+        """Return flag name when cast to a string."""
+        return self.name
+
     @classmethod
     def from_str_dict(cls, d):
         """Makes an instance from a dict containing the flag member names with an "on" value.
@@ -915,6 +919,26 @@  def show_port_stats_all(self) -> list[TestPmdPortStats]:
         iter = re.finditer(r"(^  #*.+#*$[^#]+)^  #*\r$", output, re.MULTILINE)
         return [TestPmdPortStats.parse(block.group(1)) for block in iter]
 
+    def set_vlan_tpid(self, port_id: int, tpid: int, inner_id: bool) -> None:
+        """Set ethertype tpid values using the ethdev api.
+
+        Args:
+            port_id: the ID of the port the tpid is being changed on.
+            tpid: The tpid value being changed on the port.
+            inner_id: If :data:`True`, set the inner tpid to the specified value. If
+                :data:`False`, change the outer tpid to the specified value.
+
+        Raises:
+            InteractiveCommandExecutionError: If either `port_id` or `tpid` value is invalid.
+        """
+        if tpid < 0 or tpid > 65535:
+            raise InteractiveCommandExecutionError("Invalid TPID value given.")
+        output = self.send_command(
+            f'vlan set {"inner" if inner_id else "outer"} tpid {tpid} {port_id}'
+        )
+        if output.startswith("Invalid port"):
+            raise InteractiveCommandExecutionError(f"Invalid port ID {port_id} given.")
+
     def show_port_stats(self, port_id: int) -> TestPmdPortStats:
         """Returns the given port statistics.
 
@@ -978,6 +1002,37 @@  def rx_vlan(self, vlan: int, port: int, add: bool, verify: bool = True) -> None:
                     f"Testpmd failed to {'add' if add else 'remove'} tag {vlan} on port {port}."
                 )
 
+    def set_vlan_offload_option(
+        self, port: int, offload_option: VLANOffloadFlag, on: bool, verify: bool = True
+    ) -> None:
+        """Enable extended vlans on the specified port.
+
+        Args:
+            port: The port number to use, should be within 0-32.
+            on: If :data:`True`, extended vlan turn on, otherwise it turns off.
+            offload_options: The rx vlan offload option to be set.
+            verify: If :data:`True`, the output of the command and show port info
+                is scanned to verify that vlan stripping was enabled on the specified port.
+                If not, it is considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and stripping
+                fails to update.
+        """
+        offload_output = self.send_command(
+            f"vlan set {str(offload_option).lower()} {'on' if on else 'off'} {port}"
+        )
+        if verify:
+            if on ^ (str(offload_option) in str(self.show_port_info(port_id=port).vlan_offload)):
+                self._logger.debug(
+                    f"""Failed to set {offload_option} {'on' if on else 'off'} port {port}:
+                        \n{offload_output}
+                    """
+                )
+                raise InteractiveCommandExecutionError(
+                    f"Testpmd failed to set {offload_option} {'on' if on else 'off'} port {port}."
+                )
+
     def port_stop_all(self, verify: bool = True) -> None:
         """Stop all ports.