[v1,1/2] dts: add fwd restart decorator to rx capabilities
Checks
Commit Message
Some testpmd runtime functions require that forwarding be stopped before
attempting to execute them, depending on the NIC and vendor. Adding a
decorator to these testpmdshell methods to stop, execute, and then start
forwarding again abstracts this concern away for test suite developers,
and makes for cleaner, easier to read code.
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
dts/framework/remote_session/testpmd_shell.py | 26 ++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)
Comments
On Fri, Jan 17, 2025 at 9:58 AM Nicholas Pratte <npratte@iol.unh.edu> wrote:
> - @requires_started_ports
+ @requires_forwarding_restart
> @requires_stopped_ports
> def set_port_mtu(self, port_id: int, mtu: int, verify: bool = True)
> -> None:
> """Change the MTU of a port using testpmd.
>
Is the requires_stopped_ports decorated still required, or is the
requires_forwarding_restart decorator alone sufficient?
Looks good otherwise thanks!
Reviewed-by: Patrick Robb <probb@iol.unh.edu>
Hi Patrick,
Yes! Stop ports is a required decorator because some vendors require
that the ports themselves be stopped to change their MTU values during
runtime. This is not to be confused with forwarding, which is what my
decorator does on top of a given function. That said, I suppose this
could be reworked so that forward restarting stops the ports and
starts them again as well, but my concern is that there could be other
functions created in the future that require forward restarts but also
require the ports to be actively started. I'm not 100% sure,
On Sun, Jan 19, 2025 at 8:14 PM Patrick Robb <probb@iol.unh.edu> wrote:
>
>
>
> On Fri, Jan 17, 2025 at 9:58 AM Nicholas Pratte <npratte@iol.unh.edu> wrote:
>>
>> - @requires_started_ports
>>
>> + @requires_forwarding_restart
>> @requires_stopped_ports
>> def set_port_mtu(self, port_id: int, mtu: int, verify: bool = True) -> None:
>> """Change the MTU of a port using testpmd.
>
>
> Is the requires_stopped_ports decorated still required, or is the requires_forwarding_restart decorator alone sufficient?
>
>
> Looks good otherwise thanks!
>
> Reviewed-by: Patrick Robb <probb@iol.unh.edu>
@@ -1390,6 +1390,28 @@ def _wrapper(self: "TestPmdShell", *args: P.args, **kwargs: P.kwargs):
return _wrapper
+def requires_forwarding_restart(func: TestPmdShellMethod) -> TestPmdShellMethod:
+ """Decorator for :class:`TestPmdShell` commands methods that requires forwarding restart.
+
+ If the decorated method is called while a :class:`TestPmdShell` is actively forwarding, then
+ forwarding is ceased, the wrapped function is executed, and forwarding is started again.
+
+ Args:
+ func: The :class:`TestPmdShell` method to decorate.
+ """
+
+ @functools.wraps(func)
+ def _wrapper(self: "TestPmdShell", *args: P.args, **kwargs: P.kwargs):
+ if self.currently_forwarding:
+ self._logger.debug("Forwarding needs to be restarted to continue.")
+ self.stop()
+ retval = func(self, *args, **kwargs)
+ self.start()
+ return retval
+
+ return _wrapper
+
+
def add_remove_mtu(mtu: int = 1500) -> Callable[[TestPmdShellMethod], TestPmdShellMethod]:
"""Configure MTU to `mtu` on all ports, run the decorated function, then revert.
@@ -1438,6 +1460,7 @@ class TestPmdShell(DPDKShell):
_command_extra_chars: ClassVar[str] = "\n"
ports_started: bool
+ currently_forwarding: bool
def __init__(
self,
@@ -1462,6 +1485,7 @@ def __init__(
name,
)
self.ports_started = not self._app_params.disable_device_start
+ self.currently_forwarding = not self._app_params.auto_start
self._ports = None
@property
@@ -1836,7 +1860,7 @@ def csum_set_hw(
{port_id}:\n{csum_output}"""
)
- @requires_started_ports
+ @requires_forwarding_restart
@requires_stopped_ports
def set_port_mtu(self, port_id: int, mtu: int, verify: bool = True) -> None:
"""Change the MTU of a port using testpmd.