lchan_fsm: make rsl mode-modify working again

osmo-nitb supports the modification of an lchan if the lchan is
compatible but in the wrong mode. This feature was dropped in the
transition to AoIP/bsc-split. However, osmo-bsc still has code to
generate and parse the messages, but the FSMs do not support a mode
modify yet

Lets add handling for mode-modify to the lchan_fsm and assignment_fsm in
order to support mode modify again

Change-Id: I2c5a283b1ee33745cc1fcfcc09a0f9382224e2eb
Related: OS#4549

5 files changed, 194 insertions, 47 deletions

diff --git a/include/osmocom/bsc/lchan_fsm.h b/include/osmocom/bsc/lchan_fsm.h
index 55ab02400..d935b5281 100644
--- a/include/osmocom/bsc/lchan_fsm.h
+++ b/include/osmocom/bsc/lchan_fsm.h
@@ -18,6 +18,8 @@ enum lchan_fsm_state {
 	LCHAN_ST_WAIT_TS_READY,
 	LCHAN_ST_WAIT_ACTIV_ACK, /*< After RSL Chan Act Ack, lchan is active but RTP not configured. */
 	LCHAN_ST_WAIT_RLL_RTP_ESTABLISH,
+	LCHAN_ST_WAIT_RR_CHAN_MODE_MODIF_ACK,
+	LCHAN_ST_WAIT_RSL_MT_MODE_MODIFY_ACK,
 	LCHAN_ST_ESTABLISHED, /*< Active and RTP is fully configured. */
 	LCHAN_ST_WAIT_RLL_RTP_RELEASED,
 	LCHAN_ST_WAIT_BEFORE_RF_RELEASE,
@@ -40,10 +42,11 @@ enum lchan_fsm_event {
 	LCHAN_EV_RLL_REL_CONF,
 	LCHAN_EV_RSL_RF_CHAN_REL_ACK,
 	LCHAN_EV_RLL_ERR_IND,
-
-	/* FIXME: not yet implemented: Chan Mode Modify, see assignment_fsm_start(). */
-	LCHAN_EV_CHAN_MODE_MODIF_ACK,
-	LCHAN_EV_CHAN_MODE_MODIF_ERROR,
+	LCHAN_EV_RR_CHAN_MODE_MODIF_ACK,
+	LCHAN_EV_RR_CHAN_MODE_MODIF_ERROR,
+	LCHAN_EV_RSL_MT_MODE_MODIFY_ACK,
+	LCHAN_EV_RSL_MT_MODE_MODIFY_NACK,
+	LCHAN_EV_REQUEST_MODE_MODIFY,
 };
 
 void lchan_fsm_init();
diff --git a/src/osmo-bsc/abis_rsl.c b/src/osmo-bsc/abis_rsl.c
index 6787511f5..42f3a4226 100644
--- a/src/osmo-bsc/abis_rsl.c
+++ b/src/osmo-bsc/abis_rsl.c
@@ -1186,10 +1186,12 @@ static int abis_rsl_rx_dchan(struct msgb *msg)
 	case RSL_MT_MODE_MODIFY_ACK:
 		LOG_LCHAN(msg->lchan, LOGL_DEBUG, "CHANNEL MODE MODIFY ACK\n");
 		count_codecs(sign_link->trx->bts, msg->lchan);
+		osmo_fsm_inst_dispatch(msg->lchan->fi, LCHAN_EV_RSL_MT_MODE_MODIFY_ACK, NULL);
 		break;
 	case RSL_MT_MODE_MODIFY_NACK:
 		LOG_LCHAN(msg->lchan, LOGL_DEBUG, "CHANNEL MODE MODIFY NACK\n");
 		rate_ctr_inc(&sign_link->trx->bts->bts_ctrs->ctr[BTS_CTR_MODE_MODIFY_NACK]);
+		osmo_fsm_inst_dispatch(msg->lchan->fi, LCHAN_EV_RSL_MT_MODE_MODIFY_NACK, NULL);
 		break;
 	case RSL_MT_IPAC_PDCH_ACT_ACK:
 		rc = rsl_rx_ipacc_pdch(msg, "ACT ACK", TS_EV_PDCH_ACT_ACK);
diff --git a/src/osmo-bsc/assignment_fsm.c b/src/osmo-bsc/assignment_fsm.c
index ca29daabb..0ff8610b2 100644
--- a/src/osmo-bsc/assignment_fsm.c
+++ b/src/osmo-bsc/assignment_fsm.c
@@ -379,9 +379,8 @@ static int check_requires_voice_stream(struct gsm_subscriber_connection *conn)
 	return 0;
 }
 
