fixeria has submitted this change. ( https://gerrit.osmocom.org/c/osmo-trx/+/43110?usp=email )

(

4 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: libosmo-trx/client: make public API operate on parsed messages ......................................................................

libosmo-trx/client: make public API operate on parsed messages

The TRXC client engine and the TRX endpoint module did not compose: osmo_trx_ep_rx_ctrl_msg() delivers a parsed struct osmo_trxc_msg, while osmo_trxc_client_rx() accepts a raw datagram only; likewise, the tx_msg call-back used to emit a serialized string, while osmo_trx_ep_send_ctrl_msg() takes the parsed structure. An osmo_trx_ep user would have to needlessly re-parse and re-serialize TRXC messages when gluing both modules together.

Make struct osmo_trxc_msg the currency at both module boundaries:

* add osmo_trxc_client_rx_msg(), accepting a parsed message; osmo_trxc_client_rx() becomes a parse-first convenience wrapper for applications managing the TRXC socket themselves; * change the tx_msg call-back to take a parsed message: it's now up to the transport to serialize it (osmo_trx_ep users simply call osmo_trx_ep_send_ctrl_msg(), others osmo_trxc_msg_build()).

This is a preparation for porting trxcon to libosmo-trx.

Change-Id: Idf53513d06be2337383601494b225b5722c93129 --- M libosmo-trx/include/osmocom/trx/trxc_client.h M libosmo-trx/src/trxc_client.c M tests/libosmo-trx/trxc_client_test.c 3 files changed, 57 insertions(+), 44 deletions(-)

Approvals: laforge: Looks good to me, but someone else must approve pespin: Looks good to me, but someone else must approve Jenkins Builder: Verified fixeria: Looks good to me, approved

diff --git a/libosmo-trx/include/osmocom/trx/trxc_client.h b/libosmo-trx/include/osmocom/trx/trxc_client.h index 0f0d5ec..9e5de24 100644 --- a/libosmo-trx/include/osmocom/trx/trxc_client.h +++ b/libosmo-trx/include/osmocom/trx/trxc_client.h @@ -29,17 +29,16 @@ const struct osmo_trxc_msg *rsp, void *cb_data);

-/*! Transmit call-back, invoked to emit a serialized TRXC message (mandatory, +/*! Transmit call-back, invoked to emit a parsed TRXC message (mandatory, * see osmo_trxc_client_set_tx_msg_cb()). - * E.g. write() / osmo_iofd_write_msgb() on the app's ctrl socket. + * E.g. osmo_trx_ep_send_ctrl_msg() for osmo_trx_ep users, or + * osmo_trxc_msg_build() + write() on a self-managed ctrl socket. * \param[in] client TRXC client instance - * \param[in] buf serialized TRXC message to transmit - * \param[in] len length of buf, in bytes - * \returns number of bytes transmitted on success; negative on error - * (logged by the engine, otherwise ignored: the retransmit - * timer still governs delivery) */ + * \param[in] msg TRXC message to transmit + * \returns 0 on success; negative on error (logged by the engine, + * otherwise ignored: the retransmit timer still governs delivery) */ typedef int osmo_trxc_client_tx_msg_cb(struct osmo_trxc_client *client, - const char *buf, size_t len); + const struct osmo_trxc_msg *msg);

