sliver-openvswitch.git
13 years agoovs-vsctl: Check for uncreated symbols earlier.
Ben Pfaff [Mon, 28 Feb 2011 20:45:16 +0000 (12:45 -0800)]
ovs-vsctl: Check for uncreated symbols earlier.

The check for uncreated symbols does not rely on anything that happens
during transaction commit, so there is no point in allowing the transaction
to be sent to the database server, only to fail there, before reporting
the problem locally.

13 years agoovsdb: Improve error message for duplicate uuid-name.
Ben Pfaff [Wed, 9 Mar 2011 20:42:46 +0000 (12:42 -0800)]
ovsdb: Improve error message for duplicate uuid-name.

ovsdb_execute_insert() tried to return a helpful error message when there
was a duplicate uuid-name, but ovsdb_execute() (its caller) makes any
parse error override a parse error.  Since ovsdb_execute_insert() would
skip parsing the row when the uuid-name was a duplicate, this meant that
the error actually reported would be that "row" was not allowed here, which
wasn't at all helpful (since "row" is in fact mandatory).

This commit clears up the problem by always retrieving the "row" member,
which required a small amount of refactoring, and adds a test.

13 years agoovsdb: Improve error reporting for some internal errors.
Ben Pfaff [Wed, 9 Mar 2011 20:41:37 +0000 (12:41 -0800)]
ovsdb: Improve error reporting for some internal errors.

Sometimes internal errors are generated based on an originating error.  In
these cases we were just throwing this information away.  This commit adds
this information to the internal error report so that the error will be
easier to track down.

I haven't actually seen a situation like this come up.

13 years agoovsdb-data: Verify that named-uuid string is an <id>.
Ben Pfaff [Mon, 28 Feb 2011 23:43:29 +0000 (15:43 -0800)]
ovsdb-data: Verify that named-uuid string is an <id>.

The "uuid-name" that creates symbols must be an <id> but we weren't
verifying the same constraint on the "named-uuid"s that refer to symbols,
which was a bit confusing in writing transactions by hand.  This commit
fixes the inconsistency and updates the SPECS file to clarify that a
named-uuid string has to be an <id>.

13 years agoovsdb-idlc: Initialize refType in IDL reference columns' types.
Ben Pfaff [Mon, 28 Feb 2011 20:36:34 +0000 (12:36 -0800)]
ovsdb-idlc: Initialize refType in IDL reference columns' types.

Otherwise the refType always appears to be 0 (OVSDB_REF_STRONG).

Nothing in the code that uses these structures actually uses the refType
yet, so this does not fix any existing bug, but upcoming commits will add
users.

13 years agopython/ovs/ovsuuid: Fix behavior of UUID.from_json() with no symbol table.
Ben Pfaff [Mon, 28 Feb 2011 23:33:08 +0000 (15:33 -0800)]
python/ovs/ovsuuid: Fix behavior of UUID.from_json() with no symbol table.

UUID.from_json(['named-uuid', 'x'], None) should raise an error about
invalid syntax, but instead it was raising a TypeError because it would
try to evaluate "'x' not in None".  This fixes the problem and adds a test.

13 years agoreconnect: Rename CONNECT_IN_PROGRESS state to CONNECTING (like rconn).
Andrew Evans [Wed, 9 Mar 2011 18:42:24 +0000 (10:42 -0800)]
reconnect: Rename CONNECT_IN_PROGRESS state to CONNECTING (like rconn).

reconnect uses the same connection state names as rconn with the exception of
the above. This commit makes their states identical, which should reduce
confusion for people debugging connection problems.

13 years agoxenserver: Delete Bridge when creating vlan.
Ethan Jackson [Thu, 10 Mar 2011 00:12:10 +0000 (16:12 -0800)]
xenserver: Delete Bridge when creating vlan.

Commit 34edeccfd8d4273844d12c85fe38ec47002c47cd removed a
workaround which still applies to XenServer versions older than
5.5.  This could causes creation of vlans to fail in some cases.

CC: "David Tsai" <dtsai@nicira.com>
Bug #4877.

13 years agoreconnect: Fix printf() format warning.
Ethan Jackson [Thu, 10 Mar 2011 18:21:13 +0000 (10:21 -0800)]
reconnect: Fix printf() format warning.

tests/test-reconnect.c:245:9: error: format '%llu' expects type
'long long unsigned int', but argument 2 has type 'unsigned int'

13 years agoreconnect: Track last-disconnected time.
Andrew Evans [Thu, 10 Mar 2011 02:36:26 +0000 (18:36 -0800)]
reconnect: Track last-disconnected time.

Commit a4613b01ab (ovsdb: Change the way connection duration time is reported
in Manager table.), pushed earlier today, requires this commit, so OVSDB has
been unbuildable from then to now.

13 years agodatapath: Enable vlan offload on internal devices.
Jesse Gross [Fri, 4 Mar 2011 23:52:43 +0000 (15:52 -0800)]
datapath: Enable vlan offload on internal devices.

Although it is generally best to configure vlans directly though
Open vSwitch, enabling vlan acceleration on internal devices can
avoid some issues and hardware limitations if Linux vlan devices
are used.  It is only used on kernels that support modern vlan
data structures, which are 2.6.27 and later.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agoovs-ofctl: Make add-flows command read from stdin if file name is "-".
Ben Pfaff [Wed, 9 Mar 2011 22:44:20 +0000 (14:44 -0800)]
ovs-ofctl: Make add-flows command read from stdin if file name is "-".

