dts: improve documentation

  Improve instructions for installing dependencies, configuring and
launching the project. Finally, document the configuration schema
by adding more comments to the example and documenting every
property and definition.

Reviewed-by: Paul Szczepanek <Paul.Szczepanek@arm.com>
Signed-off-by: Luca Vizzarro <Luca.Vizzarro@arm.com>
---
 .mailmap                 |   1 +
 doc/guides/tools/dts.rst | 255 +++++++++++++++++++++++++++++++++------
 dts/conf.yaml            |  31 +++--
 3 files changed, 240 insertions(+), 47 deletions(-)
  

03/01/2024 13:54, Luca Vizzarro:
> Improve instructions for installing dependencies, configuring and
> launching the project. Finally, document the configuration schema
> by adding more comments to the example and documenting every
> property and definition.

Thank you for taking care of the documentation.

> +Luca Vizzarro <Luca.Vizzarro@arm.com>

For consistency, we don't use uppercase characters in email addresses.


> -      poetry install
> +      poetry install --no-root

Please could you explain this change in the commit log?


>  DTS needs to know which nodes to connect to and what hardware to use on those nodes.
> -Once that's configured, DTS needs a DPDK tarball and it's ready to run.
> +Once that's configured, DTS needs a DPDK tarball or a git ref ID and it's ready to run.

That's assuming DTS is compiling DPDK.
We may want to provide an already compiled DPDK to DTS.


> -   usage: main.py [-h] [--config-file CONFIG_FILE] [--output-dir OUTPUT_DIR] [-t TIMEOUT]
> -                  [-v VERBOSE] [-s SKIP_SETUP] [--tarball TARBALL]
> -                  [--compile-timeout COMPILE_TIMEOUT] [--test-cases TEST_CASES]
> -                  [--re-run RE_RUN]
> +   (dts-py3.10) $ ./main.py --help

Why adding this line?
Should we remove the shell prefix referring to a specific Python version?

> +   usage: main.py [-h] [--config-file CONFIG_FILE] [--output-dir OUTPUT_DIR] [-t TIMEOUT] [-v VERBOSE]
> +                  [-s SKIP_SETUP] [--tarball TARBALL] [--compile-timeout COMPILE_TIMEOUT]
> +                  [--test-cases TEST_CASES] [--re-run RE_RUN]
>  
> -   Run DPDK test suites. All options may be specified with the environment variables provided in
> -   brackets. Command line arguments have higher priority.
> +   Run DPDK test suites. All options may be specified with the environment variables provided in brackets.

In general it is better to avoid long lines, and split after a punctation.
I think we should take the habit to always go to the next line after the end of a sentence.


> -                           [DTS_OUTPUT_DIR] Output directory where dts logs and results are
> -                           saved. (default: output)
> +                           [DTS_OUTPUT_DIR] Output directory where dts logs and results are saved.

dts -> DTS


> +Configuration Schema
> +--------------------
> +
> +Definitions
> +~~~~~~~~~~~
> +
> +_`Node name`
> +   *string* – A unique identifier for a node. **Examples**: ``SUT1``, ``TG1``.
> +
> +_`ARCH`
> +   *string* – The CPU architecture. **Supported values**: ``x86_64``, ``arm64``, ``ppc64le``.
> +
> +_`CPU`
> +   *string* – The CPU microarchitecture. Use ``native`` for x86. **Supported values**: ``native``, ``armv8a``, ``dpaa2``, ``thunderx``, ``xgene1``.
> +
> +_`OS`
> +   *string* – The operating system. **Supported values**: ``linux``.
> +
> +_`Compiler`
> +   *string* – The compiler used for building DPDK. **Supported values**: ``gcc``, ``clang``, ``icc``, ``mscv``.
> +
> +_`Build target`
> +   *object* – Build targets supported by DTS for building DPDK, described as:
> +
> +   ==================== =========================================================================================
> +   ``arch``             See `ARCH`_
> +   ``os``               See `OS`_
> +   ``cpu``              See `CPU`_
> +   ``compiler``         See `Compiler`_
> +   ``compiler_wrapper`` *string* – Value prepended to the CC variable for the DPDK build.

