]>
Commit | Line | Data |
---|---|---|
7c673cae FG |
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 <http://www.kernel.org/doc/Documentation/SubmittingPatches>`_. | |
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 master branch of the repository. | |
24 | ||
25 | The mailing list for DPDK development is `dev@dpdk.org <http://dpdk.org/ml/archives/dev/>`_. | |
26 | Contributors will need to `register for the mailing list <http://dpdk.org/ml/listinfo/dev>`_ in order to submit patches. | |
27 | It is also worth registering for the DPDK `Patchwork <http://dpdk.org/dev/patchwork/project/dpdk/list/>`_ | |
28 | ||
29 | The development process requires some familiarity with the ``git`` version control system. | |
30 | Refer to the `Pro Git Book <http://www.git-scm.com/book/>`_ for further information. | |
31 | ||
32 | ||
33 | Getting the Source Code | |
34 | ----------------------- | |
35 | ||
36 | The 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 | ||
43 | Make your Changes | |
44 | ----------------- | |
45 | ||
46 | Make 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 | ||
69 | Once the changes have been made you should commit them to your local repo. | |
70 | ||
71 | For small changes, that do not require specific explanations, it is better to keep things together in the | |
72 | same patch. | |
73 | Larger changes that require different explanations should be separated into logical patches in a patchset. | |
74 | A good way of thinking about whether a patch should be split is to consider whether the change could be | |
75 | applied without dependencies as a backport. | |
76 | ||
77 | As a guide to how patches should be structured run ``git log`` on similar files. | |
78 | ||
79 | ||
80 | Commit Messages: Subject Line | |
81 | ----------------------------- | |
82 | ||
83 | The first, summary, line of the git commit message becomes the subject line of the patch email. | |
84 | Here 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 | ||
103 | The actual email subject line should be prefixed by ``[PATCH]`` and the version, if greater than v1, | |
104 | for example: ``PATCH v2``. | |
105 | The is generally added by ``git send-email`` or ``git format-patch``, see below. | |
106 | ||
107 | If you are submitting an RFC draft of a feature you can use ``[RFC]`` instead of ``[PATCH]``. | |
108 | An RFC patch doesn't have to be complete. | |
109 | It is intended as a way of getting early feedback. | |
110 | ||
111 | ||
112 | Commit Messages: Body | |
113 | --------------------- | |
114 | ||
115 | Here 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 | ||
160 | In 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 | ||
169 | Creating Patches | |
170 | ---------------- | |
171 | ||
172 | It is possible to send patches directly from git but for new contributors it is recommended to generate the | |
173 | patches with ``git format-patch`` and then when everything looks okay, and the patches have been checked, to | |
174 | send them with ``git send-email``. | |
175 | ||
176 | Here 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 | ||
196 | Cover letters are useful for explaining a patchset and help to generate a logical threading to the patches. | |
197 | Smaller 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 | ||
214 | Version 2 and later of a patchset should also include a short log of the changes so the reviewer knows what has changed. | |
215 | This can be added to the cover letter or the annotations. | |
216 | For 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 | ||
230 | Checking the Patches | |
231 | -------------------- | |
232 | ||
233 | Patches should be checked for formatting and syntax issues using the ``checkpatches.sh`` script in the ``scripts`` | |
234 | directory of the DPDK repo. | |
235 | This uses the Linux kernel development tool ``checkpatch.pl`` which can be obtained by cloning, and periodically, | |
236 | updating the Linux kernel sources. | |
237 | ||
238 | The path to the original Linux script must be set in the environment variable ``DPDK_CHECKPATCH_PATH``. | |
239 | This, and any other configuration variables required by the development tools, are loaded from the following | |
240 | files, in order of preference:: | |
241 | ||
242 | .develconfig | |
243 | ~/.config/dpdk/devel.config | |
244 | /etc/dpdk/devel.config. | |
245 | ||
246 | Once the environment variable the script can be run as follows:: | |
247 | ||
248 | scripts/checkpatches.sh ~/patch/ | |
249 | ||
250 | The script usage is:: | |
251 | ||
252 | checkpatches.sh [-h] [-q] [-v] [patch1 [patch2] ...]]" | |
253 | ||
254 | Where: | |
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 | ||
261 | Then the git logs should be checked using the ``check-git-log.sh`` script. | |
262 | ||
263 | The script usage is:: | |
264 | ||
265 | check-git-log.sh [range] | |
266 | ||
267 | Where the range is a ``git log`` option. | |
268 | ||
269 | ||
270 | .. _contrib_check_compilation: | |
271 | ||
272 | Checking Compilation | |
273 | -------------------- | |
274 | ||
275 | Compilation of patches and changes should be tested using the the ``test-build.sh`` script in the ``scripts`` | |
276 | directory of the DPDK repo:: | |
277 | ||
278 | scripts/test-build.sh x86_64-native-linuxapp-gcc+next+shared | |
279 | ||
280 | The script usage is:: | |
281 | ||
282 | test-build.sh [-h] [-jX] [-s] [config1 [config2] ...]] | |
283 | ||
284 | Where: | |
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 | ||
291 | Examples 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 | ||
297 | The 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 | ||
306 | These can be set from the command line or in the config files shown above in the :ref:`contrib_checkpatch`. | |
307 | ||
308 | The 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 | ||
319 | Sending Patches | |
320 | --------------- | |
321 | ||
322 | Patches should be sent to the mailing list using ``git send-email``. | |
323 | You 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 | ||
331 | See the `Git send-email <https://git-scm.com/docs/git-send-email>`_ documentation for more details. | |
332 | ||
333 | The patches should be sent to ``dev@dpdk.org``. | |
334 | If the patches are a change to existing files then you should send them TO the maintainer(s) and CC ``dev@dpdk.org``. | |
335 | The 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 | ||
339 | New additions can be sent without a maintainer:: | |
340 | ||
341 | git send-email --to dev@dpdk.org 000*.patch | |
342 | ||
343 | You can test the emails by sending it to yourself or with the ``--dry-run`` option. | |
344 | ||
345 | If 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 | ||
349 | The Message ID can be found in the raw text of emails or at the top of each Patchwork patch, | |
350 | `for example <http://dpdk.org/dev/patchwork/patch/7646/>`_. | |
351 | Shallow threading (``--thread --no-chain-reply-to``) is preferred for a patch series. | |
352 | ||
353 | Once submitted your patches will appear on the mailing list and in Patchwork. | |
354 | ||
355 | Experienced committers may send patches directly with ``git send-email`` without the ``git format-patch`` step. | |
356 | The options ``--annotate`` and ``confirm = always`` are recommended for checking patches before sending. | |
357 | ||
358 | ||
359 | The Review Process | |
360 | ------------------ | |
361 | ||
362 | The more work you put into the previous steps the easier it will be to get a patch accepted. | |
363 | ||
364 | The 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. |