January 2025 - gerrit-log - lists.osmocom.org

[XS] Change in ...osmo-s1gw[master]: erab_fsm: add more logging to clarify timeout events

by pespin

Attention is currently required from: fixeria. pespin has posted comments on this change by fixeria. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/39394?usp=email ) Change subject: erab_fsm: add more logging to clarify timeout events ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/39394?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: erlang/osmo-s1gw Gerrit-Branch: master Gerrit-Change-Id: Iad64ce88e747951c0699074d95d504ccf9f61603 Gerrit-Change-Number: 39394 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Wed, 22 Jan 2025 12:30:02 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

5 months

1
0
0 0

[XS] Change in ...osmo-s1gw[master]: erab_fsm: fix copy-paste in a log message

by pespin

Attention is currently required from: fixeria. pespin has posted comments on this change by fixeria. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/39393?usp=email ) Change subject: erab_fsm: fix copy-paste in a log message ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/39393?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: erlang/osmo-s1gw Gerrit-Branch: master Gerrit-Change-Id: I50f78ac99ddb82244107ff9847acec64d2e9fce1 Gerrit-Change-Number: 39393 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Wed, 22 Jan 2025 12:29:44 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

5 months

1
0
0 0

[M] Change in osmo-pcap[master]: server: zero-copy msgb from tcp read to file write

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcap/+/39370?usp=email ) Change subject: server: zero-copy msgb from tcp read to file write ...................................................................... server: zero-copy msgb from tcp read to file write Related: SYS#7080 Change-Id: Iae9b9eaec42ad8ec86d1a8144d676115fa057766 --- M include/osmo-pcap/osmo_pcap_server.h M src/osmo_server_core.c M src/osmo_server_network.c 3 files changed, 97 insertions(+), 67 deletions(-) Approvals: osmith: Looks good to me, but someone else must approve fixeria: Looks good to me, but someone else must approve Jenkins Builder: Verified pespin: Looks good to me, approved diff --git a/include/osmo-pcap/osmo_pcap_server.h b/include/osmo-pcap/osmo_pcap_server.h index e71096b..f97e4ae 100644 --- a/include/osmo-pcap/osmo_pcap_server.h +++ b/include/osmo-pcap/osmo_pcap_server.h @@ -121,15 +121,13 @@ /* pcap stuff */ enum osmo_pcap_fmt file_fmt; bool pcapng_endian_swapped; - uint8_t *file_hdr; - uint32_t file_hdr_len; + struct msgb *file_hdr_msg; /* last time */ struct tm last_write; /* read buffering */ int state; - int pend; bool reopen_delayed; size_t data_max_len; /* size of allocated buffer in data->data. */ @@ -141,13 +139,13 @@ size_t tls_limit_read; struct osmo_tls_session tls_session; struct osmo_wqueue rem_wq; - struct osmo_pcap_data *data; /* Used to store TLS decoded data */ + struct msgb *rx_tls_dec_msg; /* Used to store TLS decoded data */ }; void osmo_pcap_conn_free(struct osmo_pcap_conn *conn); void vty_server_init(void); void osmo_pcap_conn_close(struct osmo_pcap_conn *conn); -int osmo_pcap_conn_process_data(struct osmo_pcap_conn *conn, const uint8_t *data, size_t len); +int osmo_pcap_conn_process_data(struct osmo_pcap_conn *conn, struct msgb *msg); void osmo_pcap_conn_restart_trace(struct osmo_pcap_conn *conn); void osmo_pcap_conn_close_trace(struct osmo_pcap_conn *conn); void osmo_pcap_conn_event(struct osmo_pcap_conn *conn, diff --git a/src/osmo_server_core.c b/src/osmo_server_core.c index 04c14c1..9629423 100644 --- a/src/osmo_server_core.c +++ b/src/osmo_server_core.c @@ -109,7 +109,7 @@ talloc_free(event_name); pcap_zmq_send(conn->server->zmq_publ, - conn->file_hdr, conn->file_hdr_len, + msgb_data(conn->file_hdr_msg), msgb_length(conn->file_hdr_msg), ZMQ_SNDMORE); pcap_zmq_send(conn->server->zmq_publ, data, len, @@ -199,7 +199,6 @@ INIT_LLIST_HEAD(&conn->wrf_flushing_list); conn->data_max_len = calc_data_max_len(server); - conn->data = talloc_zero_size(conn, sizeof(struct osmo_pcap_data) + conn->data_max_len); /* a bit nasty. we do not work with ids but names */ desc = talloc_zero(conn, struct rate_ctr_group_desc); if (!desc) { @@ -289,9 +288,11 @@ close(conn->rem_wq.bfd.fd); conn->rem_wq.bfd.fd = -1; osmo_tls_release(&conn->tls_session); + msgb_free(conn->rx_tls_dec_msg); + conn->rx_tls_dec_msg = NULL; } - TALLOC_FREE(conn->file_hdr); - conn->file_hdr_len = 0; + msgb_free(conn->file_hdr_msg); + conn->file_hdr_msg = NULL; osmo_pcap_conn_close_trace(conn); osmo_pcap_conn_event(conn, "disconnect", NULL); @@ -318,6 +319,7 @@ struct tm tm; int rc; char *real_base_path, *curr_filename; + struct msgb *msg; osmo_pcap_conn_close_trace(conn); @@ -352,10 +354,10 @@ if (rc < 0) return; - /* TODO: get msgb from conn object stored: */ - struct msgb *msg = msgb_alloc_c(conn->wrf, conn->file_hdr_len, "local_iofd_hdr"); - memcpy(msgb_put(msg, conn->file_hdr_len), conn->file_hdr, conn->file_hdr_len); - + /* We need to keep a clone assigned to conn to check for incoming hdr changes: */ + OSMO_ASSERT(conn->file_hdr_msg); + msg = msgb_copy_c(conn->wrf, conn->file_hdr_msg, "wrf_hdr"); + OSMO_ASSERT(msg); rc = osmo_pcap_wr_file_write_msgb(conn->wrf, msg); if (rc < 0) { LOGP(DSERVER, LOGL_ERROR, "Failed to write the header: %d\n", errno); @@ -518,16 +520,17 @@ } /* New recorded packet is received. - * Returns 0 on success, negative on error. */ -int osmo_pcap_conn_process_data(struct osmo_pcap_conn *conn, const uint8_t *data, size_t len) + * Returns 0 on success (and owns msgb), negative on error (msgb to be freed by caller). */ +int osmo_pcap_conn_process_data(struct osmo_pcap_conn *conn, struct msgb *msg) { time_t now = time(NULL); int rc; - zmq_send_client_data(conn, data, len); + zmq_send_client_data(conn, msgb_data(msg), msgb_length(msg)); if (!conn->store) { update_last_write(conn, now); + msgb_free(msg); return 0; } @@ -537,20 +540,18 @@ } /* Check if we are past the limit or on a day change. */ - if (!check_restart_pcap_max_size(conn, len)) + if (!check_restart_pcap_max_size(conn, msgb_length(msg))) check_restart_pcap_localtime(conn, now); - /* TODO: get msgb from caller: */ - struct msgb *msg = msgb_alloc_c(conn->wrf, len, "local_iofd_msg"); - memcpy(msgb_put(msg, len), data, len); - + talloc_steal(conn->wrf, msg); rc = osmo_pcap_wr_file_write_msgb(conn->wrf, msg); if (rc < 0) { LOGP(DSERVER, LOGL_ERROR, "%s: Failed writing to file\n", conn->name); - msgb_free(msg); + /* msgb will be freed by caller */ return -1; } update_last_write(conn, now); + /* msgb is now owned by conn->wrf. */ return 0; } diff --git a/src/osmo_server_network.c b/src/osmo_server_network.c index 5c7a008..da14480 100644 --- a/src/osmo_server_network.c +++ b/src/osmo_server_network.c @@ -121,9 +121,29 @@ } } -/* returns >0 on success, <= 0 on failure (closes conn) */ -static int rx_link_hdr(struct osmo_pcap_conn *conn, const struct osmo_pcap_data *data) +/* Updates conn->file_hdr_msg, owns (frees) msg. */ +static void update_conn_file_hdr_msg(struct osmo_pcap_conn *conn, struct msgb *msg) { + /* The msg chunk of memory to hold the hdr data may actually be a lot bigger than + * the actual data (len << data_len). + * Hence, since we may keep this around for a big time (life of the conn), let's + * better keep a reduced memory footprint msgb instead of the original one: */ + msgb_free(conn->file_hdr_msg); + conn->file_hdr_msg = msgb_copy_resize_c(conn, msg, msgb_length(msg), "conn_file_hdr_msg"); + OSMO_ASSERT(conn->file_hdr_msg); + msgb_free(msg); + + /* pull to l2 is done here at the copy since anyway msgb_copy_resize + * cannot shrink available headroom: */ + msgb_pull_to_l2(conn->file_hdr_msg); + + osmo_pcap_conn_restart_trace(conn); +} + +/* returns >0 on success (msg becomes owned), <= 0 on failure (closes conn) */ +static int rx_link_hdr(struct osmo_pcap_conn *conn, struct msgb *msg) +{ + struct osmo_pcap_data *data = (struct osmo_pcap_data *)msg->l1h; int rc; rc = osmo_pcap_file_discover_fmt(data->data, data->len, &conn->file_fmt); @@ -148,19 +168,11 @@ if (conn->store && !conn->wrf) { /* First received link hdr in conn */ - talloc_free(conn->file_hdr); - conn->file_hdr = talloc_size(conn, data->len); - memcpy(conn->file_hdr, data->data, data->len); - conn->file_hdr_len = data->len; - osmo_pcap_conn_restart_trace(conn); - } else if (conn->file_hdr_len != data->len || - memcmp(&conn->file_hdr, data->data, data->len) != 0) { + update_conn_file_hdr_msg(conn, msg); + } else if (msgb_l2len(conn->file_hdr_msg) != msgb_l2len(msg) || + memcmp(msgb_l2(conn->file_hdr_msg), msgb_l2(msg), msgb_l2len(msg)) != 0) { /* Client changed the link hdr in conn */ - talloc_free(conn->file_hdr); - conn->file_hdr = talloc_size(conn, data->len); - memcpy(conn->file_hdr, data->data, data->len); - conn->file_hdr_len = data->len; - osmo_pcap_conn_restart_trace(conn); + update_conn_file_hdr_msg(conn, msg); } return 1; @@ -247,22 +259,25 @@ } /* returns >0 on success, <= 0 on failure (closes conn) */ -static int rx_link_data(struct osmo_pcap_conn *conn, const struct osmo_pcap_data *data) +static int rx_link_data(struct osmo_pcap_conn *conn, struct msgb *msg) { int rc; - if ((rc = validate_link_data(conn, data)) < 0) + if ((rc = validate_link_data(conn, (struct osmo_pcap_data *)msg->l1h)) < 0) return rc; - if ((rc = osmo_pcap_conn_process_data(conn, &data->data[0], data->len)) < 0) + msgb_pull_to_l2(msg); + if ((rc = osmo_pcap_conn_process_data(conn, msg)) < 0) return rc; return 1; } /* Read segment payload, of size data->len. - * returns >0 on success, <= 0 on failure (closes conn) */ -static int rx_link(struct osmo_pcap_conn *conn, const struct osmo_pcap_data *data) + * returns >0 on success, <= 0 on failure (closes conn). + * Message is always owned or freed here. */ +static int rx_link(struct osmo_pcap_conn *conn, struct msgb *msg) { + struct osmo_pcap_data *data = (struct osmo_pcap_data *)msg->l1h; int rc; /* count the full packet we got */ @@ -275,15 +290,18 @@ switch (data->type) { case PKT_LINK_HDR: - rc = rx_link_hdr(conn, data); + rc = rx_link_hdr(conn, msg); break; case PKT_LINK_DATA: - rc = rx_link_data(conn, data); + rc = rx_link_data(conn, msg); break; default: OSMO_ASSERT(0); } + if (rc <= 0) + msgb_free(msg); + if (conn->reopen_delayed) { LOGP(DSERVER, LOGL_INFO, "Reopening log for %s now.\n", conn->name); osmo_pcap_conn_restart_trace(conn); @@ -306,60 +324,69 @@ static int tls_read_cb_initial(struct osmo_pcap_conn *conn) { int rc; + struct msgb *msg = conn->rx_tls_dec_msg; + msg->l1h = msgb_data(msg); + struct osmo_pcap_data *pdata = (struct osmo_pcap_data *)msg->l1h; + size_t pend = sizeof(*pdata) - msgb_length(msg); - rc = do_read_tls(conn, ((uint8_t *)conn->data) + sizeof(*conn->data) - conn->pend, conn->pend); + OSMO_ASSERT(sizeof(*pdata) > msgb_length(msg)); + + rc = do_read_tls(conn, msg->tail, pend); if (rc <= 0) { LOGP(DSERVER, LOGL_ERROR, - "Too short packet. Got %d, wanted %d\n", rc, conn->data->len); + "Too short packet. Got %d, wanted %zu\n", rc, pend); return -1; } - - conn->pend -= rc; - if (conn->pend < 0) { + if (rc > pend) { LOGP(DSERVER, LOGL_ERROR, - "Someone got the pending read wrong: %d\n", conn->pend); + "Someone got the pending read wrong: %zu\n", pend); return -1; } - if (conn->pend > 0) + msgb_put(msg, rc); + + if (pend > rc) return 1; /* Wait for more data before continuing */ - conn->data->len = ntohs(conn->data->len); + pdata->len = ntohs(pdata->len); - if (conn->data->len > conn->data_max_len) { + if (pdata->len + sizeof(*pdata) > conn->data_max_len) { LOGP(DSERVER, LOGL_ERROR, "Implausible data length: %u > %zu (snaplen %u)\n", - conn->data->len, conn->data_max_len, conn->server->max_snaplen); + pdata->len, conn->data_max_len, conn->server->max_snaplen); return -1; } conn->state = STATE_DATA; - conn->pend = conn->data->len; + msg->l2h = msg->tail; return 1; } static int tls_read_cb_data(struct osmo_pcap_conn *conn) { int rc; + struct msgb *msg = conn->rx_tls_dec_msg; + struct osmo_pcap_data *pdata = (struct osmo_pcap_data *)msg->l1h; + size_t pend = pdata->len - msgb_l2len(msg); - rc = do_read_tls(conn, &conn->data->data[conn->data->len - conn->pend], conn->pend); + rc = do_read_tls(conn, msg->tail, pend); if (rc <= 0) { LOGP(DSERVER, LOGL_ERROR, - "Too short packet. Got %d, wanted %d\n", rc, conn->data->len); + "Too short packet. Got %d, wanted %u\n", rc, pdata->len); return -1; } - - conn->pend -= rc; - if (conn->pend < 0) { + if (rc > pend) { LOGP(DSERVER, LOGL_ERROR, - "Someone got the pending read wrong: %d\n", conn->pend); + "Someone got the pending read wrong: %zu\n", pend); return -1; } - if (conn->pend > 0) + msgb_put(msg, rc); + + if (pend > rc) return 1; /* Wait for more data before continuing */ conn->state = STATE_INITIAL; - conn->pend = sizeof(*conn->data); + conn->rx_tls_dec_msg = msgb_alloc_c(conn, conn->data_max_len, "rx_tls_dec_data"); - return rx_link(conn, conn->data); + return rx_link(conn, msg); } /* returns >0 on success, <= 0 on failure (closes conn) */ @@ -422,11 +449,15 @@ return 0; } - data = (struct osmo_pcap_data *)msgb_data(msg); + OSMO_ASSERT(msgb_length(msg) >= sizeof(*data)); + + msg->l1h = msgb_data(msg); + data = (struct osmo_pcap_data *)msg->l1h; data->len = osmo_ntohs(data->len); - rc = rx_link(conn, data); - msgb_free(msg); + msg->l2h = msg->l1h + sizeof(*data); + + rc = rx_link(conn, msg); if (rc <= 0) osmo_pcap_conn_close(conn); return 0; @@ -485,7 +516,7 @@ } /* Prepare for first read of segment header: */ conn->state = STATE_INITIAL; - conn->pend = sizeof(struct osmo_pcap_data); + conn->rx_tls_dec_msg = msgb_alloc_c(conn, conn->data_max_len, "rx_tls_dec_data"); if (!osmo_tls_init_server_session(conn, server)) { osmo_pcap_conn_close(conn); return; -- To view, visit https://gerrit.osmocom.org/c/osmo-pcap/+/39370?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-pcap Gerrit-Branch: master Gerrit-Change-Id: Iae9b9eaec42ad8ec86d1a8144d676115fa057766 Gerrit-Change-Number: 39370 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