It is conventional for Unix tools to read from standard input if "-" is
specified as a file name.  It's easy for "ovs-ofctl add-flows" to behave
this way, too, so this commit implements it.

Suggested-by: Paul Ingram <paul@nicira.com>
13 years agoxenserver: Remove other_config option for setting controller.
Justin Pettit [Mon, 7 Mar 2011 00:54:58 +0000 (16:54 -0800)]
xenserver: Remove other_config option for setting controller.

Since XenServer 5.6, "vswitch_controller" has been a first-class key for
setting the Open vSwitch controller.  Prior to that, we stored it in
"other_config".  Since we no longer support such old versions, we can
remove references to the "other_config" option.

Signed-off-by: Justin Pettit <jpettit@nicira.com>
13 years agoutilities: Improve ovs-vlan-test man page.
Ethan Jackson [Wed, 9 Mar 2011 20:41:26 +0000 (12:41 -0800)]
utilities: Improve ovs-vlan-test man page.

The IP addresses used in the ovs-vlan-test example where more or
less random.  This caused a great deal of confusion which hopefully
this patch will remedy.

Also changes the management port to 8080 in the examples because
XenServer uses port 80 on Dom0 for something.

13 years agoovsdb: Change the way connection duration time is reported in Manager table.
Andrew Evans [Wed, 9 Mar 2011 18:46:57 +0000 (10:46 -0800)]
ovsdb: Change the way connection duration time is reported in Manager table.

Commit 0b3e7a8b71 (ovsdb-server: Write manager status information to Manager
table.) attempted to provide managers with the ability to debug manager-related
connection problems, but it turns out that reporting "time_in_state" is not
very useful, because the state is constantly changing. What people really want
is the time each manager has been connected or disconnected, depending on the
current connection state.

Replace "time_in_state" key with "time_connected" and "time_disconnected"
keys. Only one exists at a time, and time is in seconds.

Bug #4833.

13 years agoofproto: Report time connected or disconnected, not time in rconn state.
Andrew Evans [Wed, 9 Mar 2011 18:45:26 +0000 (10:45 -0800)]
ofproto: Report time connected or disconnected, not time in rconn state.

Commit bffc058934 (bridge: Add controller status to Controller table.) exposed
the underlying time in a particular connection state for each controller, but
this turns out not to be very useful, because controller state tends to
fluctuate even when the connection is unbroken. Change to set
"time_connected" or "time_disconnected" keys in the status column of the
Controller table.

Bug #4833.

13 years agodatapath: Avoid random tun_id in odp_packet_cmd_execute().
Ben Pfaff [Wed, 9 Mar 2011 18:39:30 +0000 (10:39 -0800)]
datapath: Avoid random tun_id in odp_packet_cmd_execute().

flow_extract() sets key->tun_id from OVS_CB(packet)->tun_id, which until
now has contained whatever Netlink put there in its NETLINK_CB structure.
Zero it earlier so that its value is at least predictable.

The resulting code is still not correct, because key->tun_id and
key->in_port are now set to arbitrary values.  I have known about this
since I wrote this function (and before, too, in its earlier incarnations),
but until now I did not think that it was a problem because I did not
think that there were any users along this code path.  But that is wrong:
sFlow sampling uses tun_id and in_port and ODP_ACTION_ATTR_CONTROLLER uses
in_port.  So we need a way to pass these back down from userspace.  An
upcoming commit will add a way.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
13 years agovswitch: Fix typos in schema documentation.
Andrew Evans [Tue, 8 Mar 2011 19:21:48 +0000 (11:21 -0800)]
vswitch: Fix typos in schema documentation.

13 years agonetdev-port: Fix invalid memory access in netdev_vport_poll_add().
Ben Pfaff [Fri, 4 Mar 2011 21:44:35 +0000 (13:44 -0800)]
netdev-port: Fix invalid memory access in netdev_vport_poll_add().

shash_find_data() returns an shash_node's 'data' member, but this code here
wants the shash_node itself, so it needs to use shash_find() instead.

This bug meant that any attempt to add a single netdev_vport to more than
one netdev_monitor would cause a segmentation fault.  Here's an example
command that reproduces it reliably for me under valgrind (because ofproto
always monitors its ports and the bridge monitors bond interfaces):

ovs-vsctl -- add-bond br0 bond0 p0 p1 \
          -- set interface p0 type=patch options:peer=p1 \
          -- set interface p1 type=patch options:peer=p0

Bug #4527.
Reported-by: Krishna Miriyala <krishna@nicira.com>
13 years agotunneling: Don't leak dst on vlan tagging failure.
Jesse Gross [Fri, 4 Mar 2011 20:56:25 +0000 (12:56 -0800)]
tunneling: Don't leak dst on vlan tagging failure.

When we need to send a packet through the IP stack we need to attach
a dst to the skb.  Due to some interactions with caching this doesn't
happen until late in the transmission process.  However, if we are
inserting a vlan tag and the insertion fails it is possible to leak
the dst.

Reported-by: Ben Pfaff <blp@nicira.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agoovs-dpctl: Support more than one option for "add-if" command.
Ben Pfaff [Fri, 4 Mar 2011 20:46:37 +0000 (12:46 -0800)]
ovs-dpctl: Support more than one option for "add-if" command.

This "while" loop in do_add_if() is supposed to split up everything after
the interface name with ',' as the delimiter, but it didn't do that
correctly.

