DPDK patches and discussions
 help / color / mirror / Atom feed
From: Akhil Goyal <gakhil@marvell.com>
To: Stephen Hemminger <stephen@networkplumber.org>,
	"dev@dpdk.org" <dev@dpdk.org>
Cc: Nandini Persad <nandinipersad361@gmail.com>,
	Ferruh Yigit <ferruh.yigit@amd.com>,
	Thomas Monjalon <thomas@monjalon.net>
Subject: RE: [EXTERNAL] [PATCH v2] doc: add new driver guidelines
Date: Thu, 5 Sep 2024 09:16:16 +0000	[thread overview]
Message-ID: <CO6PR18MB44848BCE6933AC04E996C656D89D2@CO6PR18MB4484.namprd18.prod.outlook.com> (raw)
In-Reply-To: <20240814190901.14912-1-stephen@networkplumber.org>

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


  reply	other threads:[~2024-09-05  9:16 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-13 20:12 [PATCH] " Nandini Persad
2024-08-14  2:35 ` [PATCH v2] " Stephen Hemminger
2024-08-14 10:10   ` David Marchand
2024-08-14 19:08   ` Stephen Hemminger
2024-09-05  9:16     ` Akhil Goyal [this message]
2024-09-05  9:49       ` [EXTERNAL] " Ferruh Yigit
2024-09-05  9:52         ` Akhil Goyal
2024-09-06  8:05     ` fengchengwen
2024-09-06  8:27       ` Ferruh Yigit
2024-09-09  1:01         ` fengchengwen
2024-09-10 14:58     ` [PATCH v3] " Nandini Persad
2024-09-11  0:16       ` Ferruh Yigit
2024-09-11 16:04         ` Nandini Persad
2024-09-12  8:13           ` Ferruh Yigit
2024-09-12 13:18             ` Nandini Persad
2024-09-12 13:37               ` Ferruh Yigit
2024-09-12 13:40                 ` Nandini Persad
2024-09-12 20:26     ` Stephen Hemminger
2024-09-13  4:19       ` WanRenyong
2024-09-13  9:07         ` Ferruh Yigit
2024-09-13 16:08           ` Stephen Hemminger
2024-09-16 16:28 ` [PATCH v4] " Stephen Hemminger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CO6PR18MB44848BCE6933AC04E996C656D89D2@CO6PR18MB4484.namprd18.prod.outlook.com \
    --to=gakhil@marvell.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@amd.com \
    --cc=nandinipersad361@gmail.com \
    --cc=stephen@networkplumber.org \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).