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 5F4F6438CD; Mon, 15 Jan 2024 10:36:21 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id CB016402C6; Mon, 15 Jan 2024 10:36:20 +0100 (CET) Received: from mail-ed1-f46.google.com (mail-ed1-f46.google.com [209.85.208.46]) by mails.dpdk.org (Postfix) with ESMTP id 4E878402C0 for ; Mon, 15 Jan 2024 10:36:19 +0100 (CET) Received: by mail-ed1-f46.google.com with SMTP id 4fb4d7f45d1cf-559533e2503so514895a12.1 for ; Mon, 15 Jan 2024 01:36:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pantheon.tech; s=google; t=1705311379; x=1705916179; darn=dpdk.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=8PP8nGR6NHMLgIMlSBIsTe5DtmV9U+oVxJAtxwOadR8=; b=U7KPhLx9O2DGFeR8/yg/zIhvh4mHYS19X7hhcRrtk1HGjSvSbJCW3o/EWHm29IXp2n D1kgtfMfcm8fAsor0+9wWPcrDS6cYb0P48fIl82AvegjWsl9rSdVybhSirpHocvYe3/R rDWhnhfnQnSFl2C9SMAZw6Sm4LpNr9Bqba1jNM1YzAMLoFhFY+Kc8kBcqa8DNypsyX3W tfcRYHFb5TKKPgjMtuoyzXY+FratP9/nHAmp9mJDDp9s5gPdEv5eoZUCjQTewhh7fba4 IbvlKyYT1Zvwf6HzAS//Dq69h58mj/f0cA9a11NWA2vNdnsxGmqOn5dm5Dbscxulma3y dqIw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705311379; x=1705916179; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=8PP8nGR6NHMLgIMlSBIsTe5DtmV9U+oVxJAtxwOadR8=; b=sUbR5xBdWflfGq/uOxYZ7pJqYYHxPayRKj1jR+xyz9ITHiWyrrTfPmsDK9StB1cH2V 7q9HDfQCjISrTkNwpGIuqQ/riilRNulQ7klg6vcz/RLu11bViUz4wFI2W9M+pi5LrVUH SVZdixUVBVEaEkSu9B2SQLNCLLJ3SuzzXMyysCin/YaL3OgNgnlw0SAGV3TmorjRHo3X Z9F8hXk9YXMabQUFLVTONDdAVnAeDuGyWSIkYfqPD1ShDo7GidF+vbltrd60Ru8IveGs prp/CUDMVEywiMx5Y51o9wIqt63R2ZwbLe5/ESxORTZErgaYMd10DH0cMEECxRx8MBpr 6XrQ== X-Gm-Message-State: AOJu0YxuUZXVbPYXZ67Zlx/fnF4YCrAcIjZxIhVSQiSV2QWNyDx0xDHE 9YH7oYc+shd1QMKwGOeTF/qLewmAaiKexaGvWMKsVRgay8KsGg== X-Google-Smtp-Source: AGHT+IGkiHhdW6h+SCKL+RjUBByHfUuSz7eU2XgbZ/f3uguepxLS+4NhYLnwQPwDj+L5NDTAlbE34VadKe26Tj3WjYY= X-Received: by 2002:a17:906:55c1:b0:a28:ff59:12eb with SMTP id z1-20020a17090655c100b00a28ff5912ebmr2328384ejp.139.1705311378776; Mon, 15 Jan 2024 01:36:18 -0800 (PST) MIME-Version: 1.0 References: <20240103125438.182098-1-Luca.Vizzarro@arm.com> <07ec0377-396a-46e9-93e4-c7e5133441c5@arm.com> In-Reply-To: <07ec0377-396a-46e9-93e4-c7e5133441c5@arm.com> From: =?UTF-8?Q?Juraj_Linke=C5=A1?= Date: Mon, 15 Jan 2024 10:36:08 +0100 Message-ID: Subject: Re: [PATCH] dts: improve documentation To: Luca Vizzarro Cc: dev@dpdk.org, Paul Szczepanek , Thomas Monjalon , Lijuan Tu Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 On Fri, Jan 12, 2024 at 6:16=E2=80=AFPM Luca Vizzarro wrote: > > Hi Juraj, > > Thank you for your review! > > On 12/01/2024 13:24, Juraj Linke=C5=A1 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 =E2=80= =93 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-tup= le-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 sen= se. > 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