pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-pcap/+/39353?usp=email )
Change subject: server: Introduce struct osmo_pcap_wr_file ......................................................................
server: Introduce struct osmo_pcap_wr_file
This encloses file state and operation, allowing for further improvement later on: * Making non-blocking/async writes * Writing to new file while finishing writes to previous files
Change-Id: I5244fbc7a5dc047e1f5f7b77954cbb9c1f610181 --- M include/osmo-pcap/osmo_pcap_server.h M src/osmo_server_core.c M src/osmo_server_network.c 3 files changed, 130 insertions(+), 67 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-pcap refs/changes/53/39353/1
diff --git a/include/osmo-pcap/osmo_pcap_server.h b/include/osmo-pcap/osmo_pcap_server.h index a808bff..9664084 100644 --- a/include/osmo-pcap/osmo_pcap_server.h +++ b/include/osmo-pcap/osmo_pcap_server.h @@ -73,6 +73,21 @@ SERVER_CTR_NOCLIENT, };
+struct osmo_pcap_wr_file { + struct osmo_pcap_conn *conn; /* backpointer */ + /* canonicalized absolute pathname of pcap file we write to */ + char *filename; + /* file descriptor of the file we write to */ + int local_fd; + /* Current write offset of the file we write to (local_fd) */ + off_t wr_offset; +}; +struct osmo_pcap_wr_file *osmo_pcap_wr_file_alloc(struct osmo_pcap_conn *conn); +void osmo_pcap_wr_file_free(struct osmo_pcap_wr_file *wrf); +int osmo_pcap_wr_file_open(struct osmo_pcap_wr_file *wrf, const char *filename, mode_t mode); +void osmo_pcap_wr_file_close(struct osmo_pcap_wr_file *wrf); +int osmo_pcap_wr_file_write(struct osmo_pcap_wr_file *wrf, const uint8_t *data, size_t len); + struct osmo_pcap_conn { /* list of connections */ struct llist_head entry; @@ -86,12 +101,7 @@
/* Remote connection */ struct osmo_stream_srv *srv; - /* canonicalized absolute pathname of pcap file we write to */ - char *curr_filename; - /* file descriptor of the file we write to */ - int local_fd; - /* Current write offset of the file we write to (local_fd) */ - off_t wr_offset; + struct osmo_pcap_wr_file *wrf;
/* pcap stuff */ enum osmo_pcap_fmt file_fmt; diff --git a/src/osmo_server_core.c b/src/osmo_server_core.c index 2230cf7..0df36c3 100644 --- a/src/osmo_server_core.c +++ b/src/osmo_server_core.c @@ -116,6 +116,66 @@ 0); }
+struct osmo_pcap_wr_file *osmo_pcap_wr_file_alloc(struct osmo_pcap_conn *conn) +{ + struct osmo_pcap_wr_file *wrf = talloc_zero(conn, struct osmo_pcap_wr_file); + OSMO_ASSERT(wrf); + + wrf->conn = conn; + wrf->local_fd = -1; + wrf->wr_offset = 0; + + return wrf; +} + +void osmo_pcap_wr_file_free(struct osmo_pcap_wr_file *wrf) +{ + if (!wrf) + return; + osmo_pcap_wr_file_close(wrf); + talloc_free(wrf); +} + +int osmo_pcap_wr_file_open(struct osmo_pcap_wr_file *wrf, const char *filename, mode_t mode) +{ + int rc; + OSMO_ASSERT(filename); + OSMO_ASSERT(wrf->local_fd == -1); + + rc = open(filename, O_CREAT|O_WRONLY|O_TRUNC, mode); + if (rc < 0) { + LOGP(DSERVER, LOGL_ERROR, "Failed to open file '%s': %s\n", + filename, strerror(errno)); + return rc; + } + wrf->local_fd = rc; + wrf->filename = talloc_strdup(wrf, filename); + OSMO_ASSERT(wrf->filename); + return rc; +} + +void osmo_pcap_wr_file_close(struct osmo_pcap_wr_file *wrf) +{ + if (wrf->local_fd > 0) { + close(wrf->local_fd); + wrf->local_fd = -1; + } +} + +int osmo_pcap_wr_file_write(struct osmo_pcap_wr_file *wrf, const uint8_t *data, size_t len) +{ + int rc = write(wrf->local_fd, data, len); + if (rc >= 0) { + wrf->wr_offset += rc; + if (rc != len) { + LOGP(DSERVER, LOGL_ERROR, "Short write '%s': ret %d != %zu\n", + wrf->filename, rc, len); + return -1; + } + } + return rc; +} + static inline size_t calc_data_max_len(const struct osmo_pcap_server *server) { size_t data_max_len; @@ -207,7 +267,6 @@ /* we never write */ osmo_wqueue_init(&conn->rem_wq, 0); conn->rem_wq.bfd.fd = -1; - conn->local_fd = -1; conn->server = server; llist_add_tail(&conn->entry, &server->conn); return conn; @@ -235,9 +294,9 @@
/* Move pcap file from base_path to completed_path, and updates * conn->curr_filename to point to new location. */ -void move_completed_trace_if_needed(struct osmo_pcap_conn *conn) +void move_completed_trace_if_needed(struct osmo_pcap_wr_file *wrf) { - struct osmo_pcap_server *server = conn->server; + struct osmo_pcap_server *server; char *curr_filename_cpy_bname = NULL; char *curr_filename_cpy_dname = NULL; char *bname = NULL; @@ -247,9 +306,10 @@ size_t new_filename_len; int rc;
- if (!conn->curr_filename) + if (!wrf) return;
+ server = wrf->conn->server; if (!server->completed_path) return;
@@ -257,8 +317,8 @@
/* basename and dirname may modify input param, and return a string * which shall not be freed, potentially pointing to the input param. */ - curr_filename_cpy_dname = talloc_strdup(conn, conn->curr_filename); - curr_filename_cpy_bname = talloc_strdup(conn, conn->curr_filename); + curr_filename_cpy_dname = talloc_strdup(wrf, wrf->filename); + curr_filename_cpy_bname = talloc_strdup(wrf, wrf->filename); if (!curr_filename_cpy_dname || !curr_filename_cpy_bname) goto ret_free1;
@@ -266,7 +326,7 @@ bname = basename(curr_filename_cpy_bname); if (!curr_dirname || !bname) { LOGP(DSERVER, LOGL_ERROR, "Failed to resolve dirname and basename for '%s'\n", - conn->curr_filename); + wrf->filename); goto ret_free1; }
@@ -278,28 +338,28 @@ }
new_filename_len = strlen(new_dirname) + 1 /* '/' */ + strlen(bname) + 1 /* '\0' */; - new_filename = talloc_size(conn, new_filename_len); + new_filename = talloc_size(wrf, new_filename_len); if (!new_filename) goto ret_free1; rc = snprintf(new_filename, new_filename_len, "%s/%s", new_dirname, bname); if (rc != new_filename_len - 1) goto ret_free2;
- LOGP(DSERVER, LOGL_INFO, "Moving completed pcap file '%s' -> '%s'\n", conn->curr_filename, new_filename); - rc = rename(conn->curr_filename, new_filename); + LOGP(DSERVER, LOGL_INFO, "Moving completed pcap file '%s' -> '%s'\n", wrf->filename, new_filename); + rc = rename(wrf->filename, new_filename); if (rc == -1) { int err = errno; LOGP(DSERVER, LOGL_ERROR, "Failed moving completed pcap file '%s' -> '%s': %s\n", - conn->curr_filename, new_filename, strerror(err)); + wrf->filename, new_filename, strerror(err)); if (err == EXDEV) LOGP(DSERVER, LOGL_ERROR, "Fix your config! %s and %s shall not be in different filesystems!\n", curr_dirname, new_dirname); goto ret_free2; }
- /* Now replace conn->curr_filename with new path: */ - talloc_free(conn->curr_filename); - conn->curr_filename = new_filename; + /* Now replace wrf->filename with new path: */ + talloc_free(wrf->filename); + wrf->filename = new_filename; /* new_filename has been assigned, so we don't want to free it, hence move to ret_free1: */ goto ret_free1;
@@ -311,27 +371,21 @@ talloc_free(curr_filename_cpy_dname); }
-static void close_local_fd(struct osmo_pcap_conn *conn) -{ - close(conn->local_fd); - conn->local_fd = -1; - conn->wr_offset = 0; -} - void osmo_pcap_conn_close_trace(struct osmo_pcap_conn *conn) { - if (conn->local_fd >= 0) - close_local_fd(conn); + if (!conn->wrf) + return;
- move_completed_trace_if_needed(conn); + osmo_pcap_wr_file_close(conn->wrf);
- if (conn->curr_filename) { - osmo_pcap_conn_event(conn, "closingtracefile", conn->curr_filename); - rate_ctr_inc2(conn->ctrg, PEER_CTR_PROTATE); - rate_ctr_inc2(conn->server->ctrg, SERVER_CTR_PROTATE); - talloc_free(conn->curr_filename); - conn->curr_filename = NULL; - } + move_completed_trace_if_needed(conn->wrf); + + osmo_pcap_conn_event(conn, "closingtracefile", conn->wrf->filename); + rate_ctr_inc2(conn->ctrg, PEER_CTR_PROTATE); + rate_ctr_inc2(conn->server->ctrg, SERVER_CTR_PROTATE); + + osmo_pcap_wr_file_free(conn->wrf); + conn->wrf = NULL; }
void osmo_pcap_conn_close(struct osmo_pcap_conn *conn) @@ -358,7 +412,7 @@ /* Update conn->last_write if needed. This field is used to keep the last time * period where we wrote to the pcap file. Once a new write period (based on * rotation VTY config) is detected, the pcap file we write to is rotated. */ -static void update_last_write(struct osmo_pcap_conn *conn, time_t now, size_t len) +static void update_last_write(struct osmo_pcap_conn *conn, time_t now) { time_t last = mktime(&conn->last_write);
@@ -368,8 +422,6 @@ * using the current one. */ if (now > last) localtime_r(&now, &conn->last_write); - - conn->wr_offset += len; }
void osmo_pcap_conn_restart_trace(struct osmo_pcap_conn *conn) @@ -377,15 +429,16 @@ time_t now = time(NULL); struct tm tm; int rc; - char *real_base_path; + char *real_base_path, *curr_filename;
osmo_pcap_conn_close_trace(conn);
/* omit any storing/creation of the file */ if (!conn->store) { - update_last_write(conn, now, 0); - talloc_free(conn->curr_filename); - conn->curr_filename = NULL; + update_last_write(conn, now); + /* TODO: Once we support async write, we'll want to schedule close here instead of freeing: */ + osmo_pcap_wr_file_free(conn->wrf); + conn->wrf = NULL; return; }
@@ -396,32 +449,31 @@ conn->server->base_path, strerror(errno)); return; } - conn->curr_filename = talloc_asprintf(conn, "%s/trace-%s-%d%.2d%.2d_%.2d%.2d%.2d.%s", - real_base_path, conn->name, - tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday, - tm.tm_hour, tm.tm_min, tm.tm_sec, - conn->file_fmt == OSMO_PCAP_FMT_PCAP ? "pcap" : "pcapng"); + curr_filename = talloc_asprintf(conn, "%s/trace-%s-%d%.2d%.2d_%.2d%.2d%.2d.%s", + real_base_path, conn->name, + tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday, + tm.tm_hour, tm.tm_min, tm.tm_sec, + conn->file_fmt == OSMO_PCAP_FMT_PCAP ? "pcap" : "pcapng"); free(real_base_path); - if (!conn->curr_filename) { + if (!curr_filename) { LOGP(DSERVER, LOGL_ERROR, "Failed to assemble filename for %s.\n", conn->name); return; }
- conn->local_fd = creat(conn->curr_filename, conn->server->permission_mask); - if (conn->local_fd < 0) { - LOGP(DSERVER, LOGL_ERROR, "Failed to create file '%s': %s\n", - conn->curr_filename, strerror(errno)); + conn->wrf = osmo_pcap_wr_file_alloc(conn); + rc = osmo_pcap_wr_file_open(conn->wrf, curr_filename, conn->server->permission_mask); + talloc_free(curr_filename); + if (rc < 0) return; - } - conn->wr_offset = 0;
- rc = write(conn->local_fd, conn->file_hdr, conn->file_hdr_len); + rc = osmo_pcap_wr_file_write(conn->wrf, conn->file_hdr, conn->file_hdr_len); if (rc != conn->file_hdr_len) { LOGP(DSERVER, LOGL_ERROR, "Failed to write the header: %d\n", errno); - close_local_fd(conn); + osmo_pcap_wr_file_free(conn->wrf); + conn->wrf = NULL; return; } - update_last_write(conn, now, rc); + update_last_write(conn, now); }
void osmo_pcap_server_reopen(struct osmo_pcap_server *server) @@ -442,9 +494,10 @@ /* Returns true if pcap was re-opened */ static bool check_restart_pcap_max_size(struct osmo_pcap_conn *conn, size_t data_len) { + OSMO_ASSERT(conn->wrf); if (!pcap_server->max_size_enabled) return false; - if (conn->wr_offset + data_len <= conn->server->max_size) + if (conn->wrf->wr_offset + data_len <= conn->server->max_size) return false;
LOGP(DSERVER, LOGL_NOTICE, "Rolling over file for %s (max-size)\n", conn->name); @@ -584,11 +637,11 @@ zmq_send_client_data(conn, data, len);
if (!conn->store) { - update_last_write(conn, now, 0); + update_last_write(conn, now); return 0; }
- if (conn->local_fd < -1) { + if (!conn->wrf) { LOGP(DSERVER, LOGL_ERROR, "No file is open. close connection.\n"); return -1; } @@ -597,12 +650,12 @@ if (!check_restart_pcap_max_size(conn, len)) check_restart_pcap_localtime(conn, now);
- rc = write(conn->local_fd, data, len); - if (rc != len) { - LOGP(DSERVER, LOGL_ERROR, "Failed to write for %s\n", conn->name); + rc = osmo_pcap_wr_file_write(conn->wrf, data, len); + if (rc < 0) { + LOGP(DSERVER, LOGL_ERROR, "%s: Failed writing to file\n", conn->name); return -1; } - update_last_write(conn, now, rc); + update_last_write(conn, now); return 0; }
diff --git a/src/osmo_server_network.c b/src/osmo_server_network.c index e6bd67b..11f92fc 100644 --- a/src/osmo_server_network.c +++ b/src/osmo_server_network.c @@ -146,7 +146,7 @@ if ((rc = validate_link_hdr(conn, data)) < 0) return rc;
- if (conn->store && conn->local_fd < 0) { + if (conn->store && !conn->wrf) { /* First received link hdr in conn */ talloc_free(conn->file_hdr); conn->file_hdr = talloc_size(conn, data->len);
-
pespin