/*! Fatal error call-back, invoked when a critical command definitively fails * (optional; default: log), see osmo_trxc_client_set_fatal_error_cb(). @@ -75,6 +74,8 @@ const char *cmd, const char *fmt, ...); void osmo_trxc_client_flush(struct osmo_trxc_client *client);

+int osmo_trxc_client_rx_msg(struct osmo_trxc_client *client, + const struct osmo_trxc_msg *rsp); int osmo_trxc_client_rx(struct osmo_trxc_client *client, const char *buf, size_t len);

/*! TRXD PDU version negotiation result call-back. diff --git a/libosmo-trx/src/trxc_client.c b/libosmo-trx/src/trxc_client.c index fead54f..6b89c7c 100644 --- a/libosmo-trx/src/trxc_client.c +++ b/libosmo-trx/src/trxc_client.c @@ -85,7 +85,6 @@ /* Transmit the first command in the queue (if any), (re)start the timer */ static void trxc_client_send_next(struct osmo_trxc_client *client) { - char buf[OSMO_TRXC_MSG_BUF_SIZE]; struct trxc_cmd_entry *e; int rc;

@@ -93,12 +92,9 @@ return; e = llist_first_entry(&client->cmd_queue, struct trxc_cmd_entry, list);

- rc = osmo_trxc_msg_build(buf, sizeof(buf), &e->msg); - OSMO_ASSERT(rc > 0); /* validated in osmo_trxc_client_send_cmd() */ - OSMO_ASSERT(client->tx_msg_cb != NULL); /* set via osmo_trxc_client_set_tx_msg_cb() */ - LOGCL(client, LOGL_DEBUG, "Tx '%s'\n", buf); - rc = client->tx_msg_cb(client, buf, rc); + LOGCL(client, LOGL_DEBUG, "Tx '%s'\n", osmo_trxc_msg_name(&e->msg)); + rc = client->tx_msg_cb(client, &e->msg); if (rc < 0) LOGCL(client, LOGL_ERROR, "tx_msg() failed with rc=%d\n", rc);

@@ -371,46 +367,39 @@ return -EINVAL; }

