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 4AAB7A00C2; Wed, 2 Nov 2022 14:15:11 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 3C7DC40223; Wed, 2 Nov 2022 14:15:11 +0100 (CET) Received: from out4-smtp.messagingengine.com (out4-smtp.messagingengine.com [66.111.4.28]) by mails.dpdk.org (Postfix) with ESMTP id 66C5540041 for ; Wed, 2 Nov 2022 14:15:09 +0100 (CET) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id C83655C010B; Wed, 2 Nov 2022 09:15:08 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Wed, 02 Nov 2022 09:15:08 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:cc:content-transfer-encoding:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to; s=fm3; t=1667394908; x= 1667481308; bh=y73kquXLgpwBUH/0nmO7RKITOagSdTaq8Y9utrjU1ac=; b=S mM0MSBbYOnx96FqS3YuLJt7dVaIokZSwwimnB5JbQIP4ZtI51LFzev0fypDTC0xj /5D4mr7cdik0yvcXUvHnSo6hRu7RIfSoHcuv6TzCi5XKS7JRwX2eCoJSP6p8egy3 8DAxGHcPg2v72uyw83CaZpFiHEp8JnB2JjszpLK85mlTjEdyVbqiVtoM6m8dFOfV UEQ+JZDoi65f8rROjXl6JoMjjVkb/x0xo7cwI/V2pPl1al+pMtCcOsa0YWieMKcj wMTMp5ZSbmw6C5zqWXgMBvn0x9glOKILaXeb3XztrWbgApFUbfu+DHaFnB+TbT7K UIXtMhdzxzAmw30XRbCEw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:date:date:feedback-id:feedback-id:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm3; t=1667394908; x= 1667481308; bh=y73kquXLgpwBUH/0nmO7RKITOagSdTaq8Y9utrjU1ac=; b=K KebcZTjhh+fGkQtG/wIZUs2KmhZTcAFwSX9c7n8MJoAcO00mQ9T2J4zw0J2PdpPz JBS4OUdd4EvacNy3REjQ9f757A6HlLzT0M6uXH2osKwI4KIl5um8/tptSzeRyKUQ 0BUWxPPedH5aiWFzpx4vvEETdufVUJ8NgQ85hSf3CHbE1Hhf/lT2BrHEVLhyn0FU h8H6bkPN7aOKT0dToAFGainN5JbWGvLjj8sIAregHtYBuUuhbV/wc4kzIeNhIf20 PSNuV/wCg5j5Px9dC+Gz6y8IQOO4+5T5FmLknoW7xNBtWlxAjlriWFY9u4qs/9PR tO1WFPZg+UDpjkmcdUpmQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvgedrudejgdegkecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvfevufffkfgjfhgggfgtsehtqhertddttdejnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg ftrfgrthhtvghrnhepgedttdeljeejgeffkeekkedtjeevtdehvedtkeeivdeuuedviedu vdelveejueejnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrh homhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth X-ME-Proxy: Feedback-ID: i47234305:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 2 Nov 2022 09:15:07 -0400 (EDT) From: Thomas Monjalon To: Owen Hilyard Cc: Juraj =?utf-8?B?TGlua2XFoQ==?= , Honnappa.Nagarahalli@arm.com, lijuan.tu@intel.com, kda@semihalf.com, bruce.richardson@intel.com, dev@dpdk.org Subject: Re: [PATCH v6 00/10] dts: ssh connection to a node Date: Wed, 02 Nov 2022 14:15:05 +0100 Message-ID: <6293479.j6PcuT4dK6@thomas> In-Reply-To: References: <20221013103517.3443997-1-juraj.linkes@pantheon.tech> <5711158.zQ0Gbyo6oJ@thomas> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="UTF-8" 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 02/11/2022 13:58, Owen Hilyard: > On Mon, Oct 31, 2022 at 3:01 PM Thomas Monjalon wro= te: >=20 > > I was about to merge this series, > > and after long thoughts, it deserves a bit more changes. > > I would like to work with you for a merge in 22.11-rc3. > > > > 13/10/2022 12:35, Juraj Linke=C5=A1: > > > All the necessary code needed to connect to a node in a topology with > > > a bit more, such as basic logging and some extra useful methods. > > > > There is also some developer tooling, > > and some documentation. > > > > [...] > > > There are configuration files with a README that help with setting up > > > the execution/development environment. > > > > I don't want to merge some doc which is not integrated > > in the doc/ directory. > > It should be in RST format in doc/guides/dts/ > > I can help with this conversion. > > > > > The code only connects to a node. You'll see logs emitted to console > > > saying where DTS connected. > > > > > > There's only a bit of documentation, as there's not much to document. > > > We'll add some real docs when there's enough functionality to documen= t, > > > when the HelloWorld testcases is in (point 4 in our roadmap below). W= hat > > > will be documented later is runtime dependencies and how to set up the > > DTS > > > control node environment. > > > > > [...] > > > .editorconfig | 2 +- > > > .gitignore | 9 +- > > > > Updating general Python guidelines in these files > > should be done separately to get broader agreement. > > > > > MAINTAINERS | 5 + > > > > You can update this file in the first patch. > > > > > devtools/python-checkpatch.sh | 39 ++ > > > > Let's postpone the integration of checkpatch. > > It should be integrated with the existing checkpatch. > > >=20 > We wanted to specifically ensure that all code met the quality requiremen= ts > for DTS from the start. The current formatting requirements are "whatever > these tools run in this order with these settings results in", since the > working group decided that fully automated rectification of minor issues > would help new contributors focus on more important things. I agree that > integrating it into existing checkpatch would be good, but I think that to > do that we would first need to run the tool over existing DPDK python cod= e. > python-checkpatch does do linting like the main checkpatch, but it will > also perform source code changes to automatically fix many formatting > issues. Do we want to keep checkpatch as a read-only script and introduce > another one which makes source-code changes, or merge them together? This > would also mean that all DPDK developers would need to run checkpatch > inside of a python virtual environment since DTS is currently very specif= ic > about what versions are used (via both version number and cryptographic > hash of the source archive). These versions are newer than what are shipp= ed > in many stable linux distributions (Ubuntu, Debian, etc), because we want > the additional capabilities that come with the newer versions and the > version in the Debian Buster repos is old enough that it does not support > the version of python that we are using. We were trying to avoid forcing > all DPDK developers to install a full suite of python tools. So who is running the script? My wish is to make it automatically run when calling checkpatch.sh. > > devtools/python-format.sh | 54 +++ > > > devtools/python-lint.sh | 26 ++ > > > > Let's postpone the integration of these tools. > > We need to discuss what is specific to DTS or not. > > > > > doc/guides/contributing/coding_style.rst | 4 +- > > > > It is not specific to DTS. > > > > > dts/.devcontainer/devcontainer.json | 30 ++ > > > dts/Dockerfile | 39 ++ > > > > Not sure about Docker tied to some personal choices. >=20 > The reason we are using docker is that it provides a canonical environment > to run the test harness in, which can then connect to the SUT and traffic > generator. It enforces isolation between these three components, and > ensures that everyone is using the exact same environment to test and > deploy the test harness. Although devcontainer started in Visual Studio > Code, it is supported by many IDEs at this point and we wanted to encoura= ge > developers along a path which avoids needing to set up a development > environment before they can start working. The very recent version of > python (3.10) we choose to use for additional type system verification > makes not using containers much harder. It also means that new > dependencies, new dependency versions or additional system dependencies c= an > be easily handled by the normal patch process and rolled out to everyone = in > a mostly automated fashion. >=20 > Additionally, although it is named Dockerfile, it is fully compatible with > podman. In general we avoid enforcing specific versions in DPDK. The idea is to make it usable from any system with good compatibility. Using a container is more convenient in general, I agree. I just would like to have the choices here discussed specifically in a sepa= rate patch. > > > dts/README.md | 154 ++++++++ > > > > As said above, it should in RST format in doc/guides/dts/ > > > > > dts/conf.yaml | 6 + > > > dts/framework/__init__.py | 4 + > > > dts/framework/config/__init__.py | 100 +++++ > > > dts/framework/config/conf_yaml_schema.json | 65 ++++ > > > dts/framework/dts.py | 68 ++++ > > > dts/framework/exception.py | 57 +++ > > > dts/framework/logger.py | 114 ++++++ > > > dts/framework/remote_session/__init__.py | 15 + > > > .../remote_session/remote_session.py | 100 +++++ > > > dts/framework/remote_session/ssh_session.py | 185 +++++++++ > > > dts/framework/settings.py | 119 ++++++ > > > dts/framework/testbed_model/__init__.py | 8 + > > > dts/framework/testbed_model/node.py | 63 ++++ > > > dts/framework/utils.py | 31 ++ > > > dts/main.py | 24 ++ > > > dts/poetry.lock | 351 ++++++++++++++++= ++ > > > > A lot of dependencies look not useful in this first series for SSH > > connection. >=20 > I've put the actual dependency list below. The lock file contains the > entire dependency tree, of which Scapy is responsible for a substantial > portion. pexpect is currently used because of the large amount of code fr= om > the old dts that was moved over, but it will be replaced by a better > solution soon with a rewrite of the dts/framework/remote_session module. > Warlock is used to handle json schema checking, which is how we provide > instant feedback about an incorrect config file to users (as opposed to o= ld > DTS, where you would be informed by a stack trace at an arbitrary point in > the program). PyYAML is used to parse the yaml config file, and > types-PyYAML is used to provide information to the python type checker > about pyyaml. Scapy provides all of the packet manipulation capabilities = in > DTS, and I was not able to find a better library for doing packet > manipulation in python, so in my eyes we can excuse its dependency tree d= ue > to how much time it will save. >=20 > [tool.poetry.dependencies] > python =3D "^3.10" > pexpect =3D "^4.8.0" > warlock =3D "^2.0.1" > PyYAML =3D "^6.0" > types-PyYAML =3D "^6.0.8" > scapy =3D "^2.4.5" Scapy is not needed for SSH connection. Is warlock used in this series?