From 55fa0147b1650557471ae3a042727c3aa04efe11 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Fri, 10 Jul 2009 13:38:58 -0700 Subject: [PATCH] Document how to submit patches for Open vSwitch. --- SubmittingPatches | 220 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 220 insertions(+) create mode 100644 SubmittingPatches diff --git a/SubmittingPatches b/SubmittingPatches new file mode 100644 index 00000000..50398f89 --- /dev/null +++ b/SubmittingPatches @@ -0,0 +1,220 @@ +How to Submit Patches for Open vSwitch +====================================== + +Send changes to Open vSwitch as patches to discuss@openvswitch.org. +One patch per email, please. More details are included below. + +If you are using Git, then "git format-patch" takes care of most of +the mechanics described below for you. + +Before You Start +---------------- + +Before you send patches at all, make sure that each patch makes sense. +In particular: + + - A given patch should not break anything, even if later + patches fix the problems that it causes. The source tree + should still build and work after each patch is applied. + (This enables "git bisect" to work best.) + + - A patch should make one logical change. Don't make + multiple, logically unconnected changes to disparate + subsystems in a single patch. + + - A patch that adds or removes user-visible features should + also update the appropriate user documentation or manpages. + +Testing is also important: + + - A patch that adds or deletes files should be tested with + "make distcheck" before submission. + + - A patch that modifies Linux kernel code should be at least + build-tested on various Linux kernel versions before + submission. I suggest versions 2.6.18, 2.6.27, and whatever + the current latest release version is at the time. + + - A patch that modifies the ofproto or vswitchd code should be + tested in at least simple cases before submission. + + - A patch that modifies xenserver code should be tested on + XenServer before submission. + +Email Subject +------------- + +The subject line of your email should be in the following format: +[PATCH /] : + + - [PATCH /] indicates that this is the nth of a series + of m patches. It helps reviewers to read patches in the + correct order. You may omit this prefix if you are sending + only one patch. + + - : indicates the area of the Open vSwitch to which the + change applies (often the name of a source file or a + directory). You may omit it if the change crosses multiple + distinct pieces of code. + + - briefly describes the change. + +The subject, minus the [PATCH /] prefix, becomes the first line +of the commit's change log message. + +Description +----------- + +The body of the email should start with a more thorough description of +the change. This becomes the body of the commit message, following +the subject. There is no need to duplicate the summary given in the +subject. + +Please limit lines in the description to 79 characters in width. + +The description should include: + + - The rationale for the change. + + - Design description and rationale (but this might be better + added as code comments). + + - Testing that you performed (or testing that should be done + but you could not for whatever reason). + +There is no need to describe what the patch actually changed, if the +reader can see it for himself. + +If the patch refers to a commit already in the Open vSwitch +repository, please include both the commit number and the subject of +the patch, e.g. 'commit 632d136c "vswitch: Remove restriction on +datapath names."'. + +If you, the person sending the patch, did not write the patch +yourself, then the very first line of the body should take the form +"From: ", followed by a blank line. This +will automatically cause the named author to be credited with +authorship in the repository. If others contributed to the patch, but +are not the main authors, then please credit them as part of the +description (e.g. "Thanks to Bob J. User for reporting this bug."). + +Comments +-------- + +If you want to include any comments in your email that should not be +part of the commit's change log message, put them after the +description, separated by a line that contains just "---". It may be +helpful to include a diffstat here for changes that touch multiple +files. + +Patch +----- + +The patch should be in the body of the email following the descrition, +separated by a blank line. + +Patches should be in "diff -up" format. We recommend that you use Git +to produce your patches, in which case you should use the -M -C +options to "git diff" (or other Git tools) if your patch renames or +copies files. Quilt (http://savannah.nongnu.org/projects/quilt) might +be useful if you do not want to use Git. + +Patches should be inline in the email message. Some email clients +corrupt white space or wrap lines in patches. There are hints on how +to configure many email clients to avoid this problem at: + http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob_plain;f=Documentation/email-clients.txt +If you cannot convince your email client not to mangle patches, then +sending the patch as an attachment is a second choice. + +Please follow the style used in the code that you are modifying. The +CodingStyle file describes the coding style used in most of Open +vSwitch. Use Linux kernel coding style for Linux kernel code. + +Example +------- + +From 632d136c7b108cd3d39a2e64fe6230e23977caf8 Mon Sep 17 00:00:00 2001 +From: Ben Pfaff +Date: Mon, 6 Jul 2009 10:17:54 -0700 +Subject: [PATCH] vswitch: Remove restriction on datapath names. + +Commit f4b96c92c "vswitch: Disallow bridges named "dpN" or "nl:N"" disabled +naming bridges "dpN" because the vswitchd code made the bad assumption that +the bridge's local port has the same name as the bridge, which was not +true (at the time) for bridges named dpN. Now that assumption has been +eliminated, so this commit eliminates the restriction too. + +This change is also a cleanup in that it eliminates one form of the +vswitch's dependence on specifics of the dpif implementation. +--- + vswitchd/bridge.c | 23 +++++------------------ + vswitchd/ovs-vswitchd.conf.5.in | 3 +-- + 2 files changed, 6 insertions(+), 20 deletions(-) + +diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c +index 32647ea..00cffbc 100644 +--- a/vswitchd/bridge.c ++++ b/vswitchd/bridge.c +@@ -351,32 +351,19 @@ bridge_configure_ssl(void) + void + bridge_reconfigure(void) + { +- struct svec old_br, new_br, raw_new_br; ++ struct svec old_br, new_br; + struct bridge *br, *next; + size_t i, j; + + COVERAGE_INC(bridge_reconfigure); + +- /* Collect old bridges. */ ++ /* Collect old and new bridges. */ + svec_init(&old_br); ++ svec_init(&new_br); + LIST_FOR_EACH (br, struct bridge, node, &all_bridges) { + svec_add(&old_br, br->name); + } +- +- /* Collect new bridges. */ +- svec_init(&raw_new_br); +- cfg_get_subsections(&raw_new_br, "bridge"); +- svec_init(&new_br); +- for (i = 0; i < raw_new_br.n; i++) { +- const char *name = raw_new_br.names[i]; +- if (!strncmp(name, "dp", 2) && isdigit(name[2])) { +- VLOG_ERR("%s is not a valid bridge name (bridges may not be " +- "named \"dp\" followed by a digit)", name); +- } else { +- svec_add(&new_br, name); +- } +- } +- svec_destroy(&raw_new_br); ++ cfg_get_subsections(&new_br, "bridge"); + + /* Get rid of deleted bridges and add new bridges. */ + svec_sort(&old_br); +@@ -793,7 +780,7 @@ bridge_create(const char *name) + br = xcalloc(1, sizeof *br); + + error = dpif_create(name, &br->dpif); +- if (error == EEXIST) { ++ if (error == EEXIST || error == EBUSY) { + error = dpif_open(name, &br->dpif); + if (error) { + VLOG_ERR("datapath %s already exists but cannot be opened: %s", +diff --git a/vswitchd/ovs-vswitchd.conf.5.in b/vswitchd/ovs-vswitchd.conf.5.in +index 5483ad5..d82a08a 100644 +--- a/vswitchd/ovs-vswitchd.conf.5.in ++++ b/vswitchd/ovs-vswitchd.conf.5.in +@@ -50,8 +50,7 @@ configure \fBovs\-vswitchd\fR. + .SS "Bridge Configuration" + A bridge (switch) with a given \fIname\fR is configured by specifying + the names of its network devices as values for key +-\fBbridge.\fIname\fB.port\fR. (The specified \fIname\fR may not begin +-with \fBdp\fR followed by a digit.) ++\fBbridge.\fIname\fB.port\fR. + .PP + The names given on \fBbridge.\fIname\fB.port\fR must be the names of + existing network devices, except for ``internal ports.'' An internal +-- +1.6.3.3 + -- 2.30.2