Also corrects a typo in the manpage pointed out by Justin Pettit.

13 years agovswitchd: Initialize configuration on active-backup bonds.
Ethan Jackson [Fri, 4 Mar 2011 18:59:40 +0000 (10:59 -0800)]
vswitchd: Initialize configuration on active-backup bonds.

Some configuration settings which were not balance-[slb|tcp]
specific were only initialized for these bond types.

Bug #4806.

13 years agoofproto: facet_execute() stats changes.
Ethan Jackson [Thu, 3 Mar 2011 23:59:25 +0000 (15:59 -0800)]
ofproto: facet_execute() stats changes.

facet_execute() should rely on facet_update_stats() to update the
relevant facet's used parameter.

Before this patch, in very rare cases, the current practice could
lead to assertion failures.  The used value of a facet could be
slightly in the future due to the clocks being out of sync between
the kernel and user space.  Thus manually setting the used
parameter of a facet could put it slightly behind an already pushed
used value causing an assertion failure.

Bug #4745.

13 years agoofproto: Update facet stats when used time increases.
Ethan Jackson [Thu, 3 Mar 2011 23:57:10 +0000 (15:57 -0800)]
ofproto: Update facet stats when used time increases.

13 years agodatapath: Add backport for USHRT_MAX.
Jesse Gross [Thu, 3 Mar 2011 21:38:09 +0000 (13:38 -0800)]
datapath: Add backport for USHRT_MAX.

Kernels before 2.6.26 didn't have type limits for short at all
and before 2.6.35 it was called SHORT instead of SHRT.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agodatapath: Drop BUG_ON when checksumming and copying.
Jesse Gross [Thu, 3 Mar 2011 21:53:08 +0000 (13:53 -0800)]
datapath: Drop BUG_ON when checksumming and copying.

Normally when performing checksum offloading the transport header
must be present in the linear data area.  However, this might not
be the case with packets processed by GRO.  On transmit these
packets are processed by GSO if emulation of checksum offloading
needs to be performed.  Unlike skb_checksum_help(), the GSO code
does not have any requirements about the packet structure.  Since
our code that copies and checksums packets to userspace is called
in conditions similar to GSO and does not have any assumptions
about layout, drop the BUG_ON assertion.

NIC-343

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agoflow: Make IPv6 userspace code match kernel.
Jesse Gross [Wed, 2 Mar 2011 23:12:48 +0000 (15:12 -0800)]
flow: Make IPv6 userspace code match kernel.

The flow extraction code for IPv6 has some deviations from both the
kernel version and other protocols in userspace.  These differences
make it difficult to compare the two for correctness.  This updates
the code to be more similar to the others in design and style.  There
is no functional change.

13 years agodatapath: Update IPv6 parsing code for kernel style.
Jesse Gross [Sat, 26 Feb 2011 00:46:19 +0000 (16:46 -0800)]
datapath: Update IPv6 parsing code for kernel style.

Fixes a number of minor elements in the IPv6 extraction and
parsing code to better conform to kernel style.  Examples include
using kernel types/functions, adding line breaks, and using
unlikely() macros.  There is no functional change.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agodatapath: Allow jumbograms through IPv6 parsing.
Jesse Gross [Wed, 2 Mar 2011 22:51:31 +0000 (14:51 -0800)]
datapath: Allow jumbograms through IPv6 parsing.

Currently we stop parsing packets that are IPv6 jumbograms.  While
it isn't possible to send such large packets to userspace, it's better
to drop them at that point rather than prematurely in the IPv6 code.
IPv6 does make some use of the payload length field but we can just as
easily use skb->len, which is what all other parsing uses.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agodatapath: Check packet len does not exceed attribute len.
Jesse Gross [Mon, 28 Feb 2011 22:32:33 +0000 (14:32 -0800)]
datapath: Check packet len does not exceed attribute len.

Netlink attributes have a maximum length of 64k.  It's theoretically
possible that a packet could exceed this length, so check for it before
we try to send the packet to userspace.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agodatapath: Record upcall errors as lost packets.
Jesse Gross [Mon, 28 Feb 2011 22:26:39 +0000 (14:26 -0800)]
datapath: Record upcall errors as lost packets.

It's possible to encounter a few different errors when preparing
to send a packet to userspace in queue_control_packet().  This
ensures that if we encounter one of these problems, the issue is
properly recorded as a lost packet.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agoflow: Avoid assertion failure on invalid IPv6 packet.
Jesse Gross [Wed, 2 Mar 2011 01:56:01 +0000 (17:56 -0800)]
flow: Avoid assertion failure on invalid IPv6 packet.

We compute the length of the IPv6 header by parsing all of the
extension headers that we know about.  However, the final result
is checked using ofpbuf_pull(), which checks the size with an
assertion.  Since the length of the final header is not checked
in any other way an invalid packet can trigger this assertion.

13 years agodatapath: Remove unneeded modify_vlan_tci() parameters.
Ben Pfaff [Wed, 2 Mar 2011 18:38:14 +0000 (10:38 -0800)]
datapath: Remove unneeded modify_vlan_tci() parameters.

These parameters were once needed but they are no longer used.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
13 years agodatapath: Avoid duplicate test in tnl_free_linked_skbs().
Ben Pfaff [Tue, 1 Mar 2011 23:33:40 +0000 (15:33 -0800)]
datapath: Avoid duplicate test in tnl_free_linked_skbs().

