From: Luca Vizzarro <Luca.Vizzarro@arm.com>
To: "Juraj Linkeš" <juraj.linkes@pantheon.tech>
Cc: dev@dpdk.org, Paul Szczepanek <Paul.Szczepanek@arm.com>,
Thomas Monjalon <thomas@monjalon.net>,
Lijuan Tu <lijuan.tu@intel.com>
Subject: Re: [PATCH] dts: improve documentation
Date: Fri, 12 Jan 2024 17:16:14 +0000 [thread overview]
Message-ID: <07ec0377-396a-46e9-93e4-c7e5133441c5@arm.com> (raw)
In-Reply-To: <CAOb5WZYz_NyNAXEiKoATd5GBCfF+mS_cZAa1QaAAYrWg6PBvHA@mail.gmail.com>
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
next prev parent reply other threads:[~2024-01-12 18:15 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-03 12:54 Luca Vizzarro
2024-01-04 10:52 ` Thomas Monjalon
2024-01-04 12:34 ` Luca Vizzarro
2024-01-04 13:15 ` Thomas Monjalon
2024-01-12 13:24 ` Juraj Linkeš
2024-01-12 17:16 ` Luca Vizzarro [this message]
2024-01-15 9:36 ` Juraj Linkeš
2024-01-15 11:44 ` Luca Vizzarro
2024-01-16 11:44 ` [PATCH v2 1/2] " Luca Vizzarro
2024-01-16 11:44 ` [PATCH v2 2/2] dts: add configuration schema docs Luca Vizzarro
2024-01-16 16:48 ` Juraj Linkeš
2024-01-16 16:47 ` [PATCH v2 1/2] dts: improve documentation Juraj Linkeš
2024-01-19 17:29 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=07ec0377-396a-46e9-93e4-c7e5133441c5@arm.com \
--to=luca.vizzarro@arm.com \
--cc=Paul.Szczepanek@arm.com \
--cc=dev@dpdk.org \
--cc=juraj.linkes@pantheon.tech \
--cc=lijuan.tu@intel.com \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).