Please don't add compilation configuration for now,
I would like to work on the schema first.
This is mostly imported from the old DTS and needs to be rethink.
  

On 04/01/2024 10:52, Thomas Monjalon wrote:
>>   DTS needs to know which nodes to connect to and what hardware to use on those nodes.
>> -Once that's configured, DTS needs a DPDK tarball and it's ready to run.
>> +Once that's configured, DTS needs a DPDK tarball or a git ref ID and it's ready to run.
> 
> That's assuming DTS is compiling DPDK.
> We may want to provide an already compiled DPDK to DTS.

Yes, that is correct. At the current state, DTS is always compiled from 
source though, so it may be reasonable to leave it as it is until this
feature may be implemented. Nonetheless, my change just informs the user
of the (already implemented) feature that uses `git archive` from the 
local repository to create a tarball. A sensible change would be to add
this explanation I have just given, but it is a technicality and it 
won't really make a difference to the user.

>> +   (dts-py3.10) $ ./main.py --help
> 
> Why adding this line?

Just running `./main.py` will just throw a confusing error to the user. 
I am in the process of sorting this out as it is misleading and not 
helpful. Specifying the line in this case just hints to the user on the 
origin of that help/usage document.

> Should we remove the shell prefix referring to a specific Python version?

I have purposely left the prefix to indicate that we are in a Poetry 
shell environment, as that is a pre-requisite to run DTS. So more of an 
implicit reminder. The Python version specified is in line with the 
minimum requirement of DTS.

> In general it is better to avoid long lines, and split after a punctation.
> I think we should take the habit to always go to the next line after the end of a sentence.

I left the output of `--help` under a code block as it is originally 
printed in the console. Could surely amend it in the docs to be easier 
to read, but the user could as easily print it themselves in their own 
terminal in the comfort of their own environment.

>> -                           [DTS_OUTPUT_DIR] Output directory where dts logs and results are
>> -                           saved. (default: output)
>> +                           [DTS_OUTPUT_DIR] Output directory where dts logs and results are saved.
> 
> dts -> DTS

As above. The output of `--help` only changed as a result of not being 
updated before in parallel with code changes. Consistently this is what 
the user would see right now. It may or may not be a good idea to update 
this whenever changed in the future.

Nonetheless, I am keen to update the code as part of this patch to 
resolve your comments.

> Please don't add compilation configuration for now,
> I would like to work on the schema first.
> This is mostly imported from the old DTS and needs to be rethink.

While I understand the concern on wanting to rework the schema, which is 
a great point you make, it may be reasonable to provide something useful 
to close the existing documentation gap. And incrementally updating from 
there. If there is no realistic timeline set in place for a schema 
rework, it may just be better to have something rather than nothing. And 
certainly it would not be very useful to upstream a partial documentation.

Thank you a lot for your review! You have made some good points which 
open up new potential tasks to add to the pipeline.

Best,
Luca
  

04/01/2024 13:34, Luca Vizzarro:
> On 04/01/2024 10:52, Thomas Monjalon wrote:
> >>   DTS needs to know which nodes to connect to and what hardware to use on those nodes.
> >> -Once that's configured, DTS needs a DPDK tarball and it's ready to run.
> >> +Once that's configured, DTS needs a DPDK tarball or a git ref ID and it's ready to run.
> > 
> > That's assuming DTS is compiling DPDK.
> > We may want to provide an already compiled DPDK to DTS.
> 
> Yes, that is correct. At the current state, DTS is always compiled from 
> source though, so it may be reasonable to leave it as it is until this
> feature may be implemented. Nonetheless, my change just informs the user
> of the (already implemented) feature that uses `git archive` from the 
> local repository to create a tarball. A sensible change would be to add
> this explanation I have just given, but it is a technicality and it 
> won't really make a difference to the user.

Yes
I would like to make it clear in this doc that DTS is compiling DPDK.
Please could you change to something like
"DTS needs a DPDK tarball or a git ref ID to compile" ?

I hope we will change it later to allow external compilation.


