From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id A65244389A; Fri, 12 Jan 2024 19:15:48 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7AAC8402AD; Fri, 12 Jan 2024 19:15:48 +0100 (CET) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mails.dpdk.org (Postfix) with ESMTP id E662F402AD for ; Fri, 12 Jan 2024 18:16:17 +0100 (CET) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 7FEDC1FB; Fri, 12 Jan 2024 09:17:03 -0800 (PST) Received: from [10.1.194.39] (unknown [10.1.194.39]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 574CD3F67D; Fri, 12 Jan 2024 09:16:16 -0800 (PST) Message-ID: <07ec0377-396a-46e9-93e4-c7e5133441c5@arm.com> Date: Fri, 12 Jan 2024 17:16:14 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] dts: improve documentation Content-Language: en-GB To: =?UTF-8?Q?Juraj_Linke=C5=A1?= Cc: dev@dpdk.org, Paul Szczepanek , Thomas Monjalon , Lijuan Tu References: <20240103125438.182098-1-Luca.Vizzarro@arm.com> From: Luca Vizzarro In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Mailman-Approved-At: Fri, 12 Jan 2024 19:15:47 +0100 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 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