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