a6b2753c88f3f7d7843a67be2fa2287c6c7bc526
[dpdk.git] / doc / guides / contributing / patches.rst
1 .. submitting_patches:
2
3 Contributing Code to DPDK
4 =========================
5
6 This document outlines the guidelines for submitting code to DPDK.
7
8 The DPDK development process is modelled (loosely) on the Linux Kernel development model so it is worth reading the
9 Linux 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>`_.
11 The rationale for many of the DPDK guidelines is explained in greater detail in the kernel guidelines.
12
13
14 The DPDK Development Process
15 ----------------------------
16
17 The 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
28 The mailing list for DPDK development is `dev@dpdk.org <http://dpdk.org/ml/archives/dev/>`_.
29 Contributors will need to `register for the mailing list <http://dpdk.org/ml/listinfo/dev>`_ in order to submit patches.
30 It is also worth registering for the DPDK `Patchwork <http://dpdk.org/dev/patchwork/project/dpdk/list/>`_
31
32 The development process requires some familiarity with the ``git`` version control system.
33 Refer to the `Pro Git Book <http://www.git-scm.com/book/>`_ for further information.
34
35
36 Maintainers and Sub-trees
37 -------------------------
38
39 The DPDK maintenance hierarchy is divided into a main repository ``dpdk`` and sub-repositories ``dpdk-next-*``.
40
41 There are maintainers for the trees and for components within the tree.
42
43 Trees and maintainers are listed in the ``MAINTAINERS`` file. For example::
44
45     Crypto Drivers
46     --------------
47     M: Some Name <some.name@email.com>
48     B: Another Name <another.name@email.com>
49     T: git://dpdk.org/next/dpdk-next-crypto
50
51     Intel AES-NI GCM PMD
52     M: Some One <some.one@email.com>
53     F: drivers/crypto/aesni_gcm/
54     F: doc/guides/cryptodevs/aesni_gcm.rst
55
56 Where:
57
58 * ``M`` is a tree or component maintainer.
59 * ``B`` is a tree backup maintainer.
60 * ``T`` is a repository tree.
61 * ``F`` is a maintained file or directory.
62
63 Additional details are given in the ``MAINTAINERS`` file.
64
65 The role of the component maintainers is to:
66
67 * Review patches for the component or delegate the review.
68   The review should be done, ideally, within 1 week of submission to the mailing list.
69 * Add an ``acked-by`` to patches, or patchsets, that are ready for committing to a tree.
70
71 Component maintainers can be added or removed by submitting a patch to the ``MAINTAINERS`` file.
72 Maintainers should have demonstrated a reasonable level of contributions or reviews to the component area.
73 The maintainer should be confirmed by an ``ack`` from an established contributor.
74 There can be more than one component maintainer if desired.
75
76 The role of the tree maintainers is to:
77
78 * Maintain the overall quality of their tree.
79   This can entail additional review, compilation checks or other tests deemed necessary by the maintainer.
80 * Commit patches that have been reviewed by component maintainers and/or other contributors.
81   The tree maintainer should determine if patches have been reviewed sufficiently.
82 * Ensure that patches are reviewed in a timely manner.
83 * Prepare the tree for integration.
84 * Ensure that there is a designated back-up maintainer and coordinate a handover for periods where the
85   tree maintainer can't perform their role.
86
87 Tree maintainers can be added or removed by submitting a patch to the ``MAINTAINERS`` file.
88 The 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.
89 The maintainer should be confirmed by an ``ack`` from an existing tree maintainer.
90 Disagreements on trees or maintainers can be brought to the Technical Board.
91
92 The backup maintainer for the master tree should be selected from the existing sub-tree maintainers from the project.
93 The backup maintainer for a sub-tree should be selected from among the component maintainers within that sub-tree.
94
95
96 Getting the Source Code
97 -----------------------
98
99 The source code can be cloned using either of the following:
100
101 main repository::
102
103     git clone git://dpdk.org/dpdk
104     git clone http://dpdk.org/git/dpdk
105
106 sub-repositories (`list <http://dpdk.org/browse/next>`_)::
107
108     git clone git://dpdk.org/next/dpdk-next-*
109     git clone http://dpdk.org/git/next/dpdk-next-*
110
111 Make your Changes
112 -----------------
113
114 Make your planned changes in the cloned ``dpdk`` repo. Here are some guidelines and requirements:
115
116 * Follow the :ref:`coding_style` guidelines.
117
118 * If you add new files or directories you should add your name to the ``MAINTAINERS`` file.
119
120 * New external functions should be added to the local ``version.map`` file.
121   See the :doc:`Guidelines for ABI policy and versioning </contributing/versioning>`.
122   New external functions should also be added in alphabetical order.
123
124 * Important changes will require an addition to the release notes in ``doc/guides/rel_notes/``.
125   See the :ref:`Release Notes section of the Documentation Guidelines <doc_guidelines>` for details.
126
127 * Test the compilation works with different targets, compilers and options, see :ref:`contrib_check_compilation`.
128
129 * Don't break compilation between commits with forward dependencies in a patchset.
130   Each commit should compile on its own to allow for ``git bisect`` and continuous integration testing.
131
132 * Add tests to the the ``app/test`` unit test framework where possible.
133
134 * Add documentation, if relevant, in the form of Doxygen comments or a User Guide in RST format.
135   See the :ref:`Documentation Guidelines <doc_guidelines>`.
136
137 Once the changes have been made you should commit them to your local repo.
138
139 For small changes, that do not require specific explanations, it is better to keep things together in the
140 same patch.
141 Larger changes that require different explanations should be separated into logical patches in a patchset.
142 A good way of thinking about whether a patch should be split is to consider whether the change could be
143 applied without dependencies as a backport.
144
145 As a guide to how patches should be structured run ``git log`` on similar files.
146
147
148 Commit Messages: Subject Line
149 -----------------------------
150
151 The first, summary, line of the git commit message becomes the subject line of the patch email.
152 Here are some guidelines for the summary line:
153
154 * The summary line must capture the area and the impact of the change.
155
156 * The summary line should be around 50 characters.
157
158 * The summary line should be lowercase apart from acronyms.
159
160 * It should be prefixed with the component name (use git log to check existing components).
161   For example::
162
163      ixgbe: fix offload config option name
164
165      config: increase max queues per port
166
167 * Use the imperative of the verb (like instructions to the code base).
168
169 * Don't add a period/full stop to the subject line or you will end up two in the patch name: ``dpdk_description..patch``.
170
171 The actual email subject line should be prefixed by ``[PATCH]`` and the version, if greater than v1,
172 for example: ``PATCH v2``.
173 The is generally added by ``git send-email`` or ``git format-patch``, see below.
174
175 If you are submitting an RFC draft of a feature you can use ``[RFC]`` instead of ``[PATCH]``.
176 An RFC patch doesn't have to be complete.
177 It is intended as a way of getting early feedback.
178
179
180 Commit Messages: Body
181 ---------------------
182
183 Here are some guidelines for the body of a commit message:
184
185 * The body of the message should describe the issue being fixed or the feature being added.
186   It is important to provide enough information to allow a reviewer to understand the purpose of the patch.
187
188 * When the change is obvious the body can be blank, apart from the signoff.
189
190 * The commit message must end with a ``Signed-off-by:`` line which is added using::
191
192       git commit --signoff # or -s
193
194   The purpose of the signoff is explained in the
195   `Developer's Certificate of Origin <https://www.kernel.org/doc/html/latest/process/submitting-patches.html#developer-s-certificate-of-origin-1-1>`_
196   section of the Linux kernel guidelines.
197
198   .. Note::
199
200      All developers must ensure that they have read and understood the
201      Developer's Certificate of Origin section of the documentation prior
202      to applying the signoff and submitting a patch.
203
204 * The signoff must be a real name and not an alias or nickname.
205   More than one signoff is allowed.
206
207 * The text of the commit message should be wrapped at 72 characters.
208
209 * When fixing a regression, it is a good idea to reference the id of the commit which introduced the bug.
210   You can generate the required text using the following git alias::
211
212      git config alias.fixline "log -1 --abbrev=12 --format='Fixes: %h (\"%s\")'"
213
214   The ``Fixes:`` line can then be added to the commit message::
215
216      doc: fix vhost sample parameter
217
218      Update the docs to reflect removed dev-index.
219
220      Fixes: 17b8320a3e11 ("vhost: remove index parameter")
221
222      Signed-off-by: Alex Smith <alex.smith@example.com>
223
224 * When fixing an error or warning it is useful to add the error message and instructions on how to reproduce it.
225
226 * Use correct capitalization, punctuation and spelling.
227
228 In addition to the ``Signed-off-by:`` name the commit messages can also have one or more of the following:
229
230 * ``Reported-by:`` The reporter of the issue.
231 * ``Tested-by:`` The tester of the change.
232 * ``Reviewed-by:`` The reviewer of the change.
233 * ``Suggested-by:`` The person who suggested the change.
234 * ``Acked-by:`` When a previous version of the patch was acked and the ack is still relevant.
235
236
237 Creating Patches
238 ----------------
239
240 It is possible to send patches directly from git but for new contributors it is recommended to generate the
241 patches with ``git format-patch`` and then when everything looks okay, and the patches have been checked, to
242 send them with ``git send-email``.
243
244 Here are some examples of using ``git format-patch`` to generate patches:
245
246 .. code-block:: console
247
248    # Generate a patch from the last commit.
249    git format-patch -1
250
251    # Generate a patch from the last 3 commits.
252    git format-patch -3
253
254    # Generate the patches in a directory.
255    git format-patch -3 -o ~/patch/
256
257    # Add a cover letter to explain a patchset.
258    git format-patch -3 -o ~/patch/ --cover-letter
259
260    # Add a prefix with a version number.
261    git format-patch -3 -o ~/patch/ -v 2
262
263
264 Cover letters are useful for explaining a patchset and help to generate a logical threading to the patches.
265 Smaller notes can be put inline in the patch after the ``---`` separator, for example::
266
267    Subject: [PATCH] fm10k/base: add FM10420 device ids
268
269    Add the device ID for Boulder Rapids and Atwood Channel to enable
270    drivers to support those devices.
271
272    Signed-off-by: Alex Smith <alex.smith@example.com>
273    ---
274
275    ADD NOTES HERE.
276
277     drivers/net/fm10k/base/fm10k_api.c  | 6 ++++++
278     drivers/net/fm10k/base/fm10k_type.h | 6 ++++++
279     2 files changed, 12 insertions(+)
280    ...
281
282 Version 2 and later of a patchset should also include a short log of the changes so the reviewer knows what has changed.
283 This can be added to the cover letter or the annotations.
284 For example::
285
286    ---
287    v3:
288    * Fixed issued with version.map.
289
290    v2:
291    * Added i40e support.
292    * Renamed ethdev functions from rte_eth_ieee15888_*() to rte_eth_timesync_*()
293      since 802.1AS can be supported through the same interfaces.
294
295
296 .. _contrib_checkpatch:
297
298 Checking the Patches
299 --------------------
300
301 Patches should be checked for formatting and syntax issues using the ``checkpatches.sh`` script in the ``devtools``
302 directory of the DPDK repo.
303 This uses the Linux kernel development tool ``checkpatch.pl`` which  can be obtained by cloning, and periodically,
304 updating the Linux kernel sources.
305
306 The path to the original Linux script must be set in the environment variable ``DPDK_CHECKPATCH_PATH``.
307 This, and any other configuration variables required by the development tools, are loaded from the following
308 files, in order of preference::
309
310    .develconfig
311    ~/.config/dpdk/devel.config
312    /etc/dpdk/devel.config.
313
314 Once the environment variable the script can be run as follows::
315
316    devtools/checkpatches.sh ~/patch/
317
318 The script usage is::
319
320    checkpatches.sh [-h] [-q] [-v] [patch1 [patch2] ...]]"
321
322 Where:
323
324 * ``-h``: help, usage.
325 * ``-q``: quiet. Don't output anything for files without issues.
326 * ``-v``: verbose.
327 * ``patchX``: path to one or more patches.
328
329 Then the git logs should be checked using the ``check-git-log.sh`` script.
330
331 The script usage is::
332
333    check-git-log.sh [range]
334
335 Where the range is a ``git log`` option.
336
337
338 .. _contrib_check_compilation:
339
340 Checking Compilation
341 --------------------
342
343 Compilation of patches and changes should be tested using the the ``test-build.sh`` script in the ``devtools``
344 directory of the DPDK repo::
345
346   devtools/test-build.sh x86_64-native-linuxapp-gcc+next+shared
347
348 The script usage is::
349
350    test-build.sh [-h] [-jX] [-s] [config1 [config2] ...]]
351
352 Where:
353
354 * ``-h``: help, usage.
355 * ``-jX``: use X parallel jobs in "make".
356 * ``-s``: short test with only first config and without examples/doc.
357 * ``config``: default config name plus config switches delimited with a ``+`` sign.
358
359 Examples of configs are::
360
361    x86_64-native-linuxapp-gcc
362    x86_64-native-linuxapp-gcc+next+shared
363    x86_64-native-linuxapp-clang+shared
364
365 The builds can be modifies via the following environmental variables:
366
367 * ``DPDK_BUILD_TEST_CONFIGS`` (target1+option1+option2 target2)
368 * ``DPDK_DEP_CFLAGS``
369 * ``DPDK_DEP_LDFLAGS``
370 * ``DPDK_DEP_MOFED`` (y/[n])
371 * ``DPDK_DEP_PCAP`` (y/[n])
372 * ``DPDK_NOTIFY`` (notify-send)
373
374 These can be set from the command line or in the config files shown above in the :ref:`contrib_checkpatch`.
375
376 The recommended configurations and options to test compilation prior to submitting patches are::
377
378    x86_64-native-linuxapp-gcc+shared+next
379    x86_64-native-linuxapp-clang+shared
380    i686-native-linuxapp-gcc
381
382    export DPDK_DEP_ZLIB=y
383    export DPDK_DEP_PCAP=y
384    export DPDK_DEP_SSL=y
385
386
387 Sending Patches
388 ---------------
389
390 Patches should be sent to the mailing list using ``git send-email``.
391 You can configure an external SMTP with something like the following::
392
393    [sendemail]
394        smtpuser = name@domain.com
395        smtpserver = smtp.domain.com
396        smtpserverport = 465
397        smtpencryption = ssl
398
399 See the `Git send-email <https://git-scm.com/docs/git-send-email>`_ documentation for more details.
400
401 The patches should be sent to ``dev@dpdk.org``.
402 If the patches are a change to existing files then you should send them TO the maintainer(s) and CC ``dev@dpdk.org``.
403 The appropriate maintainer can be found in the ``MAINTAINERS`` file::
404
405    git send-email --to maintainer@some.org --cc dev@dpdk.org 000*.patch
406
407 New additions can be sent without a maintainer::
408
409    git send-email --to dev@dpdk.org 000*.patch
410
411 You can test the emails by sending it to yourself or with the ``--dry-run`` option.
412
413 If the patch is in relation to a previous email thread you can add it to the same thread using the Message ID::
414
415    git send-email --to dev@dpdk.org --in-reply-to <1234-foo@bar.com> 000*.patch
416
417 The Message ID can be found in the raw text of emails or at the top of each Patchwork patch,
418 `for example <http://dpdk.org/dev/patchwork/patch/7646/>`_.
419 Shallow threading (``--thread --no-chain-reply-to``) is preferred for a patch series.
420
421 Once submitted your patches will appear on the mailing list and in Patchwork.
422
423 Experienced committers may send patches directly with ``git send-email`` without the ``git format-patch`` step.
424 The options ``--annotate`` and ``confirm = always`` are recommended for checking patches before sending.
425
426
427 The Review Process
428 ------------------
429
430 The more work you put into the previous steps the easier it will be to get a patch accepted.
431
432 The general cycle for patch review and acceptance is:
433
434 #. Submit the patch.
435
436 #. Check the automatic test reports in the coming hours.
437
438 #. Wait for review comments. While you are waiting review some other patches.
439
440 #. Fix the review comments and submit a ``v n+1`` patchset::
441
442       git format-patch -3 -v 2
443
444 #. Update Patchwork to mark your previous patches as "Superseded".
445
446 #. If the patch is deemed suitable for merging by the relevant maintainer(s) or other developers they will ``ack``
447    the patch with an email that includes something like::
448
449       Acked-by: Alex Smith <alex.smith@example.com>
450
451    **Note**: When acking patches please remove as much of the text of the patch email as possible.
452    It is generally best to delete everything after the ``Signed-off-by:`` line.
453
454 #. Having the patch ``Reviewed-by:`` and/or ``Tested-by:`` will also help the patch to be accepted.
455
456 #. If the patch isn't deemed suitable based on being out of scope or conflicting with existing functionality
457    it may receive a ``nack``.
458    In this case you will need to make a more convincing technical argument in favor of your patches.
459
460 #. In addition a patch will not be accepted if it doesn't address comments from a previous version with fixes or
461    valid arguments.
462
463 #. Acked patches will be merged in the current or next merge window.