pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bts/+/41650?usp=email )
Change subject: bts-trx: Convert TRXC and TRXD sockets to iofd ......................................................................
bts-trx: Convert TRXC and TRXD sockets to iofd
Since now the Tx side is driven by the event loop, we can use (and should) use OSMO_SOCK_F_NONBLOCK on the socket, avoiding potential blocking of the entire process.
We also gain io_uring support for free, which is a really nice feature to have in TRXD.
Related: OS#1579 Change-Id: I239f91efad43eabd280caf9f852c3aefbc729eaf --- M src/osmo-bts-trx/l1_if.h M src/osmo-bts-trx/trx_if.c 2 files changed, 143 insertions(+), 124 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/50/41650/1
diff --git a/src/osmo-bts-trx/l1_if.h b/src/osmo-bts-trx/l1_if.h index 84fd4b5..7cc82ad 100644 --- a/src/osmo-bts-trx/l1_if.h +++ b/src/osmo-bts-trx/l1_if.h @@ -2,6 +2,7 @@ #define L1_IF_H_TRX
#include <osmocom/core/rate_ctr.h> +#include <osmocom/core/osmo_io.h>
#include <osmo-bts/scheduler.h> #include <osmo-bts/phy_link.h> @@ -128,9 +129,9 @@ //struct gsm_bts_trx *trx; struct phy_instance *phy_inst;
- struct osmo_fd trx_ofd_ctrl; + struct osmo_io_fd *trx_ctrl_iofd; struct osmo_timer_list trx_ctrl_timer; - struct osmo_fd trx_ofd_data; + struct osmo_io_fd *trx_data_iofd;
/* transceiver config */ struct trx_config config; diff --git a/src/osmo-bts-trx/trx_if.c b/src/osmo-bts-trx/trx_if.c index 91961f4..bf0ab09 100644 --- a/src/osmo-bts-trx/trx_if.c +++ b/src/osmo-bts-trx/trx_if.c @@ -65,42 +65,6 @@ #endif /* HAVE_SYSTEMTAP */
/* - * socket helper functions - */ - -/*! convenience wrapper to open socket + fill in osmo_fd */ -static int trx_udp_open(void *priv, struct osmo_fd *ofd, const char *host_local, - uint16_t port_local, const char *host_remote, uint16_t port_remote, - int (*cb)(struct osmo_fd *fd, unsigned int what)) -{ - int rc; - - /* Init */ - ofd->fd = -1; - ofd->cb = cb; - ofd->data = priv; - - /* Listen / Binds + Connect */ - rc = osmo_sock_init2_ofd(ofd, AF_UNSPEC, SOCK_DGRAM, IPPROTO_UDP, host_local, port_local, - host_remote, port_remote, OSMO_SOCK_F_BIND | OSMO_SOCK_F_CONNECT); - if (rc < 0) - return rc; - - return 0; -} - -/* close socket + unregister osmo_fd */ -static void trx_udp_close(struct osmo_fd *ofd) -{ - if (ofd->fd >= 0) { - osmo_fd_unregister(ofd); - close(ofd->fd); - ofd->fd = -1; - } -} - - -/* * TRX clock socket */
@@ -169,25 +133,30 @@ static void trx_ctrl_send(struct trx_l1h *l1h) { struct trx_ctrl_msg *tcm; - char buf[TRXC_MSG_BUF_SIZE]; - int len; - ssize_t snd_len; + char *buf; + struct msgb *msg; + int len, rc;
/* get first command */ if (llist_empty(&l1h->trx_ctrl_list)) return; tcm = llist_entry(l1h->trx_ctrl_list.next, struct trx_ctrl_msg, list);
- len = snprintf(buf, sizeof(buf), "CMD %s%s%s", tcm->cmd, tcm->params_len ? " ":"", tcm->params); - OSMO_ASSERT(len < sizeof(buf)); + msg = msgb_alloc(TRXC_MSG_BUF_SIZE, "trxc_cmd"); + buf = (char *)msgb_data(msg); + len = snprintf(buf, msg->data_len, "CMD %s%s%s", tcm->cmd, tcm->params_len ? " ":"", tcm->params); + OSMO_ASSERT(len < msg->data_len); + msgb_put(msg, len);
LOGPPHI(l1h->phy_inst, DTRX, LOGL_DEBUG, "Sending control '%s'\n", buf); /* send command */ - snd_len = send(l1h->trx_ofd_ctrl.fd, buf, len+1, 0); - if (snd_len <= 0) { - strerror_r(errno, (char *)buf, sizeof(buf)); + rc = osmo_iofd_write_msgb(l1h->trx_ctrl_iofd, msg); + if (rc < 0) { + char errbuf[256]; + strerror_r(errno, errbuf, sizeof(errbuf)); LOGPPHI(l1h->phy_inst, DTRX, LOGL_ERROR, - "send() failed on TRXC with rc=%zd (%s)\n", snd_len, buf); + "send() failed on TRXC with rc=%d (%s)\n", rc, errbuf); + msgb_free(msg); }
/* start timer */ @@ -217,9 +186,6 @@
/* initialize ctrl queue */ INIT_LLIST_HEAD(&l1h->trx_ctrl_list); - - l1h->trx_ofd_ctrl.fd = -1; - l1h->trx_ofd_data.fd = -1; }
/*! Send a new TRX control command. @@ -673,23 +639,24 @@ }
/*! Get + parse response from TRX ctrl socket */ -static int trx_ctrl_read_cb(struct osmo_fd *ofd, unsigned int what) +static void trx_ctrl_read_cb(struct osmo_io_fd *iofd, int res, struct msgb *msg) { - struct trx_l1h *l1h = ofd->data; + struct trx_l1h *l1h = osmo_iofd_get_data(iofd); struct phy_instance *pinst = l1h->phy_inst; - char buf[TRXC_MSG_BUF_SIZE]; + const char *buf; struct trx_ctrl_rsp rsp; - int len, rc; + int rc; struct trx_ctrl_msg *tcm; bool flushed;
- len = recv(ofd->fd, buf, sizeof(buf) - 1, 0); - if (len <= 0) - return len; - buf[len] = '\0'; + if (res <= 0) + goto ret_free_msg;
- if (parse_rsp(buf, len, &rsp) < 0) - return 0; + msgb_put_u8(msg, (uint8_t)'\0'); + buf = (char *)msgb_data(msg); + + if (parse_rsp(buf, res, &rsp) < 0) + goto ret_free_msg;
LOGPPHI(l1h->phy_inst, DTRX, LOGL_INFO, "Response message: '%s'\n", buf);
@@ -702,13 +669,12 @@ if (l1h->last_acked && cmd_matches_rsp(l1h->last_acked, &rsp)) { LOGPPHI(l1h->phy_inst, DTRX, LOGL_NOTICE, "Discarding duplicated RSP " "from old CMD '%s'\n", buf); - return 0; + goto ret_free_msg; } LOGPPHI(l1h->phy_inst, DTRX, LOGL_NOTICE, "Response message without command\n"); - return -EINVAL; + goto ret_free_msg; } - tcm = llist_entry(l1h->trx_ctrl_list.next, struct trx_ctrl_msg, - list); + tcm = llist_entry(l1h->trx_ctrl_list.next, struct trx_ctrl_msg, list);
/* check if response matches command */ if (!cmd_matches_rsp(tcm, &rsp)) { @@ -716,7 +682,7 @@ if (l1h->last_acked && cmd_matches_rsp(l1h->last_acked, &rsp)) { LOGPPHI(l1h->phy_inst, DTRX, LOGL_NOTICE, "Discarding duplicated RSP " "from old CMD '%s'\n", buf); - return 0; + goto ret_free_msg; } LOGPPHI(l1h->phy_inst, DTRX, (tcm->critical) ? LOGL_FATAL : LOGL_NOTICE, "Response message '%s' does not match command " @@ -746,7 +712,7 @@ /* The queue may have been flushed in the trx_ctrl_rx_rsp(): */ if (!llist_empty(&l1h->trx_ctrl_list)) osmo_timer_schedule(&l1h->trx_ctrl_timer, rc, 0); - return 0; + goto ret_free_msg; }
if (!flushed) { @@ -760,16 +726,22 @@
/* Send next message waiting in the list: */ trx_ctrl_send(l1h); - - return 0; + msgb_free(msg); + return;
rsp_error: bts_shutdown(pinst->trx->bts, "TRX-CTRL-MSG: CRITICAL"); /* keep tcm list, so process is stopped */ - return -EIO; +ret_free_msg: + msgb_free(msg); }
+static void trx_ctrl_write_cb(struct osmo_io_fd *iofd, int res, struct msgb *msg) +{ + /* nothing to be done here */ +} + /* * TRX burst data socket */ @@ -1019,26 +991,24 @@ return buf; }
-/* TRXD buffer used by Rx/Tx handlers */ -static uint8_t trx_data_buf[TRXD_MSG_BUF_SIZE]; - /* Parse TRXD message from transceiver, compose an UL burst indication. */ -static int trx_data_read_cb(struct osmo_fd *ofd, unsigned int what) +static void trx_data_read_cb(struct osmo_io_fd *iofd, int res, struct msgb *msg) { - const uint8_t *buf = &trx_data_buf[0]; - struct trx_l1h *l1h = ofd->data; + struct trx_l1h *l1h = osmo_iofd_get_data(iofd); + const uint8_t *buf; struct trx_ul_burst_ind bi; ssize_t hdr_len, buf_len; uint8_t pdu_ver;
- buf_len = recv(ofd->fd, trx_data_buf, sizeof(trx_data_buf), 0); - if (OSMO_UNLIKELY(buf_len <= 0)) { - strerror_r(errno, (char *) trx_data_buf, sizeof(trx_data_buf)); + if (OSMO_UNLIKELY(res <= 0)) { + char errbuf[256]; + strerror_r(errno, errbuf, sizeof(errbuf)); LOGPPHI(l1h->phy_inst, DTRX, LOGL_ERROR, - "recv() failed on TRXD with rc=%zd (%s)\n", - buf_len, trx_data_buf); - return buf_len; + "recv() failed on TRXD with rc=%d\n", res); + goto ret_msg_free; } + buf = msgb_data(msg); + buf_len = msgb_length(msg);
/* Parse PDU version first */ pdu_ver = buf[0] >> 4; @@ -1048,7 +1018,7 @@ LOGPPHI(l1h->phy_inst, DTRX, LOGL_ERROR, "Rx TRXD PDU with unexpected version %u (expected %u)\n", pdu_ver, l1h->config.trxd_pdu_ver_use); - return -EIO; + goto ret_msg_free; }
/* We're about to parse the first PDU */ @@ -1064,7 +1034,7 @@ LOGPPHI(l1h->phy_inst, DTRX, LOGL_ERROR, "Rx malformed TRXDv%u PDU: len=%zd < expected %u\n", pdu_ver, buf_len, trx_data_rx_hdr_len[pdu_ver]); - return -EINVAL; + goto ret_msg_free; }
/* Parse header depending on the PDU version */ @@ -1085,13 +1055,13 @@
/* Header parsing error */ if (OSMO_UNLIKELY(hdr_len < 0)) - return hdr_len; + goto ret_msg_free;
if (OSMO_UNLIKELY(bi.fn >= GSM_TDMA_HYPERFRAME)) { LOGPPHI(l1h->phy_inst, DTRX, LOGL_ERROR, "Rx malformed TRXDv%u PDU: illegal TDMA fn=%u\n", pdu_ver, bi.fn); - return -EINVAL; + goto ret_msg_free; }
/* We're done with the header now */ @@ -1103,7 +1073,7 @@ LOGPPHI(l1h->phy_inst, DTRX, LOGL_ERROR, "Rx malformed TRXDv%u PDU: odd burst length=%zd\n", pdu_ver, buf_len); - return -EINVAL; + goto ret_msg_free; }
/* We're done with the burst bits now */ @@ -1124,7 +1094,13 @@ TRACE(OSMO_BTS_TRX_UL_DATA_DONE(l1h->phy_inst->trx->nr, bi.tn, bi.fn)); } while (bi.flags & TRX_BI_F_BATCH_IND);
- return 0; +ret_msg_free: + msgb_free(msg); +} + +static void trx_data_write_cb(struct osmo_io_fd *iofd, int res, struct msgb *msg) +{ + /* nothing to be done here */ }
/*! Send burst data for given FN/timeslot to TRX @@ -1134,10 +1110,10 @@ int trx_if_send_burst(struct trx_l1h *l1h, const struct trx_dl_burst_req *br) { uint8_t pdu_ver = l1h->config.trxd_pdu_ver_use; - static uint8_t *buf = &trx_data_buf[0]; - static uint8_t *last_pdu = NULL; + static struct msgb *trx_data_last_msg = NULL; static unsigned int pdu_num = 0; - ssize_t snd_len, buf_len; + uint8_t *buf; + int rc;
/* Make sure that the PHY is powered on */ if (OSMO_UNLIKELY(!trx_if_powered(l1h))) { @@ -1146,6 +1122,12 @@ return -ENODEV; }
+ if (!trx_data_last_msg) { + trx_data_last_msg = msgb_alloc(TRXD_MSG_BUF_SIZE, "tx_trxd"); + OSMO_ASSERT(trx_data_last_msg); + buf = msgb_data(trx_data_last_msg); + } + /* Burst batching breaker */ if (br == NULL) { if (pdu_num > 0) @@ -1153,19 +1135,20 @@ return -ENOMSG; }
- /* Pointer to the last encoded PDU */ - last_pdu = &buf[0]; + /* l2h holds Pointer to the last encoded PDU */ + trx_data_last_msg->l2h = trx_data_last_msg->tail;
switch (pdu_ver) { /* Both versions have the same PDU format */ case 0: /* TRXDv0 */ case 1: /* TRXDv1 */ + buf = (uint8_t *)msgb_put(trx_data_last_msg, 6); buf[0] = ((pdu_ver & 0x0f) << 4) | br->tn; osmo_store32be(br->fn, buf + 1); buf[5] = br->att; - buf += 6; break; case 2: /* TRXDv2 */ + buf = (uint8_t *)msgb_put(trx_data_last_msg, 8); buf[0] = br->tn; /* BATCH.ind will be unset in the last PDU */ buf[1] = (br->trx_num & 0x3f) | (1 << 7); @@ -1178,10 +1161,8 @@ /* Some fields are not present in batched PDUs */ if (pdu_num == 0) { buf[0] |= (pdu_ver & 0x0f) << 4; - osmo_store32be(br->fn, buf + 8); - buf += 4; + osmo_store32be(br->fn, (uint8_t *)msgb_put(trx_data_last_msg, 4)); } - buf += 8; break; default: /* Shall not happen */ @@ -1189,8 +1170,7 @@ }
/* copy ubits {0,1} */ - memcpy(buf, br->burst, br->burst_len); - buf += br->burst_len; + memcpy(msgb_put(trx_data_last_msg, br->burst_len), br->burst, br->burst_len);
/* One more PDU in the buffer */ pdu_num++; @@ -1206,20 +1186,19 @@
/* TRXDv2: unset BATCH.ind in the last PDU */ if (pdu_ver >= 2) - last_pdu[1] &= ~(1 << 7); + trx_data_last_msg->l2h[1] &= ~(1 << 7);
- buf_len = buf - &trx_data_buf[0]; - buf = &trx_data_buf[0]; - pdu_num = 0; - - snd_len = send(l1h->trx_ofd_data.fd, trx_data_buf, buf_len, 0); - if (OSMO_UNLIKELY(snd_len <= 0)) { - strerror_r(errno, (char *) trx_data_buf, sizeof(trx_data_buf)); + rc = osmo_iofd_write_msgb(l1h->trx_data_iofd, trx_data_last_msg); + if (OSMO_UNLIKELY(rc < 0)) { + char errbuf[256]; + strerror_r(errno, errbuf, sizeof(errbuf)); LOGPPHI(l1h->phy_inst, DTRX, LOGL_ERROR, - "send() failed on TRXD with rc=%zd (%s)\n", - snd_len, trx_data_buf); - return -2; + "send() failed on TRXD with rc=%d (%s)\n", + rc, errbuf); + msgb_free(trx_data_last_msg); } + trx_data_last_msg = NULL; + pdu_num = 0;
return 0; } @@ -1261,8 +1240,10 @@ trx_if_flush(l1h);
/* close sockets */ - trx_udp_close(&l1h->trx_ofd_ctrl); - trx_udp_close(&l1h->trx_ofd_data); + osmo_iofd_free(l1h->trx_ctrl_iofd); + l1h->trx_ctrl_iofd = NULL; + osmo_iofd_free(l1h->trx_data_iofd); + l1h->trx_data_iofd = NULL; }
/*! compute UDP port number used for TRX protocol */ @@ -1280,6 +1261,16 @@ return plink->u.osmotrx.base_port_local + (pinst->num << 1) + inc; }
+static const struct osmo_io_ops trx_ctrl_ioops = { + .read_cb = trx_ctrl_read_cb, + .write_cb = trx_ctrl_write_cb, +}; + +static const struct osmo_io_ops trx_data_ioops = { + .read_cb = trx_data_read_cb, + .write_cb = trx_data_write_cb, +}; + static const struct osmo_io_ops trx_clk_ioops = { .read_cb = trx_clk_read_cb, .write_cb = trx_clk_write_cb, @@ -1292,25 +1283,52 @@ struct phy_link *plink = pinst->phy_link; int rc;
+ unsigned int flags = OSMO_SOCK_F_BIND | OSMO_SOCK_F_CONNECT | OSMO_SOCK_F_NONBLOCK; + LOGPPHI(pinst, DTRX, LOGL_NOTICE, "Opening TRXC/TRXD connections to %s\n", plink->u.osmotrx.remote_ip);
- /* open sockets */ - rc = trx_udp_open(l1h, &l1h->trx_ofd_ctrl, - plink->u.osmotrx.local_ip, - compute_port(pinst, false, false), - plink->u.osmotrx.remote_ip, - compute_port(pinst, true, false), trx_ctrl_read_cb); - if (rc < 0) - return rc; - rc = trx_udp_open(l1h, &l1h->trx_ofd_data, - plink->u.osmotrx.local_ip, - compute_port(pinst, false, true), - plink->u.osmotrx.remote_ip, - compute_port(pinst, true, true), trx_data_read_cb); + /* open TRXC socket */ + rc = osmo_sock_init2(AF_UNSPEC, SOCK_DGRAM, IPPROTO_UDP, + plink->u.osmotrx.local_ip, + compute_port(pinst, false, false), + plink->u.osmotrx.remote_ip, + compute_port(pinst, true, false), + flags); if (rc < 0) return rc;
+ l1h->trx_ctrl_iofd = osmo_iofd_setup(l1h, rc, NULL, OSMO_IO_FD_MODE_READ_WRITE, &trx_ctrl_ioops, l1h); + if (!l1h->trx_ctrl_iofd) { + close(rc); + return -ENOMEDIUM; + } + osmo_iofd_set_alloc_info(l1h->trx_ctrl_iofd, TRXC_MSG_BUF_SIZE, 0); + + /* open TRXD socket */ + rc = osmo_sock_init2(AF_UNSPEC, SOCK_DGRAM, IPPROTO_UDP, + plink->u.osmotrx.local_ip, + compute_port(pinst, false, true), + plink->u.osmotrx.remote_ip, + compute_port(pinst, true, true), + flags); + if (rc < 0) + goto ret_close_trxc; + l1h->trx_data_iofd = osmo_iofd_setup(l1h, rc, NULL, OSMO_IO_FD_MODE_READ_WRITE, &trx_data_ioops, l1h); + if (!l1h->trx_ctrl_iofd) { + close(rc); + goto ret_close_trxc; + } + osmo_iofd_set_alloc_info(l1h->trx_data_iofd, TRXD_MSG_BUF_SIZE, 0); + + /* register sockets */ + osmo_iofd_register(l1h->trx_ctrl_iofd, -1); + osmo_iofd_register(l1h->trx_data_iofd, -1); return 0; + +ret_close_trxc: + osmo_iofd_free(l1h->trx_ctrl_iofd); + l1h->trx_ctrl_iofd = NULL; + return rc; }
/*! close the control + burst data sockets for one phy_instance */