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