Ben Pfaff [Thu, 12 Jan 2012 23:48:19 +0000 (15:48 -0800)]
Better abstract OpenFlow error codes.
This commit switches from using the actual protocol values of error codes
internally in Open vSwitch, to using abstract values that are translated to
and from protocol values at message parsing and serialization time. I
believe that this makes the code easier to read and to write.
This is also one step along the way toward OpenFlow 1.1 support because
OpenFlow 1.1 renumbered a bunch of error codes.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Jesse Gross [Wed, 11 Jan 2012 23:01:37 +0000 (15:01 -0800)]
datapath: IFF_OVS_DATAPATH is backported by RHEL 6.2.
On kernels 2.6.37 and above IFF_OVS_DATAPATH has a unique value upstream,
on 2.6.36 we define it to be IFF_BRIDGE_PORT and below to 0. There isn't
really a good reason to use IFF_BRIDGE_PORT on 2.6.36 (and it's perhaps
bad because it's checked in a few places and we don't want those checks
to trigger on just this kernel) and it makes it difficult to know what
value we should use in the face of backporting. Therefore, this just
uses 0 on the places where we don't have a real value.
Reported-by: Benoit ML <ben42ml@gmail.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
Ben Pfaff [Thu, 12 Jan 2012 23:23:23 +0000 (15:23 -0800)]
dpif-netdev: Make port numbers predictable for dummy dpif, for unit tests.
The unit tests feed a lot of flows through the ofproto-dpif "trace"
command, which means that they need to know the port numbers of the ports
that they create. Until now, they've had to actually query those port
numbers from the database, which is a bit of unnecessary overhead for unit
tests.
This commit makes dummy dpif port numbers predictable: if the name of a
port contains a number, then the dummy dpif uses that number, if it is
valid and available, as the port number.
This commit also simplifies the unit tests that previously queried port
numbers to depend on the new behavior.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Thu, 12 Jan 2012 19:35:57 +0000 (11:35 -0800)]
ofp-util: Extend message decoding data structures with version field.
This makes it possible to add entries for decoding OpenFlow messages with
newer versions, e.g. OpenFlow 1.1 or 1.2. However, no actual messages for
newer versions are actually implemented yet; that will come later.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Thu, 12 Jan 2012 19:35:50 +0000 (11:35 -0800)]
nicira-ext: Rename "struct nxt_*" to "struct nx_*".
Most structures in this file have an "nx_" prefix, so this makes naming
more consistent.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Thu, 12 Jan 2012 17:08:28 +0000 (09:08 -0800)]
SubmittingPatches: Fix typo.
Reported-by: Reid Price <reid@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ethan Jackson [Wed, 11 Jan 2012 22:15:49 +0000 (14:15 -0800)]
vswitchd: Document maximum queue_id for Linux QoS.
The code enforces an exclusive maximum of 0xf000 on queue_ids for
Linux queueing disciplines, but the documentation doesn't mention
this limitation.
Reported-by: Michael Mao <mmoa@nicira.com>
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Pravin B Shelar [Wed, 11 Jan 2012 20:44:23 +0000 (12:44 -0800)]
odp-util: Handle ipv6 in set nw action.
Rather than silently skipping ipv6 action generation, following patch
generates OVS_ACTION_ATTR_SET action for ipv6. Datapath which do not
support ipv6 action can reject this action.
Bug #8758
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Ethan Jackson [Wed, 11 Jan 2012 03:01:04 +0000 (19:01 -0800)]
ofproto-dpif: Honor statistics in packet_out().
A "packet out" ordinarily isn't accounted to any OpenFlow rule,
because its actions come directly from the controller instead of
from an OpenFlow rule. However, any "resubmit" actions or output
to OFPP_TABLE within the set of actions do bring in actions from a
rule, and the "packet out" should be accounted to the rule in that
case. This commit implements that behavior.
Reported-by: Jesse Gross <jesse@nicira.com>
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Justin Pettit [Wed, 11 Jan 2012 18:45:29 +0000 (10:45 -0800)]
Don't specify program name in passive connection man page fragments.
The vconn and ovsdb passive connection man page fragments used the
PN (program name) macro to describe their functionality. This was
usually correct, but in the case of ovs-vsctl, they may be used to
describe configuring ovs-vswitchd. This commit rewords the fragments to
make them correct regardless of whether they're describing a local or
remote passive connection.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Ben Pfaff [Wed, 11 Jan 2012 18:23:10 +0000 (10:23 -0800)]
ovsdb-tool: Use typical db and schema install locations as defaults.
This makes ovsdb-tool easier to use in the common case.
Feature #7756.
Requested-by: Reid Price <reid@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Wed, 11 Jan 2012 17:58:17 +0000 (09:58 -0800)]
datapath: Fix multipart datapath dumps.
The logic to split up the list of datapaths into multiple Netlink messages
was simply wrong, causing the list to be terminated after the first part.
Only about the first 50 datapaths would be dumped. This fixes the
problem.
Bug #9124.
Reported-by: Paul Ingram <paul@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Ben Pfaff [Wed, 11 Jan 2012 17:56:35 +0000 (09:56 -0800)]
NEWS: Fix typo.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Wed, 11 Jan 2012 17:55:53 +0000 (09:55 -0800)]
ovs-vsctl: Add set relational operators to "find" command.
Requested-by: Shih-Hao Li <shli@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Wed, 30 Nov 2011 18:43:59 +0000 (10:43 -0800)]
ovsdb-data: Short-circuit ovsdb_datum_includes_all() in trivial case.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ethan Jackson [Wed, 11 Jan 2012 01:25:59 +0000 (17:25 -0800)]
news: Mention NXM_PACKET_IN.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ethan Jackson [Fri, 9 Dec 2011 23:48:26 +0000 (15:48 -0800)]
openflow: New Nicira Extended PACKET_IN format.
The new PACKET_IN format implemented in this patch includes flow
metadata such as the cookie, table_id, and registers.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ethan Jackson [Thu, 15 Dec 2011 23:21:23 +0000 (15:21 -0800)]
ofproto-dpif: Implement PACKET_IN in userspace.
In future patches, PACKET_IN messages will include meta-data which
is only available in userspace during action translation. Either,
this data needs to be stored until it's required by a userspace
datapath action, or the PACKET_IN messages must be sent at the time
the data is available. This patch implements the latter.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ethan Jackson [Tue, 3 Jan 2012 21:30:45 +0000 (13:30 -0800)]
nx-match: New function nx_pull_match_loose().
Future patches will need the ability to skip over unsupported NXM
headers.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ethan Jackson [Thu, 5 Jan 2012 00:40:13 +0000 (16:40 -0800)]
flow: Create new flow_metadata structure for packet_in messages.
This will ease the implementation of future patches.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ethan Jackson [Fri, 16 Dec 2011 01:58:23 +0000 (17:58 -0800)]
packets: New packet_set_*() helper functions.
This commit pulls code used to modify L3 and L4 header fields
from dp_netdev into the packet library. An additional user will
be added in a future commit.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ethan Jackson [Fri, 23 Dec 2011 00:35:23 +0000 (16:35 -0800)]
ofputil: New function ofputil_decode_packet_in().
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ethan Jackson [Thu, 22 Dec 2011 03:01:18 +0000 (19:01 -0800)]
ofp-util: Don't use ofpbuf in ofputil_packet_in struct.
This will make the memory ownership clearer in future patches which
make more extensive use of ofputil_packet_in.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ethan Jackson [Tue, 3 Jan 2012 22:22:44 +0000 (14:22 -0800)]
ofproto: Always clone packets in PACKET_IN message.
This patch removes an optimization which significantly complicates
the code in ways which would get worse in future patches if not
removed. Furthermore, future patches will have fewer cases which
can take advantage of the optimization further mitigating its
justification.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ethan Jackson [Wed, 21 Dec 2011 01:40:25 +0000 (17:40 -0800)]
tests: Unit test PACKET_IN.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ethan Jackson [Tue, 20 Dec 2011 23:31:34 +0000 (15:31 -0800)]
ovs-ofctl: Support daemonization for monitor and snoop.
This will ease implementation of future unit tests.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ethan Jackson [Wed, 21 Dec 2011 03:56:43 +0000 (19:56 -0800)]
ofp-print: Print checksum in ofp_packet_to_string().
This will improve the unit tests of future patches.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ethan Jackson [Wed, 21 Dec 2011 20:59:28 +0000 (12:59 -0800)]
ofp-print: Remove tcpdump from ofp_packet_to_string().
Instead this patch uses flow_format() which gives very similar
output. This patch will improve the reliability of unit tests in
future patches which rely on the results of ofp_packet_to_string().
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ethan Jackson [Tue, 3 Jan 2012 18:42:56 +0000 (10:42 -0800)]
packets: Mask out CFI bit in eth_push_vlan().
We should never push a VLAN tag with the CFI bit set. This patch
defensively enforces this invariant.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ethan Jackson [Tue, 3 Jan 2012 18:27:04 +0000 (10:27 -0800)]
pktbuf: Directly use pointers in pktbuf_save().
In future patches, directly using a void * pointer in the
pktbuf_save() definition will simplify the code.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ethan Jackson [Fri, 23 Dec 2011 01:47:15 +0000 (17:47 -0800)]
ofp-print: Remove vestigial 'total_len' argument.
ofp_print_packet() and ofp_packet_to_string() don't use the
'total_len' argument which they require callers to supply.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ethan Jackson [Tue, 20 Dec 2011 20:57:04 +0000 (12:57 -0800)]
flow: Improve flow_format() output.
I find this significantly easier to read.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ethan Jackson [Tue, 27 Dec 2011 18:35:56 +0000 (10:35 -0800)]
ofpbuf: Maintain header pointers in clone functions.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ethan Jackson [Wed, 28 Dec 2011 18:51:15 +0000 (10:51 -0800)]
classifier: Update cls_rule_set_in_port() prototype.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ben Pfaff [Tue, 10 Jan 2012 22:22:22 +0000 (14:22 -0800)]
debian: Look in /lib/modules instead of /usr/src for DKMS kernel sources.
DKMS packages usually look in /lib/modules for kernel sources, since that
is the "standard" location, but our packages was looking directly in
/usr/src. This fixes the problem.
Reported-by: Alban Browaeys <prahal@yahoo.com>
Tested-by: Alban Browaeys <prahal@yahoo.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Tue, 10 Jan 2012 18:31:05 +0000 (10:31 -0800)]
debian: Ignore temporary files for openvswitch-test package too.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Justin Pettit [Tue, 10 Jan 2012 02:46:23 +0000 (18:46 -0800)]
ovs-monitor-ipsec: Don't reconfigure cert-based authentication as often.
ovs-monitor-ipsec wakes up when the Interface table is modified. To
prevent needless reconfiguration, it maintains a dictionary of the
currently implemented configuration and compares it to any new changes.
Unfortunately, for certificate-based authentication we create a new
"peer_cert_file" key in our local dictionary, which always causes the
comparison to fail. This forces expensive renegotiation for any change
in the Interface tables. This commit uses set difference to detect
changes from the previous configuration as opposed to a straight simple
comparison.
Bug #9103
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Gurucharan Shetty [Fri, 6 Jan 2012 02:28:20 +0000 (18:28 -0800)]
ovs-ctl: Pull system_type and system_version from config file
Currently system-type and system-version can only be set through
ovs-vsctl and ovs-ctl in the commandline. This patch allows you
to place system-type.conf and system-version.conf files in $etcdir
and their contents will be used if no arguments are specified with
ovs-vsctl or ovs-ctl.
Bug #9033.
Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Ben Pfaff [Fri, 6 Jan 2012 23:03:07 +0000 (15:03 -0800)]
ofproto-dpif: Keep subfacets longer to avoid assert-fail in facet_account().
If a subfacet expired when its facet still had statistics that had not
yet been pushed into the rule, and the facet either used the "normal"
action or the bridge contained a bond port, then facet_account() would
be called after the last subfacet was removed from its facet's list of
subfacets, triggering an assertion failure in list_front().
This fixes the problem by always running facet_flush_stats() (which calls
facet_account()) before deleting the last subfacet from a facet.
This problem took a while to surface because subfacets usually expire only
long after their statistics have been pushed into the rule.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Reported-by: Mike Kruze <mkruze@nicira.com>
Bug #9074.
Ben Pfaff [Thu, 5 Jan 2012 18:44:40 +0000 (10:44 -0800)]
ovs-benchmark: Improve output format for "rate" command.
Before, the "rate" command kept the status of the test continually updated
on the screen. This form of output is nice to watch, but it isn't very
useful for reporting, because you only end up with a single number at the
end, and redirecting it to a file yields one long line with lots of
carriage returns.
This commit simplifies the output format by simply printing one line per
second.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Justin Pettit [Thu, 5 Jan 2012 18:19:15 +0000 (10:19 -0800)]
ofproto-dpif.at: Add test for normal action with OFPP_NONE in_port.
Commit 33158a (ofproto-dpif: Fake-up OFPP_NONE input bundle for
mirroring and normal.) fixed handling of packets sent from a controller
with an ingress port of OFPP_NONE. It includes a unit test for the
mirroring case, but not the "normal" processing case. The reason is
that the test requires commit fadc05 (ofproto-dpif: Fix nondeterministic
flow revalidation behavior.), which removes a check that causes the test
to fail. That fix isn't available in all branches that 33158a is on, so
this test is only being run on those that are.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Ben Pfaff [Thu, 5 Jan 2012 00:12:08 +0000 (16:12 -0800)]
tests: Fix bug in NetFlow test case.
The "sed" command here is intended to replace something like 1234...1234
(where the two numbers are the same) with <moment> and something like
1234...2345 (where the two numbers differ) with <range>, but in fact it
accidentally changed, e.g., 10...1016 into <moment>16. This commit fixes
that.
This fixes only a test case, not the code under test.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Bug #9057.
Pravin B Shelar [Thu, 5 Jan 2012 01:23:03 +0000 (17:23 -0800)]
datapath: Implement flow table re-hashing.
Following patch introduces a timer based event to rehash flow-hash
table. It makes finding collisions difficult to for an attacker.
Suggested-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Pravin B Shelar [Thu, 5 Jan 2012 01:22:07 +0000 (17:22 -0800)]
datapath: Fix OVS compat workqueue support.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Pravin B Shelar [Thu, 5 Jan 2012 01:20:08 +0000 (17:20 -0800)]
datapath: Add genl_exec().
genl_lock is not exported from older kernel. Following patch add
genl_exec() which can run any function (passed as arg) with
genl_lock held.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Justin Pettit [Tue, 3 Jan 2012 21:34:20 +0000 (13:34 -0800)]
ofproto-dpif: Fake-up OFPP_NONE input bundle for mirroring and normal.
Both mirroring and "normal" processing make use of the input bundle to
perform various sanity checks. Controller-generated traffic typically
uses an ingress port of OFPP_NONE, which doesn't have a corresponding
input bundle. This commit fakes one up well enough that mirroring and
"normal" processing succeed.
We looked at creating an actual bundle based on the "real" OFPP_NONE.
This was even uglier, since there were even more special-cases that
needed to be handled, including having to hide it from port queries.
Reported-by: Jesse Gross <jesse@nicira.com>
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Justin Pettit [Tue, 3 Jan 2012 00:06:48 +0000 (16:06 -0800)]
ofproto-dpif: Have lookup_input_bundle() return a bundle.
Previously, the function returned an "ofport_dpif", but it makes more
sense to return an "ofbundle". This will also be helpful in a
forthcoming commit.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Ben Pfaff [Fri, 23 Dec 2011 00:48:50 +0000 (16:48 -0800)]
ofproto-dpif: Fix nondeterministic flow revalidation behavior.
SLB bonds are very strange beasts. It's taken OVS a while to figure out
how they should really work. Way back in the mists of time, when we were
in the midst of this process, we noticed that the following could happen:
1. Local VM sends a packet to the OVS bridge.
2. OVS bridge learns VM's MAC, forwards packets to SLB bond.
3. Remote switch hasn't learned packet's destination and
forwards packet back to other interfaces in the SLB bond.
Normally nothing bad happens in this scenario because OVS has already
learned the local port for the VM's MAC in step 2 and the rules for SLB
bonding (this is rule #2 in vswitchd/INTERNALS). But at the time we were
implementing this, OVS didn't yet use active flows to keep MAC learning
entries alive; only new flows prevented a MAC entry from aging out. So
in steady state (e.g. just "ping" traffic) OVS would regularly forget MAC
addresses. If the remote switch also happened to forward a packet back to
one of the SLB bond interfaces, then OVS would learn the VM's address on
the bond, with the result that any traffic coming in from the remote switch
would be black-holed until the VM sent a new packet. This was not good.
The fix we applied at the time was commit
2416b8ecea (bridge: Eject NORMAL
flows without a learning entry from datapath.) followed by a small
refinement in commit
e96a4d8035 (bridge: Feed flow stats into learning
table.). This fix causes flows that don't have a learning entry to be
ejected from the datapath if revalidation occurs. This forced the next
packet in the flow to go to userspace, which in turn caused learning to
happen, fixing the problem.
However, this isn't a good solution for several reasons:
* It forces more packets to userspace, which is expensive.
* It doesn't just affect the cases where it helps, those where an
SLB bond is actually involved. (This could be fixed, but it is
not worth it.)
* It means that flow installability becomes nondeterministic. When
the first packet shows up for a flow, we install it. But later
if we revalidate it, we have to uninstall it. That doesn't make
sense; a flow should be either installable or not installable,
not some weird mix.
Fortunately, the situation has improved since this fix was originally
designed. First, active flows now keep MAC learning entries alive, since
commit
e96a4d8035367 (bridge: Feed flow stats into learning table.)
Second, gratuitous ARP locking, added in commit
7febb9100b (bridge: Filter
some gratuitous ARPs on bond slaves.) means that gratuitous ARPs reflected
on bond slaves don't cause confusion (this is rule #4 in
vswitchd/INTERNALS).
These improvements mean that it is no longer necessary to have this
strange special case at all. Therefore, this commit removes it.
I found this while investigating reports from code that I added to
occasionally check that flow actions were correct.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Wed, 4 Jan 2012 22:38:46 +0000 (14:38 -0800)]
ofproto: Fix detection of in-use VLANs based on the flow table.
I swear I tested this, but the code was obviously wrong.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Reported-by: Brendan Kelley <bkelley@nicira.com>
Bug #8729.
Ben Pfaff [Tue, 3 Jan 2012 22:10:41 +0000 (14:10 -0800)]
AUTHORS: Move Rob Hoes to list of commit contributors.
He is the author of at least commit
3249bb907a1d (xenserver: Reduce number of
xapi DB calls in plugin).
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Tue, 3 Jan 2012 22:08:43 +0000 (14:08 -0800)]
AUTHORS: Add Dominic Curran <dominic.curran@citrix.com>.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Tue, 3 Jan 2012 21:34:47 +0000 (13:34 -0800)]
vswitchd: Also consider access port VLANs as "in use" for VLAN splinters.
It's natural to add an access port to a bridge and expect that trunk ports
will then immediately start to work properly with that VLAN without
additional configuration. This change makes that happen.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Thu, 22 Dec 2011 23:51:39 +0000 (15:51 -0800)]
unixctl: Improve readability of "help" output for long command names.
Without this change, if a unixctl command name is 23 character long or
longer, no space appeared between the command name and its usage. This
commit ensures that at least one space always appears.
No command yet has a name this long. I discovered this issue when I added
one that does.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Mon, 2 Jan 2012 20:46:17 +0000 (12:46 -0800)]
ofproto-dpif: Fix GCC warning.
gcc (GCC) 4.1.2
20080704 (Red Hat 4.1.2-48) warned:
ofproto/ofproto-dpif.c: In function 'bundle_send_learning_packets':
ofproto/ofproto-dpif.c:1835: warning: dereferencing type-punned pointer
will break strict-aliasing rules
I agree that its analysis matches what the C standard says. This commit
fixes the problem and avoids the warning.
The assignment to 'port' isn't actually necessary. I included it because
I like to have a variable with the correct type near the use of that type
through a "void *". Then "grep" for that type is more effective, and the
compiler will be able to diagnose any later change to (in this case) the
type of the first parameter to send_packet().
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Mon, 26 Dec 2011 22:40:54 +0000 (14:40 -0800)]
ofproto-dpif: Omit "execute" operation entirely when there are no actions.
There's no point in executing an empty set of actions.
dpif_execute() has had this optimization internally for a long time but
dpif_operate() doesn't. For dpif_operate() it seems like a bigger win to
optimize it at this higher level, avoiding adding any operation to the
array at all.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Simon Horman [Fri, 30 Dec 2011 05:37:52 +0000 (14:37 +0900)]
Debian: Depend on python (>= 2.7) | python-argparse
Depend on python (>= 2.7) | python-argparse instead of
python-argparse to avoid pulling in python2.6
See: http://bugs.debian.org/653645
Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Justin Pettit [Sat, 31 Dec 2011 01:56:08 +0000 (17:56 -0800)]
ovs-ofctl: Fix "cookie mask" issue in diff-flows command.
Commit
e729e79 (Add ability to restrict flow mods and flow stats
requests to cookies.) introduced a cookie_mask field to the
ofputil_flow_stats_request struct that allowed restricting the flows to
a particular cookie pattern. The diff-flows command uses the
read_flows_from_switch() function, which did not properly initialize
this field, which would cause it to miss flows. This commit sets the
value to zero, which allows any cookie.
Bug #8984
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Reported-by: Luca Giraudo <lgiraudo@nicira.com>
Justin Pettit [Fri, 23 Dec 2011 20:23:24 +0000 (12:23 -0800)]
Add ability to restrict flow mods and flow stats requests to cookies.
With this commit, it is possible to limit flow deletions and
modifications to specific cookies. It also provides the ability to
dump flows based on their cookies.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Justin Pettit [Thu, 22 Dec 2011 22:39:54 +0000 (14:39 -0800)]
ovs-ofctl: Fix error in man page related to TCI masks.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Ben Pfaff [Tue, 27 Dec 2011 23:44:41 +0000 (15:44 -0800)]
bundle: Fix returned error code in one bundle_check() corner case.
In C, the || operator yields 0 or 1, not (as in some other languages) the
value of its first nonzero operand.
Found by inspection.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Tue, 27 Dec 2011 20:34:57 +0000 (12:34 -0800)]
ofproto-dpif: Fix bug in VLAN splinters.
Bug #8671.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Jesse Gross [Tue, 27 Dec 2011 01:33:03 +0000 (17:33 -0800)]
packet: Match userspace and kernel definition of ICMP header.
Current userspace considers an ICMP header to be 4 bytes consisting
of the type, code, and checksum. The kernel considers it to be 8
bytes because it also counts the two data fields that contain
type-specific information (and are always present). Since flow
extract will zero out headers that are not completely present this
means that an ICMP packet that has a header of 5-7 bytes will be
interpreted differently by userspace and kernel. This fixes the
problem by adopting the kernel's version of the ICMP header in
userspace.
Signed-off-by: Jesse Gross <jesse@nicira.com>
Ben Pfaff [Wed, 21 Dec 2011 23:52:23 +0000 (15:52 -0800)]
flow: Rename 'priority' to 'skb_priority'.
This priority's mean is completely different from the priority of an
OpenFlow rule, so it is confusing for it to have the same name.
We should be on the lookout for a less Linux-specific name, but this one
seems fine for now.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ethan Jackson [Wed, 21 Dec 2011 19:19:28 +0000 (11:19 -0800)]
dpif-netdev: Fix UDP checksum calculation.
dpif-netdev incorrectly calculated the UDP checksum when modifying
source and destination ports.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ben Pfaff [Wed, 21 Dec 2011 19:01:45 +0000 (11:01 -0800)]
NEWS: Fix typo in entry for 1.3.0.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ansis Atteka [Wed, 21 Dec 2011 17:59:16 +0000 (09:59 -0800)]
vswitchd: fix "implicit declaration of function" build warning.
Included stream.h header file for connmgr.c and bridge.c files.
Signed-off-by: Ansis Atteka <aatteka@nicira.com>
Reported-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Wed, 21 Dec 2011 00:50:17 +0000 (16:50 -0800)]
vswitchd: Document what changes to the database flush the flow table.
Requested-by: Hao Zheng <hzheng@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Wed, 21 Dec 2011 00:48:40 +0000 (16:48 -0800)]
Add release date for 1.3.0.
This made it into branch-1.3 but not master.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ansis Atteka [Mon, 19 Dec 2011 20:43:34 +0000 (12:43 -0800)]
vswitchd: In-band rules for Controller are missing after executing force-reload-kmod command
In current implementation vswitchd adds Controller in-band rules only if
there is a route in kernel routing table that might route traffic to the
Controller. But, when executing force-reload-kmod command, network
configuration (e.g. assigned IP addresses, routes) are flushed away,
hence Controller in-band rules are not added.
This commit fixes this limitation and allows vswitchd to add Controller
in-band rules even if there are no routes in the kernel routing table.
Issue: #8625
Signed-off-by: Ansis Atteka <aatteka@nicira.com>
Ethan Jackson [Mon, 19 Dec 2011 23:44:57 +0000 (15:44 -0800)]
tests: Update gitignore to include test-netflow.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ben Pfaff [Mon, 19 Dec 2011 22:46:16 +0000 (14:46 -0800)]
netflow: Add basic unit tests.
These tests would have caught the flow statistics bug introduced by commit
501f8d1fd75 (ofproto-dpif: Batch interacting with the dpif on flow miss
operations.)
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Mon, 19 Dec 2011 22:18:00 +0000 (14:18 -0800)]
netflow: Move packet definitions to header file.
An upcoming commit will introduce code outside of ofproto/netflow.c that
works with NetFlow packets, so we need the protocol definitions in a common
location.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Thu, 8 Dec 2011 21:22:50 +0000 (13:22 -0800)]
tests: Add expected output parameter to OVS_VSWITCHD_START.
It's convenient to pass commands for ovs-vsctl directly to
OVS_VSWITCHD_START, but until now those commands could have no output
because that would cause a test failure. This commit adds an optional
argument to supply ovs-vsctl expected output, and adjusts some test
cases to use it instead of calling ovs-vsctl a second time.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Thu, 8 Dec 2011 20:45:18 +0000 (12:45 -0800)]
tests: Update ovs-vswitchd log messages to ignore.
Commit
579a77e024 (tests: Allow unit tests to run as root.) means that
ovs-vswitchd doesn't output either of these log messages anymore when
the unit tests run, so there's no need to ignore them anymore.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Mon, 19 Dec 2011 22:45:23 +0000 (14:45 -0800)]
flow: Make flow_compose() fake packets slightly more realistic.
This makes flow_extract() happier with TCP packets that this function
produces.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Wed, 7 Dec 2011 21:15:54 +0000 (13:15 -0800)]
timeval: Add ability to fast-forward time, for unit testing.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Mon, 19 Dec 2011 22:28:42 +0000 (14:28 -0800)]
poll-loop: Track the poll timeout as an absolute, not a relative time.
This is a necessary prerequisite for allowing time to be "fast forwarded"
in unit tests, to keep tests that depend on the passage of time from
running in real time. Without this change, a code sequence like this:
poll_timer_wait(1000);
...fast forward time 5 seconds...
poll_block();
would still sleep for a second, because the poll_loop module would still
have a relative timeout of 1000 ms.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Tue, 6 Dec 2011 22:22:27 +0000 (14:22 -0800)]
netdev-dummy: Allow injecting traffic.
This paves the way to allow unit tests to test the whole switch
on an end-to-end basis.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Tue, 6 Dec 2011 22:09:10 +0000 (14:09 -0800)]
packets: New function eth_from_hex().
An upcoming commit will add another user.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Fri, 2 Dec 2011 23:29:19 +0000 (15:29 -0800)]
unixctl: Implement quoting.
The protocol used by ovs-appctl has a long-standing bug that there
is no way to distinguish "ovs-appctl a b c" from "ovs-appctl 'a b c'".
This isn't a big deal because none of the current commands really
want to accept arguments that include spaces, but it's kind of a silly
limitation.
At the same time, the internal API is awkward because every user is
stuck doing its own argument parsing, which is no fun.
This commit fixes both problems, by adding shell-like quoting to the
protocol and modifying the internal API from one that passes a string
to one that passes in an array of pre-parsed strings. Command
implementations may now specify how many arguments they expect. This
simplifies some command implementations significantly.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Tue, 6 Dec 2011 23:57:15 +0000 (15:57 -0800)]
socket-util: Don't try to listen to a UDP socket.
The "listen" system call doesn't work and isn't necessary for UDP, but
inet_open_passive() would still try to call it (and fail).
This doesn't fix a real bug because the two existing callers both use
inet_open_passive() to listen for TCP connections.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Fri, 2 Dec 2011 23:22:20 +0000 (15:22 -0800)]
netdev-linux: Report error for truncated packets on receive.
Found by inspection.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Tue, 6 Dec 2011 22:22:00 +0000 (14:22 -0800)]
netdev-linux: Translate errno value to name in log message.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Thu, 8 Dec 2011 20:40:01 +0000 (12:40 -0800)]
ofproto-dpif: Include datapath flow misses in flow statistics.
Commit
501f8d1fd75 (ofproto-dpif: Batch interacting with the dpif on flow
miss operations.) caused packets handled manually in userspace not to be
counted in flow statistics. This patch fixes the problem.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Bug #8494.
Ethan Jackson [Thu, 15 Dec 2011 21:39:38 +0000 (13:39 -0800)]
bond: Warn of imminent default bond_mode change.
Post version 1.4.*, we are planning to change the default bond_mode
from balance-slb to active-backup. This commit warns users of the
change so that they can prepare.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ben Pfaff [Fri, 16 Dec 2011 18:09:30 +0000 (10:09 -0800)]
ofproto-dpif: Fix use-after-free for OFPP_CONTROLLER flows.
When a flow consists solely of an output to OFPP_CONTROLLER, we avoid a
round trip to the kernel and back by calling execute_controller_action()
from handle_flow_miss(). However, execute_controller_action() frees the
packet passed in. This is dangerous, because the packet and the upcall
key are in the same block of malloc()'d memory, as the comment on struct
dpif_upcall says:
/* A packet passed up from the datapath to userspace.
*
* If 'key' or 'actions' is nonnull, then it points into data owned by
* 'packet', so their memory cannot be freed separately. (This is hardly a
* great way to do things but it works out OK for the dpif providers and
* clients that exist so far.)
*/
Thus, we get a use-after-free later on in handle_flow_miss() and eventually
a double free.
This fixes the problem by making execute_controller_action() clone the
packet in this case.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Fri, 16 Dec 2011 18:02:51 +0000 (10:02 -0800)]
ofproto-dpif: Flush MACs for deleted ports from every bridge.
Consider this scenario: two hypervisors HV-1 and HV-2, connected to a
common physical network over SLB bonds. Two virtual machines VM-1 and VM-2
are both running on HV-1. Patch ports are in use, so that each VM is not
connected to a bridge with a physical Ethernet port but is actually one
virtual "hop" away across a patch port to a second OVS bridge. VM-2 is
running a "ping" process directed at VM-1.
Now migrate VM-1 to HV-2. Suppose that VM-1 fails to send out a gratuitous
ARP following migration, or that the gratuitous ARPs are lost, e.g. because
they are sent before the OpenFlow controller manages to populate the flow
table with rules to allow the VM's traffic
Now we are in a situation where HV-1 has learned that VM-1 is local and
HV-2 has learned that VM-1 is on its bond; both are wrong. One would
expect the problem to resolve itself as soon the VM-1 sends out its first
packet. However, SLB bonds (for important reasons documented in
vswitchd/INTERNALS) are very reluctant to learn that a currently local MAC
is actually on the bond: the only ways to learn that the MAC is on the bond
are to receive a gratuitous ARP (which we won't, since they were dropped)
or for the MAC learning entry to expire after 60 seconds. This means that
VM-1 can send out as much ordinary traffic as it wants (even ARP requests
and other broadcasts) but HV-1 will drop all of it at the physical Ethernet
since it believes that VM-1 is local.
(In an ordinary setup with a single bridge, HV-1 would have unlearned the
address for VM-1 when VM-1's port was deleted, but that didn't happen
because HV-1 only learned that VM-1 was on the patch port that leads to the
integration bridge. The patch port didn't get deleted.)
HV-2 does quickly learn that VM-1 is now local. SLB bonds are only
reluctant to learn that something they think is local is actually on the
bond, not the reverse.
This commit attempts to work around the problem by flushing the MAC
associated with a port from *every* bridge when a port is deleted.
This commit demonstrates yet another good reason not to use SLB bonds.
Build and unit tested only.
Bug #7978.
Bug #7687.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Fri, 16 Dec 2011 18:03:08 +0000 (10:03 -0800)]
dpif-netdev: Correctly report types of dummy netdevs.
When dpif-netdev is in use as a "dummy" datapath, the devices that are
added to it have type "dummy", but the dpif methods that report the types
of ports reported that they had type "system". This meant that every time
the bridge code reconfigured, it would see that the ports had the wrong
types and delete and re-add them, which had other undesirable effects such
as flushing the entire MAC learning table whenever ovs-vsctl changed the
configuration.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Tue, 13 Dec 2011 22:42:11 +0000 (14:42 -0800)]
ofproto-dpif: Avoid segfault for ports with bundles in add_mirror_actions().
Not every port has an associated bundle, so we must not unconditionally
dereference ofport->bundle without first checking that it is nonnull.
(One example of a port without a bundle is a VLAN splinter port.)
Bug #8671.
Reported-by: Michael Mao <mmao@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Tue, 13 Dec 2011 22:20:54 +0000 (14:20 -0800)]
SubmittingPatches: Suggest parentheses for commit subjects.
Suggested-by: Ethan Jackson <ethan@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Fri, 9 Dec 2011 23:54:43 +0000 (15:54 -0800)]
dpif-linux: Avoid valgrind warning in epoll_ctl() call.
Valgrind points out correctly that there are uninitialized bytes in the
'event' structure. That's OK, but it doesn't hurt to suppress the warning
by zeroing all of the bytes.
This doesn't fix a real bug.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Mon, 12 Dec 2011 22:44:23 +0000 (14:44 -0800)]
bridge: Enable support for access and native VLAN ports on bonds.
Since Open vSwitch's inception we've disabled the use of bonds as access
ports, for no particularly good reason. This also unintentionally
prevented bonds from being used as native VLAN ports.
This commit removes the code that prevented using bonds these ways
Reported-and-tested-by: "Michael A. Collins" <mike.a.collins@ark-net.org>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Mon, 12 Dec 2011 18:32:44 +0000 (10:32 -0800)]
debian: Consistently use American spelling of "license" in copyright.in.
Consistent spelling may make searches easier.
Reported-by: Justin Pettit <jpettit@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Mon, 12 Dec 2011 17:42:09 +0000 (09:42 -0800)]
debian: Add sFlow license information and text to copyright.in.
sFlow licensing information was omitted from debian/copyright due to an
oversight. This commit adds it.
Reported-by: Chris Wright <chrisw@sous-sol.org>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Mon, 12 Dec 2011 17:37:34 +0000 (09:37 -0800)]
debian: Correct licensing information for user/kernel shared header files.
The intent is to license all shared user/kernel header files under both
GPLv2 and Apache v2. The license statement here said GPLv3 instead of
GPLv2, so this commit fixes that problem.
Also, include/openvswitch used to be where all the shared user/kernel
header files were located, but this is no longer true, and now there is a
userspace-only header file also in include/openvswitch, so this commit now
lists all of the user/kernel header files explicitly.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Neil McKee [Sat, 10 Dec 2011 00:56:32 +0000 (16:56 -0800)]
sFlow: add Sun Industry Standards Source License 1.1 as licensing option
The sFlow License was not on the list for the Fedora Project, but the
Sun Industry Standards Source License 1.1 was (and it has the right
properties). So this patch includes it as a licensing option in the
relevant places (COPYING and the lib/sflow* sources). Let me know
if this looks OK or not.
Signed-off-by: Neil McKee <neil.mckee@inmon.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Chris Wright <chrisw@sous-sol.org>
Ben Pfaff [Fri, 9 Dec 2011 23:57:55 +0000 (15:57 -0800)]
bridge: Avoid use-after-free with VLAN splinters and multiple bridges.
The VLAN splinters feature uses a "pool" to track and free allocated
blocks. There's only one pool, but the implementation was freeing all of
the blocks in it for every bridge during reconfiguration, not just once for
each reconfiguration, so caused a use-after-free when there was more than
one bridge and a bridge other than the last one in the list of bridges had
a VLAN splinter port.
Bug #8671.
Reported-by: Michael Mao <mmao@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Chris Wright [Fri, 9 Dec 2011 23:03:18 +0000 (15:03 -0800)]
SubmittingPatches: document the use of DCO
This adds the Developer's Certificate of Origin to SubmittingPatches
to document the requirement to add Signed-off-by to patches.
Signed-off-by: Chris Wright <chrisw@sous-sol.org>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Fri, 9 Dec 2011 21:09:23 +0000 (13:09 -0800)]
lacp: Avoid valgrind warning in lacp_configure() if custom timing not used.
The caller currently doesn't fill in s->custom_time unless it actually
wants a custom LACP time, but lacp_configure() still does a calculation
with it, provoking a warning from valgrind. This eliminates the warning.
The calculated value was not actually used in this case, so this commit
does not fix a real bug.
Ben Pfaff [Fri, 9 Dec 2011 17:58:16 +0000 (09:58 -0800)]
ovs-ofctl: Fix misspelling in manpage.
Reported-by: Paul Ingram <paul@nicira.com>