diff mbox series

[v3,7/7] dts: allow configuring MTU of ports

Message ID 20231113202833.12900-8-jspewock@iol.unh.edu (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series dts: Port scatter suite over |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation warning apply issues
ci/github-robot: build success github build: passed

Commit Message

Jeremy Spewock Nov. 13, 2023, 8:28 p.m. UTC 
  From: Jeremy Spewock <jspewock@iol.unh.edu>

Adds methods in both os_session and linux session to allow for setting
MTU of port interfaces in an OS agnostic way.

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/remote_session/linux_session.py | 7 +++++++
 dts/framework/remote_session/os_session.py    | 9 +++++++++
 2 files changed, 16 insertions(+)

Comments

Juraj Linkeš Nov. 16, 2023, 6:05 p.m. UTC | #1 
On Mon, Nov 13, 2023 at 9:28 PM <jspewock@iol.unh.edu> wrote:
>
> From: Jeremy Spewock <jspewock@iol.unh.edu>
>
> Adds methods in both os_session and linux session to allow for setting
> MTU of port interfaces in an OS agnostic way.
>
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---
>  dts/framework/remote_session/linux_session.py | 7 +++++++
>  dts/framework/remote_session/os_session.py    | 9 +++++++++
>  2 files changed, 16 insertions(+)
>
> diff --git a/dts/framework/remote_session/linux_session.py b/dts/framework/remote_session/linux_session.py
> index a3f1a6bf3b..dab68d41b1 100644
> --- a/dts/framework/remote_session/linux_session.py
> +++ b/dts/framework/remote_session/linux_session.py
> @@ -196,6 +196,13 @@ def configure_port_ip_address(
>              verify=True,
>          )
>
> +    def configure_port_mtu(self, mtu: int, port: Port) -> None:

This is missing a docstring.
The way I've decided to document these overridden abstract methods is
to just to link to the superclass's method, used heavily in
https://patches.dpdk.org/project/dpdk/patch/20231115130959.39420-17-juraj.linkes@pantheon.tech/:
"""Overrides :meth:`~.os_session.OSSession.configure_port_mtu`."""

The docstring checker complains if the docstring is missing. There may
be better ways, such as with @typing.override (but that requires
Python 3.12 or typing_extensions, but there's a bug in Pylama that
prevents us from using that solution:
https://github.com/klen/pylama/pull/247).

> +        self.send_command(
> +            f"ip link set dev {port.logical_name} mtu {mtu}",
> +            privileged=True,
> +            verify=True,
> +        )
> +
>      def configure_ipv4_forwarding(self, enable: bool) -> None:
>          state = 1 if enable else 0
>          self.send_command(f"sysctl -w net.ipv4.ip_forward={state}", privileged=True)
> diff --git a/dts/framework/remote_session/os_session.py b/dts/framework/remote_session/os_session.py
> index 8a709eac1c..c038f78b79 100644
> --- a/dts/framework/remote_session/os_session.py
> +++ b/dts/framework/remote_session/os_session.py
> @@ -277,6 +277,15 @@ def configure_port_ip_address(
>          Configure (add or delete) an IP address of the input port.
>          """
>
> +    @abstractmethod
> +    def configure_port_mtu(self, mtu: int, port: Port) -> None:
> +        """Configure MTU on a given port.
> +
> +        Args:
> +            mtu: Desired MTU value.
> +            port: Port to set the MTU on.
> +        """

I've compiled the rules for composing docstrings here:
https://patches.dpdk.org/project/dpdk/patch/20231115130959.39420-4-juraj.linkes@pantheon.tech/.

The relevant part here is:

When referencing a parameter of a function or a method in their
docstring, don't use
any articles and put the parameter into single backticks. This mimics
the style of
`Python's documentation <https://docs.python.org/3/index.html>`_.

Both the mtu and the port parameters are mentioned, so they should be
without articles and in backticks.

> +
>      @abstractmethod
>      def configure_ipv4_forwarding(self, enable: bool) -> None:
>          """
> --
> 2.42.0
>
Jeremy Spewock Nov. 17, 2023, 5:06 p.m. UTC | #2 
Both good points, I'll be sure to add/adjust those docstrings.

On Thu, Nov 16, 2023 at 1:05 PM Juraj Linkeš <juraj.linkes@pantheon.tech>
wrote:

> On Mon, Nov 13, 2023 at 9:28 PM <jspewock@iol.unh.edu> wrote:
> >
> > From: Jeremy Spewock <jspewock@iol.unh.edu>
> >
> > Adds methods in both os_session and linux session to allow for setting
> > MTU of port interfaces in an OS agnostic way.
> >
> > Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> > ---
> >  dts/framework/remote_session/linux_session.py | 7 +++++++
> >  dts/framework/remote_session/os_session.py    | 9 +++++++++
> >  2 files changed, 16 insertions(+)
> >
> > diff --git a/dts/framework/remote_session/linux_session.py
> b/dts/framework/remote_session/linux_session.py
> > index a3f1a6bf3b..dab68d41b1 100644
> > --- a/dts/framework/remote_session/linux_session.py
> > +++ b/dts/framework/remote_session/linux_session.py
> > @@ -196,6 +196,13 @@ def configure_port_ip_address(
> >              verify=True,
> >          )
> >
> > +    def configure_port_mtu(self, mtu: int, port: Port) -> None:
>
> This is missing a docstring.
> The way I've decided to document these overridden abstract methods is
> to just to link to the superclass's method, used heavily in
>
> https://patches.dpdk.org/project/dpdk/patch/20231115130959.39420-17-juraj.linkes@pantheon.tech/
> :
> """Overrides :meth:`~.os_session.OSSession.configure_port_mtu`."""
>
> The docstring checker complains if the docstring is missing. There may
> be better ways, such as with @typing.override (but that requires
> Python 3.12 or typing_extensions, but there's a bug in Pylama that
> prevents us from using that solution:
> https://github.com/klen/pylama/pull/247).
>
> > +        self.send_command(
> > +            f"ip link set dev {port.logical_name} mtu {mtu}",
> > +            privileged=True,
> > +            verify=True,
> > +        )
> > +
> >      def configure_ipv4_forwarding(self, enable: bool) -> None:
> >          state = 1 if enable else 0
> >          self.send_command(f"sysctl -w net.ipv4.ip_forward={state}",
> privileged=True)
> > diff --git a/dts/framework/remote_session/os_session.py
> b/dts/framework/remote_session/os_session.py
> > index 8a709eac1c..c038f78b79 100644
> > --- a/dts/framework/remote_session/os_session.py
> > +++ b/dts/framework/remote_session/os_session.py
> > @@ -277,6 +277,15 @@ def configure_port_ip_address(
> >          Configure (add or delete) an IP address of the input port.
> >          """
> >
> > +    @abstractmethod
> > +    def configure_port_mtu(self, mtu: int, port: Port) -> None:
> > +        """Configure MTU on a given port.
> > +
> > +        Args:
> > +            mtu: Desired MTU value.
> > +            port: Port to set the MTU on.
> > +        """
>
> I've compiled the rules for composing docstrings here:
>
> https://patches.dpdk.org/project/dpdk/patch/20231115130959.39420-4-juraj.linkes@pantheon.tech/
> .
>
> The relevant part here is:
>
> When referencing a parameter of a function or a method in their
> docstring, don't use
> any articles and put the parameter into single backticks. This mimics
> the style of
> `Python's documentation <https://docs.python.org/3/index.html>`_.
>
> Both the mtu and the port parameters are mentioned, so they should be
> without articles and in backticks.
>
> > +
> >      @abstractmethod
> >      def configure_ipv4_forwarding(self, enable: bool) -> None:
> >          """
> > --
> > 2.42.0
> >
>
diff mbox series

Patch

diff --git a/dts/framework/remote_session/linux_session.py b/dts/framework/remote_session/linux_session.py
index a3f1a6bf3b..dab68d41b1 100644
--- a/dts/framework/remote_session/linux_session.py
+++ b/dts/framework/remote_session/linux_session.py
@@ -196,6 +196,13 @@  def configure_port_ip_address(
             verify=True,
         )
 
+    def configure_port_mtu(self, mtu: int, port: Port) -> None:
+        self.send_command(
+            f"ip link set dev {port.logical_name} mtu {mtu}",
+            privileged=True,
+            verify=True,
+        )
+
     def configure_ipv4_forwarding(self, enable: bool) -> None:
         state = 1 if enable else 0
         self.send_command(f"sysctl -w net.ipv4.ip_forward={state}", privileged=True)
diff --git a/dts/framework/remote_session/os_session.py b/dts/framework/remote_session/os_session.py
index 8a709eac1c..c038f78b79 100644
--- a/dts/framework/remote_session/os_session.py
+++ b/dts/framework/remote_session/os_session.py
@@ -277,6 +277,15 @@  def configure_port_ip_address(
         Configure (add or delete) an IP address of the input port.
         """
 
+    @abstractmethod
+    def configure_port_mtu(self, mtu: int, port: Port) -> None:
+        """Configure MTU on a given port.
+
+        Args:
+            mtu: Desired MTU value.
+            port: Port to set the MTU on.
+        """
+
     @abstractmethod
     def configure_ipv4_forwarding(self, enable: bool) -> None:
         """