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