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 9FEF043382; Wed, 22 Nov 2023 14:36:07 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7A3DF402E8; Wed, 22 Nov 2023 14:36:07 +0100 (CET) Received: from mail-ej1-f45.google.com (mail-ej1-f45.google.com [209.85.218.45]) by mails.dpdk.org (Postfix) with ESMTP id 9325A4028C for ; Wed, 22 Nov 2023 14:36:05 +0100 (CET) Received: by mail-ej1-f45.google.com with SMTP id a640c23a62f3a-a03a900956dso168747366b.1 for ; Wed, 22 Nov 2023 05:36:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pantheon.tech; s=google; t=1700660165; x=1701264965; darn=dpdk.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=bwla7/+WtIqxSZFJA0cO1YQXnOf4xmsVZqILH9vSvFM=; b=OsZ/ZpkWeQcw1COtwU+3jIfis0315lkEdBd8t9LmyMaIPIVnknomFZ+9faLq0a6Brn q1JKhyQHUCadeTp06Pv6dzOwRnQ1F7YzYrnJEJkMMX/iEDQ1ApFWTNw5oNNO5WAzrV+U FyAW/WpHeN7wsHhYCP5j7cLKMym5btJxlPTEiT9R+EMiYS/H3Y+FsJqUnSXDrFGf2WK3 EmLhBeX0OhNOuJgjNHzIGsm+3OpsimVcByaDAoGCjdEf4frlXqagMNmh6lxQ4W6cwff4 /R+rkCLGIgK2bt834s7VLhRVnmuyWLX8KANhwMySGTW1/wDJ4tUEnU3U/+u6K0m8Uueu QVfg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700660165; x=1701264965; h=content-transfer-encoding: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=bwla7/+WtIqxSZFJA0cO1YQXnOf4xmsVZqILH9vSvFM=; b=gefy8NGgH2HAa3ftAxmuZcTp4iEVGBJ1qIX+E1YOTrjHPtT2j/SxSbQrWKPkx6kObN YV6+aqHG/Ov40aY6N8gLZBj17bWSnqzXJj72k0i6gGODQ096xIHCEe50En7++hVQBe4k 3gf8N1AkYfXX6c7rkQ1mhNo0bCLjam/ZDpf/z4bOdTD3MMSNNBOig0WYnaAALGu/eaot i4DMBZf1X4bRtDUfzJYtCML69hg4G36u77OwGdF61SgKDlVoEDDS7v6QX4XeLBZzbEzh swHbAeielfdCCJZktjVsVTEWgWlcSvd+xc9fQP5kCCkPgbt9R8WqmobLOFg4f/xKSiHR nIUw== X-Gm-Message-State: AOJu0Yxy+4v44L/RZ/hVe/pjr4XNhv0HU5CKU1gcxX6h93w8poYHxxft 2zUw7gkTbeVgCRJCfrX/+sP5/dYwNWeE5SjroNqeFw== X-Google-Smtp-Source: AGHT+IFVV6oODifsMzUy05FrrkDqKavieKen1/xUNKWX+OVqnksAnXT6X5tEdebFUvqhEhEzcsdMPJB1XRrtLn2W0rg= X-Received: by 2002:a17:906:3f45:b0:9fa:7c87:b10a with SMTP id f5-20020a1709063f4500b009fa7c87b10amr2233145ejj.10.1700660165066; Wed, 22 Nov 2023 05:36:05 -0800 (PST) MIME-Version: 1.0 References: <20231108125324.191005-23-juraj.linkes@pantheon.tech> <20231115130959.39420-1-juraj.linkes@pantheon.tech> <20231115130959.39420-17-juraj.linkes@pantheon.tech> <6b14a66b-9b05-4ada-9a92-c3b6ead5eb8e@foss.arm.com> In-Reply-To: <6b14a66b-9b05-4ada-9a92-c3b6ead5eb8e@foss.arm.com> From: =?UTF-8?Q?Juraj_Linke=C5=A1?= Date: Wed, 22 Nov 2023 14:35:54 +0100 Message-ID: Subject: Re: [PATCH v7 16/21] dts: posix and linux sessions docstring update To: Yoan Picchi Cc: thomas@monjalon.net, Honnappa.Nagarahalli@arm.com, jspewock@iol.unh.edu, probb@iol.unh.edu, paul.szczepanek@arm.com, dev@dpdk.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 On Wed, Nov 22, 2023 at 2:24=E2=80=AFPM Yoan Picchi wrote: > > On 11/15/23 13:09, Juraj Linke=C5=A1 wrote: > > Format according to the Google format and PEP257, with slight > > deviations. > > > > Signed-off-by: Juraj Linke=C5=A1 > > --- > > dts/framework/testbed_model/linux_session.py | 63 ++++++++++----- > > dts/framework/testbed_model/posix_session.py | 81 +++++++++++++++++--= - > > 2 files changed, 113 insertions(+), 31 deletions(-) > > > > diff --git a/dts/framework/testbed_model/linux_session.py b/dts/framewo= rk/testbed_model/linux_session.py > > index f472bb8f0f..279954ff63 100644 > > --- a/dts/framework/testbed_model/linux_session.py > > +++ b/dts/framework/testbed_model/linux_session.py > > @@ -2,6 +2,13 @@ > > # Copyright(c) 2023 PANTHEON.tech s.r.o. > > # Copyright(c) 2023 University of New Hampshire > > > > +"""Linux OS translator. > > + > > +Translate OS-unaware calls into Linux calls/utilities. Most of Linux d= istributions are mostly > > +compliant with POSIX standards, so this module only implements the par= ts that aren't. > > +This intermediate module implements the common parts of mostly POSIX c= ompliant distributions. > > +""" > > + > > import json > > from ipaddress import IPv4Interface, IPv6Interface > > from typing import TypedDict, Union > > @@ -17,43 +24,51 @@ > > > > > > class LshwConfigurationOutput(TypedDict): > > + """The relevant parts of ``lshw``'s ``configuration`` section.""" > > + > > + #: > > link: str > > > > > > class LshwOutput(TypedDict): > > - """ > > - A model of the relevant information from json lshw output, e.g.: > > - { > > - ... > > - "businfo" : "pci@0000:08:00.0", > > - "logicalname" : "enp8s0", > > - "version" : "00", > > - "serial" : "52:54:00:59:e1:ac", > > - ... > > - "configuration" : { > > - ... > > - "link" : "yes", > > - ... > > - }, > > - ... > > + """A model of the relevant information from ``lshw``'s json output= . > > + > > + e.g.:: > > + > > + { > > + ... > > + "businfo" : "pci@0000:08:00.0", > > + "logicalname" : "enp8s0", > > + "version" : "00", > > + "serial" : "52:54:00:59:e1:ac", > > + ... > > + "configuration" : { > > + ... > > + "link" : "yes", > > + ... > > + }, > > + ... > > """ > > > > + #: > > businfo: str > > + #: > > logicalname: NotRequired[str] > > + #: > > serial: NotRequired[str] > > + #: > > configuration: LshwConfigurationOutput > > > > > > class LinuxSession(PosixSession): > > - """ > > - The implementation of non-Posix compliant parts of Linux remote se= ssions. > > - """ > > + """The implementation of non-Posix compliant parts of Linux.""" > > > > @staticmethod > > def _get_privileged_command(command: str) -> str: > > return f"sudo -- sh -c '{command}'" > > > > def get_remote_cpus(self, use_first_core: bool) -> list[LogicalCo= re]: > > + """Overrides :meth:`~.os_session.OSSession.get_remote_cpus`.""= " > > cpu_info =3D self.send_command("lscpu -p=3DCPU,CORE,SOCKET,NO= DE|grep -v \\#").stdout > > lcores =3D [] > > for cpu_line in cpu_info.splitlines(): > > @@ -65,18 +80,20 @@ def get_remote_cpus(self, use_first_core: bool) -> = list[LogicalCore]: > > return lcores > > > > def get_dpdk_file_prefix(self, dpdk_prefix: str) -> str: > > + """Overrides :meth:`~.os_session.OSSession.get_dpdk_file_prefi= x`.""" > > return dpdk_prefix > > > > - def setup_hugepages(self, hugepage_amount: int, force_first_numa: = bool) -> None: > > + def setup_hugepages(self, hugepage_count: int, force_first_numa: b= ool) -> None: > > + """Overrides :meth:`~.os_session.OSSession.setup_hugepages`.""= " > > self._logger.info("Getting Hugepage information.") > > hugepage_size =3D self._get_hugepage_size() > > hugepages_total =3D self._get_hugepages_total() > > self._numa_nodes =3D self._get_numa_nodes() > > > > - if force_first_numa or hugepages_total !=3D hugepage_amount: > > + if force_first_numa or hugepages_total !=3D hugepage_count: > > # when forcing numa, we need to clear existing hugepages = regardless > > # of size, so they can be moved to the first numa node > > - self._configure_huge_pages(hugepage_amount, hugepage_size,= force_first_numa) > > + self._configure_huge_pages(hugepage_count, hugepage_size, = force_first_numa) > > else: > > self._logger.info("Hugepages already configured.") > > self._mount_huge_pages() > > @@ -140,6 +157,7 @@ def _configure_huge_pages( > > ) > > > > def update_ports(self, ports: list[Port]) -> None: > > + """Overrides :meth:`~.os_session.OSSession.update_ports`.""" > > self._logger.debug("Gathering port info.") > > for port in ports: > > assert ( > > @@ -178,6 +196,7 @@ def _update_port_attr( > > ) > > > > def configure_port_state(self, port: Port, enable: bool) -> None: > > + """Overrides :meth:`~.os_session.OSSession.configure_port_stat= e`.""" > > state =3D "up" if enable else "down" > > self.send_command( > > f"ip link set dev {port.logical_name} {state}", privilege= d=3DTrue > > @@ -189,6 +208,7 @@ def configure_port_ip_address( > > port: Port, > > delete: bool, > > ) -> None: > > + """Overrides :meth:`~.os_session.OSSession.configure_port_ip_a= ddress`.""" > > command =3D "del" if delete else "add" > > self.send_command( > > f"ip address {command} {address} dev {port.logical_name}"= , > > @@ -197,5 +217,6 @@ def configure_port_ip_address( > > ) > > > > def configure_ipv4_forwarding(self, enable: bool) -> None: > > + """Overrides :meth:`~.os_session.OSSession.configure_ipv4_forw= arding`.""" > > 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/testbed_model/posix_session.py b/dts/framewo= rk/testbed_model/posix_session.py > > index 1d1d5b1b26..a4824aa274 100644 > > --- a/dts/framework/testbed_model/posix_session.py > > +++ b/dts/framework/testbed_model/posix_session.py > > @@ -2,6 +2,15 @@ > > # Copyright(c) 2023 PANTHEON.tech s.r.o. > > # Copyright(c) 2023 University of New Hampshire > > > > +"""POSIX compliant OS translator. > > + > > +Translates OS-unaware calls into POSIX compliant calls/utilities. POSI= X is a set of standards > > +for portability between Unix operating systems which not all Linux dis= tributions > > +(or the tools most frequently bundled with said distributions) adhere = to. Most of Linux > > +distributions are mostly compliant though. > > +This intermediate module implements the common parts of mostly POSIX c= ompliant distributions. > > +""" > > + > > import re > > from collections.abc import Iterable > > from pathlib import PurePath, PurePosixPath > > @@ -15,13 +24,21 @@ > > > > > > class PosixSession(OSSession): > > - """ > > - An intermediary class implementing the Posix compliant parts of > > - Linux and other OS remote sessions. > > - """ > > + """An intermediary class implementing the POSIX standard.""" > > > > @staticmethod > > def combine_short_options(**opts: bool) -> str: > > + """Combine shell options into one argument. > > + > > + These are options such as ``-x``, ``-v``, ``-f`` which are com= bined into ``-xvf``. > > + > > + Args: > > + opts: The keys are option names (usually one letter) and t= he bool values indicate > > + whether to include the option in the resulting argumen= t. > > + > > + Returns: > > + The options combined into one argument. > > + """ > > ret_opts =3D "" > > for opt, include in opts.items(): > > if include: > > @@ -33,17 +50,19 @@ def combine_short_options(**opts: bool) -> str: > > return ret_opts > > > > def guess_dpdk_remote_dir(self, remote_dir: str | PurePath) -> Pu= rePosixPath: > > + """Overrides :meth:`~.os_session.OSSession.guess_dpdk_remote_d= ir`.""" > > remote_guess =3D self.join_remote_path(remote_dir, "dpdk-*") > > result =3D self.send_command(f"ls -d {remote_guess} | tail -1= ") > > return PurePosixPath(result.stdout) > > > > def get_remote_tmp_dir(self) -> PurePosixPath: > > + """Overrides :meth:`~.os_session.OSSession.get_remote_tmp_dir`= .""" > > return PurePosixPath("/tmp") > > > > def get_dpdk_build_env_vars(self, arch: Architecture) -> dict: > > - """ > > - Create extra environment variables needed for i686 arch build.= Get information > > - from the node if needed. > > + """Overrides :meth:`~.os_session.OSSession.get_dpdk_build_env_= vars`. > > + > > + Supported architecture: ``i686``. > > """ > > env_vars =3D {} > > if arch =3D=3D Architecture.i686: > > @@ -63,6 +82,7 @@ def get_dpdk_build_env_vars(self, arch: Architecture)= -> dict: > > return env_vars > > > > def join_remote_path(self, *args: str | PurePath) -> PurePosixPat= h: > > + """Overrides :meth:`~.os_session.OSSession.join_remote_path`."= "" > > return PurePosixPath(*args) > > > > def copy_from( > > @@ -70,6 +90,7 @@ def copy_from( > > source_file: str | PurePath, > > destination_file: str | PurePath, > > ) -> None: > > + """Overrides :meth:`~.os_session.OSSession.copy_from`.""" > > self.remote_session.copy_from(source_file, destination_file) > > > > def copy_to( > > @@ -77,6 +98,7 @@ def copy_to( > > source_file: str | PurePath, > > destination_file: str | PurePath, > > ) -> None: > > + """Overrides :meth:`~.os_session.OSSession.copy_to`.""" > > self.remote_session.copy_to(source_file, destination_file) > > > > def remove_remote_dir( > > @@ -85,6 +107,7 @@ def remove_remote_dir( > > recursive: bool =3D True, > > force: bool =3D True, > > ) -> None: > > + """Overrides :meth:`~.os_session.OSSession.remove_remote_dir`.= """ > > opts =3D PosixSession.combine_short_options(r=3Drecursive, f= =3Dforce) > > self.send_command(f"rm{opts} {remote_dir_path}") > > > > @@ -93,6 +116,7 @@ def extract_remote_tarball( > > remote_tarball_path: str | PurePath, > > expected_dir: str | PurePath | None =3D None, > > ) -> None: > > + """Overrides :meth:`~.os_session.OSSession.extract_remote_tarb= all`.""" > > self.send_command( > > f"tar xfm {remote_tarball_path} " > > f"-C {PurePosixPath(remote_tarball_path).parent}", > > @@ -110,6 +134,7 @@ def build_dpdk( > > rebuild: bool =3D False, > > timeout: float =3D SETTINGS.compile_timeout, > > ) -> None: > > + """Overrides :meth:`~.os_session.OSSession.build_dpdk`.""" > > try: > > if rebuild: > > # reconfigure, then build > > @@ -140,12 +165,14 @@ def build_dpdk( > > raise DPDKBuildError(f"DPDK build failed when doing '{e.c= ommand}'.") > > > > def get_dpdk_version(self, build_dir: str | PurePath) -> str: > > + """Overrides :meth:`~.os_session.OSSession.get_dpdk_version`."= "" > > out =3D self.send_command( > > f"cat {self.join_remote_path(build_dir, 'VERSION')}", ver= ify=3DTrue > > ) > > return out.stdout > > > > def kill_cleanup_dpdk_apps(self, dpdk_prefix_list: Iterable[str])= -> None: > > + """Overrides :meth:`~.os_session.OSSession.kill_cleanup_dpdk_a= pps`.""" > > self._logger.info("Cleaning up DPDK apps.") > > dpdk_runtime_dirs =3D self._get_dpdk_runtime_dirs(dpdk_prefix= _list) > > if dpdk_runtime_dirs: > > @@ -159,6 +186,14 @@ def kill_cleanup_dpdk_apps(self, dpdk_prefix_list:= Iterable[str]) -> None: > > def _get_dpdk_runtime_dirs( > > self, dpdk_prefix_list: Iterable[str] > > ) -> list[PurePosixPath]: > > + """Find runtime directories DPDK apps are currently using. > > + > > + Args: > > + dpdk_prefix_list: The prefixes DPDK apps were started wi= th. > > + > > + Returns: > > + The paths of DPDK apps' runtime dirs. > > + """ > > prefix =3D PurePosixPath("/var", "run", "dpdk") > > if not dpdk_prefix_list: > > remote_prefixes =3D self._list_remote_dirs(prefix) > > @@ -170,9 +205,13 @@ def _get_dpdk_runtime_dirs( > > return [PurePosixPath(prefix, dpdk_prefix) for dpdk_prefix in= dpdk_prefix_list] > > > > def _list_remote_dirs(self, remote_path: str | PurePath) -> list[= str] | None: > > - """ > > - Return a list of directories of the remote_dir. > > - If remote_path doesn't exist, return None. > > + """Contents of remote_path. > > + > > + Args: > > + remote_path: List the contents of this path. > > + > > + Returns: > > + The contents of remote_path. If remote_path doesn't exist,= return None. > > """ > > out =3D self.send_command( > > f"ls -l {remote_path} | awk '/^d/ {{print $NF}}'" > > @@ -183,6 +222,17 @@ def _list_remote_dirs(self, remote_path: str | Pur= ePath) -> list[str] | None: > > return out.splitlines() > > > > def _get_dpdk_pids(self, dpdk_runtime_dirs: Iterable[str | PurePa= th]) -> list[int]: > > + """Find PIDs of running DPDK apps. > > + > > + Look at each "config" file found in dpdk_runtime_dirs and find= the PIDs of processes > > + that opened those file. > > + > > + Args: > > + dpdk_runtime_dirs: The paths of DPDK apps' runtime dirs. > > + > > + Returns: > > + The PIDs of running DPDK apps. > > + """ > > pids =3D [] > > pid_regex =3D r"p(\d+)" > > for dpdk_runtime_dir in dpdk_runtime_dirs: > > @@ -203,6 +253,14 @@ def _remote_files_exists(self, remote_path: PurePa= th) -> bool: > > def _check_dpdk_hugepages( > > self, dpdk_runtime_dirs: Iterable[str | PurePath] > > ) -> None: > > + """Check there aren't any leftover hugepages. > > + > > + If any hugegapes are found, emit a warning. The hugepages are = investigated in the > > hugegapes -> hugepages > Ack. > > + "hugepage_info" file of dpdk_runtime_dirs. > > + > > + Args: > > + dpdk_runtime_dirs: The paths of DPDK apps' runtime dirs. > > + """ > > for dpdk_runtime_dir in dpdk_runtime_dirs: > > hugepage_info =3D PurePosixPath(dpdk_runtime_dir, "hugepa= ge_info") > > if self._remote_files_exists(hugepage_info): > > @@ -220,9 +278,11 @@ def _remove_dpdk_runtime_dirs( > > self.remove_remote_dir(dpdk_runtime_dir) > > > > def get_dpdk_file_prefix(self, dpdk_prefix: str) -> str: > > + """Overrides :meth:`~.os_session.OSSession.get_dpdk_file_prefi= x`.""" > > return "" > > > > def get_compiler_version(self, compiler_name: str) -> str: > > + """Overrides :meth:`~.os_session.OSSession.get_compiler_versio= n`.""" > > match compiler_name: > > case "gcc": > > return self.send_command( > > @@ -240,6 +300,7 @@ def get_compiler_version(self, compiler_name: str) = -> str: > > raise ValueError(f"Unknown compiler {compiler_name}") > > > > def get_node_info(self) -> NodeInfo: > > + """Overrides :meth:`~.os_session.OSSession.get_node_info`.""" > > os_release_info =3D self.send_command( > > "awk -F=3D '$1 ~ /^NAME$|^VERSION$/ {print $2}' /etc/os-r= elease", > > SETTINGS.timeout, >