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