> >> +   (dts-py3.10) $ ./main.py --help
> > 
> > Why adding this line?
> 
> Just running `./main.py` will just throw a confusing error to the user. 
> I am in the process of sorting this out as it is misleading and not 
> helpful. Specifying the line in this case just hints to the user on the 
> origin of that help/usage document.

Yes would be good to have a message to help the user instead of a confusing error.

> > Should we remove the shell prefix referring to a specific Python version?
> 
> I have purposely left the prefix to indicate that we are in a Poetry 
> shell environment, as that is a pre-requisite to run DTS. So more of an 
> implicit reminder. The Python version specified is in line with the 
> minimum requirement of DTS.

OK

> > In general it is better to avoid long lines, and split after a punctation.
> > I think we should take the habit to always go to the next line after the end of a sentence.
> 
> I left the output of `--help` under a code block as it is originally 
> printed in the console. Could surely amend it in the docs to be easier 
> to read, but the user could as easily print it themselves in their own 
> terminal in the comfort of their own environment.

I was not referring to the console output.
Maybe I misunderstood it.
For the doc sentences, please try to split sentences on different lines.


> >> -                           [DTS_OUTPUT_DIR] Output directory where dts logs and results are
> >> -                           saved. (default: output)
> >> +                           [DTS_OUTPUT_DIR] Output directory where dts logs and results are saved.
> > 
> > dts -> DTS
> 
> As above. The output of `--help` only changed as a result of not being 
> updated before in parallel with code changes. Consistently this is what 
> the user would see right now. It may or may not be a good idea to update 
> this whenever changed in the future.

I did not understand it is part of the help message.

> Nonetheless, I am keen to update the code as part of this patch to 
> resolve your comments.

Yes please update the code for this small wording fix.

> > Please don't add compilation configuration for now,
> > I would like to work on the schema first.
> > This is mostly imported from the old DTS and needs to be rethink.
> 
> While I understand the concern on wanting to rework the schema, which is 
> a great point you make, it may be reasonable to provide something useful 
> to close the existing documentation gap. And incrementally updating from 
> there. If there is no realistic timeline set in place for a schema 
> rework, it may just be better to have something rather than nothing. And 
> certainly it would not be very useful to upstream a partial documentation.

I don't know. I have big doubts about the current schema.
I will review it with your doc patches.
Can you please split this patch in 2 so that the schema doc is in a different patch?

> Thank you a lot for your review! You have made some good points which 
> open up new potential tasks to add to the pipeline.
  

I have two extra suggestions apart from the comments below:
There's a typo inside the "How To Write a Test Suite" section:
In that case, nothing will happen when they're is executed.

And Mypy is missing from the list of linters in the "DTS Developer
Tools" section, could you please add it?

> diff --git a/doc/guides/tools/dts.rst b/doc/guides/tools/dts.rst
> index 32c18ee472..31495cad51 100644
> --- a/doc/guides/tools/dts.rst
> +++ b/doc/guides/tools/dts.rst
<snip>
> @@ -335,3 +336,183 @@ There are three tools used in DTS to help with code checking, style and formatti
>  These three tools are all used in ``devtools/dts-check-format.sh``,
>  the DTS code check and format script.
>  Refer to the script for usage: ``devtools/dts-check-format.sh -h``.
> +
> +Configuration Schema
> +--------------------

Just out of curiosity, is this generated from the schema? It's a
pretty neat format, but maintaining it could be a nightmare without a
script that would always produce the same format.

> +
> +Definitions
> +~~~~~~~~~~~
> +

The section names look to be taken from the schema and all of the
terminology is taken from the schema. Would it make sense to use YAML
terminology here, since people will try to use this information in
YAML files? Or maybe explain what we mean by definitions, properties,
objects, arrays or maybe some other things which YAML doesn't specify.

