[PATCH] osmo-bsc[master]: HO: fix recovery from failed handover

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/.

Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Fri Jan 19 03:03:37 UTC 2018


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

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
---
M include/osmocom/bsc/osmo_bsc_mgcp.h
M src/libbsc/handover_logic.c
M src/osmo-bsc/osmo_bsc_audio.c
M src/osmo-bsc/osmo_bsc_mgcp.c
4 files changed, 66 insertions(+), 16 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/91/5891/1

diff --git a/include/osmocom/bsc/osmo_bsc_mgcp.h b/include/osmocom/bsc/osmo_bsc_mgcp.h
index 1e06331..dc2ba34 100644
--- a/include/osmocom/bsc/osmo_bsc_mgcp.h
+++ b/include/osmocom/bsc/osmo_bsc_mgcp.h
@@ -58,5 +58,4 @@
 				  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 a30cd09..cad0144 100644
--- a/src/libbsc/handover_logic.c
+++ b/src/libbsc/handover_logic.c
@@ -325,7 +325,11 @@
 		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 433dc6c..ceec469 100644
--- a/src/osmo-bsc/osmo_bsc_audio.c
+++ b/src/osmo-bsc/osmo_bsc_audio.c
@@ -50,11 +50,6 @@
 
 	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 @@
 
 	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 519eaf4..6f3480b 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 @@
  * 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);
@@ -1066,6 +1067,59 @@
 	osmo_fsm_inst_dispatch(mgcp_ctx->fsm, EV_HANDOVER, mgcp_ctx);
 
 	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
@@ -1090,4 +1144,5 @@
 void mgcp_init(struct gsm_network *net)
 {
 	osmo_fsm_register(&fsm_bsc_mgcp);
+	osmo_signal_register_handler(SS_LCHAN, mgcp_sig_cb, NULL);
 }

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iec58c5fcc5697f1775da7ec0111135108ed1fc8f
Gerrit-PatchSet: 1
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>



More information about the gerrit-log mailing list