[v2] doc: add new driver guidelines

Message ID 20240814190901.14912-1-stephen@networkplumber.org (mailing list archive)
State Superseded
Delegated to: Thomas Monjalon
Headers
Series [v2] 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-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-marvell-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS

Commit Message

Stephen Hemminger Aug. 14, 2024, 7:08 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>
---

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
  

Comments

Akhil Goyal Sept. 5, 2024, 9:16 a.m. UTC | #1
Few crypto specific suggestions.

> 
> 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
> 
> 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..4aa1fd1d05
> --- /dev/null
> +++ b/doc/guides/contributing/new_driver.rst
> @@ -0,0 +1,202 @@
> +.. 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              Start queues
> +Simple Rx / Tx            Simple Data Processing
> +Statistics                Statistics
> +Device info
> +Link interrupt
> +Burst mode info
> +Promisc all-multicast
> +RSS
> +=======================   ========================

For crypto, 
Initialization
Configure queues
Configure sessions
Add capabilities.
Statistics and device info
Simple data processing

> +
> +
> +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
> +----------------------
> +
.ini file shall be updated for each of the new feature added in the same patch as code.

> +* 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.
> +* 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.
> +
> +
> +.. _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
> --
> 2.43.0
  
Ferruh Yigit Sept. 5, 2024, 9:49 a.m. UTC | #2
On 9/5/2024 10:16 AM, Akhil Goyal wrote:
> Few crypto specific suggestions.
> 
>>
>> 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
>>
>> 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..4aa1fd1d05
>> --- /dev/null
>> +++ b/doc/guides/contributing/new_driver.rst
>> @@ -0,0 +1,202 @@
>> +.. 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              Start queues
>> +Simple Rx / Tx            Simple Data Processing
>> +Statistics                Statistics
>> +Device info
>> +Link interrupt
>> +Burst mode info
>> +Promisc all-multicast
>> +RSS
>> +=======================   ========================
> 
> For crypto, 
> Initialization
> Configure queues
> Configure sessions
> Add capabilities.
> Statistics and device info
> Simple data processing
> 
>> +
>> +
>> +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
>> +----------------------
>> +
> .ini file shall be updated for each of the new feature added in the same patch as code.
> 

This is mentioned above as:
"Update relevant documentation and <driver>.ini file with each patch."
  
Akhil Goyal Sept. 5, 2024, 9:52 a.m. UTC | #3
> >> +Additional Suggestions
> >> +----------------------
> >> +
> > .ini file shall be updated for each of the new feature added in the same patch as
> code.
> >
> 
> This is mentioned above as:
> "Update relevant documentation and <driver>.ini file with each patch."
Ok. I missed that.
  
fengchengwen Sept. 6, 2024, 8:05 a.m. UTC | #4
On 2024/8/15 3:08, Stephen Hemminger wrote:
> 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>
> ---
> 
> 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) ?

> +* 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) ?

> +
> +
> +.. _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.
  
Ferruh Yigit Sept. 6, 2024, 8:27 a.m. UTC | #5
On 9/6/2024 9:05 AM, fengchengwen wrote:
> On 2024/8/15 3:08, Stephen Hemminger wrote:
>> 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>
>> ---
>>
>> 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.`
  
fengchengwen Sept. 9, 2024, 1:01 a.m. UTC | #6
Hi Ferruh,

Thanks for the explanation.

In the next new version:
Acked-by: Chengwen Feng <fengchengwen@huawei.com>

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 <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>
>>> ---
>>>
>>> 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

> 
> .
>
  

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..4aa1fd1d05
--- /dev/null
+++ b/doc/guides/contributing/new_driver.rst
@@ -0,0 +1,202 @@ 
+.. 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              Start queues
+Simple Rx / Tx            Simple Data Processing
+Statistics                Statistics
+Device info
+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
+=============================
+
+
+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.
+* Do not use DPDK version checks 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.
+
+
+.. _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