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 4EF8645941; Mon, 9 Sep 2024 03:01:35 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D6DEF406A2; Mon, 9 Sep 2024 03:01:34 +0200 (CEST) Received: from szxga05-in.huawei.com (szxga05-in.huawei.com [45.249.212.191]) by mails.dpdk.org (Postfix) with ESMTP id 2EDAA402A8 for ; Mon, 9 Sep 2024 03:01:32 +0200 (CEST) Received: from mail.maildlp.com (unknown [172.19.88.234]) by szxga05-in.huawei.com (SkyGuard) with ESMTP id 4X27jf2SRfz1HJMK; Mon, 9 Sep 2024 08:57:58 +0800 (CST) Received: from dggpeml500024.china.huawei.com (unknown [7.185.36.10]) by mail.maildlp.com (Postfix) with ESMTPS id D7FC1140134; Mon, 9 Sep 2024 09:01:30 +0800 (CST) Received: from [10.67.121.161] (10.67.121.161) by dggpeml500024.china.huawei.com (7.185.36.10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.39; Mon, 9 Sep 2024 09:01:30 +0800 Subject: Re: [PATCH v2] doc: add new driver guidelines To: Ferruh Yigit , Stephen Hemminger , CC: Nandini Persad , Thomas Monjalon References: <20240814023604.124686-1-stephen@networkplumber.org> <20240814190901.14912-1-stephen@networkplumber.org> <5f8f4577-7143-4b02-a196-63b63d000dd7@amd.com> From: fengchengwen Message-ID: Date: Mon, 9 Sep 2024 09:01:30 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 MIME-Version: 1.0 In-Reply-To: <5f8f4577-7143-4b02-a196-63b63d000dd7@amd.com> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.67.121.161] X-ClientProxiedBy: dggems703-chm.china.huawei.com (10.3.19.180) To dggpeml500024.china.huawei.com (7.185.36.10) 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 Ferruh, Thanks for the explanation. In the next new version: Acked-by: Chengwen Feng Thanks On 2024/9/6 16:27, Ferruh Yigit wrote: > On 9/6/2024 9:05 AM, fengchengwen wrote: >> On 2024/8/15 3:08, Stephen Hemminger wrote: >>> From: Nandini Persad >>> >>> This document was created to assist contributors in creating DPDK drivers >>> and provides suggestions and guidelines on how to upstream effectively. >>> >>> Co-authored-by: Ferruh Yigit >>> Co-authored-by: Thomas Monjalon >>> Signed-off-by: Nandini Persad >>> Reviewed-by: Stephen Hemminger >>> --- >>> >>> v2 - review feedback >>> - add co-author and reviewed-by >>> >>> doc/guides/contributing/index.rst | 1 + >>> doc/guides/contributing/new_driver.rst | 202 +++++++++++++++++++++++++ >>> 2 files changed, 203 insertions(+) >>> create mode 100644 doc/guides/contributing/new_driver.rst >>> >> >> ... >> >>> + >>> +Finalizing >>> +---------- >>> + >>> +Once the driver has been upstreamed, the author has >>> +a responsibility to the community to maintain it. >>> + >>> +This includes the public test report. Authors must send a public >>> +test report after the first upstreaming of the PMD. The same >>> +public test procedure may be reproduced regularly per release. >>> + >>> +After the PMD is upstreamed, the author should send a patch >>> +to update the website with the name of the new PMD and supported devices >>> +via the DPDK mailing list.. >> >> .. -> . >> >>> + >>> +For more information about the role of maintainers, see :doc:`patches`. >>> + >>> + >>> + >>> +Splitting into Patches >>> +---------------------- >>> + >> >> ... >> >>> + >>> + >>> +The following order in the patch series is as suggested below. >>> + >>> +The first patch should have the driver's skeleton which should include: >>> + >>> +* Maintainer's file update >>> +* Driver documentation >>> +* Document must have links to official product documentation web page >>> +* The new document should be added into the index (`doc/guides/index.rst`) >> >> The new -> The new >> >> ... >> >>> + >>> +Additional Suggestions >>> +---------------------- >>> + >>> +* We recommend using DPDK macros instead of inventing new ones in the PMD. >>> +* Do not include unused headers. Use the ./devtools/process-iwyu.py tool. >>> +* Do not disable compiler warnings in the build file. >>> +* Do not use #ifdef with driver-defined macros, instead prefer runtime configuration. >>> +* Document device parameters in the driver guide. >>> +* Make device operations struct 'const'. >>> +* Use dynamic logging. >>> +* Do not use DPDK version checks in the upstream code. >> >> Could you explain it (DPDK version check) ? >> > > It refers usage of 'RTE_VERSION_NUM' macro. This may be required for out > of tree drivers, as they may be supporting multiple DPDK version. > > Not sure adding too much details for sure, what about following update: > `* Do not use DPDK version checks (via RTE_VERSION_NUM) in the upstream > code.` Got it, thanks > > >>> +* Be sure to have SPDX license tags and copyright notice on each side. >>> + Use ./devtools/check-spdx-tag.sh >>> +* Run the Coccinelle scripts ./devtools/cocci.sh which check for common cleanups such as >>> + useless null checks before calling free routines. >>> + >>> +Dependencies >>> +------------ >>> + >>> +At times, drivers may have dependencies to external software. >>> +For driver dependencies, same DPDK rules for dependencies applies. >>> +Dependencies should be publicly and freely available, >>> +or this is a blocker for upstreaming the driver. >> >> Could you explain it (what's the blocker) ? >> > > It is trying to say, this prevents upstreaming, wording can be updated > to clarify, what about following: > > `Dependencies should be publicly and freely available to be able to > upstream the driver.` +1 > > >>> + >>> + >>> +.. _tool_list: >>> + >>> +Test Tools >>> +---------- >>> + >>> +Build and check the driver's documentation. Make sure there are no >>> +warnings and driver shows up in the relevant index page. >>> + >>> +Be sure to run the following test tools per patch in a patch series: >>> + >>> +* checkpatches.sh >>> +* check-git-log.sh >>> +* check-meson.py >>> +* check-doc-vs-code.sh >>> >> >> Some drivers already provide private APIs, I think we should add note >> for "not add private APIs, prefer to extend the corresponding framework API" for new drivers. >> > > Ack. > What about adding this to "Additional Suggestions", like following: > `Do not introduce public APIs directly from the driver.` +1 > > . >