From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 62F5D43357; Fri, 17 Nov 2023 18:07:03 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 505DD402EA; Fri, 17 Nov 2023 18:07:03 +0100 (CET) Received: from mail-pg1-f172.google.com (mail-pg1-f172.google.com [209.85.215.172]) by mails.dpdk.org (Postfix) with ESMTP id 975CD40285 for ; Fri, 17 Nov 2023 18:07:02 +0100 (CET) Received: by mail-pg1-f172.google.com with SMTP id 41be03b00d2f7-5bcfc508d14so1774085a12.3 for ; Fri, 17 Nov 2023 09:07:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1700240821; x=1700845621; darn=dpdk.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=yFfPQDw+E/ODw/I9idDxrikKmoPMuZU9vxOregS2bW0=; b=X1pPFZw6aMGQ5gCokgjqlxqVBcy9wFQVPHE2KrJiXtXwZt1ueOC62tPCiFCwZN07zw qGHBBaSaF84EZEiEGAqb4UdH0co0pH1iA+D6q6o8qZEXVZUUtwicUSFAL3pCar9fUBXT OoJD7XCxhUshAhG0EAYI4P86TRgd2ba4PJPVY= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700240821; x=1700845621; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=yFfPQDw+E/ODw/I9idDxrikKmoPMuZU9vxOregS2bW0=; b=eSPAgleUgFJkY2OlbJkxZ9Rx0jEWn16vpIS2f6BR7mRGwoL8PlDIJC0oIRRLw5WpdM aOCVGQ8W+5O1xhtIUZqbubHEkDlW4xADYg+XqICGP3nBpbdtIChnnAzgEoukXA+obqx5 4iQ0V2M5czcr91FshxAL3Un4R3SU8HM39osxHNrftAZ3pCtqU2TedWazlqtlkFsP6K2t TKfKAOUFW8aYAAPSRCseu0bR37PyeixoC6M/UJmnpl4VwsopcajBupDuPi187gd1Ilq1 1cqrLzCuN1cRkAhWHrewACCgfnp7xF1ocBnVuabhrlCzayVpRhmeCv3IcGqhSZ0IKIbQ Z8dw== X-Gm-Message-State: AOJu0Yy6BauXjw+0MTGm7L5Z1h5flDj+IrFWb7y/lJMWW7hyHVgMSuWo XtWKUMw75uJC6SnF55qW/9JJijvTd2N66YYyKy8w1g== X-Google-Smtp-Source: AGHT+IGoNcfVbDeptkofnR7BikBD/0Z7YzfOKz3D7BXby0vMPQV8GWqGdovLFXT67A16Qsa/u4IyOWc7kPZgGYsJwic= X-Received: by 2002:a17:90a:9901:b0:263:f630:228f with SMTP id b1-20020a17090a990100b00263f630228fmr60685pjp.23.1700240821349; Fri, 17 Nov 2023 09:07:01 -0800 (PST) MIME-Version: 1.0 References: <20231113202833.12900-1-jspewock@iol.unh.edu> <20231113202833.12900-8-jspewock@iol.unh.edu> In-Reply-To: From: Jeremy Spewock Date: Fri, 17 Nov 2023 12:06:49 -0500 Message-ID: Subject: Re: [PATCH v3 7/7] dts: allow configuring MTU of ports To: =?UTF-8?Q?Juraj_Linke=C5=A1?= Cc: Honnappa.Nagarahalli@arm.com, thomas@monjalon.net, wathsala.vithanage@arm.com, probb@iol.unh.edu, paul.szczepanek@arm.com, yoan.picchi@foss.arm.com, ferruh.yigit@amd.com, andrew.rybchenko@oktetlabs.ru, dev@dpdk.org Content-Type: multipart/alternative; boundary="00000000000030832e060a5c2a38" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org --00000000000030832e060a5c2a38 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Both good points, I'll be sure to add/adjust those docstrings. On Thu, Nov 16, 2023 at 1:05=E2=80=AFPM Juraj Linke=C5=A1 wrote: > On Mon, Nov 13, 2023 at 9:28=E2=80=AFPM wrote: > > > > From: Jeremy Spewock > > > > 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 > > --- > > 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=3DTrue, > > ) > > > > + 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=3DTrue, > > + verify=3DTrue, > > + ) > > + > > def configure_ipv4_forwarding(self, enable: bool) -> None: > > state =3D 1 if enable else 0 > > self.send_command(f"sysctl -w net.ipv4.ip_forward=3D{state}", > privileged=3DTrue) > > 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 `_. > > 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 > > > --00000000000030832e060a5c2a38 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Both good points, I'll be sure to add/adjust those docstrings= .

On Thu, Nov 16, 2023 at 1:05=E2=80=AFPM Juraj Linke=C5=A1 <ju= raj.linkes@pantheon.tech> wrote:
On Mon, Nov 13, 2023 at 9:28=E2=80=AFPM <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>
> ---
>=C2=A0 dts/framework/remote_session/linux_session.py | 7 +++++++
>=C2=A0 dts/framework/remote_session/os_session.py=C2=A0 =C2=A0 | 9 ++++= +++++
>=C2=A0 2 files changed, 16 insertions(+)
>
> diff --git a/dts/framework/remote_session/linux_session.py b/dts/frame= work/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(
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 verify=3DTrue,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 )
>
> +=C2=A0 =C2=A0 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_mt= u`."""

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).

> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.send_command(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 f"ip link set dev {por= t.logical_name} mtu {mtu}",
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 privileged=3DTrue,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 verify=3DTrue,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 )
> +
>=C2=A0 =C2=A0 =C2=A0 def configure_ipv4_forwarding(self, enable: bool) = -> None:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 state =3D 1 if enable else 0
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self.send_command(f"sysctl -w n= et.ipv4.ip_forward=3D{state}", privileged=3DTrue)
> diff --git a/dts/framework/remote_session/os_session.py b/dts/framewor= k/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(
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Configure (add or delete) an IP addr= ess of the input port.
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 """
>
> +=C2=A0 =C2=A0 @abstractmethod
> +=C2=A0 =C2=A0 def configure_port_mtu(self, mtu: int, port: Port) ->= ; None:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """Configure MTU on a give= n port.
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Args:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 mtu: Desired MTU value.
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 port: Port to set the MTU o= n.
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """

I've compiled the rules for composing docstrings here:
https:= //patches.dpdk.org/project/dpdk/patch/20231115130959.39420-4-juraj.linkes@p= antheon.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.

> +
>=C2=A0 =C2=A0 =C2=A0 @abstractmethod
>=C2=A0 =C2=A0 =C2=A0 def configure_ipv4_forwarding(self, enable: bool) = -> None:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> --
> 2.42.0
>
--00000000000030832e060a5c2a38--