Re: [PATCH v6 00/10] dts: ssh connection to a node

[Owen Hilyard]

On Mon, Oct 31, 2022 at 3:01 PM Thomas Monjalon  wrote:

> 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š:
> > 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 document,
> > when the HelloWorld testcases is in (point 4 in our roadmap below). What
> > 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.
>

We wanted to specifically ensure that all code met the quality requirements
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 code.
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 specific
about what versions are used (via both version number and cryptographic
hash of the source archive). These versions are newer than what are shipped
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.

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

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 encourage
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 can
be easily handled by the normal patch process and rolled out to everyone in
a mostly automated fashion.

Additionally, although it is named Dockerfile, it is fully compatible with
podman.


> >  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_s

Re: [PATCH v3 00/10] dts: add hello world testcase

[Owen Hilyard]

Everything looks good to me with the exception of some issues I ran into
with terminal codes. Setting TERM=dumb before running fixed it, but we
might want to set that inside of DTS since I can't think of a reason why we
would need colors or any of the other "fancy" features of the vt220, and
setting everything to teletype mode should make our lives easier when
parsing output.

I think that later on it might make sense to have "CPU" be a device class
like NIC or Cryptodev, but that can be revisited once we get closer to
interacting with hardware.

On Tue, Jan 17, 2023 at 10:49 AM Juraj Linkeš 
wrote:

> Add code needed to run the HelloWorld testcase which just runs the hello
> world dpdk application.
>
> The patchset currently heavily refactors this original DTS code needed
> to run the testcase:
> * The whole architecture has been redone into more sensible class
>   hierarchy
> * DPDK build on the System under Test
> * DPDK eal args construction, app running and shutting down
> * SUT hugepage memory configuration
> * Test runner
> * Test results
> * TestSuite class
> * Test runner parts interfacing with TestSuite
> * The HelloWorld testsuite itself
>
> The code is divided into sub-packages, some of which are divided
> further.
>
> There patch may need to be divided into smaller chunks. If so, proposals
> on where exactly to split it would be very helpful.
>
> v3:
> Finished refactoring everything in this patch, with test suite and test
> results being the last parts.
> Also changed the directory structure. It's now simplified and the
> imports look much better.
> I've also many many minor changes such as renaming variables here and
> there.
>
> Juraj Linkeš (10):
>   dts: add node and os abstractions
>   dts: add ssh command verification
>   dts: add dpdk build on sut
>   dts: add dpdk execution handling
>   dts: add node memory setup
>   dts: add test suite module
>   dts: add hello world testplan
>   dts: add hello world testsuite
>   dts: add test suite config and runner
>   dts: add test results module
>
>  dts/conf.yaml |  19 +-
>  dts/framework/config/__init__.py  | 132 +++-
>  dts/framework/config/arch.py  |  57 
>  dts/framework/config/conf_yaml_schema.json| 150 -
>  dts/framework/dts.py  | 185 --
>  dts/framework/exception.py| 100 +-
>  dts/framework/logger.py   |  24 +-
>  dts/framework/remote_session/__init__.py  |  30 +-
>  dts/framework/remote_session/linux_session.py | 114 +++
>  dts/framework/remote_session/os_session.py| 177 ++
>  dts/framework/remote_session/posix_session.py | 221 
>  .../remote_session/remote/__init__.py |  16 +
>  .../remote_session/remote/remote_session.py   | 155 +
>  .../{ => remote}/ssh_session.py   |  91 -
>  .../remote_session/remote_session.py  |  95 --
>  dts/framework/settings.py |  79 -
>  dts/framework/test_result.py  | 316 ++
>  dts/framework/test_suite.py   | 254 ++
>  dts/framework/testbed_model/__init__.py   |  20 +-
>  dts/framework/testbed_model/dpdk.py   |  78 +
>  dts/framework/testbed_model/hw/__init__.py|  27 ++
>  dts/framework/testbed_model/hw/cpu.py | 253 ++
>  .../testbed_model/hw/virtual_device.py|  16 +
>  dts/framework/testbed_model/node.py   | 165 +++--
>  dts/framework/testbed_model/sut_node.py   | 261 +++
>  dts/framework/utils.py|  39 ++-
>  dts/test_plans/hello_world_test_plan.rst  |  68 
>  dts/tests/TestSuite_hello_world.py|  59 
>  28 files changed, 2998 insertions(+), 203 deletions(-)
>  create mode 100644 dts/framework/config/arch.py
>  create mode 100644 dts/framework/remote_session/linux_session.py
>  create mode 100644 dts/framework/remote_session/os_session.py
>  create mode 100644 dts/framework/remote_session/posix_session.py
>  create mode 100644 dts/framework/remote_session/remote/__init__.py
>  create mode 100644 dts/framework/remote_session/remote/remote_session.py
>  rename dts/framework/remote_session/{ => remote}/ssh_session.py (65%)
>  delete mode 100644 dts/framework/remote_session/remote_session.py
>  create mode 100644 dts/framework/test_result.py
>  create mode 100644 dts/framework/test_suite.py
>  create mode 100644 dts/framework/testbed_model/dpdk.py
>  create mode 100644 dts/framework/testbed_model/hw/__init__.py
>  create mode 100644 dts/framework/testbed_model/hw/cpu.py
>  create mode 100644 dts/framework/testbed_model/hw/virtual_device.py
>  create mode 100644 dts/framework/testbed_model/sut_node.py
>  create mode 100644 dts/test_plans/hello_world_test_plan.rst
>  create mode 100644 dts/tests/TestSuite_hello_world.py
>
> --
> 2.30.2
>
>

[dpdk-dev] dpdk-next-crypto/master is failing tests

[Owen Hilyard]

The Intel 10G and Intel 40G systems at the community lab had test jobs
begin to crash a few hours ago. This was the result of DTS crashing
severely enough that the job was unable to continue. A brief investigation
makes it seem like the "-a" flag for testpmd has been removed, which is
causing the timeout. Logs and the tarball which caused the issue are
attached.
 dpdk.tar.gz

 output.zip

DTS WG Meeting Minutes - 5/15

[Owen Hilyard]

This meeting had a lot of book-keeping. In previous meetings we've had to
skip reviewing some action items due to time constraints. This meeting we
had time to review most of them, so we have a larger-than-normal "Review
Action Items" section.


Attendees

   -

   Owen Hilyard
   -

   Juraj Linkes
   -

   Lijuan Tu
   -

   Anatoly Burakov
   -

   Bruce Richardson
   -

   John McNamara

Agenda

   -

   Additions to the agenda
   -

   Merging DTS into DPDK
   -

   Review/Carry forward pending action items

MinutesMerging DTS into DPDK - Current Status

   -

   Once the class renaming patch is merged, Juraj will respin the ‘hello
   world test case’ patch and send it to the DPDK community for review. Please
   note the pending config parser work in the cover letter.
   -

   Config parser change will be implemented only for the framework config
   files. But the changes will be in framework as well as test cases. The test
   case config files will not be changed for now. Target to complete the
   config parser changes in this week (assuming no other burning issues in UNH
   lab)

Documentation

   -

   The class renaming patch has made changes to the documentation as well
   -

   ‘Hello World’ patch requires documentation. The existing quick start
   guide would be sufficient for the community to be able to run this test
   case. The quick start guide can be added to this patch.


Review Action Items

   -

   Lijuan - Unit tests replaced with a meson runner using JUnit XML output.
   -

   Juraj - Review the DTS user guide to ensure the identified items in must
   have are incorporated. - Done
   -

   Lijuan - Submit a patch for TestSuite_efd.test_perf_efd_valuesize to use
   the -c_args option in meson configure command - Done
   -

   Lijuan - TestSuite_malicious_driver_event_indication - Understand if
   this test suite can be dropped. - Intel will use this
   -

   Lijuan - TestSuite_distributor - DPDK code is corrected and merged,
   remove the patch from DTS - Done
   -

   Lijuan - Unpack tcl tools (needed for IXIA), store in dep/,
   test and submit a patch. Place all of the data files into tests/data and
   make the necessary changes to DTS to support that. - A patch will be
   submitted to do this
   -

   Lijuan - Merge the main branch to future-dts branch - Done
   -

   Lijuan - Test the class renaming patch before merging it - Done

Action Items

   -

   Honnappa - Patches to DPDK to address DPDK modifications that DTS does.
   -

   Honnappa - Review the must have list from
   
https://docs.google.com/spreadsheets/d/1s_Y3Ph1sVptYs6YjOxkUI8rVzrPFGpTZzd75gkpgIXY/edit#gid=1128536548
   to ensure correct priority
   -

   Lijuan - To create the DTS test suite developer document.
   -

   Owen - Write a script to check if the formatters are throwing any errors.
   -

   Lijuan/Owen - Create a patch to move the framework/virt_*,
   framework/flow (and any other, Juraj’s RFC (v2, 00/23) has the list of
   these) and common files to tests/util directory, Lijuan will test this.
   -

   Owen/Honnappa - Communication on the future-dts branch to DTS community
   -

   Owen - Take a look at the ‘patch verification scripts’ patch to send it
   to the DPDK community
   -

   Owen - Add the config parser to the ‘hello world’ patch from Juraj
   -

   Owen - Write down information for the Intel Interns about how to look
   through the documentation and why them doing it is useful to DTS
   -

   All - Review Juraj’s email about the DTS user guide, the email is old,
   so changing the definitions should also be included.
   -

   Lijuan - Remove DPDK modifications from
   TestSuiteMaliciousDrvEventIndication
   -

   Lijuan - Try to find the source of dep/tgen.tgz:/tgen if it is an open
   source program, we should compile from source or require it to already be
   installed.

Any other business

   -

   Next Meeting: June 22, 2022, 12:00 pm UTC
   -

   Zoom link for meetings:
   
https://www.google.com/url?q=https://armltd.zoom.us/j/97503259680?pwd%3DVVlmWnlnTXJkVGkwR2JOU3R3b3Vndz09%26from%3Daddon&sa=D&source=calendar&ust=1655816095565517&usg=AOvVaw1-gkOorglbvhkHy2aljrvv

[RFC] DTS Commit Policies

[Owen Hilyard]

Hello Everyone,

The DTS Working Group wanted to let everyone give feedback on the proposed
DTS commit policies. Please leave a comment with any feedback you have. We
will be reviewing feedback in the DTS Working Group meeting on January
26th, at which point the comment period will close.

The purpose of the commit policies is to help standardize the code that
makes up DTS, as well as help new additions meet community requirements,
such as avoiding breaking changes, ensuring runtime stability, and aiding
debugability.

Owen Hilyard

DTS Working Group meeting room:
https://armltd.zoom.us/j/97503259680?pwd=VVlmWnlnTXJkVGkwR2JOU3R3b3Vndz09&from=addon

Re: [RFC] DTS Commit Policies

[Owen Hilyard]

Juraj,

I think that both of those make sense. We should try to add those into the
document next meeting.

Owen

Adding a link to the document here in case anyone doesn't want to go
through the meeting minutes:
https://docs.google.com/document/d/1G7_AEA-4MAd88bxjfP-IOcIy_6mnrMz3HCsUI8e-NN4/edit


On Mon, Jan 17, 2022 at 7:23 AM Juraj Linkeš 
wrote:

>
>
>
>
> *From:* Owen Hilyard 
> *Sent:* Wednesday, January 12, 2022 4:08 PM
> *To:* dev ; d...@dpdk.org
> *Subject:* [RFC] DTS Commit Policies
>
>
>
> Hello Everyone,
>
>
>
> The DTS Working Group wanted to let everyone give feedback on the proposed
> DTS commit policies. Please leave a comment with any feedback you have. We
> will be reviewing feedback in the DTS Working Group meeting on January
> 26th, at which point the comment period will close.
>
>
>
> The purpose of the commit policies is to help standardize the code that
> makes up DTS, as well as help new additions meet community requirements,
> such as avoiding breaking changes, ensuring runtime stability, and aiding
> debugability.
>
>
>
> Owen Hilyard
>
>
>
> DTS Working Group meeting room:
> https://armltd.zoom.us/j/97503259680?pwd=VVlmWnlnTXJkVGkwR2JOU3R3b3Vndz09&from=addon
>
>
>
> I always like to follow the practices captured in this article:
> https://cbea.ms/git-commit/
>
>
>
> I think it also makes sense to be as close as possible to DPDK, so trying
> to use what we can from their guidelines [0] would be a good start.
>
>
>
> Juraj
>
>
>
> [0]
> https://doc.dpdk.org/guides/contributing/patches.html#commit-messages-subject-line
>

[RFC] Python Version for DTS

[Owen Hilyard]

Hello Everyone,

DTS has operated using the version of python available in the oldest
supported Ubuntu LTS for a while now, but after some discussion in the
working group, we decided that DTS should designate a Python version
explicitly for the sake of compatibility. My initial proposal is Python
3.6, due to Ubuntu 18.04 LTS shipping with it by default. Currently, the
Community CI Lab runs Python 3.8 on all testers (Ubuntu 20.04 LTS), but for
the sake of compatibility a lower python version might be desirable.

The working group is specifically looking for reasons to not use Python 3.6.

The Community CI Lab perspective is that versions higher than 3.8 are not
acceptable due to the issues with changing the default Python installation
on Ubuntu. This may be raised to 3.10 in June of this year after we have
had time to evaluate the 22.04 release of Ubuntu.

Owen Hilyard

DTS WG Meeting Minutes 1/19/22

[Owen Hilyard]

Hello Everyone,

Sorry about being so late in sending these out.

This document will always be up to date:
https://docs.google.com/document/d/1E2mkTHNQ5vyln1JvnJTil15cjacUTXP7LXb9K960Vxs/edit#heading=h.6k5h0aafbuyy


Here are the minutes:

DTS Working Group Meeting Minutes for 2022
January 19, 2022Attendees

   -

   Owen Hilyard
   -

   Lincoln Lavoie
   -

   Lijuan Tu
   -

   Juraj Linkes
   -

   Honnappa Nagarahalli

Agenda

   -

   Review/Carry forward pending action items
   -

   Merging DTS into DPDK
   -

   Review additional work items