5 months

1
0
0 0

[M] Change in osmo-pcap[master]: server: Implement non-blocking write to pcap file with osmo_io

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcap/+/39367?usp=email ) Change subject: server: Implement non-blocking write to pcap file with osmo_io ...................................................................... server: Implement non-blocking write to pcap file with osmo_io Actual zero-copy msgb passing from read tcp socket will be implemented in follow-up patches. Change-Id: I098a9455a2a4cc626444e6fc13aa88c4cc9694f0 Related: SYS#7080 --- M include/osmo-pcap/osmo_pcap_server.h M src/osmo_pcap_wr_file.c M src/osmo_server_core.c 3 files changed, 173 insertions(+), 39 deletions(-) Approvals: osmith: Looks good to me, but someone else must approve Jenkins Builder: Verified pespin: Looks good to me, approved laforge: Looks good to me, but someone else must approve diff --git a/include/osmo-pcap/osmo_pcap_server.h b/include/osmo-pcap/osmo_pcap_server.h index 9b93888..e71096b 100644 --- a/include/osmo-pcap/osmo_pcap_server.h +++ b/include/osmo-pcap/osmo_pcap_server.h @@ -29,6 +29,7 @@ #include <pcap.h> +#include <osmocom/core/osmo_io.h> #include <osmocom/core/select.h> #include <osmocom/core/linuxlist.h> #include <osmocom/core/write_queue.h> @@ -73,20 +74,30 @@ SERVER_CTR_NOCLIENT, }; +struct osmo_pcap_wr_file; +typedef void (*osmo_pcap_wr_file_flush_completed_cb_t)(struct osmo_pcap_wr_file *wrf, void *data); struct osmo_pcap_wr_file { + struct llist_head entry; /* entry into (osmo_pcap_conn)->wrf_flushing_list */ void *data; /* user backpointer */ /* canonicalized absolute pathname of pcap file we write to */ char *filename; /* file descriptor of the file we write to */ - int local_fd; + struct osmo_io_fd *local_iofd; /* Current write offset of the file we write to (local_fd) */ off_t wr_offset; + /* Number of bytes confirmed to be written, <=wr_offset */ + off_t wr_completed; + osmo_pcap_wr_file_flush_completed_cb_t flush_completed_cb; }; struct osmo_pcap_wr_file *osmo_pcap_wr_file_alloc(void *ctx, void *data); void osmo_pcap_wr_file_free(struct osmo_pcap_wr_file *wrf); +void osmo_pcap_wr_file_set_flush_completed_cb(struct osmo_pcap_wr_file *wrf, osmo_pcap_wr_file_flush_completed_cb_t flush_completed_cb); 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); +int osmo_pcap_wr_file_write_msgb(struct osmo_pcap_wr_file *wrf, struct msgb *msg); +bool osmo_pcap_wr_file_has_pending_writes(const struct osmo_pcap_wr_file *wrf); +int osmo_pcap_wr_file_flush(struct osmo_pcap_wr_file *wrf, struct llist_head *wrf_flushing_list); +bool osmo_pcap_wr_file_is_flushing(const struct osmo_pcap_wr_file *wrf); void osmo_pcap_wr_file_move_to_dir(struct osmo_pcap_wr_file *wrf, const char *dst_dirpath); struct osmo_pcap_conn { @@ -103,6 +114,9 @@ /* Remote connection */ struct osmo_stream_srv *srv; struct osmo_pcap_wr_file *wrf; + /* list of osmo_pcap_wr_file->entry. + * wrf which we want to close but still have pending writes to be completed */ + struct llist_head wrf_flushing_list; /* pcap stuff */ enum osmo_pcap_fmt file_fmt; diff --git a/src/osmo_pcap_wr_file.c b/src/osmo_pcap_wr_file.c index a66e5e2..582caa8 100644 --- a/src/osmo_pcap_wr_file.c +++ b/src/osmo_pcap_wr_file.c @@ -1,5 +1,5 @@ /* - * Write to a file + * Asynchronous non-blocking write to a file * * (C) 2025 by sysmocom - s.f.m.c. GmbH <info(a)sysmocom.de> * All Rights Reserved @@ -32,14 +32,51 @@ #include <osmo-pcap/common.h> #include <osmo-pcap/osmo_pcap_server.h> +static void local_iofd_write_cb(struct osmo_io_fd *iofd, int res, struct msgb *msg) +{ + struct osmo_pcap_wr_file *wrf = osmo_iofd_get_data(iofd); + + LOGP(DSERVER, LOGL_DEBUG, "%s,fd=%d,wr_completed=%zu: write_cb(res=%d)\n", + osmo_iofd_get_name(iofd), osmo_iofd_get_fd(iofd), (size_t)wrf->wr_completed, res); + if (res <= 0) { + LOGP(DSERVER, LOGL_ERROR, "%s,fd=%d,wr_completed=%zu: Failed writing: %s (%d)\n", + osmo_iofd_get_name(iofd), osmo_iofd_get_fd(iofd), (size_t)wrf->wr_completed, + strerror(-res), res); + /* Trigger cb to tell user to free it, even if it was not being flushed. + * Special attention must be kept at the user regarding this code path, ie. + * user can't assume the wrf was actually in flushing state... + */ + if (wrf->flush_completed_cb) + wrf->flush_completed_cb(wrf, wrf->data); + /* wrf may be freed here. */ + return; + } + + wrf->wr_completed += res; + + if (osmo_pcap_wr_file_is_flushing(wrf)) { + if (!osmo_pcap_wr_file_has_pending_writes(wrf)) { + LOGP(DSERVER, LOGL_DEBUG, "%s,fd=%d,wr_completed=%zu: closing now after completed data write\n", + osmo_iofd_get_name(iofd), osmo_iofd_get_fd(iofd), (size_t)wrf->wr_completed); + if (wrf->flush_completed_cb) + wrf->flush_completed_cb(wrf, wrf->data); + /* wrf may be freed here. */ + return; + } + } +} + struct osmo_pcap_wr_file *osmo_pcap_wr_file_alloc(void *ctx, void *data) { struct osmo_pcap_wr_file *wrf = talloc_zero(ctx, struct osmo_pcap_wr_file); OSMO_ASSERT(wrf); + /* Initialize entry so that we can know whether we are included in a + * list in osmo_pcap_wr_file_is_flushing(): */ + INIT_LLIST_HEAD(&wrf->entry); wrf->data = data; - wrf->local_fd = -1; wrf->wr_offset = 0; + wrf->wr_completed = 0; return wrf; } @@ -49,22 +86,43 @@ if (!wrf) return; osmo_pcap_wr_file_close(wrf); + if (osmo_pcap_wr_file_is_flushing(wrf)) + llist_del(&wrf->entry); talloc_free(wrf); } +void osmo_pcap_wr_file_set_flush_completed_cb(struct osmo_pcap_wr_file *wrf, osmo_pcap_wr_file_flush_completed_cb_t flush_completed_cb) +{ + wrf->flush_completed_cb = flush_completed_cb; +} + int osmo_pcap_wr_file_open(struct osmo_pcap_wr_file *wrf, const char *filename, mode_t mode) { + struct osmo_io_ops ioops = { + .read_cb = NULL, + .write_cb = local_iofd_write_cb, + }; int rc; OSMO_ASSERT(filename); - OSMO_ASSERT(wrf->local_fd == -1); + OSMO_ASSERT(wrf->local_iofd == NULL); - rc = open(filename, O_CREAT|O_WRONLY|O_TRUNC, mode); + rc = open(filename, O_CREAT|O_WRONLY|O_TRUNC|O_NONBLOCK, 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->local_iofd = osmo_iofd_setup(wrf, rc, filename, OSMO_IO_FD_MODE_READ_WRITE, + &ioops, wrf); + if (!wrf->local_iofd) + return -EBADFD; + if (osmo_iofd_register(wrf->local_iofd, -1) < 0) { + osmo_iofd_free(wrf->local_iofd); + wrf->local_iofd = NULL; + return -ENAVAIL; + } + wrf->filename = talloc_strdup(wrf, filename); OSMO_ASSERT(wrf->filename); return rc; @@ -72,26 +130,55 @@ 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; - } + osmo_iofd_free(wrf->local_iofd); + wrf->local_iofd = NULL; } -int osmo_pcap_wr_file_write(struct osmo_pcap_wr_file *wrf, const uint8_t *data, size_t len) +int osmo_pcap_wr_file_write_msgb(struct osmo_pcap_wr_file *wrf, struct msgb *msg) { - 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; - } - } + int rc = osmo_iofd_write_msgb(wrf->local_iofd, msg); + if (rc < 0) + return rc; + wrf->wr_offset += msgb_length(msg); return rc; } +bool osmo_pcap_wr_file_has_pending_writes(const struct osmo_pcap_wr_file *wrf) +{ + return wrf->wr_completed < wrf->wr_offset; +} + +/* Mark the wrf as done writing to it. It will be closed and freed + * asynchronously when all data has been written to it. + * wrf may be freed during the call to this function, so don't use it anymore. */ +int osmo_pcap_wr_file_flush(struct osmo_pcap_wr_file *wrf, struct llist_head *wrf_flushing_list) +{ + if (osmo_pcap_wr_file_is_flushing(wrf)) { + LOGP(DSERVER, LOGL_ERROR, "Trying to flush a file which was already being flushed: '%s'\n", + wrf->filename); + return -EINVAL; + } + + if (!osmo_pcap_wr_file_has_pending_writes(wrf)) { + if (wrf->flush_completed_cb) + wrf->flush_completed_cb(wrf, wrf->data); + /* wrf may be freed here. */ + return 0; + } + + /* Put it in the flushing list, it will be closed freed once pending writes complete. */ + llist_add_tail(&wrf->entry, wrf_flushing_list); + return 0; +} + +/* whether we finished pushing more data to the wrf and we are waiting for it to + * finish writing before closing: + */ +bool osmo_pcap_wr_file_is_flushing(const struct osmo_pcap_wr_file *wrf) +{ + return !llist_empty(&wrf->entry); +} + /* Move file from current dir to dst_dirpath, and updates wrf->filename to point to new location. */ void osmo_pcap_wr_file_move_to_dir(struct osmo_pcap_wr_file *wrf, const char *dst_dirpath) { diff --git a/src/osmo_server_core.c b/src/osmo_server_core.c index 782ead9..04c14c1 100644 --- a/src/osmo_server_core.c +++ b/src/osmo_server_core.c @@ -116,6 +116,35 @@ 0); } +/* wrf has written all data and can safely be closed, rotated, etc. */ +static void osmo_pcap_wr_file_flush_completed_cb(struct osmo_pcap_wr_file *wrf, void *data) +{ + struct osmo_pcap_conn *conn = data; + + if (wrf->wr_completed < wrf->wr_offset) { + LOGP(DSERVER, LOGL_NOTICE, "%s: Closing file with pending writes (%zu completed bytes < %zu wrote bytes)\n", + wrf->filename, wrf->wr_completed, wrf->wr_offset); + } + + if (!osmo_pcap_wr_file_is_flushing(wrf)) { + /* If it is not flushing, it probably is still assigned to conn; + * unassign it: */ + if (conn->wrf == wrf) + conn->wrf = NULL; + } + + osmo_pcap_wr_file_close(wrf); + + if (conn->server->completed_path) + osmo_pcap_wr_file_move_to_dir(wrf, conn->server->completed_path); + + osmo_pcap_conn_event(conn, "closingtracefile", wrf->filename); + rate_ctr_inc2(conn->ctrg, PEER_CTR_PROTATE); + rate_ctr_inc2(conn->server->ctrg, SERVER_CTR_PROTATE); + + osmo_pcap_wr_file_free(wrf); +} + static inline size_t calc_data_max_len(const struct osmo_pcap_server *server) { size_t data_max_len; @@ -168,6 +197,7 @@ return NULL; } + INIT_LLIST_HEAD(&conn->wrf_flushing_list); conn->data_max_len = calc_data_max_len(server); conn->data = talloc_zero_size(conn, sizeof(struct osmo_pcap_data) + conn->data_max_len); /* a bit nasty. we do not work with ids but names */ @@ -227,7 +257,11 @@ void osmo_pcap_conn_free(struct osmo_pcap_conn *conn) { + struct osmo_pcap_wr_file *wrf; osmo_pcap_conn_close(conn); + /* We are freeing, make sure all files are processed even if we may be losing some data... */ + while ((wrf = llist_first_entry_or_null(&conn->wrf_flushing_list, struct osmo_pcap_wr_file, entry))) + osmo_pcap_wr_file_flush_completed_cb(wrf, conn); llist_del(&conn->entry); talloc_free(conn); } @@ -237,16 +271,8 @@ if (!conn->wrf) return; - osmo_pcap_wr_file_close(conn->wrf); - - if (conn->server->completed_path) - osmo_pcap_wr_file_move_to_dir(conn->wrf, conn->server->completed_path); - - 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); + osmo_pcap_wr_file_flush(conn->wrf, &conn->wrf_flushing_list); + /* conn->wrf may have been freed or moved to conn->wrf_flushing_list: */ conn->wrf = NULL; } @@ -298,9 +324,6 @@ /* omit any storing/creation of the file */ if (!conn->store) { 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; } @@ -323,16 +346,21 @@ } conn->wrf = osmo_pcap_wr_file_alloc(conn, conn); + osmo_pcap_wr_file_set_flush_completed_cb(conn->wrf, osmo_pcap_wr_file_flush_completed_cb); rc = osmo_pcap_wr_file_open(conn->wrf, curr_filename, conn->server->permission_mask); talloc_free(curr_filename); if (rc < 0) return; - rc = osmo_pcap_wr_file_write(conn->wrf, conn->file_hdr, conn->file_hdr_len); - if (rc != conn->file_hdr_len) { + /* TODO: get msgb from conn object stored: */ + struct msgb *msg = msgb_alloc_c(conn->wrf, conn->file_hdr_len, "local_iofd_hdr"); + memcpy(msgb_put(msg, conn->file_hdr_len), conn->file_hdr, conn->file_hdr_len); + + rc = osmo_pcap_wr_file_write_msgb(conn->wrf, msg); + if (rc < 0) { LOGP(DSERVER, LOGL_ERROR, "Failed to write the header: %d\n", errno); - osmo_pcap_wr_file_free(conn->wrf); - conn->wrf = NULL; + msgb_free(msg); + osmo_pcap_conn_close_trace(conn); return; } update_last_write(conn, now); @@ -512,9 +540,14 @@ if (!check_restart_pcap_max_size(conn, len)) check_restart_pcap_localtime(conn, now); - rc = osmo_pcap_wr_file_write(conn->wrf, data, len); + /* TODO: get msgb from caller: */ + struct msgb *msg = msgb_alloc_c(conn->wrf, len, "local_iofd_msg"); + memcpy(msgb_put(msg, len), data, len); + + rc = osmo_pcap_wr_file_write_msgb(conn->wrf, msg); if (rc < 0) { LOGP(DSERVER, LOGL_ERROR, "%s: Failed writing to file\n", conn->name); + msgb_free(msg); return -1; } update_last_write(conn, now); -- To view, visit https://gerrit.osmocom.org/c/osmo-pcap/+/39367?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-pcap Gerrit-Branch: master Gerrit-Change-Id: I098a9455a2a4cc626444e6fc13aa88c4cc9694f0 Gerrit-Change-Number: 39367 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

