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