[v3] doc: add new driver guidelines

Message ID 20240912202734.24440-1-stephen@networkplumber.org (mailing list archive)
State Superseded
Delegated to: Thomas Monjalon
Headers
Series [v3] doc: add new driver guidelines |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/github-robot: build success github build: passed
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-marvell-Functional success Functional Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-sample-apps-testing success Testing PASS

Commit Message

Stephen Hemminger Sept. 12, 2024, 8:26 p.m. UTC
From: Nandini Persad <nandinipersad361@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.yigit@amd.com>
Co-authored-by: Thomas Monjalon <thomas@monjalon.net>
Signed-off-by: Nandini Persad <nandinipersad361@gmail.com>
Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
---
v3 - incorporate review feedback

 doc/guides/contributing/index.rst      |   1 +
 doc/guides/contributing/new_driver.rst | 217 +++++++++++++++++++++++++
 2 files changed, 218 insertions(+)
 create mode 100644 doc/guides/contributing/new_driver.rst
  

Comments

WanRenyong Sept. 13, 2024, 4:19 a.m. UTC | #1
On 2024/9/13 4:26, Stephen Hemminger wrote
> +
> +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.
Is there any guildelines about how to write a test report? Is there any 
template?
> +
> +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,
> +drivers which depend on non-available components will not be accepted.
> +If the required dependency is not yet publicly available, then wait to submit
> +the driver until the dependent library is available.
> +
Could you please interpret dependencies publicly and freely?There are 4 
scenarios as below:
1. A dependency is niche software, but it's open-sourced on github, is 
it publicly or freely?
2. A dependency which belongs to our company and open-sourced on github, 
is it publicly or freely?
3. A dependency which is not available in the upstream distribution, but 
available in the downstream distribution. For instance, a kernel driver 
dependent upon by PMD, which is not available in kernel.org,but it's 
available in openeuler kernel, the openeuler kernel is publicly and 
freely.  Is it publicly or freely?4. If a distribution does not include 
the dependency, I redistribute it with the dependency and open source, 
this is somewhat similar to mlnx_ofed, is it publicly or freely?

Hello, Stephen,

These guildelines are very useful for begineers like me :). I have some 
questions above, could you please explain them? Thank you.
  
Ferruh Yigit Sept. 13, 2024, 9:07 a.m. UTC | #2
On 9/13/2024 5:19 AM, WanRenyong wrote:
> On 2024/9/13 4:26, Stephen Hemminger wrote
>> +
>> +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.
> Is there any guildelines about how to write a test report? Is there any 
> template?
>

This is a good question and indeed I got it a few times before, I think
we don't have it documented anywhere.
@Nandini, @Thomas, can this be next thing to work on?

We are using free format for test result reporting, you can find some
samples in mail list, please check replies to release candidate emails
in dev mailing list for samples, like:
https://inbox.dpdk.org/dev/MW4PR11MB5912FBE30092B821029858EA9FDE2@MW4PR11MB5912.namprd11.prod.outlook.com/

>> +
>> +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,
>> +drivers which depend on non-available components will not be accepted.
>> +If the required dependency is not yet publicly available, then wait to submit
>> +the driver until the dependent library is available.
>> +
> Could you please interpret dependencies publicly and freely?There are 4 
> scenarios as below:
> 1. A dependency is niche software, but it's open-sourced on github, is 
> it publicly or freely?
> 2. A dependency which belongs to our company and open-sourced on github, 
> is it publicly or freely?
> 3. A dependency which is not available in the upstream distribution, but 
> available in the downstream distribution. For instance, a kernel driver 
> dependent upon by PMD, which is not available in kernel.org,but it's 
> available in openeuler kernel, the openeuler kernel is publicly and 
> freely.  Is it publicly or freely?
> 4. If a distribution does not include > the dependency, I redistribute
it with the dependency and open source,
> this is somewhat similar to mlnx_ofed, is it publicly or freely?
> 

All looks good from DPDK perspective, although it is preferred that
dependency upstreamed to its upstream distribution.

Problematic cases are like (not limited to), dependency only delivered
if you purchase the HW, or it is distributed only if you sign some
agreement, or you need to reach out to company and provide some
information to be able to get the SW etc...


> Hello, Stephen,
> 
> These guildelines are very useful for begineers like me :). I have some 
> questions above, could you please explain them? Thank you.
>
  
Stephen Hemminger Sept. 13, 2024, 4:08 p.m. UTC | #3
On Fri, 13 Sep 2024 10:07:26 +0100
Ferruh Yigit <ferruh.yigit@amd.com> wrote:

> > 4. If a distribution does not include > the dependency, I redistribute  
> it with the dependency and open source,
> > this is somewhat similar to mlnx_ofed, is it publicly or freely?
> >   
> 
> All looks good from DPDK perspective, although it is preferred that
> dependency upstreamed to its upstream distribution.
> 
> Problematic cases are like (not limited to), dependency only delivered
> if you purchase the HW, or it is distributed only if you sign some
> agreement, or you need to reach out to company and provide some
> information to be able to get the SW etc...


This policy is based on three principles:
  1. DPDK test infrastructure must be able to cover the driver during
     build and release process. Even if the DPDK CI does not have the
     hardware, want to make sure that every release and patch still builds
     and no regression slips in.

  2. DPDK developers should be able to make broad changes to the internal
     API's and be able to validate that all drivers still build. If a driver
     depended on a non-available library, then it would likely get overlooked
     and suffer bit rot.

  3. DPDK is open source software, we don't want to be seen as being a way
     to "open source wash" a proprietary internal driver.