-/*! Feed a datagram received on the ctrl socket into the engine. +/*! Feed a received (parsed) response message into the engine. * - * To be called by the application for every datagram read from the TRXC - * socket. The engine parses the message, filters duplicate responses - * caused by retransmissions, matches the response against the command - * in flight, invokes its response call-back and transmits the next - * queued command (if any). + * The engine filters duplicate responses caused by retransmissions, + * matches the response against the command in flight, invokes its + * response call-back and transmits the next queued command (if any). + * E.g. an osmo_trx_ep user calls this from osmo_trx_ep_rx_ctrl_msg(). * * \param[in] client TRXC client instance - * \param[in] buf received datagram (not necessarily zero-terminated) - * \param[in] len length of the datagram + * \param[in] rsp received message (rsp->type must be OSMO_TRXC_MT_RSP) * \returns 0 on success; negative on error */ -int osmo_trxc_client_rx(struct osmo_trxc_client *client, const char *buf, size_t len) +int osmo_trxc_client_rx_msg(struct osmo_trxc_client *client, + const struct osmo_trxc_msg *rsp) { - struct osmo_trxc_msg rsp; struct trxc_cmd_entry *e; bool flushed; int rc;

- rc = osmo_trxc_msg_parse(&rsp, buf, len); - if (rc < 0) { - LOGCL(client, LOGL_NOTICE, "Rx malformed TRXC message (rc=%d)\n", rc); - return rc; - } - if (rsp.type != OSMO_TRXC_MT_RSP) { + if (rsp->type != OSMO_TRXC_MT_RSP) { LOGCL(client, LOGL_NOTICE, "Rx unexpected TRXC message '%s'\n", - osmo_trxc_msg_name(&rsp)); + osmo_trxc_msg_name(rsp)); return -EINVAL; }

- LOGCL(client, LOGL_INFO, "Rx '%s'\n", osmo_trxc_msg_name(&rsp)); + LOGCL(client, LOGL_INFO, "Rx '%s'\n", osmo_trxc_msg_name(rsp));

/* abort the retransmit timer */ osmo_timer_del(&client->retrans_timer);

if (llist_empty(&client->cmd_queue)) { /* a response from a retransmission, skip it */ - if (client->last_acked != NULL && cmd_matches_rsp(client->last_acked, &rsp)) { + if (client->last_acked != NULL && cmd_matches_rsp(client->last_acked, rsp)) { LOGCL(client, LOGL_NOTICE, "Discarding duplicate response '%s'\n", - osmo_trxc_msg_name(&rsp)); + osmo_trxc_msg_name(rsp)); return 0; } LOGCL(client, LOGL_NOTICE, "Rx response without a pending command\n"); @@ -419,11 +408,11 @@

e = llist_first_entry(&client->cmd_queue, struct trxc_cmd_entry, list);

- if (!cmd_matches_rsp(e, &rsp)) { + if (!cmd_matches_rsp(e, rsp)) { /* a response from a retransmission, skip it */ - if (client->last_acked != NULL && cmd_matches_rsp(client->last_acked, &rsp)) { + if (client->last_acked != NULL && cmd_matches_rsp(client->last_acked, rsp)) { LOGCL(client, LOGL_NOTICE, "Discarding duplicate response '%s'\n", - osmo_trxc_msg_name(&rsp)); + osmo_trxc_msg_name(rsp)); /* the command in flight still awaits its response */ osmo_timer_schedule(&client->retrans_timer, client->retrans_sec, 0); return 0; @@ -431,10 +420,10 @@

LOGCL(client, (e->flags & OSMO_TRXC_F_CRITICAL) ? LOGL_FATAL : LOGL_NOTICE, "Response '%s' does not match pending '" CMD_NAME_FMT "'\n", - osmo_trxc_msg_name(&rsp), CMD_NAME_ARGS(e)); + osmo_trxc_msg_name(rsp), CMD_NAME_ARGS(e));

if (e->flags & OSMO_TRXC_F_CRITICAL) - return trxc_client_fatal(client, &rsp); + return trxc_client_fatal(client, rsp);

/* We may get 'RSP ERR 1' for non-critical commands not * supported by the transceiver. Deliver such responses to @@ -447,15 +436,15 @@

client->in_rx = true; if (e->rsp_cb != NULL) - rc = e->rsp_cb(client, &rsp, e->cb_data); + rc = e->rsp_cb(client, rsp, e->cb_data); else - rc = trxc_client_default_rsp_cb(client, e, &rsp); + rc = trxc_client_default_rsp_cb(client, e, rsp); flushed = client->flushed_in_rx; client->flushed_in_rx = false; client->in_rx = false;

if (rc < 0) - return trxc_client_fatal(client, &rsp); + return trxc_client_fatal(client, rsp);

/* the call-back requested a re-transmission in rc seconds */ if (rc > 0) { @@ -478,6 +467,29 @@ return 0; }

+/*! Feed a datagram received on the ctrl socket into the engine. + * + * A convenience wrapper around osmo_trxc_client_rx_msg() for applications + * managing the TRXC socket themselves: parses the given datagram first. + * + * \param[in] client TRXC client instance + * \param[in] buf received datagram (not necessarily zero-terminated) + * \param[in] len length of the datagram + * \returns 0 on success; negative on error */ +int osmo_trxc_client_rx(struct osmo_trxc_client *client, const char *buf, size_t len) +{ + struct osmo_trxc_msg rsp; + int rc; + + rc = osmo_trxc_msg_parse(&rsp, buf, len); + if (rc < 0) { + LOGCL(client, LOGL_NOTICE, "Rx malformed TRXC message (rc=%d)\n", rc); + return rc; + } + + return osmo_trxc_client_rx_msg(client, &rsp); +} + /*********************************************************************** * TRXD PDU version negotiation (SETFORMAT) ***********************************************************************/ diff --git a/tests/libosmo-trx/trxc_client_test.c b/tests/libosmo-trx/trxc_client_test.c index 2773b2e..9a9fadd 100644 --- a/tests/libosmo-trx/trxc_client_test.c +++ b/tests/libosmo-trx/trxc_client_test.c @@ -58,9 +58,9 @@ osmo_timers_update(); }

-static int tx_msg_cb(struct osmo_trxc_client *client, const char *buf, size_t len) +static int tx_msg_cb(struct osmo_trxc_client *client, const struct osmo_trxc_msg *msg) { - printf("tx_msg: '%s'\n", buf); + printf("tx_msg: '%s'\n", osmo_trxc_msg_name(msg)); return 0; }