I don't see the value in testing 'skb' for nonnull twice.  This is only
called on error paths, so it seems really doubtful to me that there is any
value in the "unlikely", either.  (It could be put into the "while"
expression if it is valuable.)

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
13 years agodatapath: Simplify send_frags().
Ben Pfaff [Tue, 1 Mar 2011 23:31:32 +0000 (15:31 -0800)]
datapath: Simplify send_frags().

This should make no difference at runtime, but it is shorter and looks more
straightforward to me.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
13 years agostream-ssl: Only set SSL client session once.
Ben Pfaff [Fri, 28 Jan 2011 23:32:57 +0000 (15:32 -0800)]
stream-ssl: Only set SSL client session once.

Attempting to call SSL_set_session() on every trip through the SSL
connection state machine seems like it could cause the session to be
re-set to the cached one even after the server has told us which session
is actually to be used.

In testing, this change didn't make any difference, but it seems seems like
the right thing to do.

Bug #4448.

13 years agostream-ssl: Break interpretation of queued error into new function.
Ben Pfaff [Fri, 28 Jan 2011 22:39:23 +0000 (14:39 -0800)]
stream-ssl: Break interpretation of queued error into new function.

The following commit will introduce another user.

13 years agoovs-ofctl: Implement documented semantics of --flow-format for flow_mods.
Ben Pfaff [Tue, 22 Feb 2011 21:43:14 +0000 (13:43 -0800)]
ovs-ofctl: Implement documented semantics of --flow-format for flow_mods.

Also adds a test and moves some code around in tests/ to make sure that
OFPROTO_START and OFPROTO_STOP are available in tests/ovs-ofctl.at.

Reported-by: Michael Mao <mmao@nicira.com>
Bug #4566.

13 years agoofp-util: Make NXM required for 64-bit cookies in is_nxm_required().
Ben Pfaff [Tue, 22 Feb 2011 21:17:51 +0000 (13:17 -0800)]
ofp-util: Make NXM required for 64-bit cookies in is_nxm_required().

Only NXM supports 64-bit cookies, but this code didn't properly check
for that.  This commit fixes the problem and makes the code much more
explicit about what it is checking.

This will hide bug #4566, but the following commit actually fixes it.

13 years agoxenserver: Add support for disabling in-band management via XAPI.
Andrew Evans [Tue, 1 Mar 2011 02:26:04 +0000 (18:26 -0800)]
xenserver: Add support for disabling in-band management via XAPI.

Allow users or applications to enable or disable in-band management of
individual bridges by setting the 'vswitch-disable-in-band' key in the
'other_config' attribute of the corresponding network to 'true' or
'false'.

Bug #4749.

13 years agoAUTHORS: Add Valient Gough.
Jesse Gross [Fri, 25 Feb 2011 20:35:52 +0000 (12:35 -0800)]
AUTHORS: Add Valient Gough.

13 years agodatapath: call tnl_ops only after setting options
Valient Gough [Fri, 25 Feb 2011 05:45:18 +0000 (21:45 -0800)]
datapath: call tnl_ops only after setting options

Calling methods in tnl_ops should be done only after initializing the tunnel
configuration options.  This patch moves the hdr_len call after the output key
setup.

Signed-off-by: Valient Gough <vgough@pobox.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
13 years agodatapath: Destroy internal devices before freeing datapath.
Jesse Gross [Thu, 24 Feb 2011 22:07:29 +0000 (14:07 -0800)]
datapath: Destroy internal devices before freeing datapath.

When destroying vports we account for two types of synchronization
mechanisms: RTNL and RCU.  However, it is possible to call into
network device methods with just a device reference without either
of these.  These device methods can use the datapath data structures
but we don't wait for all of the references to go away before freeing
the datapath.  The actual wait happens in rtnl_unlock(), so by moving
up that call we can avoid the possibility of use after free with
internal devices.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agodatapath: Don't free vport until all references are gone.
Jesse Gross [Thu, 24 Feb 2011 22:16:24 +0000 (14:16 -0800)]
datapath: Don't free vport until all references are gone.

We currently call vport_free() for internal devices after the
device is unregistered.  This takes care of callers that use
either RTNL or RCU but not ones that have only a device reference.
In particular, if stats are requested while a datapath is being
unregistered we can try to use the vport data structures which
have already been freed.

Bug #4736

Reported-by: Brad Hall <brad@nicira.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agoofproto: Log warning if controller requests an invalid table.
Ben Pfaff [Fri, 25 Feb 2011 01:03:28 +0000 (17:03 -0800)]
ofproto: Log warning if controller requests an invalid table.

This might have saved us some time debugging.

13 years agoofp-print: Don't print priority for flow stats requests.
Ben Pfaff [Fri, 25 Feb 2011 00:58:40 +0000 (16:58 -0800)]
ofp-print: Don't print priority for flow stats requests.

A flow stats or aggregate stats request does not have a priority, but we
were printing one anyway.

Reported-by: Justin Pettit <jpettit@nicira.com>
13 years agoofproto: Change account_cb to use uint64_t.
Ethan Jackson [Fri, 25 Feb 2011 01:04:47 +0000 (17:04 -0800)]
ofproto: Change account_cb to use uint64_t.

This is more consistent with ofproto internals and its users.

13 years agoutil: Avoid uninitialized pointer complaints from Coverity.
Ben Pfaff [Thu, 24 Feb 2011 23:33:57 +0000 (15:33 -0800)]
util: Avoid uninitialized pointer complaints from Coverity.