> +_`Node name`
> +   *string* – A unique identifier for a node. **Examples**: ``SUT1``, ``TG1``.
> +
> +_`ARCH`
> +   *string* – The CPU architecture. **Supported values**: ``x86_64``, ``arm64``, ``ppc64le``.
> +
> +_`CPU`
> +   *string* – The CPU microarchitecture. Use ``native`` for x86. **Supported values**: ``native``, ``armv8a``, ``dpaa2``, ``thunderx``, ``xgene1``.
> +
> +_`OS`
> +   *string* – The operating system. **Supported values**: ``linux``.
> +
> +_`Compiler`
> +   *string* – The compiler used for building DPDK. **Supported values**: ``gcc``, ``clang``, ``icc``, ``mscv``.
> +
> +_`Build target`
> +   *object* – Build targets supported by DTS for building DPDK, described as:
> +
> +   ==================== =========================================================================================
> +   ``arch``             See `ARCH`_
> +   ``os``               See `OS`_
> +   ``cpu``              See `CPU`_
> +   ``compiler``         See `Compiler`_
> +   ``compiler_wrapper`` *string* – Value prepended to the CC variable for the DPDK build.
> +
> +                        **Example**: ``ccache``
> +   ==================== =========================================================================================
> +
> +_`hugepages`
> +   *object* – hugepages described as:
> +
> +   ==================== ================================================================
> +   ``amount``           *integer* – The amount of hugepages to configure.
> +

We should update "amount" (uncountable) to something that's countable,
such as count or number.

> +                        Hugepage size will be the system default.
> +   ``force_first_numa`` (*optional*, defaults to ``false``) – If ``true``, it forces the
> +
> +                        configuration of hugepages on the first NUMA node.
> +   ==================== ================================================================
> +
> +_`Network port`
> +   *object* – the NIC port described as:
> +
> +   ====================== =================================================================================
> +   ``pci``                *string* – the local PCI address of the port. **Example**: ``0000:00:08.0``
> +   ``os_driver_for_dpdk`` | *string* – this port's device driver when using with DPDK
> +                          | When setting up the SUT, DTS will bind the network device to this driver
> +                          | for compatibility with DPDK.
> +
> +                          **Examples**: ``vfio-pci``, ``mlx5_core``
> +   ``os_driver``          | *string* – this port's device driver when **not** using with DPDK
> +                          | When tearing down the tests on the SUT, DTS will bind the network device
> +                          | *back* to this driver. This driver is meant to be the one that the SUT would
> +                          | normally use for this device, or whichever driver it is preferred to leave the
> +                          | device bound to after testing.
> +

Of note here is that some traffic generators (to which the port config
also applies) won't be using os_driver_for_dpdk (such as Scapy), but
rather os_driver, so the use is broader.

> +                          **Examples**: ``i40e``, ``mlx5_core``
> +   ``peer_node``          *string* – the name of the peer node connected to this port.
> +   ``peer_pci``           *string* – the PCI address of the peer node port. **Example**: ``000a:01:00.1``
> +   ====================== =================================================================================
<snip>
> +Example
> +~~~~~~~
> +
> +The following example (which can be found in ``dts/conf.yaml``) sets up two nodes:
> +
> +* ``SUT1`` which is already setup with the DPDK build requirements and any other
> +  required for execution;
> +* ``TG1`` which already has Scapy installed in the system.
> +
> +And they both have two network ports which are physically connected to each other.
> +
> +.. note::
> +   This example assumes that you have setup SSH keys in both the system under test
> +   and traffic generator nodes.
> +
> +.. literalinclude:: ../../../dts/conf.yaml
> +   :language: yaml
> +   :start-at: executions:
> \ No newline at end of file

The last newline is missing.

> diff --git a/dts/conf.yaml b/dts/conf.yaml
> index 37967daea0..2d6fa38a2c 100644
> --- a/dts/conf.yaml
> +++ b/dts/conf.yaml
> @@ -1,65 +1,76 @@
>  # SPDX-License-Identifier: BSD-3-Clause
>  # Copyright 2022-2023 The DPDK contributors
> +# Copyright 2023 Arm Limited
>
>  executions:
> +  # define one execution environment
>    - build_targets:
>        - arch: x86_64
>          os: linux
>          cpu: native
> +        # the combination of the following two makes CC="ccache gcc"
>          compiler: gcc
>          compiler_wrapper: ccache
> -    perf: false
> -    func: true
> -    skip_smoke_tests: false # optional flag that allows you to skip smoke tests
> -    test_suites:
> +    perf: false # disable performance testing
> +    func: true # enable functional testing
> +    skip_smoke_tests: false

