Ben Pfaff [Wed, 12 May 2010 19:53:07 +0000 (12:53 -0700)]
poll-loop: New function poll_timer_wait_until().
Many of poll_timer_wait()'s callers actually want to wait until a specific
time, so it's convenient for them to offer a function that does this.
Ben Pfaff [Wed, 12 May 2010 17:56:45 +0000 (10:56 -0700)]
poll-loop: Change poll_timer_wait() parameter from "int" to "long long".
Every so often I get concerned because OVS does most of its time arithmetic
in "long long int" but poll_timer_wait() takes an "int", so there is
potential for truncating a large value to a small value or a positive value
to a negative value. That would cause excessive wakeups and possibly 100%
CPU usage.
This commit therefore changes poll_timer_wait()'s parameter type from "int"
to "long long int". The file-scope 'timeout' variable remains type "int"
because that is the type of poll()'s timeout argument.
Factoring poll_timer_wait() into two functions is not necessary here but it
comes in handy in the following patch.
Ben Pfaff [Tue, 25 May 2010 22:49:26 +0000 (15:49 -0700)]
unixctl: Sort list of commands output by "help".
Feature #2873.
Ben Pfaff [Tue, 25 May 2010 22:42:44 +0000 (15:42 -0700)]
datapath: Fix ODP_PORT_GROUP_GET implementation.
The final argument to do_get_port_group() is supposed to be a user pointer
to the number of ports, to be updated with put_user(), but it was actually
a kernel pointer, so "ovs-dpctl dump-groups" and anything else that used
this ioctl would always fail with -EFAULT. This commit fixes it.
Bug introduced in commit
44e05eca "datapath: Prepare to support 32-bit
compatibility ioctls" for normal ioctls and for compat ioctls at their
introduction in commit
3fbd517acf"datapath: Add 32-bit compatibility
ioctls."
Ben Pfaff [Tue, 25 May 2010 22:35:20 +0000 (15:35 -0700)]
Revert "Add definition of AS_ECHO for compatibility with Autoconf before 2.62."
This reverts commit
d0a5f97842ef1598d07a6e3f10bab1543a38aa4d.
Partner corrected earlier report that the reverted commit helped. It
didn't.
Ben Pfaff [Mon, 24 May 2010 16:56:30 +0000 (09:56 -0700)]
Add definition of AS_ECHO for compatibility with Autoconf before 2.62.
Requested and tested by partner.
Wei Yongjun [Mon, 24 May 2010 02:20:10 +0000 (10:20 +0800)]
datapath: introduce a method to install kernel modules
After we choose to build the Linux kernel module, install
them using make modules_install.
% ./configure --with-l26=/lib/modules/`uname -r`/build
% make
% cd datapath/linux-2.6/
% make modules_install
the openswitch kernel modules will be installed to dir
/lib/modules/`uname -r`/extra/
Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
Justin Pettit [Thu, 20 May 2010 20:26:48 +0000 (13:26 -0700)]
dpif: Include stat.h header
Todd Deshane [Thu, 20 May 2010 17:03:11 +0000 (10:03 -0700)]
Add instructions for using Open vSwitch with KVM.
Jesse Gross [Wed, 19 May 2010 21:12:27 +0000 (14:12 -0700)]
netdev-linux: Quiet down ingress policing.
If we attempt to remove ingress policing and receive "invalid
argument" it means that policing isn't compiled into the kernel.
If it isn't compiled in then accept that policing has been
successfully removed.
Ben Pfaff [Wed, 19 May 2010 16:43:50 +0000 (09:43 -0700)]
netflow: Use macro for constant instead of static const int.
It's just bizarre to put a static const int in a header file, at least in
C.
Jesse Gross [Wed, 19 May 2010 00:30:42 +0000 (17:30 -0700)]
gre: Check whether IPv6 is compiled into the kernel.
Add guards to check whether IPv6 is supported by the kernel instead
of causing the module to fail to load. If IPv6 is not supported
these packets can still be encapsulated but they will not receive any
special treatment such as path MTU discovery.
Jesse Gross [Fri, 14 May 2010 20:54:16 +0000 (13:54 -0700)]
patch: Remove veth driver.
Now that we have a new patch implementation, remove the veth driver
and its userspace components. Then rename 'patchnew' to 'patch'.
The new implementation is a drop-in replacement for the old one.
Jesse Gross [Fri, 14 May 2010 20:49:43 +0000 (13:49 -0700)]
netdev: Add 'patch' userspace implementation.
Add a netdev to talk to the 'patch' vport in the kenerl. Since
there is currently a 'patch' implementation using the veth driver,
this one is temporarily called 'patchnew'.
Jesse Gross [Mon, 17 May 2010 22:04:10 +0000 (15:04 -0700)]
netdev: Extract netdev vport functions.
All devices implemented as vports have a common interface, so
pull out ioctl code from the GRE netdev so it can be used in
other places as well.
Jesse Gross [Mon, 10 May 2010 20:07:28 +0000 (13:07 -0700)]
datapath: Add 'patch' vport.
Adds a new 'patch' vport that acts like a virtual patch cable to
connect together two datapaths.
Jesse Gross [Tue, 11 May 2010 00:40:22 +0000 (17:40 -0700)]
vport: Extract common functions for virtual devices.
Pull some generic implementations of vport functions out of the
GRE vport so they can be used by others.
Also move the code to set the MTUs of internal devices to the minimum
of attached devices to the generic vport_set_mtu layer.
Justin Pettit [Mon, 17 May 2010 21:13:32 +0000 (14:13 -0700)]
datapath: Fix build with backported dev_get_stats().
An upcoming backport adds dev_get_stats() to a pre-2.6.29 Linux kernel, so we
have to check for the presence of an dev_get_stats() definition instead of for
the particular kernel version.
Justin Pettit [Mon, 17 May 2010 20:57:29 +0000 (13:57 -0700)]
Don't define "skb_dst_drop" if other skb_dst accessor funcs are defined
When "skb_dst" is included in the kernel sources, don't redefine
"skb_dst_drop", which is defined in dst.h.
Justin Pettit [Mon, 17 May 2010 20:06:59 +0000 (13:06 -0700)]
datapath: Add stddef.h compat header to list of files in distributions
Older kernels won't compile without a definition of the bool values.
Wei Yongjun [Mon, 17 May 2010 07:15:44 +0000 (15:15 +0800)]
tests: fix compile warning of tests/test-ovsdb.c
This patch fixed the following compile warning:
tests/test-ovsdb.c: In function 'do_evaluate_conditions':
tests/test-ovsdb.c:744: warning: format '%2d' expects type 'int', but argument 2 has type 'size_t'
tests/test-ovsdb.c: In function 'do_execute_mutations':
tests/test-ovsdb.c:850: warning: format '%2d' expects type 'int', but argument 2 has type 'size_t'
tests/test-ovsdb.c: In function 'do_query':
tests/test-ovsdb.c:972: warning: format '%2d' expects type 'int', but argument 2 has type 'size_t'
tests/test-ovsdb.c: In function 'do_query_distinct':
tests/test-ovsdb.c:1105: warning: format '%2d' expects type 'int', but argument 2 has type 'size_t'
tests/test-ovsdb.c: In function 'parse_uuids':
tests/test-ovsdb.c:1600: warning: format '%d' expects type 'int', but argument 2 has type 'size_t'
Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
Wei Yongjun [Mon, 17 May 2010 07:14:55 +0000 (15:14 +0800)]
ofproto: fix compile warning of ofproto/ofproto.c
This patch fixed the following compile warning:
ofproto/ofproto.c: In function 'handle_role_request':
ofproto/ofproto.c:3543: warning: format '%zu' expects type 'size_t', but argument 5 has type 'int'
ofproto/ofproto.c: In function 'handle_vendor':
ofproto/ofproto.c:3593: warning: format '%zu' expects type 'size_t', but argument 5 has type 'int'
ofproto/ofproto.c:3602: warning: format '%zu' expects type 'size_t', but argument 5 has type 'int'
Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
Wei Yongjun [Mon, 17 May 2010 07:08:17 +0000 (15:08 +0800)]
ovsdb-idl: fix compile warning of lib/ovsdb-idl.c
This patch fixed the following compile warning:
lib/ovsdb-idl.c: In function 'ovsdb_idl_txn_process_inc_reply':
lib/ovsdb-idl.c:1524: warning: format '%u' expects type 'unsigned int', but argument 5 has type 'size_t'
lib/ovsdb-idl.c:1538: warning: format '%ld' expects type 'long int', but argument 5 has type 'long long int'
lib/ovsdb-idl.c:1550: warning: format '%u' expects type 'unsigned int', but argument 5 has type 'size_t'
lib/ovsdb-idl.c: In function 'ovsdb_idl_txn_process_insert_reply':
lib/ovsdb-idl.c:1579: warning: format '%u' expects type 'unsigned int', but argument 5 has type 'size_t'
Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
Ben Pfaff [Fri, 14 May 2010 22:44:39 +0000 (15:44 -0700)]
datapath: Fix build with backported skb_dst().
An upcoming backport adds skb_dst() to a pre-2.6.31 Linux kernel, so we
have to check for the presence of an skb_dst() definition instead of for
the particular kernel version.
Ben Pfaff [Fri, 14 May 2010 22:39:48 +0000 (15:39 -0700)]
acinclude.m4: Reorganize checks for kernel identifiers.
These were getting to be hard to read, so I put them into alphabetical
groups by file and then sorted by identifier within a file.
Justin Pettit [Sat, 15 May 2010 07:23:50 +0000 (00:23 -0700)]
Release Open vSwitch 1.0.0
Jesse Gross [Thu, 13 May 2010 23:52:14 +0000 (16:52 -0700)]
datapath: Use per_cpu_ptr instead of percpu_ptr.
percpu_ptr was removed in 2.6.30, so update the one remaining user
and take out the compatibility code.
Suggested-by: Ben Pfaff <blp@nicira.com>
Jesse Gross [Wed, 12 May 2010 20:45:49 +0000 (13:45 -0700)]
datapath: Hold rcu_read_lock where we claim to.
Many of the vport operations require that either RTNL lock or
rcu_read_lock be held. However, operations from userspace often
hold a different lock so grab rcu_read_lock as well.
Jesse Gross [Wed, 12 May 2010 19:40:45 +0000 (12:40 -0700)]
datapath: Don't expect bottom-halves to be disabled.
We currently document that BHs need to be disabled when handling
received packets. However, this isn't actually generally the
case (usually preemption is disabled but not BHs). Only one place
actually relies on BHs being disabled so fix that and update the
documentation of our expectations.
Jesse Gross [Wed, 12 May 2010 18:40:58 +0000 (11:40 -0700)]
datapath: Disable bottom-halves where necessary.
Places that update per-cpu stats without locking need to have bottom
halves disabled. Otherwise we can be running in process context and
in the middle of an operation and be interrupted by a softirq.
Jesse Gross [Wed, 12 May 2010 18:26:55 +0000 (11:26 -0700)]
datapath: Use spin_lock_bh() consistently.
We are never called in hardirq context - only process or softirq.
Therefore it is not necessary to disable interrupts with
spin_lock_irqsave(), so use spin_lock_bh() everywhere.
Ben Pfaff [Thu, 13 May 2010 23:08:14 +0000 (16:08 -0700)]
stream-ssl: Flush OpenSSL error queue after calling SSL_shutdown().
The OpenSSL manpage for SSL_get_error() says this:
In addition to ssl and ret, SSL_get_error() inspects the current
thread's OpenSSL error queue. Thus, SSL_get_error() must be used in
the same thread that performed the TLS/SSL I/O operation, and no other
OpenSSL function calls should appear in between. The current thread's
error queue must be empty before the TLS/SSL I/O operation is
attempted, or SSL_get_error() will not work reliably.
We weren't taking this advice literally enough, which meant that this
would happen:
1. Call SSL_shutdown() on one connection.
2. Call SSL_read() on another connection, returning 0 bytes. (This is
normal. It just means that no more data has arrived yet.)
3. Call SSL_get_error() for that second connection to check whether
the 0-byte return value was a real error. (This should return
SSL_ERROR_WANT_READ to indicate that more data is needed.)
4. Actually get some other error indicating that the SSL_shutdown()
call returned an error.
This commit fixes the problem by flushing the OpenSSL error queue after
calling SSL_shutdown().
Without this commit, starting an ovsdb-server with two active SSL remotes,
running two ovsdb-clients listening for connections from the ovsdb-server
remotes, then killing one of the ovsdb-clients (with e.g. Control+C), will
cause ovsdb-server to drop the other ovsdb-client connnection the next time
that SSL_read() is called on it. With this commit, this scenario works
correctly (e.g. ovsdb-server keeps the remaining connection up).
CC: Jeremy Stribling <strib@nicira.com>
Ben Pfaff [Thu, 13 May 2010 22:25:27 +0000 (15:25 -0700)]
datapath: Add 32-bit compatibility ioctls.
When a 32-bit userspace program runs on a 64-bit kernel, data structures
that contain members whose sizes or alignments change from 32- to 64-bit
must be translated when they are passed to ioctls. This commit adds such
support for openvswitch_mod.
We should really reconsider some parts of the Open vSwitch ioctl interface
to avoid needing as much translation as we do.
Lightly tested with 32-bit userspace on sparc64.
Ben Pfaff [Thu, 13 May 2010 20:18:22 +0000 (13:18 -0700)]
datapath: Avoid __copy_to/from_user(), __get/put_user() functions.
The advantages of the double-underscore variants of copy_to_user(),
copy_from_user(), get_user(), and put_user() are pretty marginal, at best,
in the places where we are using them, and it's not always obvious that we
are making the right calls to access_ok() beforehand. So switch to the
safe variants without double underscores.
Suggested-by: Jesse Gross <jesse@nicira.com>
Ben Pfaff [Thu, 13 May 2010 20:21:33 +0000 (13:21 -0700)]
datapath: Prepare to support 32-bit compatibility ioctls.
This commit prepares the core of datapath.c and vport.c to reduce the
amount of new code duplication when the following commit adds support for
32-bit compatibility ioctls. It breaks a number of functions apart into
pairs of functions: one that copies data to and from userspace and another
that does the real work.
This change is a pure refactoring that should not change behavior.
Ben Pfaff [Thu, 13 May 2010 18:04:56 +0000 (11:04 -0700)]
datapath: Avoid possibility of negative 'n_ports' in struct odp_portvec.
'n_ports' should never be negative so it's better to use an unsigned type.
Suggested-by: Jesse Gross <jesse@nicira.com>
Ben Pfaff [Mon, 10 May 2010 20:53:26 +0000 (13:53 -0700)]
datapath: Avoid possibility of negative 'n_flows' in struct odp_flowvec.
do_flowvec_ioctl() was checking for too-big 'n_flows' but not negative
'n_flows'. We could add that check too, but 'n_flows' should never be
negative so it's better to just use an unsigned type.
Andy Southgate [Tue, 11 May 2010 18:46:52 +0000 (11:46 -0700)]
Add ovsdbmonitor GUI tool by Andy Southgate, contributed by Citrix.
With Makefiles and Autoconfiscation by Ben Pfaff.
Signed-off-by: Thomas Lacroix <thomas.lacroix@citrix.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Wed, 12 May 2010 17:02:23 +0000 (10:02 -0700)]
daemon: Throttle max respawning rate.
If a monitored daemon dies quickly at startup, the system can waste a lot
of CPU time continually restarting it. This commit prevents a given
daemon from restarting more than once every 10 seconds.
Ben Pfaff [Tue, 11 May 2010 17:56:10 +0000 (10:56 -0700)]
daemon: Allow monitored daemon to dump core no more than once.
If the monitored daemon dumps core frequently, then this can quickly
exhaust the host's disk space. This commit limits core dumps to at most
one per monitored session (typically, once per boot).
Ben Pfaff [Thu, 13 May 2010 16:43:33 +0000 (09:43 -0700)]
ofproto: Prefer "master" and "other" connections for snooping over "slave".
This makes "ovs-ofctl snoop" and anything else that connects to the
switch's "snoop" socket typically more useful in the presence of multiple
controllers, since the "master" connection is the one with the most
interesting traffic.
Suggested-by: Justin Pettit <jpettit@nicira.com>
Ben Pfaff [Fri, 7 May 2010 16:29:02 +0000 (09:29 -0700)]
ofproto-sflow: Maintain table of ports even when clearing configuration.
When ofproto_sflow_set_options() fails, it calls ofproto_sflow_clear() to
deconfigure the ofproto_sflow object. But ofproto_sflow_clear() deletes
all of the object's record of datapath ports. That means that the next
call to ofproto_sflow_set_options(), if it succeeds, will believe that the
datapath has no ports.
This commit fixes the problem by only clearing ofproto_sflow's record of
datapath ports when it is destroyed, not just when a configuration error
occurs.
Reported-by: Neil McKee <neil.mckee@inmon.com>
Ben Pfaff [Tue, 11 May 2010 19:44:58 +0000 (12:44 -0700)]
ovs-ofctl: Add "snoop" command.
The "snoop" command does roughly the same thing as "monitor", but it is
easier to use in the common case where one just wants to look at the
OpenFlow controller connection for a bridge. Instead of, for example,
ovs-ofctl monitor unix:/var/run/openvswitch/br0.snoop
one merely types
ovs-ofctl snoop br0
Ben Pfaff [Thu, 6 May 2010 00:08:28 +0000 (17:08 -0700)]
ofproto: Don't send async messages on transient connections by default.
ofproto supports listening for "transient connections" from clients such
as ovs-ofctl. These OpenFlow connections are not supposed to receive
asynchronous messages by default, unless they ask for them by setting an
nonzero packet-in send length. This feature got broken some time back.
This commit fixes it.
Ben Pfaff [Tue, 11 May 2010 19:42:00 +0000 (12:42 -0700)]
ofproto: Fix segfault sending packet_ins on transient connections.
Ben Pfaff [Tue, 11 May 2010 19:22:17 +0000 (12:22 -0700)]
ovs-ofctl: "monitor" command takes no more than 2 arguments
The "monitor" command would accept 3 arguments and ignore the third one.
Reject such usage instead.
Ben Pfaff [Thu, 6 May 2010 00:06:10 +0000 (17:06 -0700)]
ovs-ofctl: Fix usage message for "monitor" command.
The "monitor" command takes at most two arguments, and the second one is
optional, so express that correctly.
Ben Pfaff [Wed, 5 May 2010 17:31:44 +0000 (10:31 -0700)]
Diagnose attempts to connect the wrong protocol to a network port.
Sometimes, when a user asks me to help debug a problem, it turns out that
an SSL connection was being made on a TCP port, or vice versa, or that an
OpenFlow connection was being made on a JSON-RPC port, or vice versa, and
so on. This commit adds log messages that diagnose this kind of problem,
e.g. "tcp:127.0.0.1:6633: received JSON-RPC data on OpenFlow channel".
Ben Pfaff [Mon, 10 May 2010 22:01:25 +0000 (15:01 -0700)]
vconn-stream: Refactor vconn_stream_recv() for readability.
Backward "goto" statement are rarely a good idea, so this rewrites that
code for readability.
Jesse Gross [Mon, 10 May 2010 23:16:33 +0000 (16:16 -0700)]
vport: Record and free packets on unattached ports.
We throw away packets that are received on vports not attached to
a datapath but we are actually leaking them. This records that an
error took place and frees the skb.
Jesse Gross [Tue, 11 May 2010 01:23:18 +0000 (18:23 -0700)]
flow: Zero tun_id if wildcarded.
Normally match fields are zeroed if they are wildcarded in
normalize_match(). However, tun_id isn't part of struct ofp_match
so do it when we convert to a flow instead.
Jesse Gross [Sat, 8 May 2010 01:50:23 +0000 (18:50 -0700)]
gre: Include route headers in headroom calculation.
When calculating the amount of headroom required include the route,
which avoids a copy later when doing transforms like IPsec.
Jesse Gross [Mon, 10 May 2010 20:34:41 +0000 (13:34 -0700)]
bridge: Add iface to hash table before calling iface_is_internal().
When creating an interface we need to check whether it is internal.
However, the function iface_is_internal() does a lookup on the
interface name but we haven't added it to the hash table yet. This
adds the interface to the table early on in iface_create.
NIC-78
Ben Pfaff [Mon, 10 May 2010 17:55:29 +0000 (10:55 -0700)]
bridge: Fix double-free bug in port_reconfigure().
Reported-by: Peter Balland <peter@nicira.com>
Bug #2794
Ben Pfaff [Fri, 7 May 2010 18:43:18 +0000 (11:43 -0700)]
flow: Fix misaligned access.
The testsuite for the flow extractor triggered this.
With this commit, "make check" passes on SPARC.
Ben Pfaff [Fri, 7 May 2010 21:31:04 +0000 (14:31 -0700)]
Add header for access to potentially unaligned data.
I had been under the impression that "memcpy" was a valid way to copy
unaligned data into an aligned location for access. But testing on SPARC
has shown that GCC doesn't always honor that intention. It seems that, if
GCC can see that there is a pointer of a type that requires alignment to
a given object, then it will access it directly regardless of whether
memcpy() is used to copy it.
This commit adds a new header with functions to access unaligned data. I
managed to come up with two techniques, one GCC-specific, one generic, that
do avoid the misaligned access in my test case. The GCC-specific technique
is the same one used by the Linux kernel (although no code has been
literally copied). The other one seemed obvious but possibly slow
depending on the compiler's ability to optimize.
The following commit adds a user.
Ben Pfaff [Fri, 7 May 2010 17:44:01 +0000 (10:44 -0700)]
ovsdb-client: Serialize columns in predictable order on "monitor" command.
The "monitor" command goes to some trouble to write its output in a
predictable order, so that test programs can reliably compare it against
expectations. This commit fixes up one part that was missing, which is
that the columns were not being ordered predictably (it depended on
hash order, which differs between big-endian and little-endian systems).
It also updates the test suite to expect the new order.
Ben Pfaff [Fri, 7 May 2010 17:41:06 +0000 (10:41 -0700)]
tests: Fix bug in "ovsdb-tool compact" test.
This test examines the OVSDB database log and checks that it contains what
it should for specified transactions. However, the database log ordering
differs between big-endian and little-endian architectures because it is
written out in hash order. We don't want to incur the expense of always
sorting the log as we write it out, so instead this commit fixes the
problem by sorting the log as it reads it, using the "test-json" program.
Ben Pfaff [Fri, 7 May 2010 17:38:13 +0000 (10:38 -0700)]
tests: Fix bug in "weak references" test.
One part of the "weak references" test inserts invalid all-zeros weak
references into two columns and expects to get an error message mentioning
one of them. Unfortunately the one that actually gets mentioned depends
on hash ordering and thus differs between big-endian and little-endian
machines. This commit fixes the problem by only putting an invalid
reference in a single column, instead of two of them.
Ben Pfaff [Fri, 7 May 2010 17:34:35 +0000 (10:34 -0700)]
tests: Fix bug in ovsdb-server test suite.
The formatting of OVSDB syntax errors differed between big-endian and
little-endian systems, which caused the "database multiplexing
implementation" test to fail on SPARC. This commit fixes the problem by
always outputting JSON in syntax errors in deterministic (sorted) order.
Ben Pfaff [Fri, 7 May 2010 00:04:11 +0000 (17:04 -0700)]
util: Fix GCC false-positive warning for CONTAINER_OF.
On sparc, GCC would issue the following warning for every use of
CONTAINER_OF:
warning: cast increases required alignment of target type
This is a false positive: assuming that the data structure that it is
applied to was allocated properly, the use of CONTAINER_OF to reach it is
valid. And if it was not allocated properly, then code that accesses it
other ways will have trouble too.
Jesse Gross [Thu, 22 Apr 2010 12:11:50 +0000 (08:11 -0400)]
datapath: Enable offloading on internal devices.
Enables checksum offloading, scatter/gather, and TSO on internal
devices. While these optimizations were not previously enabled on
internal ports we already could receive these types of packets from
Xen guests. This has the obvious performance benefits when these
packets can be passed directly to hardware.
There is also a more subtle benefit for GRE on Xen. GRE packets
pass through OVS twice - once before encapsulation and once after
encapsulation, moving through an internal device in the process.
If it is a SG packet (as is common on Xen), a copy was necessary
to linearize for the internal device. However, Xen uses the
memory allocator to track packets so when the original packet is
freed after the copy netback notifies the guest that the packet
has been sent, despite the fact that it is actually sitting in the
transmit queue. The guest then sends packets as fast as the CPU
can handle, overflowing the transmit queue. By enabling SG on
the internal device, we avoid the copy and keep the accounting
correct.
In certain circumstances this patch can decrease performance for
TCP. TCP has its own mechanism for tracking in-flight packets
and therefore does not benefit from the corrected socket accounting.
However, certain NICs do not like SG when it is not being used for
TSO (these packets can no longer be handled by TSO after GRE
encapsulation). These NICs presumably enable SG even though they
can't handle it well because TSO requires SG.
Tested controllers (all 1G):
Marvell
88E8053 (large performance hit)
Broadcom BCM5721 (small performance hit)
Intel
82571EB (no change)
Ben Pfaff [Fri, 23 Apr 2010 20:15:53 +0000 (13:15 -0700)]
ovsdb: In documentation, add commas to make large numbers easier to read.
Jesse Gross [Thu, 6 May 2010 22:15:50 +0000 (15:15 -0700)]
datapath: Break out test for dev_disable_lro().
It seems that dev_disable_lro() and skb_warn_if_lro() are not always
defined at the same time, despite the fact that they are typically
used together. This independently tests for them.
Justin Pettit [Thu, 6 May 2010 21:05:25 +0000 (14:05 -0700)]
Fix issue with "strict" deletion of flows
OpenFlow provides the ability to delete flows that match a "strict"
description. This means that wildcards are not active, and thus will
only match a single flow that exactly matches the description. The code
that checks for a match is pretty dumb and still compares the values of
fields that are wildcarded. A recent change added a "tun_id" matching
field, but did not zero out the field when it was supposed to be
ignored, which broke the matching used by strict deletions. This sets
the field regardless of whether the field is wildcarded or not.
Reported-by: Natasha Gude <natasha@nicira.com>
Bug #2775
Jesse Gross [Thu, 6 May 2010 20:37:49 +0000 (13:37 -0700)]
datapath: Fix build issue with LRO.
The last commit added a configure test for skb_warn_if_lro() but
reversed an #ifdef causing a function to be compiled when it wasn't
needed.
Jesse Gross [Thu, 6 May 2010 19:31:43 +0000 (12:31 -0700)]
datapath: Add configure test for skb_warn_if_lro().
Some distributions backport this function, so use a configure
test instead of a version check.
CC: Alexey I. Froloff <raorn@altlinux.org>
Ben Pfaff [Tue, 4 May 2010 19:29:39 +0000 (12:29 -0700)]
ofproto: Implement ofp_action_output "max_len" feature.
The "max_len" feature of ofp_action_output is completely unimplemented
currently. Implement it.
Reported-by: Tetsuo NAKAGAWA <nakagawa@mxc.nes.nec.co.jp>
Ben Pfaff [Mon, 3 May 2010 22:38:31 +0000 (15:38 -0700)]
netdev-linux: Optimize removing policing from an interface.
It is very expensive to start a subprocess and, especially, to wait for it
to complete. This replaces the most common subprocess operation in
netdev_linux_set_policing() by a Netlink socket operation, which is much
faster.
Without this and the other netdev-linux commits, my 1000-interface test
case runs in 1 min 48 s. With them, it runs in 25 seconds.
Ben Pfaff [Mon, 3 May 2010 22:37:24 +0000 (15:37 -0700)]
netdev-linux: Cache policing values.
Without this and the following netdev-linux commits, my 1000-interface test
case runs in 1 min 48 s. With them, it runs in 25 seconds.
Ben Pfaff [Mon, 3 May 2010 22:31:38 +0000 (15:31 -0700)]
netdev-linux: Factor out removing policing.
This is duplicated code that the following commit will rewrite.
Ben Pfaff [Mon, 3 May 2010 21:31:16 +0000 (14:31 -0700)]
netdev-linux: Factor out obtaining an RTNL socket.
Another function needs this same functionality in an upcoming commit, so
factor this into a new function get_rtnl_sock().
Ben Pfaff [Mon, 3 May 2010 20:47:28 +0000 (13:47 -0700)]
dpif-linux: Use hash instead of sorted array.
With 1000 network devices being added or removed, sorting the array was a
profiling hot spot. Using a hash makes it drop off the profile.
Ben Pfaff [Mon, 3 May 2010 18:47:56 +0000 (11:47 -0700)]
bridge: Optimize trunk port common case.
Profiling with qprof showed that bitmap_set_multiple() and bitmap_equal()
were eating up quite a bit of CPU time during bridge reconfiguration (up
to about 10% of total runtime). This is completely avoidable in the common
case where a port trunks all VLANs, where we don't really need a bitmap at
all. This commit implements that optimization.
Ben Pfaff [Mon, 3 May 2010 19:30:37 +0000 (12:30 -0700)]
dynamic-string: Optimize ds_put_char().
A qprof profile showed ds_put_char() and ds_put_uninit() as 4% of total
runtime. This commit inlines the common case, which reduces them to 1%
total.
Ben Pfaff [Mon, 3 May 2010 20:42:39 +0000 (13:42 -0700)]
bridge: Optimize port_lookup() using a hash.
Before this commit and the preceding one, with 1000 interfaces strcmp()
took 36% and port_lookup() took 8% of total runtime when reconfiguring
bridges. With these two commits the percentage is reduced to 3% and 0%,
respectively.
Ben Pfaff [Wed, 5 May 2010 21:00:47 +0000 (14:00 -0700)]
bridge: Optimize iface_lookup() and port_lookup_iface() with a hash.
Before this commit and the following one, with 1000 interfaces strcmp()
took 36% and port_lookup() took 8% of total runtime when reconfiguring
bridges. With these two commits the percentage is reduced to 3% and 0%,
respectively.
Ben Pfaff [Mon, 3 May 2010 22:43:49 +0000 (15:43 -0700)]
ovs-vswitchd: Implement "exit" unixctl command.
This is useful for profiling, since common profilers do not print anything
until the process terminates, and only if the process terminates in the
ordinary way by calling exit().
Neil McKee [Wed, 5 May 2010 20:26:23 +0000 (13:26 -0700)]
sflow: Always add poller and sampler together.
he ofproto_sflow_add_poller() and ofproto_sflow_add_sampler() calls
should always be made together, either when a port is added
dynamically with ofproto_sflow_add_port() or when the sflow_agent is
first created in ofproto_sflow_set_options(). I was seeing odd
behavior where either the pollers or the samplers would never be
instantiated depending on the order that things happened. (It's OK to
add the same sampler or poller again, because the library routines
sfl_agent_addPoller() and sfl_agent_addSampler() will just return the
existing one if it is there. Perhaps we should add a comment to make
that clear?).
I changed the parameters to ofproto_sflow_add_sampler to make it work
the same way as ofproto_sflow_add_poller, where the options are
extracted from os->options within the function itself.
Neil McKee [Wed, 5 May 2010 20:24:27 +0000 (13:24 -0700)]
sflow: Properly fill in initial destination VLAN in sFlow output.
Neil McKee [Wed, 5 May 2010 20:24:03 +0000 (13:24 -0700)]
sflow: Include Ethernet FCS in frame_length to comply with sFlow spec.
Ben Pfaff [Wed, 5 May 2010 17:52:46 +0000 (10:52 -0700)]
vswitchd: Fix documentation of "agent" column in "sFlow" table.
The documentation now accurately reflects the implementation. The
implementation, however, leaves a great deal to be desired.
Ben Pfaff [Wed, 5 May 2010 17:50:38 +0000 (10:50 -0700)]
bridge: Fix double-free in sFlow configuration.
Ben Pfaff [Wed, 5 May 2010 17:38:24 +0000 (10:38 -0700)]
ovs-vsctl: Add sFlow to supported set of tables.
Somehow this one got left out accidentally.
Reported-by: Neil McKee <neil.mckee@inmon.com>
Ben Pfaff [Tue, 4 May 2010 17:20:58 +0000 (10:20 -0700)]
xenserver: Make Open vSwitch disable itself in "bridge" mode.
When /etc/xensource/network.conf contains the word "bridge", the system
is supposed to use the Linux bridge, not Open vSwitch. This commit makes
OVS honor this setting, which until now it has mainly ignored.
Reported-by: Reid Price <reid@nicira.com>
Ben Pfaff [Sat, 1 May 2010 21:27:53 +0000 (14:27 -0700)]
xenserverd: Give XAPI a grace period before refreshing network UUIDs.
XAPI updates the pool.conf file before it actually refreshes its database
from the new pool, so we need to wait for that to happen. Hard-coded
delays aren't a good idea, but in the long term XAPI will probably be
adding a hook script for us to use, so this may be an OK stopgap measure.
Bug #2756.
Jesse Gross [Tue, 27 Apr 2010 01:08:54 +0000 (18:08 -0700)]
datapath: Don't hold dp_mutex when setting internal devs MTU.
We currently acquire dp_mutex when we are notified that the MTU
of a device attached to the datapath has changed so that we can
set the internal devices to the minimum MTU. However, it is not
required to hold dp_mutex because we already have RTNL lock and it
causes a deadlock, so don't do it.
Specifically, the issue is that DP mutex is acquired twice: once in
dp_device_event() before calling set_internal_devs_mtu() and then
again in internal_dev_change_mtu() when it is actually being changed
(since the MTU can also be set directly). Since it's not a recursive
mutex, deadlock.
Justin Pettit [Fri, 30 Apr 2010 22:09:34 +0000 (15:09 -0700)]
xenserver: Wire up emergency reset plug-in and call it on manager change
Add code to "emergency_reset" plug-in method to actually do its work.
Also, when the manager is changed, call the emergency reset command to
clear out any configuration that may have been done by a previous
controller.
Justin Pettit [Fri, 30 Apr 2010 22:06:51 +0000 (15:06 -0700)]
ovs-vsctl: Add emergency reset command
Add the "emer-reset" command, which is used to clear the configuration of
items likely to have been configured by the manager. This will leave
the core networking configuration as it was.
Ben Pfaff [Fri, 30 Apr 2010 21:16:25 +0000 (14:16 -0700)]
ovsdb-idl: Add "safe" iterator macro to generated code.
Jesse Gross [Mon, 26 Apr 2010 21:19:28 +0000 (14:19 -0700)]
datapath: Ensure packet length matches headers during checksum setup.
During the setup of checksumming pointers we need to make sure that
the transport headers are in the skb linear data area. However, we
don't currently verify that the lengths in the packet headers are
within the size of the packet. This makes that check before a
BUG() check does it for us.
CC: "Nick Couchman" <Nick.Couchman@seakr.com>
Jesse Gross [Thu, 29 Apr 2010 21:54:10 +0000 (14:54 -0700)]
bridge: Immediately drop interfaces that can't be opened.
Previously we would keep interfaces around that couldn't be opened
because they might be internal interfaces that are created later.
However, this leads to a race condition if the interface appears
after we try to create it and fails since some operations may
succeed. Instead, give up on the interface immediately if it can't
be opened and isn't internal (which we control and so won't have
this issue).
Bug #2737
Ben Pfaff [Tue, 27 Apr 2010 19:41:11 +0000 (12:41 -0700)]
vport: Better handle too-long network device names in vport_del().
The 'count' argument to strncpy_from_user() is supposed to include space
for the null terminator, so add it in. Also, refuse names that have more
than IFNAMSIZ-1 characters outright, instead of truncating them.
Ben Pfaff [Tue, 27 Apr 2010 17:45:28 +0000 (10:45 -0700)]
datapath: Check device name length more carefully in create_dp().
I don't see any value in silently truncating device names. Doing so will
sow confusion in userspace. This commit makes too-long device names
return ENAMETOOLONG.
Ben Pfaff [Tue, 27 Apr 2010 17:43:24 +0000 (10:43 -0700)]
datapath: Always null-terminate network device name in create_dp().
strncpy() does not null-terminate its output buffer if the source string's
length is at least as large as its 'count' argument. We know that the
source and destination buffers are the same size and that the source buffer
is null-terminated, so just use strcpy().
This fixes a kernel BUG message that often occurred when strlen(devname)
was exactly IFNAMSIZ-1. In such a case, if
internal_dev_port.devname[IFNAMSIZ-1] happened to be nonzero, it would
eventually fail the following check in alloc_netdev_mq():
BUG_ON(strlen(name) >= sizeof(dev->name));
Bug #2722.
Ben Pfaff [Tue, 27 Apr 2010 17:21:12 +0000 (10:21 -0700)]
datapath: Fix argument to strncpy_from_user().
The strncpy_from_user() function's 'count' argument is documented to
include the trailing null byte, but create_dp() did not include it. This
commit adds it in.
Ben Pfaff [Tue, 27 Apr 2010 16:40:46 +0000 (09:40 -0700)]
ofproto: Avoid buffer copy in OFPT_PACKET_IN path.
When a dpif passes an odp_msg down to ofproto, and ofproto transforms it
into an ofp_packet_in to send to the controller, until now this always
involved a full copy of the packet inside ofproto. This commit eliminates
this copy by ensuring that there is always enough headroom in the ofpbuf
that holds the odp_msg to replace it by an ofp_packet_in in-place.
From Jean Tourrilhes <jt@hpl.hp.com>, with some revisions.
Ben Pfaff [Tue, 27 Apr 2010 16:37:06 +0000 (09:37 -0700)]
xenserver: Use start_daemon for xenserverd also in /etc/init.d/openvswitch.
Reported-by: Justin Pettit <jpettit@nicira.com>
Ben Pfaff [Tue, 27 Apr 2010 16:36:30 +0000 (09:36 -0700)]
xenserver: Report correct daemon names at startup in /etc/init.d/openvswitch.
Reported-by: Justin Pettit <jpettit@nicira.com>
Ben Pfaff [Tue, 27 Apr 2010 16:35:45 +0000 (09:35 -0700)]
xenserver: Use daemon-specific dir for pidfile in /etc/init.d/openvswitch.
Reported-by: Justin Pettit <jpettit@nicira.com>