Hi,
this series of patches makes sure that the logs generated by osmo_pcap_server are cleaned up. I wasn't able to use logrotate since we wanted to keep the timestamped filenames and logrotate would treat every log as independent. The script now sorts the pcap logs per-client and deletes all but the newest N files. The oldest M remaining files will be compressed with gzip. Some small fixes for problems I encountered are included. I hope the requirement for libosmocore 0.3.2 is okay.
Regards,
Daniel Willmann (4): server: Fix memory leak and error handling in restart_pcap server: Register signal handler to reopen logfiles on SIGHUP Catch up with API change in osmo_sock_init contrib: Add a script to clean up in regular intervals
Makefile.am | 2 +- configure.ac | 3 +- contrib/Makefile.am | 1 + contrib/clean_old | 46 ++++++++++++++++++++++++++++++++++ include/osmo-pcap/osmo_pcap_server.h | 2 + src/osmo_client_network.c | 2 +- src/osmo_server_main.c | 4 +++ src/osmo_server_network.c | 31 +++++++++++++++++++++- 8 files changed, 86 insertions(+), 5 deletions(-) create mode 100644 contrib/Makefile.am create mode 100755 contrib/clean_old
--- src/osmo_server_network.c | 9 ++++++++- 1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/src/osmo_server_network.c b/src/osmo_server_network.c index 121aa2c..be61d03 100644 --- a/src/osmo_server_network.c +++ b/src/osmo_server_network.c @@ -62,14 +62,20 @@ static void restart_pcap(struct osmo_pcap_conn *conn) conn->local_fd = -1; }
- filename = talloc_asprintf(conn, "%s/trace-%s-%d%.2d%.2d_%.2d%.2d%.2d.pcap", conn->server->base_path, conn->name, tm->tm_year + 1900, tm->tm_mon + 1, tm->tm_mday, tm->tm_hour, tm->tm_min, tm->tm_sec); + + if (!filename) { + LOGP(DSERVER, LOGL_ERROR, "Failed to assemble filename for %s.\n", conn->name); + return; + } + conn->local_fd = creat(filename, 0440); if (conn->local_fd < 0) { LOGP(DSERVER, LOGL_ERROR, "Failed to file: '%s'\n", filename); + talloc_free(filename); return; }
@@ -78,6 +84,7 @@ static void restart_pcap(struct osmo_pcap_conn *conn) LOGP(DSERVER, LOGL_ERROR, "Failed to write the header: %d\n", errno); close(conn->local_fd); conn->local_fd = -1; + talloc_free(filename); return; }
--- include/osmo-pcap/osmo_pcap_server.h | 2 ++ src/osmo_server_main.c | 4 ++++ src/osmo_server_network.c | 20 ++++++++++++++++++++ 3 files changed, 26 insertions(+), 0 deletions(-)
diff --git a/include/osmo-pcap/osmo_pcap_server.h b/include/osmo-pcap/osmo_pcap_server.h index b408c47..1f2f6fc 100644 --- a/include/osmo-pcap/osmo_pcap_server.h +++ b/include/osmo-pcap/osmo_pcap_server.h @@ -63,6 +63,7 @@ struct osmo_pcap_conn { /* read buffering */ int state; int pend; + int reopen; char buf[4096]; struct osmo_pcap_data *data; }; @@ -80,6 +81,7 @@ struct osmo_pcap_server {
extern struct osmo_pcap_server *pcap_server;
+void osmo_pcap_server_reopen(struct osmo_pcap_server *server); int osmo_pcap_server_listen(struct osmo_pcap_server *server); struct osmo_pcap_conn *osmo_pcap_server_find(struct osmo_pcap_server *ser, const char *name); diff --git a/src/osmo_server_main.c b/src/osmo_server_main.c index 5c1ad58..4e67163 100644 --- a/src/osmo_server_main.c +++ b/src/osmo_server_main.c @@ -140,6 +140,9 @@ static void signal_handler(int signal) talloc_report(tall_vty_ctx, stderr); talloc_report_full(tall_bsc_ctx, stderr); break; + case SIGHUP: + osmo_pcap_server_reopen(pcap_server); + break; default: break; } @@ -175,6 +178,7 @@ int main(int argc, char **argv) signal(SIGABRT, &signal_handler); signal(SIGUSR1, &signal_handler); osmo_init_ignore_signals(); + signal(SIGHUP, &signal_handler);
telnet_init(tall_bsc_ctx, NULL, 4241);
diff --git a/src/osmo_server_network.c b/src/osmo_server_network.c index be61d03..513ca1f 100644 --- a/src/osmo_server_network.c +++ b/src/osmo_server_network.c @@ -253,6 +253,11 @@ static int read_cb(struct osmo_fd *fd, unsigned int what) conn = fd->data;
if (conn->state == STATE_INITIAL) { + if (conn->reopen) { + LOGP(DSERVER, LOGL_INFO, "Reopening log for %s now.\n", conn->name); + restart_pcap(conn); + conn->reopen = 0; + } return read_cb_initial(fd, conn); } else if (conn->state == STATE_DATA) { return read_cb_data(fd, conn); @@ -336,3 +341,18 @@ int osmo_pcap_server_listen(struct osmo_pcap_server *server)
return 0; } + +void osmo_pcap_server_reopen(struct osmo_pcap_server *server) +{ + struct osmo_pcap_conn *conn; + LOGP(DSERVER, LOGL_INFO, "Reopening all logfiles.\n"); + llist_for_each_entry(conn, &server->conn, entry) { + /* Write the complete packet out first */ + if (conn->state == STATE_INITIAL) { + restart_pcap(conn); + } else { + LOGP(DSERVER, LOGL_INFO, "Delaying %s until current packet is complete.\n", conn->name); + conn->reopen = 1; + } + } +}
The connect0_bind1 parameter has been replaced by a generic flag parameter. With this patch osmo-pcap works (only) with versions of libosmocore 0.3.2 or newer - configure.ac changed to reflects that. --- configure.ac | 2 +- src/osmo_client_network.c | 2 +- src/osmo_server_network.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/configure.ac b/configure.ac index 51669ec..9977ef6 100644 --- a/configure.ac +++ b/configure.ac @@ -15,7 +15,7 @@ AC_PROG_INSTALL AC_PROG_RANLIB
dnl checks for libraries -PKG_CHECK_MODULES(LIBOSMOCORE, libosmocore >= 0.3.0) +PKG_CHECK_MODULES(LIBOSMOCORE, libosmocore >= 0.3.2) PKG_CHECK_MODULES(LIBOSMOVTY, libosmovty >= 0.3.0)
dnl checks for header files diff --git a/src/osmo_client_network.c b/src/osmo_client_network.c index 281e8eb..bd1b9db 100644 --- a/src/osmo_client_network.c +++ b/src/osmo_client_network.c @@ -163,7 +163,7 @@ void osmo_client_connect(struct osmo_pcap_client *client) osmo_wqueue_clear(&client->wqueue);
fd = osmo_sock_init(AF_INET, SOCK_STREAM, IPPROTO_TCP, - client->srv_ip, client->srv_port, 0); + client->srv_ip, client->srv_port, OSMO_SOCK_F_CONNECT); if (fd < 0) { LOGP(DCLIENT, LOGL_ERROR, "Failed to connect to %s:%d\n", diff --git a/src/osmo_server_network.c b/src/osmo_server_network.c index 513ca1f..a67f07a 100644 --- a/src/osmo_server_network.c +++ b/src/osmo_server_network.c @@ -322,7 +322,7 @@ int osmo_pcap_server_listen(struct osmo_pcap_server *server) int fd;
fd = osmo_sock_init(AF_INET, SOCK_STREAM, IPPROTO_TCP, - server->addr, server->port, 1); + server->addr, server->port, OSMO_SOCK_F_BIND); if (fd < 0) { LOGP(DSERVER, LOGL_ERROR, "Failed to create the server socket.\n"); return -1;
This script should be run from cron. It compresses and deletes older files. --- Makefile.am | 2 +- configure.ac | 1 + contrib/Makefile.am | 1 + contrib/clean_old | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 49 insertions(+), 1 deletions(-) create mode 100644 contrib/Makefile.am create mode 100755 contrib/clean_old
diff --git a/Makefile.am b/Makefile.am index b9a4331..7c9c65b 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1,6 +1,6 @@ AUTOMAKE_OPTIONS = foreign dist-bzip2 1.6
-SUBDIRS = include src +SUBDIRS = include src contrib
BUILT_SOURCES = $(top_srcdir)/.version EXTRA_DIST = git-version-gen diff --git a/configure.ac b/configure.ac index 9977ef6..2f4de9d 100644 --- a/configure.ac +++ b/configure.ac @@ -57,4 +57,5 @@ AC_OUTPUT( include/Makefile include/osmo-pcap/Makefile src/Makefile + contrib/Makefile Makefile) diff --git a/contrib/Makefile.am b/contrib/Makefile.am new file mode 100644 index 0000000..b392196 --- /dev/null +++ b/contrib/Makefile.am @@ -0,0 +1 @@ +dist_pkgdata_DATA = clean_old diff --git a/contrib/clean_old b/contrib/clean_old new file mode 100755 index 0000000..3294c45 --- /dev/null +++ b/contrib/clean_old @@ -0,0 +1,46 @@ +#! /bin/sh + +# Script designed to clean up (zip/delete) old files +# Adjust the variables below and then copy/symlink this script +# to /etc/cron/cron.{hourly,daily} + +# We want to keep the filenames dated and that confuses logrotate, +# hence this script. + +# Number of pcap files per client +NUMFILES=8 +ZIPAFTER=3 +VERBOSE=0 + +# Path where the logfiles reside in +BASEPATH=/var/lib/osmo-pcap/ + +# Find the client names present in basepath +# Check how many files there are for each client +# Delete files in excess of NUMFILES + +CLIENTS=$(find $BASEPATH -name "trace-*" |sed -e "s/.*trace-([^-]+).*/\1/" |sort |uniq) + +do_cleanup() +{ + i=1 + for log in $(find $BASEPATH -name "trace-$1*" |sort); do + if [ $i -gt $NUMFILES ]; then + [ $VERBOSE -eq 1 ] && echo "Deleting file $log" + rm -f $log + elif [ $i -gt $ZIPAFTER ]; then + if [ "${log##*.}" != "gz" ]; then + [ $VERBOSE -eq 1 ] && echo "Compressing file $log" + gzip $log + fi + else + [ $VERBOSE -eq 1 ] && echo "Noop for file $log" + fi + i=$(($i+1)) + done +} + +for client in $CLIENTS; do + [ $VERBOSE -eq 1 ] && echo "Cleaning logs for $client" + do_cleanup $client +done
-
Daniel Willmann -
Holger Hans Peter Freyther