]>
Commit | Line | Data |
---|---|---|
d0e53b15 SF |
1 | .. |
2 | Licensed under the Apache License, Version 2.0 (the "License"); you may | |
3 | not use this file except in compliance with the License. You may obtain | |
4 | a copy of the License at | |
5 | ||
6 | http://www.apache.org/licenses/LICENSE-2.0 | |
7 | ||
8 | Unless required by applicable law or agreed to in writing, software | |
9 | distributed under the License is distributed on an "AS IS" BASIS, WITHOUT | |
10 | WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the | |
11 | License for the specific language governing permissions and limitations | |
12 | under the License. | |
13 | ||
14 | Convention for heading levels in Open vSwitch documentation: | |
15 | ||
16 | ======= Heading 0 (reserved for the title in a document) | |
17 | ------- Heading 1 | |
18 | ~~~~~~~ Heading 2 | |
19 | +++++++ Heading 3 | |
20 | ''''''' Heading 4 | |
21 | ||
22 | Avoid deeper levels because they do not render well. | |
23 | ||
24 | ================== | |
25 | Submitting Patches | |
26 | ================== | |
27 | ||
28 | Send changes to Open vSwitch as patches to dev@openvswitch.org. One patch per | |
29 | email. More details are included below. | |
30 | ||
31 | If you are using Git, then `git format-patch` takes care of most of the | |
32 | mechanics described below for you. | |
33 | ||
34 | Before You Start | |
35 | ---------------- | |
36 | ||
37 | Before you send patches at all, make sure that each patch makes sense. In | |
38 | particular: | |
39 | ||
40 | - A given patch should not break anything, even if later patches fix the | |
41 | problems that it causes. The source tree should still build and work after | |
42 | each patch is applied. (This enables `git bisect` to work best.) | |
43 | ||
44 | - A patch should make one logical change. Don't make multiple, logically | |
45 | unconnected changes to disparate subsystems in a single patch. | |
46 | ||
80d590ec BP |
47 | - A patch that adds or removes user-visible features should also |
48 | update the appropriate user documentation or manpages. Consider | |
49 | adding an item to NEWS for nontrivial changes. Check "Feature | |
50 | Deprecation Guidelines" section in this document if you intend to | |
51 | remove user-visible feature. | |
d0e53b15 SF |
52 | |
53 | Testing is also important: | |
54 | ||
80d590ec BP |
55 | - Test a patch that modifies existing code with ``make check`` before |
56 | submission. Refer to the "Unit Tests" in :doc:`/topics/testing`, for more | |
57 | information. We also encourage running the kernel and userspace system | |
58 | tests. | |
d0e53b15 | 59 | |
80d590ec | 60 | - Consider testing a patch that adds or deletes files with ``make |
d0e53b15 SF |
61 | distcheck`` before submission. |
62 | ||
63 | - A patch that modifies Linux kernel code should be at least build-tested on | |
64 | various Linux kernel versions before submission. I suggest versions 3.10 and | |
65 | whatever the current latest release version is at the time. | |
66 | ||
80d590ec BP |
67 | - A patch that adds a new feature should add appropriate tests for the |
68 | feature. A bug fix patch should preferably add a test that would | |
69 | fail if the bug recurs. | |
d0e53b15 | 70 | |
6cb2f5a6 IM |
71 | If you are using GitHub, then you may utilize the travis-ci.org and the GitHub |
72 | Actions CI build systems. They will run some of the above tests automatically | |
73 | when you push changes to your repository. See the "Continuous Integration with | |
74 | Travis-CI" in :doc:`/topics/testing` for details on how to set it up. | |
d0e53b15 SF |
75 | |
76 | Email Subject | |
77 | ------------- | |
78 | ||
79 | The subject line of your email should be in the following format: | |
80 | ||
81 | [PATCH <n>/<m>] <area>: <summary> | |
82 | ||
83 | Where: | |
84 | ||
85 | ``[PATCH <n>/<m>]``: | |
86 | indicates that this is the nth of a series of m patches. It helps reviewers | |
87 | to read patches in the correct order. You may omit this prefix if you are | |
88 | sending only one patch. | |
89 | ||
90 | ``<area>``: | |
91 | indicates the area of the Open vSwitch to which the change applies (often the | |
92 | name of a source file or a directory). You may omit it if the change crosses | |
93 | multiple distinct pieces of code. | |
94 | ||
95 | ``<summary>``: | |
96 | ||
dfec5030 | 97 | briefly describes the change. Use the imperative form, |
d0e53b15 SF |
98 | e.g. "Force SNAT for multiple gateway routers." or "Fix daemon exit |
99 | for bad datapaths or flows." Try to keep the summary short, about | |
100 | 50 characters wide. | |
101 | ||
102 | The subject, minus the ``[PATCH <n>/<m>]`` prefix, becomes the first line of | |
103 | the commit's change log message. | |
104 | ||
105 | Description | |
106 | ----------- | |
107 | ||
108 | The body of the email should start with a more thorough description of the | |
109 | change. This becomes the body of the commit message, following the subject. | |
110 | There is no need to duplicate the summary given in the subject. | |
111 | ||
112 | Please limit lines in the description to 75 characters in width. That | |
113 | allows the description to format properly even when indented (e.g. by | |
114 | "git log" or in email quotations). | |
115 | ||
116 | The description should include: | |
117 | ||
118 | - The rationale for the change. | |
119 | ||
120 | - Design description and rationale (but this might be better added as code | |
121 | comments). | |
122 | ||
123 | - Testing that you performed (or testing that should be done but you could not | |
124 | for whatever reason). | |
125 | ||
126 | - Tags (see below). | |
127 | ||
128 | There is no need to describe what the patch actually changed, if the reader can | |
129 | see it for himself. | |
130 | ||
131 | If the patch refers to a commit already in the Open vSwitch repository, please | |
132 | include both the commit number and the subject of the patch, e.g. 'commit | |
133 | 632d136c (vswitch: Remove restriction on datapath names.)'. | |
134 | ||
135 | If you, the person sending the patch, did not write the patch yourself, then | |
136 | the very first line of the body should take the form ``From: <author name> | |
137 | <author email>``, followed by a blank line. This will automatically cause the | |
138 | named author to be credited with authorship in the repository. | |
139 | ||
140 | Tags | |
141 | ---- | |
142 | ||
143 | The description ends with a series of tags, written one to a line as the last | |
144 | paragraph of the email. Each tag indicates some property of the patch in an | |
145 | easily machine-parseable manner. | |
146 | ||
f992b097 BP |
147 | Please don't wrap a tag across multiple lines. If necessary, it's OK to have a |
148 | tag extend beyond the customary maximum width of a commit message. | |
149 | ||
d0e53b15 SF |
150 | Examples of common tags follow. |
151 | ||
152 | ``Signed-off-by: Author Name <author.name@email.address...>`` | |
153 | ||
154 | Informally, this indicates that Author Name is the author or submitter of a | |
155 | patch and has the authority to submit it under the terms of the license. The | |
156 | formal meaning is to agree to the Developer's Certificate of Origin (see | |
157 | below). | |
158 | ||
159 | If the author and submitter are different, each must sign off. If the patch | |
160 | has more than one author, all must sign off. | |
161 | ||
fd5261d1 BP |
162 | Signed-off-by tags should be the last tags in the commit message. If the |
163 | author (or authors) and submitter are different, the author tags should come | |
164 | first. More generally, occasionally a patch might pass through a chain of | |
165 | submitters, and in such a case the sign-offs should be arranged in | |
166 | chronological order. | |
167 | ||
d0e53b15 SF |
168 | :: |
169 | ||
170 | Signed-off-by: Author Name <author.name@email.address...> | |
171 | Signed-off-by: Submitter Name <submitter.name@email.address...> | |
172 | ||
173 | ``Co-authored-by: Author Name <author.name@email.address...>`` | |
174 | ||
175 | Git can only record a single person as the author of a given patch. In the | |
176 | rare event that a patch has multiple authors, one must be given the credit in | |
177 | Git and the others must be credited via Co-authored-by: tags. (All | |
178 | co-authors must also sign off.) | |
179 | ||
180 | ``Acked-by: Reviewer Name <reviewer.name@email.address...>`` | |
181 | ||
182 | Reviewers will often give an ``Acked-by:`` tag to code of which they approve. | |
183 | It is polite for the submitter to add the tag before posting the next version | |
184 | of the patch or applying the patch to the repository. Quality reviewing is | |
185 | hard work, so this gives a small amount of credit to the reviewer. | |
186 | ||
187 | Not all reviewers give ``Acked-by:`` tags when they provide positive reviews. | |
188 | It's customary only to add tags from reviewers who actually provide them | |
189 | explicitly. | |
190 | ||
191 | ``Tested-by: Tester Name <reviewer.name@email.address...>`` | |
192 | ||
193 | When someone tests a patch, it is customary to add a Tested-by: tag | |
194 | indicating that. It's rare for a tester to actually provide the tag; usually | |
195 | the patch submitter makes the tag himself in response to an email indicating | |
196 | successful testing results. | |
197 | ||
198 | ``Tested-at: <URL>`` | |
199 | ||
200 | When a test report is publicly available, this provides a way to reference | |
201 | it. Typical <URL>s would be build logs from autobuilders or references to | |
202 | mailing list archives. | |
203 | ||
204 | Some autobuilders only retain their logs for a limited amount of time. It is | |
205 | less useful to cite these because they may be dead links for a developer | |
206 | reading the commit message months or years later. | |
207 | ||
208 | ``Reported-by: Reporter Name <reporter.name@email.address...>`` | |
209 | ||
210 | When a patch fixes a bug reported by some person, please credit the reporter | |
211 | in the commit log in this fashion. Please also add the reporter's name and | |
212 | email address to the list of people who provided helpful bug reports in the | |
213 | AUTHORS file at the top of the source tree. | |
214 | ||
215 | Fairly often, the reporter of a bug also tests the fix. Occasionally one | |
216 | sees a combined "Reported-and-tested-by:" tag used to indicate this. It is | |
217 | also acceptable, and more common, to include both tags separately. | |
218 | ||
219 | (If a bug report is received privately, it might not always be appropriate to | |
220 | publicly credit the reporter. If in doubt, please ask the reporter.) | |
221 | ||
222 | ``Requested-by: Requester Name <requester.name@email.address...>`` | |
223 | ||
224 | When a patch implements a request or a suggestion made by some | |
225 | person, please credit that person in the commit log in this | |
226 | fashion. For a helpful suggestion, please also add the | |
227 | person's name and email address to the list of people who | |
228 | provided suggestions in the AUTHORS file at the top of the | |
229 | source tree. | |
230 | ||
231 | (If a suggestion or a request is received privately, it might | |
232 | not always be appropriate to publicly give credit. If in | |
233 | doubt, please ask.) | |
234 | ||
235 | ``Suggested-by: Suggester Name <suggester.name@email.address...>`` | |
236 | ||
237 | See ``Requested-by:``. | |
238 | ||
239 | ``CC: Person <name@email>`` | |
240 | ||
241 | This is a way to tag a patch for the attention of a person | |
242 | when no more specific tag is appropriate. One use is to | |
243 | request a review from a particular person. It doesn't make | |
244 | sense to include the same person in CC and another tag, so | |
245 | e.g. if someone who is CCed later provides an Acked-by, add | |
246 | the Acked-by and remove the CC at the same time. | |
247 | ||
248 | ``Reported-at: <URL>`` | |
249 | ||
250 | If a patch fixes or is otherwise related to a bug reported in | |
251 | a public bug tracker, please include a reference to the bug in | |
252 | the form of a URL to the specific bug, e.g.: | |
253 | ||
254 | :: | |
255 | ||
256 | Reported-at: https://bugs.debian.org/743635 | |
257 | ||
258 | This is also an appropriate way to refer to bug report emails | |
259 | in public email archives, e.g.: | |
260 | ||
261 | :: | |
262 | ||
8a7903c6 | 263 | Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2014-June/284495.html |
d0e53b15 SF |
264 | |
265 | ``Submitted-at: <URL>`` | |
266 | ||
267 | If a patch was submitted somewhere other than the Open vSwitch | |
268 | development mailing list, such as a GitHub pull request, this header can | |
269 | be used to reference the source. | |
270 | ||
271 | :: | |
272 | ||
273 | Submitted-at: https://github.com/openvswitch/ovs/pull/92 | |
274 | ||
275 | ``VMware-BZ: #1234567`` | |
276 | ||
277 | If a patch fixes or is otherwise related to a bug reported in | |
278 | a private bug tracker, you may include some tracking ID for | |
279 | the bug for your own reference. Please include some | |
280 | identifier to make the origin clear, e.g. "VMware-BZ" refers | |
281 | to VMware's internal Bugzilla instance and "ONF-JIRA" refers | |
282 | to the Open Networking Foundation's JIRA bug tracker. | |
283 | ||
284 | ``ONF-JIRA: EXT-12345`` | |
285 | ||
286 | See ``VMware-BZ:``. | |
287 | ||
288 | ``Bug #1234567.`` | |
289 | ||
290 | These are obsolete forms of VMware-BZ: that can still be seen | |
291 | in old change log entries. (They are obsolete because they do | |
292 | not tell the reader what bug tracker is referred to.) | |
293 | ||
294 | ``Issue: 1234567`` | |
295 | ||
296 | See ``Bug:``. | |
297 | ||
298 | ``Fixes: 63bc9fb1c69f (“packets: Reorder CS_* flags to remove gap.”)`` | |
299 | ||
300 | If you would like to record which commit introduced a bug being fixed, | |
301 | you may do that with a “Fixes” header. This assists in determining | |
302 | which OVS releases have the bug, so the patch can be applied to all | |
303 | affected versions. The easiest way to generate the header in the | |
304 | proper format is with this git command. This command also CCs the | |
305 | author of the commit being fixed, which makes sense unless the | |
306 | author also made the fix or is already named in another tag: | |
307 | ||
308 | :: | |
309 | ||
310 | $ git log -1 --pretty=format:"CC: %an <%ae>%nFixes: %h (\"%s\")" \ | |
311 | --abbrev=12 COMMIT_REF | |
312 | ||
313 | ``Vulnerability: CVE-2016-2074`` | |
314 | ||
315 | Specifies that the patch fixes or is otherwise related to a | |
316 | security vulnerability with the given CVE identifier. Other | |
317 | identifiers in public vulnerability databases are also | |
318 | suitable. | |
319 | ||
320 | If the vulnerability was reported publicly, then it is also | |
321 | appropriate to cite the URL to the report in a Reported-at | |
322 | tag. Use a Reported-by tag to acknowledge the reporters. | |
323 | ||
324 | Developer's Certificate of Origin | |
325 | --------------------------------- | |
326 | ||
327 | To help track the author of a patch as well as the submission chain, and be | |
328 | clear that the developer has authority to submit a patch for inclusion in | |
dfec5030 | 329 | Open vSwitch please sign off your work. The sign off certifies the following: |
d0e53b15 SF |
330 | |
331 | :: | |
332 | ||
333 | Developer's Certificate of Origin 1.1 | |
334 | ||
335 | By making a contribution to this project, I certify that: | |
336 | ||
337 | (a) The contribution was created in whole or in part by me and I | |
338 | have the right to submit it under the open source license | |
339 | indicated in the file; or | |
340 | ||
341 | (b) The contribution is based upon previous work that, to the best | |
342 | of my knowledge, is covered under an appropriate open source | |
343 | license and I have the right under that license to submit that | |
344 | work with modifications, whether created in whole or in part | |
345 | by me, under the same open source license (unless I am | |
346 | permitted to submit under a different license), as indicated | |
347 | in the file; or | |
348 | ||
349 | (c) The contribution was provided directly to me by some other | |
350 | person who certified (a), (b) or (c) and I have not modified | |
351 | it. | |
352 | ||
353 | (d) I understand and agree that this project and the contribution | |
354 | are public and that a record of the contribution (including all | |
355 | personal information I submit with it, including my sign-off) is | |
356 | maintained indefinitely and may be redistributed consistent with | |
357 | this project or the open source license(s) involved. | |
358 | ||
359 | See also http://developercertificate.org/. | |
360 | ||
361 | Feature Deprecation Guidelines | |
362 | ------------------------------ | |
363 | ||
364 | Open vSwitch is intended to be user friendly. This means that under normal | |
365 | circumstances we don't abruptly remove features from OVS that some users might | |
366 | still be using. Otherwise, if we would, then we would possibly break our user | |
367 | setup when they upgrade and would receive bug reports. | |
368 | ||
369 | Typical process to deprecate a feature in Open vSwitch is to: | |
370 | ||
371 | (a) Mention deprecation of a feature in the NEWS file. Also, mention expected | |
372 | release or absolute time when this feature would be removed from OVS | |
373 | altogether. Don't use relative time (e.g. "in 6 months") because that is | |
374 | not clearly interpretable. | |
375 | ||
376 | (b) If Open vSwitch is configured to use deprecated feature it should print | |
377 | a warning message to the log files clearly indicating that feature is | |
378 | deprecated and that use of it should be avoided. | |
379 | ||
380 | (c) If this feature is mentioned in man pages, then add "Deprecated" keyword | |
381 | to it. | |
382 | ||
383 | Also, if there is alternative feature to the one that is about to be marked as | |
384 | deprecated, then mention it in (a), (b) and (c) as well. | |
385 | ||
386 | Remember to follow-up and actually remove the feature from OVS codebase once | |
387 | deprecation grace period has expired and users had opportunity to use at least | |
388 | one OVS release that would have informed them about feature deprecation! | |
389 | ||
390 | Comments | |
391 | -------- | |
392 | ||
393 | If you want to include any comments in your email that should not be part of | |
394 | the commit's change log message, put them after the description, separated by a | |
0fb03722 | 395 | line that contains just ``---``. It may be helpful to include a diffstat here |
d0e53b15 SF |
396 | for changes that touch multiple files. |
397 | ||
398 | Patch | |
399 | ----- | |
400 | ||
401 | The patch should be in the body of the email following the description, | |
402 | separated by a blank line. | |
403 | ||
404 | Patches should be in ``diff -up`` format. We recommend that you use Git to | |
405 | produce your patches, in which case you should use the ``-M -C`` options to | |
406 | ``git diff`` (or other Git tools) if your patch renames or copies files. | |
407 | `Quilt <http://savannah.nongnu.org/projects/quilt>`__ might be useful if you do | |
408 | not want to use Git. | |
409 | ||
410 | Patches should be inline in the email message. Some email clients corrupt | |
411 | white space or wrap lines in patches. There are hints on how to configure many | |
412 | email clients to avoid this problem on `kernel.org | |
34aa9cf9 SF |
413 | <https://static.lwn.net/kerneldoc/process/email-clients.html>`__. If you |
414 | cannot convince your email client not to mangle patches, then sending the patch | |
415 | as an attachment is a second choice. | |
d0e53b15 SF |
416 | |
417 | Follow the style used in the code that you are modifying. :doc:`coding-style` | |
418 | file describes the coding style used in most of Open vSwitch. Use Linux kernel | |
419 | coding style for Linux kernel code. | |
420 | ||
421 | If your code is non-datapath code, you may use the ``utilities/checkpatch.py`` | |
dfec5030 | 422 | utility as a quick check for certain commonly occurring mistakes (improper |
d0e53b15 | 423 | leading/trailing whitespace, missing signoffs, some improper formatted patch |
dfec5030 | 424 | files). For Linux datapath code, it is a good idea to use the Linux script |
d0e53b15 SF |
425 | ``checkpatch.pl``. |
426 | ||
427 | Example | |
428 | ------- | |
429 | ||
430 | :: | |
431 | ||
432 | From fa29a1c2c17682879e79a21bb0cdd5bbe67fa7c0 Mon Sep 17 00:00:00 2001 | |
433 | From: Jesse Gross <jesse@nicira.com> | |
434 | Date: Thu, 8 Dec 2011 13:17:24 -0800 | |
435 | Subject: [PATCH] datapath: Alphabetize include/net/ipv6.h compat header. | |
436 | ||
437 | Signed-off-by: Jesse Gross <jesse@nicira.com> | |
438 | --- | |
439 | datapath/linux/Modules.mk | 2 +- | |
440 | 1 files changed, 1 insertions(+), 1 deletions(-) | |
441 | ||
442 | diff --git a/datapath/linux/Modules.mk b/datapath/linux/Modules.mk | |
443 | index fdd952e..f6cb88e 100644 | |
444 | --- a/datapath/linux/Modules.mk | |
445 | +++ b/datapath/linux/Modules.mk | |
446 | @@ -56,11 +56,11 @@ openvswitch_headers += \ | |
447 | linux/compat/include/net/dst.h \ | |
448 | linux/compat/include/net/genetlink.h \ | |
449 | linux/compat/include/net/ip.h \ | |
450 | + linux/compat/include/net/ipv6.h \ | |
451 | linux/compat/include/net/net_namespace.h \ | |
452 | linux/compat/include/net/netlink.h \ | |
453 | linux/compat/include/net/protocol.h \ | |
454 | linux/compat/include/net/route.h \ | |
455 | - linux/compat/include/net/ipv6.h \ | |
456 | linux/compat/genetlink.inc | |
457 | ||
458 | both_modules += brcompat | |
459 | -- | |
460 | 1.7.7.3 |