[ovs-dev] [PATCHv2 1/2] vlog: abstract out interface to syslog daemon

Ansis Atteka aatteka at nicira.com
Thu Jun 25 19:54:22 UTC 2015


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 at nicira.com>
---
 NEWS                       |   2 +
 include/openvswitch/vlog.h |   8 ++++
 lib/automake.mk            |   5 +++
 lib/syslog-direct.c        | 110 +++++++++++++++++++++++++++++++++++++++++++++
 lib/syslog-direct.h        |  22 +++++++++
 lib/syslog-libc.c          |  76 +++++++++++++++++++++++++++++++
 lib/syslog-libc.h          |  22 +++++++++
 lib/syslog-provider.h      |  50 +++++++++++++++++++++
 lib/vlog.c                 |  41 ++++++++++++-----
 lib/vlog.man               |  25 +++++++++++
 10 files changed, 350 insertions(+), 11 deletions(-)
 create mode 100644 lib/syslog-direct.c
 create mode 100644 lib/syslog-direct.h
 create mode 100644 lib/syslog-libc.c
 create mode 100644 lib/syslog-libc.h
 create mode 100644 lib/syslog-provider.h

diff --git a/NEWS b/NEWS
index 395444b..2c26a04 100644
--- a/NEWS
+++ b/NEWS
@@ -116,6 +116,8 @@ v2.4.0 - xx xxx xxxx
    - Support for STT tunneling.
    - ovs-sim: New developer tool for simulating multiple OVS instances.
      See ovs-sim(1) for more information.
+   - Support to configure method (--syslog-method argument) that determines
+     how daemons will talk with syslog.
 
 
 v2.3.0 - 14 Aug 2014
diff --git a/include/openvswitch/vlog.h b/include/openvswitch/vlog.h
index 680fba4..f2fedae 100644
--- a/include/openvswitch/vlog.h
+++ b/include/openvswitch/vlog.h
@@ -143,6 +143,9 @@ void vlog_set_pattern(enum vlog_destination, const char *pattern);
 int vlog_set_log_file(const char *file_name);
 int vlog_reopen_log_file(void);
 
+/* Configure method how vlog should send messages to syslog server. */
+void vlog_set_syslog_method(const char *method);
+
 /* Configure syslog target. */
 void vlog_set_syslog_target(const char *target);
 