13 years agoofproto: Guarantee uninstalled facets have no dp_packet_count.
Ethan Jackson [Thu, 24 Feb 2011 21:53:02 +0000 (13:53 -0800)]
ofproto: Guarantee uninstalled facets have no dp_packet_count.

facet_push_stats() implicitly assumes that uninstalled facets have
no dp_[packet|byte]_count.  This commit guarantees and enforces
this invariant.

Bug #4732.

13 years agoofproto: Reset facet's rs_used at rule changes.
Ethan Jackson [Thu, 24 Feb 2011 20:38:30 +0000 (12:38 -0800)]
ofproto: Reset facet's rs_used at rule changes.

When a facet changes rules it's 'used' timer is set to the new
rule's created time.  This is possibly before the time stored in
'rs_used' which could cause an assertion failure in
facet_push_stats().

Bug #4732.

13 years agodatapath: Memset executed packet's fields using memset.
Jesse Gross [Thu, 24 Feb 2011 04:04:31 +0000 (20:04 -0800)]
datapath: Memset executed packet's fields using memset.

Currently we explicitly zero out each of the fields in the OVS_CB for
executed packets.  However, it seems simpler and more robust to just
memset the whole thing to zero.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agoutil: Make out_of_memory() call abort() instead of exit(EXIT_FAILURE).
Ben Pfaff [Wed, 23 Feb 2011 23:43:34 +0000 (15:43 -0800)]
util: Make out_of_memory() call abort() instead of exit(EXIT_FAILURE).

exit(EXIT_FAILURE) will make a monitoring process (the one created by
--monitor) think that it should exit.  But the most likely reason for
out_of_memory() to be called is a bug: probably, the process is trying
to allocate more memory than there is available address space, e.g.
something like malloc(-1).  So it's better, in my opinion, to call abort()
instead, so that the monitor process restarts the daemon and we are more
likely to stay alive and, in addition, get a core dump and a useful bug
report.

I decided to implement a new general-purpose function for this purpose in
case we run into other similar situations in the future.

(I haven't actually run into this problem in practice.  This commit is
just speculation about what is better behavior.)

13 years agoovsdb: Explicitly ignore sscanf() return value in is_valid_version().
Ben Pfaff [Wed, 23 Feb 2011 21:13:44 +0000 (13:13 -0800)]
ovsdb: Explicitly ignore sscanf() return value in is_valid_version().

The return value isn't interesting here: it will always be 0.

Coverity #10698.

13 years agotest-ovsdb: Check uuid_from_string() return value.
Ben Pfaff [Wed, 23 Feb 2011 21:11:35 +0000 (13:11 -0800)]
test-ovsdb: Check uuid_from_string() return value.

Coverity #10699.

13 years agodhcp-client: Remove useless call to timeout() from send_reliably().
Ben Pfaff [Wed, 23 Feb 2011 21:09:33 +0000 (13:09 -0800)]
dhcp-client: Remove useless call to timeout() from send_reliably().

timeout() has no side effects so calling it without looking at the return
value is pointless.

Coverity #10700.

13 years agoovs-vsctl: Clarify that uuid_from_string() shouldn't fail in post_create().
Ben Pfaff [Wed, 23 Feb 2011 21:05:25 +0000 (13:05 -0800)]
ovs-vsctl: Clarify that uuid_from_string() shouldn't fail in post_create().

Coverity #10701.

13 years agoovs-kill: Remove.
Ben Pfaff [Wed, 23 Feb 2011 20:56:31 +0000 (12:56 -0800)]
ovs-kill: Remove.

This utility isn't used anywhere (except INSTALL.Linux), so remove it.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Coverity #10708.

13 years agoovs-vsctl: Simplify parse_column_key_value().
Ben Pfaff [Wed, 23 Feb 2011 20:20:38 +0000 (12:20 -0800)]
ovs-vsctl: Simplify parse_column_key_value().

Coverity pointed out some inconsistencies on tests for whether columnp and
keyp were nonnull.  These tests were, at best, confusing, but in fact every
caller always passed nonnull for both parameters, so this commit drops all
of those tests.

Coverity #10715, 10710.

13 years agoovs-brcompatd: Simplify code in brc_recv_update().
Ben Pfaff [Wed, 23 Feb 2011 18:27:04 +0000 (10:27 -0800)]
ovs-brcompatd: Simplify code in brc_recv_update().

This code was baffling and it confused Coverity, too.

Coverity #11070.

13 years agoovsdb-idl: JSONRPC_REPLY message always has nonnull 'id'.
Ben Pfaff [Wed, 23 Feb 2011 20:42:44 +0000 (12:42 -0800)]
ovsdb-idl: JSONRPC_REPLY message always has nonnull 'id'.

A JSONRPC_REPLY message always have a nonnull 'id' member, as ensured by
jsonrpc_msg_is_valid().  Checking for NULL here confused Coverity into
believing that the call to ovsdb_idl_txn_process_reply() just below could
cause a null pointer dereference, since ovsdb_idl_txn_process_reply() uses
the 'id' member without checking it for null.

Coverity #10713.

13 years agorconn: Clarify rconn_run_wait().
Ben Pfaff [Wed, 23 Feb 2011 20:35:32 +0000 (12:35 -0800)]
rconn: Clarify rconn_run_wait().

