* RE: [EXTERNAL] [PATCH v2] doc: add new driver guidelines
2024-08-14 19:08 ` Stephen Hemminger
@ 2024-09-05 9:16 ` Akhil Goyal
2024-09-05 9:49 ` Ferruh Yigit
2024-09-06 8:05 ` fengchengwen
` (2 subsequent siblings)
3 siblings, 1 reply; 29+ messages in thread
From: Akhil Goyal @ 2024-09-05 9:16 UTC (permalink / raw)
To: Stephen Hemminger, dev; +Cc: Nandini Persad, Ferruh Yigit, Thomas Monjalon
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
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [EXTERNAL] [PATCH v2] doc: add new driver guidelines
2024-09-05 9:16 ` [EXTERNAL] " Akhil Goyal
@ 2024-09-05 9:49 ` Ferruh Yigit
2024-09-05 9:52 ` Akhil Goyal
0 siblings, 1 reply; 29+ messages in thread
From: Ferruh Yigit @ 2024-09-05 9:49 UTC (permalink / raw)
To: Akhil Goyal, Stephen Hemminger, dev; +Cc: Nandini Persad, Thomas Monjalon
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."
^ permalink raw reply [flat|nested] 29+ messages in thread
* RE: [EXTERNAL] [PATCH v2] doc: add new driver guidelines
2024-09-05 9:49 ` Ferruh Yigit
@ 2024-09-05 9:52 ` Akhil Goyal
0 siblings, 0 replies; 29+ messages in thread
From: Akhil Goyal @ 2024-09-05 9:52 UTC (permalink / raw)
To: Ferruh Yigit, Stephen Hemminger, dev; +Cc: Nandini Persad, Thomas Monjalon
> >> +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.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2] doc: add new driver guidelines
2024-08-14 19:08 ` Stephen Hemminger
2024-09-05 9:16 ` [EXTERNAL] " Akhil Goyal
@ 2024-09-06 8:05 ` fengchengwen
2024-09-06 8:27 ` Ferruh Yigit
2024-09-10 14:58 ` [PATCH v3] " Nandini Persad
2024-09-12 20:26 ` Stephen Hemminger
3 siblings, 1 reply; 29+ messages in thread
From: fengchengwen @ 2024-09-06 8:05 UTC (permalink / raw)
To: Stephen Hemminger, dev; +Cc: Nandini Persad, Ferruh Yigit, Thomas Monjalon
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.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2] doc: add new driver guidelines
2024-09-06 8:05 ` fengchengwen
@ 2024-09-06 8:27 ` Ferruh Yigit
2024-09-09 1:01 ` fengchengwen
0 siblings, 1 reply; 29+ messages in thread
From: Ferruh Yigit @ 2024-09-06 8:27 UTC (permalink / raw)
To: fengchengwen, Stephen Hemminger, dev; +Cc: Nandini Persad, Thomas Monjalon
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.`
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2] doc: add new driver guidelines
2024-09-06 8:27 ` Ferruh Yigit
@ 2024-09-09 1:01 ` fengchengwen
0 siblings, 0 replies; 29+ messages in thread
From: fengchengwen @ 2024-09-09 1:01 UTC (permalink / raw)
To: Ferruh Yigit, Stephen Hemminger, dev; +Cc: Nandini Persad, Thomas Monjalon
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
>
> .
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v3] doc: add new driver guidelines
2024-08-14 19:08 ` Stephen Hemminger
2024-09-05 9:16 ` [EXTERNAL] " Akhil Goyal
2024-09-06 8:05 ` fengchengwen
@ 2024-09-10 14:58 ` Nandini Persad
2024-09-11 0:16 ` Ferruh Yigit
2024-09-12 20:26 ` Stephen Hemminger
3 siblings, 1 reply; 29+ messages in thread
From: Nandini Persad @ 2024-09-10 14:58 UTC (permalink / raw)
To: dev; +Cc: Ferruh Yigit, Thomas Mojalon, Chengwen Feng
This document was created to assist contributors
in creating DPDK drivers, providing suggestions
and guidelines for how to upstream effectively.
Co-authored-by: Ferruh Yigit <ferruh.yigit@amd.com>
Co-authored-by: Thomas Mojalon <thomas@monjalon.net>
Signed-off-by: Nandini Persad <nandinipersad361@gmail.com>
Acked-by: Chengwen Feng <fengChengwen@huawei.com>
---
doc/guides/contributing/new_driver.rst | 200 +++++++++++++++++++++++++
1 file changed, 200 insertions(+)
create mode 100644 doc/guides/contributing/new_driver.rst
diff --git a/doc/guides/contributing/new_driver.rst b/doc/guides/contributing/new_driver.rst
new file mode 100644
index 0000000000..2a4781e673
--- /dev/null
+++ b/doc/guides/contributing/new_driver.rst
@@ -0,0 +1,200 @@
+.. SPDX-License-Identifier: BSD-3-Clause
+ Copyright 2024 The DPDK contributors
+
+
+Upsreaming New DPDK Drivers Guide
+=================================
+
+The DPDK project continously 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 (PMDs).
+
+Public support for a device ensures accessibility across various operating
+systems and guarantees community maintenance in future releases.
+If a new device is similar to an existing one,
+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 allowing the work to mature.
+
+Public discussions help align development 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 your driver has been added, you, as the author,
+have 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.
+
+For more information about the role of maintainers, see the
+:doc:'dpdk/doc/guides/contributing/patches.rst' page.
+
+
+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 undertand 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 funtionality.
+
+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 (more on this below).
+* 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
+Device info Configure queues
+Link interrupt Start queues
+Configure queues Simple Data Processing
+Start queues Statistics
+Simple Rx / Tx
+Burst mode info
+Promisc all-multicast
+RSS
+Statistics
+
+======================= ========================
+
+
+Advanced features should be in the next group of patches.
+The suggestions for these, listed below, are in no specific order:
+
+* 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 (process-iwyu.py).
+* Do not disable compiler warning 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 (via RTE_VERSION_NUM) in the upstream code.
+* Be sure to have SPDX license tags and copyright notice on each side.
+* Do not introduce public Apis directly from the driver.
+
+
+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 to
+upstream the driver.
+
+
+Test Tools
+----------
+
+Per patch in a patch series, be sure to use the proper test tools.
+
+* checkpatches.sh
+* check-git-log.sh
+* check-meson.py
+* check-doc-vs-code.sh
+* check-spdx-tag.sh
+* Build documentation and validate how output looks
+
+For more information on contributing to DPDK,
+refer to the :doc:'dpdk/doc/guides/contributing/patches.rst'
+in the Contributor's Guidelines.
+
--
2.34.1
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3] doc: add new driver guidelines
2024-09-10 14:58 ` [PATCH v3] " Nandini Persad
@ 2024-09-11 0:16 ` Ferruh Yigit
2024-09-11 16:04 ` Nandini Persad
0 siblings, 1 reply; 29+ messages in thread
From: Ferruh Yigit @ 2024-09-11 0:16 UTC (permalink / raw)
To: Nandini Persad, dev; +Cc: Thomas Mojalon, Chengwen Feng
On 9/10/2024 3:58 PM, Nandini Persad wrote:
> This document was created to assist contributors
> in creating DPDK drivers, providing suggestions
> and guidelines for how to upstream effectively.
>
There are minor differences in this v3 and v2, isn't this version on top
of v2, can those changes be from Stephen?
<...>
> +
> +Additional Suggestions
> +----------------------
> +
> +* We recommend using DPDK macros instead of inventing new ones in the PMD.
> +* Do not include unused headers (process-iwyu.py).
> +* Do not disable compiler warning 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 (via RTE_VERSION_NUM) in the upstream code.
> +* Be sure to have SPDX license tags and copyright notice on each side.
> +* Do not introduce public Apis directly from the driver.
>
API (Application Programming Interface) is an acronym and should be all
uppercase, like 'APIs'.
Overall the language in this list is imperative, I think it helps to
make it simple, but I am not sure about the tone, I wonder if we can do
better, do you have any suggestions?
> +
> +
> +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 to
> +upstream the driver.
> +
> +
> +Test Tools
> +----------
> +
> +Per patch in a patch series, be sure to use the proper test tools.
> +
> +* checkpatches.sh
> +* check-git-log.sh
> +* check-meson.py
> +* check-doc-vs-code.sh
> +* check-spdx-tag.sh
>
`check-spdx-tag.sh` seems moved in v2 to "additional suggestions", I am
for keeping it here, as "additional suggestions" are more things to take
into consideration during design/development, above are actual scripts
that we can use to verify code.
And long term intention was to move this "tools to run list" to a more
generic documentation, as these are not really specific to new PMD
guide, but "additional suggestions" will stay in this document.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3] doc: add new driver guidelines
2024-09-11 0:16 ` Ferruh Yigit
@ 2024-09-11 16:04 ` Nandini Persad
2024-09-12 8:13 ` Ferruh Yigit
0 siblings, 1 reply; 29+ messages in thread
From: Nandini Persad @ 2024-09-11 16:04 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: dev, Thomas Mojalon, Stephen Hemminger
[-- Attachment #1: Type: text/plain, Size: 3305 bytes --]
Hi Ferruh,
I will work with Stephen on this. For the tone of the list, I believe we
can try different ways to make the tone more friendly, while still being
concise.
What about something like this:
- Avoid including unused headers (process-iwyu.py).
- Keep compiler warnings enabled in the build file.
- Instead of using #ifdef with driver-specific macros, opt for runtime
configuration.
- Document device parameters in the driver guide.
- Make device operations structs 'const'.
- Use dynamic logging.
- Skip DPDK version checks (RTE_VERSION_NUM) in the upstream code.
- Add SPDX license tags and copyright notices on all sides.
- Don’t introduce public APIs directly from the driver.
It's slightly more friendly.
Let me know what you think, I'm open to trying another way.
On Tue, Sep 10, 2024 at 5:16 PM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
> On 9/10/2024 3:58 PM, Nandini Persad wrote:
> > This document was created to assist contributors
> > in creating DPDK drivers, providing suggestions
> > and guidelines for how to upstream effectively.
> >
>
> There are minor differences in this v3 and v2, isn't this version on top
> of v2, can those changes be from Stephen?
>
> <...>
>
> > +
> > +Additional Suggestions
> > +----------------------
> > +
> > +* We recommend using DPDK macros instead of inventing new ones in the
> PMD.
> > +* Do not include unused headers (process-iwyu.py).
> > +* Do not disable compiler warning 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 (via RTE_VERSION_NUM) in the upstream
> code.
> > +* Be sure to have SPDX license tags and copyright notice on each side.
> > +* Do not introduce public Apis directly from the driver.
> >
>
> API (Application Programming Interface) is an acronym and should be all
> uppercase, like 'APIs'.
>
> Overall the language in this list is imperative, I think it helps to
> make it simple, but I am not sure about the tone, I wonder if we can do
> better, do you have any suggestions?
>
>
> > +
> > +
> > +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 to
> > +upstream the driver.
> > +
> > +
> > +Test Tools
> > +----------
> > +
> > +Per patch in a patch series, be sure to use the proper test tools.
> > +
> > +* checkpatches.sh
> > +* check-git-log.sh
> > +* check-meson.py
> > +* check-doc-vs-code.sh
> > +* check-spdx-tag.sh
> >
>
> `check-spdx-tag.sh` seems moved in v2 to "additional suggestions", I am
> for keeping it here, as "additional suggestions" are more things to take
> into consideration during design/development, above are actual scripts
> that we can use to verify code.
>
> And long term intention was to move this "tools to run list" to a more
> generic documentation, as these are not really specific to new PMD
> guide, but "additional suggestions" will stay in this document.
>
>
[-- Attachment #2: Type: text/html, Size: 3999 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3] doc: add new driver guidelines
2024-09-11 16:04 ` Nandini Persad
@ 2024-09-12 8:13 ` Ferruh Yigit
2024-09-12 13:18 ` Nandini Persad
0 siblings, 1 reply; 29+ messages in thread
From: Ferruh Yigit @ 2024-09-12 8:13 UTC (permalink / raw)
To: Nandini Persad; +Cc: dev, Thomas Mojalon, Stephen Hemminger
On 9/11/2024 5:04 PM, Nandini Persad wrote:
> Hi Ferruh,
>
> I will work with Stephen on this. For the tone of the list, I believe we
> can try different ways to make the tone more friendly, while still being
> concise.
>
> What about something like this:
> # Avoid including unused headers (process-iwyu.py).
> # Keep compiler warnings enabled in the build file.
> # Instead of using #ifdef with driver-specific macros, opt for runtime
> configuration.
> # Document device parameters in the driver guide.
> # Make device operations structs 'const'.
> # Use dynamic logging.
> # Skip DPDK version checks (RTE_VERSION_NUM) in the upstream code.
> # Add SPDX license tags and copyright notices on all sides.
> # Don’t introduce public APIs directly from the driver.
>
> It's slightly more friendly.
>
> Let me know what you think, I'm open to trying another way.
>
I think above is better.
Another option can be separating it as "Do" and "Do Not" list, as
following, do you think does it help, or makes it harder to understand?
Avoid doing:
- Using PMD specific macros when DPDK ones exist
- Including unused headers
- Disable compiler warnings for driver
- #ifdef with driver-defined macros
- DPDK version checks (via RTE_VERSION_NUM) in the upstream code
- Public APIs directly from the driver
Suggested to do:
- Runtime configuration when applicable
- Document device parameters in the driver guide
- Make device operations struct 'const'
- Dynamic logging
- SPDX license tags and copyright notice on each file
> On Tue, Sep 10, 2024 at 5:16 PM Ferruh Yigit <ferruh.yigit@amd.com
> <mailto:ferruh.yigit@amd.com>> wrote:
>
> On 9/10/2024 3:58 PM, Nandini Persad wrote:
> > This document was created to assist contributors
> > in creating DPDK drivers, providing suggestions
> > and guidelines for how to upstream effectively.
> >
>
> There are minor differences in this v3 and v2, isn't this version on top
> of v2, can those changes be from Stephen?
>
> <...>
>
> > +
> > +Additional Suggestions
> > +----------------------
> > +
> > +* We recommend using DPDK macros instead of inventing new ones in
> the PMD.
> > +* Do not include unused headers (process-iwyu.py).
> > +* Do not disable compiler warning 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 (via RTE_VERSION_NUM) in the
> upstream code.
> > +* Be sure to have SPDX license tags and copyright notice on each
> side.
> > +* Do not introduce public Apis directly from the driver.
> >
>
> API (Application Programming Interface) is an acronym and should be all
> uppercase, like 'APIs'.
>
> Overall the language in this list is imperative, I think it helps to
> make it simple, but I am not sure about the tone, I wonder if we can do
> better, do you have any suggestions?
>
>
> > +
> > +
> > +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 to
> > +upstream the driver.
> > +
> > +
> > +Test Tools
> > +----------
> > +
> > +Per patch in a patch series, be sure to use the proper test tools.
> > +
> > +* checkpatches.sh
> > +* check-git-log.sh
> > +* check-meson.py
> > +* check-doc-vs-code.sh
> > +* check-spdx-tag.sh
> >
>
> `check-spdx-tag.sh` seems moved in v2 to "additional suggestions", I am
> for keeping it here, as "additional suggestions" are more things to take
> into consideration during design/development, above are actual scripts
> that we can use to verify code.
>
> And long term intention was to move this "tools to run list" to a more
> generic documentation, as these are not really specific to new PMD
> guide, but "additional suggestions" will stay in this document.
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3] doc: add new driver guidelines
2024-09-12 8:13 ` Ferruh Yigit
@ 2024-09-12 13:18 ` Nandini Persad
2024-09-12 13:37 ` Ferruh Yigit
0 siblings, 1 reply; 29+ messages in thread
From: Nandini Persad @ 2024-09-12 13:18 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: dev, Thomas Mojalon, Stephen Hemminger
[-- Attachment #1: Type: text/plain, Size: 5027 bytes --]
I like the separation. I can include it in V4 and see, it would be helpful to know if it’s more or less confusing that way.
For the prompt before each list, can we say something like “Avoid doing the following” and “Suggested actions” or something a little better grammatically. We could also just say “Avoid”.
________________________________
From: Ferruh Yigit <ferruh.yigit@amd.com>
Sent: Thursday, September 12, 2024 1:13:33 AM
To: Nandini Persad <nandinipersad361@gmail.com>
Cc: dev@dpdk.org <dev@dpdk.org>; Thomas Mojalon <thomas@monjalon.net>; Stephen Hemminger <stephen@networkplumber.org>
Subject: Re: [PATCH v3] doc: add new driver guidelines
On 9/11/2024 5:04 PM, Nandini Persad wrote:
> Hi Ferruh,
>
> I will work with Stephen on this. For the tone of the list, I believe we
> can try different ways to make the tone more friendly, while still being
> concise.
>
> What about something like this:
> # Avoid including unused headers (process-iwyu.py).
> # Keep compiler warnings enabled in the build file.
> # Instead of using #ifdef with driver-specific macros, opt for runtime
> configuration.
> # Document device parameters in the driver guide.
> # Make device operations structs 'const'.
> # Use dynamic logging.
> # Skip DPDK version checks (RTE_VERSION_NUM) in the upstream code.
> # Add SPDX license tags and copyright notices on all sides.
> # Don’t introduce public APIs directly from the driver.
>
> It's slightly more friendly.
>
> Let me know what you think, I'm open to trying another way.
>
I think above is better.
Another option can be separating it as "Do" and "Do Not" list, as
following, do you think does it help, or makes it harder to understand?
Avoid doing:
- Using PMD specific macros when DPDK ones exist
- Including unused headers
- Disable compiler warnings for driver
- #ifdef with driver-defined macros
- DPDK version checks (via RTE_VERSION_NUM) in the upstream code
- Public APIs directly from the driver
Suggested to do:
- Runtime configuration when applicable
- Document device parameters in the driver guide
- Make device operations struct 'const'
- Dynamic logging
- SPDX license tags and copyright notice on each file
> On Tue, Sep 10, 2024 at 5:16 PM Ferruh Yigit <ferruh.yigit@amd.com
> <mailto:ferruh.yigit@amd.com>> wrote:
>
> On 9/10/2024 3:58 PM, Nandini Persad wrote:
> > This document was created to assist contributors
> > in creating DPDK drivers, providing suggestions
> > and guidelines for how to upstream effectively.
> >
>
> There are minor differences in this v3 and v2, isn't this version on top
> of v2, can those changes be from Stephen?
>
> <...>
>
> > +
> > +Additional Suggestions
> > +----------------------
> > +
> > +* We recommend using DPDK macros instead of inventing new ones in
> the PMD.
> > +* Do not include unused headers (process-iwyu.py).
> > +* Do not disable compiler warning 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 (via RTE_VERSION_NUM) in the
> upstream code.
> > +* Be sure to have SPDX license tags and copyright notice on each
> side.
> > +* Do not introduce public Apis directly from the driver.
> >
>
> API (Application Programming Interface) is an acronym and should be all
> uppercase, like 'APIs'.
>
> Overall the language in this list is imperative, I think it helps to
> make it simple, but I am not sure about the tone, I wonder if we can do
> better, do you have any suggestions?
>
>
> > +
> > +
> > +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 to
> > +upstream the driver.
> > +
> > +
> > +Test Tools
> > +----------
> > +
> > +Per patch in a patch series, be sure to use the proper test tools.
> > +
> > +* checkpatches.sh
> > +* check-git-log.sh
> > +* check-meson.py
> > +* check-doc-vs-code.sh
> > +* check-spdx-tag.sh
> >
>
> `check-spdx-tag.sh` seems moved in v2 to "additional suggestions", I am
> for keeping it here, as "additional suggestions" are more things to take
> into consideration during design/development, above are actual scripts
> that we can use to verify code.
>
> And long term intention was to move this "tools to run list" to a more
> generic documentation, as these are not really specific to new PMD
> guide, but "additional suggestions" will stay in this document.
>
[-- Attachment #2: Type: text/html, Size: 7927 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3] doc: add new driver guidelines
2024-09-12 13:18 ` Nandini Persad
@ 2024-09-12 13:37 ` Ferruh Yigit
2024-09-12 13:40 ` Nandini Persad
0 siblings, 1 reply; 29+ messages in thread
From: Ferruh Yigit @ 2024-09-12 13:37 UTC (permalink / raw)
To: Nandini Persad; +Cc: dev, Thomas Mojalon, Stephen Hemminger
On 9/12/2024 2:18 PM, Nandini Persad wrote:
> I like the separation. I can include it in V4 and see, it would be
> helpful to know if it’s more or less confusing that way.
>
> For the prompt before each list, can we say something like “Avoid doing
> the following” and “Suggested actions” or something a little better
> grammatically. We could also just say “Avoid”.
>
Agree to have better header for the lists, what about:
"Avoid doing the following" and
"Remember to do the following"
Or we can go with whatever you think more convenient.
> ------------------------------------------------------------------------
> *From:* Ferruh Yigit <ferruh.yigit@amd.com>
> *Sent:* Thursday, September 12, 2024 1:13:33 AM
> *To:* Nandini Persad <nandinipersad361@gmail.com>
> *Cc:* dev@dpdk.org <dev@dpdk.org>; Thomas Mojalon <thomas@monjalon.net>;
> Stephen Hemminger <stephen@networkplumber.org>
> *Subject:* Re: [PATCH v3] doc: add new driver guidelines
>
> On 9/11/2024 5:04 PM, Nandini Persad wrote:
>> Hi Ferruh,
>>
>> I will work with Stephen on this. For the tone of the list, I believe we
>> can try different ways to make the tone more friendly, while still being
>> concise.
>>
>> What about something like this:
>> # Avoid including unused headers (process-iwyu.py).
>> # Keep compiler warnings enabled in the build file.
>> # Instead of using #ifdef with driver-specific macros, opt for runtime
>> configuration.
>> # Document device parameters in the driver guide.
>> # Make device operations structs 'const'.
>> # Use dynamic logging.
>> # Skip DPDK version checks (RTE_VERSION_NUM) in the upstream code.
>> # Add SPDX license tags and copyright notices on all sides.
>> # Don’t introduce public APIs directly from the driver.
>>
>> It's slightly more friendly.
>>
>> Let me know what you think, I'm open to trying another way.
>>
>
> I think above is better.
>
> Another option can be separating it as "Do" and "Do Not" list, as
> following, do you think does it help, or makes it harder to understand?
>
> Avoid doing:
> - Using PMD specific macros when DPDK ones exist
> - Including unused headers
> - Disable compiler warnings for driver
> - #ifdef with driver-defined macros
> - DPDK version checks (via RTE_VERSION_NUM) in the upstream code
> - Public APIs directly from the driver
>
> Suggested to do:
> - Runtime configuration when applicable
> - Document device parameters in the driver guide
> - Make device operations struct 'const'
> - Dynamic logging
> - SPDX license tags and copyright notice on each file
>
>
>> On Tue, Sep 10, 2024 at 5:16 PM Ferruh Yigit <ferruh.yigit@amd.com
>> <mailto:ferruh.yigit@amd.com <mailto:ferruh.yigit@amd.com>>> wrote:
>>
>> On 9/10/2024 3:58 PM, Nandini Persad wrote:
>> > This document was created to assist contributors
>> > in creating DPDK drivers, providing suggestions
>> > and guidelines for how to upstream effectively.
>> >
>>
>> There are minor differences in this v3 and v2, isn't this version on top
>> of v2, can those changes be from Stephen?
>>
>> <...>
>>
>> > +
>> > +Additional Suggestions
>> > +----------------------
>> > +
>> > +* We recommend using DPDK macros instead of inventing new ones in
>> the PMD.
>> > +* Do not include unused headers (process-iwyu.py).
>> > +* Do not disable compiler warning 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 (via RTE_VERSION_NUM) in the
>> upstream code.
>> > +* Be sure to have SPDX license tags and copyright notice on each
>> side.
>> > +* Do not introduce public Apis directly from the driver.
>> >
>>
>> API (Application Programming Interface) is an acronym and should be all
>> uppercase, like 'APIs'.
>>
>> Overall the language in this list is imperative, I think it helps to
>> make it simple, but I am not sure about the tone, I wonder if we can do
>> better, do you have any suggestions?
>>
>>
>> > +
>> > +
>> > +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 to
>> > +upstream the driver.
>> > +
>> > +
>> > +Test Tools
>> > +----------
>> > +
>> > +Per patch in a patch series, be sure to use the proper test tools.
>> > +
>> > +* checkpatches.sh
>> > +* check-git-log.sh
>> > +* check-meson.py
>> > +* check-doc-vs-code.sh
>> > +* check-spdx-tag.sh
>> >
>>
>> `check-spdx-tag.sh` seems moved in v2 to "additional suggestions", I am
>> for keeping it here, as "additional suggestions" are more things to take
>> into consideration during design/development, above are actual scripts
>> that we can use to verify code.
>>
>> And long term intention was to move this "tools to run list" to a more
>> generic documentation, as these are not really specific to new PMD
>> guide, but "additional suggestions" will stay in this document.
>>
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3] doc: add new driver guidelines
2024-09-12 13:37 ` Ferruh Yigit
@ 2024-09-12 13:40 ` Nandini Persad
0 siblings, 0 replies; 29+ messages in thread
From: Nandini Persad @ 2024-09-12 13:40 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: dev, Thomas Mojalon, Stephen Hemminger
[-- Attachment #1: Type: text/plain, Size: 5905 bytes --]
Excellent. Will send the update in accordingly.
________________________________
From: Ferruh Yigit <ferruh.yigit@amd.com>
Sent: Thursday, September 12, 2024 6:37:35 AM
To: Nandini Persad <nandinipersad361@gmail.com>
Cc: dev@dpdk.org <dev@dpdk.org>; Thomas Mojalon <thomas@monjalon.net>; Stephen Hemminger <stephen@networkplumber.org>
Subject: Re: [PATCH v3] doc: add new driver guidelines
On 9/12/2024 2:18 PM, Nandini Persad wrote:
> I like the separation. I can include it in V4 and see, it would be
> helpful to know if it’s more or less confusing that way.
>
> For the prompt before each list, can we say something like “Avoid doing
> the following” and “Suggested actions” or something a little better
> grammatically. We could also just say “Avoid”.
>
Agree to have better header for the lists, what about:
"Avoid doing the following" and
"Remember to do the following"
Or we can go with whatever you think more convenient.
> ------------------------------------------------------------------------
> *From:* Ferruh Yigit <ferruh.yigit@amd.com>
> *Sent:* Thursday, September 12, 2024 1:13:33 AM
> *To:* Nandini Persad <nandinipersad361@gmail.com>
> *Cc:* dev@dpdk.org <dev@dpdk.org>; Thomas Mojalon <thomas@monjalon.net>;
> Stephen Hemminger <stephen@networkplumber.org>
> *Subject:* Re: [PATCH v3] doc: add new driver guidelines
>
> On 9/11/2024 5:04 PM, Nandini Persad wrote:
>> Hi Ferruh,
>>
>> I will work with Stephen on this. For the tone of the list, I believe we
>> can try different ways to make the tone more friendly, while still being
>> concise.
>>
>> What about something like this:
>> # Avoid including unused headers (process-iwyu.py).
>> # Keep compiler warnings enabled in the build file.
>> # Instead of using #ifdef with driver-specific macros, opt for runtime
>> configuration.
>> # Document device parameters in the driver guide.
>> # Make device operations structs 'const'.
>> # Use dynamic logging.
>> # Skip DPDK version checks (RTE_VERSION_NUM) in the upstream code.
>> # Add SPDX license tags and copyright notices on all sides.
>> # Don’t introduce public APIs directly from the driver.
>>
>> It's slightly more friendly.
>>
>> Let me know what you think, I'm open to trying another way.
>>
>
> I think above is better.
>
> Another option can be separating it as "Do" and "Do Not" list, as
> following, do you think does it help, or makes it harder to understand?
>
> Avoid doing:
> - Using PMD specific macros when DPDK ones exist
> - Including unused headers
> - Disable compiler warnings for driver
> - #ifdef with driver-defined macros
> - DPDK version checks (via RTE_VERSION_NUM) in the upstream code
> - Public APIs directly from the driver
>
> Suggested to do:
> - Runtime configuration when applicable
> - Document device parameters in the driver guide
> - Make device operations struct 'const'
> - Dynamic logging
> - SPDX license tags and copyright notice on each file
>
>
>> On Tue, Sep 10, 2024 at 5:16 PM Ferruh Yigit <ferruh.yigit@amd.com
>> <mailto:ferruh.yigit@amd.com <mailto:ferruh.yigit@amd.com>>> wrote:
>>
>> On 9/10/2024 3:58 PM, Nandini Persad wrote:
>> > This document was created to assist contributors
>> > in creating DPDK drivers, providing suggestions
>> > and guidelines for how to upstream effectively.
>> >
>>
>> There are minor differences in this v3 and v2, isn't this version on top
>> of v2, can those changes be from Stephen?
>>
>> <...>
>>
>> > +
>> > +Additional Suggestions
>> > +----------------------
>> > +
>> > +* We recommend using DPDK macros instead of inventing new ones in
>> the PMD.
>> > +* Do not include unused headers (process-iwyu.py).
>> > +* Do not disable compiler warning 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 (via RTE_VERSION_NUM) in the
>> upstream code.
>> > +* Be sure to have SPDX license tags and copyright notice on each
>> side.
>> > +* Do not introduce public Apis directly from the driver.
>> >
>>
>> API (Application Programming Interface) is an acronym and should be all
>> uppercase, like 'APIs'.
>>
>> Overall the language in this list is imperative, I think it helps to
>> make it simple, but I am not sure about the tone, I wonder if we can do
>> better, do you have any suggestions?
>>
>>
>> > +
>> > +
>> > +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 to
>> > +upstream the driver.
>> > +
>> > +
>> > +Test Tools
>> > +----------
>> > +
>> > +Per patch in a patch series, be sure to use the proper test tools.
>> > +
>> > +* checkpatches.sh
>> > +* check-git-log.sh
>> > +* check-meson.py
>> > +* check-doc-vs-code.sh
>> > +* check-spdx-tag.sh
>> >
>>
>> `check-spdx-tag.sh` seems moved in v2 to "additional suggestions", I am
>> for keeping it here, as "additional suggestions" are more things to take
>> into consideration during design/development, above are actual scripts
>> that we can use to verify code.
>>
>> And long term intention was to move this "tools to run list" to a more
>> generic documentation, as these are not really specific to new PMD
>> guide, but "additional suggestions" will stay in this document.
>>
>
[-- Attachment #2: Type: text/html, Size: 9344 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v3] doc: add new driver guidelines
2024-08-14 19:08 ` Stephen Hemminger
` (2 preceding siblings ...)
2024-09-10 14:58 ` [PATCH v3] " Nandini Persad
@ 2024-09-12 20:26 ` Stephen Hemminger
2024-09-13 4:19 ` WanRenyong
3 siblings, 1 reply; 29+ messages in thread
From: Stephen Hemminger @ 2024-09-12 20:26 UTC (permalink / raw)
To: dev; +Cc: Nandini Persad, Ferruh Yigit, Thomas Monjalon, Stephen Hemminger
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
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
--
2.45.2
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v3] doc: add new driver guidelines
2024-09-12 20:26 ` Stephen Hemminger
@ 2024-09-13 4:19 ` WanRenyong
2024-09-13 9:07 ` Ferruh Yigit
0 siblings, 1 reply; 29+ messages in thread
From: WanRenyong @ 2024-09-13 4:19 UTC (permalink / raw)
To: Stephen Hemminger, dev; +Cc: Nandini Persad, Ferruh Yigit, Thomas Monjalon
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.
--
Thanks,
WanRenyong
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3] doc: add new driver guidelines
2024-09-13 4:19 ` WanRenyong
@ 2024-09-13 9:07 ` Ferruh Yigit
2024-09-13 16:08 ` Stephen Hemminger
0 siblings, 1 reply; 29+ messages in thread
From: Ferruh Yigit @ 2024-09-13 9:07 UTC (permalink / raw)
To: WanRenyong, Stephen Hemminger, dev; +Cc: Nandini Persad, Thomas Monjalon
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.
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3] doc: add new driver guidelines
2024-09-13 9:07 ` Ferruh Yigit
@ 2024-09-13 16:08 ` Stephen Hemminger
0 siblings, 0 replies; 29+ messages in thread
From: Stephen Hemminger @ 2024-09-13 16:08 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: WanRenyong, dev, Nandini Persad, Thomas Monjalon
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.
^ permalink raw reply [flat|nested] 29+ messages in thread