On Mon, 2005-07-25 at 22:05 +0200, e09f6a7593f8ae3994ea57e1117f67ec wrote:
Hi, I have the same problem as someone 2 years ago (https://lists.balabit.hu/pipermail/syslog-ng/2003-September/005414.html) with UDP messages.
Problem:
My OpenWRT based wifi router messages are not recognized and written to file via syslog-ng-1.9.5. The configuration file constsits of single udp() source and destination to single file.
Symptoms:
I am running `./syslog-ng -F -v -d -e` and after receiving an UDP log message I get a log_reader_fd_prepare(); messages in console window, so that means that the packet has arrived. (also checked with tcpdump for that). Despite packet arrival nothing is written to log file. The udp message does not end with NULL character as expected in logreader.c/log_reader_iterate_buf() function. I scanned the RFC for that but there is not defined that the message must terminate with a NULL character.
Workaround: (needs testing for side-efects)
Replace in afsocket.c:425 line with self->reader = log_reader_new(... (self->flags & AFSOCKET_LOCAL) ? LR_LOCAL|LR_PKTTERM : LR_PKTTERM,...); (Insert fixed LR_PKTTERM flag)
Thanks for the detailed problem report. I have added a different solution though, trying to merge the diverging code paths for SOCK_DGRAM and SOCK_STREAM. I've committed a fix to my tla archive, the snapshot for tomorrow should contain the fix, or you can apply the patch below to your tree directly. Can you please test if this one works (I've compile tested and slightly runtime tested and it seems to work fine). --- orig/src/afsocket.c +++ mod/src/afsocket.c @@ -191,7 +191,7 @@ afsocket_sc_init(LogPipe *s, GlobalConfi { AFSocketSourceConnection *self = (AFSocketSourceConnection *) s; - self->reader = log_reader_new(fd_read_new(self->sock, 0), + self->reader = log_reader_new(fd_read_new(self->sock, (self->owner->flags & AFSOCKET_DGRAM) ? FR_RECV : 0), (self->owner->flags & AFSOCKET_LOCAL) ? LR_LOCAL : 0 | (self->owner->flags & AFSOCKET_DGRAM) ? LR_PKTTERM : 0 | (self->owner->flags & AFSOCKET_PROTO_RFC3164) ? LR_STRICT : 0, @@ -218,7 +218,13 @@ afsocket_sc_queue(LogPipe *s, LogMessage AFSocketSourceConnection *self = (AFSocketSourceConnection *) s; if (!msg->saddr) - msg->saddr = g_sockaddr_ref(self->peer_addr); + { + if (self->peer_addr) + msg->saddr = g_sockaddr_ref(self->peer_addr); + else + msg_notice("Internal error, message without source address;", NULL); + } + log_pipe_queue(s->pipe_next, msg, path_flags); } @@ -302,6 +308,19 @@ afsocket_sd_set_max_connections(LogDrive self->max_connections = max_connections; } +static inline gchar * +afsocket_sd_format_persist_name(AFSocketSourceDriver *self, gboolean listener_name) +{ + static gchar persist_name[128]; + gchar buf[64]; + + g_snprintf(persist_name, sizeof(persist_name), + listener_name ? "afsocket_sd_listen_fd(%s,%s)" : "afsocket_sd_connections(%s,%s)", + !!(self->flags & AFSOCKET_STREAM) ? "stream" : "dgram", + g_sockaddr_format(self->bind_addr, buf, sizeof(buf))); + return persist_name; +} + gboolean afsocket_sd_process_connection(AFSocketSourceDriver *self, GSockAddr *peer_addr, gint fd) { @@ -371,32 +390,43 @@ gboolean afsocket_sd_init(LogPipe *s, GlobalConfig *cfg, PersistentConfig *persist) { AFSocketSourceDriver *self = (AFSocketSourceDriver *) s; - gchar persist_name[256], buf[128]; gint sock; + gboolean res = FALSE; log_reader_options_init(&self->reader_options, cfg); - g_snprintf(persist_name, sizeof(persist_name), - "afsocket_sd_listen_fd(%s,%s)", - !!(self->flags & AFSOCKET_STREAM) ? "stream" : "dgram", - g_sockaddr_format(self->bind_addr, buf, sizeof(buf))); - - /* NOTE: this assumes that fd 0 will never be used for listening fds, - * main.c opens fd 0 so this assumption can hold */ - - sock = GPOINTER_TO_UINT(persist_config_fetch(persist, persist_name)) - 1; - - if (sock == -1 && !afsocket_open_socket(self->bind_addr, !!(self->flags & AFSOCKET_STREAM), &sock)) + /* fetch persistent connections first */ + if ((self->flags & AFSOCKET_KEEP_ALIVE)) { - return FALSE; + GList *p; + + self->connections = persist_config_fetch(persist, afsocket_sd_format_persist_name(self, FALSE)); + for (p = self->connections; p; p = p->next) + { + log_pipe_append((LogPipe *) p->data, s); + } } + + /* ok, we have connection list, check if we need to open a listener */ + sock = -1; if (self->flags & AFSOCKET_STREAM) { GSource *source; - GList *p; + if (self->flags & AFSOCKET_KEEP_ALIVE) + { + /* NOTE: this assumes that fd 0 will never be used for listening fds, + * main.c opens fd 0 so this assumption can hold */ + sock = GPOINTER_TO_UINT(persist_config_fetch(persist, afsocket_sd_format_persist_name(self, TRUE))) - 1; + } + + if (sock == -1 && !afsocket_open_socket(self->bind_addr, !!(self->flags & AFSOCKET_STREAM), &sock)) + { + return FALSE; + } + - /* create listening source */ + /* set up listening source */ self->fd = sock; listen(sock, self->listen_backlog); @@ -407,26 +437,21 @@ afsocket_sd_init(LogPipe *s, GlobalConfi g_source_set_callback(source, afsocket_sd_accept, self, (GDestroyNotify) log_pipe_unref); self->source_id = g_source_attach(source, NULL); g_source_unref(source); - - /* fetch kept alive connections */ - - g_snprintf(persist_name, sizeof(persist_name), - "afsocket_sd_connections(%s)", - g_sockaddr_format(self->bind_addr, buf, sizeof(buf))); - self->connections = persist_config_fetch(persist, persist_name); - for (p = self->connections; p; p = p->next) - { - log_pipe_append((LogPipe *) p->data, s); - } } else { - self->fd = sock; - self->reader = log_reader_new(fd_read_new(sock, FR_RECV | FR_DONTCLOSE), (self->flags & AFSOCKET_LOCAL) ? LR_LOCAL : 0, &self->super.super, &self->reader_options); - log_pipe_append(self->reader, &self->super.super); - log_pipe_init(self->reader, NULL, NULL); + if (!(self->flags & AFSOCKET_KEEP_ALIVE) || !self->connections || !afsocket_open_socket(self->bind_addr, !!(self->flags & AFSOCKET_STREAM), &sock)) + { + /* we could not recover persistent listener and could not open one either */ + return FALSE; + } + self->fd = -1; + + /* we either have self->connections != NULL, or sock contains a new fd */ + if (self->connections || afsocket_sd_process_connection(self, NULL, sock)) + res = TRUE; } - return TRUE; + return res; } static void @@ -440,30 +465,32 @@ gboolean afsocket_sd_deinit(LogPipe *s, GlobalConfig *cfg, PersistentConfig *persist) { AFSocketSourceDriver *self = (AFSocketSourceDriver *) s; - gchar persist_name[256], buf[128]; - if (self->flags & AFSOCKET_STREAM) + if ((self->flags & AFSOCKET_KEEP_ALIVE) == 0 || !persist) { GList *p, *next; - - if ((self->flags & AFSOCKET_KEEP_ALIVE) == 0 || !persist) - { - for (p = self->connections; p; p = next) - { - next = p->next; - afsocket_sd_kill_connection((AFSocketSourceConnection *) p->data); - g_list_free_1(p); - } - } - else + + /* we don't store anything across HUPs */ + for (p = self->connections; p; p = next) { - - g_snprintf(persist_name, sizeof(persist_name), - "afsocket_sd_connections(%s)", - g_sockaddr_format(self->bind_addr, buf, sizeof(buf))); - persist_config_add(persist, persist_name, self->connections, (GDestroyNotify) afsocket_sd_kill_connection); + next = p->next; + afsocket_sd_kill_connection((AFSocketSourceConnection *) p->data); + g_list_free_1(p); } - self->connections = NULL; + } + else + { + /* for AFSOCKET_STREAM source drivers this is a list, for + * AFSOCKET_DGRAM this is a single connection */ + + persist_config_add(persist, afsocket_sd_format_persist_name(self, FALSE), self->connections, (GDestroyNotify) afsocket_sd_kill_connection); + } + self->connections = NULL; + + + if (self->flags & AFSOCKET_STREAM) + { + g_source_remove(self->source_id); self->source_id = 0; if ((self->flags & AFSOCKET_LISTENER_KEEP_ALIVE) == 0) @@ -473,27 +500,23 @@ afsocket_sd_deinit(LogPipe *s, GlobalCon NULL); close(self->fd); } + else + { + /* NOTE: the fd is incremented by one when added to persistent config + * as persist config cannot store NULL */ + + persist_config_add(persist, afsocket_sd_format_persist_name(self, TRUE), GUINT_TO_POINTER(self->fd + 1), afsocket_sd_close_fd); + } } else if (self->flags & AFSOCKET_DGRAM) { - log_pipe_deinit(self->reader, NULL, NULL); - log_pipe_unref(self->reader); - self->reader = NULL; - } - - if (self->flags & AFSOCKET_LISTENER_KEEP_ALIVE) - { - g_snprintf(persist_name, sizeof(persist_name), - "afsocket_sd_listen_fd(%s,%s)", - !!(self->flags & AFSOCKET_STREAM) ? "stream" : "dgram", - g_sockaddr_format(self->bind_addr, buf, sizeof(buf))); - - /* NOTE: the fd is incremented by one when added to persistent config - * as persist config cannot store NULL */ - - persist_config_add(persist, persist_name, GUINT_TO_POINTER(self->fd + 1), afsocket_sd_close_fd); + /* we don't need to close the listening fd here as we have a + * single connection which will close it */ + + ; } + return TRUE; } @@ -519,8 +542,6 @@ afsocket_sd_free_instance(AFSocketSource g_sockaddr_unref(self->bind_addr); self->bind_addr = NULL; - g_assert(!self->reader); - log_drv_free_instance(&self->super); } --- orig/src/afsocket.h +++ mod/src/afsocket.h @@ -45,7 +45,6 @@ struct _AFSocketSourceDriver guint32 flags; gint fd; guint source_id; - LogPipe *reader; LogReaderOptions reader_options; GSockAddr *bind_addr; -- Bazsi