Calling VLOG_FATAL() while holding the 'log_file_mutex" may lead to
deadlock since VLOG_FATAL() implementation tries to acquire the
same lock. Fix this by building the error message first, then
call VLOG_FATAL() after the 'log_file_mutex' has been released.
This bug is not likely show up in practice since chown() usually
won't fail. It is still better to have a correct implementation.
Reported-by: Daniele Di Proietto <ddiproietto@vmware.com>
Signed-off-by: Andy Zhou <azhou@ovn.org>
Acked-by: Daniele Di Proietto <ddiproietto@vmware.com>
vlog log file can be created when parsing --log-file option, before
switching user, in case the --user option is also specified. While this
does not directly cause errors for the running daemons, it can
leave the log files on the disk as created under the "root" user.
This patch fix the log file ownership to the user specified with --user.
Signed-off-by: Andy Zhou <azhou@nicira.com>
Acked-by: Ansis Atteka <aatteka@nicira.com>
Commit fe089c0d1e ("vlog: abstract out interface to syslog daemon")
introduced --syslog-method flag that supersedes --syslog-target flag by:
1. making logging format configurable
2. letting daemon to also talk over UNIX domain socket (this is handy
when local rsyslog daemon is running in different network namespace
on the same host)
Signed-off-by: Ansis Atteka <aatteka@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
This patch allows to query logging format at the runtime for each destination
with "vlog/list-pattern" command.
Signed-off-by: Ansis Atteka <aatteka@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
This patch helps to address two issues that are present on Ubuntu
15.04 (and most likely other Linux distributions) where rsyslog daemon
is configured to relay log messages from OVS to a remote log collector
and syslog format being used is something other than the one defined in
RFC 3164. These two issues are:
1. libc syslog() function always adds RFC 3164 prefix to syslog
messages before sending them over /dev/log Unix domain socket.
This does not allow us to use libc syslog() function to log in
RFC 5424 format; and
2. rsyslogd daemon that comes with Ubuntu 15.04 is too old and
uses hardcoded syslog message parser when it received messages
over /dev/log UNIX domain socket.
Solution to those two issues would be to use the newly introduced
--syslog-method=udp:127.0.0.1:514 command line argument when starting
OVS.
Signed-off-by: Ansis Atteka <aatteka@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
When Open vSwitch is run in hundreds of hypervisors, it is
useful to collect log messages through log collectors. To
collect log messages like this, it is useful to log them
in a particular RFC5424 facility in the local system. The
log collectors can then be used to collect logs anytime
desired.
This commit provides a sysadmin the ability to specify the
facility through which the log messages are logged.
Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
In OVS, we currently use the term 'facility' to mean the place
where we log (syslog, console or file). In Linux's syslog() and
rfc5424, the term 'facility' is used to specify what type of program
is logging the message (e.g: LOG_DAEMON). This causes confusion
while reading vlog's code. This commit changes the term 'facility'
to 'destination'.
Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
A new function vlog_insert_module() is introduced to avoid using
list_insert() from the vlog.h header.
Signed-off-by: Thomas Graf <tgraf@noironetworks.com>
Acked-by: Ben Pfaff <blp@nicira.com>
Expose the struct ovs_list definition in <openvswitch/list.h>. Keep the
list access API private for now.
Signed-off-by: Thomas Graf <tgraf@noironetworks.com>
Acked-by: Ben Pfaff <blp@nicira.com>
struct list is a common name and can't be used in public headers.
Signed-off-by: Thomas Graf <tgraf@noironetworks.com>
Acked-by: Ben Pfaff <blp@nicira.com>
The following macros are renamed to avoid conflicts with other headers:
* WARN_UNUSED_RESULT to OVS_WARN_UNUSED_RESULT
* PRINTF_FORMAT to OVS_PRINTF_FORMAT
* NO_RETURN to OVS_NO_RETURN
Signed-off-by: Thomas Graf <tgraf@noironetworks.com>
Acked-by: Ben Pfaff <blp@nicira.com>
Anything inside vlog_init__() that tried to log a message was going to
deadlock, since it would hit pthread_once() recursively and wait for the
previous call to complete. Unfortunately, there was a VLOG_ERR call inside
vlog_init__(), only called in the corner case where the system's clock was
wrong.
This fixes the problem by rearranging code so that the logging isn't
inside the "once-only" region.
Found by inspection.
Signed-off-by: Ben Pfaff <blp@nicira.com>
This allows to get rid of some special segment handling to allow
distributed registering of vlog modules.
Instead use a global list and vlog module constructor functions to
build up the list. That means vlog modules reside within the
compilation unit they are defined in and can be iterated upon
by using the global list vlog_modules.
Signed-off-by: Helmut Schaa <helmut.schaa@googlemail.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
sparse warns if a non-static variable with external linkage has an
initializer at first declaration. This commit suppresses the
warnings issued when adding custom section is not supported by
compiler.
Signed-off-by: Alex Wang <alexw@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
We've seen a number of deadlocks in the tree since thread safety was
introduced. So far, all of these are self-deadlocks, that is, a single
thread acquiring a lock and then attempting to re-acquire the same lock
recursively. When this has happened, the process simply hung, and it was
somewhat difficult to find the cause.
POSIX "error-checking" mutexes check for this specific problem (and
others). This commit switches from other types of mutexes to
error-checking mutexes everywhere that we can, that is, everywhere that
we're not using recursive mutexes. This ought to help find problems more
quickly in the future.
There might be performance advantages to other kinds of mutexes in some
cases. However, the existing mutex type choices were just guesses, so I'd
rather go for easy detection of errors until we know that other mutex
types actually perform better in specific cases. Also, I did a quick
microbenchmark of glibc mutex types on my host and found that the
error checking mutexes weren't any slower than the other types, at least
when the mutex is uncontended.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
DEFINE_PER_THREAD_DATA always declared its data item as "static", meaning
that it was only directly visible within a single translation unit.
This commit adds additional forms of per-thread data that allow the data
to be accessible from multiple translation units.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Until now, the async append interface has required async_append_enable()
to be called while the process was still single-threaded, with the
rationale being that async_append_enable() could race with
async_append_write() on some existing async_append object. This was a
difficult problem when the async append interface was introduced, because
at the time Open vSwitch did not have any infrastructure for inter-thread
synchronization.
Now it is easy to solve, by introducing synchronization into the
async append module. However, that's more or less wasted, because the
client is already required to serialize access to async append objects.
Moreover, vlog, the only existing client, needs to serialize access for
other reasons, so it wouldn't even be possible to just drop the client's
synchronization.
This commit therefore takes another approach. It drops the
async_append_enable() interface entirely. Now any existing async_append
object is always enabled. The responsibility for "enabling", then, now
rests in whether the client creates and uses an async_append object, and
so vlog now takes care of that by itself. Also, since vlog now has to
deal with sometimes having an async_append and sometimes not having one,
we might as well allow creating an async_append to fail, thereby slightly
simplifying the "no async I/O" implementation from "write synchronously"
to "always fail creating an async_append".
Reported-by: Shih-Hao Li <shihli@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
This commit adds annotations for thread safety check. And the
check can be conducted by using -Wthread-safety flag in clang.
Co-authored-by: Alex Wang <alexw@nicira.com>
Signed-off-by: Alex Wang <alexw@nicira.com>
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
The worker process implementation isn't thread-safe and, once OVS
itself is threaded, it doesn't make much sense to have a worker
process anyway.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ed Maste <emaste@freebsd.org>
strftime() returns 0 and leaves the contents of the output buffer
unspecified if the output buffer is not big enough. Thus, one should
check strftime()'s return value. Until now, OVS has had a few invocations
of strftime() that did not check the return value. This commit fixes
those. I believe that the buffers were always large enough in each case,
but it's better to be safe.
Reported-by: Andy Zhou <azhou@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Two of the users of vlog_set_levels_from_string() in the tests could have
silently failed, if their arguments were invalid. This avoids that problem
(and a memory leak).
Found by Coverity.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
In general, with a few specific exceptions, ovs_assert is now preferred
over assert, so this commit adds a check for that in the top-level
Makefile.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
This is a straight search-and-replace, except that I also removed #include
<assert.h> from each file where there were no assert calls left.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
The following call stack was possible:
vlog
-> worker_request()
-> poll_block()
-> vlog
-> worker_request()
which caused problems because worker_request() is not reentrant. In a
little more detail, the second worker_request() shoves its RPC protocol
data into the middle of the first. This means that, first, you get
some binary crud in the log (the header for the second RPC). And,
second, text from the first RPC log message gets treated by the worker
as the subsequent RPC's header. That, in turn, typically causes the
worker to try to xmalloc() a huge number of bytes (0x20000000 or more,
since "space" has ASCII value 0x20), which causes the worker to die
with "virtual memory exhausted". The main process then dies because
the worker's death closes the socket it uses to communicate with it
("connection reset").
Bug #14616.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
A few times while troubleshooting it would have been useful to get
complete logs, rather than post-rate-limiting snapshots of them. These
ovs-appctl commands make that possible.
Signed-off-by: Ben Pfaff <blp@nicira.com>
A couple of calls to write() would generate warnings when the
"-Wunused-result" compiler option is enabled. This change ignores the
return value, since we can't do anything about it in logging code.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Casts are sometimes necessary. One common reason that they are necessary
is for discarding a "const" qualifier. However, this can impede
maintenance: if the type of the expression being cast changes, then the
presence of the cast can hide a necessary change in the code that does the
cast. Using CONST_CAST, instead of a bare cast, makes these changes
visible.
Inspired by my own work elsewhere:
http://git.savannah.gnu.org/cgit/pspp.git/tree/src/libpspp/cast.h#n80
Signed-off-by: Ben Pfaff <blp@nicira.com>
Writes to regular files under Unix-like kernels, including Linux, typically
block until the write is complete, regardless of O_NONBLOCK. When the I/O
subsystem is busy, this can cause indefinite delays. We have actually
observed "write" calls sleep for 5 seconds or more for this reason.
Delegating to a subprocess through the worker mechanism should solve the
problem.
Signed-off-by: Ben Pfaff <blp@nicira.com>
This will be easier to deal with in an upcoming commit that will use a
subprocess to do asynchronous log I/O.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Whereas VLOG_FATAL() eventually calls exit(1), VLOG_ABORT()
eventually calls abort(). The key difference is that abort()
will cause a "monitor" process to restart, where exit(1) will
cause it to exit along with the monitored process.
Signed-off-by: Ben Pfaff <blp@nicira.com>
This will be more useful later when we introduces "worker" subprocesses.
I don't have any current plans to introduce threading, but I can't
think of a disadvantage to wording this in a general manner.
Signed-off-by: Ben Pfaff <blp@nicira.com>
This commit converts two rate-limiters in the tree to use the library.
I intend to use the library elsewhere in the future.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Replaced all instances of Nicira Networks(, Inc) to Nicira, Inc.
Feature #10593
Signed-off-by: Raju Subramanian <rsubramanian@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
The unixctl library had used the vde2 management protocol since the
early days of Open vSwitch. As Open vSwitch has matured, several
Python daemons have been added to the code base which would benefit
from a unixctl implementations. Instead of implementing the old
unixctl protocol in Python, this patch changes unixctl to use JSON
RPC for which we already have an implementation in both Python and
C. Future patches will need to implement a unixctl library in
Python on top of JSON RPC.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