MinutesReview/Carry forward pending action items

   -

   Honnappa - Propose changes to DPDK to address changing the constants and
   printfs, refer to Lijuan’s email.
   -

  Work is started to add options for configuring the RX and TX queue
  sizes for L3fwd sample application
  -

  Looking to remove the examples/distributor from DPDK which will end
  up removing the corresponding test case from DTS.
  -

   Owen - Send out the DTS contributions guide to DTS and DPDK mailing
   list. 2 weeks of RFC followed by a patch.
   -

  Email is sent
  -

   Owen, Lijuan - Look for merging the makefile removal patch (
   http://patchwork.dpdk.org/project/dts/list/?series=20610)
   -

  No issues for CI
  -

  Add a depreciation warning for makefiles when run and in the release
  notes
  -

   Honnappa - Understand how the SPDX change was done for the DPDK repo
   -

  State of an automated solution is unknown
  -

  We are legally ok to change files with BSD licenses and files with no
  license to SPDX with BSD-3 clause
  -

   Honnappa - Talk to techboard about files with the GPL2 license in DTS.
   -

  Work in progress
  -

   Owen - Take a deeper look at the license and copyright assignments for
   the files.
   -

  Completed
  -

  If there is a possibility of issues, a file-by-file examination would
  be warranted
  -

   Juraj - Convert the makefile build for documentation to use meson build.
   -

  Juraj will look at this


DTS Contribution Guide

   -

   Integrate suggestions from Juraj

Merging DTS into DPDK

   -

   Try for 22.06
   -

   Focus on framework and test cases used in CI
   -

  pf_smoke
  -

  vf_smoke
  -

  virtio_smoke
  -

  nic_single_core_perf
  -

  mtu_update
  -

  stats_checks
  -

  dynamic_config
  -

  dynamic_queue
  -

  mac_filter
  -

  queue_start_stop
  -

  scatter
  -

  unit_tests_mbuf

Review additional work items

   -

Action Items

   -

   Honnappa - Propose changes to DPDK to address changing the constants and
   printfs, refer to Lijuan’s email.
   -

   All - Review Owen’s DTS contributions guide
   -

   Owen, Lijuan - Look for merging the makefile removal patch (
   http://patchwork.dpdk.org/project/dts/list/?series=20610)
   -

   Honnappa - Understand how the SPDX change was done for the DPDK repo
   -

   Honnappa - Talk to techboard about files with the GPL2 license in DTS.
   -

   Juraj - Convert the makefile build for documentation to use meson build.
   -

   Owen - RFC for making the python version 3.6
   -

   Owen - Look at making DTS build using the DPDK repo it is in.

Any other business

   -

   Next Meeting: January 26, 2022
   -

   Subject for next meeting: Focus on getting framework ready to move

Re: [RFC] Python Version for DTS

[Owen Hilyard]

After some discussion in the working group, we decided that this is mostly
a question of what the labs can support. The UNH Lab uses the version of
python that ships with the latest Ubuntu LTS, which is 3.8. The Intel Lab
is in a similar situation. Are there any objections to python 3.8 with
updates trailing Ubuntu LTS by a few months for stability reasons?

Re: [PATCH v1] drivers/net: use internal API to get eth dev from name

[Owen Hilyard]

The Community CI hasn't been able to schedule downtime to update dpdk-ci
across all of our systems since the refactoring Ali did. However, the patch
was still applied on next-net and had a compilation error. See
https://lab.dpdk.org/results/dashboard/patchsets/20895/. It was applied
onto
https://git.dpdk.org/next/dpdk-next-net/commit/?id=7445a787de053776616e41ab1d79090bd0f5ce33
.

Owen

Here's the build log in case you were having issues seeing it:

[1/1511] Compiling C object
drivers/librte_net_null.so.22.1.p/meson-generated_.._rte_net_null.pmd.c.o
[2/1511] Compiling C object
drivers/librte_net_octeontx.so.22.1.p/meson-generated_.._rte_net_octeontx.pmd.c.o
[3/1511] Compiling C object
drivers/librte_net_null.a.p/meson-generated_.._rte_net_null.pmd.c.o
[4/1511] Compiling C object
drivers/librte_net_octeontx_ep.so.22.1.p/meson-generated_.._rte_net_octeontx_ep.pmd.c.o
[5/1511] Compiling C object
drivers/librte_net_octeontx.a.p/meson-generated_.._rte_net_octeontx.pmd.c.o
[6/1511] Compiling C object
drivers/librte_net_pcap.so.22.1.p/meson-generated_.._rte_net_pcap.pmd.c.o
[7/1511] Compiling C object
drivers/librte_net_pfe.a.p/meson-generated_.._rte_net_pfe.pmd.c.o
[8/1511] Linking static target drivers/librte_net_null.a
[9/1511] Linking static target drivers/librte_net_octeontx.a
[10/1511] Compiling C object
drivers/net/qede/base/libqede_base.a.p/ecore_init_ops.c.o
[11/1511] Linking static target drivers/librte_net_pfe.a
[12/1511] Linking target drivers/librte_net_mlx5.so.22.1
[13/1511] Linking target drivers/librte_common_sfc_efx.so.22.1
[14/1511] Linking target drivers/librte_common_cnxk.so.22.1
[15/1511] Compiling C object drivers/net/qede/base/libqede_base.a.p/ecore_hw.c.o
[16/1511] Compiling C object
drivers/librte_net_octeontx_ep.a.p/meson-generated_.._rte_net_octeontx_ep.pmd.c.o
[17/1511] Compiling C object
drivers/net/qede/base/libqede_base.a.p/ecore_init_fw_funcs.c.o
[18/1511] Linking target drivers/librte_net_bond.so.22.1
FAILED: drivers/librte_net_bond.so.22.1
cc  -o drivers/librte_net_bond.so.22.1
drivers/librte_net_bond.so.22.1.p/meson-generated_.._rte_net_bond.pmd.c.o
drivers/libtmp_rte_net_bond.a.p/net_bonding_rte_eth_bond_8023ad.c.o
drivers/libtmp_rte_net_bond.a.p/net_bonding_rte_eth_bond_alb.c.o
drivers/libtmp_rte_net_bond.a.p/net_bonding_rte_eth_bond_api.c.o
drivers/libtmp_rte_net_bond.a.p/net_bonding_rte_eth_bond_args.c.o
drivers/libtmp_rte_net_bond.a.p/net_bonding_rte_eth_bond_flow.c.o
drivers/libtmp_rte_net_bond.a.p/net_bonding_rte_eth_bond_pmd.c.o
-Wl,--as-needed -Wl,--no-undefined -Wl,-O1 -shared -fPIC
-Wl,--start-group -Wl,-soname,librte_net_bond.so.22 -Wl,--no-as-needed
-pthread -lm -ldl -lnuma -lfdt '-Wl,-rpath,$ORIGIN/../lib:$ORIGIN/'
-Wl,-rpath-link,/home-local/jenkins-local/jenkins-agent/workspace/Apply-Custom-Patch-Set/dpdk/build/lib
-Wl,-rpath-link,/home-local/jenkins-local/jenkins-agent/workspace/Apply-Custom-Patch-Set/dpdk/build/drivers
lib/librte_ethdev.so.22.1 lib/librte_eal.so.22.1
lib/librte_kvargs.so.22.1 lib/librte_telemetry.so.22.1
lib/librte_net.so.22.1 lib/librte_mbuf.so.22.1
lib/librte_mempool.so.22.1 lib/librte_ring.so.22.1
lib/librte_meter.so.22.1 drivers/librte_bus_pci.so.22.1
lib/librte_pci.so.22.1 drivers/librte_bus_vdev.so.22.1
lib/librte_sched.so.22.1 lib/librte_ip_frag.so.22.1
lib/librte_hash.so.22.1 lib/librte_rcu.so.22.1 -Wl,--end-group
-Wl,--version-script=/home-local/jenkins-local/jenkins-agent/workspace/Apply-Custom-Patch-Set/dpdk/drivers/net/bonding/version.map
drivers/libtmp_rte_net_bond.a.p/net_bonding_rte_eth_bond_api.c.o: In
function `rte_eth_bond_create':
rte_eth_bond_api.c:(.text+0x12e6): undefined reference to
`rte_eth_dev_get_by_name'
collect2: error: ld returned 1 exit status
[19/1511] Linking target drivers/librte_net_ipn3ke.so.22.1
FAILED: drivers/librte_net_ipn3ke.so.22.1
cc  -o drivers/librte_net_ipn3ke.so.22.1
drivers/librte_net_ipn3ke.so.22.1.p/meson-generated_.._rte_net_ipn3ke.pmd.c.o
drivers/libtmp_rte_net_ipn3ke.a.p/net_ipn3ke_ipn3ke_ethdev.c.o
drivers/libtmp_rte_net_ipn3ke.a.p/net_ipn3ke_ipn3ke_flow.c.o
drivers/libtmp_rte_net_ipn3ke.a.p/net_ipn3ke_ipn3ke_representor.c.o
drivers/libtmp_rte_net_ipn3ke.a.p/net_ipn3ke_ipn3ke_tm.c.o
-Wl,--as-needed -Wl,--no-undefined -Wl,-O1 -shared -fPIC
-Wl,--start-group -Wl,-soname,librte_net_ipn3ke.so.22
-Wl,--no-as-needed -pthread -lm -ldl -lnuma -lfdt
'-Wl,-rpath,$ORIGIN/../lib:$ORIGIN/'
-Wl,-rpath-link,/home-local/jenkins-local/jenkins-agent/workspace/Apply-Custom-Patch-Set/dpdk/build/lib
-Wl,-rpath-link,/home-local/jenkins-local/jenkins-agent/workspace/Apply-Custom-Patch-Set/dpdk/build/drivers
lib/librte_ethdev.so.22.1 lib/librte_eal.so.22.1
lib/librte_kvargs.so.22.1 lib/librte_telemetry.so.22.1
lib/librte_net.so.22.1 lib/librte_mbuf.so.22.1
lib/librte_mempool.so.22.1 lib/librte_ring.so.22.1
lib/librte_meter.so.22.1 drivers/librte_bus_pci.so.22.1
lib/librte_pci.so.22.1 drivers/librte_bus_vdev.so.22.1
drivers/librte_bus_ifpga.so.22.1 lib/librte_rawdev.so.22.1
lib/librte_sched.so.22.1 -Wl,--end-group
-

[RFC] Testpmd Maintenance affecting DTS

[Owen Hilyard]

Currently, DTS tests DPDK via the example applications (/examples) and the
apps (/app) included with DPDK. The DTS Working group has been trying to
move to primarily using Testpmd, since it is the most complete and only
using one application causes less maintenance work. Since Testpmd provides
a text interface into DPDK, it must be continually updated to expose this
new functionality, sometimes it may lag behind the current feature set of
DPDK. For example, Testpmd currently does not support all of the options
available for rte flow, which limits the testing coverage that can be
provided by DTS for RTE flow. Because DTS is a Python application, it is
not able to directly interface with DPDK. This means that whenever Testpmd
falls behind DPDK, those unexposed parts of DPDK are functionally
untestable as far as DTS is concerned. DTS is the tool used by the CI and
labs to functionally test DPDK in a working configuration, compared to the
unit testing, that may test individual components or parts of the stack.
The DTS improvements WG and the Community Lab have a goal to continue the
expansion of the test functional and performance test coverage of DPDK.
This means that there must be some way for everything that the DPDK
community wants to be tested to be exposed to DTS. I have a few possible
solutions to this problem:


   1.

   Continue to update Testpmd every time new functionality is added to DPDK
   to expose that functionality
   2.

   Use a parser generator or some other method to make adding new options
   to Testpmd much easier, updating this every time new functionality is added
   to testpmd
   3.

   Create a dedicated testing application for DPDK that uses a binding
   generator and Python’s XMLRPC to allow more programmatic access to DPDK on
   the DUT, adding new RPC calls to expose new functionality.


I personally think that option 3 is the best because it would involve a bit
of work up front to make it much easier to expose parts of DPDK to DTS.
This is because after the initial work is done, we would just need to write
a wrapper function in C to expose the functionality we want, and then
expose the wrapper function via RPC.

If you have thoughts or suggestions, we will be discussing this at the DTS
working group meeting.

2:00 PM UTC, Wednesday, February 16
https://armltd.zoom.us/j/97503259680?pwd=VVlmWnlnTXJkVGkwR2JOU3R3b3Vndz09&from=addon

Re: [RFC] Testpmd Maintenance affecting DTS

[Owen Hilyard]

>
> Yes, any feature in ethdev should have a testpmd entry.
> Which part of rte_flow is not testable with testpmd?


> In general, any device API should be testable with the app/ directory.


 There are currently a lot of fields that appear in the documentation for
rte_flow that are not exposed in testpmd. For example, 5/9 fields
in ARP_ETH_IPV4 are not exposed via testpmd. I could compile a full list if
you want me to.

Please could you elaborate how [a parser generator] would work?


 Instead of the current approach of hand-rolling a parser, something like
Yacc (Yet Another Compiler Compiler) or Bison (GNU Yacc) could be used to
make adding new items much easier. Although they are both designed to aid
in creating compilers, they also work very well as parsers which output a
parsed structure. When I used Bison in the past to write an AWK
implementation, what I did is have it return a struct. In the case of
testpmd, an array of a tagged union might be easier to handle, so you'd get
something like this back:
[
  {
type: ITEM_ETH,
inner: {
  eth: {
dst_is_present: true,
dst: "00:00:5e:00:53:af",
src_is_present: true,
src: "00:00:5e:00:53:af",
type_is_present: false,
type: 0
  }
}
  },
  {
type: ITEM_IPV4,
inner: {
  ipv4: {
dst_is_present: true,
dst: "127.0.0.1",
src_is_present: true,
src: "127.0.0.1",
  }
}
  }
]

Once you have the tagged union and the inner structs set up, adding a new
field or item should be fairly easy. Also, the _is_present could be
replaced with specified "not present" values (NULL for strings, etc), this
is just emulating the Option/Optional type from higher-level languages more
directly. The process for adding a new field would involve first adding it
to the bison grammar, then adding the field to the output struct.

Why do you need RPC when a binding is generated?


I guess that this is two ideas. First, have C expose an XMLRPC api. How
hard this would be to do would depend greatly on the availability and
quality of libraries, and it would add a fairly large dependency to this
proposed app. The other idea is to generate Python bindings and then use
the RPC server built into Python. This might make adding the RPC layer
easier, and it would mean that we could more easily send Python types to
the DUT. I think that the second one might be easier to work with, if a bit
slower.

performance is not good


All of the performance-sensitive things, like packet generation, would
still be happening entirely in C code. Also, the last time I profiled DTS
it spent most of its time inside of the SSH library, so moving to Python
RPC API may still increase performance, although not as much as a C RPC
API. A good chunk of the rest of the time was spent either in timed
performance tests (nic_single_core_perf) or waiting for testpmd to start up.


> it is testing a binding, not the real API


Python was built to be glue code for C libraries, so it has a very mature
ecosystem in that area. If we use well-tested binding generators, then most
of the underlying behavior would still be DPDK. As is, DTS doesn't really
test DPDK directly, it tests testpmd, and assumes that testpmd interacts
with DPDK correctly. Making a Python RPC API or Python Bindings would mean
moving from a human-oriented API wrapper to a machine-oriented one.


> it is a huge maintenance effort for everybody


I am not advocating hand-writing Python bindings, that would be way too
much work. I am advocating using a binding generator like SWIG to consume
high-level DPDK header files and create python modules from them. Once this
is set up, it should only need occasional maintenance due to build system
changes. It might still require some work to plug new stuff into the RPC
framework, but that would probably be about the same amount of work as
adding the features to testpmd.

I am also open to using languages other than Python, it's just that C
strikes me as a bad choice for building something like this. It's possible
that we could make use of gRPC or something similar that is designed to
perform cross-language RPC easily, in which case directly exposing RPC in C
might be much easier.

Owen Hilyard

Re: [PATCH v1] drivers/net: use internal API to get eth dev from name

[Owen Hilyard]

The latest script in CI correctly selects 'next-net' for this patch.

>

Re: [dpdk-dev] [dpdk-ci] [PATCH v12 01/10] eal: add basic threading functions

[Owen Hilyard]

It seems like meson encountered an error when building

app/test/meson.build:472:11: ERROR: Index 1 out of bounds of array of size
> 1.
>
> A full log can be found at
> /home-local/jenkins-local/jenkins-agent/workspace/Apply-Custom-Patch-Set/dpdk/build/meson-logs/meson-log.txt
> ninja: error: loading 'build.ninja': No such file or directory
>

I can also reproduce the issue building locally (Meson version: 0.58.1):
1. Get the DPDK main branch (f12b844b54f4ea7908ecb08ade04c7366ede031d)
2. apply all patches
3. meson $BUILD_DIR

Meson doesn't really give any extra information aside from that, but
looking at the build file, it looks like one of the fast_tests has no
arguments and at least one is expected.

On Mon, Aug 2, 2021 at 5:37 PM Dmitry Kozlyuk 
wrote:

> + c...@dpdk.org
>
> 2021-08-02 14:08 (UTC-0700), Narcisa Ana Maria Vasile:
> > On Mon, Aug 02, 2021 at 10:32:17AM -0700, Narcisa Ana Maria Vasile wrote:
> > > From: Narcisa Vasile 
> > >
> > > Use a portable, type-safe representation for the thread identifier.
> > > Add functions for comparing thread ids and obtaining the thread id
> > > for the current thread.
> > >
> > > Signed-off-by: Narcisa Vasile 
> > > ---
> > >  lib/eal/common/meson.build|  1 +
> > >  lib/eal/{unix => common}/rte_thread.c | 57 ---
> > >  lib/eal/include/rte_thread.h  | 48 +-
> > >  lib/eal/unix/meson.build  |  1 -
> > >  lib/eal/version.map   |  3 ++
> > >  lib/eal/windows/rte_thread.c  | 17 
> > >  6 files changed, 95 insertions(+), 32 deletions(-)
> > >  rename lib/eal/{unix => common}/rte_thread.c (66%)
> > >
> > > diff --git a/lib/eal/common/meson.build b/lib/eal/common/meson.build
> >
> > Hello,
> >
> > I see the following error on this patch:
> >
> > ninja: error: loading 'build.ninja': No such file or directory
> >
> > https://lab.dpdk.org/results/dashboard/patchsets/18090/
> >
> > Locally, the build succeedes.
> > How can I see more information about this build error?
>

Re: [dpdk-dev] [dpdk-ci] [PATCH v12 01/10] eal: add basic threading functions

[Owen Hilyard]

Our windows servers are both running 0.57.1, but all of the *nix hosts are
running 0.58.1. This issue also happens on 0.57.1 and 0.57.2, with the
exact same steps to reproduce.

On Tue, Aug 3, 2021 at 11:38 AM Dmitry Kozlyuk 
wrote:

> 2021-08-03 11:11 (UTC-0400), Owen Hilyard:
> > It seems like meson encountered an error when building
> >
> > app/test/meson.build:472:11: ERROR: Index 1 out of bounds of array of
> size
> > > 1.
> > >
> > > A full log can be found at
> > >
> /home-local/jenkins-local/jenkins-agent/workspace/Apply-Custom-Patch-Set/dpdk/build/meson-logs/meson-log.txt
> > > ninja: error: loading 'build.ninja': No such file or directory
> > >
> >
> > I can also reproduce the issue building locally (Meson version: 0.58.1):
>
> Meson 0.58+ has a known issue on Windows:
> https://github.com/mesonbuild/meson/issues/8981
> The last known good version is 0.57.2.
>
> > 1. Get the DPDK main branch (f12b844b54f4ea7908ecb08ade04c7366ede031d)
> > 2. apply all patches
> > 3. meson $BUILD_DIR
> >
> > Meson doesn't really give any extra information aside from that, but
> > looking at the build file, it looks like one of the fast_tests has no
> > arguments and at least one is expected.
> >
> > On Mon, Aug 2, 2021 at 5:37 PM Dmitry Kozlyuk 
> > wrote:
> >
> > > + c...@dpdk.org
> > >
> > > 2021-08-02 14:08 (UTC-0700), Narcisa Ana Maria Vasile:
> > > > On Mon, Aug 02, 2021 at 10:32:17AM -0700, Narcisa Ana Maria Vasile
> wrote:
> > > > > From: Narcisa Vasile 
> > > > >
> > > > > Use a portable, type-safe representation for the thread identifier.
> > > > > Add functions for comparing thread ids and obtaining the thread id
> > > > > for the current thread.
> > > > >
> > > > > Signed-off-by: Narcisa Vasile 
> > > > > ---
> > > > >  lib/eal/common/meson.build|  1 +
> > > > >  lib/eal/{unix => common}/rte_thread.c | 57
> ---
> > > > >  lib/eal/include/rte_thread.h  | 48 +-
> > > > >  lib/eal/unix/meson.build  |  1 -
> > > > >  lib/eal/version.map   |  3 ++
> > > > >  lib/eal/windows/rte_thread.c  | 17 
> > > > >  6 files changed, 95 insertions(+), 32 deletions(-)
> > > > >  rename lib/eal/{unix => common}/rte_thread.c (66%)
> > > > >
> > > > > diff --git a/lib/eal/common/meson.build
> b/lib/eal/common/meson.build
> > > >
> > > > Hello,
> > > >
> > > > I see the following error on this patch:
> > > >
> > > > ninja: error: loading 'build.ninja': No such file or directory
> > > >
> > > > https://lab.dpdk.org/results/dashboard/patchsets/18090/
> > > >
> > > > Locally, the build succeedes.
> > > > How can I see more information about this build error?
> > >
>
>

Re: [PATCH v1] Remove Owen Hilyard as a DTS Maintainer

[Owen Hilyard]

On Tue, Feb 7, 2023 at 10:53 AM Thomas Monjalon  wrote:

> 07/02/2023 15:42, ohily...@iol.unh.edu:
> > From: Owen Hilyard 
> >
> > I (Owen Hilyard) am stepping down as a DTS maintainer.
>
> Thanks for your work Owen, you will be missed.
> Hope we'll see you again in future.
>
>
Thank you for the kind words Thomas, I hope to be back some day.

Re: [PATCH v4 1/9] dts: add project tools config

[Owen Hilyard]

> E203 - whitespace before ‘,’, ‘;’, or ‘:’
> E266 - too many leading ‘#’ for block comment
> E501 - line too long
> E731 - do not assign a lambda expression, use a def
> C0111 - Missing %s docstring
> F0401 - Unable to import %s

E203, E266 and E501 were disabled due to pylama fighting with the
autoformatters, so I decided to let the autoformatters win. I think
that C0111 was suppressed because this set of suppressions was from
mainline DTS and that has a lot of functions without documentation. F0401
is disabled due to dependencies on TRex vendored python libraries,
since those will not be possible to import inside of the container. I don't
remember why E731 is set, but it may be due to the rte flow rule generator
I wrote for mainline DTS, which makes use of lambdas extensively to enable
lazy evaluation, so that DTS doesn't need to keep ~2 billion rules in
memory.



On Fri, Sep 9, 2022 at 10:13 AM Juraj Linkeš 
wrote:

>
>
> > -Original Message-
> > From: Bruce Richardson 
> > Sent: Friday, September 9, 2022 3:53 PM
> > To: Juraj Linkeš 
> > Cc: tho...@monjalon.net; david.march...@redhat.com;
> > ronan.rand...@intel.com; honnappa.nagaraha...@arm.com;
> > ohily...@iol.unh.edu; lijuan...@intel.com; dev@dpdk.org
> > Subject: Re: [PATCH v4 1/9] dts: add project tools config
> >
> > On Fri, Sep 09, 2022 at 01:38:33PM +, Juraj Linkeš wrote:
> > >
> > >
> > > > -Original Message-
> > > > From: Bruce Richardson 
> > > > Sent: Wednesday, September 7, 2022 6:17 PM
> > > > To: Juraj Linkeš 
> > > > Cc: tho...@monjalon.net; david.march...@redhat.com;
> > > > ronan.rand...@intel.com; honnappa.nagaraha...@arm.com;
> > > > ohily...@iol.unh.edu; lijuan...@intel.com; dev@dpdk.org
> > > > Subject: Re: [PATCH v4 1/9] dts: add project tools config
> > > >
> > > > On Fri, Jul 29, 2022 at 10:55:42AM +, Juraj Linkeš wrote:
> > > > > .gitignore contains standard Python-related files.
> > > > >
> > > > > Apart from that, add configuration for Python tools used in DTS:
> > > > > Poetry, dependency and package manager Black, formatter Pylama,
> > > > > static analysis Isort, import sorting
> > > > >
> > > > > .editorconfig modifies the line length to 88, which is the default
> > > > > Black uses. It seems to be the best of all worlds. [0]
> > > > >
> > > > > [0]
> > > > > https://black.readthedocs.io/en/stable/the_black_code_style/curren
> > > > > t_st
> > > > > yle.html#line-length
> > > > >
> > > > > Signed-off-by: Owen Hilyard 
> > > > > Signed-off-by: Juraj Linkeš 
> > > >
> > > > Thanks for the work on this. Some review comments inline below.
> > > >
> > > > /Bruce
> > > >
> > > > > ---
> > > > >  dts/.editorconfig  |   7 +
> > > > >  dts/.gitignore |  14 ++
> > > > >  dts/README.md  |  15 ++
> > > > >  dts/poetry.lock| 474
> > > > +
> > > > >  dts/pylama.ini |   8 +
> > > > >  dts/pyproject.toml |  43 
> > > > >  6 files changed, 561 insertions(+)  create mode 100644
> > > > > dts/.editorconfig  create mode 100644 dts/.gitignore  create mode
> > > > > 100644 dts/README.md  create mode 100644 dts/poetry.lock  create
> > > > > mode 100644 dts/pylama.ini  create mode 100644 dts/pyproject.toml
> > > > >
> > > > > diff --git a/dts/.editorconfig b/dts/.editorconfig new file mode
> > > > > 100644 index 00..657f959030
> > > > > --- /dev/null
> > > > > +++ b/dts/.editorconfig
> > > > > @@ -0,0 +1,7 @@
> > > > > +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2022
> > > > > +PANTHEON.tech s.r.o.
> > > > > +# See https://editorconfig.org/ for syntax reference.
> > > > > +#
> > > > > +
> > > > > +[*.py]
> > > > > +max_line_length = 88
> > > >
> > > > It seems strange to have two different editorconfig settings in
> > > > DPDK. Is there a reason that:
> > > > a) we can't use 79, the current DPDK default and recommended length
> by
> > > >pycodestyle? Or alternatively:
> > > > b) change all of DPDK to use the 88 setting?
> > > >
> > > > 

