fixeria submitted this change.

View Change



4 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.

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
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(-)

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;
}


To view, visit change 43110. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-MessageType: merged
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Change-Id: Idf53513d06be2337383601494b225b5722c93129
Gerrit-Change-Number: 43110
Gerrit-PatchSet: 6
Gerrit-Owner: fixeria <vyanitskiy@sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de>
Gerrit-Reviewer: laforge <laforge@osmocom.org>
Gerrit-Reviewer: pespin <pespin@sysmocom.de>