Ben Pfaff [Mon, 24 Jan 2011 22:03:57 +0000 (14:03 -0800)]
datapath: Move rcu_read_lock_held(), rcu_dereference_*() to rcupdate.h.
<linux/rcupdate.h> is the normal location of these functions, so we should
not put compat code for them in <linux/rtnetlink.h>.
This caused build failures for datapath/table.c, which in later commits
uses rcu_dereference_check() and rcu_dereference_protected() but does not
need RTNL.
Signed-off-by: Ben Pfaff <blp@nicira.com> Acked-by: Jesse Gross <jesse@nicira.com>
Ethan Jackson [Mon, 24 Jan 2011 18:54:17 +0000 (10:54 -0800)]
xenserver: Change all bridge's fail_mode when updating.
The ovs plugin's update command was only setting fail_mode for
bridges attached to pifs. This patch applies the fail_mode
setting to all bridges on the XenServer system.
Reported-by: David Tsai <dtsai@nicira.com> Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ben Pfaff [Sat, 22 Jan 2011 01:09:21 +0000 (17:09 -0800)]
multipath: Fix off-by-one in hash_threshold multipath calculation.
0xffffffff / (0xffffffff / n) can have a value as large as n (consider the
n == 1 case), but we need a value no bigger than n-1. So add 1 before
dividing to fix the problem.
This caused a test failure on Debian "lenny" amd64 when apparently
unrelated code changed.
Ben Pfaff [Fri, 21 Jan 2011 22:49:01 +0000 (14:49 -0800)]
ofproto: Correctly compute length of flow stats in multipart flow stats.
In a flow table dump, the length of the first nx_flow_stats in the second
and subsequent OpenFlow messages was being miscalculated, because
append_nxstats_reply() changed the length of the message in such cases.
So we need to load the starting length after calling append_nxstats_reply()
instead of before.
Ben Pfaff [Fri, 21 Jan 2011 00:34:15 +0000 (16:34 -0800)]
datapath: Remove vport_del_all() because it is now a no-op.
vport_del_all() was created when vports could exist without being attached
to any datapath. Now, a vport is always attached to a datapath. This
function was only called on module unload, but the module can't be unloaded
if any datapath exists, so it won't ever have any work to do, and we might
as well delete it entirely.
Justin Pettit [Tue, 18 Jan 2011 08:01:24 +0000 (00:01 -0800)]
ovs-monitor-ipsec: Use "require" when adding SPD entries.
Previously, SPD entries were added with a level of "default". This uses
the system-wide default for the protocol when processing a matching
packet. Switch the level to "require" so that a SA is always used when
sending the packet.
Ethan Jackson [Wed, 19 Jan 2011 22:46:57 +0000 (14:46 -0800)]
xenserver: Allow NULL pool in configuration cache.
InterfaceReconfigure assumes the pool is in the configuration
cache. This is always true except when upgrading from an older
version of openvswitch which does record the pool. This can cause
upgrades to lock up.
Ethan Jackson [Mon, 17 Jan 2011 19:31:46 +0000 (11:31 -0800)]
xenserver: Allow unknown other-config attributes.
When saving the database cache, InterfaceReconfigure can crash if
unexpected attributes are in an object's other-config column. This
commit causes it to skip that attribute and log a warning.
Signed-off-by: Ethan Jackson <ethan@nicira.com> CC: Ian Campbell <Ian.Campbell@citrix.com> CC: Dominic Curran <Dominic.Curran@citrix.com>
Ethan Jackson [Fri, 14 Jan 2011 00:29:31 +0000 (16:29 -0800)]
route-table: Handle route updates more robustly.
The kernel does not broadcast rtnetlink route messages in all cases
one would expect. This can cause stale entires to end up in the
route table which may cause incorrect results for
route_table_get_ifindex() queries. This commit causes rtnetlink
route messages to dump the entire route table on the next
route_table_get_ifindex() query.
Ethan Jackson [Wed, 12 Jan 2011 22:55:18 +0000 (14:55 -0800)]
lib: Simplify rtnetlink routing functionality.
This commit removes the rtnetlink-route module and replaces it with
a much simpler to use route-table module. The route-table uses
rtnetlink to maintain a routing table which may be used to query
the egress interface of particular addresses.
This commit also converts netdev-vport to use the new route-table
module.
Ben Pfaff [Wed, 12 Jan 2011 18:44:43 +0000 (10:44 -0800)]
datapath: Get rid of compat.h, compat26.h in favor of modern approach.
I had completely forgotten that we had a top-level compat.h and compat26.h.
It's better to distribute their contents to individual compat headers, so
this commit does so and deletes them.
Signed-off-by: Ben Pfaff <blp@nicira.com> Acked-by: Jesse Gross <jesse@nicira.com>
Sajjad Lateef [Thu, 13 Jan 2011 01:07:58 +0000 (17:07 -0800)]
debian: Modify ovs-bugtool to include collectd rrd logs
collectd is a utility that stores rrd logs in the directory
/var/lib/collectd/rrd/ by default. rrd logs are created when
collectd is configured to output logs in Round Robin Database
format.
With this change, ovs-bugtool will include collectd logs in the
output bundle from /var/lib/collectd/rrd. The output bundle will
include all sub-directories of the specified collectd directory.
ovs-bugtool checks if a directory exists before it attempts to
include the directory contents in the output bundle. So, running
ovs-bugtool on a system that does not have collectd logs should
not negatively affect the behavior of ovs-bugtool.
Ben Pfaff [Wed, 12 Jan 2011 21:57:53 +0000 (13:57 -0800)]
ofp-print: Add tests for OpenFlow message formatting.
It's become clear over the past few weeks that some people really depend
on the formatting of ofp-print output. It has also become clear that OVS
doesn't have anything that actually checks or enforces the formatting of
the output. The former is not something I want to encourage, but the
latter is somewhat worrisome because the formatting does change from time
to time and I want to make sure that that is intentional.
So this commit adds at least one test for each supported OpenFlow and
Nicira extension message, so that we can better keep track of changes.
Ethan Jackson [Wed, 5 Jan 2011 19:51:15 +0000 (11:51 -0800)]
bridge: Move tunnel_egress_iface to status column.
This commit removes the tunnel_egress_iface column from the
interface table and moves it's data to the status column. In the
process it reverts the database to version 1.0.0.
Justin Pettit [Tue, 11 Jan 2011 05:32:09 +0000 (21:32 -0800)]
xenserver: Remove "Required-" parameters from "openvswitch" init script.
Commit 72dfb3b (xenserver: Add LSB init info to scripts.) added LSB init
info to the XenServer scripts. This was verified to work on XenServer
5.5, but newer versions of XenServer interpret these values and alter
when "openvswitch" gets called. This causes XenServer to not come up
with its networking configuration properly.
By removing the "Required-" parameters, OVS will properly come up. This
is just a temporary fix, and a more proper one will be forthcoming.
Ben Pfaff [Mon, 10 Jan 2011 20:54:26 +0000 (12:54 -0800)]
tests: Fix Y2011 bug in testsuite.
The tests have been failing for a few days now, because the PKI expired a
few days into 2011. This commit instead generates the PKI at "make check"
time, which has the additional benefit of getting some test exposure for
the ovs-pki program.
Reported-by: Aaron M. Ucko <ucko@debian.org> CC: 609506@bugs.debian.org
Patrick Mullaney [Wed, 15 Dec 2010 19:07:10 +0000 (11:07 -0800)]
xenserver: Remove Linux bridge on startup if no bridges exist.
Otherwise, if the bridge module is loaded but no bridges exist, on all
but the most recent Linux kernels this prevents the Open vSwitch kernel
module from loading.
Patrick Mullaney [Wed, 15 Dec 2010 18:39:08 +0000 (10:39 -0800)]
xenserver: Don't require /etc/xensource/network.conf if not on XenServer.
On XenServer systems, the "network mode" (that is, whether the network
uses OpenvSwitch or the Linux bridge) is stored in
/etc/xensource/network.conf. On other systems, we can't expect it to be
there. This changes the init script to tolerate its absence, treating that
as meaning that Open vSwitch should be used (otherwise, why did the admin
install it?).
NETWORK_MODE can always be overridden in /etc/sysconfig/openvswitch anyway.
Signed-off-by: Patrick Mullaney <pm.mullaney@gmail.com>
[changed back to read /etc/xensource/network.conf if present] Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Wed, 5 Jan 2011 19:08:25 +0000 (11:08 -0800)]
xenserver: Generate system UUID for non-XenServer systems.
Currently the scripts in xensever/ are intended specifically for XenServer,
but supporting other distros seems like a worthy goal, so this is a step
in the right direction.
It would be good to be able to determine a suitable system-type and
system-version, but those are less important than system-id.
Signed-off-by: Patrick Mullaney <pm.mullaney@gmail.com>
[changed not to set xs-system-uuid on non-XenServer systems]
[whitespace changes]
[changed to handle missing uuidgen, suggested by Justin Pettit] Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Wed, 5 Jan 2011 19:00:41 +0000 (11:00 -0800)]
xenserver: Be compatible with non-XenServer func libraries in init script.
XenServer has its function library in /etc/init.d/functions but other
distros have it in different places. Currently this init script is
specifically intended for XenServer but adding compatibility with other
distros seems like a worthwhile goal.
Also, SuSE does not have the "action" function, so test whether it is
implemented and when not supply a fallback.
Signed-off-by: Patrick Mullaney <pm.mullaney@gmail.com>
[changed to only define action() if not already defined] Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Wed, 5 Jan 2011 18:52:36 +0000 (10:52 -0800)]
xenserver: Add LSB init info to scripts.
XenServer itself does not use this information as part of startup, but
other distributions that may wish to derive their startup scripts from
these may need it.
Signed-off-by: Patrick Mullaney <pm.mullaney@gmail.com>
[updated some info in xenserver/etc_init.d_openvswitch-xapi-update]
[used Debian LSB info for xenserver/etc_init.d/openvswitch] Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Fri, 7 Jan 2011 17:35:19 +0000 (09:35 -0800)]
debian: Update check for Debian package version.
A version number like "1.1.0pre2" is properly translated into a Debian
version number as "1.1.0~pre2", so we need to make that translation. Also,
the Debian version number will often have suffixes on it, so don't check
for a trailing parenthesis.
Both tunnel.c and vport-patch.c have set_config() functions. They are
static so this does not confuse the compiler, but it does (slightly)
confuse me from time to time, so this commit changes their names to
tnl_set_config() and patch_set_config(), respectively.
Signed-off-by: Ben Pfaff <blp@nicira.com> Acked-by: Jesse Gross <jesse@nicira.com>
Ben Pfaff [Tue, 4 Jan 2011 20:36:07 +0000 (12:36 -0800)]
datapath: Improve vport-patch names that are too generic.
"device_config" and "devconf" always sound to me like something that would
be part of the kernel's core device layer. Give them the more-specific
names "patch_config" and "patchconf".
Signed-off-by: Ben Pfaff <blp@nicira.com> Acked-by: Jesse Gross <jesse@nicira.com>
Ben Pfaff [Wed, 5 Jan 2011 18:17:17 +0000 (10:17 -0800)]
datapath: Fix vport_get_stats() in !VPORT_F_GEN_STATS case.
When VPORT_F_GEN_STATS was not set, vport_get_stats() would always return
an error (either an error returned by ->get_stats(), otherwise
-EOPNOTSUPP). This fixes the problem.
Found by inspection.
Signed-off-by: Ben Pfaff <blp@nicira.com> Acked-by: Jesse Gross <jesse@nicira.com>
Simon Horman [Thu, 6 Jan 2011 01:26:42 +0000 (10:26 +0900)]
datapath: Treat GSO skbs as if they were fragments
In dp_output_control() UDP GSO skbs are split into fragments which are
passed to userspace. So the resulting flow set-up by the controller
(I am using ovs-vswitchd) is created based on a fragment. This means
that the UDP source and destination port of the flow is zero.
In order for the datapath to match the resulting flow flow_extract() needs
to treat UDP GSO skbs as if they are fragments. That is, set the UDP
source and destination port to 0.
A flow established for a UDP GSO skb with this change won't match any
subsequent non-GSO skbs, they will need to be passed to the controller and
a new flow established. But without this change no UDP GSO skbs will ever
match any flow.
I noticed this while using KVM using virtio with VhostNet and netperf's
UDP_STREAM test. The result was that the test sent ~5Gbit/s but only a
small fraction of that was received by the other side. Much less than the
1Gbit/s available on the physical link between the host (and guest) and the
machine running netserver. 100% of one of the host's CPUs was consumed, 50%
for the host and 50% for the guest. The host consumption was contributed
to largely by ovs-vswitchd.
With this change I get a much nicer result of a fraction under 1Gbit/s sent
and almost all packets ending up at the other end.
Signed-off-by: Simon Horman <horms@verge.net.au> Signed-off-by: Jesse Gross <jesse@nicira.com>
Jesse Gross [Wed, 5 Jan 2011 15:25:41 +0000 (07:25 -0800)]
datapath: Fix double counting of packet stats for Linux devices.
The kernel augments stats for Linux devices that only provide 32-bit stats
with its own internal 64-bit counters. When doing this it takes the error
stats from the device but uses the packet and byte values from its local
counters. However, we were also taking the packet and byte counts from
the device, leading to double counting.
Jesse Gross [Wed, 5 Jan 2011 20:39:57 +0000 (12:39 -0800)]
datapath: Use call_rcu() when deleting a datapath.
When deleting a datapath, we remove all of the vports and then immediately
free the datapath data structures. Since the vports are allowed to use
call_rcu() to free their data, it's possible for them to return immediately
while packet processing is still taking place. This breaks apart the dropping
of references and the freeing of the data using call_rcu() for protection.
This race cannot actually occur in practice since the last port to be
deleted is an internal device, which uses synchronize_rcu() itself
(implicitly through unregister_netdevice()). However, there is no
requirement that it must do this nor should there be.
Reported-by: Ben Pfaff <blp@nicira.com> Signed-off-by: Jesse Gross <jesse@nicira.com> Acked-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Tue, 4 Jan 2011 18:19:59 +0000 (10:19 -0800)]
debian: Remove openvswitch-pki-server package.
The functionality provided by this package was used only by
openvswitch-switch-config, which was removed some time ago. If I had been
more careful at the time, I would have removed this package at the same
time.
Ethan Jackson [Tue, 21 Dec 2010 21:44:37 +0000 (13:44 -0800)]
rtnetlink: Remove LINK specific messages from rtnetlink
Abstracted rtnetlink so that it may be used for messages other than
RTM LINK messages. Created a new rtnetlink-link module which
specifically deals with these kinds of messages and follows the old
rtnetlink API.
Ethan Jackson [Sun, 2 Jan 2011 20:52:11 +0000 (12:52 -0800)]
ofp-parse: ofp-parse fails to properly validate DROP.
The str_to_action function of ofp-parse was not properly
incrementing it's actions counter. Thus it did not enforce the
requirement that DROP actions not be preceded by other actions.
Jesse Gross [Wed, 29 Dec 2010 19:42:13 +0000 (11:42 -0800)]
datapath: Drop constness of datapath pointers.
A few places marked struct datapath pointers as const since they
didn't expect to make modifications. However, when compiled with
lockdep the datapath mutex pointer is passed to lockdep_is_held(),
which has a non-const argument. That provoked warnings about
casting away the const, so this drops the const from the original
pointers.
Reported-by: Ben Pfaff <blp@nicira.com> Signed-off-by: Jesse Gross <jesse@nicira.com> Acked-by: Ben Pfaff <blp@nicira.com>
Jesse Gross [Wed, 29 Dec 2010 18:59:01 +0000 (10:59 -0800)]
datapath: Report ifindex of 0 if vport doesn't have one.
If a vport is a virtual device then it doesn't have a system ifindex.
We currently return the ifindex of the bridge device in this situation
but that's somewhat misleading, so this replaces it with 0. Nothing
actually reads the ifindex for devices other than the bridge device,
so this doesn't have a functional change.
Suggested-by: Ben Pfaff <blp@nicira.com> Signed-off-by: Jesse Gross <jesse@nicira.com> Acked-by: Ben Pfaff <blp@nicira.com>
Jesse Gross [Wed, 29 Dec 2010 23:05:02 +0000 (15:05 -0800)]
datapath: Don't check for RCU in free_buckets in table.
free_buckets() is only called in places where the lifetime of its
container has ended: on allocation failure and on deletion after
a grace period. If the container can no longer be referenced then
neither can the buckets, so it is safe to directly free them.
sparse complains if the pointer is directly dereferenced and lockdep
complains if the RCU functions are used without some type of lock,
both of which are fine in this case. This adds an explicit cast to
avoid the complaints.
Found with lockdep.
Signed-off-by: Jesse Gross <jesse@nicira.com> Acked-by: Ben Pfaff <blp@nicira.com>
Jesse Gross [Wed, 29 Dec 2010 22:48:38 +0000 (14:48 -0800)]
datapath: Hold dp->mutex when calling new_vport().
On datapath creation we hold dp_mutex but not dp->mutex when
creating the vport for the datapath device. However, there are
lockdep checks that validate that we hold dp->mutex during the call
to new_vport(). The lock isn't actually necessary in this case
because no one else can access the datapath but it's good to have
the lock assertions, so this holds dp->mutex while initializing
the datapath.
Found with lockdep.
Signed-off-by: Jesse Gross <jesse@nicira.com> Acked-by: Ben Pfaff <blp@nicira.com>
Jesse Gross [Wed, 29 Dec 2010 22:10:50 +0000 (14:10 -0800)]
datapath: Add owner to file_operations declaration.
It's currently possible for operations on our character device to
be still running when we unload the module. This will result in
an oops when the executing code is suddenly freed. The chrdev
code has a way to avoid this by taking a reference on the module
every time the device is opened, which means that we can't be
unloaded as long as there is an open file descriptor and therefore
the possibility of an operation. However, our file_operations
structure doesn't include an owner member, which prevents this
mechanism from working. This adds one.
Signed-off-by: Jesse Gross <jesse@nicira.com> Acked-by: Ben Pfaff <blp@nicira.com>
Jesse Gross [Wed, 29 Dec 2010 21:09:56 +0000 (13:09 -0800)]
datapath: Acquire dp->mutex when deleting a datapath.
It's possible that someone is using the datapath data structures
when we attempt to delete the datapath. The first writer will
only hold dp->mutex, which we don't currently acquire when deleting.
This adds that lock to prevent a potential race (this can't currently
happen because userspace is single threaded, as long as "ovs-dpctl
del-dp" is not used at the same time).
Signed-off-by: Jesse Gross <jesse@nicira.com> Acked-by: Ben Pfaff <blp@nicira.com>
Jesse Gross [Wed, 29 Dec 2010 21:08:15 +0000 (13:08 -0800)]
datapath: Merge do_destroy_dp into destroy_dp.
Both do_destroy_dp() and destroy_dp() are small functions and
only have a single caller. There's no good reason for them to
be separate so this merges them together. It also makes things
more logically consistent and easier to read in the next commit,
which adds additional locking as everything is in one place.
Signed-off-by: Jesse Gross <jesse@nicira.com> Acked-by: Ben Pfaff <blp@nicira.com>
projects / openvswitch / log