Re: [PATCH v4 4/9] dts: add ssh pexpect library

[Owen Hilyard]

>
> On Fri, Jul 29, 2022 at 10:55:45AM +, Juraj Linkeš wrote:
> 
> > +self.session = pxssh.pxssh(encoding="utf-8")
> > +self.session.login(
> > +self.node,
> > +self.username,
> > +self.password,
> > +original_prompt="[$#>]",
> > +password_regex=r"(?i)(?:password:)|(?:passphrase
> for key)|(?i)(password for .+:)",
> > +)
> > +self.logger.info(f"Connection to {self.node}
> succeeded")
> > +self.send_expect("stty -echo", "#")
> > +self.send_expect("stty columns 1000", "#")
> First of all, thanks for those changes! Having DTS inside DPDK makes
> test synchronization a lot easier. I'm happy to say (unsurprisingly)
> that it works with my RISC-V HiFive Unmatched board like a charm.


> Though there is a small issue with the lines above. They assume "#" as
> the prompt sign, even though original_prompt was set to "[$#>]". This
> touches on two problems:
> 1. # is usually a root prompt - is DTS assumed to be run with root
>privileges? DPDK may (in theory) run without them with some permission
>adjustment (hugetlb, VFIO container, etc.). If we assume DTS needs
>root access, this has to be both documented and validated before
>running the whole suite. Otherwise it'll be hard to debug.
>

Around a year ago there were some attempts to get DTS to not require root.
This ended up running into issues because DTS sets up drivers for you,
which requires root as far as I know, as well as setting up hugepages,
which I think also requires root. The current version of DTS can probably
run without root, but it will probably stop working as soon as DTS starts
interacting with PCI devices. Elevating privileges using pkexec or sudo is
less portable and would require supporting a lot more forms of
authentication (kerberos/ldap for enterprise deployments, passwords, 2fa,
etc). It is much easier to say that the default SSH agent must provide root
access to the SUT and Traffic Generator either with a password or
pre-configured passwordless authentication (ssh keys, kerberos, etc).

I agree it should be documented. I honestly didn't consider that anyone
would try running DTS as a non-root user.


> 2. Different shells use different prompts on different distros. Hence
>perhaps there should be a regex here (same as with original_prompt)
>and there could be a conf.yaml option to modify it on a per-host
>basis?


As far as customizing the prompts, I think that is doable via a
configuration option.

As far as different shells, I don't think we were planning to support
anything besides either bash or posix-compatible shells. At the moment all
of the community lab systems use bash, and for ease of test development it
will be easier to mandate that everyone uses one shell. Otherwise DTS CI
will need to run once for each shell to catch issues, which in my opinion
are resources better spent on more in-depth testing of DTS and DPDK.

Re: [PATCH v4 5/9] dts: add ssh connection extension

[Owen Hilyard]

>
> On Fri, Jul 29, 2022 at 10:55:46AM +, Juraj Linkeš wrote:
> > The class adds logging and history records to existing pexpect methods.
> >
> > Signed-off-by: Owen Hilyard 
> > Signed-off-by: Juraj Linkeš 
> > ---
> >  dts/framework/ssh_connection.py | 70 +
> >  1 file changed, 70 insertions(+)
> >  create mode 100644 dts/framework/ssh_connection.py
> >


> One comment inline below.


> /Bruce

Two questions on this function:

* Is the getattr() check not equivalent to "if self.logger:"?


It is. I missed it when looking over this code. I know that this close
function can run in a context where it loses the ability to make system
calls (an exit hook), but that doesn't matter for this as far as I know.


> * Why the check for a non-none logger in this function, when other

  functions above always seem to call the logger directly without checking?


"close" can be called before "init_log" if the program crashes early
enough, so this is avoiding calling a function on a null object. No other
function can have that issue because by the time control is returned to the
user the logger is properly initalized. This is especially important
because an early failure in the community lab will only be able to use logs
to figure out what happened.


>
> > +
> > +self.session.close(force)
> > --

> 2.30.2
> >

Re: [PATCH v4 6/9] dts: add config parser module

[Owen Hilyard]

> > +# Frozen makes the object immutable. This enables further optimizations,
> > +# and makes it thread safe should we every want to move in that
> direction.
> > +@dataclass(slots=True, frozen=True)
> > +class NodeConfiguration:
> > +name: str
> > +hostname: str
> > +user: str
> > +password: Optional[str]
> > +
> > +@staticmethod
> > +def from_dict(d: dict) -> "NodeConfiguration":
> > +return NodeConfiguration(
> > +name=d["name"],
> > +hostname=d["hostname"],
> > +user=d["user"],
> > +password=d.get("password"),
> > +)
> > +
> Out of curiosity, what is the reason for having a static "from_dict" method
> rather than just a regular constructor function that takes a dict as
> parameter?
>

@dataclass(...) is a class annotation that transforms the thing it
annotates into a dataclass. This means it creates the constructor for you
based on the property type annotations. If you create your own constructor,
you need a constructor that can either take a single dictionary or all of
the parameters like a normal constructor. Making it a static method also
means that each class can manage how it should be constructed from a
dictionary. Some of the other classes will transform lists or perform other
assertions. It also makes it easier to have specialized types. For
instance, a NICConfiguration class would have to handle all of the possible
device arguments that could be passed to any PMD driver if things were
passed as parameters.


> > +
> > +@dataclass(slots=True, frozen=True)
> > +class ExecutionConfiguration:
> > +system_under_test: NodeConfiguration
> > +
> Minor comment: seems strange having only a single member variable in this
> class, effectively duplicating the class above.
>

More is intended to go here. For instance, what tests to run, configuration
for virtual machines, the traffic generator node.



> > +@staticmethod
> > +def from_dict(d: dict, node_map: dict) -> "ExecutionConfiguration":
> from reading the code it appears that node_map is a dict of
> NodeConfiguration objects, right? Might be worth adding that to the
> definition for clarity, and also the specific type of the dict "d" (if it
> has one)
> > +sut_name = d["system_under_test"]
> > +assert sut_name in node_map, f"Unknown SUT {sut_name} in
> execution {d}"
> > +
> > +return ExecutionConfiguration(
> > +system_under_test=node_map[sut_name],
> > +)
> > +
> > +
> > +@dataclass(slots=True, frozen=True)
> > +class Configuration:
> > +executions: list[ExecutionConfiguration]
> > +
> > +@staticmethod
> > +def from_dict(d: dict) -> "Configuration":
> > +nodes: list[NodeConfiguration] = list(
> > +map(NodeConfiguration.from_dict, d["nodes"])
> So "d" is a dict of dicts?
>

d is a dictionary which matches the json schema for the class. In the case
of the Configuration class, it is a dictionary matching the entire json
schema.


> > +)
> > +assert len(nodes) > 0, "There must be a node to test"
> > +
> > +node_map = {node.name: node for node in nodes}
> > +assert len(nodes) == len(node_map), "Duplicate node names are
> not allowed"
> > +
> > +executions: list[ExecutionConfiguration] = list(
> > +map(
> > +ExecutionConfiguration.from_dict, d["executions"],
> [node_map for _ in d]
> > +)
> > +)
> > +
> > +return Configuration(executions=executions)
> > +
> > +
> > +def load_config() -> Configuration:
> > +"""
> > +Loads the configuration file and the configuration file schema,
> > +validates the configuration file, and creates a configuration
> object.
> > +"""
> > +with open(SETTINGS.config_file_path, "r") as f:
> > +config_data = yaml.safe_load(f)
> > +
> > +schema_path = os.path.join(
> > +pathlib.Path(__file__).parent.resolve(), "conf_yaml_schema.json"
> > +)
> > +
> > +with open(schema_path, "r") as f:
> > +schema = json.load(f)
> > +config: dict[str, Any] = warlock.model_factory(schema,
> name="_Config")(config_data)
> > +config_obj: Configuration = Configuration.from_dict(dict(config))
> > +return config_obj
> > +
> > +
> > +CONFIGURATION = load_config()
> 

Re: [PATCH v4 5/9] dts: add ssh connection extension

[Owen Hilyard]

>
> On Wed, Sep 14, 2022 at 3:46 AM Bruce Richardson <
> bruce.richard...@intel.com> wrote:
> >  On Fri, Jul 29, 2022 at 10:55:46AM +, Juraj Linkeš wrote:
> >  > The class adds logging and history records to existing pexpect
> >  methods.
> >  >
> >  > Signed-off-by: Owen Hilyard <[1]ohily...@iol.unh.edu>
> >  > Signed-off-by: Juraj Linkeš 
> >  > ---
> >  >  dts/framework/ssh_connection.py | 70
> >  +
> >  >  1 file changed, 70 insertions(+)
> >  >  create mode 100644 dts/framework/ssh_connection.py
> >  >
> >
> >  One comment inline below.
> >
> >  /Bruce
> >
> >  Two questions on this function:
> >
> >  * Is the getattr() check not equivalent to "if self.logger:"?
> >
> >It is. I missed it when looking over this code. I know that this close
> >function can run in a context where it loses the ability to make
> system
> >calls (an exit hook), but that doesn't matter for this as far as I
> >know.
> >
> >  * Why the check for a non-none logger in this function, when other
> >
> >functions above always seem to call the logger directly without
> >  checking?
> >
> >"close" can be called before "init_log" if the program crashes early
> >enough, so this is avoiding calling a function on a null object. No
> >other function can have that issue because by the time control is
> >returned to the user the logger is properly initalized. This is
> >especially important because an early failure in the community lab
> will
> >only be able to use logs to figure out what happened.
> >
> I'm a little confused by that explanation - can you perhaps clarify? This
> close function in part of an class, and the logger member is assigned its
> value
> in the constructor for that class, so how is it possible to call
> obj.close() before obj has been constructed?


Due to how we are forced to add the hooks to flush logger buffers to disk
before shutdown, even in the case of failures or SIGTERM, close can run
WHILE the constructor is in the middle of executing. All of the resources
except for the logger can be freed by python, but the logger requires
special handling, so we check if it is null and then flush the buffers to
disk if it is not. The actual logger object is only assigned to self.logger
after it is completely initalized, so if it's not null, then it is in a
good state and can be safely flushed. Here's a sequence of events that
would lead to that check being needed:

1. Start constructing logging handle
2. Execute first line of the constructor
3. SIGTERM

self.logger == None, since the entire world stops and moves to the cleanup
functions registered with the interpreter. If we don't do this check, then
the cleanup context crashes in this case.

Re: [PATCH v4 4/9] dts: add ssh pexpect library

[Owen Hilyard]

On Wed, Sep 14, 2022 at 3:57 PM Honnappa Nagarahalli <
honnappa.nagaraha...@arm.com> wrote:

> 
>
> > >
> > >  On Fri, Jul 29, 2022 at 10:55:45AM +, Juraj Linkeš wrote:
> > >  
> > >  > +self.session = pxssh.pxssh(encoding="utf-8")
> > >  > +self.session.login(
> > >  > +self.node,
> > >  > +self.username,
> > >  > +self.password,
> > >  > +original_prompt="[$#>]",
> > >  > +
> > >  password_regex=r"(?i)(?:password:)|(?:passphrase for
> > >  key)|(?i)(password for .+:)",
> > >  > +)
> > >  > +[1]self.logger.info(f"Connection to
> {self.node}
> > >  succeeded")
> > >  > +self.send_expect("stty -echo", "#")
> > >  > +self.send_expect("stty columns 1000", "#")
> > >  First of all, thanks for those changes! Having DTS inside DPDK
> makes
> > >  test synchronization a lot easier. I'm happy to say
> (unsurprisingly)
> > >  that it works with my RISC-V HiFive Unmatched board like a charm.
> > >
> > >
> > >  Though there is a small issue with the lines above. They assume
> "#"
> > >  as
> > >  the prompt sign, even though original_prompt was set to "[$#>]".
> > >  This
> > >  touches on two problems:
> > >  1. # is usually a root prompt - is DTS assumed to be run with root
> > > privileges? DPDK may (in theory) run without them with some
> > >  permission
> > > adjustment (hugetlb, VFIO container, etc.). If we assume DTS
> > >  needs
> > > root access, this has to be both documented and validated
> before
> > > running the whole suite. Otherwise it'll be hard to debug.
> > >
> > >
> > >Around a year ago there were some attempts to get DTS to not require
> > >root. This ended up running into issues because DTS sets up drivers
> for
> > >you, which requires root as far as I know, as well as setting up
> > >hugepages, which I think also requires root. The current version of
> DTS
> > >can probably run without root, but it will probably stop working as
> > >soon as DTS starts interacting with PCI devices. Elevating
> privileges
> > >using pkexec or sudo is less portable and would require supporting a
> > >lot more forms of authentication (kerberos/ldap for enterprise
> > >deployments, passwords, 2fa, etc). It is much easier to say that the
> > >default SSH agent must provide root access to the SUT and Traffic
> > >Generator either with a password or pre-configured passwordless
> > >authentication (ssh keys, kerberos, etc).
> > >
> > >[Honnappa] One of the feedback we collected asks to deprecate the
> use
> > >of clear text passwords in config files and root user. It suggests
> to
> > >use keys and sudo. It is a ‘Must Have’ item.
> > >
> > >
> > >I agree it should be documented. I honestly didn't consider that
> anyone
> > >would try running DTS as a non-root user.
> > >
> > >[Honnappa] +1 for supporting root users for now and documenting.
> > >
> > >
> > >  2. Different shells use different prompts on different distros.
> > >  Hence
> > > perhaps there should be a regex here (same as with
> > >  original_prompt)
> > > and there could be a conf.yaml option to modify it on a
> per-host
> > > basis?
> > >
> > >
> > >As far as customizing the prompts, I think that is doable via a
> > >configuration option.
> > >As far as different shells, I don't think we were planning to
> support
> > >anything besides either bash or posix-compatible shells. At the
> moment
> > >all of the community lab systems use bash, and for ease of test
> > >development it will be easier to mandate that everyone uses one
> shell.
> > >Otherwise DTS CI will need to run once for each shell to catch
> issues,
> > >which in my opinion are resources better spent on more in-depth
> testing
> > >of DTS and DPDK.
> > >
> > >[Honnappa] +1 for using just bash, we can document this as well.
> > >
> >
> > I would agree overall. Just supporting one shell is fine - certainly for
> now. Also
> > completely agree that we need to remove hard-coded passwords and ideally
> > non-root. However, I think for the initial versions the main thing
> should be
> > removing the passwords so I would be ok for keeping the "root"
> > login requirement, so long as we support using ssh keys for login rather
> than
> > hard-coded passwords.
> I would be for dropping support for the hard-coded passwords completely.
> Setting up the password-less SSH is straightforward (not sure if you meant
> the same).
>
> >
> > /Bruce
>

I think the question is whether there are any platforms/devices that should
be tested by DTS that do not support passwordless SSH.  Right now, the
community lab is using SSH keys for everything. If Intel also does

Re: [PATCH v5 00/10] dts: ssh connection to a node

[Owen Hilyard]

On Mon, Sep 26, 2022 at 10:17 AM Juraj Linkeš 
wrote:

> 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.
>
> To run the code, modify the config file, conf.yaml and execute ./main.py
> from the root dts folder. Here's an example config:
> executions:
>   - system_under_test: "SUT 1"
> nodes:
>   - name: "SUT 1"
> hostname: 127.0.0.1
> user: root
> password: mypw.change.me
>
> There are configuration files with a README that help with setting up
> the execution/development environment.
>
> 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 document,
> when the HelloWorld testcases is in (point 4 in our roadmap below). What
> will be documented later is runtime dependencies and how to set up the DTS
> control node environment.
>
> This is our current roadmap:
> 1. Review this patchset and do the rest of the items in parallel, if
> possible.
> 2. We have extracted the code needed to run the most basic testcase,
> HelloWorld, which runs the DPDK Hello World application. We'll split
> this along logical/functional boundaries and send after 1 is done.
> 3. Once we have 2 applied, we're planning on adding a basic functional
> testcase - pf_smoke. This send a bit of traffic, so the big addition is
> the software traffic generator, Scapy. There's some work already done on
> Traffic generators we'll be sending as a dependence on this patch
> series.
> 4. After 3, we'll add a basic performance testcase which doesn't use
> Scapy, but Trex or Ixia instead.
> 5. This is far in the future, but at this point we should have all of
> the core functionality in place. What then remains is adding the rest of
> the testcases.
>
> We're already working on items 2-4 and we may send more patches even
> before this patch series is accepted if that's beneficial. The new
> patches would then depend on this patch.
>
> This patch, as well as all others in the pipeline, are the result of
> extensive DTS workgroup review which happens internally. If you'd like
> us to make it more public we'd have no problem with that.
>
> v3:
> Added project config files and developer tools.
> Removed locks for parallel nodes, which are not needed now and will be
> implemented much later (in a different patch).
>
> v4:
> Minor fixes - added missing Exception and utils function.
>
> v5:
> Reordered commits because the dependencies between commits changed.
> Added more developer tools.
> Added definitions of DTS testbed elements.
> Reworked SSH implementation - split it so that the split between an
> abstraction and the actual implementation is clearer.
> Modified the directory structure to better organize the current and the
> future code.
>
> Juraj Linkeš (9):
>   dts: add project tools config
>   dts: add developer tools
>   dts: add basic logging facility
>   dts: add remote session abstraction
>   dts: add ssh connection module
>   dts: add node base class
>   dts: add dts workflow module
>   dts: add dts executable script
>   maintainers: add dts maintainers
>
> Owen Hilyard (1):
>   dts: add config parser module
>
>  .editorconfig |   2 +-
>  .gitignore|   9 +-
>  MAINTAINERS   |   5 +
>  devtools/python-checkpatch.sh |  39 ++
>  devtools/python-format.sh |  54 +++
>  devtools/python-lint.sh   |  26 ++
>  doc/guides/contributing/coding_style.rst  |   4 +-
>  dts/.devcontainer/devcontainer.json   |  30 ++
>  dts/Dockerfile|  39 ++
>  dts/README.md | 154 
>  dts/conf.yaml |   6 +
>  dts/framework/__init__.py |   4 +
>  dts/framework/config/__init__.py  |  99 +
>  dts/framework/config/conf_yaml_schema.json|  73 
>  dts/framework/dts.py  |  69 
>  dts/framework/exception.py|  71 
>  dts/framework/logger.py   | 115 ++
>  dts/framework/remote_session/__init__.py  |   5 +
>  .../remote_session/remote_session.py  | 100 +
>  .../remote_session/session_factory.py |  16 +
>  dts/framework/remote_session/ssh_session.py   | 189 ++
>  dts/framework/settings.py   

Re: testpmd logging

[Owen Hilyard]

On Tue, Nov 8, 2022 at 9:37 AM Honnappa Nagarahalli <
honnappa.nagaraha...@arm.com> wrote:

> + Lijuan, Owen, Juraj
>
> > -Original Message-
> > From: Thomas Monjalon 
> > Sent: Tuesday, November 8, 2022 7:02 AM
> > To: Aman Singh ; Yuying Zhang
> > 
> > Cc: david.march...@redhat.com; dev@dpdk.org
> > Subject: testpmd logging
> >
> > Hello,
> >
> > I see testpmd is doing log with EAL entity. Example:
> >   RTE_LOG(ERR, EAL, "The device: %s has been added!\n"
> > We could also discuss the log level.
> >
> > Please can we work on replacing these logs?
>

If we want to revisit logging in testpmd, even though I know I'm invoking a
case of second system syndrome here, I would love to see some form of
structured logging output. Even just writing out records to a CSV file with
proper escaping would work great, or doing a json object per log record as
a backup. JSON might be easier since libjansson is already in DPDK, and we
could disable the json logging flag if libjansson was not present at
compile time.


> DTS uses some of the logs from testpmd to validate the results. I am not
> sure what all logs are used.


The old version of dts (the one with its own repository) is fairly tightly
bound to testpmd's logging.

> We need to choose how to log (printf or registered log entity) and be
> > consistent.
> > What do you think?
>

Registered log entries are much better in my opinion since they provide a
more machine-friendly format and DTS has to parse testpmd output.

Re: [PATCH v8 0/9] dts: ssh connection to a node

[Owen Hilyard]

On Wed, Nov 9, 2022 at 11:23 AM Honnappa Nagarahalli <
honnappa.nagaraha...@arm.com> wrote:

> 
>
> >
> > 04/11/2022 12:05, Juraj Linkeš:
> > > 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.
> >
> > Applied, thanks.
> >
> > That's the first step towards integration of DTS in DPDK repository.
> > Nice to see this becoming a reality.
> Thanks Thomas and the community for the reviews and suggestions. This is
> the first important step, hopefully we can continue the collaboration in
> the future releases.
>
> >
> > [...]
> > > This is our current roadmap:
> > > 1. Review this patchset and do the rest of the items in parallel, if
> > > possible.
> > > 2. We have extracted the code needed to run the most basic testcase,
> > > HelloWorld, which runs the DPDK Hello World application. We'll split
> > > this along logical/functional boundaries and send after 1 is done.
> > > 3. Once we have 2 applied, we're planning on adding a basic functional
> > > testcase - pf_smoke. This send a bit of traffic, so the big addition
> > > is the software traffic generator, Scapy. There's some work already
> > > done on Traffic generators we'll be sending as a dependence on this
> > > patch series.
> > > 4. After 3, we'll add a basic performance testcase which doesn't use
> > > Scapy, but Trex or Ixia instead.
> > > 5. This is far in the future, but at this point we should have all of
> > > the core functionality in place. What then remains is adding the rest
> > > of the testcases.
> >
> > Let's join the force and help making this project a pleasant tool.
> >
>
>
I would also like to thank everyone for their reviews and suggestions
toward DTS. The primary goal of this rewrite is make it as easy as possible
to write robust tests so that DPDK can be heavily and reliably tested. This
rewrite allows us at the DTS Working Group to leverage more than 8 years of
experience with the prior DTS to help make it much easier for anyone to
write tests or set up their own testing, but we also want feedback from the
community, so please take a look and give us more feedback.

I would also like to remind everyone that if you have any features that you
want to see in DTS you should bring them to us as soon as possible. We are
starting with support for NICs, but want to be able to branch out to test
all DPDK-supported devices. If you have concerns or required features that
you don't want to discuss on the mailing list for some reason, you can send
your concerns directly to me and I will make sure your needs are
represented in discussions around DTS.

Planned but Unimplemented Features:
* telnet connections for the DUT
* scapy traffic generator (low performance but easy to use)
* trex traffic generator (DPDK-based software traffic generator)
* IXIA traffic generator (hardware traffic generator)
* Traffic generator abstraction layer
* Automatic skipping of tests based on the hardware under test, os and
other factors
* Abstractions to test development to allow simple tests (send this list of
packets then expect this list, etc) to be written as easily as possible.
* Automatic setup and teardown of virtual machines for virtio testing
* The ability to cross compile for a given target on the system running DTS
(assuming relevant libraries/compilers are installed), then install onto
the system under test (intended for embedded systems or SOCs).
* Structured logging for automated analysis
* and many more

Re: [PATCH] devtools: set DTS directory to format check

[Owen Hilyard]

On Wed, Nov 9, 2022 at 12:09 PM Thomas Monjalon  wrote:

> The script was running on the current directory.
> If not in the DTS directory, it would re-format every Python files.
>
> A new positional argument is added to specify the directory to check.
> In most cases, the (new) default value should be enough.
>
> While updating argument handling,
> the usage is printed in case of wrong argument.
>
> Signed-off-by: Thomas Monjalon 
> ---
>  devtools/dts-check-format.sh | 14 ++
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/devtools/dts-check-format.sh b/devtools/dts-check-format.sh
> index dc07150775..eb1bdd2a01 100755
> --- a/devtools/dts-check-format.sh
> +++ b/devtools/dts-check-format.sh
> @@ -3,11 +3,10 @@
>  # Copyright(c) 2022 University of New Hampshire
>
>  usage() {
> -   echo "Run formatting and linting programs for DTS. Usage:"
> -
> +   echo 'Usage: $(basename $0) [options] [directory]'
> +   echo 'Options:'
> # Get source code comments after getopts arguments and print them
> both
> grep -E '[a-zA-Z]+\) +#' "$0" | tr -d '#'
> -   exit 0
>  }
>
>  format=true
> @@ -17,7 +16,9 @@ lint=true
>  while getopts "hfl" arg; do
> case $arg in
> h) # Display this message
> +   echo 'Run formatting and linting programs for DTS.'
> usage
> +   exit 0
> ;;
> f) # Don't run formatters
> format=false
> @@ -25,10 +26,15 @@ while getopts "hfl" arg; do
> l) # Don't run linter
> lint=false
> ;;
> -   *)
> +   ?)
> +   usage
> +   exit 1
> esac
>  done
> +shift $(($OPTIND - 1))
>
> +directory=${1:-$(dirname $0)/../dts}
> +cd $directory || exit 1
>
>  errors=0
>
> --
> 2.36.1
>
>
Looks good to me.

