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

["Juraj Linkeš" <juraj.linkes@pantheon.tech>]

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
>