On 9/6/2024 9:05 AM, fengchengwen wrote: > On 2024/8/15 3:08, Stephen Hemminger wrote: >> From: Nandini Persad <nandinipersad...@gmail.com> >> >> 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 <ferruh.yi...@amd.com> >> Co-authored-by: Thomas Monjalon <tho...@monjalon.net> >> Signed-off-by: Nandini Persad <nandinipersad...@gmail.com> >> Reviewed-by: Stephen Hemminger <step...@networkplumber.org> >> --- >> >> 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.` >> +* 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.` >> + >> + >> +.. _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.`