Reviewed-by: Owen Hilyard 

Re: [RFC PATCH v2 03/10] dts: add dpdk build on sut

[Owen Hilyard]

On Mon, Nov 14, 2022 at 11:54 AM Juraj Linkeš 
wrote:

> Add the ability to build DPDK and apps, using a configured target.
>
> Signed-off-by: Juraj Linkeš 
> ---
>  dts/framework/exception.py|  17 +++
>  dts/framework/remote_session/os/os_session.py |  90 +++-
>  .../remote_session/os/posix_session.py| 128 +
>  .../remote_session/remote_session.py  |  34 -
>  dts/framework/remote_session/ssh_session.py   |  64 -
>  dts/framework/settings.py |  40 +-
>  dts/framework/testbed_model/node/sut_node.py  | 131 ++
>  dts/framework/utils.py|  15 ++
>  8 files changed, 505 insertions(+), 14 deletions(-)
>
> diff --git a/dts/framework/exception.py b/dts/framework/exception.py
> index b282e48198..93d99432ae 100644
> --- a/dts/framework/exception.py
> +++ b/dts/framework/exception.py
> @@ -26,6 +26,7 @@ class ReturnCode(IntEnum):
>  GENERIC_ERR = 1
>  SSH_ERR = 2
>  REMOTE_CMD_EXEC_ERR = 3
> +DPDK_BUILD_ERR = 10
>  NODE_SETUP_ERR = 20
>  NODE_CLEANUP_ERR = 21
>
> @@ -110,6 +111,22 @@ def __str__(self) -> str:
>  )
>
>
> +class RemoteDirectoryExistsError(DTSError):
> +"""
> +Raised when a remote directory to be created already exists.
> +"""
> +
> +return_code: ClassVar[ReturnCode] = ReturnCode.REMOTE_CMD_EXEC_ERR
> +
> +
> +class DPDKBuildError(DTSError):
> +"""
> +Raised when DPDK build fails for any reason.
> +"""
> +
> +return_code: ClassVar[ReturnCode] = ReturnCode.DPDK_BUILD_ERR
> +
> +
>  class NodeSetupError(DTSError):
>  """
>  Raised when setting up a node.
> diff --git a/dts/framework/remote_session/os/os_session.py
> b/dts/framework/remote_session/os/os_session.py
> index 2a72082628..57e2865282 100644
> --- a/dts/framework/remote_session/os/os_session.py
> +++ b/dts/framework/remote_session/os/os_session.py
> @@ -2,12 +2,15 @@
>  # Copyright(c) 2022 PANTHEON.tech s.r.o.
>  # Copyright(c) 2022 University of New Hampshire
>
> -from abc import ABC
> +from abc import ABC, abstractmethod
> +from pathlib import PurePath
>
> -from framework.config import NodeConfiguration
> +from framework.config import Architecture, NodeConfiguration
>  from framework.logger import DTSLOG
>  from framework.remote_session.factory import create_remote_session
>  from framework.remote_session.remote_session import RemoteSession
> +from framework.settings import SETTINGS
> +from framework.utils import EnvVarsDict
>
>
>  class OSSession(ABC):
> @@ -44,3 +47,86 @@ def is_alive(self) -> bool:
>  Check whether the remote session is still responding.
>  """
>  return self.remote_session.is_alive()
> +
> +@abstractmethod
> +def guess_dpdk_remote_dir(self, remote_dir) -> PurePath:
> +"""
> +Try to find DPDK remote dir in remote_dir.
> +"""
> +
> +@abstractmethod
> +def get_remote_tmp_dir(self) -> PurePath:
> +"""
> +Get the path of the temporary directory of the remote OS.
> +"""
> +
> +@abstractmethod
> +def get_dpdk_build_env_vars(self, arch: Architecture) -> dict:
> +"""
> +Create extra environment variables needed for the target
> architecture. Get
> +information from the node if needed.
> +"""
> +
> +@abstractmethod
> +def join_remote_path(self, *args: str | PurePath) -> PurePath:
> +"""
> +Join path parts using the path separator that fits the remote OS.
> +"""
> +
> +@abstractmethod
> +def copy_file(
> +self,
> +source_file: str | PurePath,
> +destination_file: str | PurePath,
> +source_remote: bool = False,
> +) -> None:
> +"""
> +Copy source_file from local storage to destination_file on the
> remote Node
> +associated with the remote session.
> +If source_remote is True, reverse the direction - copy
> source_file from the
> +associated remote Node to destination_file on local storage.
> +"""
> +
> +@abstractmethod
> +def remove_remote_dir(
> +self,
> +remote_dir_path: str | PurePath,
> +recursive: bool = True,
> +force: bool = True,
> +) -> None:
> +"""
> +Remove remote directory, by default remove recursively and
> forcefully.
> +"""
> +
> +@abstractmethod
> +def extract_remote_tarball(
> +self,
> +remote_tarball_path: str | PurePath,
> +expected_dir: str | PurePath | None = None,
> +) -> None:
> +"""
> +Extract remote tarball in place. If expected_dir is a non-empty
> string, check
> +whether the dir exists after extracting the archive.
> +"""
> +
> +@abstractmethod
> +def build_dpdk(
> +self,
> +env_vars: EnvVarsDict,
> +meson_args: str,
> +remote_dpdk_dir: str | PurePath,
> +target_name: str,
> 

Re: [RFC PATCH v2 04/10] dts: add dpdk execution handling

[Owen Hilyard]

On Mon, Nov 14, 2022 at 11:54 AM Juraj Linkeš 
wrote:

> Add methods for setting up and shutting down DPDK apps and for
> constructing EAL parameters.
>
> Signed-off-by: Juraj Linkeš 
> ---
>  dts/conf.yaml |   4 +
>  dts/framework/config/__init__.py  |  85 -
>  dts/framework/config/conf_yaml_schema.json|  22 +++
>  .../remote_session/os/linux_session.py|  15 ++
>  dts/framework/remote_session/os/os_session.py |  16 +-
>  .../remote_session/os/posix_session.py|  80 
>  dts/framework/testbed_model/hw/__init__.py|  17 ++
>  dts/framework/testbed_model/hw/cpu.py | 164 
>  dts/framework/testbed_model/node/node.py  |  36 
>  dts/framework/testbed_model/node/sut_node.py  | 178 +-
>  dts/framework/utils.py|  20 ++
>  11 files changed, 634 insertions(+), 3 deletions(-)
>  create mode 100644 dts/framework/testbed_model/hw/__init__.py
>  create mode 100644 dts/framework/testbed_model/hw/cpu.py
>
> diff --git a/dts/conf.yaml b/dts/conf.yaml
> index 6b0bc5c2bf..976888a88e 100644
> --- a/dts/conf.yaml
> +++ b/dts/conf.yaml
> @@ -12,4 +12,8 @@ nodes:
>- name: "SUT 1"
>  hostname: sut1.change.me.localhost
>  user: root
> +arch: x86_64
>  os: linux
> +bypass_core0: true
> +cpus: ""
> +memory_channels: 4
> diff --git a/dts/framework/config/__init__.py
> b/dts/framework/config/__init__.py
> index 1b97dc3ab9..344d697a69 100644
> --- a/dts/framework/config/__init__.py
> +++ b/dts/framework/config/__init__.py
> @@ -11,12 +11,13 @@
>  import pathlib
>  from dataclasses import dataclass
>  from enum import Enum, auto, unique
> -from typing import Any
> +from typing import Any, Iterable
>
>  import warlock  # type: ignore
>  import yaml
>
>  from framework.settings import SETTINGS
> +from framework.utils import expand_range
>
>
>  class StrEnum(Enum):
> @@ -60,6 +61,80 @@ class Compiler(StrEnum):
>  msvc = auto()
>
>
> +@dataclass(slots=True, frozen=True)
> +class CPU:
> +cpu: int
> +core: int
> +socket: int
> +node: int
> +
> +def __str__(self) -> str:
> +return str(self.cpu)
> +
> +
> +class CPUList(object):
> +"""
> +Convert these options into a list of int cpus
> +cpu_list=[CPU1, CPU2] - a list of CPUs
> +cpu_list=[0,1,2,3] - a list of int indices
> +cpu_list=['0','1','2-3'] - a list of str indices; ranges are supported
> +cpu_list='0,1,2-3' - a comma delimited str of indices; ranges are
> supported
> +
> +The class creates a unified format used across the framework and
> allows
> +the user to use either a str representation (using str(instance) or
> directly
> +in f-strings) or a list representation (by accessing
> instance.cpu_list).
> +Empty cpu_list is allowed.
> +"""
> +
> +_cpu_list: list[int]
> +
> +def __init__(self, cpu_list: list[int | str | CPU] | str):
> +self._cpu_list = []
> +if isinstance(cpu_list, str):
> +self._from_str(cpu_list.split(","))
> +else:
> +self._from_str((str(cpu) for cpu in cpu_list))
> +
> +# the input cpus may not be sorted
> +self._cpu_list.sort()
> +
> +@property
> +def cpu_list(self) -> list[int]:
> +return self._cpu_list
> +
> +def _from_str(self, cpu_list: Iterable[str]) -> None:
> +for cpu in cpu_list:
> +self._cpu_list.extend(expand_range(cpu))
> +
> +def _get_consecutive_cpus_range(self, cpu_list: list[int]) ->
> list[str]:
> +formatted_core_list = []
> +tmp_cpus_list = list(sorted(cpu_list))
> +segment = tmp_cpus_list[:1]
> +for core_id in tmp_cpus_list[1:]:
> +if core_id - segment[-1] == 1:
> +segment.append(core_id)
> +else:
> +formatted_core_list.append(
> +f"{segment[0]}-{segment[-1]}"
> +if len(segment) > 1
> +else f"{segment[0]}"
> +)
> +current_core_index = tmp_cpus_list.index(core_id)
> +formatted_core_list.extend(
> +
> self._get_consecutive_cpus_range(tmp_cpus_list[current_core_index:])
> +)
> +segment.clear()
> +break
> +if len(segment) > 0:
> +formatted_core_list.append(
> +f"{segment[0]}-{segment[-1]}" if len(segment) > 1 else
> f"{segment[0]}"
> +)
> +return formatted_core_list
> +
> +def __str__(self) -> str:
> +return
> f'{",".join(self._get_consecutive_cpus_range(self._cpu_list))}'
> +
> +
>  # Slots enables some optimizations, by pre-allocating space for the
> defined
>  # attributes in the underlying data structure.
>  #
> @@ -71,7 +146,11 @@ class NodeConfiguration:
>  hostname: str
>  user: str
>  password: str | None
> +arch: Architecture
>  os: OS
> +bypass_co

Re: [RFC PATCH v2 05/10] dts: add node memory setup

[Owen Hilyard]

On Mon, Nov 14, 2022 at 11:54 AM Juraj Linkeš 
wrote:

> Setup hugepages on nodes. This is useful not only on SUT nodes, but
> also on TG nodes which use TGs that utilize hugepages.
>
> Signed-off-by: Juraj Linkeš 
> ---
>  dts/framework/remote_session/__init__.py  |  1 +
>  dts/framework/remote_session/arch/__init__.py | 20 +
>  dts/framework/remote_session/arch/arch.py | 57 +
>  .../remote_session/os/linux_session.py| 85 +++
>  dts/framework/remote_session/os/os_session.py | 10 +++
>  dts/framework/testbed_model/node/node.py  | 15 +++-
>  6 files changed, 187 insertions(+), 1 deletion(-)
>  create mode 100644 dts/framework/remote_session/arch/__init__.py
>  create mode 100644 dts/framework/remote_session/arch/arch.py
>
> diff --git a/dts/framework/remote_session/__init__.py
> b/dts/framework/remote_session/__init__.py
> index f2339b20bd..f0deeadac6 100644
> --- a/dts/framework/remote_session/__init__.py
> +++ b/dts/framework/remote_session/__init__.py
> @@ -11,4 +11,5 @@
>
>  # pylama:ignore=W0611
>
> +from .arch import Arch, create_arch
>  from .os import OSSession, create_session
> diff --git a/dts/framework/remote_session/arch/__init__.py
> b/dts/framework/remote_session/arch/__init__.py
> new file mode 100644
> index 00..d78ad42ac5
> --- /dev/null
> +++ b/dts/framework/remote_session/arch/__init__.py
> @@ -0,0 +1,20 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2022 PANTHEON.tech s.r.o.
> +
> +from framework.config import Architecture, NodeConfiguration
> +
> +from .arch import PPC64, Arch, Arm64, i686, x86_32, x86_64
> +
> +
> +def create_arch(node_config: NodeConfiguration) -> Arch:
> +match node_config.arch:
> +case Architecture.x86_64:
> +return x86_64()
> +case Architecture.x86_32:
> +return x86_32()
> +case Architecture.i686:
> +return i686()
> +case Architecture.ppc64le:
> +return PPC64()
> +case Architecture.arm64:
> +return Arm64()
> diff --git a/dts/framework/remote_session/arch/arch.py
> b/dts/framework/remote_session/arch/arch.py
> new file mode 100644
> index 00..05c7602def
> --- /dev/null
> +++ b/dts/framework/remote_session/arch/arch.py
> @@ -0,0 +1,57 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2022 PANTHEON.tech s.r.o.
> +
> +
> +class Arch(object):
> +"""
> +Stores architecture-specific information.
> +"""
> +
> +@property
> +def default_hugepage_memory(self) -> int:
> +"""
> +Return the default amount of memory allocated for hugepages DPDK
> will use.
> +The default is an amount equal to 256 2MB hugepages (512MB
> memory).
> +"""
> +return 256 * 2048
> +
> +@property
> +def hugepage_force_first_numa(self) -> bool:
> +"""
> +An architecture may need to force configuration of hugepages to
> first socket.
> +"""
> +return False
> +
> +
> +class x86_64(Arch):
> +@property
> +def default_hugepage_memory(self) -> int:
> +return 4096 * 2048
> +
> +
> +class x86_32(Arch):
> +@property
> +def hugepage_force_first_numa(self) -> bool:
> +return True
> +
> +
> +class i686(Arch):
> +@property
> +def default_hugepage_memory(self) -> int:
> +return 512 * 2048
> +
> +@property
> +def hugepage_force_first_numa(self) -> bool:
> +return True
> +
> +
> +class PPC64(Arch):
> +@property
> +def default_hugepage_memory(self) -> int:
> +return 512 * 2048
> +
> +
> +class Arm64(Arch):
> +@property
> +def default_hugepage_memory(self) -> int:
> +return 2048 * 2048
> diff --git a/dts/framework/remote_session/os/linux_session.py
> b/dts/framework/remote_session/os/linux_session.py
> index 21f117b714..fad33d7613 100644
> --- a/dts/framework/remote_session/os/linux_session.py
> +++ b/dts/framework/remote_session/os/linux_session.py
> @@ -3,6 +3,8 @@
>  # Copyright(c) 2022 University of New Hampshire
>
>  from framework.config import CPU
> +from framework.exception import RemoteCommandExecutionError
> +from framework.utils import expand_range
>
>  from .posix_session import PosixSession
>
> @@ -24,3 +26,86 @@ def get_remote_cpus(self, bypass_core0: bool) ->
> list[CPU]:
>  continue
>  cpus.append(CPU(int(cpu), int(core), int(socket), int(node)))
>  return cpus
> +
> +def setup_hugepages(
> +self, hugepage_amount: int = -1, force_first_numa: bool = False
>

I think that hugepage_amount: int | None = None is better, since it
expresses it is an optional argument and the type checker will force anyone
using the value to check if it is none, whereas that will not happen with
-1.


> +) -> None:
> +self.logger.info("Getting Hugepage information.")
> +hugepage_size = self._get_hugepage_size()
> +hugepages_total = self._get_hugepages_total()
> +   

Re: [RFC PATCH v2 07/10] dts: add simple stats report

[Owen Hilyard]

You are missing type annotations throughout this.

On Mon, Nov 14, 2022 at 11:54 AM Juraj Linkeš 
wrote:

> Provide a summary of testcase passed/failed/blocked counts.
>
> Signed-off-by: Juraj Linkeš 
> ---
>  dts/framework/dts.py|  3 ++
>  dts/framework/stats_reporter.py | 65 +
>  2 files changed, 68 insertions(+)
>  create mode 100644 dts/framework/stats_reporter.py
>
> diff --git a/dts/framework/dts.py b/dts/framework/dts.py
> index d606f8de2e..a7c243a5c3 100644
> --- a/dts/framework/dts.py
> +++ b/dts/framework/dts.py
> @@ -14,11 +14,13 @@
>  from .exception import DTSError, ReturnCode
>  from .logger import DTSLOG, getLogger
>  from .settings import SETTINGS
> +from .stats_reporter import TestStats
>  from .test_result import Result
>  from .utils import check_dts_python_version
>
>  dts_logger: DTSLOG = getLogger("dts")
>  result: Result = Result()
> +test_stats: TestStats = TestStats(SETTINGS.output_dir + "/statistics.txt")
>
>
>  def run_all() -> None:
> @@ -29,6 +31,7 @@ def run_all() -> None:
>  return_code = ReturnCode.NO_ERR
>  global dts_logger
>  global result
> +global test_stats
>
>  # check the python version of the server that run dts
>  check_dts_python_version()
> diff --git a/dts/framework/stats_reporter.py
> b/dts/framework/stats_reporter.py
> new file mode 100644
> index 00..a2735d0a1d
> --- /dev/null
> +++ b/dts/framework/stats_reporter.py
> @@ -0,0 +1,65 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2010-2014 Intel Corporation
> +# Copyright(c) 2022 PANTHEON.tech s.r.o.
> +
> +"""
> +Simple text file statistics generator
> +"""
> +
> +
> +class TestStats(object):
> +"""
> +Generates a small statistics file containing the number of passing,
> +failing and blocked tests. It makes use of a Result instance as input.
> +"""
> +
> +def __init__(self, filename):
> +self.filename = filename
> +
> +def __add_stat(self, test_result):


I think that this should probably be an option of an enum that gets matched
over. ex:

match test_result:
case None:
pass
case PASSED:
self.passed += 1
case FAILED:
self.failed += 1
case BLOCKED:
self.blocked += 1
case unknown:
# log this and throw an error.


> +if test_result is not None:
> +if test_result[0] == "PASSED":
> +self.passed += 1
> +if test_result[0] == "FAILED":
> +self.failed += 1
> +if test_result[0] == "BLOCKED":
> +self.blocked += 1
> +self.total += 1
> +
> +def __count_stats(self):
> +for sut in self.result.all_suts():
> +for target in self.result.all_targets(sut):
> +for suite in self.result.all_test_suites(sut, target):
> +for case in self.result.all_test_cases(sut, target,
> suite):
> +test_result = self.result.result_for(sut, target,
> suite, case)
> +if len(test_result):
> +self.__add_stat(test_result)
> +
> +def __write_stats(self):
> +sut_nodes = self.result.all_suts()
> +if len(sut_nodes) == 1:
> +self.stats_file.write(
> +f"dpdk_version =
> {self.result.current_dpdk_version(sut_nodes[0])}\n"
> +)
> +else:
> +for sut in sut_nodes:
> +dpdk_version = self.result.current_dpdk_version(sut)
> +self.stats_file.write(f"{sut}.dpdk_version =
> {dpdk_version}\n")
> +self.__count_stats()
> +self.stats_file.write(f"Passed = {self.passed}\n")
> +self.stats_file.write(f"Failed = {self.failed}\n")
> +self.stats_file.write(f"Blocked= {self.blocked}\n")
> +rate = 0
> +if self.total > 0:
> +rate = self.passed * 100.0 / self.total
> +self.stats_file.write(f"Pass rate  = {rate:.1f}\n")
> +
> +def save(self, result):
> +self.passed = 0
> +self.failed = 0
> +self.blocked = 0
> +self.total = 0
> +self.stats_file = open(self.filename, "w+")
> +self.result = result
> +self.__write_stats()
> +self.stats_file.close()
> --
> 2.30.2
>
>

Re: [RFC PATCH v2 08/10] dts: add testsuite class

[Owen Hilyard]

On Mon, Nov 14, 2022 at 11:54 AM Juraj Linkeš 
wrote:

> This is the base class that all test suites inherit from. The base class
> implements methods common to all test suites. The derived test suites
> implement tests and any particular setup needed for the suite or tests.
>
> Signed-off-by: Juraj Linkeš 
> ---
>  dts/conf.yaml  |   4 +
>  dts/framework/config/__init__.py   |  33 ++-
>  dts/framework/config/conf_yaml_schema.json |  49 
>  dts/framework/dts.py   |  29 +++
>  dts/framework/exception.py |  65 ++
>  dts/framework/settings.py  |  25 +++
>  dts/framework/test_case.py | 246 +
>  7 files changed, 450 insertions(+), 1 deletion(-)
>  create mode 100644 dts/framework/test_case.py
>
> diff --git a/dts/conf.yaml b/dts/conf.yaml
> index 976888a88e..0b0f2c59b0 100644
> --- a/dts/conf.yaml
> +++ b/dts/conf.yaml
> @@ -7,6 +7,10 @@ executions:
>  os: linux
>  cpu: native
>  compiler: gcc
> +perf: false
> +func: true
> +test_suites:
> +  - hello_world
>  system_under_test: "SUT 1"
>  nodes:
>- name: "SUT 1"
> diff --git a/dts/framework/config/__init__.py
> b/dts/framework/config/__init__.py
> index 344d697a69..8874b10030 100644
> --- a/dts/framework/config/__init__.py
> +++ b/dts/framework/config/__init__.py
> @@ -11,7 +11,7 @@
>  import pathlib
>  from dataclasses import dataclass
>  from enum import Enum, auto, unique
> -from typing import Any, Iterable
> +from typing import Any, Iterable, TypedDict
>
>  import warlock  # type: ignore
>  import yaml
> @@ -186,9 +186,34 @@ def from_dict(d: dict) -> "BuildTargetConfiguration":
>  )
>
>
> +class TestSuiteConfigDict(TypedDict):
> +suite: str
> +cases: list[str]
> +
> +
> +@dataclass(slots=True, frozen=True)
> +class TestSuiteConfig:
> +test_suite: str
> +test_cases: list[str]
> +
> +@staticmethod
> +def from_dict(
> +entry: str | TestSuiteConfigDict,
> +) -> "TestSuiteConfig":
> +if isinstance(entry, str):
> +return TestSuiteConfig(test_suite=entry, test_cases=[])
> +elif isinstance(entry, dict):
> +return TestSuiteConfig(test_suite=entry["suite"],
> test_cases=entry["cases"])
> +else:
> +raise TypeError(f"{type(entry)} is not valid for a test suite
> config.")
> +
> +
>  @dataclass(slots=True, frozen=True)
>  class ExecutionConfiguration:
>  build_targets: list[BuildTargetConfiguration]
> +perf: bool
> +func: bool
> +test_suites: list[TestSuiteConfig]
>  system_under_test: NodeConfiguration
>
>  @staticmethod
> @@ -196,11 +221,17 @@ def from_dict(d: dict, node_map: dict) ->
> "ExecutionConfiguration":
>  build_targets: list[BuildTargetConfiguration] = list(
>  map(BuildTargetConfiguration.from_dict, d["build_targets"])
>  )
> +test_suites: list[TestSuiteConfig] = list(
> +map(TestSuiteConfig.from_dict, d["test_suites"])
> +)
>  sut_name = d["system_under_test"]
>  assert sut_name in node_map, f"Unknown SUT {sut_name} in
> execution {d}"
>
>  return ExecutionConfiguration(
>  build_targets=build_targets,
> +perf=d["perf"],
> +func=d["func"],
> +test_suites=test_suites,
>  system_under_test=node_map[sut_name],
>  )
>
> diff --git a/dts/framework/config/conf_yaml_schema.json
> b/dts/framework/config/conf_yaml_schema.json
> index c59d3e30e6..e37ced65fe 100644
> --- a/dts/framework/config/conf_yaml_schema.json
> +++ b/dts/framework/config/conf_yaml_schema.json
> @@ -63,6 +63,31 @@
>  }
>},
>"additionalProperties": false
> +},
> +"test_suite": {
> +  "type": "string",
> +  "enum": [
> +"hello_world"
> +  ]
> +},
> +"test_target": {
> +  "type": "object",
> +  "properties": {
> +"suite": {
> +  "$ref": "#/definitions/test_suite"
> +},
> +"cases": {
> +  "type": "array",
> +  "items": {
> +"type": "string"
> +  },
> +  "minimum": 1
> +}
> +  },
> +  "required": [
> +"suite"
> +  ],
> +  "additionalProperties": false
>  }
>},
>"type": "object",
> @@ -130,6 +155,27 @@
>  },
>  "minimum": 1
>},
> +  "perf": {
> +"type": "boolean",
> +"description": "Enable performance testing"
> +  },
> +  "func": {
> +"type": "boolean",
> +"description": "Enable functional testing"
> +  },
> +  "test_suites": {
> +"type": "array",
> +"items": {
> +  "oneOf": [
> +{
> +  "$ref": "#/definitions/test_suite"
> +},
> +{
> +  

Re: [RFC PATCH v2 04/10] dts: add dpdk execution handling

[Owen Hilyard]

On Wed, Nov 23, 2022 at 8:03 AM Juraj Linkeš 
wrote:

> Again, apologies for removing recipients in my earlier reply.
>
>
>
> *From:* Owen Hilyard 
> *Sent:* Monday, November 21, 2022 1:40 PM
> *To:* Juraj Linkeš 
> *Subject:* Re: [RFC PATCH v2 04/10] dts: add dpdk execution handling
>
>
>
> On Fri, Nov 18, 2022 at 8:00 AM Juraj Linkeš 
> wrote:
>
> diff --git a/dts/framework/config/conf_yaml_schema.json
> b/dts/framework/config/conf_yaml_schema.json
> index 409ce7ac74..c59d3e30e6 100644
> --- a/dts/framework/config/conf_yaml_schema.json
> +++ b/dts/framework/config/conf_yaml_schema.json
> @@ -6,6 +6,12 @@
>"type": "string",
>"description": "A unique identifier for a node"
>  },
> +"ARCH": {
> +  "type": "string",
> +  "enum": [
> +"x86_64"
>
> arm64 and ppc64le should probably be included here. I think that we can
> focus on 64 bit arches for now.
>
> [Juraj] Seems safe enough. At this point it doesn't matter, but when we
> have a number of testcases, we may need to revisit this (if we can't verify
> an architecture for example).
>
>
>
> [Owen] The reason I want this is because I want there to always be an
> architecture that is not the one being developed on that developers need to
> handle properly. LoongArch might actually be a good candidate for this if
> support gets merged, since to my knowledge almost no one has access to
> their server-class CPUs yet. Essentially, I want to force anyone who does
> something that is architecture dependent to consider other architectures,
> not just have the "the entire world is x86" mentality.
>
>
>
> Alright, good to know.
>
> I have a semi-related point, we specify arch (and os as well) in both
> build target and SUT config. Are these even going to be different? I see
> cpu (or platform in meson config) being different, but not the other two
> and that could simplify the config a bit.
>

[Owen] If I remember correctly, the older DTS has i686 (32 bit x86)
support, and you might want to run i686 on an x86_64 cpu. That is the only
use case I can see for differing build arch and SUT arch. The community lab
doesn't have any 32 bit hardware, so any future 32 bit testing would need
to happen on a 64 bit system running in a compatibility mode.


> 
>
> +def kill_cleanup_dpdk_apps(self) -> None:
> +"""
> +Kill all dpdk applications on the SUT. Cleanup hugepages.
> +"""
> +if self._dpdk_kill_session and self._dpdk_kill_session.is_alive():
> +# we can use the session if it exists and responds
> +
> self._dpdk_kill_session.kill_cleanup_dpdk_apps(self.dpdk_prefix_list)
> +else:
> +# otherwise, we need to (re)create it
> +self._dpdk_kill_session = self.create_session("dpdk_kill")
> +self.dpdk_prefix_list = []
> +
> +def create_eal_parameters(
> +self,
> +fixed_prefix: bool = False,
> +core_filter_specifier: CPUAmount | CPUList = CPUAmount(),
> +ascending_cores: bool = True,
> +prefix: str = "",
> +no_pci: bool = False,
> +vdevs: list[str] = None,
>
> I would prefer to have vdevs be a list of objects, even if for now that
> class just takes a string in its constructor. Later on we can add
> subclasses for specific vdevs that might see heavy use, such
> as librte_net_pcap and crypto_openssl.
>
> [Juraj] Ok, this is simple enough, I'll add it.
>
>

Re: Mysterious CI/IOL failures in Patchwork

[Owen Hilyard]

Hello Morten,

It looks like our Fedora 35 and 36 environments were erroneously hidden
from public view. This has been fixed and you should now be able to see the
issues on the dashboard at the link you provided. Also, please CC
dpdk...@iol.unh.edu when you want to contact the community lab. That
address will send the email to everyone involved with DPDK here at UNH so
you will usually get faster response times.

Sorry for any inconvenience
Owen Hilyard

[RFC] DPDK Distro Support Policies

[Owen Hilyard]

Hello Everyone,

At the tech board meeting today I brought up the topic of supported Linux
distros/operating systems. We came up with the following proposal, but want
community feedback before any decision is made. If you want a distro/OS
added, please reply saying so and provide a compelling reason why the
proposed distro/OS should be supported by DPDK, or why you think that more
or fewer versions should be supported. The policy is as follows;

Supported Distros/OSes:
* Alpine
* Arch
* CentOS Stream
* Debian
* Fedora
* Fedora Rawhide
* FreeBSD
* OpenSUSE Leap
* RHEL
* Ubuntu
* Windows

Supported Versions:

Distros or OS versions in General Support will be tested. Support will be
dropped for EOL versions or versions older than 5 years.

RHEL 7 has been granted an expectation to this due to widespread use, and
will continue to be supported until the tech board decides it is out of
widespread use.

Additionally, "binary compatible" distros will have a single distro
selected for testing. For example, RHEL 7 will be treated as providing
testing for CentOS 7.

Owen Hilyard
UNH InterOperability Lab

DTS Improvement Working Group Minutes (March 2)

[Owen Hilyard]

Attendees

   -

   Owen Hilyard
   -

   Honnappa Nagarahalli
   -

   Lijuan Tu
   -

   Lincoln Lavoie

Agenda

   -

   Additions to the agenda
   -

   Review/Carry forward pending action items
   -

   Review DTS DPDK Modifications excel sheet
   -

   Merging DTS into DPDK
   -

   Review additional work items

MinutesReview/Carry forward pending action items

   -

   Honnappa - Patches to DPDK to address changing the constants and printfs
   under progress.
   -

  Still in progress
  -

   Honnappa - The changes to DPDK need to be backported to LTS releases.
   Discuss with Lijuan and the community.
   -

  Patches will be backported
  -

   Owen - Owen to review Juraj’s feedback.
   -

  Done
  -

   Honnappa - Follow up with DPDK governing board to get the approval/legal
   opinion on including the GPL licensed files from DTS in the DPDK tree.
   -

  Governing board has not met
  -

   Owen - Look at making DTS build using the common DPDK repo in addition
   to the existing tarball option.
   -

  Looks to be possible, blocked by decisions on the placement of DTS
  inside of DPDK
  -

   Lijuan - The makefile removal patch is ready to get merged (
   http://patchwork.dpdk.org/project/dts/list/?series=20610). Merge it in
   the 22.03 DTS release.
   -

  Additional issues discovered
  -

 Meson configuration changed improperly
 -

  Makefiles will be removed as the meson issues are fixed
  -

   Juraj - Postponed: Convert the makefile build for documentation to use
   meson build after merging the repositories or the directory structure is
   known.
   -

  Not present
  -

   Honnappa - Discuss merge permissions for Owen for the DTS tree with the
   Tech Board
   -

  Merge permissions have been granted
  -

   Lijuan/Owen - Check if dep/scapy_modules/Dot1BR.py file is required to
   be in DTS. If it is required, understand options for replacement/removing
   this file.
   -

  Look into rewriting the file so it’s not GPL
  -

   Lijuan/Owen - Check if dep/QMP/qmp.py can be installed on DUT as part
   of  the qemu-guest-agent package.
   -

  In progress
  -

   Honnappa/Owen - Discuss Python bindings for DPDK in March 9th Techboard
   call.
   -

  Meeting has not happened yet

Merging DTS into DPDK

   -

   Are we going to integrate DTS with the DPDK build system? No resolution
   yet.
   -

   Try for 22.07
   -

   Focus on framework and test cases used in CI
   -

   Do a deep dive on Feb 9th - identify what needs to be done and estimate
   the work.


DPDK Modifications Review

   -

   Some issues are fixed
   -

   Many changes are still present.

Action Items

   -

   Honnappa - Patches to DPDK to address changing the constants and printfs
   under progress.
   -

   Owen - Write a script to run formatters (Black + Isort). Pin versions in
   dts requirements file. As part of the patch series, format DTS again.
   -

   Jurai - Rewrite dep/scapy_modules/Dot1BR.py from scratch so we can
   change the license.
   -

   Lijuan/Owen - Check if dep/QMP/qmp.py can be installed on DUT as part
   of  the qemu-guest-agent package.
   -

   Lijuan/Owen - Remove ABI_stable test suite
   -

   Lijuan/Owen - Investigate DTS test suites to check for redundant tests
   -

  Unit testing (handled by meson now)
  -

  “Does it compile?” tests
  -

  etc

Any other business

   -

   Next Meeting: March 9, 2022

[dpdk-dev] Memory leak in rte_pci_scan

[Owen Hilyard]

Hello All,

As part of the community lab's work to deploy static analysis tools, we've
been doing test runs of the various tools. One of the problems we found is
that rte_pci_scan leaks 214368 Bytes every time it is run. There are also
numerous (180856) instances of the error message "EAL: recvmsg failed, Bad
file descriptor". Attached is the log from running the fast-tests suite
with ASAN enabled, and a 10-second timeout. The real timeout seems to be
the timeout argument multiplied by 10, making this really a 100-second
timeout. It is attached as a google drive link since the log itself is too
large for the mailing list.

Test Output:
https://drive.google.com/file/d/1NejCuba-HnvH9MMg7thBdGuOhQBXKiFh/view?usp=sharing

Owen Hilyard

Re: [dpdk-dev] [dpdk-ci] [PATCH v2] pmdinfogen: allow padding after NUL terminator

[Owen Hilyard]

Hi Dmitry,

Those failures are the result of a known issue with that machine. Those
tests were disabled on that machine shortly after you submitted that patch.
I've re-run the patch.

Owen Hilyard

On Wed, Jun 9, 2021 at 12:48 PM Dmitry Kozlyuk 
wrote:

> 2021-06-09 12:01 (UTC-0400), Lincoln Lavoie:
> > Hi Dmitry,
> >
> > If the failing test is the unit test func_reentrancy_autotest, that is
> > being looked into , as it seems like the test case does not reliably run.
> > It passes / fails between different systems running both on bare metal
> and
> > in virtual environments, on different OSes and architectures.
> >
> > Do you have a link to your patch in patchworks or the lab dashboard?
>
> Hi Lincoln,
>
> Yes, this is func_reentrancy_autotest failure.
> Dashboard link:
> https://lab.dpdk.org/results/dashboard/patchsets/17240/
> MTU and stats can hardly be affected by this patch.
>

[dpdk-dev] Define statement with UB prevents compilation using UBSAN

[Owen Hilyard]

Hello,

While starting work on adding UBSAN to the community lab CI, I found that
DPDK does not compile in its default configuration with UBSAN enabled,
failing with the error message:

../drivers/net/bnx2x/bnx2x.c: In function ‘bnx2x_check_blocks_with_parity3’:
../drivers/net/bnx2x/bnx2x.c:3363:4: error: case label does not reduce to
an integer constant
 3363 |case AEU_INPUTS_ATTN_BITS_MCP_LATCHED_SCPAD_PARITY:
  |^~~~

Working backward to the define
statement, AEU_INPUTS_ATTN_BITS_MCP_LATCHED_SCPAD_PARITY is defined as

#define AEU_INPUTS_ATTN_BITS_MCP_LATCHED_SCPAD_PARITY (0x1 << 31)

While this does set the most significant bit of the integer, it also seems
to make UBSAN unable to properly track its type. Replacing this with

#define AEU_INPUTS_ATTN_BITS_MCP_LATCHED_SCPAD_PARITY (-2147483648)

or

#define AEU_INPUTS_ATTN_BITS_MCP_LATCHED_SCPAD_PARITY
(0b1000)

Allows compilation to finish on x86_64, but substantially breaks the style
of the rest of the file. There are a few other places where this undefined
behavior happens:

./drivers/baseband/acc100/rte_acc100_pmd.c:4490: value = (1 << 31) + (23 <<
8) + (1 << 6) + 7;
./drivers/net/bnxt/tf_core/tf_shadow_tcam.c:60:#define
TF_SHADOW_TCAM_HB_HANDLE_IS_VALID(hndl) (((hndl) & (1 << 31)) != 0)
./drivers/net/bnxt/tf_core/tf_shadow_tcam.c:61:#define
TF_SHADOW_TCAM_HB_HANDLE_CREATE(idx, be) ((1 << 31) | \
./drivers/net/bnxt/tf_core/tf_shadow_tbl.c:60:#define
TF_SHADOW_HB_HANDLE_IS_VALID(hndl) (((hndl) & (1 << 31)) != 0)
./drivers/net/bnxt/tf_core/tf_shadow_tbl.c:61:#define
TF_SHADOW_HB_HANDLE_CREATE(idx, be) ((1 << 31) | \
./drivers/net/e1000/base/e1000_regs.h:197:#define E1000_TQAVCC_QUEUE_MODE
(1 << 31) /* SP vs. SR Tx mode */
./drivers/net/e1000/base/e1000_regs.h:607:#define E1000_ETQF_QUEUE_ENABLE
(1 << 31)
./drivers/net/igc/base/igc_regs.h:201:#define IGC_TQAVCC_QUEUE_MODE (1 <<
31) /* SP vs. SR Tx mode */
./drivers/net/igc/base/igc_82575.h:248:#define IGC_ETQF_QUEUE_ENABLE (1 <<
31)
./drivers/net/igc/base/igc_82575.h:271:#define IGC_DTXSWC_VMDQ_LOOPBACK_EN
(1 << 31)  /* global VF LB enable */
./drivers/net/bnx2x/ecore_hsi.h:2597: #define TRIGGER_MDUMP_ONCE  (1 <<
31)
./drivers/net/bnx2x/ecore_hsi.h:3904:#define IGU_REGULAR_BCLEANUP (0x1 <<
31)
./drivers/net/bnx2x/ecore_reg.h:4366:#define
AEU_INPUTS_ATTN_BITS_CCM_HW_INTERRUPT (0x1 << 31)
./drivers/net/bnx2x/ecore_reg.h:4391:#define
AEU_INPUTS_ATTN_BITS_PBCLIENT_HW_INTERRUPT (0x1 << 31)
./drivers/net/ixgbe/base/ixgbe_type.h:4295:#define
IXGBE_KRM_LINK_CTRL_1_TETH_AN_RESTART (1 << 31)
./drivers/net/ixgbe/base/ixgbe_type.h:4325:#define
IXGBE_KRM_TX_COEFF_CTRL_1_OVRRD_EN (1 << 31)
./drivers/crypto/qat/qat_sym_pmd.h:24:#define QAT_SYM_CAP_VALID (1 << 31)
./drivers/bus/fslmc/portal/dpaa2_hw_pvt.h:301:#define
DPAA2_SET_FLE_FIN(fle) ((fle)->fin_bpid_offset |= 1 << 31)
./drivers/raw/ioat/ioat_spec.h:329:#define CMDSTATUS_ACTIVE_MASK (1 << 31)
./drivers/common/mlx5/windows/mlx5_win_defs.h:114: IBV_RX_HASH_INNER = (1
<< 31)

Aside from normal CI, I don't have a good way to verify that a patch to fix
this wouldn't have hard to detect effects. As far as I know, some of these
drivers don't have DTS run on them in the course of CI, so I'm somewhat
wary of trying to make a change to them. Until this is fixed, deploying
UBSAN in any real capacity won't be useful since every patch will fail.
What do the two of you think about steps moving forward on this problem?

Owen Hilyard

Re: [dpdk-dev] Define statement with UB prevents compilation using UBSAN

[Owen Hilyard]

Seeing the discussion so far, do we want to change the single definition to
be (0b1u << 31) so it works, or should we make this change in a wider scope
(file, directory, project-wide). If we do make the change in a wider scope,
should we only change instances where there is UB (1 << 31) or should we
change all of the bitflags and similar constructs to uint32_t? If we change
a lot, it may require special testing since I don't think every driver is
tested on a regular basis, and making a change like this in a wide-reaching
fashion has the potential to break a lot of things.

On Fri, Jun 11, 2021 at 10:34 AM Aaron Conole  wrote:

> Stephen Hemminger  writes:
>
> > On Thu, 10 Jun 2021 16:51:37 -0400
> > Owen Hilyard  wrote:
> >
> >> Working backward to the define
> >> statement, AEU_INPUTS_ATTN_BITS_MCP_LATCHED_SCPAD_PARITY is defined as
> >>
> >> #define AEU_INPUTS_ATTN_BITS_MCP_LATCHED_SCPAD_PARITY (0x1 << 31)
> >
> > Why not (1u << 31)?
>
> +1
>
> CC'd the QLogic maintainers as well.
>
>

Re: [dpdk-dev] [dts] [PATCH] malloc: fix size annotation for NUMA-aware realloc

[Owen Hilyard]

That test suite has been disabled on the system. Someone of the Intel team
should be looking at it soon, since stats_checks also has similar issues.

On Fri, Jun 11, 2021 at 3:40 AM David Marchand 
wrote:

> On Fri, Jun 11, 2021 at 9:26 AM David Marchand
>  wrote:
> >
> > On Thu, Jun 10, 2021 at 2:09 PM David Marchand
> >  wrote:
> > >
> > > __rte_alloc_size is mapped to compiler alloc_size attribute.
> >
> > I get the following splat from dts.
>
> There is the exact same output in last night next-net-intel test:
> https://lab.dpdk.org/results/dashboard/tarballs/15461/
>
>
> --
> David Marchand
>
>

Re: [dpdk-dev] Memory leak in rte_pci_scan

[Owen Hilyard]

>From what I've seen so far, that fixes the PCI leak. That just leaves a few
other places. I'll try to get the complete list to you tomorrow, since
running the full set of unit tests takes quite a few hours when ASAN is
involved.

On Mon, Jun 14, 2021 at 6:30 AM David Marchand 
wrote:

> On Mon, Jun 14, 2021 at 11:11 AM David Marchand
>  wrote:
> >
> > On Tue, Jun 8, 2021 at 8:48 PM Owen Hilyard 
> wrote:
> > >
> > > Hello All,
> > >
> > > As part of the community lab's work to deploy static analysis tools,
> we've
> > > been doing test runs of the various tools. One of the problems we
> found is
> > > that rte_pci_scan leaks 214368 Bytes every time it is run. There are
> also
> >
> > I suspect the "leak" is on pci device objects that are not released
> > unless hot(un)plugging.
> > Cc: Gaetan.
>
> I think I found a leak at:
> https://git.dpdk.org/dpdk/tree/drivers/bus/pci/linux/pci.c#n333
>
> For devices with no kernel driver, 'dev' will be leaked, since there
> is no reference to it in the pci device list.
>
> There will be more things to fix (there is a proposed patch on
> annotating the dpdk memory allocator for ASAN) but can you try this
> diff below with the mp patch [1] I sent to see if the situation gets
> better?
>
> diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
> index 0dc99e9cb2..5ea76bc867 100644
> --- a/drivers/bus/pci/linux/pci.c
> +++ b/drivers/bus/pci/linux/pci.c
> @@ -331,7 +331,7 @@ pci_scan_one(const char *dirname, const struct
> rte_pci_addr *addr)
> else
> dev->kdrv = RTE_PCI_KDRV_UNKNOWN;
> } else {
> -   dev->kdrv = RTE_PCI_KDRV_NONE;
> +   free(dev);
> return 0;
> }
> /* device is valid, add in list (sorted) */
>
>
> 1:
> http://patchwork.dpdk.org/project/dpdk/patch/20210614091213.3953-1-david.march...@redhat.com/
>
> --
> David Marchand
>
>

Re: [PATCH v1 0/4] [RFC] Testpmd RPC API

[Owen Hilyard]

>
> scheme is probably over-engineered


I tried my hardest to keep this as simple as possible. The requirements
imposed by DTS being a distributed system in Python restricted what I could
do a lot. Needing to be compatible with DPDK's license also got rid of a
lot of options. Binding generators are made for simple projects, and DPDK
is not a simple project. There were some other options related to choice in
the RPC framework, but very few RPC protocols seem to work well with C and
be usable from Python, which is why I ended up using C++ with gRPC. Most of
the code in api_impl.cc is taken from /app/test-acl/main.c, and the new
part is mostly the C++ class at the bottom. Overall, this proposal comes
out to ~100 lines of new C++, 9 lines of C, 12 lines of gRPC Protobuf and
100 lines of Meson. gRPC may be able to do a lot more than I need it to for
the proof of concept, but many of the features that are not used, like
bi-directional streaming, become very useful in writing more complicated
tests. Overall, this solution is probably more capable than we need it to
be, but I think that those extra capabilities don't come at a very large
cost.


> Now that, Test code is also part of DPDK.
>

DTS is pure python. I tried to use FFI to call directly into DPDK from
Python and then use xmlrpc from the python standard library. As mentioned
in the writeup, I couldn't find a binding generator that would properly
handle DPDK's allocators, which made it so that anything passed to DPDK
using python was allocated using the system malloc. I don't think it is
wise to attempt to programmatically re-write the generated code to allow
for custom allocators. The original reason for needing to have DTS and DPDK
in the same repository was so that tests could be committed and run
alongside the feature patch.

Interactive - Testpmd one, I believe, Feeding stdin programmatically would
> suffice to test all the combinations.
>

One of the issues this is trying to address is that human-readable strings
are a poor way to pass complex information between two programs. DTS is a
distributed system, and it can have up to 3 physical servers involved in
any given test. This means that it's not stdin via a pipe, it's an entire
SSH session. This adds a noticeable amount of overhead when trying to send
and verify the result of sending 1,000+ packets, since the lack of
structured output means each packet must be checked before the next can be
sent. This might be solvable by adding a structured output mode to testpmd,
but that would involve committing to writing output twice for every
function in testpmd forever.

We need to add all test cases in this model and we need to maintain two
> sets of programs.(Traditional tests and gRPC model-based tests).
>

Assuming by traditional tests you mean the unit tests run by Meson, I would
argue that we are already maintaining 2 kinds of tests. The unit tests, and
the python-based DTS tests. My intention is to create a thin wrapper around
DPDK that would be exposed via gRPC, like you see here, and use that as
midware. Then, we would have two front-ends. Testpmd, which takes text and
then calls midware as it does now, and the gRPC frontend, which parses
messages from the RPC server and runs the midware. This would enable
testpmd to still be used to sanity check a DPDK installation, but we would
not need to continually expand Testpmd. The primary issue is that, right
now, anything not included in Testpmd is not really testable by DTS. This
includes portions of the RTE Flow API, which was part of my reason for
proposing this. The RTE Flow API would, in my estimation, if fully
implemented into Testpmd, probably add at least another 10,000 lines of
code. As mentioned in my proposal, Testpmd already does more parsing and
lexing than it does interaction with DPDK by line count. Also, since I am
proposing making this a separate application, we would be able to gradually
migrate the tests inside of DTS. This would have no effect on anything
except for Testpmd, the new application and the addition of a flag to
toggle the use of a C++ compiler.

I'm not sure exactly what you mean by gRPC model-based tests. gRPC uses
classes to model services, but for this usecase we are essentially using it
to transfer function arguments across the internet and then pass the return
value back. Any RPC framework would function similarly if I ignored the
restrictions of which languages to use, and the choice is not important to
how tests are conducted. Put another way, how you write a test for DTS will
not change much if you are using this or testpmd, it's just how you
transfer data and get it back that I want to change.

Re: [RFC PATCH v1 00/15] merge DTS core files to DPDK

[Owen Hilyard]

>
>  3) In my private discussions with David Marchand, he expressed interest
> in getting the git log history. The current review process will not help in
> this regard. Is this a must? If yes, are there any known methods to do this?


The git docs point to https://github.com/newren/git-filter-repo/, since
apparently git filter-branch is full of footguns.  From there, I would
follow an example from Ruby,
https://gist.github.com/x-yuri/9890ab1079cf4357d6f269d073fd9731, which was
merging two reasonably-sized tooling repos. Instead of using git merge, we
would be using git rebase. I did it locally to try it out, and it does have
to replay the ENTIRE git history of dts. This means whoever does it may
want to grab a server and do the rebase in /run. This would also mean that
we would have to bypass the mailing list, since sending out more than 32000
emails doesn't seem reasonable. Possibly just a patch with the script to
run to do the rebase instead of putting all the patches on the mailing list?

Doing this would also probably mean all of our work would be in the DTS
repo until we are ready to merge. That might make it harder to review
changes to DTS in small parts.

Re: [PATCH v1 0/4] [RFC] Testpmd RPC API

[Owen Hilyard]

>
> If so, I think, gRPC service would be along with existing
> testpmd functions, like start_packet_forwarding().


It was my intention to re-use existing functions. I used the ACL tests as
an example because they are more self-contained then Testpmd, which made
creating the proof of concept much easier.

Also, We don't need to rewrite the existing testpmd, Instead, RPC service,
> we can add in existing app/test-pmd/
>

The reason that I split out the services is that there doesn't seem to be a
way to produce multiple binaries without re-writing that section of the
build system. I wanted to avoid the hard requirement of having a C++
compiler available in order to be able to use testpmd, since that may
affect what platforms Testpmd can run on and I want to avoid this being any
kind of breaking change. If we decide to go the route of putting it all in
a single application, we would need to conditionally enable the gRPC
service at build time. Meson's current lack of support for conditionally
detecting compilers causes issues here.

I think, DPDK has only one interactive test case which is testpmd,
>

Could you point me to that test case? Either invocation or source is ok. I
can't see anything that would lead me to assume use of testpmd in "meson
test --list". To my knowledge, all of the test cases that use testpmd are
in DTS. If there is a test that uses testpmd but is not part of DTS, I
think it would be a candidate for moving into DTS assuming it's not a unit
test.

How you are planning to do noninteractive test cases?


I'm not planning to make any change to unit testing, you can read more
about how testing is currently conducted here:
https://www.dpdk.org/blog/2021/07/05/dpdk-testing-approaches/

If there is a unit test that involves testpmd, there are two possibilities.
1. If we are making a separate application for Testpmd with the gRPC api,
then nothing changes except for possibly changing where some of the testpmd
source lives in order to enable code reuse between the two applications.
2. If gRPC is being added to Testpmd, then the unit test should still
function as it previously did if I do any necessary refactoring as
correctly.

I think, key should be leveraging existing test cases as much as possible
> and make easy for developers to add new test cases.


That is part of the reason why I want to be able to do this. Adding a new
test in DTS is very easy if the functionality needed already exists in
Testpmd. If the functionality does not exist, then adding the test becomes
difficult, due to the required modifications to the Testpmd lexer and
parser to accommodate the new command. My plan is to leave unit testing in
C, but help make it easier to expose C functions to Python for integration
testing. This gives us the best of both worlds in terms of access to DPDK
and the ability to use a high-level language to write the tests.

On Tue, Apr 12, 2022 at 2:07 AM Jerin Jacob  wrote:

> On Mon, Apr 11, 2022 at 11:19 PM Owen Hilyard 
> wrote:
> >>
> >> scheme is probably over-engineered
> >
> >
> > I tried my hardest to keep this as simple as possible. The requirements
> imposed by DTS being a distributed system in Python restricted what I could
> do a lot. Needing to be compatible with DPDK's license also got rid of a
> lot of options. Binding generators are made for simple projects, and DPDK
> is not a simple project. There were some other options related to choice in
> the RPC framework, but very few RPC protocols seem to work well with C and
> be usable from Python, which is why I ended up using C++ with gRPC. Most of
> the code in api_impl.cc is taken from /app/test-acl/main.c, and the new
> part is mostly the C++ class at the bottom. Overall, this proposal comes
> out to ~100 lines of new C++, 9 lines of C, 12 lines of gRPC Protobuf and
> 100 lines of Meson. gRPC may be able to do a lot more than I need it to for
> the proof of concept, but many of the features that are not used, like
> bi-directional streaming, become very useful in writing more complicated
> tests. Overall, this solution is probably more capable than we need it to
> be, but I think that those extra capabilities don't come at a very large
> cost.
>
>
> Now it is clear, I was carried away with the POC test application and
> I was not knowing existing DTS tests are based on python
>
> Is below a fair summary?
>
> 1) DPDK has interactive test cases and no interactive test cases.
>
> For The interactive test case like testpmd, I agree that we can enable
> RPC service via gRPC server in C++ as  and client in Python, and
> something along the lines of exposing the existing test-pmd command
> line function as service
> to avoid command line parsing and reuse the existing python test suite.
>
> If so, I think, gRPC service would be along

Re: [PATCH v1 0/4] [RFC] Testpmd RPC API

[Owen Hilyard]

>
>   Though I think we shouldn’t remove existing CLI interface.
>

I agree, it's a very useful debugging tool for validating environments. I
think having two "frontends", the CLI and API, which both consume one
"backend" testpmd library would be the easiest way to go about doing that
while keeping long-term maintenance low.

Conditional compilation (new meson flag or so) is probably good enough for
> this case.
>

One of the changes I made was an on-by-default meson flag to enable C++
compilation. If that flag is on, and all dependencies are present, then the
application will be built.

Would it be possible to try implement something more realistic with testpmd
> itself


I would consider it a "phase 2" version of this RFC. The hard part was
getting gRPC working inside of Meson, which is why I picked a simple app to
port. If this RFC moves forward, I can look at porting the functionality
needed for the nic single core performance test (
http://git.dpdk.org/tools/dts/tree/test_plans/nic_single_core_perf_test_plan.rst
).

On Thu, Apr 14, 2022 at 8:08 AM Ananyev, Konstantin <
konstantin.anan...@intel.com> wrote:

>
> Hi everyone,
>
> First of all thanks Owen for stepping forward with this RFC.
> Few thoughts on this subject below.
> Konstantin
>
> > -Original Message-
> > From: Ananyev, Konstantin 
> > Sent: Thursday, April 14, 2022 12:59 PM
> > To: Ananyev, Konstantin 
> > Subject: FW: [PATCH v1 0/4] [RFC] Testpmd RPC API
> >
> >
> >
> > From: Owen Hilyard 
> > Sent: Wednesday, April 13, 2022 1:47 PM
> > To: Jerin Jacob 
> > Cc: dpdk-dev ; Honnappa Nagarahalli <
> honnappa.nagaraha...@arm.com>; Thomas Monjalon 
> > Subject: Re: [PATCH v1 0/4] [RFC] Testpmd RPC API
> >
> > If so, I think, gRPC service would be along with existing testpmd
> functions, like start_packet_forwarding().
> >
> > It was my intention to re-use existing functions. I used the ACL tests
> as an example because they are more self-contained then Testpmd,
> > which made creating the proof of concept much easier.
> >
> > Also, We don't need to rewrite the existing testpmd, Instead, RPC
> service, we can add in existing app/test-pmd/
> >
> > The reason that I split out the services is that there doesn't seem to
> be a way to produce multiple binaries without re-writing that section of
> > the build system. I wanted to avoid the hard requirement of having a C++
> compiler available in order to be able to use testpmd, since that
> > may affect what platforms Testpmd can run on and I want to avoid this
> being any kind of breaking change. If we decide to go the route of
> > putting it all in a single application, we would need to conditionally
> enable the gRPC service at build time. Meson's current lack of support
> > for conditionally detecting compilers causes issues here.
> >
> > I think, DPDK has only one interactive test case which is testpmd,
> >
> > Could you point me to that test case? Either invocation or source is ok.
> I can't see anything that would lead me to assume use of testpmd in
> > "meson test --list". To my knowledge, all of the test cases that use
> testpmd are in DTS. If there is a test that uses testpmd but is not part of
> > DTS, I think it would be a candidate for moving into DTS assuming it's
> not a unit test.
> >
> > How you are planning to do noninteractive test cases?
> >
> > I'm not planning to make any change to unit testing, you can read more
> about how testing is currently conducted
> > here: https://www.dpdk.org/blog/2021/07/05/dpdk-testing-approaches/
> >
> > If there is a unit test that involves testpmd, there are two
> possibilities.
> > 1. If we are making a separate application for Testpmd with the gRPC
> api, then nothing changes except for possibly changing where some
> > of the testpmd source lives in order to enable code reuse between the
> two applications.
> > 2. If gRPC is being added to Testpmd, then the unit test should still
> function as it previously did if I do any necessary refactoring as
> correctly.
> >
> > I think, key should be leveraging existing test cases as much as
> possible and make easy for developers to add new test cases.
> >
> > That is part of the reason why I want to be able to do this. Adding a
> new test in DTS is very easy if the functionality needed already exists in
> > Testpmd. If the functionality does not exist, then adding the test
> becomes difficult, due to the required modifications to the Testpmd lexer
> > and parser to accommodate the new command. My plan is to leave unit
> testin

[dpdk-dev] Multicast MAC Filter Test Case

[Owen Hilyard]

Hello Everyone,

I wanted to see if anyone had any suggestions regarding testing the
multicast mac filter. My current plan is to send a frame and check it's not
received, then add the multicast address to the filter and send another
frame, checking that it is received. Suggestions of other test cases for
this feature are also welcome.

Thanks for your help,
Owen Hilyard

[dpdk-dev] Hardware Checksum Checks Offload Feature

[Owen Hilyard]

Hello,

To my understanding, this feature is the ability of the driver to
offload checksum verification on received packets to the hardware
level. If that is incorrect, then please let me know. My plan for
testing this feature is as follows;

This feature will be verified by a series of test cases sharing a
single helper method. It will be located inside of the
“checksum_offload” test suite. The test case names will follow the
pattern of test_hardware_checksum_check__.
Each test case  will send packets with a L3/L4 combination, the
complete list being IP/UDP, IP/TCP, IP/SCTP, IPv6/UDP and IPv6/TCP.
Each packet will contain a payload of 50 bytes of 0x58. Each test case
will consist of first enabling verbosity level 1 on the dut’s testpmd
instance. This will cause testpmd to display good/bad checksum flags.
After that, packet forwarding will be started. Then, a packet with a
checksum will be sent to the dut and the output from testpmd will be
checked to ensure that the flags are correct. Next, a packet will be
sent which intentionally has a bad checksum (0xF). In the case of
packets using IPv4, both the L3 and L4 checksums will be set to 0xF.
The flags will then be checked for the correct flags, in this case bad
checksum flags.

I decided to separate out the test cases instead of doing it like the
other ones in that area of the test suite because I noticed that a NIC
doesn't necessarily need to support offloading all checksum types, and
if I wrote the tests in the same way as the other ones in the test, it
would fail everything if the first protocol wasn't supported, rather
than failing only that protocol. I thought that the solution of having
more test cases, although it would lead to slower test times and more
verbose output, would be beneficial as it would allow for more
granular pass/fail results. The helper method for the tests would go
something like this:

1. Start TestPmd
2. Enable hardware checksum
3. Fill in template parameters in the strings provided by the test
method with mac addresses and packet options.
4. Send a packet with a bad checksum, and then check for the flags
which mean invalid checksum.
5. Send a packet with a good checksum, and then check for the flags
which mean valid checksum.

Please let me know if you think any part of this methodology is flawed
or if there are certain things I should be aware of such as a special
way to enable these checks aside from the checksum aside from
TestChecksumOffload::checksum_enablehw.

Thanks,
Owen Hilyard
Software Developer
UNH Interoperability Lab

Re: [dpdk-dev] Hardware Checksum Checks Offload Feature

[Owen Hilyard]

Hello,

In regards to the outer layers, having grepped through the code for
"[\w_]+_GOOD|[\w_]+_BAD", I wasn't able to find the flags that I expected.
I expected something like PKT_RX_OUTER_IP_CKSUM_BAD and
PKT_RX_OUTER_IP_CKSUM_GOOD to show up since that seems to be the format for
flags to be printed, but there wasn't anything in the grep output related
to that. Am I missing something? I can do outer UDP fairly easily, but it
doesn't look like there is support for outer TCP and outer SCTP in TestPmd.
I will plan to add tests

> > I decided to separate out the test cases instead of doing it like the
> > other ones in that area of the test suite because I noticed that a NIC
> > doesn't necessarily need to support offloading all checksum types, and
> > if I wrote the tests in the same way as the other ones in the test, it
> > would fail everything if the first protocol wasn't supported,
>
> No, if it is not supported, the result must have a special value
"UNKNOWN".

To clarify my meaning there, I mean that the verify statements in the test
case will abort the test on the first failed statement, so I am splitting
the tests up so that I don't need to collect all of the errors and then
spit them all out at once. Also, as far as I can tell, unknown only occurs
when it is not possible to decode a layer. The NIC I am testing with
doesn't support offloading outer udp, and TestPmd gives me an error message
and then leaves the option set to software mode. When I send a packet
without any tunneling, then it gives me something like
"PKT_RX_OUTER_L4_CKSUM_UNKNOWN". Is this assessment incorrect?

>  I think you should describe all the protocols you want to test.

Could you please elaborate on this?

Thanks for your feedback

Re: [dpdk-dev] Hardware Checksum Checks Offload Feature

[Owen Hilyard]

I can do that.

Thanks for the clarification

On Thu, Jun 25, 2020 at 11:25 AM Thomas Monjalon 
wrote:

> > >  I think you should describe all the protocols you want to test.
> >
> > Could you please elaborate on this?
>
> I mean doing a test matrix inluding IP, TCP, UDP, VXLAN, GENEVE, etc.
>
>
>

Re: [dpdk-dev] Hardware Checksum Checks Offload Feature

[Owen Hilyard]

It seems that GENEVE is not supported in the version of scapy that is
currently used. It is supported in the next version. I didn't want to make
the decision to either force an update, spend time attempting to backport
the protocol and then adding a way to automatically add that patch onto an
existing version, or drop the protocol from the test matrix without
community input.

Thoughts?

On Thu, Jun 25, 2020 at 11:54 AM Owen Hilyard  wrote:

> I can do that.
>
> Thanks for the clarification
>
> On Thu, Jun 25, 2020 at 11:25 AM Thomas Monjalon 
> wrote:
>
>> > >  I think you should describe all the protocols you want to test.
>> >
>> > Could you please elaborate on this?
>>
>> I mean doing a test matrix inluding IP, TCP, UDP, VXLAN, GENEVE, etc.
>>
>>
>>

[dpdk-dev] Fwd: Request for High Priority Features

[Owen Hilyard]

-- Forwarded message -
From: Owen Hilyard 
Date: Tue, Jul 28, 2020 at 8:04 AM
Subject: Request for High Priority Features
To: 


Hello Everyone,

Those of us at the IOL have been working on new DTS test cases for a while
now, and we are starting to get to the point where we've completed most of
the simpler test cases we are aware of. The remaining features are ones we
have internally agreed are either not supported by most NICS (Table 1.1
<http://doc.dpdk.org/guides/nics/overview.html>) or are features that we
think will require a large time investment to complete. This large time
investment may be due to the complexity required of a test which properly
checks different scenarios, a lack of knowledge about specific parts of the
spec we are testing against, or a lack of support in dpdk for the feature.
We don't want to spend a large amount of time focusing on a feature which
isn't important, and as such we are requesting that the community send us a
list of high priority features. We don't need a consensus before these are
sent to us, since we can look at multiple people's lists and find the
important ones from the aggregate.

The current state of our development progress can be found in the attached
google sheet.
 DPDK Test Cases Status List
<https://docs.google.com/spreadsheets/d/1xeGQZdt6R3FrxMUSfbLRMKObDs7yGEODuXSLw4OjSco/edit?usp=drive_web>


Thanks for your assistance,
The IOL Team

[dpdk-dev] Userspace testing

[Owen Hilyard]

Hello all,

I was wondering what everyone's thoughts on doing both userspace testing
and unprivileged testing of dpdk applications is. DTS currently runs all
commands on the tester and the dut as the root user. Please correct me if
I'm wrong, but I was under the assumption that most applications written
with dpdk would not run as root. This could present a problem since it is
possible that permissions errors could arise and we wouldn't notice it due
to the way we currently test. Given that, I was wondering what should and
should not be possible as a normal (non-root) user, and what would be the
best way to go about verifying this.

Thanks

Re: [dpdk-dev] Userspace testing

[Owen Hilyard]

Thanks for the advice.

I was wondering about the state of the "Setup VFIO permissions" option in
the setup script. It seems to just modify the character device's
permissions and then check their memory limit. Should this option also
handle the hugepages setup?

Thanks

On Wed, Jul 29, 2020 at 11:35 AM Burakov, Anatoly 
wrote:

> On 29-Jul-20 3:34 PM, Owen Hilyard wrote:
> > Hello all,
> >
> > I was wondering what everyone's thoughts on doing both userspace testing
> > and unprivileged testing of dpdk applications is. DTS currently runs all
> > commands on the tester and the dut as the root user. Please correct me if
> > I'm wrong, but I was under the assumption that most applications written
> > with dpdk would not run as root. This could present a problem since it is
> > possible that permissions errors could arise and we wouldn't notice it
> due
> > to the way we currently test. Given that, I was wondering what should and
> > should not be possible as a normal (non-root) user, and what would be the
> > best way to go about verifying this.
> >
> > Thanks
> >
>
> This is useful, but not everything is supposed to work with limited
> privileges. Things that definitely *won't* work are KNI and anything
> igb_uio-related. Everything VFIO should work fine, and setting up
> correct permissions for hugepages and VFIO is one of the trickier things
> that even I don't know how to do correctly off the top of my head :D
>
> An easy stopgap way of running almost everything as an unprivileged user
> is to use in-memory mode (--in-memory EAL switch); this will cause EAL
> to reserve hugepages etc. without touching the filesystem, sacrificing
> secondary process support in the process (so e.g. EAL autotest won't
> work in --in-memory mode as it relies on secondary process support).
>
> So, i would say that it would be a valuable thing to test for, but be
> aware that not everything is expected to work.
>
> --
> Thanks,
> Anatoly
>

Re: [dpdk-dev] Userspace testing

[Owen Hilyard]

On Fri, Jul 31, 2020 at 5:12 AM Burakov, Anatoly 
wrote:

> On 30-Jul-20 5:54 PM, Owen Hilyard wrote:
> > Thanks for the advice.
> >
> > I was wondering about the state of the "Setup VFIO permissions" option
> > in the setup script. It seems to just modify the character device's
> > permissions and then check their memory limit. Should this option also
> > handle the hugepages setup?
>
> I was under the (mis?)impression that the hugepage setup part of the
> script did that?
>
>
It doesn't appear to set them up so that non-root users can use them. From
what I can tell it only creates the pages, but doesn't change permissions
on them in any way.

[dpdk-dev] Rx interrupt testing

[Owen Hilyard]

Hello all,

I've been looking at creating a test suite for the Rx interrupt feature,
but I haven't been able to find a good way to interface with it and double
check that the interrupts are handled correctly. I was wondering if there's
an option I'm missing somewhere or if I should submit a ticket to get a
feature like that added into testpmd. I'd also appreciate any ideas for
test cases.

Thanks
Owen Hilyard

Re: [RFC PATCH] add rust binding support to DPDK

[Owen Hilyard]

Hello all,

I wanted to chime in as someone who uses DPDK from Rust in quite a few projects.

No snips so the conversation can continue past me for things I don't comment on.
Hello Harry,

 My concern is how to properly maintain Rust crate once DPDK starts to 
 implement
 it's own API.
>>>
>>> I'm not really sure what is meant here. I don't understand what "own" word 
>>> refers to?
>>>
>>> I see it like this:
>>> - DPDK has the public C API exported (and that stays the same as today, 
>>> with ABI rules, version.map files, etc)
>>> - The Rust crate consumes the public C API (via bindgen, as done in this 
>>> patch. More detail about bindgen below.)
>>>
>>
>> Bindgen cannot provide access to all DPDK public API.
>
> Ah - you're referring to C static inline functions, declared in header files, 
> which bindgen doesn't wrap.
> Correct - thanks - I understand your point now.
>
>> A good example here is rte_eth_rx_burst().
>> That function is defined as inline and bindgen does not translate it.
>> Also, the function definition references rte_eth_fp_ops array that is not 
>> part of the
>> public DPDK API. That means Rust cannot duplicate rte_eth_rx_burst() "as-is" 
>> and
>> the solution can require extensions to existing DPDK API.
>>
>> I added a new public API that exports rte_eth_fp_ops for a given port Id.
>>
>> Rust implementation of rte_eth_rx_burst() does not have to follow the 
>> original
>> approach.
>> Usage of rte_eth_fp_ops is good for C, but Rust has different methods.
>
> Agreed there is a certain "mismatch" sometimes, if functions aren't in the
> actually "C ABI" then they can't be called via bindgen.
>
> Agree that elegant solutions (clean, maintainable, and high performance) will 
> have
> to be found here. Many existing solutions just wrap the "static inline" 
> function into
> a "non-static" function, and export it as a public symbol. That allows 
> calling into it
> from Rust (via bindgen-generated header) however causes an actual function 
> call..
> (LTO _might_ fix/inline it, but not everybody compiles with LTO.. link times!)
>

The core DPDK code is maintained as a self-contained pure C project.
What about extending that model and provide direct access to DPDK resources that
are needed for Rust API ?
That can be arranged as part of "native" DPDK API or as extension for
Rust-enabled DPDK only.
I'm currently experimenting with the latter in
https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fgetelson-at-mellanox%2Frdpdk%2Ftree%2Fmain%2Fdpdk-patches&data=05%7C02%7Cowen.hilyard%40unh.edu%7C59c29799f1544a59f44108dd7a62352a%7Cd6241893512d46dc8d2bbe47e25f5666%7C0%7C0%7C638801284429834568%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=3YrOkklWRWlwKyfg9BblBnLsNqnCEf7RdAxdUW3JGgM%3D&reserved=0
There are some performance wins to be found along this path. It's not a lot, 
but you can "devirtualize" a lot of DPDK if you use Rust's generics to 
specialize on particular hardware combinations, at the cost of multiplying code 
size by the number of supported NICs. I can do this in my bindings since I tend 
to only need to test a few software PMDs and the hardware I have, with a 
fallback to "dyn dpdk::EthDev" which pulls data from info structs all of the 
time, instead of only pulling runtime-only information (ex: mac addr, rss key). 
This is paired with a top-level function which does dispatch for different 
hardware to "lift" the runtime information about what NIC is used to compile 
time. I think the main win here is from inlining driver functions, but I 
haven't done a detailed analysis of the "why". These are marginal improvements, 
around a half percent for nic_single_core_perf on my hardware, but there may be 
more wins for less heavily optimized paths through DPDK where the compiler can 
do more heavy lifting.
> As DPDK uses static-inline functions primarily for "packet-at-a-time" 
> performance reasons,
> it is unfortunate to give-up (some small amounts of..?) performance by having 
> a C->Rust FFI call.
> We have work to do to find/propose the best solution.

What exactly is a small amount of performance degradation ?
For some existing performance oriented projects loosing performance to
eliminate the `unsafe {}` construct is not an option.
If you are willing to compile with clang and use LTO, both rustc and clang will 
emit LLVM IR, which gets inlined at link time. I haven't been able to measure a 
performance difference vs C code even when calling "static inline" hot path 
function wrappers across the FFI boundary. This does increase compile times by 
quite a bit, more than LTO with pure C would increase them due to the amount of 
LLVM IR that rustc emits, but it means that the performance difference barely 
exists if it does at all.
>
>> For conclusion, Rust DPDK infrastructure cannot relay on b

Re: [PATCH] rust: RFC/demo of safe API for Dpdk Eal, Eth and Rxq

[Owen Hilyard]

From: Van Haaren, Harry 
Sent: Friday, May 2, 2025 9:58 AM
To: Etelson, Gregory ; Richardson, Bruce 

Cc: dev@dpdk.org ; Owen Hilyard 
Subject: Re: [PATCH] rust: RFC/demo of safe API for Dpdk Eal, Eth and Rxq

> From: Etelson, Gregory
> Sent: Friday, May 02, 2025 1:46 PM
> To: Richardson, Bruce
> Cc: Gregory Etelson; Van Haaren, Harry; dev@dpdk.org; owen.hily...@unh.edu
> Subject: Re: [PATCH] rust: RFC/demo of safe API for Dpdk Eal, Eth and Rxq
>
> Hello Bruce,

Hi All,
Hi All,

> > Thanks for sharing. However, IMHO using EAL for thread management in rust
> > is the wrong interface to expose.
>
> EAL is a singleton object in DPDK architecture.
> I see it as a hub for other resources.

Yep, i tend to agree here; EAL is central to the rest of DPDK working correctly.
And given EALs implementation is heavily relying on global static variables, it 
is
certainly a "singleton" instance, yes.
I think a singleton one way to implement this, but then you lose some of the 
RAII/automatic resource management behavior. It would, however, make some APIs 
inherently unsafe or very unergonomic unless we were to force rte_eal_cleanup 
to be run via atexit(3) or the platform equivalent and forbid the user from 
running it themselves. For a lot of Rust runtimes similar to the EAL (tokio, 
glommio, etc), once you spawn a runtime it's around until process exit. The 
other option is to have a handle which represents the state of the EAL on the 
Rust side and runs rte_eal_init on creation and rte_eal_cleanup on destruction. 
There are two ways we can make that safe. First, reference counting, once the 
handles are created, they can be passed around easily, and the last one runs 
rte_eal_cleanup when it gets dropped.  This avoids having tons of complicated 
lifetimes and I think that, everywhere that it shouldn't affect fast path 
performance, we should use refcounting. The other option is to use lifetimes. 
This is doable, but is going to force people who are more likely to primarily 
be C or C++ developers to dive deep into Rust's type system if they want to 
build abstractions over it. If we add async into the mix, as many people are 
going to want to do, it's going to become much, much harder. As a result, I'd 
advocate for only using it for data path components where refcounting isn't an 
option.

> Following that idea, the EAL structure can be divided to hold the
> "original" resources inherited from librte_eal and new resources
> introduced in Rust EAL.

Here we can look from different perspectives. Should "Rust EAL" even exist?
If so, why? The DPDK C APIs were designed in baremetal/linux days, where
certain "best-practices" didn't exist yet, and Rust language was pre 1.0 
release.

Of course, certain parts of Rust API must depend on EAL being initialized.
There is a logical flow to DPDK initialization, these must be kept for correct 
functionality.

I guess I'm saying, perhaps we can do better than mirroring the concept of
"DPDK EAL in C" in to "DPDK EAL in Rust".

I think that there will need to be some kind of runtime exposed by the library. 
A lot of the existing EAL abstractions may need to be reworked, especially 
those dealing with memory, but I think a lot of things can be layered on top of 
the C API. However, I think many of the invariants in the EAL could be enforced 
at compile time for free, which may mean the creation of a lot of "unchecked" 
function variants which skip over null checks and other validation.

As was mentioned before, it may also make sense for some abstractions in the C 
EAL to be lifted to compile time. I've spent a lot of time thinking about how 
to use something like Rust's traits for "it just works" capabilities where you 
can declare what features you want (ex: scatter/gather) and it will either be 
done in hardware or fall back to software, since you were going to need to do 
it anyway. This might lead to parameterizing a lot of user code on the devices 
they expect to interact with and then having some "dyn EthDev" as a fallback, 
which should be roughly equivalent to what we have now. I can explain that in 
more detail if there's interest.

> > Instead, I believe we should be
> > encouraging native rust thread management, and not exposing any DPDK
> > threading APIs except those necessary to have rust threads work with DPDK,
> > i.e. with an lcore ID. Many years ago when DPDK started, and in the C
> > world, having DPDK as a runtime environment made sense, but times have
> > changed and for Rust, there is a whole ecosystem out there already that we
> > need to "play nice with", so having Rust (not DPDK) do all thread
> > management is the way to go (again IMHO).
> >
>
> I'm not sure what exposed DPDK API you refer to

Re: [PATCH] rust: RFC/demo of safe API for Dpdk Eal, Eth and Rxq

[Owen Hilyard]

> ‎From: Van Haaren, Harry 
> Sent: Friday, May 9, 2025 12:24 PM
> To: Owen Hilyard ; Etelson, Gregory 
> ; Richardson, Bruce 
> Cc: dev@dpdk.org 
> Subject: Re: [PATCH] rust: RFC/demo of safe API for Dpdk Eal, Eth and Rxq
> 
> > From: Owen Hilyard
> > Sent: Friday, May 09, 2025 12:53 AM
> > To: Van Haaren, Harry; Etelson, Gregory; Richardson, Bruce
> > Cc: dev@dpdk.org
> > Subject: Re: [PATCH] rust: RFC/demo of safe API for Dpdk Eal, Eth and Rxq
> >


> > > Maybe it will help to split the conversation into two threads, with one 
> > > focussing on
> > "DPDK used through Safe Rust abstractions", and the other on "future cool 
> > use-cases".
> >
> > Agree.
> > 
> > > Perhaps I jumped a bit too far ahead mentioning async runtimes, and while 
> > > I like the enthusiasm for designing "cool new stuff", it is probably 
> > > better to be realistic around what will get "done": my bad.
> > >
> > > I'll reply to the "DPDK via Safe Rust" topics below, and start a new 
> > > thread (with same folks on CC) for "future cool use-cases" when I've had 
> > > a chance to clean up a little demo to showcase them.
> > > 
> > >
> > > > > > Thanks for sharing. However, IMHO using EAL for thread management 
> > > > > > in rust
> > > > > > is the wrong interface to expose.
> > > > >
> > > > > EAL is a singleton object in DPDK architecture.
> > > > > I see it as a hub for other resources.
> > >
> > > Harry Wrote:
> > > > Yep, i tend to agree here; EAL is central to the rest of DPDK working 
> > > > correctly.
> > > > And given EALs implementation is heavily relying on global static 
> > > > variables, it is
> > > > certainly a "singleton" instance, yes.
> > >
> > > Owen wrote:
> > > > I think a singleton one way to implement this, but then you lose some 
> > > > of the RAII/automatic resource management behavior. It would, however, 
> > > > make some APIs inherently unsafe or very unergonomic unless we were to 
> > > > force rte_eal_cleanup to be run via atexit(3) or the platform 
> > > > equivalent and forbid the user from running it themselves. For a lot of 
> > > > Rust runtimes similar to the EAL (tokio, glommio, etc), once you spawn 
> > > > a runtime it's around until process exit. The other option is to have a 
> > > > handle which represents the state of the EAL on the Rust side and runs 
> > > > rte_eal_init on creation and rte_eal_cleanup on destruction. There are 
> > > > two ways we can make that safe. First, reference counting, once the 
> > > > handles are created, they can be passed around easily, and the last one 
> > > > runs rte_eal_cleanup when it gets dropped.  This avoids having tons of 
> > > > complicated lifetimes and I think that, everywhere that it shouldn't 
> > > > affect fast path performance, we should use refcounting.
> > >
> > > Agreed, refcounts for EAL "singleton" concept yes. For the record, the 
> > > initial patch actually returns a
> > "dpdk" object from dpdk::Eal::init(), and Drop impl has a // TODO 
> > rte_eal_cleanup(), so well aligned on approach here.
> > > https://patches.dpdk.org/project/dpdk/patch/20250418132324.4085336-1-harry.van.haa...@intel.com/
> >
> > One thing I think I'd like to see is using a "newtype" for important 
> > numbers (ex: "struct EthDevQueueId(pub u16)"). This prevents some classes 
> > of error but if we make the constructor public it's at most a minor 
> > inconvenience to anyone who has to do something a bit odd.
> >
> > > > Owen wrote:
> > > The other option is to use lifetimes. This is doable, but is going to 
> > > force people who are more likely to primarily be C or C++ developers to 
> > > dive deep into Rust's type system if they want to build abstractions over 
> > > it. If we add async into the mix, as many people are going to want to do, 
> > > it's going to become much, much harder. As a result, I'd advocate for 
> > > only using it for data path components where refcounting isn't an option.
> > >
> > > +1 to not using lifetimes here, it is not the right solution for this EAL 
> > > / singleton type problem.
> >
> > Having now looked over the initial patchset in m

Re: [PATCH] rust: RFC/demo of safe API for Dpdk Eal, Eth and Rxq

[Owen Hilyard]

> ‎From: Van Haaren, Harry 
> Sent: Tuesday, May 6, 2025 12:39 PM
> To: Owen Hilyard ; Etelson, Gregory 
> ; Richardson, Bruce 
> Cc: dev@dpdk.org 
> Subject: Re: [PATCH] rust: RFC/demo of safe API for Dpdk Eal, Eth and Rxq

> > From: Owen Hilyard
> > Sent: Saturday, May 03, 2025 6:13 PM
> > To: Van Haaren, Harry; Etelson, Gregory; Richardson, Bruce
> > Cc: dev@dpdk.org
> > Subject: Re: [PATCH] rust: RFC/demo of safe API for Dpdk Eal, Eth and Rxq
> >
> > From: Van Haaren, Harry 
> > Sent: Friday, May 2, 2025 9:58 AM
> > To: Etelson, Gregory ; Richardson, Bruce 
> > 
> > Cc: dev@dpdk.org ; Owen Hilyard 
> > Subject: Re: [PATCH] rust: RFC/demo of safe API for Dpdk Eal, Eth and Rxq
> >
> > > From: Etelson, Gregory
> > > Sent: Friday, May 02, 2025 1:46 PM
> > > To: Richardson, Bruce
> > > Cc: Gregory Etelson; Van Haaren, Harry; dev@dpdk.org; owen.hily...@unh.edu
> > > Subject: Re: [PATCH] rust: RFC/demo of safe API for Dpdk Eal, Eth and Rxq
> > >
> > > Hello Bruce,
> >
> > Hi All,
> > Hi All,
>
> Hi All!
>
> Great to see passionate & detailed replies & input!
>
> Please folks - lets try remember to send plain-text emails, and use  >  to 
> indent each reply.
>Its hard to identify what I wrote (1) compared to Owen's replies (2) in the 
>archives otherwise.
> (Adding some "Harry wrote" and "Owen wrote" annotations to try help future 
> readability.)

My apologies, I'll be more careful with that.

> Maybe it will help to split the conversation into two threads, with one 
> focussing on
"DPDK used through Safe Rust abstractions", and the other on "future cool 
use-cases".

Agree. 

> Perhaps I jumped a bit too far ahead mentioning async runtimes, and while I 
> like the enthusiasm for designing "cool new stuff", it is probably better to 
> be realistic around what will get "done": my bad.
> 
> I'll reply to the "DPDK via Safe Rust" topics below, and start a new thread 
> (with same folks on CC) for "future cool use-cases" when I've had a chance to 
> clean up a little demo to showcase them.
>
>
> > > > Thanks for sharing. However, IMHO using EAL for thread management in 
> > > > rust
> > > > is the wrong interface to expose.
> > >
> > > EAL is a singleton object in DPDK architecture.
> > > I see it as a hub for other resources.
>
> Harry Wrote:
> > Yep, i tend to agree here; EAL is central to the rest of DPDK working 
> > correctly.
> > And given EALs implementation is heavily relying on global static 
> > variables, it is
> > certainly a "singleton" instance, yes.
> 
> Owen wrote:
> > I think a singleton one way to implement this, but then you lose some of 
> > the RAII/automatic resource management behavior. It would, however, make 
> > some APIs inherently unsafe or very unergonomic unless we were to force 
> > rte_eal_cleanup to be run via atexit(3) or the platform equivalent and 
> > forbid the user from running it themselves. For a lot of Rust runtimes 
> > similar to the EAL (tokio, glommio, etc), once you spawn a runtime it's 
> > around until process exit. The other option is to have a handle which 
> > represents the state of the EAL on the Rust side and runs rte_eal_init on 
> > creation and rte_eal_cleanup on destruction. There are two ways we can make 
> > that safe. First, reference counting, once the handles are created, they 
> > can be passed around easily, and the last one runs rte_eal_cleanup when it 
> > gets dropped.  This avoids having tons of complicated lifetimes and I think 
> > that, everywhere that it shouldn't affect fast path performance, we should 
> > use refcounting.
>
> Agreed, refcounts for EAL "singleton" concept yes. For the record, the 
> initial patch actually returns a
"dpdk" object from dpdk::Eal::init(), and Drop impl has a // TODO 
rte_eal_cleanup(), so well aligned on approach here.
> https://patches.dpdk.org/project/dpdk/patch/20250418132324.4085336-1-harry.van.haa...@intel.com/

One thing I think I'd like to see is using a "newtype" for important numbers 
(ex: "struct EthDevQueueId(pub u16)"). This prevents some classes of error but 
if we make the constructor public it's at most a minor inconvenience to anyone 
who has to do something a bit odd. 

> > Owen wrote:
> > The other option is to use lifetimes. This is doable, but is going to force 
> > people who are more likely to primarily be C or C++ developers to dive deep 
> >