HO: fix recovery from failed handover

Do not instruct the MGW to move the RTP to the new lchan before we have
received a HANDOVER DETECT.

Before:

  Chan Activ
  Chan Activ Ack
  IPACC-CRCX
   -ACK
  IPACC-MDCX
   -ACK
  MGCP MDCX --> MGW
  ...
  HANDOVER DETECT
  Call continues on new lchan

In above sequence, if the HANDOVER DETECT times out, the MGW has moved to the
new lchan which never becomes used and is released. Furthermore, from the IPACC
MDCX until the HANDOVER DETECT, the RTP stream would break off momentarily.

After:

  Chan Activ
  Chan Activ Ack
  IPACC-CRCX
   -ACK
  IPACC-MDCX
   -ACK
  ...
  HANDOVER DETECT
  MGCP MDCX --> MGW
  Call continues on new lchan

If the HANDOVER DETECT times out, the call happily continues on the old lchan.

This change is inspired by Ivan Kluchnikov's HO work, who implemented a similar
fix in the openbsc.git codebase (branch fairwaves/master-rebase): his patch
moves ipacc_mdcx() to connect RTP to the new lchan from switch_for_handover()
(which triggered on S_ABISIP_CRCX_ACK, i.e. creation of the new lchan) to later
on in ho_detect() a.k.a. the S_LCHAN_HANDOVER_DETECT signal handler:
http://git.osmocom.org/openbsc/commit/?h=fairwaves/master-rebase&id=9507a7a1ea627e07370c9d264816bb190b3b91b8

This patch does essentially the same: remove the mgcp_handover() call from the
MDCX-ACK handling (creation of the new lchan), and add a signal handler for
S_LCHAN_HANDOVER_DETECT to osmo_bsc_mgcp.c to effect the MGW switchover.

Note, it would have been possible to call mgcp_handover() directly from rx of
the HANDOVER DETECT message, but that produces linking fallout in some utils/
projects, which then need to link the mgcp code as well. That is because those
aren't properly separated from the more complex parts of libbsc. Using the
signal is a bit bloaty, but saves the linking hell for now. I've faced a
similar problem twice recently, it would pay off to separate out the simpler
utils/ and ipaccess/ tools so that they don't need to link all of libbsc and
osmo-bsc, at some point (TM).

Change-Id: Iec58c5fcc5697f1775da7ec0111135108ed1fc8f

4 files changed, 66 insertions, 16 deletions

diff --git a/include/osmocom/bsc/osmo_bsc_mgcp.h b/include/osmocom/bsc/osmo_bsc_mgcp.h
index 1e0633170..dc2ba3475 100644
--- a/include/osmocom/bsc/osmo_bsc_mgcp.h
+++ b/include/osmocom/bsc/osmo_bsc_mgcp.h
@@ -58,5 +58,4 @@ struct mgcp_ctx *mgcp_assignm_req(void *ctx, struct mgcp_client *mgcp, struct os
 				  enum gsm48_chan_mode chan_mode, bool full_rate);
 void mgcp_clear_complete(struct mgcp_ctx *mgcp_ctx, struct msgb *resp);
 void mgcp_ass_complete(struct mgcp_ctx *mgcp_ctx, struct gsm_lchan *lchan);
-void mgcp_handover(struct mgcp_ctx *mgcp_ctx, struct gsm_lchan *ho_lchan);
 void mgcp_free_ctx(struct mgcp_ctx *mgcp_ctx);
diff --git a/src/libbsc/handover_logic.c b/src/libbsc/handover_logic.c
index a30cd0912..cad014426 100644
--- a/src/libbsc/handover_logic.c
+++ b/src/libbsc/handover_logic.c
@@ -325,7 +325,11 @@ static int ho_rsl_detect(struct gsm_lchan *new_lchan)
 		return -ENODEV;
 	}
 
-	/* FIXME: do we actually want to do something here ? */
+	LOGP(DHO, LOGL_DEBUG, "%s Handover RACH detected\n", gsm_lchan_name(new_lchan));
+
+	/* This is just for logging on the DHO category. The actual MGCP switchover happens in
+	 * osmo_bsc_mgcp.c by receiving the same S_LCHAN_HANDOVER_DETECT signal.
+	 * (Calling mgcp_handover() directly currently breaks linking in utils/...) */
 
 	return 0;
 }