My other concern is that if Linux kernel drivers are hard. Drivers that have
not been reviewed and merged into kernel.org are likely to have bugs.
For example, both KNI and igb_uio have serious issues that could be exploited
for security exploits. And if a non-upstream driver uses some unsupported API
it is going to be stuck running on some old unstable kernel version.

If DPDK depends on such an unstable driver, when the next security disaster
happens (like Crowdstrike), DPDK might get blamed but the real culprit would
be the proprietary kernel driver. Since DPDK is LF project, that might also
get involved.

This is not an absolute rule, and probably other Technical Board members
have different opinions.
  

Patch

diff --git a/doc/guides/contributing/index.rst b/doc/guides/contributing/index.rst
index dcb9b1fbf0..7fc6511361 100644
--- a/doc/guides/contributing/index.rst
+++ b/doc/guides/contributing/index.rst
@@ -15,6 +15,7 @@  Contributor's Guidelines
     documentation
     unit_test
     new_library
+    new_driver
     patches
     vulnerability
     stable
diff --git a/doc/guides/contributing/new_driver.rst b/doc/guides/contributing/new_driver.rst
new file mode 100644
index 0000000000..5cb2a9c9bf
--- /dev/null
+++ b/doc/guides/contributing/new_driver.rst
@@ -0,0 +1,217 @@ 
+.. SPDX-License-Identifier: BSD-3-Clause
+   Copyright 2024 The DPDK contributors
+
+
+Upstreaming New DPDK Drivers Guide
+==================================
+
+The DPDK project continuously grows its ecosystem by adding support for new devices.
+This document is designed to assist contributors in creating DPDK
+drivers, also known as Poll Mode Drivers (PMD's).
+
+By having public support for a device, we can ensure accessibility across various
+operating systems and guarantee community maintenance in future releases.
+If a new device is similar to a device already supported by an existing driver,
+it is more efficient to update the existing driver.
+
+Here are our best practice recommendations for creating a new driver.
+
+
+Early Engagement with the Community
+-----------------------------------
+
+When creating a new driver, we highly recommend engaging with the DPDK
+community early instead of waiting the work to mature.
+
+These public discussions help align development of your driver with DPDK expectations.
+You may submit a roadmap before the release to inform the community of
+your plans. Additionally, sending a Request for Comments (RFC) early in
+the release cycle, or even during the prior release, is advisable.
+
+DPDK is mainly consumed via Long Term Support (LTS) releases.
+It is common to target a new PMD to a LTS release. For this, it is
+suggested to start upstreaming at least one release before a LTS release.
+
+
+Progressive Work
+----------------
+
+To continually progress your work, we recommend planning for incremental
+upstreaming across multiple patch series or releases.
+
+It's important to prioritize quality of the driver over upstreaming
+in a single release or single patch series.
+
+
+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
+----------------------
+
+We recommend that drivers are split into patches, so that each patch represents
+a single feature. If the driver code is already developed, it may be challenging
+to split. However, there are many benefits to doing so.
+
+Splitting patches makes it easier to understand a feature and clarifies the
+list of components/files that compose that specific feature.
+
+It also enables the ability to track from the source code to the feature
+it is enabled for and helps users to understand the reasoning and intention
+of implementation. This kind of tracing is regularly required
+for defect resolution and refactoring.
+
+Another benefit of splitting the codebase per feature is that it highlights
+unnecessary or irrelevant code, as any code not belonging to any specific
+feature becomes obvious.
+
+Git bisect is also more useful if patches are split per patch.
+
+The split should focus on logical features
+rather than file-based divisions.
+
+Each patch in the series must compile without errors
+and should maintain functionality.
+
+Enable the build as early as possible within the series
+to facilitate continuous integration and testing.
+This approach ensures a clear and manageable development process.
+
+We suggest splitting patches following this approach:
+
+* Each patch should be organized logically as a new feature.
+* Run test tools per patch (See :ref:`tool_list`:).
+* Update relevant documentation and <driver>.ini file with each patch.
+
+
+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`)
+* Initial <drive>.ini file
+* Release notes announcement for the new driver
+
+
+The next patches should include basic device features.
+The following is suggested sample list to include in these patches:
+
+=======================   ========================
+Net                       Crypto
+=======================   ========================
+Initialization            Initialization
+Configure queues          Configure queues
+Start queues              Configure sessions
+Simple Rx / Tx            Add capabilities
+Statistics                Statistics and device info
+Device info               Simple data processing
+Link interrupt
+Burst mode info
+Promisc all-multicast
+RSS
+=======================   ========================
+
+
+Advanced features should be in the next group of patches.
+The suggestions for these, listed below, are in no specific order:
+
+=============================
+Net
+=============================
+Advanced Rx / Tx
+Scatter Support
+Vector Support
+TSO / LRO
+Rx / Tx Descriptor Status
+RX / Tx Queue Info
+Flow Offload
+Traffic Management/Metering
+Extended statistics
+Secondary Process Support
+FreeBSD / Windows Support
+Flow control
+FEC
+EEPROM access
+Register Dump
+Time Synchronization, PTP
+Perf documentation
+=============================
+
+
+=============================
+Crypto
+=============================
+Chained operations
+Scatter Gather
+Security protocols - IPsec, MACsec etc.
+Asymmetric crypto
+=============================
+
+
+After all features are enabled, if there is remaining base code that
+is not upstreamed, they can be upstreamed at the end of the patch series.
+However, we recommend these patches are still split into logical groups.
+
+
+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.
+* The driver must be target for the current release.
+  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.
+* Avoid adding driver private API's. If a new feature is needed it is
+  better to extend the corresponding framework API;
+
+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,
+drivers which depend on non-available components will not be accepted.
+If the required dependency is not yet publicly available, then wait to submit
+the driver until the dependent library is available.
+
+
+.. _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