[RFC,v3,1/2] dts: add port config mtu options to testpmd shell
Checks
Commit Message
Testpmd offers mtu configuration options that omit ethernet overhead
calculations when set. This patch adds easy-of-use methods to leverage
these runtime options.
Bugzilla ID: 1421
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
dts/framework/remote_session/testpmd_shell.py | 20 ++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
Comments
Hey Nick,
Looks good to me, I just had one comment about what looks like a
mistake in a merge, and then another more general question.
On Fri, Jul 26, 2024 at 10:13 AM Nicholas Pratte <npratte@iol.unh.edu> wrote:
>
> Testpmd offers mtu configuration options that omit ethernet overhead
> calculations when set. This patch adds easy-of-use methods to leverage
> these runtime options.
>
> Bugzilla ID: 1421
>
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
> ---
> dts/framework/remote_session/testpmd_shell.py | 20 ++++++++++++++++++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
> index eda6eb320f..83f7961359 100644
> --- a/dts/framework/remote_session/testpmd_shell.py
> +++ b/dts/framework/remote_session/testpmd_shell.py
> @@ -804,7 +804,25 @@ def show_port_stats(self, port_id: int) -> TestPmdPortStats:
>
> return TestPmdPortStats.parse(output)
>
> - def _close(self) -> None:
> + def configure_port_mtu(self, port_id: int, mtu_length: int) -> None:
Was there a reason you decided to omit the verify parameter? I think
it would still be valuable to have just so the developer knows that if
they get past this step they are guaranteed to have a modified MTU. I
think you can do it with port info and I actually wrote the same
method in my (albeit, old and outdated now) scatter expansion patch if
you wanted to see an example of what I mean [1]. Additionally, I think
on some NICs you also have to stop the port before you can adjust the
MTU, so this could fail in some cases.
> + """Set the MTU length on a designated port.
> +
> + Args:
> + port_id: The ID of the port being configured.
> + mtu_length: The length, in bytes, of the MTU being set.
> + """
> + self.send_command(f"port config mtu {port_id} {mtu_length}")
> +
> + def configure_port_mtu_all(self, mtu_length: int) -> None:
> + """Set the MTU length on all designated ports.
> +
> + Args:
> + mtu_length: The MTU length to be set on all ports.
> + """
> + for port in self.show_port_info_all():
> + self.send_command(f"port config mtu {port.id} {mtu_length}")
> +
> + def close(self) -> None:
This looks like something that went wrong in the merge, this method is
called _close on main and that is the one that
SingleActiveIntactiveShells use to properly close.
> """Overrides :meth:`~.interactive_shell.close`."""
> self.stop()
> self.send_command("quit", "Bye...")
> --
> 2.44.0
>
[1] https://patchwork.dpdk.org/project/dpdk/patch/20240709175341.183888-2-jspewock@iol.unh.edu/
>> + def configure_port_mtu_all(self, mtu_length: int) -> None:
>> + """Set the MTU length on all designated ports.
>> +
>> + Args:
>> + mtu_length: The MTU length to be set on all ports.
>> + """
>> + for port in self.show_port_info_all():
>> + self.send_command(f"port config mtu {port.id} {mtu_length}")
There's a patch in my capabilities series [0] that could be use here.
I'll create a standalone patch from it.
[0]
https://patches.dpdk.org/project/dpdk/patch/20240821145315.97974-8-juraj.linkes@pantheon.tech/
>> +
>> + def close(self) -> None:
>
> This looks like something that went wrong in the merge, this method is
> called _close on main and that is the one that
> SingleActiveIntactiveShells use to properly close.
>
>> """Overrides :meth:`~.interactive_shell.close`."""
>> self.stop()
>> self.send_command("quit", "Bye...")
>> --
>> 2.44.0
>>
> [1] https://patchwork.dpdk.org/project/dpdk/patch/20240709175341.183888-2-jspewock@iol.unh.edu/
These two are basically the same patch. Let's coordinate this and create
one standalone patch that will be used for all three patches (I'm
counting my capabilities patch as well here).
@@ -804,7 +804,25 @@ def show_port_stats(self, port_id: int) -> TestPmdPortStats:
return TestPmdPortStats.parse(output)
- def _close(self) -> None:
+ def configure_port_mtu(self, port_id: int, mtu_length: int) -> None:
+ """Set the MTU length on a designated port.
+
+ Args:
+ port_id: The ID of the port being configured.
+ mtu_length: The length, in bytes, of the MTU being set.
+ """
+ self.send_command(f"port config mtu {port_id} {mtu_length}")
+
+ def configure_port_mtu_all(self, mtu_length: int) -> None:
+ """Set the MTU length on all designated ports.
+
+ Args:
+ mtu_length: The MTU length to be set on all ports.
+ """
+ for port in self.show_port_info_all():
+ self.send_command(f"port config mtu {port.id} {mtu_length}")
+
+ def close(self) -> None:
"""Overrides :meth:`~.interactive_shell.close`."""
self.stop()
self.send_command("quit", "Bye...")