5 months

1
0
0 0

[M] Change in osmo-pcap[master]: server: zero-copy msgb from tcp read to file write

by pespin

Attention is currently required from: laforge. pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-pcap/+/39370?usp=email ) Change subject: server: zero-copy msgb from tcp read to file write ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcap/+/39370?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-pcap Gerrit-Branch: master Gerrit-Change-Id: Iae9b9eaec42ad8ec86d1a8144d676115fa057766 Gerrit-Change-Number: 39370 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Wed, 22 Jan 2025 12:19:51 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

5 months

1
0
0 0

[S] Change in libosmo-sigtran[master]: ipa: Split msgb generation into its own helper function

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/39395?usp=email ) Change subject: ipa: Split msgb generation into its own helper function ...................................................................... ipa: Split msgb generation into its own helper function This allows having a clearer picture when comparing against other protocol stacks like M3UA and SUA, since both have a sua_to_msg() and m3ua_to_msg() functions (which in turn call xua_to_msg()). This way ipa_tx_xua_as() also becomes much more similar to m3ua_tx_xua_as() and sua_tx_xua_as(). Change-Id: Ic0a405ab4d1811efea137167dcb08c9308a4d7e7 --- M src/ipa.c 1 file changed, 23 insertions(+), 13 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-sigtran refs/changes/95/39395/1 diff --git a/src/ipa.c b/src/ipa.c index d62d710..59dfa68 100644 --- a/src/ipa.c +++ b/src/ipa.c @@ -52,12 +52,7 @@ #include "ss7_internal.h" #include "xua_asp_fsm.h" - -/*! \brief Send a given xUA message via a given IPA "Application Server" - * \param[in] as Application Server through which to send \a xua - * \param[in] xua xUA message to be sent - * \return 0 on success; negative on error */ -int ipa_tx_xua_as(struct osmo_ss7_as *as, struct xua_msg *xua) +static struct msgb *ipa_to_msg(struct xua_msg *xua) { struct xua_msg_part *data_ie; struct m3ua_data_hdr *data_hdr; @@ -66,18 +61,16 @@ const uint8_t *src; uint8_t *dst; - OSMO_ASSERT(as->cfg.proto == OSMO_SS7_ASP_PROT_IPA); - /* we're actually only interested in the data part */ data_ie = xua_msg_find_tag(xua, M3UA_IEI_PROT_DATA); if (!data_ie || data_ie->len < sizeof(struct m3ua_data_hdr)) - return -1; + return NULL; data_hdr = (struct m3ua_data_hdr *) data_ie->dat; if (data_hdr->si != MTP_SI_SCCP) { - LOGPAS(as, DLSS7, LOGL_ERROR, "Cannot transmit non-SCCP SI (%u) to IPA peer\n", - data_hdr->si); - return -1; + LOGP(DLSS7, LOGL_ERROR, "Cannot transmit non-SCCP SI (%u) to IPA peer\n", + data_hdr->si); + return NULL; } /* and even the data part still has the header prepended */ @@ -87,7 +80,7 @@ /* sufficient headroom for osmo_ipa_msg_push_header() */ msg = ipa_msg_alloc(16); if (!msg) - return -1; + return NULL; dst = msgb_put(msg, src_len); memcpy(dst, src, src_len); @@ -95,6 +88,23 @@ /* TODO: if we ever need something beyond SCCP, we can use the * M3UA SIO to determine the protocol */ osmo_ipa_msg_push_header(msg, IPAC_PROTO_SCCP); + return msg; +} + +/*! \brief Send a given xUA message via a given IPA "Application Server" + * \param[in] as Application Server through which to send \a xua + * \param[in] xua xUA message to be sent + * \return 0 on success; negative on error */ +int ipa_tx_xua_as(struct osmo_ss7_as *as, struct xua_msg *xua) +{ + struct msgb *msg = ipa_to_msg(xua); + + OSMO_ASSERT(as->cfg.proto == OSMO_SS7_ASP_PROT_IPA); + + if (!msg) { + LOGPAS(as, DLSS7, LOGL_ERROR, "Error encoding IPA Msg\n"); + return -1; + } return xua_as_transmit_msg(as, msg); } -- To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/39395?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: libosmo-sigtran Gerrit-Branch: master Gerrit-Change-Id: Ic0a405ab4d1811efea137167dcb08c9308a4d7e7 Gerrit-Change-Number: 39395 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