Let's keep the note that the skip_smoke_tests flag is optional

> +    test_suites: # the following test suites will be run in their entirety
>        - hello_world
>        - os_udp
> +    # The machine running the DPDK test executable
>      system_under_test_node:
>        node_name: "SUT 1"
>        vdevs: # optional; if removed, vdevs won't be used in the execution
>          - "crypto_openssl"
> +    # Traffic generator node to use for this execution environment
>      traffic_generator_node: "TG 1"
>  nodes:
> +  # Define a system under test node, having two network ports physically
> +  # connected to the corresponding ports in TG 1 (the peer node)
>    - name: "SUT 1"
>      hostname: sut1.change.me.localhost
>      user: dtsuser
>      arch: x86_64
>      os: linux
> -    lcores: ""
> -    use_first_core: false
> -    memory_channels: 4
> +    lcores: "" # use all the available logical cores
> +    use_first_core: false # tells DPDK to use any physical core
> +    memory_channels: 4 # tells DPDK to use 4 memory channels
>      hugepages:  # optional; if removed, will use system hugepage configuration
>          amount: 256
>          force_first_numa: false
>      ports:
> +      # sets up the physical link between "SUT 1"@000:00:08.0 and "TG 1"@0000:00:08.0
>        - pci: "0000:00:08.0"
>          os_driver_for_dpdk: vfio-pci # OS driver that DPDK will use
> -        os_driver: i40e
> +        os_driver: i40e              # OS driver to bind when the tests are not running
>          peer_node: "TG 1"
>          peer_pci: "0000:00:08.0"
> +      # sets up the physical link between "SUT 1"@000:00:08.1 and "TG 1"@0000:00:08.1
>        - pci: "0000:00:08.1"
>          os_driver_for_dpdk: vfio-pci
>          os_driver: i40e
>          peer_node: "TG 1"
>          peer_pci: "0000:00:08.1"
> +  # Define a Scapy traffic generator node, having two network ports
> +  # physically connected to the corresponding ports in SUT 1 (the peer node).
>    - name: "TG 1"
>      hostname: tg1.change.me.localhost
>      user: dtsuser
>      arch: x86_64
>      os: linux
> -    lcores: ""

The traffic generator may need this core configuration. However, since
it's not required for all traffic generators (such as Scapy), we could
just move to the traffic_generator section. That would require some
code modifications though, but even the removal of lcores and
use_first_core should be addressed in the configuration classes as
well. Have you tried running DTS with these changes?

>      ports:
> +      # sets up the physical link between "TG 1"@000:00:08.0 and "SUT 1"@0000:00:08.0
>        - pci: "0000:00:08.0"
>          os_driver_for_dpdk: rdma
>          os_driver: rdma
>          peer_node: "SUT 1"
>          peer_pci: "0000:00:08.0"
> +      # sets up the physical link between "SUT 1"@000:00:08.0 and "TG 1"@0000:00:08.0
>        - pci: "0000:00:08.1"
>          os_driver_for_dpdk: rdma
>          os_driver: rdma
>          peer_node: "SUT 1"
>          peer_pci: "0000:00:08.1"
> -    use_first_core: false
>      hugepages:  # optional; if removed, will use system hugepage configuration
>          amount: 256
>          force_first_numa: false
> --
> 2.34.1
>
  

Hi Juraj,

Thank you for your review!

On 12/01/2024 13:24, Juraj Linkeš wrote:
> I have two extra suggestions apart from the comments below:
> There's a typo inside the "How To Write a Test Suite" section:
> In that case, nothing will happen when they're is executed.
> 
> And Mypy is missing from the list of linters in the "DTS Developer
> Tools" section, could you please add it?

Ack.

> Just out of curiosity, is this generated from the schema? It's a
> pretty neat format, but maintaining it could be a nightmare without a
> script that would always produce the same format.

I originally found only one tool that would generate rst. The generated 
output was quite messy though. Only after hacking its few configuration 
options and the schema, it produced something decent. But as it is only 
available on pip and it would have to become a DPDK docs requirement to 
generate them, it wouldn't be acceptable.