At first glance the vconn_wait() call looks risky because this function
checked whether rc->vconn is nonnull at the top.  In fact it's OK because
rc->state will be S_ACTIVE or S_IDLE only if rc->vconn is nonnull, but
there's no harm in putting that check inside the block that only runs if
rc->vconn is nonnull.

Coverity #10714.

13 years agoleak-checker: Avoid printing freed pointer.
Ben Pfaff [Wed, 23 Feb 2011 18:31:41 +0000 (10:31 -0800)]
leak-checker: Avoid printing freed pointer.

I think that this will work OK, and it should avoid complaints from static
checkers about using a freed pointer.

Coverity #11069.

13 years agostream: Make stream_report_content() tolerate negative size.
Ben Pfaff [Wed, 23 Feb 2011 19:29:37 +0000 (11:29 -0800)]
stream: Make stream_report_content() tolerate negative size.

A negative size probably means that a system call failed.  The caller could
set that to 0 but we might as well just tolerate it in
stream_report_content() by making the parameter type signed.

Coverity #10718.

13 years agoprocess: Avoid late failure if /dev/null cannot be opened.
Ben Pfaff [Wed, 23 Feb 2011 19:16:07 +0000 (11:16 -0800)]
process: Avoid late failure if /dev/null cannot be opened.

It is (very slightly) risky to open /dev/null every time that we need it,
because open can fail.  So this commit opens /dev/null in advance instead.

Coverity #10719.

13 years agobridge: Ethernet address is 6 bytes, not 4 or 8.
Ben Pfaff [Wed, 23 Feb 2011 19:21:52 +0000 (11:21 -0800)]
bridge: Ethernet address is 6 bytes, not 4 or 8.

'ea' here is a function parameter declared as an array, so "sizeof ea" is
sizeof(uint8_t *), which is either 4 or 8.

Coverity #10689, 10735.

13 years agoovsdb: Warn when result of ovsdb_txn_commit() is ignored and fix caller.
Justin Pettit [Wed, 23 Feb 2011 18:39:21 +0000 (10:39 -0800)]
ovsdb: Warn when result of ovsdb_txn_commit() is ignored and fix caller.

ovsdb_txn_commit() may return a ovsdb_error structure, which should be
freed by the caller.  The only remaining caller that discards the result
is in ovsdb_file_open__(), which this fixes.

Suggested-by: Ben Pfaff <blp@nicira.com>
13 years agoovsdb-idl: Remove deadcode related to "reply" in ovsdb_idl_run().
Justin Pettit [Wed, 23 Feb 2011 18:38:59 +0000 (10:38 -0800)]
ovsdb-idl: Remove deadcode related to "reply" in ovsdb_idl_run().

Coverity #10705

13 years agonx-match: Don't fall through case when handling dest mcast mac addresses.
Justin Pettit [Wed, 23 Feb 2011 03:07:05 +0000 (19:07 -0800)]
nx-match: Don't fall through case when handling dest mcast mac addresses.

When an NXM wildcard entry that includes a multicast address is parsed,
it would fall through to the next case statement, which would also set
an inappropriate source mac address match.

Coverity #10717

13 years agoovsdb-tool: Close "log" in do_show_log().
Justin Pettit [Wed, 23 Feb 2011 02:09:16 +0000 (18:09 -0800)]
ovsdb-tool: Close "log" in do_show_log().

Coverity #10726

13 years agotest-ovsdb: Don't leak return value from ovsdb_txn_commit().
Justin Pettit [Wed, 23 Feb 2011 02:02:20 +0000 (18:02 -0800)]
test-ovsdb: Don't leak return value from ovsdb_txn_commit().

Coverity #10727

13 years agoovs-vsctl: Free "columns" in cmd_find().
Justin Pettit [Wed, 23 Feb 2011 01:59:01 +0000 (17:59 -0800)]
ovs-vsctl: Free "columns" in cmd_find().

Coverity #10730

13 years agonetdev-linux: Zero-out "sin" in netdev_linux_arp_lookup().
Justin Pettit [Wed, 23 Feb 2011 01:50:19 +0000 (17:50 -0800)]
netdev-linux: Zero-out "sin" in netdev_linux_arp_lookup().

Coverity complains that we're copying the unitialized "sin_zero" member
from "sin" into "r".  I don't think this is an actual problem, but
there's no harm in zeroing out the structure, either.

Coverity #10916

13 years agotable: Free "error" in cell_to_text().
Justin Pettit [Tue, 22 Feb 2011 18:43:00 +0000 (10:43 -0800)]
table: Free "error" in cell_to_text().

Coverity #10722

13 years agomultipath: Validate multipath actions more thoroughly in multipath_parse().
Ben Pfaff [Wed, 23 Feb 2011 00:24:19 +0000 (16:24 -0800)]
multipath: Validate multipath actions more thoroughly in multipath_parse().

The stricter validation requires updates to the calls to test-multipath
to supply a valid n_links value.  test-multipath doesn't actually use
that value (it runs over different values in an internal "for" loop), so
this doesn't change any behavior.

Also adds a test to exercise each possible multipath_parse() error message.

Reported-by: Reid Price <reid@nicira.com>
Bug #4462.

13 years agoofp-parse: Don't segfault when an OpenFlow action's argument is missing.
Ben Pfaff [Tue, 22 Feb 2011 22:55:39 +0000 (14:55 -0800)]
ofp-parse: Don't segfault when an OpenFlow action's argument is missing.