5 months

1
0
0 0

[XS] Change in ...osmo-s1gw[master]: erab_fsm: fix copy-paste in a log message

by fixeria

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/39393?usp=email ) Change subject: erab_fsm: fix copy-paste in a log message ...................................................................... erab_fsm: fix copy-paste in a log message Change-Id: I50f78ac99ddb82244107ff9847acec64d2e9fce1 --- M src/erab_fsm.erl 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-s1gw refs/changes/93/39393/1 diff --git a/src/erab_fsm.erl b/src/erab_fsm.erl index b249cd4..0b4904d 100644 --- a/src/erab_fsm.erl +++ b/src/erab_fsm.erl @@ -336,7 +336,7 @@ session_delete(state_timeout, _Timeout, #erab_state{from = From}) -> - ?LOG_ERROR("PFCP session modification timeout"), + ?LOG_ERROR("PFCP session deletion timeout"), {stop_and_reply, {shutdown, timeout}, {reply, From, {error, {timeout, ?FUNCTION_NAME}}}}; -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/39393?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: erlang/osmo-s1gw Gerrit-Branch: master Gerrit-Change-Id: I50f78ac99ddb82244107ff9847acec64d2e9fce1 Gerrit-Change-Number: 39393 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>

5 months

1
0
0 0

[XS] Change in ...osmo-s1gw[master]: erab_fsm: add more logging to clarify timeout events

by fixeria

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/39394?usp=email ) Change subject: erab_fsm: add more logging to clarify timeout events ...................................................................... erab_fsm: add more logging to clarify timeout events Change-Id: Iad64ce88e747951c0699074d95d504ccf9f61603 --- M src/erab_fsm.erl 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-s1gw refs/changes/94/39394/1 diff --git a/src/erab_fsm.erl b/src/erab_fsm.erl index 0b4904d..ea1c11b 100644 --- a/src/erab_fsm.erl +++ b/src/erab_fsm.erl @@ -249,6 +249,7 @@ [{state_timeout, ?ERAB_T_WAIT_SETUP_RSP, ?FUNCTION_NAME}]}; erab_wait_setup_rsp(state_timeout, _Timeout, _S) -> + ?LOG_ERROR("Timeout waiting for E-RAB SETUP Rsp"), {stop, {shutdown, timeout}}; erab_wait_setup_rsp({call, From}, @@ -377,6 +378,7 @@ [{state_timeout, ?ERAB_T_WAIT_RELEASE_RSP, ?FUNCTION_NAME}]}; erab_wait_release_rsp(state_timeout, _Timeout, _S) -> + ?LOG_ERROR("Timeout waiting for E-RAB RELEASE Rsp"), {stop, {shutdown, timeout}}; erab_wait_release_rsp(cast, erab_release_rsp, -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/39394?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: erlang/osmo-s1gw Gerrit-Branch: master Gerrit-Change-Id: Iad64ce88e747951c0699074d95d504ccf9f61603 Gerrit-Change-Number: 39394 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>

5 months

1
0
0 0

[M] Change in osmo-pcap[master]: server: zero-copy msgb from tcp read to file write

by fixeria

Attention is currently required from: laforge, pespin. fixeria has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-pcap/+/39370?usp=email ) Change subject: server: zero-copy msgb from tcp read to file write ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcap/+/39370?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-pcap Gerrit-Branch: master Gerrit-Change-Id: Iae9b9eaec42ad8ec86d1a8144d676115fa057766 Gerrit-Change-Number: 39370 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 22 Jan 2025 12:11:12 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