-/* Check if the conn is already associated with an lchan. If yes, we will check
- * if that lchan is compatible with the preferred rate/codec. If the lchan
- * turns out to be incompatible we try with the alternate rate/codec. */
+/* Decide if we should re-use an existing lchan. For this we check if the
+ * current lchan is compatible with one of the requested modes. */
 static bool reuse_existing_lchan(struct gsm_subscriber_connection *conn)
 {
 	struct assignment_request *req = &conn->assignment.req;
@@ -395,22 +394,10 @@ static bool reuse_existing_lchan(struct gsm_subscriber_connection *conn)
 	for (i = 0; i < req->n_ch_mode_rate; i++)
 		if (lchan_type_compat_with_mode(conn->lchan->type, &req->ch_mode_rate[i])) {
 			conn->lchan->ch_mode_rate = req->ch_mode_rate[i];
-			break;
+			return true;
 		}
 
-	if (i == req->n_ch_mode_rate)
-		return false;
-
-	if (conn->lchan->tch_mode != conn->lchan->ch_mode_rate.chan_mode) {
-		/* FIXME: send Channel Mode Modify to put the current lchan in the right mode, and kick
-		 * off its RTP stream setup code path. See gsm48_lchan_modify() and
-		 * gsm48_rx_rr_modif_ack(), and see lchan_fsm.h LCHAN_EV_CHAN_MODE_MODIF_* */
-		LOG_ASSIGNMENT(conn, LOGL_DEBUG,
-			       "Current lchan would be compatible, but Channel Mode Modify is not implemented\n");
-		return false;
-	}
-
-	return true;
+	return false;
 }
 
 void assignment_fsm_start(struct gsm_subscriber_connection *conn, struct gsm_bts *bts,
@@ -447,22 +434,57 @@ void assignment_fsm_start(struct gsm_subscriber_connection *conn, struct gsm_bts
 		return;
 
 	/* There may be an already existing lchan, if yes, try to work with
-	 * the existing lchan */
+	 * the existing lchan. */
 	if (reuse_existing_lchan(conn)) {
+
+		/* If the requested mode and the current TCH mode matches up, just send the
+		 * assignment complete directly and be done with the assignment procedure. */
+		if (conn->lchan->tch_mode == conn->lchan->ch_mode_rate.chan_mode) {
+			LOG_ASSIGNMENT(conn, LOGL_DEBUG,
+				       "Current lchan mode is compatible with requested chan_mode,"
+				       " sending BSSMAP Assignment Complete directly."
+				       " requested chan_mode=%s; current lchan is %s\n",
+				       gsm48_chan_mode_name(conn->lchan->ch_mode_rate.chan_mode),
+				       gsm_lchan_name(conn->lchan));
+
+			send_assignment_complete(conn);
+			/* If something went wrong during send_assignment_complete(),
+			 * the fi will be gone from error handling in there. */
+			if (conn->assignment.fi) {
+				assignment_count_result(CTR_ASSIGNMENT_COMPLETED);
+				osmo_fsm_inst_term(conn->assignment.fi, OSMO_FSM_TERM_REGULAR, 0);
+			}
+			return;
+		}
+
+		/* The requested mode does not match the current TCH mode but the lchan is
+		 * compatible. We will initiate a mode modify procedure. */
 		LOG_ASSIGNMENT(conn, LOGL_DEBUG,
-			       "Current lchan is compatible with requested chan_mode,"
-			       " sending BSSMAP Assignment Complete directly."
-			       " requested chan_mode=%s; current lchan is %s\n",
+			       "Current lchan mode is not compatible with requested chan_mode,"
+			       " so we will modify it. requested chan_mode=%s; current lchan is %s\n",
 			       gsm48_chan_mode_name(conn->lchan->ch_mode_rate.chan_mode),
 			       gsm_lchan_name(conn->lchan));
 
-		send_assignment_complete(conn);
-		/* If something went wrong during send_assignment_complete(), the fi will be gone from
-		 * error handling in there. */
-		if (conn->assignment.fi) {
-			assignment_count_result(CTR_ASSIGNMENT_COMPLETED);
-			osmo_fsm_inst_term(conn->assignment.fi, OSMO_FSM_TERM_REGULAR, 0);
-		}
+		info = (struct lchan_activate_info){
+			.activ_for = FOR_ASSIGNMENT,
+			.for_conn = conn,
+			.chan_mode = conn->lchan->ch_mode_rate.chan_mode,
+			.encr = conn->lchan->encr,
+			.s15_s0 = conn->lchan->ch_mode_rate.s15_s0,
+			.requires_voice_stream = conn->assignment.requires_voice_stream,
+			.msc_assigned_cic = req->msc_assigned_cic,
+			.re_use_mgw_endpoint_from_lchan = conn->lchan,
+		};
+
+		osmo_fsm_inst_dispatch(conn->lchan->fi, LCHAN_EV_REQUEST_MODE_MODIFY, &info);
+
+		/* Since we opted not to allocate a new lchan, the new lchan is still the old lchan. */
+		conn->assignment.new_lchan = conn->lchan;
+
+		/* Also we need to skip the RR assignment, so we jump forward and wait for the lchan_fsm until it
+                 * reaches the established state again. */
+		assignment_fsm_state_chg(ASSIGNMENT_ST_WAIT_LCHAN_ESTABLISHED);
+
 		return;
 	}
 
@@ -682,6 +704,7 @@ static const struct osmo_fsm_state assignment_fsm_states[] = {
 		.out_state_mask = 0
 			| S(ASSIGNMENT_ST_WAIT_LCHAN_ACTIVE)
 			| S(ASSIGNMENT_ST_WAIT_RR_ASS_COMPLETE)
+			| S(ASSIGNMENT_ST_WAIT_LCHAN_ESTABLISHED) /* MODE MODIFY */
 			,
 	},
 	[ASSIGNMENT_ST_WAIT_RR_ASS_COMPLETE] = {
diff --git a/src/osmo-bsc/gsm_04_08_rr.c b/src/osmo-bsc/gsm_04_08_rr.c
index 69ec4d3f9..6cfc2ba55 100644
--- a/src/osmo-bsc/gsm_04_08_rr.c
+++ b/src/osmo-bsc/gsm_04_08_rr.c
@@ -654,7 +654,6 @@ int gsm48_lchan_modify(struct gsm_lchan *lchan, uint8_t mode)
 
 int gsm48_rx_rr_modif_ack(struct msgb *msg)
 {
-	int rc;
 	struct gsm48_hdr *gh = msgb_l3(msg);
 	struct gsm48_chan_mode_modify *mod =
 				(struct gsm48_chan_mode_modify *) gh->data;
@@ -688,15 +687,7 @@ int gsm48_rx_rr_modif_ack(struct msgb *msg)
 		break;
 	}
 
-	/* We've successfully modified the MS side of the channel,
-	 * now go on to modify the BTS side of the channel */
-	rc = rsl_chan_mode_modify_req(msg->lchan);
-
-	/* FIXME: we not only need to do this after mode modify, but
-	 * also after channel activation */
-	if (is_ipaccess_bts(msg->lchan->ts->trx->bts) && mod->mode != GSM48_CMODE_SIGN)
-		rsl_tx_ipacc_crcx(msg->lchan);
-	return rc;
+	return 0;
 }
 
 int gsm48_parse_meas_rep(struct gsm_meas_rep *rep, struct msgb *msg)
@@ -969,9 +960,9 @@ static void dispatch_dtap(struct gsm_subscriber_connection *conn,
 		case GSM48_MT_RR_CHAN_MODE_MODIF_ACK:
 			rc = gsm48_rx_rr_modif_ack(msg);
 			if (rc < 0)
-				osmo_fsm_inst_dispatch(msg->lchan->fi, LCHAN_EV_CHAN_MODE_MODIF_ERROR, &rc);
+				osmo_fsm_inst_dispatch(msg->lchan->fi, LCHAN_EV_RR_CHAN_MODE_MODIF_ERROR, &rc);
 			else
-				osmo_fsm_inst_dispatch(msg->lchan->fi, LCHAN_EV_CHAN_MODE_MODIF_ACK, msg);
+				osmo_fsm_inst_dispatch(msg->lchan->fi, LCHAN_EV_RR_CHAN_MODE_MODIF_ACK, msg);
 			break;
 		case GSM48_MT_RR_CLSM_CHG:
 			handle_classmark_chg(conn, msg);
diff --git a/src/osmo-bsc/lchan_fsm.c b/src/osmo-bsc/lchan_fsm.c
index 26bfd2929..f6b3f2b5f 100644
--- a/src/osmo-bsc/lchan_fsm.c
+++ b/src/osmo-bsc/lchan_fsm.c
@@ -59,6 +59,7 @@ bool lchan_may_receive_data(struct gsm_lchan *lchan)
 	switch (lchan->fi->state) {
 	case LCHAN_ST_WAIT_RLL_RTP_ESTABLISH:
 	case LCHAN_ST_ESTABLISHED:
+	case LCHAN_ST_WAIT_RR_CHAN_MODE_MODIF_ACK:
 		return true;
 	default:
 		return false;
@@ -249,7 +250,7 @@ struct osmo_tdef_state_timeout lchan_fsm_timeouts[32] = {
 	} while(0)
 
 /* Which state to transition to when lchan_fail() is called in a given state. */
-uint32_t lchan_fsm_on_error[32] = {
+uint32_t lchan_fsm_on_error[34] = {
 	[LCHAN_ST_UNUSED] 			= LCHAN_ST_UNUSED,
 	[LCHAN_ST_WAIT_TS_READY] 		= LCHAN_ST_UNUSED,
 	[LCHAN_ST_WAIT_ACTIV_ACK] 		= LCHAN_ST_BORKEN,
@@ -260,6 +261,8 @@ uint32_t lchan_fsm_on_error[32] = {
 	[LCHAN_ST_WAIT_RF_RELEASE_ACK] 		= LCHAN_ST_BORKEN,
 	[LCHAN_ST_WAIT_AFTER_ERROR] 		= LCHAN_ST_UNUSED,
 	[LCHAN_ST_BORKEN] 			= LCHAN_ST_BORKEN,
+	[LCHAN_ST_WAIT_RR_CHAN_MODE_MODIF_ACK] 	= LCHAN_ST_BORKEN,
+	[LCHAN_ST_WAIT_RSL_MT_MODE_MODIFY_ACK] 	= LCHAN_ST_BORKEN,
 };
 
 #define lchan_fail(fmt, args...) lchan_fail_to(lchan_fsm_on_error[fi->state], fmt, ## args)
@@ -833,6 +836,97 @@ static void lchan_fsm_wait_rll_rtp_establish(struct osmo_fsm_inst *fi, uint32_t
 	}
 }
 
+static void lchan_fsm_wait_rr_chan_mode_modif_ack_onenter(struct osmo_fsm_inst *fi, uint32_t prev_state)
+{
+	struct gsm_lchan *lchan = lchan_fi_lchan(fi);
+	struct lchan_activate_info *info = &lchan->activate.info;
+	struct osmo_mgcpc_ep_ci *use_mgwep_ci;
+
+	use_mgwep_ci = lchan_use_mgw_endpoint_ci_bts(lchan);
+
+	if (info->chan_mode == GSM48_CMODE_SPEECH_AMR) {
+		if (lchan_mr_config(lchan, info->s15_s0) < 0) {
+			lchan_fail("Can not generate multirate configuration IE\n");
+			return;
+		}
+	}
+
+	LOG_LCHAN(lchan, LOGL_INFO,
+		  "Modification requested: %s voice=%s MGW-ci=%s type=%s tch-mode=%s encr-alg=A5/%u ck=%s\n",
+		  lchan_activate_mode_name(lchan->activate.info.activ_for),
+		  lchan->activate.info.requires_voice_stream ? "yes" : "no",
+		  lchan->activate.info.requires_voice_stream ?
+		  (use_mgwep_ci ? osmo_mgcpc_ep_ci_name(use_mgwep_ci) : "new")
+		  : "none",
+		  gsm_lchant_name(lchan->type),
+		  gsm48_chan_mode_name(lchan->tch_mode),
+		  (lchan->activate.info.encr.alg_id ? : 1) - 1,
+		  lchan->activate.info.encr.key_len ? osmo_hexdump_nospc(lchan->activate.info.encr.key,
+									 lchan->activate.info.encr.key_len) : "none");
+
+	/* While the mode is changed the lchan is virtually "not activated", at least
+	 * from the FSM implementations perspective */
+	lchan->activate.concluded = false;
+
+	gsm48_lchan_modify(lchan, info->chan_mode);
+}
+
+static void lchan_fsm_wait_rr_chan_mode_modif_ack(struct osmo_fsm_inst *fi, uint32_t event, void *data)
+{
+	switch (event) {
+
+	case LCHAN_EV_RR_CHAN_MODE_MODIF_ACK:
+		lchan_fsm_state_chg(LCHAN_ST_WAIT_RSL_MT_MODE_MODIFY_ACK);
+		return;
+
+	case LCHAN_EV_RR_CHAN_MODE_MODIF_ERROR:
+		lchan_fail("Failed to change channel mode on the MS side: %s in state %s\n",
+			   osmo_fsm_event_name(fi->fsm, event),
+			   osmo_fsm_inst_state_name(fi));
+		return;
+
+	default:
+		OSMO_ASSERT(false);
+	}
+}
+
+static void lchan_fsm_wait_rsl_mt_mode_modify_ack_onenter(struct osmo_fsm_inst *fi, uint32_t prev_state)
+{
+	int rc;
+	struct gsm_lchan *lchan = lchan_fi_lchan(fi);
+
+	rc = rsl_chan_mode_modify_req(lchan);
+	if (rc < 0) {
+		lchan_fail("Failed to send rsl message to change the channel mode on the BTS side in state %s\n",
+			   osmo_fsm_inst_state_name(fi));
+	}
+}
+
+static void lchan_fsm_wait_rsl_mt_mode_modify_ack(struct osmo_fsm_inst *fi, uint32_t event, void *data)
+{
+	struct gsm_lchan *lchan = lchan_fi_lchan(fi);
+	switch (event) {
+
+	case LCHAN_EV_RSL_MT_MODE_MODIFY_ACK:
+		/* Prepare an MGW endpoint CI if appropriate. */
+		if (lchan->activate.info.requires_voice_stream) {
+			lchan_fsm_state_chg(LCHAN_ST_WAIT_RLL_RTP_ESTABLISH);
+			lchan_rtp_fsm_start(lchan);
+		} else
+			lchan_fsm_state_chg(LCHAN_ST_ESTABLISHED);
+		return;
+
+	case LCHAN_EV_RSL_MT_MODE_MODIFY_NACK:
+		lchan_fail("Failed to change channel mode on the BTS side: %s in state %s\n",
+			   osmo_fsm_event_name(fi->fsm, event),
+			   osmo_fsm_inst_state_name(fi));
+		return;
+
+	default:
+		OSMO_ASSERT(false);
+	}
+}
+
 static void lchan_fsm_established_onenter(struct osmo_fsm_inst *fi, uint32_t prev_state)
 {
 	struct gsm_lchan *lchan = lchan_fi_lchan(fi);
@@ -935,6 +1029,10 @@ static void lchan_fsm_established(struct osmo_fsm_inst *fi, uint32_t event, void
 			   osmo_fsm_inst_state_name(fi));
 		return;
 
+	case LCHAN_EV_REQUEST_MODE_MODIFY:
+		lchan->activate.info = *(struct lchan_activate_info *)data;
+		lchan_fsm_state_chg(LCHAN_ST_WAIT_RR_CHAN_MODE_MODIF_ACK);
+
 	default:
 		OSMO_ASSERT(false);
 	}
@@ -1223,6 +1321,32 @@ static const struct osmo_fsm_state lchan_fsm_states[] = {
 			| S(LCHAN_ST_WAIT_RLL_RTP_RELEASED)
 			,
 	},
+	[LCHAN_ST_WAIT_RR_CHAN_MODE_MODIF_ACK] = {
+		.name = "WAIT_CHAN_MODE_MODIF_ACK",
+                .onenter = lchan_fsm_wait_rr_chan_mode_modif_ack_onenter,
+		.action = lchan_fsm_wait_rr_chan_mode_modif_ack,
+		.in_event_mask = 0
+			| S(LCHAN_EV_RR_CHAN_MODE_MODIF_ACK)
+			| S(LCHAN_EV_RR_CHAN_MODE_MODIF_ERROR)
+			,
+		.out_state_mask = 0
+			| S(LCHAN_ST_BORKEN)
+			| S(LCHAN_ST_WAIT_RSL_MT_MODE_MODIFY_ACK)
+			,
+	},
+	[LCHAN_ST_WAIT_RSL_MT_MODE_MODIFY_ACK] = {
+		.name = "WAIT_RSL_MT_MODE_MODIFY_ACK",
+		.onenter = lchan_fsm_wait_rsl_mt_mode_modify_ack_onenter,
+		.action = lchan_fsm_wait_rsl_mt_mode_modify_ack,
+		.in_event_mask = 0
+			| S(LCHAN_EV_RSL_MT_MODE_MODIFY_ACK)
+			| S(LCHAN_EV_RSL_MT_MODE_MODIFY_NACK)
+			,
+		.out_state_mask = 0
+			| S(LCHAN_ST_BORKEN)
+			| S(LCHAN_ST_WAIT_RLL_RTP_ESTABLISH)
+			,
+	},
 	[LCHAN_ST_ESTABLISHED] = {
 		.name = "ESTABLISHED",
 		.onenter = lchan_fsm_established_onenter,
@@ -1233,12 +1357,14 @@ static const struct osmo_fsm_state lchan_fsm_states[] = {
 			| S(LCHAN_EV_RLL_ESTABLISH_IND) /* ignored */
 			| S(LCHAN_EV_RTP_ERROR)
 			| S(LCHAN_EV_RTP_RELEASED)
+			| S(LCHAN_EV_REQUEST_MODE_MODIFY)
 			,
 		.out_state_mask = 0
 			| S(LCHAN_ST_UNUSED)
 			| S(LCHAN_ST_WAIT_RLL_RTP_RELEASED)
 			| S(LCHAN_ST_WAIT_BEFORE_RF_RELEASE)
 			| S(LCHAN_ST_WAIT_RF_RELEASE_ACK)
+			| S(LCHAN_ST_WAIT_RR_CHAN_MODE_MODIF_ACK)
 			,
 	},
 	[LCHAN_ST_WAIT_RLL_RTP_RELEASED] = {
@@ -1324,8 +1450,10 @@ static const struct value_string lchan_fsm_event_names[] = {
 	OSMO_VALUE_STRING(LCHAN_EV_RLL_REL_CONF),
 	OSMO_VALUE_STRING(LCHAN_EV_RSL_RF_CHAN_REL_ACK),
 	OSMO_VALUE_STRING(LCHAN_EV_RLL_ERR_IND),
-	OSMO_VALUE_STRING(LCHAN_EV_CHAN_MODE_MODIF_ACK),
-	OSMO_VALUE_STRING(LCHAN_EV_CHAN_MODE_MODIF_ERROR),
+	OSMO_VALUE_STRING(LCHAN_EV_RR_CHAN_MODE_MODIF_ACK),
+	OSMO_VALUE_STRING(LCHAN_EV_RR_CHAN_MODE_MODIF_ERROR),
+	OSMO_VALUE_STRING(LCHAN_EV_RSL_MT_MODE_MODIFY_ACK),
+	OSMO_VALUE_STRING(LCHAN_EV_RSL_MT_MODE_MODIFY_NACK),
 	{}
 };