Some actions checked that 'arg' was nonnull before attempting to parse it
but a lot of them didn't.  This commit avoids the segfault by substituting
an empty string when no argument is given.  It also updates a few of the
action implementations to correspond.

Reported-by: Reid Price <reid@nicira.com>
Bug #4462.
Coverity #10712.

13 years agonicira-ext: Fix comment.
Ben Pfaff [Wed, 23 Feb 2011 00:19:54 +0000 (16:19 -0800)]
nicira-ext: Fix comment.

Multipath actions only support registers as destinations, but this was
defined by reference in nicira-ext.h and the referenced text changed.

13 years agoovs-vsctl: --bootstrap only has an effect if the ca cert is missing.
Ben Pfaff [Wed, 23 Feb 2011 00:57:51 +0000 (16:57 -0800)]
ovs-vsctl: --bootstrap only has an effect if the ca cert is missing.

Someone (I can't remember who) asked me about this a while ago.  I hope
this clarifies.

13 years agoconfigure: Reject incompatible XenServer version.
Ben Pfaff [Tue, 22 Feb 2011 22:47:19 +0000 (14:47 -0800)]
configure: Reject incompatible XenServer version.

Suggested-by: Andrew Evans <aevans@nicira.com>
13 years agoutil: New function ovs_strzcpy().
Ben Pfaff [Tue, 22 Feb 2011 18:58:36 +0000 (10:58 -0800)]
util: New function ovs_strzcpy().

Static analyzers hate strncpy().  This new function shares its property of
initializing an entire buffer, without its nasty habit of failing to
null-terminate long strings.

Coverity #10697,10696,10695,10694,10693,10692,10691,10690.

13 years agoutil: Don't read over 'size - 1' bytes of source string in ovs_strlcpy().
Ben Pfaff [Tue, 22 Feb 2011 18:41:15 +0000 (10:41 -0800)]
util: Don't read over 'size - 1' bytes of source string in ovs_strlcpy().

The blind replacement of strncpy() by ovs_strlcpy() is risky because
strncpy() never reads more bytes from its source string than necessary to
write its destination string, but ovs_strlcpy() and the OpenBSD function
that inspired it both read the entire source string.  This avoids that
problem.

Given that change, we can use ovs_strlcpy() in a few more places, and
this commit does that too.

Coverity #10697,10696,10695,10694,10693,10692,10691,10690.

13 years agostring: Implement strnlen() if it is missing.
Ben Pfaff [Tue, 22 Feb 2011 18:36:13 +0000 (10:36 -0800)]
string: Implement strnlen() if it is missing.

13 years agovconn: Fix comparison that should be assignment in vconn_open_block().
Justin Pettit [Tue, 22 Feb 2011 01:21:20 +0000 (17:21 -0800)]
vconn: Fix comparison that should be assignment in vconn_open_block().

Coverity #10702

13 years agoovs-vsctl: Check "key" is valid before dereferencing in get_external_id().
Justin Pettit [Tue, 22 Feb 2011 01:19:18 +0000 (17:19 -0800)]
ovs-vsctl: Check "key" is valid before dereferencing in get_external_id().

Coverity #10716

13 years agoovsdb: Don't check "date" before assignment in ovsdb_file_txn_from_json().
Justin Pettit [Tue, 22 Feb 2011 01:02:35 +0000 (17:02 -0800)]
ovsdb: Don't check "date" before assignment in ovsdb_file_txn_from_json().

There's no indication that "date" is optional in the description of
ovsdb_file_txn_from_json(), and the one caller always passes it in, so
don't bother checking whether it exists.

Coverity #10732

13 years agosocket-util: Free strings in make_sockaddr_un() error handling.
Justin Pettit [Tue, 22 Feb 2011 00:38:39 +0000 (16:38 -0800)]
socket-util: Free strings in make_sockaddr_un() error handling.

Coverity #10721,10720

13 years agoovsdb: Free "error" string in ovsdb_execute_insert().
Justin Pettit [Tue, 22 Feb 2011 00:30:01 +0000 (16:30 -0800)]
ovsdb: Free "error" string in ovsdb_execute_insert().

Coverity #10723

13 years agodaemon: Fix leak of string in make_pidfile().
Justin Pettit [Tue, 22 Feb 2011 00:27:22 +0000 (16:27 -0800)]
daemon: Fix leak of string in make_pidfile().

Coverity #10724

13 years agoovsdb-data: Free string leaked in ovsdb_datum_from_string().
Justin Pettit [Tue, 22 Feb 2011 00:23:50 +0000 (16:23 -0800)]
ovsdb-data: Free string leaked in ovsdb_datum_from_string().

Coverity #10725

13 years agodiscovery: Free "buffer" in discovery_set_accept_controller_re().
Justin Pettit [Tue, 22 Feb 2011 00:06:28 +0000 (16:06 -0800)]
discovery: Free "buffer" in discovery_set_accept_controller_re().

Coverity #10728

13 years agoofproto: Free "controller_name" in ofconn_run().
Justin Pettit [Tue, 22 Feb 2011 00:04:55 +0000 (16:04 -0800)]
ofproto: Free "controller_name" in ofconn_run().

Coverity #10729

13 years agoovs-brcompatd: Fix memory leak in handle_fdb_query_cmd().
Justin Pettit [Mon, 21 Feb 2011 23:58:19 +0000 (15:58 -0800)]
ovs-brcompatd: Fix memory leak in handle_fdb_query_cmd().

Coverity #10731

13 years agoovsdb: Fix double free in ovsdb_file_open_log().
Justin Pettit [Mon, 21 Feb 2011 23:33:19 +0000 (15:33 -0800)]
ovsdb: Fix double free in ovsdb_file_open_log().

Coverity #11066

13 years agojsonrpc.py: Import "ovs.json".
Justin Pettit [Tue, 22 Feb 2011 07:56:07 +0000 (23:56 -0800)]
jsonrpc.py: Import "ovs.json".

Reported-by: Giuseppe de Candia <giuseppe.decandia@gmail.com>
13 years agoxenserver: Split kernel/userspace into separate RPMs for supplemental packs.
Andrew Evans [Thu, 17 Feb 2011 06:40:46 +0000 (22:40 -0800)]
xenserver: Split kernel/userspace into separate RPMs for supplemental packs.

XenServer has the concept of a "supplemental pack", which is just a collection
of RPMs rolled into an ISO image for installation as a unit, either during
XenServer installation or later as an update.

The script used to build supplemental packs requires that a single RPM not mix
kernel and userspace components. It also enforces a particular naming scheme
for kernel module RPMs. This commit modifies the OVS RPM spec file to create
separate RPMs in accordance with these conventions.

In an attempt to ensure that the userspace and kernel module packages are both
present and compatible, the kernel package provides a fictitious capability,
openvswitch_mod.ko.0, and the userspace package requires that capability. If a
future change breaks backward compatibility between userspace and kernel, we'll
increment the numeric suffix, preventing mismatched installs.

Reviewed by Ben Pfaff.

13 years agodatapath: Properly initialize ovs_skb_cb of packet from userspace.
Ben Pfaff [Sat, 19 Feb 2011 04:53:51 +0000 (20:53 -0800)]
datapath: Properly initialize ovs_skb_cb of packet from userspace.

The ovs_skb_cb in 'packet' in this function is initially a clone of the
corresponding area in 'skb', which came from the Netlink layer and thus
isn't necessarily all-zeros.  This commit initializes it properly before
passing it along to execute_actions().

The most common problem caused by failing to initialize the ovs_skb_cb
properly was that on Linux 2.6.26 and earlier, where Open vSwitch keeps
its own vlan_tci field inside ovs_skb_cb, the first packet of a flow would
get sent out tagged with a random VLAN (usually 0x0001 or 0xffff in our
testing).  This commit should fix that problem.

Another likely problem would be for turning on sFlow to randomly panic the
kernel.  That problem would not be kernel version dependent.  We haven't
been testing sFlow so we haven't noticed this problem.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Justin Pettit <jpettit@nicira.com>
Reported-by: Pankaj Thakkar <thakkar@nicira.com>
13 years agoxenserver: Revert only the XenServer scripts no longer replaced by OVS.
Andrew Evans [Thu, 17 Feb 2011 20:51:34 +0000 (12:51 -0800)]
xenserver: Revert only the XenServer scripts no longer replaced by OVS.

Commit d66880ee (xenserver: Clean up /usr/sbin/brctl dangling symlink.)
attempted to revert the original XenServer-shipped versions of scripts replaced
by OVS during an RPM upgrade, but the logic was incorrect. It assumed that
%postun of the package being replaced was run before the %post of the new
version replacing it. The reverse is actually true.

Make upgrade and erase cases both work correctly by simply checking whether any
of the files ever replaced in any OVS version are dangling symlinks, and if so,
attempt to copy the saved XenServer original back to its rightful place. In the
upgrade case, if the newly-installed version of OVS lacks any of the scripts in
the previous version, those will be reverted. In the erase case, none of the
OVS replacements will exist, so they will all be dangling symlinks and will all
be reverted.

Furthermore, replace any dangling symlink from /usr/sbin/xen-bugtool to the
now-nonexistent OVS replacement (caused by commit 92dbd5c9 (xenserver: Replace
customized xen-bugtool with plugin to collect qdisc info.)).

Lastly, prevent accidental reversion of files replaced by OVS during rpm -U
(also caused by commit 92dbd5c9).

Bug #4696.

13 years agoofproto: Resubmit Statistics.
Ethan Jackson [Fri, 11 Feb 2011 00:35:48 +0000 (16:35 -0800)]
ofproto: Resubmit Statistics.

This patch causes statistics to be updated for rules which are
resubmitted into. Once per second statistics are queried from the
datapath and pushed along the resubmit graph (calculated on demand
from the action list).  This approach is simple, easy to understand,
and in most cases accurate.  However, when the resubmit graph
changes, it is possible that some statistics will be accounted to
the wrong rule for a short period of time.

Bug #3730.

13 years agoofproto: Batch statistics updates.
Ethan Jackson [Wed, 9 Feb 2011 21:18:51 +0000 (13:18 -0800)]
ofproto: Batch statistics updates.

Facet statistics are updated once per second during
ofproto_expire() instead of upon request.  This will greatly
simplify implementation of future patches. This commit also changes
each facet's packet and byte counters to include the statistics
stored in the datapath.

13 years agoofproto: Display idle time in bridge/dump-flows.
Ethan Jackson [Sat, 12 Feb 2011 00:03:26 +0000 (16:03 -0800)]
ofproto: Display idle time in bridge/dump-flows.

13 years agoofproto: flow_stats_ds() print flows properly.
Ethan Jackson [Mon, 14 Feb 2011 20:10:52 +0000 (12:10 -0800)]
ofproto: flow_stats_ds() print flows properly.