xref: /dpdk/doc/guides/contributing/patches.rst (revision 250c9eb3ca895127f21a729caf4a928eb2f04d2c)
1.. submitting_patches:
2
3Contributing Code to DPDK
4=========================
5
6This document outlines the guidelines for submitting code to DPDK.
7
8The DPDK development process is modelled (loosely) on the Linux Kernel development model so it is worth reading the
9Linux kernel guide on submitting patches:
10`How to Get Your Change Into the Linux Kernel <https://www.kernel.org/doc/html/latest/process/submitting-patches.html>`_.
11The rationale for many of the DPDK guidelines is explained in greater detail in the kernel guidelines.
12
13
14The DPDK Development Process
15----------------------------
16
17The DPDK development process has the following features:
18
19* The code is hosted in a public git repository.
20* There is a mailing list where developers submit patches.
21* There are maintainers for hierarchical components.
22* Patches are reviewed publicly on the mailing list.
23* Successfully reviewed patches are merged to the repository.
24* Patches should be sent to the target repository or sub-tree, see below.
25* All sub-repositories are merged into main repository for ``-rc1`` and ``-rc2`` versions of the release.
26* After the ``-rc2`` release all patches should target the main repository.
27
28The mailing list for DPDK development is `dev@dpdk.org <http://dpdk.org/ml/archives/dev/>`_.
29Contributors will need to `register for the mailing list <http://dpdk.org/ml/listinfo/dev>`_ in order to submit patches.
30It is also worth registering for the DPDK `Patchwork <http://dpdk.org/dev/patchwork/project/dpdk/list/>`_
31
32The development process requires some familiarity with the ``git`` version control system.
33Refer to the `Pro Git Book <http://www.git-scm.com/book/>`_ for further information.
34
35Source License
36--------------
37
38The DPDK uses the Open Source BSD-3-Clause license for the core libraries and
39drivers. The kernel components are GPL-2.0 licensed. DPDK uses single line
40reference to Unique License Identifiers in source files as defined by the Linux
41Foundation's `SPDX project <http://spdx.org/>`_.
42
43DPDK uses first line of the file to be SPDX tag. In case of *#!* scripts, SPDX
44tag can be placed in 2nd line of the file.
45
46For example, to label a file as subject to the BSD-3-Clause license,
47the following text would be used:
48
49``SPDX-License-Identifier: BSD-3-Clause``
50
51To label a file as dual-licensed with BSD-3-Clause and GPL-2.0 (e.g., for code
52that is shared between the kernel and userspace), the following text would be
53used:
54
55``SPDX-License-Identifier: (BSD-3-Clause OR GPL-2.0)``
56
57Refer to ``licenses/README`` for more details.
58
59Maintainers and Sub-trees
60-------------------------
61
62The DPDK maintenance hierarchy is divided into a main repository ``dpdk`` and sub-repositories ``dpdk-next-*``.
63
64There are maintainers for the trees and for components within the tree.
65
66Trees and maintainers are listed in the ``MAINTAINERS`` file. For example::
67
68    Crypto Drivers
69    --------------
70    M: Some Name <some.name@email.com>
71    B: Another Name <another.name@email.com>
72    T: git://dpdk.org/next/dpdk-next-crypto
73
74    Intel AES-NI GCM PMD
75    M: Some One <some.one@email.com>
76    F: drivers/crypto/aesni_gcm/
77    F: doc/guides/cryptodevs/aesni_gcm.rst
78
79Where:
80
81* ``M`` is a tree or component maintainer.
82* ``B`` is a tree backup maintainer.
83* ``T`` is a repository tree.
84* ``F`` is a maintained file or directory.
85
86Additional details are given in the ``MAINTAINERS`` file.
87
88The role of the component maintainers is to:
89
90* Review patches for the component or delegate the review.
91  The review should be done, ideally, within 1 week of submission to the mailing list.
92* Add an ``acked-by`` to patches, or patchsets, that are ready for committing to a tree.
93* Reply to questions asked about the component.
94
95Component maintainers can be added or removed by submitting a patch to the ``MAINTAINERS`` file.
96Maintainers should have demonstrated a reasonable level of contributions or reviews to the component area.
97The maintainer should be confirmed by an ``ack`` from an established contributor.
98There can be more than one component maintainer if desired.
99
100The role of the tree maintainers is to:
101
102* Maintain the overall quality of their tree.
103  This can entail additional review, compilation checks or other tests deemed necessary by the maintainer.
104* Commit patches that have been reviewed by component maintainers and/or other contributors.
105  The tree maintainer should determine if patches have been reviewed sufficiently.
106* Ensure that patches are reviewed in a timely manner.
107* Prepare the tree for integration.
108* Ensure that there is a designated back-up maintainer and coordinate a handover for periods where the
109  tree maintainer can't perform their role.
110
111Tree maintainers can be added or removed by submitting a patch to the ``MAINTAINERS`` file.
112The proposer should justify the need for a new sub-tree and should have demonstrated a sufficient level of contributions in the area or to a similar area.
113The maintainer should be confirmed by an ``ack`` from an existing tree maintainer.
114Disagreements on trees or maintainers can be brought to the Technical Board.
115
116The backup maintainer for the master tree should be selected from the existing sub-tree maintainers from the project.
117The backup maintainer for a sub-tree should be selected from among the component maintainers within that sub-tree.
118
119
120Getting the Source Code
121-----------------------
122
123The source code can be cloned using either of the following:
124
125main repository::
126
127    git clone git://dpdk.org/dpdk
128    git clone http://dpdk.org/git/dpdk
129
130sub-repositories (`list <http://dpdk.org/browse/next>`_)::
131
132    git clone git://dpdk.org/next/dpdk-next-*
133    git clone http://dpdk.org/git/next/dpdk-next-*
134
135Make your Changes
136-----------------
137
138Make your planned changes in the cloned ``dpdk`` repo. Here are some guidelines and requirements:
139
140* Follow the :ref:`coding_style` guidelines.
141
142* If you add new files or directories you should add your name to the ``MAINTAINERS`` file.
143
144* New external functions should be added to the local ``version.map`` file.
145  See the :doc:`Guidelines for ABI policy and versioning </contributing/versioning>`.
146  New external functions should also be added in alphabetical order.
147
148* Important changes will require an addition to the release notes in ``doc/guides/rel_notes/``.
149  See the :ref:`Release Notes section of the Documentation Guidelines <doc_guidelines>` for details.
150
151* Test the compilation works with different targets, compilers and options, see :ref:`contrib_check_compilation`.
152
153* Don't break compilation between commits with forward dependencies in a patchset.
154  Each commit should compile on its own to allow for ``git bisect`` and continuous integration testing.
155
156* Add tests to the ``app/test`` unit test framework where possible.
157
158* Add documentation, if relevant, in the form of Doxygen comments or a User Guide in RST format.
159  See the :ref:`Documentation Guidelines <doc_guidelines>`.
160
161Once the changes have been made you should commit them to your local repo.
162
163For small changes, that do not require specific explanations, it is better to keep things together in the
164same patch.
165Larger changes that require different explanations should be separated into logical patches in a patchset.
166A good way of thinking about whether a patch should be split is to consider whether the change could be
167applied without dependencies as a backport.
168
169As a guide to how patches should be structured run ``git log`` on similar files.
170
171
172Commit Messages: Subject Line
173-----------------------------
174
175The first, summary, line of the git commit message becomes the subject line of the patch email.
176Here are some guidelines for the summary line:
177
178* The summary line must capture the area and the impact of the change.
179
180* The summary line should be around 50 characters.
181
182* The summary line should be lowercase apart from acronyms.
183
184* It should be prefixed with the component name (use git log to check existing components).
185  For example::
186
187     ixgbe: fix offload config option name
188
189     config: increase max queues per port
190
191* Use the imperative of the verb (like instructions to the code base).
192
193* Don't add a period/full stop to the subject line or you will end up two in the patch name: ``dpdk_description..patch``.
194
195The actual email subject line should be prefixed by ``[PATCH]`` and the version, if greater than v1,
196for example: ``PATCH v2``.
197The is generally added by ``git send-email`` or ``git format-patch``, see below.
198
199If you are submitting an RFC draft of a feature you can use ``[RFC]`` instead of ``[PATCH]``.
200An RFC patch doesn't have to be complete.
201It is intended as a way of getting early feedback.
202
203
204Commit Messages: Body
205---------------------
206
207Here are some guidelines for the body of a commit message:
208
209* The body of the message should describe the issue being fixed or the feature being added.
210  It is important to provide enough information to allow a reviewer to understand the purpose of the patch.
211
212* When the change is obvious the body can be blank, apart from the signoff.
213
214* The commit message must end with a ``Signed-off-by:`` line which is added using::
215
216      git commit --signoff # or -s
217
218  The purpose of the signoff is explained in the
219  `Developer's Certificate of Origin <https://www.kernel.org/doc/html/latest/process/submitting-patches.html#developer-s-certificate-of-origin-1-1>`_
220  section of the Linux kernel guidelines.
221
222  .. Note::
223
224     All developers must ensure that they have read and understood the
225     Developer's Certificate of Origin section of the documentation prior
226     to applying the signoff and submitting a patch.
227
228* The signoff must be a real name and not an alias or nickname.
229  More than one signoff is allowed.
230
231* The text of the commit message should be wrapped at 72 characters.
232
233* When fixing a regression, it is required to reference the id of the commit
234  which introduced the bug, and put the original author of that commit on CC.
235  You can generate the required lines using the following git alias, which prints
236  the commit SHA and the author of the original code::
237
238     git config alias.fixline "log -1 --abbrev=12 --format='Fixes: %h (\"%s\")%nCc: %ae'"
239
240  The output of ``git fixline <SHA>`` must then be added to the commit message::
241
242     doc: fix some parameter description
243
244     Update the docs, fixing description of some parameter.
245
246     Fixes: abcdefgh1234 ("doc: add some parameter")
247     Cc: author@example.com
248
249     Signed-off-by: Alex Smith <alex.smith@example.com>
250
251* When fixing an error or warning it is useful to add the error message and instructions on how to reproduce it.
252
253* Use correct capitalization, punctuation and spelling.
254
255In addition to the ``Signed-off-by:`` name the commit messages can also have
256tags for who reported, suggested, tested and reviewed the patch being
257posted. Please refer to the `Tested, Acked and Reviewed by`_ section.
258
259Patch Fix Related Issues
260~~~~~~~~~~~~~~~~~~~~~~~~
261
262`Coverity <https://scan.coverity.com/projects/dpdk-data-plane-development-kit>`_
263is a tool for static code analysis.
264It is used as a cloud-based service used to scan the DPDK source code,
265and alert developers of any potential defects in the source code.
266When fixing an issue found by Coverity, the patch must contain a Coverity issue ID
267in the body of the commit message. For example::
268
269
270     doc: fix some parameter description
271
272     Update the docs, fixing description of some parameter.
273
274     Coverity issue: 12345
275     Fixes: abcdefgh1234 ("doc: add some parameter")
276     Cc: author@example.com
277
278     Signed-off-by: Alex Smith <alex.smith@example.com>
279
280
281`Bugzilla <https://dpdk.org/tracker>`_
282is a bug- or issue-tracking system.
283Bug-tracking systems allow individual or groups of developers
284effectively to keep track of outstanding problems with their product.
285When fixing an issue raised in Bugzilla, the patch must contain
286a Bugzilla issue ID in the body of the commit message.
287For example::
288
289    doc: fix some parameter description
290
291    Update the docs, fixing description of some parameter.
292
293    Bugzilla ID: 12345
294    Fixes: abcdefgh1234 ("doc: add some parameter")
295    Cc: author@example.com
296
297    Signed-off-by: Alex Smith <alex.smith@example.com>
298
299Patch for Stable Releases
300~~~~~~~~~~~~~~~~~~~~~~~~~
301
302All fix patches to the master branch that are candidates for backporting
303should also be CCed to the `stable@dpdk.org <http://dpdk.org/ml/listinfo/stable>`_
304mailing list.
305In the commit message body the Cc: stable@dpdk.org should be inserted as follows::
306
307     doc: fix some parameter description
308
309     Update the docs, fixing description of some parameter.
310
311     Fixes: abcdefgh1234 ("doc: add some parameter")
312     Cc: stable@dpdk.org
313
314     Signed-off-by: Alex Smith <alex.smith@example.com>
315
316For further information on stable contribution you can go to
317:doc:`Stable Contribution Guide <stable>`.
318
319
320Creating Patches
321----------------
322
323It is possible to send patches directly from git but for new contributors it is recommended to generate the
324patches with ``git format-patch`` and then when everything looks okay, and the patches have been checked, to
325send them with ``git send-email``.
326
327Here are some examples of using ``git format-patch`` to generate patches:
328
329.. code-block:: console
330
331   # Generate a patch from the last commit.
332   git format-patch -1
333
334   # Generate a patch from the last 3 commits.
335   git format-patch -3
336
337   # Generate the patches in a directory.
338   git format-patch -3 -o ~/patch/
339
340   # Add a cover letter to explain a patchset.
341   git format-patch -3 -o ~/patch/ --cover-letter
342
343   # Add a prefix with a version number.
344   git format-patch -3 -o ~/patch/ -v 2
345
346
347Cover letters are useful for explaining a patchset and help to generate a logical threading to the patches.
348Smaller notes can be put inline in the patch after the ``---`` separator, for example::
349
350   Subject: [PATCH] fm10k/base: add FM10420 device ids
351
352   Add the device ID for Boulder Rapids and Atwood Channel to enable
353   drivers to support those devices.
354
355   Signed-off-by: Alex Smith <alex.smith@example.com>
356   ---
357
358   ADD NOTES HERE.
359
360    drivers/net/fm10k/base/fm10k_api.c  | 6 ++++++
361    drivers/net/fm10k/base/fm10k_type.h | 6 ++++++
362    2 files changed, 12 insertions(+)
363   ...
364
365Version 2 and later of a patchset should also include a short log of the changes so the reviewer knows what has changed.
366This can be added to the cover letter or the annotations.
367For example::
368
369   ---
370   v3:
371   * Fixed issued with version.map.
372
373   v2:
374   * Added i40e support.
375   * Renamed ethdev functions from rte_eth_ieee15888_*() to rte_eth_timesync_*()
376     since 802.1AS can be supported through the same interfaces.
377
378
379.. _contrib_checkpatch:
380
381Checking the Patches
382--------------------
383
384Patches should be checked for formatting and syntax issues using the ``checkpatches.sh`` script in the ``devtools``
385directory of the DPDK repo.
386This uses the Linux kernel development tool ``checkpatch.pl`` which  can be obtained by cloning, and periodically,
387updating the Linux kernel sources.
388
389The path to the original Linux script must be set in the environment variable ``DPDK_CHECKPATCH_PATH``.
390This, and any other configuration variables required by the development tools, are loaded from the following
391files, in order of preference::
392
393   .develconfig
394   ~/.config/dpdk/devel.config
395   /etc/dpdk/devel.config.
396
397Once the environment variable the script can be run as follows::
398
399   devtools/checkpatches.sh ~/patch/
400
401The script usage is::
402
403   checkpatches.sh [-h] [-q] [-v] [patch1 [patch2] ...]]"
404
405Where:
406
407* ``-h``: help, usage.
408* ``-q``: quiet. Don't output anything for files without issues.
409* ``-v``: verbose.
410* ``patchX``: path to one or more patches.
411
412Then the git logs should be checked using the ``check-git-log.sh`` script.
413
414The script usage is::
415
416   check-git-log.sh [range]
417
418Where the range is a ``git log`` option.
419
420
421.. _contrib_check_compilation:
422
423Checking Compilation
424--------------------
425
426Compilation of patches and changes should be tested using the ``test-build.sh`` script in the ``devtools``
427directory of the DPDK repo::
428
429  devtools/test-build.sh x86_64-native-linuxapp-gcc+next+shared
430
431The script usage is::
432
433   test-build.sh [-h] [-jX] [-s] [config1 [config2] ...]]
434
435Where:
436
437* ``-h``: help, usage.
438* ``-jX``: use X parallel jobs in "make".
439* ``-s``: short test with only first config and without examples/doc.
440* ``config``: default config name plus config switches delimited with a ``+`` sign.
441
442Examples of configs are::
443
444   x86_64-native-linuxapp-gcc
445   x86_64-native-linuxapp-gcc+next+shared
446   x86_64-native-linuxapp-clang+shared
447
448The builds can be modified via the following environmental variables:
449
450* ``DPDK_BUILD_TEST_CONFIGS`` (target1+option1+option2 target2)
451* ``DPDK_DEP_CFLAGS``
452* ``DPDK_DEP_LDFLAGS``
453* ``DPDK_DEP_PCAP`` (y/[n])
454* ``DPDK_NOTIFY`` (notify-send)
455
456These can be set from the command line or in the config files shown above in the :ref:`contrib_checkpatch`.
457
458The recommended configurations and options to test compilation prior to submitting patches are::
459
460   x86_64-native-linuxapp-gcc+shared+next
461   x86_64-native-linuxapp-clang+shared
462   i686-native-linuxapp-gcc
463
464   export DPDK_DEP_ZLIB=y
465   export DPDK_DEP_PCAP=y
466   export DPDK_DEP_SSL=y
467
468
469Sending Patches
470---------------
471
472Patches should be sent to the mailing list using ``git send-email``.
473You can configure an external SMTP with something like the following::
474
475   [sendemail]
476       smtpuser = name@domain.com
477       smtpserver = smtp.domain.com
478       smtpserverport = 465
479       smtpencryption = ssl
480
481See the `Git send-email <https://git-scm.com/docs/git-send-email>`_ documentation for more details.
482
483The patches should be sent to ``dev@dpdk.org``.
484If the patches are a change to existing files then you should send them TO the maintainer(s) and CC ``dev@dpdk.org``.
485The appropriate maintainer can be found in the ``MAINTAINERS`` file::
486
487   git send-email --to maintainer@some.org --cc dev@dpdk.org 000*.patch
488
489Script ``get-maintainer.sh`` can be used to select maintainers automatically::
490
491  git send-email --to-cmd ./devtools/get-maintainer.sh --cc dev@dpdk.org 000*.patch
492
493New additions can be sent without a maintainer::
494
495   git send-email --to dev@dpdk.org 000*.patch
496
497You can test the emails by sending it to yourself or with the ``--dry-run`` option.
498
499If the patch is in relation to a previous email thread you can add it to the same thread using the Message ID::
500
501   git send-email --to dev@dpdk.org --in-reply-to <1234-foo@bar.com> 000*.patch
502
503The Message ID can be found in the raw text of emails or at the top of each Patchwork patch,
504`for example <http://dpdk.org/dev/patchwork/patch/7646/>`_.
505Shallow threading (``--thread --no-chain-reply-to``) is preferred for a patch series.
506
507Once submitted your patches will appear on the mailing list and in Patchwork.
508
509Experienced committers may send patches directly with ``git send-email`` without the ``git format-patch`` step.
510The options ``--annotate`` and ``confirm = always`` are recommended for checking patches before sending.
511
512
513Backporting patches for Stable Releases
514~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
515
516Sometimes a maintainer or contributor wishes, or can be asked, to send a patch
517for a stable release rather than mainline.
518In this case the patch(es) should be sent to ``stable@dpdk.org``,
519not to ``dev@dpdk.org``.
520
521Given that there are multiple stable releases being maintained at the same time,
522please specify exactly which branch(es) the patch is for
523using ``git send-email --subject-prefix='PATCH 16.11' ...``
524and also optionally in the cover letter or in the annotation.
525
526
527The Review Process
528------------------
529
530Patches are reviewed by the community, relying on the experience and
531collaboration of the members to double-check each other's work. There are a
532number of ways to indicate that you have checked a patch on the mailing list.
533
534
535Tested, Acked and Reviewed by
536~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
537
538To indicate that you have interacted with a patch on the mailing list you
539should respond to the patch in an email with one of the following tags:
540
541 * Reviewed-by:
542 * Acked-by:
543 * Tested-by:
544 * Reported-by:
545 * Suggested-by:
546
547The tag should be on a separate line as follows::
548
549   tag-here: Name Surname <email@address.com>
550
551Each of these tags has a specific meaning. In general, the DPDK community
552follows the kernel usage of the tags. A short summary of the meanings of each
553tag is given here for reference:
554
555.. _statement: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#reviewer-s-statement-of-oversight
556
557``Reviewed-by:`` is a strong statement_ that the patch is an appropriate state
558for merging without any remaining serious technical issues. Reviews from
559community members who are known to understand the subject area and to perform
560thorough reviews will increase the likelihood of the patch getting merged.
561
562``Acked-by:`` is a record that the person named was not directly involved in
563the preparation of the patch but wishes to signify and record their acceptance
564and approval of it.
565
566``Tested-by:`` indicates that the patch has been successfully tested (in some
567environment) by the person named.
568
569``Reported-by:`` is used to acknowledge person who found or reported the bug.
570
571``Suggested-by:`` indicates that the patch idea was suggested by the named
572person.
573
574
575
576Steps to getting your patch merged
577~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
578
579The more work you put into the previous steps the easier it will be to get a
580patch accepted. The general cycle for patch review and acceptance is:
581
582#. Submit the patch.
583
584#. Check the automatic test reports in the coming hours.
585
586#. Wait for review comments. While you are waiting review some other patches.
587
588#. Fix the review comments and submit a ``v n+1`` patchset::
589
590      git format-patch -3 -v 2
591
592#. Update Patchwork to mark your previous patches as "Superseded".
593
594#. If the patch is deemed suitable for merging by the relevant maintainer(s) or other developers they will ``ack``
595   the patch with an email that includes something like::
596
597      Acked-by: Alex Smith <alex.smith@example.com>
598
599   **Note**: When acking patches please remove as much of the text of the patch email as possible.
600   It is generally best to delete everything after the ``Signed-off-by:`` line.
601
602#. Having the patch ``Reviewed-by:`` and/or ``Tested-by:`` will also help the patch to be accepted.
603
604#. If the patch isn't deemed suitable based on being out of scope or conflicting with existing functionality
605   it may receive a ``nack``.
606   In this case you will need to make a more convincing technical argument in favor of your patches.
607
608#. In addition a patch will not be accepted if it doesn't address comments from a previous version with fixes or
609   valid arguments.
610
611#. It is the responsibility of a maintainer to ensure that patches are reviewed and to provide an ``ack`` or
612   ``nack`` of those patches as appropriate.
613
614#. Once a patch has been acked by the relevant maintainer, reviewers may still comment on it for a further
615   two weeks. After that time, the patch should be merged into the relevant git tree for the next release.
616   Additional notes and restrictions:
617
618   * Patches should be acked by a maintainer at least two days before the release merge
619     deadline, in order to make that release.
620   * For patches acked with less than two weeks to go to the merge deadline, all additional
621     comments should be made no later than two days before the merge deadline.
622   * After the appropriate time for additional feedback has passed, if the patch has not yet
623     been merged to the relevant tree by the committer, it should be treated as though it had,
624     in that any additional changes needed to it must be addressed by a follow-on patch, rather
625     than rework of the original.
626   * Trivial patches may be merged sooner than described above at the tree committer's
627     discretion.
628
629DPDK Maintainers
630----------------
631
632The following are the DPDK maintainers as listed in the ``MAINTAINERS`` file
633in the DPDK root directory.
634
635.. literalinclude:: ../../../MAINTAINERS
636   :lines: 3-
637