diff --git a/src/osmo-bsc/osmo_bsc_audio.c b/src/osmo-bsc/osmo_bsc_audio.c
index 433dc6c5f..ceec469ca 100644
--- a/src/osmo-bsc/osmo_bsc_audio.c
+++ b/src/osmo-bsc/osmo_bsc_audio.c
@@ -50,11 +50,6 @@ static int handle_abisip_signal(unsigned int subsys, unsigned int signal,
 
 	switch (signal) {
 	case S_ABISIP_CRCX_ACK:
-		/*
-		 * TODO: handle handover here... then the audio should go to
-		 * the old mgcp port..
-		 */
-
 		/* we can ask it to connect now */
 		LOGP(DMSC, LOGL_DEBUG, "Connecting BTS to port: %d conn: %d\n",
 		     con->sccp_con->user_plane.rtp_port, lchan->abis_ip.conn_id);
@@ -77,14 +72,11 @@ static int handle_abisip_signal(unsigned int subsys, unsigned int signal,
 
 	case S_ABISIP_MDCX_ACK:
 		if (con->ho_lchan) {
-			/* NOTE: When an ho_lchan exists, the MDCX is part of an
-			 * handover operation (intra-bsc). This means we will not
-			 * inform the MSC about the event, which means that no
-			 * assignment complete message is transmitted, we just
-			 * inform the logic that controls the MGW about the new
-			 * connection info */
-			LOGP(DMSC, LOGL_INFO,"RTP connection handover initiated...\n");
-			mgcp_handover(con->sccp_con->user_plane.mgcp_ctx, con->ho_lchan);
+			LOGP(DHO, LOGL_DEBUG, "%s -> %s BTS sent MDCX ACK\n", gsm_lchan_name(lchan),
+			     gsm_lchan_name(con->ho_lchan));
+			/* No need to do anything for handover here. As soon as a HANDOVER DETECT
+			 * happens, osmo_bsc_mgcp.c will trigger the MGCP MDCX towards MGW by
+			 * receiving an S_LCHAN_HANDOVER_DETECT signal. */
 		} else if (is_ipaccess_bts(conn_get_bts(con)) && con->sccp_con->user_plane.rtp_ip) {
 			/* NOTE: This is only relevant on AoIP networks with
 			 * IPA based base stations. See also osmo_bsc_api.c,
diff --git a/src/osmo-bsc/osmo_bsc_mgcp.c b/src/osmo-bsc/osmo_bsc_mgcp.c
index 519eaf4b1..bdf2dd283 100644
--- a/src/osmo-bsc/osmo_bsc_mgcp.c
+++ b/src/osmo-bsc/osmo_bsc_mgcp.c
@@ -23,6 +23,7 @@
 #include <osmocom/bsc/osmo_bsc_mgcp.h>
 #include <osmocom/bsc/debug.h>
 #include <osmocom/bsc/osmo_bsc.h>
+#include <osmocom/bsc/signal.h>
 #include <osmocom/core/logging.h>
 #include <osmocom/core/utils.h>
 #include <osmocom/core/timer.h>
@@ -1048,7 +1049,7 @@ void mgcp_ass_complete(struct mgcp_ctx *mgcp_ctx, struct gsm_lchan *lchan)
  * Parameter:
  * mgcp_ctx: context information (FSM, and pointer to external system data)
  * ho_lchan: the lchan on the new BTS */
-void mgcp_handover(struct mgcp_ctx *mgcp_ctx, struct gsm_lchan *ho_lchan)
+static void mgcp_handover(struct mgcp_ctx *mgcp_ctx, struct gsm_lchan *ho_lchan)
 {
 	OSMO_ASSERT(mgcp_ctx);
 	OSMO_ASSERT(ho_lchan);
@@ -1068,6 +1069,59 @@ void mgcp_handover(struct mgcp_ctx *mgcp_ctx, struct gsm_lchan *ho_lchan)
 	return;
 }
 
+/* GSM 08.58 HANDOVER DETECT has been received */
+static int mgcp_sig_ho_detect(struct gsm_lchan *new_lchan)
+{
+	struct gsm_subscriber_connection *conn;
+	if (!new_lchan) {
+		LOGP(DHO, LOGL_ERROR, "HO Detect signal for NULL lchan\n");
+		return -EINVAL;
+	}
+
+	conn = new_lchan->conn;
+
+	if (!conn) {
+		LOGP(DHO, LOGL_ERROR, "%s HO Detect for lchan without conn\n",
+		     gsm_lchan_name(new_lchan));
+		return -EINVAL;
+	}
+
+	if (!conn->sccp_con) {
+		LOGP(DHO, LOGL_ERROR, "%s HO Detect for conn without sccp_con\n",
+		     gsm_lchan_name(new_lchan));
+		return -EINVAL;
+	}
+
+	if (!conn->sccp_con->user_plane.mgcp_ctx) {
+		LOGP(DHO, LOGL_ERROR, "%s HO Detect for conn without MGCP ctx\n",
+		     gsm_lchan_name(new_lchan));
+		return -EINVAL;
+	}
+
+	mgcp_handover(conn->sccp_con->user_plane.mgcp_ctx, new_lchan);
+	return 0;
+}
+
+static int mgcp_sig_cb(unsigned int subsys, unsigned int signal,
+				void *handler_data, void *signal_data)
+{
+	struct lchan_signal_data *lchan_data;
+
+	switch (subsys) {
+	case SS_LCHAN:
+		lchan_data = signal_data;
+		switch (signal) {
+		case S_LCHAN_HANDOVER_DETECT:
+			return mgcp_sig_ho_detect(lchan_data->lchan);
+		}
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
 /* Free an existing mgcp context gracefully
  * Parameter:
  * mgcp_ctx: context information (FSM, and pointer to external system data) */
@@ -1090,4 +1144,5 @@ void mgcp_free_ctx(struct mgcp_ctx *mgcp_ctx)
 void mgcp_init(struct gsm_network *net)
 {
 	osmo_fsm_register(&fsm_bsc_mgcp);
+	osmo_signal_register_handler(SS_LCHAN, mgcp_sig_cb, NULL);
 }