5 months

1
0
0 0

[S] Change in osmocom-bb[master]: trx_toolkit/clck_gen: Fix clock generator to emit ticks with exactly ...

by fixeria

fixeria has submitted this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/39392?usp=email ) Change subject: trx_toolkit/clck_gen: Fix clock generator to emit ticks with exactly GSM frame period ...................................................................... trx_toolkit/clck_gen: Fix clock generator to emit ticks with exactly GSM frame period Since fake_trx beginning in 3187c8e6 (target/fake_trx: initial release of virtual transceiver) CLCKGen was tuned to emitting ticks with sleep period being time of 1 GSM frame _decreased_ a bit by "Average loop back delay". The idea for this decrease probably was to compensate the time spent in each tick handler, so that combined sleep + tick work would occupy time of 1 GSM frame more or less. The idea of using hardcoded compensation turned out to be not very good, because for the overall tick period to be exactly as defined the compensation should be dynamic and take into account time spent in each tick handler. For example on one machine "loopback delay" is one value, while on another it will be another value. And if we attach more work to tick handler, like it already happened with adding tx queue, the compensation needs to take all that into account as well. abc63d8d (trx_toolkit/clck_gen.py: Fix clock generator not to accumulate timing error) explains the problem in detail and adds dynamic compensation so that the tick period stays as defined instead of drifting. But it missed to adjust CLCKgen to stop decreasing desired tick period a bit by "average loop back delay". So after that patch, because CLCKgen now follows desired period without drifting, its period was 4.615ms - 0.09ms instead of exact 4.615ms, which resulted in e.g. fake_trx and bts-trx clocks to become constantly dissynchronized with the following emitted by bts-trx non-stop: 20250122135431420 <0006> scheduler_trx.c:576 GSM clock skew: old fn=0, new fn=102 20250122135431882 <0006> scheduler_trx.c:604 We were 3 FN slower than TRX, compensated 20250122135432344 <0006> scheduler_trx.c:604 We were 2 FN slower than TRX, compensated 20250122135432805 <0006> scheduler_trx.c:604 We were 2 FN slower than TRX, compensated 20250122135433267 <0006> scheduler_trx.c:604 We were 2 FN slower than TRX, compensated 20250122135433728 <0006> scheduler_trx.c:604 We were 2 FN slower than TRX, compensated 20250122135434190 <0006> scheduler_trx.c:604 We were 2 FN slower than TRX, compensated 20250122135434651 <0006> scheduler_trx.c:604 We were 2 FN slower than TRX, compensated 20250122135435113 <0006> scheduler_trx.c:604 We were 2 FN slower than TRX, compensated 20250122135435575 <0006> scheduler_trx.c:604 We were 2 FN slower than TRX, compensated 20250122135436036 <0006> scheduler_trx.c:604 We were 2 FN slower than TRX, compensated 20250122135436498 <0006> scheduler_trx.c:604 We were 2 FN slower than TRX, compensated 20250122135436959 <0006> scheduler_trx.c:604 We were 2 FN slower than TRX, compensated 20250122135437421 <0006> scheduler_trx.c:604 We were 2 FN slower than TRX, compensated ... What happens here is that there are ~ 216 GSM frames every second, and since fake_trx drifts by 0.09ms every frame, it results in drifting by ~ 20ms every second. Which results in "2 FN slower than TRX" emitted approximately twice per second as above log excerpt confirms. -> Fix this by adjusting CLCKgen to emit ticks with exactly GSM frame period by default. Change-Id: Ie12fbe8138bac1398805fa270b869e7a333fcba0 --- M src/target/trx_toolkit/clck_gen.py M src/target/trx_toolkit/test_clck_gen.py 2 files changed, 6 insertions(+), 4 deletions(-) Approvals: fixeria: Looks good to me, approved Jenkins Builder: Verified pespin: Looks good to me, but someone else must approve diff --git a/src/target/trx_toolkit/clck_gen.py b/src/target/trx_toolkit/clck_gen.py index bf56f64..c6da8f6 100755 --- a/src/target/trx_toolkit/clck_gen.py +++ b/src/target/trx_toolkit/clck_gen.py @@ -34,9 +34,6 @@ SEC_DELAY_US = 1000 * 1000 GSM_FRAME_US = 4615.0 - # Average loop back delay - LO_DELAY_US = 90.0 - def __init__(self, clck_links, clck_start = 0, ind_period = 102): # This event is needed to control the thread self._breaker = threading.Event() @@ -47,7 +44,7 @@ self.clck_start = clck_start # Calculate counter time - self.ctr_interval = self.GSM_FRAME_US - self.LO_DELAY_US + self.ctr_interval = self.GSM_FRAME_US self.ctr_interval /= self.SEC_DELAY_US # (Optional) clock consumer diff --git a/src/target/trx_toolkit/test_clck_gen.py b/src/target/trx_toolkit/test_clck_gen.py index a920ce1..92e5e55 100644 --- a/src/target/trx_toolkit/test_clck_gen.py +++ b/src/target/trx_toolkit/test_clck_gen.py @@ -63,6 +63,11 @@ finally: clck.stop() + # verify that default tick interval is 1 TDMA frame exactly + def test_period_is_gsm_frame(self): + clck = clck_gen.CLCKGen([]) + self.assertEqual(clck.ctr_interval, 4.615e-3) + if __name__ == '__main__': unittest.main() -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/39392?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Ie12fbe8138bac1398805fa270b869e7a333fcba0 Gerrit-Change-Number: 39392 Gerrit-PatchSet: 1 Gerrit-Owner: kirr <kirr(a)nexedi.com> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: osmith <osmith(a)sysmocom.de>

5 months

1
0
0 0

2025

2024

2023

2022

gerrit-log January 2025