* [PATCH] dts: add RSS functions to testpmd
@ 2024-08-29 12:50 Alex Chapman
2024-09-06 14:29 ` Juraj Linkeš
0 siblings, 1 reply; 3+ messages in thread
From: Alex Chapman @ 2024-08-29 12:50 UTC (permalink / raw)
To: dev
Cc: Alex Chapman, Honnappa Nagarahalli, Paul Szczepanek,
Luca Vizzarro, Juraj Linkeš
This patch adds the required functionality for the RSS key_update,
RETA, and hash test suites. This includes:
The setting of custom RETA values for routing packets to specific
queues.
The setting of the RSS mode on all ports, to specify how to hash
the packets.
The updating of the RSS hash key used during the hashing process.
Alongside this, there is the addition of a __str__ method to the
RSSOffloadTypesFlags class, so that when flag names are cast to
a string they will use '-' as separators, instead of '_'.
This allows them to be directly used within testpmd RSS commands
without any further changes.
Signed-off-by: Alex Chapman <alex.chapman@arm.com>
Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>
---
dts/framework/remote_session/testpmd_shell.py | 84 ++++++++++++++++++-
1 file changed, 83 insertions(+), 1 deletion(-)
diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
index 43e9f56517..7b66901f6e 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -23,7 +23,7 @@
from typing_extensions import Self, Unpack
-from framework.exception import InteractiveCommandExecutionError
+from framework.exception import InteractiveCommandExecutionError, InternalError
from framework.params.testpmd import SimpleForwardingModes, TestPmdParams
from framework.params.types import TestPmdParamsDict
from framework.parser import ParserFn, TextParser
@@ -305,6 +305,12 @@ def make_parser(cls) -> ParserFn:
RSSOffloadTypesFlag.from_list_string,
)
+ def __str__(self):
+ """Stringifies the flag name."""
+ if self.name is None:
+ return ""
+ return self.name.replace("_", "-")
+
class DeviceCapabilitiesFlag(Flag):
"""Flag representing the device capabilities."""
@@ -723,6 +729,82 @@ def set_forward_mode(self, mode: SimpleForwardingModes, verify: bool = True):
f"Test pmd failed to set fwd mode to {mode.value}"
)
+ def port_config_rss_reta(
+ self, port_id: int, hash_index: int, queue_id: int, verify: bool = True
+ ) -> None:
+ """Configures a port's RSS redirection table.
+
+ Args:
+ port_id: Port where redirection table will be configured.
+ hash_index: The index into the redirection table associated with the destination queue.
+ queue_id: The destination queue of the packet.
+ verify: If :data:`True`, it verifies if a port's redirection table
+ was correctly configured.
+
+ Raises:
+ InteractiveCommandExecutionError: If `verify` is :data:`True`
+ Testpmd failed to config RSS reta.
+ """
+ out = self.send_command(f"port config {port_id} rss reta ({hash_index},{queue_id})")
+ if verify:
+ if f"The reta size of port {port_id} is" not in out:
+ self._logger.debug(f"Failed to config RSS reta: \n{out}")
+ raise InteractiveCommandExecutionError("Testpmd failed to config RSS reta.")
+
+ def port_config_all_rss_offload_type(
+ self, flag: RSSOffloadTypesFlag, verify: bool = True
+ ) -> None:
+ """Set the RSS mode on all ports.
+
+ Args:
+ rss_offload_type: The RSS iptype all ports will be configured to.
+ verify: If :data:`True`, it verifies if all ports RSS offload type
+ was correctly configured.
+
+ Raises:
+ InternalError: Offload Flag has contradictory values set.
+ InteractiveCommandExecutionError: If `verify` is :data:`True`
+ Testpmd failed to config the RSS mode on all ports.
+ """
+ if not flag.name:
+ raise InternalError("Offload Flag has contradictory values set")
+
+ out = self.send_command(f"port config all rss {flag.name}")
+
+ if verify:
+ if "error" in out:
+ self._logger.debug(f"Failed to config the RSS mode on all ports: \n{out}")
+ raise InteractiveCommandExecutionError(
+ "Testpmd failed to config the RSS mode on all ports"
+ )
+
+ def port_config_rss_hash_key(
+ self, port_id: int, offload_type: RSSOffloadTypesFlag, hex_str: str, verify: bool = True
+ ) -> str:
+ """Set the RSS hash key for the specified port.
+
+ Args:
+ port_id: Port the hash key will be set.
+ offload_type: The offload type the hash key will be applied to.
+ hex_str: The new hash key.
+ verify: If :data:`True`, RSS hash key has been correctly set.
+
+ Raises:
+ InteractiveCommandExecutionError: If `verify` is :data:`True`
+ Testpmd failed to set the RSS hash key.
+ """
+ output = self.send_command(f"port config {port_id} rss-hash-key {offload_type} {hex_str}")
+
+ if verify:
+ if (
+ "invalid - key must be a string of" in output
+ or "Bad arguments" in output
+ or "Error during getting device" in output
+ ):
+ self._logger.debug(f"Failed to set rss hash key: \n{output}")
+ raise InteractiveCommandExecutionError("Testpmd failed to set the RSS hash key.")
+ return output
+
def show_port_info_all(self) -> list[TestPmdPort]:
"""Returns the information of all the ports.
--
2.34.1
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] dts: add RSS functions to testpmd
2024-08-29 12:50 [PATCH] dts: add RSS functions to testpmd Alex Chapman
@ 2024-09-06 14:29 ` Juraj Linkeš
0 siblings, 0 replies; 3+ messages in thread
From: Juraj Linkeš @ 2024-09-06 14:29 UTC (permalink / raw)
To: Alex Chapman, dev; +Cc: Honnappa Nagarahalli, Paul Szczepanek, Luca Vizzarro
Two notes on sending patches. The patch is missing a version. I believe
this is version 2?
Second, looks like you didn't send this as a reply to the first version
of the patch. The message-id to use can be found in the patchwork patch
page.
On 29. 8. 2024 14:50, Alex Chapman wrote:
> This patch adds the required functionality for the RSS key_update,
> RETA, and hash test suites. This includes:
> The setting of custom RETA values for routing packets to specific
> queues.
> The setting of the RSS mode on all ports, to specify how to hash
> the packets.
> The updating of the RSS hash key used during the hashing process.
>
> Alongside this, there is the addition of a __str__ method to the
> RSSOffloadTypesFlags class, so that when flag names are cast to
> a string they will use '-' as separators, instead of '_'.
> This allows them to be directly used within testpmd RSS commands
> without any further changes.
>
> Signed-off-by: Alex Chapman <alex.chapman@arm.com>
> Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>
> ---
> dts/framework/remote_session/testpmd_shell.py | 84 ++++++++++++++++++-
> 1 file changed, 83 insertions(+), 1 deletion(-)
>
> diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
> index 43e9f56517..7b66901f6e 100644
> --- a/dts/framework/remote_session/testpmd_shell.py
> +++ b/dts/framework/remote_session/testpmd_shell.py
> @@ -23,7 +23,7 @@
>
> from typing_extensions import Self, Unpack
>
> -from framework.exception import InteractiveCommandExecutionError
> +from framework.exception import InteractiveCommandExecutionError, InternalError
> from framework.params.testpmd import SimpleForwardingModes, TestPmdParams
> from framework.params.types import TestPmdParamsDict
> from framework.parser import ParserFn, TextParser
> @@ -305,6 +305,12 @@ def make_parser(cls) -> ParserFn:
> RSSOffloadTypesFlag.from_list_string,
> )
>
> + def __str__(self):
> + """Stringifies the flag name."""
Since the stringifying is basically a given, I'd put something else
here, such as "Replace undercores with hyphens to produce valid testpmd
value." or something similar.
> + if self.name is None:
> + return ""
> + return self.name.replace("_", "-")
> +
>
> class DeviceCapabilitiesFlag(Flag):
> """Flag representing the device capabilities."""
> @@ -723,6 +729,82 @@ def set_forward_mode(self, mode: SimpleForwardingModes, verify: bool = True):
> f"Test pmd failed to set fwd mode to {mode.value}"
> )
>
> + def port_config_rss_reta(
> + self, port_id: int, hash_index: int, queue_id: int, verify: bool = True
> + ) -> None:
> + """Configures a port's RSS redirection table.
Just a note here: I like to use imperative (configure), but we don't
really enforce this in the framework (we have both imperative and third
person).
> +
> + Args:
> + port_id: Port where redirection table will be configured.
This is missing definite articles (which are present everywhere else):
port where redirection table -> the port where the redirection table
> + hash_index: The index into the redirection table associated with the destination queue.
> + queue_id: The destination queue of the packet.
> + verify: If :data:`True`, it verifies if a port's redirection table
the "it" is redundant
> + was correctly configured.
> +
> + Raises:
> + InteractiveCommandExecutionError: If `verify` is :data:`True`
> + Testpmd failed to config RSS reta.
> + """
> + out = self.send_command(f"port config {port_id} rss reta ({hash_index},{queue_id})")
> + if verify:
> + if f"The reta size of port {port_id} is" not in out:
> + self._logger.debug(f"Failed to config RSS reta: \n{out}")
> + raise InteractiveCommandExecutionError("Testpmd failed to config RSS reta.")
> +
> + def port_config_all_rss_offload_type(
> + self, flag: RSSOffloadTypesFlag, verify: bool = True
> + ) -> None:
> + """Set the RSS mode on all ports.
> +
> + Args:
> + rss_offload_type: The RSS iptype all ports will be configured to.
This doesn't match the arg name, we should probably rename flag to
rss_offload_type.
Also, RSSOffloadTypesFlag says:
Flag representing the RSS offload flow types supported by the NIC port.
I don't know what iptype is, we could just replace iptype with "offload
flow type".
> + verify: If :data:`True`, it verifies if all ports RSS offload type
> + was correctly configured.
> +
> + Raises:
> + InternalError: Offload Flag has contradictory values set.
> + InteractiveCommandExecutionError: If `verify` is :data:`True`
> + Testpmd failed to config the RSS mode on all ports.
> + """
> + if not flag.name:
> + raise InternalError("Offload Flag has contradictory values set")
Out of curiosity, when can this happen?
Also, two more points:
1. Please end sentences with a dot. Check the whole patch.
2. I really don't like messages constructed like this as it doesn't help
that much when debugging. Let's add more info, like what the
contradiction is.
> +
> + out = self.send_command(f"port config all rss {flag.name}")
> +
> + if verify:
> + if "error" in out:
> + self._logger.debug(f"Failed to config the RSS mode on all ports: \n{out}")
> + raise InteractiveCommandExecutionError(
> + "Testpmd failed to config the RSS mode on all ports"
> + )
Here we could also add what mode we were configuring.
> +
> + def port_config_rss_hash_key(
> + self, port_id: int, offload_type: RSSOffloadTypesFlag, hex_str: str, verify: bool = True
> + ) -> str:
> + """Set the RSS hash key for the specified port.
Now this is in imperative instead of third person. Let's pick one for
the entire patch.
> +
> + Args:
> + port_id: Port the hash key will be set.
This looks like an unfinished sentence, should it be set for (as in the
docsting's first line)?
> + offload_type: The offload type the hash key will be applied to.
> + hex_str: The new hash key.
> + verify: If :data:`True`, RSS hash key has been correctly set.
verify that RSS has key
> +
> + Raises:
> + InteractiveCommandExecutionError: If `verify` is :data:`True`
> + Testpmd failed to set the RSS hash key.
> + """
> + output = self.send_command(f"port config {port_id} rss-hash-key {offload_type} {hex_str}")
> +
> + if verify:
> + if (
> + "invalid - key must be a string of" in output
> + or "Bad arguments" in output
> + or "Error during getting device" in output
> + ):
Would it make sense to check for one positive instead of multiple
negatives? Or is it not that simple?
> + self._logger.debug(f"Failed to set rss hash key: \n{output}")
> + raise InteractiveCommandExecutionError("Testpmd failed to set the RSS hash key.")
We can also add extra info here: port, offload_type and the hex str.
> + return output
> +
> def show_port_info_all(self) -> list[TestPmdPort]:
> """Returns the information of all the ports.
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] dts: add RSS functions to testpmd
@ 2024-08-21 16:09 Alex Chapman
0 siblings, 0 replies; 3+ messages in thread
From: Alex Chapman @ 2024-08-21 16:09 UTC (permalink / raw)
To: dev
Cc: Alex Chapman, Honnappa Nagarahalli, Paul Szczepanek,
Luca Vizzarro, Juraj Linkeš
This patch adds the required functionality for the RSS key_update,
RETA, and hash test suites. This includes:
The setting of custom RETA values for routing packets to specific
queues.
The setting of the RSS mode on all ports, to specify how to hash
the packets.
The updating of the RSS hash key used during the hashing process.
Alongside this, there is the addition of a __str__ method to the
RSSOffloadTypesFlags class, so that when flag names are cast to
a string they will use '-' as separators, instead of '_'.
This allows them to be directly used within testpmd RSS commands
without any further changes.
Signed-off-by: Alex Chapman <alex.chapman@arm.com>
Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>
---
dts/framework/remote_session/testpmd_shell.py | 84 ++++++++++++++++++-
1 file changed, 83 insertions(+), 1 deletion(-)
diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
index 18b0533658..6a2d8be6fa 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -23,7 +23,7 @@
from typing_extensions import Self, Unpack
-from framework.exception import InteractiveCommandExecutionError
+from framework.exception import InteractiveCommandExecutionError, InternalError
from framework.params.testpmd import SimpleForwardingModes, TestPmdParams
from framework.params.types import TestPmdParamsDict
from framework.parser import ParserFn, TextParser
@@ -305,6 +305,12 @@ def make_parser(cls) -> ParserFn:
RSSOffloadTypesFlag.from_list_string,
)
+ def __str__(self):
+ """Stringifies the flag name."""
+ if self.name is None:
+ return ""
+ return self.name.replace("_", "-")
+
class DeviceCapabilitiesFlag(Flag):
"""Flag representing the device capabilities."""
@@ -1178,6 +1184,82 @@ def set_forward_mode(self, mode: SimpleForwardingModes, verify: bool = True):
f"Test pmd failed to set fwd mode to {mode.value}"
)
+ def port_config_rss_reta(
+ self, port_id: int, hash_index: int, queue_id: int, verify: bool = True
+ ) -> None:
+ """Configures a port's RSS redirection table.
+
+ Args:
+ port_id: Port where redirection table will be configured.
+ hash_index: The index into the redirection table associated with the destination queue.
+ queue_id: The destination queue of the packet.
+ verify: If :data:`True`, it verifies if a port's redirection table
+ was correctly configured.
+
+ Raises:
+ InteractiveCommandExecutionError: If `verify` is :data:`True`
+ Testpmd failed to config RSS reta.
+ """
+ out = self.send_command(f"port config {port_id} rss reta ({hash_index},{queue_id})")
+ if verify:
+ if f"The reta size of port {port_id} is" not in out:
+ self._logger.debug(f"Failed to config RSS reta: \n{out}")
+ raise InteractiveCommandExecutionError("Testpmd failed to config RSS reta.")
+
+ def port_config_all_rss_offload_type(
+ self, flag: RSSOffloadTypesFlag, verify: bool = True
+ ) -> None:
+ """Set the RSS mode on all ports.
+
+ Args:
+ rss_offload_type: The RSS iptype all ports will be configured to.
+ verify: If :data:`True`, it verifies if all ports RSS offload type
+ was correctly configured.
+
+ Raises:
+ InternalError: Offload Flag has contradictory values set.
+ InteractiveCommandExecutionError: If `verify` is :data:`True`
+ Testpmd failed to config the RSS mode on all ports.
+ """
+ if not flag.name:
+ raise InternalError("Offload Flag has contradictory values set")
+
+ out = self.send_command(f"port config all rss {flag.name}")
+
+ if verify:
+ if "error" in out:
+ self._logger.debug(f"Failed to config the RSS mode on all ports: \n{out}")
+ raise InteractiveCommandExecutionError(
+ "Testpmd failed to config the RSS mode on all ports"
+ )
+
+ def port_config_rss_hash_key(
+ self, port_id: int, offload_type: RSSOffloadTypesFlag, hex_str: str, verify: bool = True
+ ) -> str:
+ """Set the RSS hash key for the specified port.
+
+ Args:
+ port_id: Port the hash key will be set.
+ offload_type: The offload type the hash key will be applied to.
+ hex_str: The new hash key.
+ verify: If :data:`True`, RSS hash key has been correctly set.
+
+ Raises:
+ InteractiveCommandExecutionError: If `verify` is :data:`True`
+ Testpmd failed to set the RSS hash key.
+ """
+ output = self.send_command(f"port config {port_id} rss-hash-key {offload_type} {hex_str}")
+
+ if verify:
+ if (
+ "invalid - key must be a string of" in output
+ or "Bad arguments" in output
+ or "Error during getting device" in output
+ ):
+ self._logger.debug(f"Failed to set rss hash key: \n{output}")
+ raise InteractiveCommandExecutionError("Testpmd failed to set the RSS hash key.")
+ return output
+
def show_port_info_all(self) -> list[TestPmdPort]:
"""Returns the information of all the ports.
--
2.34.1
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-09-06 14:29 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-08-29 12:50 [PATCH] dts: add RSS functions to testpmd Alex Chapman
2024-09-06 14:29 ` Juraj Linkeš
-- strict thread matches above, loose matches on Subject: below --
2024-08-21 16:09 Alex Chapman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).