Unless someone comes up with a good tool that could match our needs, 
unfortunately manual work is the only solution for the time being...and 
I won't deny it took me a bit of time to format it. The only major 
problem is all the extra whitespaces and the alignment of the columns 
needed to make sphinx happy. Once most of the work is done though – as 
it is in this case, changing it from there shouldn't be too bad.

> The section names look to be taken from the schema and all of the
> terminology is taken from the schema. Would it make sense to use YAML
> terminology here, since people will try to use this information in
> YAML files? Or maybe explain what we mean by definitions, properties,
> objects, arrays or maybe some other things which YAML doesn't specify.

I understand your point of view. I had a quick look at the YAML glossary 
and I think it may be a lot more confusing than using generic software 
engineering terminology. Also we may not want to be constrained to YAML.

The YAML glossary refers to what we would call dictionary in Python and 
objects in JSON as mappings. And arrays and lists as sequences. Maybe 
changing array to list could be a good idea. Objects instead is 
something I don't personally like either. I took it from the JSON schema 
to rst generator tool, as I am not sure what fits best here.

I welcome suggestions to change specific terms. On the other hand, I am 
not so sure about explaining them as it is out of scope. Moreover, the 
configuration template should cover all of the scenarios, so one can 
also infer usage from there.

> We should update "amount" (uncountable) to something that's countable,
> such as count or number.

Ack.

> Of note here is that some traffic generators (to which the port config
> also applies) won't be using os_driver_for_dpdk (such as Scapy), but
> rather os_driver, so the use is broader.

Ack. Thanks for the explanation, makes sense.

> The last newline is missing.

Ack.

> Let's keep the note that the skip_smoke_tests flag is optional

Ack.

> The traffic generator may need this core configuration. However, since
> it's not required for all traffic generators (such as Scapy), we could
> just move to the traffic_generator section. That would require some
> code modifications though, but even the removal of lcores and
> use_first_core should be addressed in the configuration classes as
> well. Have you tried running DTS with these changes?

Please correct me if I'm wrong. At the current state of DTS it seems 
that these two properties are only used with DPDK. If this is correct, 
it may be misleading to the user to add them for the traffic generator 
node, hinting that they may make a difference when they don't.

It would definitely makes sense to have dedicated properties for DPDK 
and for the traffic generators, instead of common ones. But this is 
possibly more of a concern when and if support for other traffic 
generators will be added in the future.

The removal of `lcores` and `use_first_core` does not affect the 
execution as both have default values they fallback to, as seen in
`dts/framework/config/__init__.py#NodeConfiguration.from_dict`.

Yes, I tried running DTS and wasn't met by anything unusual.

Best,
Luca
  

On Fri, Jan 12, 2024 at 6:16 PM Luca Vizzarro <Luca.Vizzarro@arm.com> wrote:
>
> Hi Juraj,
>
> Thank you for your review!
>
> On 12/01/2024 13:24, Juraj Linkeš wrote:
> > I have two extra suggestions apart from the comments below:
> > There's a typo inside the "How To Write a Test Suite" section:
> > In that case, nothing will happen when they're is executed.
> >
> > And Mypy is missing from the list of linters in the "DTS Developer
> > Tools" section, could you please add it?
>
> Ack.
>
> > Just out of curiosity, is this generated from the schema? It's a
> > pretty neat format, but maintaining it could be a nightmare without a
> > script that would always produce the same format.
>
> I originally found only one tool that would generate rst. The generated
> output was quite messy though. Only after hacking its few configuration
> options and the schema, it produced something decent. But as it is only
> available on pip and it would have to become a DPDK docs requirement to
> generate them, it wouldn't be acceptable.
>

This wouldn't need to be a hard requirement. It could just be a tool
that submitters could use as a starting point (which they would
optionally install and use).

> Unless someone comes up with a good tool that could match our needs,
> unfortunately manual work is the only solution for the time being...and
> I won't deny it took me a bit of time to format it. The only major
> problem is all the extra whitespaces and the alignment of the columns
> needed to make sphinx happy. Once most of the work is done though – as
> it is in this case, changing it from there shouldn't be too bad.
>

