[MERGED] osmocom-bb[fixeria/trx]: trxcon: clarify L1CTL message length field

[Harald Welte]

Harald Welte has submitted this change and it was merged.

Change subject: trxcon: clarify L1CTL message length field
......................................................................


trxcon: clarify L1CTL message length field

Each L1CTL message gets its own length pushed in front before
sending. This isn't specified in the 'l1ctl_proto.h', but
assumed in the code. Let's clarify this.

Change-Id: I118d00613aeaf5ff0bad1188fa5f7450d4ca8122
---
M src/host/trxcon/l1ctl.c
M src/host/trxcon/l1ctl_link.c
M src/host/trxcon/l1ctl_link.h
3 files changed, 19 insertions(+), 5 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/host/trxcon/l1ctl.c b/src/host/trxcon/l1ctl.c
index 58b8a4f..f138b88 100644
--- a/src/host/trxcon/l1ctl.c
+++ b/src/host/trxcon/l1ctl.c
@@ -49,8 +49,14 @@
 static struct msgb *l1ctl_alloc_msg(uint8_t msg_type)
 {
 	struct l1ctl_hdr *l1h;
-	struct msgb *msg = msgb_alloc_headroom(256, 4, "l1ctl_tx_msg");
+	struct msgb *msg;
 
+	/**
+	 * Each L1CTL message gets its own length pushed in front
+	 * before sending. This is why we need this small headroom.
+	 */
+	msg = msgb_alloc_headroom(L1CTL_LENGTH + L1CTL_MSG_LEN_FIELD,
+		L1CTL_MSG_LEN_FIELD, "l1ctl_tx_msg");
 	if (!msg) {
 		LOGP(DL1C, LOGL_ERROR, "Failed to allocate memory\n");
 		return NULL;
diff --git a/src/host/trxcon/l1ctl_link.c b/src/host/trxcon/l1ctl_link.c
index a7277ea..0fa3efe 100644
--- a/src/host/trxcon/l1ctl_link.c
+++ b/src/host/trxcon/l1ctl_link.c
@@ -84,8 +84,8 @@
 	}
 
 	/* Attempt to read from socket */
-	rc = read(bfd->fd, &len, sizeof(len));
-	if (rc < sizeof(len)) {
+	rc = read(bfd->fd, &len, L1CTL_MSG_LEN_FIELD);
+	if (rc < L1CTL_MSG_LEN_FIELD) {
 		LOGP(DL1D, LOGL_NOTICE, "L1CTL has lost connection\n");
 		msgb_free(msg);
 		if (rc >= 0)
@@ -198,8 +198,8 @@
 		LOGP(DL1D, LOGL_INFO, "Message L1 header != Message Data\n");
 
 	/* Prepend 16-bit length before sending */
-	len = (uint16_t *) msgb_push(msg, sizeof(*len));
-	*len = htons(msg->len - sizeof(*len));
+	len = (uint16_t *) msgb_push(msg, L1CTL_MSG_LEN_FIELD);
+	*len = htons(msg->len - L1CTL_MSG_LEN_FIELD);
 
 	if (osmo_wqueue_enqueue(&l1l->wq, msg) != 0) {
 		LOGP(DL1D, LOGL_ERROR, "Failed to enqueue msg!\n");
diff --git a/src/host/trxcon/l1ctl_link.h b/src/host/trxcon/l1ctl_link.h
index f9d91f1..01103dc 100644
--- a/src/host/trxcon/l1ctl_link.h
+++ b/src/host/trxcon/l1ctl_link.h
@@ -1,5 +1,7 @@
 #pragma once
 
+#include <stdint.h>
+
 #include <osmocom/core/write_queue.h>
 #include <osmocom/core/select.h>
 #include <osmocom/core/timer.h>
@@ -9,6 +11,12 @@
 #define L1CTL_LENGTH 256
 #define L1CTL_HEADROOM 32
 
+/**
+ * Each L1CTL message gets its own length pushed
+ * as two bytes in front before sending.
+ */
+#define L1CTL_MSG_LEN_FIELD 2
+
 /* Forward declaration to avoid mutual include */
 struct trx_instance;
 

-- 
To view, visit https://gerrit.osmocom.org/7208
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I118d00613aeaf5ff0bad1188fa5f7450d4ca8122
Gerrit-PatchSet: 2
Gerrit-Project: osmocom-bb
Gerrit-Branch: fixeria/trx
Gerrit-Owner: Vadim Yanitskiy <axilirator at gmail.com>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder