[PATCH] osmocom-bb[master]: host/trxcon: integrate osmo-fsm framework

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Harald Welte gerrit-no-reply at lists.osmocom.org
Thu Feb 22 15:32:30 UTC 2018


Review at  https://gerrit.osmocom.org/6686

host/trxcon: integrate osmo-fsm framework

This change introduces the following state machines:

  - trxcon_app_fsm - main application state machine.

    This state machine handles different events, raised
    from program modules (such as trx_if.c or l1ctl.c).

  - l1ctl_link_fsm - L1CTL server state machine.

  - trx_interface_fsm - TRX interface state machine.

The program modules (such as trx_if.c or l1ctl.c) should be as
much independent from each other as possible. In other words,
one module should not call methods from another, e.g. L1CTL
handlers are not able to send any command to transceiver directly.

Instead of that, they should use shared event set to notify the
main state machine about something. Depending on current state
and received event, main state machine 'decides' what to do. This
approach would allow to easily reuse the source code almost 'as is'
anywhere outside the project.

Change-Id: I7ee6fc891abe5f775f5b7ebbf093181a97950dea
---
M src/host/trxcon/l1ctl_link.c
M src/host/trxcon/l1ctl_link.h
M src/host/trxcon/trx_if.c
M src/host/trxcon/trx_if.h
M src/host/trxcon/trxcon.c
A src/host/trxcon/trxcon.h
6 files changed, 238 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/86/6686/1

diff --git a/src/host/trxcon/l1ctl_link.c b/src/host/trxcon/l1ctl_link.c
index 62e8943..34aa4aa 100644
--- a/src/host/trxcon/l1ctl_link.c
+++ b/src/host/trxcon/l1ctl_link.c
@@ -34,15 +34,36 @@
 #include <arpa/inet.h>
 #include <sys/socket.h>
 
+#include <osmocom/core/fsm.h>
 #include <osmocom/core/talloc.h>
 #include <osmocom/core/select.h>
 #include <osmocom/core/socket.h>
 #include <osmocom/core/write_queue.h>
 
+#include "trxcon.h"
 #include "logging.h"
 #include "l1ctl_link.h"
 
 extern void *tall_trx_ctx;
+extern struct osmo_fsm_inst *trxcon_fsm;
+
+static struct osmo_fsm_state l1ctl_fsm_states[] = {
+	[L1CTL_STATE_IDLE] = {
+		.out_state_mask = GEN_MASK(L1CTL_STATE_CONNECTED),
+		.name = "IDLE",
+	},
+	[L1CTL_STATE_CONNECTED] = {
+		.out_state_mask = GEN_MASK(L1CTL_STATE_IDLE),
+		.name = "CONNECTED",
+	},
+};
+
+static struct osmo_fsm l1ctl_fsm = {
+	.name = "l1ctl_link_fsm",
+	.states = l1ctl_fsm_states,
+	.num_states = ARRAY_SIZE(l1ctl_fsm_states),
+	.log_subsys = DL1C,
+};
 
 static int l1ctl_link_read_cb(struct osmo_fd *bfd)
 {
@@ -154,7 +175,9 @@
 		return -1;
 	}
 
-	/* TODO: switch the bridge to CONNECTED state */
+	osmo_fsm_inst_dispatch(trxcon_fsm, L1CTL_EVENT_CONNECT, l1l);
+	osmo_fsm_inst_state_chg(l1l->fsm, L1CTL_STATE_CONNECTED, 0, 0);
+
 	LOGP(DL1C, LOGL_NOTICE, "L1CTL has a new connection\n");
 
 	return 0;
@@ -199,7 +222,9 @@
 	/* Clear pending messages */
 	osmo_wqueue_clear(&l1l->wq);
 
-	/* TODO: switch the bridge to IDLE state */
+	osmo_fsm_inst_dispatch(trxcon_fsm, L1CTL_EVENT_DISCONNECT, l1l);
+	osmo_fsm_inst_state_chg(l1l->fsm, L1CTL_STATE_IDLE, 0, 0);
+
 	return 0;
 }
 
@@ -238,6 +263,11 @@
 	 */
 	l1l_new->wq.bfd.fd = -1;
 
+	/* Allocate a new dedicated state machine */
+	osmo_fsm_register(&l1ctl_fsm);
+	l1l_new->fsm = osmo_fsm_inst_alloc(&l1ctl_fsm, l1l_new,
+		NULL, LOGL_DEBUG, sock_path);
+
 	*l1l = l1l_new;
 
 	return 0;
@@ -266,5 +296,6 @@
 		listen_bfd->fd = -1;
 	}
 
+	osmo_fsm_inst_free(l1l->fsm);
 	talloc_free(l1l);
 }
diff --git a/src/host/trxcon/l1ctl_link.h b/src/host/trxcon/l1ctl_link.h
index 417dc75..620dde5 100644
--- a/src/host/trxcon/l1ctl_link.h
+++ b/src/host/trxcon/l1ctl_link.h
@@ -3,11 +3,18 @@
 #include <osmocom/core/write_queue.h>
 #include <osmocom/core/select.h>
 #include <osmocom/core/msgb.h>
+#include <osmocom/core/fsm.h>
 
 #define L1CTL_LENGTH 256
 #define L1CTL_HEADROOM 32
 
+enum l1ctl_fsm_states {
+	L1CTL_STATE_IDLE = 0,
+	L1CTL_STATE_CONNECTED,
+};
+
 struct l1ctl_link {
+	struct osmo_fsm_inst *fsm;
 	struct osmo_fd listen_bfd;
 	struct osmo_wqueue wq;
 };
diff --git a/src/host/trxcon/trx_if.c b/src/host/trxcon/trx_if.c
index 5b0b7b1..8f93b0a 100644
--- a/src/host/trxcon/trx_if.c
+++ b/src/host/trxcon/trx_if.c
@@ -35,13 +35,49 @@
 #include <osmocom/core/socket.h>
 #include <osmocom/core/talloc.h>
 #include <osmocom/core/bits.h>
+#include <osmocom/core/fsm.h>
 
 #include <osmocom/gsm/gsm_utils.h>
 
+#include "trxcon.h"
 #include "trx_if.h"
 #include "logging.h"
 
 extern void *tall_trx_ctx;
+extern struct osmo_fsm_inst *trxcon_fsm;
+
+static struct osmo_fsm_state trx_fsm_states[] = {
+	[TRX_STATE_OFFLINE] = {
+		.out_state_mask = (
+			GEN_MASK(TRX_STATE_IDLE) |
+			GEN_MASK(TRX_STATE_RSP_WAIT)),
+		.name = "OFFLINE",
+	},
+	[TRX_STATE_IDLE] = {
+		.out_state_mask = UINT32_MAX,
+		.name = "IDLE",
+	},
+	[TRX_STATE_ACTIVE] = {
+		.out_state_mask = (
+			GEN_MASK(TRX_STATE_IDLE) |
+			GEN_MASK(TRX_STATE_RSP_WAIT)),
+		.name = "ACTIVE",
+	},
+	[TRX_STATE_RSP_WAIT] = {
+		.out_state_mask = (
+			GEN_MASK(TRX_STATE_IDLE) |
+			GEN_MASK(TRX_STATE_ACTIVE) |
+			GEN_MASK(TRX_STATE_OFFLINE)),
+		.name = "RSP_WAIT",
+	},
+};
+
+static struct osmo_fsm trx_fsm = {
+	.name = "trx_interface_fsm",
+	.states = trx_fsm_states,
+	.num_states = ARRAY_SIZE(trx_fsm_states),
+	.log_subsys = DTRX,
+};
 
 static int trx_udp_open(void *priv, struct osmo_fd *ofd, const char *host,
 	uint16_t port_local, uint16_t port_remote,
@@ -178,6 +214,12 @@
 	LOGP(DTRX, LOGL_DEBUG, "Sending control '%s'\n", tcm->cmd);
 	send(trx->trx_ofd_ctrl.fd, tcm->cmd, strlen(tcm->cmd) + 1, 0);
 
+	/* Trigger state machine */
+	if (trx->fsm->state != TRX_STATE_RSP_WAIT) {
+		trx->prev_state = trx->fsm->state;
+		osmo_fsm_inst_state_chg(trx->fsm, TRX_STATE_RSP_WAIT, 0, 0);
+	}
+
 	/* Start expire timer */
 	trx->trx_ctrl_timer.data = trx;
 	trx->trx_ctrl_timer.cb = trx_ctrl_timer_cb;
@@ -186,10 +228,25 @@
 
 static void trx_ctrl_timer_cb(void *data)
 {
+	struct trx_instance *trx = (struct trx_instance *) data;
+	struct trx_ctrl_msg *tcm;
+
+	/* Queue may be cleaned at this moment */
+	if (llist_empty(&trx->trx_ctrl_list))
+		return;
+
 	LOGP(DTRX, LOGL_NOTICE, "No response from transceiver...\n");
 
+	tcm = llist_entry(trx->trx_ctrl_list.next, struct trx_ctrl_msg, list);
+	if (++tcm->retry_cnt > 3) {
+		LOGP(DTRX, LOGL_NOTICE, "Transceiver offline\n");
+		osmo_fsm_inst_state_chg(trx->fsm, TRX_STATE_OFFLINE, 0, 0);
+		osmo_fsm_inst_dispatch(trxcon_fsm, TRX_EVENT_OFFLINE, trx);
+		return;
+	}
+
 	/* Attempt to send a command again */
-	trx_ctrl_send((struct trx_instance *) data);
+	trx_ctrl_send(trx);
 }
 
 /* Add a new CTRL command to the trx_ctrl_list */
@@ -200,11 +257,7 @@
 	int len, pending = 0;
 	va_list ap;
 
-	/*if (!transceiver_available && !!strcmp(cmd, "POWEROFF")) {
-		LOGP(DTRX, LOGL_ERROR, "CTRL ignored: No clock from "
-			"transceiver, please fix!\n");
-		return -EIO;
-	}*/
+	/* TODO: make sure that transceiver online */
 
 	if (!llist_empty(&trx->trx_ctrl_list))
 		pending = 1;
@@ -419,6 +472,18 @@
 			goto rsp_error;
 	}
 
+	/* Trigger state machine */
+	if (!strncmp(tcm->cmd + 4, "POWERON", 7))
+		osmo_fsm_inst_state_chg(trx->fsm, TRX_STATE_ACTIVE, 0, 0);
+	else if (!strncmp(tcm->cmd + 4, "POWEROFF", 8))
+		osmo_fsm_inst_state_chg(trx->fsm, TRX_STATE_IDLE, 0, 0);
+	else if (!strncmp(tcm->cmd + 4, "ECHO", 4)) {
+		osmo_fsm_inst_state_chg(trx->fsm, TRX_STATE_IDLE, 0, 0);
+		osmo_fsm_inst_dispatch(trxcon_fsm,
+			TRX_EVENT_RESET_IND, trx);
+	} else
+		osmo_fsm_inst_state_chg(trx->fsm, trx->prev_state, 0, 0);
+
 	/* Remove command from list */
 	llist_del(&tcm->list);
 	talloc_free(tcm);
@@ -429,10 +494,8 @@
 	return 0;
 
 rsp_error:
-	/**
-	 * TODO: stop/freeze trxcon process
-	 * or notify higher layers about the problem with L1
-	 */
+	/* Notify higher layers about the problem */
+	osmo_fsm_inst_dispatch(trxcon_fsm, TRX_EVENT_RSP_ERROR, trx);
 	return -EIO;
 }
 
@@ -512,6 +575,18 @@
 {
 	uint8_t buf[256];
 
+	/**
+	 * We must be sure that we have clock,
+	 * and we have sent all control data
+	 *
+	 * TODO: should we wait in TRX_STATE_RSP_WAIT state?
+	 */
+	if (trx->fsm->state != TRX_STATE_ACTIVE) {
+		LOGP(DTRX, LOGL_DEBUG, "Ignoring TX data, "
+			"transceiver isn't ready\n");
+		return -EAGAIN;
+	}
+
 	LOGP(DTRX, LOGL_DEBUG, "TX burst tn=%u fn=%u pwr=%u\n", tn, fn, pwr);
 
 	buf[0] = tn;
@@ -524,17 +599,8 @@
 	/* Copy ubits {0,1} */
 	memcpy(buf + 6, bits, 148);
 
-	/**
-	 * TODO: is transceiver available???
-	 *
-	 * We must be sure that we have clock,
-	 * and we have sent all control data
-	 *
-	 * if (transceiver_available && llist_empty(&l1h->trx_ctrl_list))
-	 *   send(l1h->trx_ofd_data.fd, buf, 154, 0);
-	 * else
-	 *   LOGP(DTRX, LOGL_DEBUG, "Ignoring TX data, transceiver offline.\n");
-	 */
+	/* Send data to transceiver */
+	send(trx->trx_ofd_data.fd, buf, 154, 0);
 
 	return 0;
 }
@@ -546,6 +612,7 @@
 int trx_if_open(struct trx_instance **trx, const char *host, uint16_t port)
 {
 	struct trx_instance *trx_new;
+	char *inst_name;
 	int rc;
 
 	LOGP(DTRX, LOGL_NOTICE, "Init transceiver interface\n");
@@ -576,6 +643,13 @@
 	if (rc < 0)
 		goto error;
 
+	/* Allocate a new dedicated state machine */
+	osmo_fsm_register(&trx_fsm);
+	inst_name = talloc_asprintf(trx_new, "%s:%u", host, port);
+	trx_new->fsm = osmo_fsm_inst_alloc(&trx_fsm, trx_new,
+		NULL, LOGL_DEBUG, inst_name);
+	talloc_free(inst_name);
+
 	*trx = trx_new;
 
 	return 0;
@@ -587,10 +661,14 @@
 }
 
 /* Flush pending control messages */
-static void trx_if_flush_ctrl(struct trx_instance *trx)
+void trx_if_flush_ctrl(struct trx_instance *trx)
 {
 	struct trx_ctrl_msg *tcm;
 
+	/* Reset state machine */
+	osmo_fsm_inst_state_chg(trx->fsm, TRX_STATE_IDLE, 0, 0);
+
+	/* Clear command queue */
 	while (!llist_empty(&trx->trx_ctrl_list)) {
 		tcm = llist_entry(trx->trx_ctrl_list.next,
 			struct trx_ctrl_msg, list);
@@ -616,5 +694,6 @@
 	trx_udp_close(&trx->trx_ofd_data);
 
 	/* Free memory */
+	osmo_fsm_inst_free(trx->fsm);
 	talloc_free(trx);
 }
diff --git a/src/host/trxcon/trx_if.h b/src/host/trxcon/trx_if.h
index 6f54b3e..a81da37 100644
--- a/src/host/trxcon/trx_if.h
+++ b/src/host/trxcon/trx_if.h
@@ -3,6 +3,14 @@
 #include <osmocom/core/linuxlist.h>
 #include <osmocom/core/select.h>
 #include <osmocom/core/timer.h>
+#include <osmocom/core/fsm.h>
+
+enum trx_fsm_states {
+	TRX_STATE_OFFLINE = 0,
+	TRX_STATE_IDLE,
+	TRX_STATE_ACTIVE,
+	TRX_STATE_RSP_WAIT,
+};
 
 struct trx_instance {
 	struct osmo_fd trx_ofd_clck;
@@ -11,16 +19,20 @@
 
 	struct osmo_timer_list trx_ctrl_timer;
 	struct llist_head trx_ctrl_list;
+	struct osmo_fsm_inst *fsm;
+	uint32_t prev_state;
 };
 
 struct trx_ctrl_msg {
 	struct llist_head list;
 	char cmd[128];
+	int retry_cnt;
 	int critical;
 	int cmd_len;
 };
 
 int trx_if_open(struct trx_instance **trx, const char *host, uint16_t port);
+void trx_if_flush_ctrl(struct trx_instance *trx);
 void trx_if_close(struct trx_instance *trx);
 
 int trx_if_cmd_poweron(struct trx_instance *trx);
diff --git a/src/host/trxcon/trxcon.c b/src/host/trxcon/trxcon.c
index 781942a..5874560 100644
--- a/src/host/trxcon/trxcon.c
+++ b/src/host/trxcon/trxcon.c
@@ -29,12 +29,14 @@
 #include <unistd.h>
 #include <signal.h>
 
+#include <osmocom/core/fsm.h>
 #include <osmocom/core/msgb.h>
 #include <osmocom/core/talloc.h>
 #include <osmocom/core/signal.h>
 #include <osmocom/core/select.h>
 #include <osmocom/core/application.h>
 
+#include "trxcon.h"
 #include "trx_if.h"
 #include "logging.h"
 #include "l1ctl_link.h"
@@ -62,6 +64,62 @@
 } app_data;
 
 void *tall_trx_ctx = NULL;
+struct osmo_fsm_inst *trxcon_fsm;
+
+static void trxcon_fsm_idle_action(struct osmo_fsm_inst *fi,
+	uint32_t event, void *data)
+{
+	if (event == L1CTL_EVENT_CONNECT)
+		osmo_fsm_inst_state_chg(trxcon_fsm, TRXCON_STATE_MANAGED, 0, 0);
+}
+
+static void trxcon_fsm_managed_action(struct osmo_fsm_inst *fi,
+	uint32_t event, void *data)
+{
+	switch (event) {
+	case L1CTL_EVENT_DISCONNECT:
+		osmo_fsm_inst_state_chg(trxcon_fsm, TRXCON_STATE_IDLE, 0, 0);
+
+		if (app_data.trx->fsm->state != TRX_STATE_OFFLINE) {
+			trx_if_flush_ctrl(app_data.trx);
+			trx_if_cmd_poweroff(app_data.trx);
+		}
+		break;
+	case TRX_EVENT_RESET_IND:
+	case TRX_EVENT_RSP_ERROR:
+	case TRX_EVENT_OFFLINE:
+		/* TODO: notify L2 & L3 about that */
+		break;
+	default:
+		LOGPFSML(fi, LOGL_ERROR, "Unhandled event %u\n", event);
+	}
+}
+
+static struct osmo_fsm_state trxcon_fsm_states[] = {
+	[TRXCON_STATE_IDLE] = {
+		.in_event_mask = GEN_MASK(L1CTL_EVENT_CONNECT),
+		.out_state_mask = GEN_MASK(TRXCON_STATE_MANAGED),
+		.name = "IDLE",
+		.action = trxcon_fsm_idle_action,
+	},
+	[TRXCON_STATE_MANAGED] = {
+		.in_event_mask = (
+			GEN_MASK(L1CTL_EVENT_DISCONNECT) |
+			GEN_MASK(TRX_EVENT_RESET_IND) |
+			GEN_MASK(TRX_EVENT_RSP_ERROR) |
+			GEN_MASK(TRX_EVENT_OFFLINE)),
+		.out_state_mask = GEN_MASK(TRXCON_STATE_IDLE),
+		.name = "MANAGED",
+		.action = trxcon_fsm_managed_action,
+	},
+};
+
+static struct osmo_fsm trxcon_fsm_def = {
+	.name = "trxcon_app_fsm",
+	.states = trxcon_fsm_states,
+	.num_states = ARRAY_SIZE(trxcon_fsm_states),
+	.log_subsys = DAPP,
+};
 
 static void print_usage(const char *app)
 {
@@ -175,6 +233,11 @@
 	/* Init logging system */
 	trx_log_init(app_data.debug_mask);
 
+	/* Allocate the application state machine */
+	osmo_fsm_register(&trxcon_fsm_def);
+	trxcon_fsm = osmo_fsm_inst_alloc(&trxcon_fsm_def, tall_trx_ctx,
+		NULL, LOGL_DEBUG, "main");
+
 	/* Init L1CTL server */
 	rc = l1ctl_link_init(&app_data.l1l, app_data.bind_socket);
 	if (rc)
@@ -203,6 +266,9 @@
 	l1ctl_link_shutdown(app_data.l1l);
 	trx_if_close(app_data.trx);
 
+	/* Shutdown main state machine */
+	osmo_fsm_inst_free(trxcon_fsm);
+
 	/* Make Valgrind happy */
 	log_fini();
 	talloc_free(tall_trx_ctx);
diff --git a/src/host/trxcon/trxcon.h b/src/host/trxcon/trxcon.h
new file mode 100644
index 0000000..a7a3a65
--- /dev/null
+++ b/src/host/trxcon/trxcon.h
@@ -0,0 +1,19 @@
+#pragma once
+
+#define GEN_MASK(state) (0x01 << state)
+
+enum trxcon_fsm_states {
+	TRXCON_STATE_IDLE = 0,
+	TRXCON_STATE_MANAGED,
+};
+
+enum trxcon_fsm_events {
+	/* L1CTL specific events */
+	L1CTL_EVENT_CONNECT,
+	L1CTL_EVENT_DISCONNECT,
+
+	/* TRX specific events */
+	TRX_EVENT_RESET_IND,
+	TRX_EVENT_RSP_ERROR,
+	TRX_EVENT_OFFLINE,
+};

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7ee6fc891abe5f775f5b7ebbf093181a97950dea
Gerrit-PatchSet: 1
Gerrit-Project: osmocom-bb
Gerrit-Branch: master
Gerrit-Owner: Harald Welte <laforge at gnumonks.org>



More information about the gerrit-log mailing list