@@ -229,11 +232,13 @@ void vlog_rate_limit(const struct vlog_module *, enum vlog_level,
 /* Command line processing. */
 #define VLOG_OPTION_ENUMS                       \
         OPT_LOG_FILE,                           \
+        OPT_SYSLOG_IMPL,                        \
         OPT_SYSLOG_TARGET
 
 #define VLOG_LONG_OPTIONS                                               \
         {"verbose",       optional_argument, NULL, 'v'},                \
         {"log-file",      optional_argument, NULL, OPT_LOG_FILE},       \
+        {"syslog-method", optional_argument, NULL, OPT_SYSLOG_IMPL},    \
         {"syslog-target", required_argument, NULL, OPT_SYSLOG_TARGET}
 
 #define VLOG_OPTION_HANDLERS                    \
@@ -243,6 +248,9 @@ void vlog_rate_limit(const struct vlog_module *, enum vlog_level,
         case OPT_LOG_FILE:                      \
             vlog_set_log_file(optarg);          \
             break;                              \
+        case OPT_SYSLOG_IMPL:                   \
+            vlog_set_syslog_method(optarg);     \
+            break;                              \
         case OPT_SYSLOG_TARGET:                 \
             vlog_set_syslog_target(optarg);     \
             break;
diff --git a/lib/automake.mk b/lib/automake.mk
index 706ff4b..761f3a0 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -237,6 +237,11 @@ lib_libopenvswitch_la_SOURCES = \
 	lib/string.c \
 	lib/svec.c \
 	lib/svec.h \
+	lib/syslog-direct.c \
+	lib/syslog-direct.h \
+	lib/syslog-libc.c \
+	lib/syslog-libc.h \
+	lib/syslog-provider.h \
 	lib/table.c \
 	lib/table.h \
 	lib/tag.c \
diff --git a/lib/syslog-direct.c b/lib/syslog-direct.c
new file mode 100644
index 0000000..d62c3b5
--- /dev/null
+++ b/lib/syslog-direct.c
@@ -0,0 +1,110 @@
+/*
+ * Copyright (c) 2015 Nicira, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include "syslog-direct.h"
+
+#include <config.h>
+
+#include <string.h>
+#include <unistd.h>
+
+#include "compiler.h"
+#include "dynamic-string.h"
+#include "socket-util.h"
+#include "syslog-provider.h"
+#include "util.h"
+
+#define FACILITY_MASK 0x03f8
+
+static void syslog_direct_open(struct syslogger *this, int facility);
+static void syslog_direct_log(struct syslogger *this, int pri,
+                              const char *msg);
+
+struct syslog_class syslog_direct_class = {
+    syslog_direct_open,
+    syslog_direct_log,
+};
+
+struct syslog_direct {
+    struct syslogger parent;
+    int fd;  /* Negative number in error case.  Otherwise, socket. */
+    int facility;
+};
+
+
+/* This function creates object that directly interacts with syslog over
+ * UDP or Unix domain socket specified in 'method'. */
+struct syslogger *
+syslog_direct_create(const char *method)
+{
+    struct syslog_direct *this = xmalloc(sizeof *this);
+
+    this->parent.class = &syslog_direct_class;
+    this->parent.prefix = "<%B>";
+
+    /* socket is created from here (opposed to syslog_direct_open())
+     * so that deadlocks would be avoided.  The problem is that these
+     * functions that create socket might call VLOG() */
+    if (!strncmp(method, "udp:", 4)) {
+        inet_open_active(SOCK_DGRAM, &method[4], 514, NULL, &this->fd, 0);
+    } else if (!strncmp(method, "unix:", 5)) {
+        this->fd = make_unix_socket(SOCK_DGRAM, true, NULL, &method[5]);
+    } else {
+        this->fd = -1;
+    }
+
+    return (struct syslogger *)this;
+}
+
+static void
+syslog_direct_open(struct syslogger *this, int facility)
+{
+    struct syslog_direct *this_ = (struct syslog_direct*) this;
+
+    this_->facility = facility;
+}
+
+static void
+syslog_direct_log(struct syslogger *this, int pri, const char *msg)
+{
+    static size_t max_len = SIZE_MAX; /* max message size we have discovered
+                                       * to be able to send() without failing
+                                       * with EMSGSIZE. */
+
+    struct syslog_direct *this_ = (struct syslog_direct*) this;
+    struct ds ds = DS_EMPTY_INITIALIZER;
+    const char *wire_msg;
+    size_t send_len;
+
+    if (this_->fd < 0) {
+        /* Failed to open socket for logging. */
+        return;
+    }
+
+    if (!(pri & FACILITY_MASK)) {
+        pri |= this_->facility;
+    }
+    ds_put_format(&ds, "<%u>%s", pri, msg);
+    wire_msg = ds_cstr(&ds);
+    send_len = MIN(strlen(wire_msg), max_len);;
+    while (send(this_->fd, wire_msg, send_len, 0) < 0 && errno == EMSGSIZE) {
+        /* If message was too large for send() function then try to discover
+         * max_len supported for this particular socket and retry sending a
+         * truncated version of the same message. */
+        send_len -= send_len / 20;
+        max_len = send_len;
+    }
+    ds_destroy(&ds);
+}
diff --git a/lib/syslog-direct.h b/lib/syslog-direct.h
new file mode 100644
index 0000000..8e974da
--- /dev/null
+++ b/lib/syslog-direct.h
@@ -0,0 +1,22 @@
+/*
+ * Copyright (c) 2015 Nicira, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef SYSLOG_DIRECT_H
+#define SYSLOG_DIRECT_H 1
+
+struct syslogger *syslog_direct_create(const char *method);
+
+#endif /* syslog-direct.h */
diff --git a/lib/syslog-libc.c b/lib/syslog-libc.c
new file mode 100644
index 0000000..b755eb9
--- /dev/null
+++ b/lib/syslog-libc.c
@@ -0,0 +1,76 @@
+/*
+ * Copyright (c) 2015 Nicira, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include "syslog-libc.h"
+
+#include <config.h>
+
+#include <string.h>
+#include <syslog.h>
+#include <unistd.h>
+
+#include "compiler.h"
+#include "dynamic-string.h"
+#include "socket-util.h"
+#include "syslog-provider.h"
+#include "util.h"
+
+
+static void syslog_libc_open(struct syslogger *this, int facility);
+static void syslog_libc_log(struct syslogger *this, int pri, const char *msg);
+
+struct syslog_class syslog_libc_class = {
+    syslog_libc_open,
+    syslog_libc_log,
+};
+
+struct syslog_libc {
+    struct syslogger parent;
+};
+
+
+/* This function  creates object that delegate all logging to libc's
+ * syslog implementation. */
+struct syslogger *
+syslog_libc_create(void)
+{
+    struct syslog_libc *this = xmalloc(sizeof *this);
+
+    this->parent.class = &syslog_libc_class;
+    this->parent.prefix = "<%B> %D{%h %e %T} %E %A:";
+
+    return (struct syslogger *)this;
+}
+
+static void
+syslog_libc_open(struct syslogger *this OVS_UNUSED, int facility)
+{
+    static char *ident;
+
+    /* openlog() is allowed to keep the pointer passed in, without making a
+     * copy.  The daemonize code sometimes frees and replaces
+     * 'program_name', so make a private copy just for openlog().  (We keep
+     * a pointer to the private copy to suppress memory leak warnings in
+     * case openlog() does make its own copy.) */
+    ident = program_name ? xstrdup(program_name) : NULL;
+
+    openlog(ident, LOG_NDELAY, facility);
+}
+
+static void
+syslog_libc_log(struct syslogger *this OVS_UNUSED, int pri, const char *msg)
+{
+    syslog(pri, "%s", msg);
+}
diff --git a/lib/syslog-libc.h b/lib/syslog-libc.h
new file mode 100644
index 0000000..690bbc9
--- /dev/null
+++ b/lib/syslog-libc.h
@@ -0,0 +1,22 @@
+/*
+ * Copyright (c) 2015 Nicira, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef SYSLOG_LIBC_H
+#define SYSLOG_LIBC_H 1
+
+struct syslogger *syslog_libc_create(void);
+
+#endif /* syslog-libc.h */
diff --git a/lib/syslog-provider.h b/lib/syslog-provider.h
new file mode 100644
index 0000000..d629ffc
--- /dev/null
+++ b/lib/syslog-provider.h
@@ -0,0 +1,50 @@
+/*
+ * Copyright (c) 2015 Nicira, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef SYSLOG_PROVIDER_H
+#define SYSLOG_PROVIDER_H 1
+
+
+/* Open vSwitch interface to syslog daemon's interface.
+ *
+ * 'syslogger' is the base class that provides abstraction. */
+struct syslogger {
+    const struct syslog_class *class;  /* Virtual functions for concrete
+                                        * syslogger implementations. */
+    const char *prefix;                /* Prefix that is enforced by concrete
+                                        * syslogger implementation.  Used
+                                        * in vlog/list-pattern function. */
+};
+
+/* Each concrete syslogger implementation must define it's own table with
+ * following functions.  These functions must never call any other VLOG_
+ * function to prevent deadlocks. */
+struct syslog_class {
+    /* openlog() function should be called before syslog() function.  It
+     * should initialize all system resources needed to perform logging. */
+    void (*openlog)(struct syslogger *this, int facility);
+
+    /* syslog() function sends message 'msg' to syslog daemon. */
+    void (*syslog)(struct syslogger *this, int pri, const char *msg);
+};
+
+static inline const char *
+syslog_get_prefix(struct syslogger *this)
+{
+    return this->prefix;
+}
+
+#endif /* syslog-provider.h */
diff --git a/lib/vlog.c b/lib/vlog.c
index da7a307..359c888 100644
--- a/lib/vlog.c
+++ b/lib/vlog.c
@@ -37,6 +37,9 @@
 #include "sat-math.h"
 #include "socket-util.h"
 #include "svec.h"
+#include "syslog-direct.h"
+#include "syslog-libc.h"
+#include "syslog-provider.h"
 #include "timeval.h"
 #include "unixctl.h"
 #include "util.h"
@@ -106,6 +109,7 @@ static char *log_file_name OVS_GUARDED_BY(log_file_mutex);
 static int log_fd OVS_GUARDED_BY(log_file_mutex) = -1;
 static struct async_append *log_writer OVS_GUARDED_BY(log_file_mutex);
 static bool log_async OVS_GUARDED_BY(log_file_mutex);
+static struct syslogger *syslogger = NULL;
 
 /* Syslog export configuration. */
 static int syslog_fd OVS_GUARDED_BY(pattern_rwlock) = -1;
@@ -534,6 +538,24 @@ vlog_set_verbosity(const char *arg)
     }
 }
 
+void
+vlog_set_syslog_method(const char *method)
+{
+    if (syslogger) {
+        /* Set syslogger only, if one is not already set.  This effectively
+         * means that only the first --syslog-method argument is honored. */
+        return;
+    }
+
+    if (!strcmp(method, "libc")) {
+        syslogger = (struct syslogger *) syslog_libc_create();
+    } else if (!strncmp(method, "udp:", 4) || !strncmp(method, "unix:", 5)) {
+        syslogger = (struct syslogger *) syslog_direct_create(method);
+    } else {
+        ovs_fatal(0, "unsupported syslog method '%s'", method);
+    }
+}
+
 /* Set the vlog udp syslog target. */
 void
 vlog_set_syslog_target(const char *target)
@@ -671,23 +693,17 @@ vlog_init(void)
     static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
 
     if (ovsthread_once_start(&once)) {
-        static char *program_name_copy;
         long long int now;
         int facility;
 
         /* Do initialization work that needs to be done before any logging
          * occurs.  We want to keep this really minimal because any attempt to
          * log anything before calling ovsthread_once_done() will deadlock. */
-
-        /* openlog() is allowed to keep the pointer passed in, without making a
-         * copy.  The daemonize code sometimes frees and replaces
-         * 'program_name', so make a private copy just for openlog().  (We keep
-         * a pointer to the private copy to suppress memory leak warnings in
-         * case openlog() does make its own copy.) */
-        program_name_copy = program_name ? xstrdup(program_name) : NULL;
         atomic_read_explicit(&log_facility, &facility, memory_order_relaxed);
-        openlog(program_name_copy, LOG_NDELAY,
-                facility ? facility : LOG_DAEMON);
+        if (!syslogger) {
+            syslogger = syslog_libc_create();
+        }
+        syslogger->class->openlog(syslogger, facility ? facility : LOG_DAEMON);
         ovsthread_once_done(&once);
 
         /* Now do anything that we want to happen only once but doesn't have to
@@ -971,7 +987,7 @@ vlog_valist(const struct vlog_module *module, enum vlog_level level,
                  line = strtok_r(NULL, "\n", &save_ptr)) {
                 atomic_read_explicit(&log_facility, &facility,
                                      memory_order_relaxed);
-                syslog(syslog_level|facility, "%s", line);
+                syslogger->class->syslog(syslogger, syslog_level|facility, line);
             }
 
             if (syslog_fd >= 0) {
@@ -1154,6 +1170,9 @@ Logging options:\n\
   -v, --verbose            set maximum verbosity level\n\
   --log-file[=FILE]        enable logging to specified FILE\n\
                            (default: %s/%s.log)\n\
+  --syslog-method=(libc|unix:file|udp:ip:port)\n\
+                           specify method how messages to syslog daemon\n\
+                           should be sent\n\
   --syslog-target=HOST:PORT  also send syslog msgs to HOST:PORT via UDP\n",
            ovs_logdir(), program_name);
 }
diff --git a/lib/vlog.man b/lib/vlog.man
index 5ee34f8..b08b6b8 100644
--- a/lib/vlog.man
+++ b/lib/vlog.man
@@ -75,3 +75,28 @@ used if \fIfile\fR is omitted is \fB at LOGDIR@/\*(PN.log\fR.
 Send syslog messages to UDP \fIport\fR on \fIhost\fR, in addition to
 the system syslog.  The \fIhost\fR must be a numerical IP address, not
 a hostname.
+.
+.IP "\fB\-\-syslog\-method=\fImethod\fR"
+Specify \fImethod\fR how syslog messages should be sent to syslog daemon.
+Following forms are supported:
+.IP \(bu
+\fBlibc\fR, use libc \fBsyslog()\fR function.  This is the default behavior.
+Downside of using this options is that libc adds fixed prefix to every
+message before it is actually sent to the syslog daemon over \fB/dev/log\fR
+UNIX domain socket.
+.IP \(bu
+\fBunix:\fIfile\fR\fR, use UNIX domain socket directly.  It is possible to
+specify arbitrary message format with this option.  However,
+\fBrsyslogd 8.9\fR and older versions use hard coded parser function anyway
+that limits UNIX domain socket use.  If you want to use arbitrary message
+format with older \fBrsyslogd\fR versions, then use UDP socket to localhost
+IP address instead.
+.IP \(bu
+\fBudp:\fIip\fR:\fIport\fR\fR, use UDP socket.  With this method it is
+possible to use arbitrary message format also with older \fBrsyslogd\fR.
+When sending syslog messages over UDP socket extra precaution needs to
+be taken into account, for example, syslog daemon needs to be configured
+to listen on the specified UDP port, accidental iptables rules could be
+interfering with local syslog traffic and there are some security
+considerations that apply to UDP sockets, but do not apply to UNIX domain
+sockets.
-- 
2.1.4




More information about the dev mailing list