bsc_msc_ip: Attempt to handle assignment failures more properly

1.) when we do get a assignment failure from the MS. It is coming
    on the old channel and not the new one. Fix the comparison. Also
    always reset the msc_data to NULL before dropping the reference
2.) the LCHAN signal handler in bssap.c claims that the T10 expire
    cb should free the secondary channel. It currently does not do
    it and we have to do it now...

the whole thing was not tested and even after this commit this
behavior is not heavily excercised... with OsmocoreBB we would be
able to do this in the future.

2 files changed, 47 insertions, 6 deletions

diff --git a/openbsc/src/bsc_msc_ip.c b/openbsc/src/bsc_msc_ip.c
index 0a1d1be27..ce37fe590 100644
--- a/openbsc/src/bsc_msc_ip.c
+++ b/openbsc/src/bsc_msc_ip.c
@@ -351,7 +351,7 @@ static int handle_ass_compl(struct msgb *msg)
 	}
 
 	if (msgb_l3len(msg) - sizeof(*gh) != 1) {
-		LOGP(DMSC, LOGL_ERROR, "assignment failure invalid: %d\n",
+		LOGP(DMSC, LOGL_ERROR, "assignment compl invalid: %d\n",
 			msgb_l3len(msg) - sizeof(*gh));
 		put_subscr_con(&msg->lchan->conn, 0);
 		return -1;
@@ -382,6 +382,7 @@ static int handle_ass_compl(struct msgb *msg)
  */
 static int handle_ass_fail(struct msgb *msg)
 {
+	u_int8_t *rr_cause;
 	struct gsm48_hdr *gh = msgb_l3(msg);
 
 	LOGP(DMSC, LOGL_ERROR, "ASSIGNMENT FAILURE from MS, forwarding to MSC\n");
@@ -391,21 +392,26 @@ static int handle_ass_fail(struct msgb *msg)
 		return -1;
 	}
 
-	if (msg->lchan->msc_data->secondary_lchan != msg->lchan) {
-		LOGP(DMSC, LOGL_NOTICE, "Wrong assignment complete.\n");
+	/* assignment failure comes on the old link */
+	if (msg->lchan->msc_data->lchan != msg->lchan) {
+		LOGP(DMSC, LOGL_NOTICE, "Failure should come on the old link.\n");
+		msg->lchan->msc_data = NULL;
 		put_subscr_con(&msg->lchan->conn, 0);
 		return -1;
 	}
 
+	/* Giving up the secondary will happen in bssap */
 	if (msgb_l3len(msg) - sizeof(*gh) != 1) {
 		LOGP(DMSC, LOGL_ERROR, "assignment failure invalid: %d\n",
 			msgb_l3len(msg) - sizeof(*gh));
-		put_subscr_con(&msg->lchan->conn, 0);
-		return -1;
+		rr_cause = NULL;
+	} else {
+		rr_cause = &gh->data[0];
 	}
 
+	/* this will also free the secondary channel */
 	gsm0808_send_assignment_failure(msg->lchan,
-		GSM0808_CAUSE_RADIO_INTERFACE_MESSAGE_FAILURE, &gh->data[0]);
+		GSM0808_CAUSE_RADIO_INTERFACE_MESSAGE_FAILURE, rr_cause);
 	return 1;
 }
 
diff --git a/openbsc/src/bssap.c b/openbsc/src/bssap.c
index ee51a392f..edbe66717 100644
--- a/openbsc/src/bssap.c
+++ b/openbsc/src/bssap.c
@@ -295,14 +295,48 @@ reject:
 }
 
 /*
+ * handle network failures... and free the secondary lchan
+ */
+static void bssmap_free_secondary(struct bss_sccp_connection_data *data)
+{
+	struct gsm_lchan *lchan;
+
+	if (!data || !data->secondary_lchan)
+		return;
+
+	lchan = data->secondary_lchan;
+	if (lchan->msc_data != data) {
+		LOGP(DMSC, LOGL_ERROR, "MSC data does not match on lchan and cb.\n");
+		data->secondary_lchan = NULL;
+	}
+
+	/* give up additional data */
+	lchan->msc_data->secondary_lchan = NULL;
+	if (lchan->msc_data->lchan == lchan)
+		lchan->msc_data->lchan = NULL;
+	lchan->msc_data = NULL;
+
+	/* give up the new channel to not do a SACCH deactivate */
+	subscr_put(lchan->conn.subscr);
+	lchan->conn.subscr = NULL;
+	put_subscr_con(&lchan->conn, 1);
+}
+
+/*
  * Handle the network configurable T10 parameter
  */
 static void bssmap_t10_fired(void *_conn)
 {
+	struct bss_sccp_connection_data *msc_data;
 	struct sccp_connection *conn = (struct sccp_connection *) _conn;
 	struct msgb *resp;
 
 	LOGP(DMSC, LOGL_ERROR, "T10 fired, assignment failed: %p\n", conn);
+
+	/* free the secondary channel if we have one */
+	msc_data = conn->data_ctx;
+	bssmap_free_secondary(msc_data);
+
 	resp = bssmap_create_assignment_failure(
 		GSM0808_CAUSE_NO_RADIO_RESOURCE_AVAILABLE, NULL);
 	if (!resp) {
@@ -1293,6 +1327,7 @@ void gsm0808_send_assignment_failure(struct gsm_lchan *lchan, u_int8_t cause, u_
 	struct msgb *resp;
 
 	bsc_del_timer(&lchan->msc_data->T10);
+	bssmap_free_secondary(lchan->msc_data);
 	resp = bssmap_create_assignment_failure(cause, rr_value);
 	if (!resp) {
 		LOGP(DMSC, LOGL_ERROR, "Allocation failure: %p\n", lchan_get_sccp(lchan));