Alright, I hope people won't be too frustrated with it. :-) But it's
likely the section is not going to be changed that much so it's not a
big deal.

> > The section names look to be taken from the schema and all of the
> > terminology is taken from the schema. Would it make sense to use YAML
> > terminology here, since people will try to use this information in
> > YAML files? Or maybe explain what we mean by definitions, properties,
> > objects, arrays or maybe some other things which YAML doesn't specify.
>
> I understand your point of view. I had a quick look at the YAML glossary
> and I think it may be a lot more confusing than using generic software
> engineering terminology. Also we may not want to be constrained to YAML.
>
> The YAML glossary refers to what we would call dictionary in Python and
> objects in JSON as mappings. And arrays and lists as sequences. Maybe
> changing array to list could be a good idea. Objects instead is
> something I don't personally like either. I took it from the JSON schema
> to rst generator tool, as I am not sure what fits best here.
>

I like mappings [0] and sequences [1], as that is the standard Python
terminology.

[0] https://docs.python.org/3/library/stdtypes.html#mapping-types-dict
[1] https://docs.python.org/3/library/stdtypes.html#sequence-types-list-tuple-range

> I welcome suggestions to change specific terms. On the other hand, I am
> not so sure about explaining them as it is out of scope. Moreover, the
> configuration template should cover all of the scenarios, so one can
> also infer usage from there.
>

One more thing to like about mappings and sequences, we don't (or
shouldn't) have to explain those. I think those would work fine on
both fronts.

> > We should update "amount" (uncountable) to something that's countable,
> > such as count or number.
>
> Ack.
>
> > Of note here is that some traffic generators (to which the port config
> > also applies) won't be using os_driver_for_dpdk (such as Scapy), but
> > rather os_driver, so the use is broader.
>
> Ack. Thanks for the explanation, makes sense.
>
> > The last newline is missing.
>
> Ack.
>
> > Let's keep the note that the skip_smoke_tests flag is optional
>
> Ack.
>
> > The traffic generator may need this core configuration. However, since
> > it's not required for all traffic generators (such as Scapy), we could
> > just move to the traffic_generator section. That would require some
> > code modifications though, but even the removal of lcores and
> > use_first_core should be addressed in the configuration classes as
> > well. Have you tried running DTS with these changes?
>
> Please correct me if I'm wrong. At the current state of DTS it seems
> that these two properties are only used with DPDK. If this is correct,
> it may be misleading to the user to add them for the traffic generator
> node, hinting that they may make a difference when they don't.
>

That's right, it's only used in DPDK.

> It would definitely makes sense to have dedicated properties for DPDK
> and for the traffic generators, instead of common ones.

The dedicated DPDK/TG properties look like the way to go, it just makes sense.

> But this is
> possibly more of a concern when and if support for other traffic
> generators will be added in the future.
>

We'll need to add support for at least one traffic generator that's
suitable for performance testing (Scapy is too slow) and then we'll
need to extra config (such as cores for T-Rex).

> The removal of `lcores` and `use_first_core` does not affect the
> execution as both have default values they fallback to, as seen in
> `dts/framework/config/__init__.py#NodeConfiguration.from_dict`.
>
> Yes, I tried running DTS and wasn't met by anything unusual.

Ah, the defaults made it work. Let's remove the tg node config from
the yaml file then, as you suggested.

>
> Best,
> Luca
  

On 15/01/2024 09:36, Juraj Linkeš wrote:
> This wouldn't need to be a hard requirement. It could just be a tool
> that submitters could use as a starting point (which they would
> optionally install and use).

Sorry, forgot to mention that the tool is actually a sphinx plugin, 
which is used through directives in the doc pages. It doesn't actually 
generate rst, but rather renders tables when running sphinx-build.
Otherwise, yeah, it would have been feasible.
> I like mappings [0] and sequences [1], as that is the standard Python
> terminology.

Ack. Sounds good to me. I could also add these links for reference.

> One more thing to like about mappings and sequences, we don't (or
> shouldn't) have to explain those. I think those would work fine on
> both fronts.