rsl: Avoid double channel release procedure in error conditions

When we receive an ERROR INDICATION and CONNECTION FAILURE we
might call rsl_rf_chan_release multiple times. The channel release
handling is still a bit messy and there too many paths that lead
to the call.

1.) In case we receive an ERROR INDICATION for SAPI=3. A RLL
error signal will be emitted that leads to the release of the
channel through the SMS code in case of the NITB.  The call to
rsl_rf_chan_release might be a double release.

2.) In case a CONNECTION FAILURE is received when the release
process has already been started we would unconditionally
call rsl_rf_chan_release as well.

Because the lchan state is changed by the callers of the
rsl_rf_chan_release we can not move the state checking into this
code but need to do it in the caller. The issue was seen in a trace
from Rhizomatica and I created the DoubleRelease.st to re-produce
the issue and verified that we have no duplicate RF Channel Releses.

The other option would be to introduce a new state to track
the release process and see if we have already released SAPIs
deactivated the SACCH or such. We can not simply look at these
as for a channel that fails to activate they will be null already.

1 files changed, 20 insertions, 5 deletions

diff --git a/openbsc/src/libbsc/abis_rsl.c b/openbsc/src/libbsc/abis_rsl.c
index 383d79b4d..2ab66d650 100644
--- a/openbsc/src/libbsc/abis_rsl.c
+++ b/openbsc/src/libbsc/abis_rsl.c
@@ -1013,9 +1013,15 @@ static int rsl_rx_conn_fail(struct msgb *msg)
 	struct abis_rsl_dchan_hdr *dh = msgb_l2(msg);
 	struct tlv_parsed tp;
 
-	/* FIXME: print which channel */
-	LOGP(DRSL, LOGL_NOTICE, "%s CONNECTION FAIL: RELEASING ",
-	     gsm_lchan_name(msg->lchan));
+	LOGP(DRSL, LOGL_NOTICE, "%s CONNECTION FAIL: RELEASING state %s ",
+	     gsm_lchan_name(msg->lchan),
+	     gsm_lchans_name(msg->lchan->state));
+
+	/* We might have already received an ERROR INDICATION */
+	if (msg->lchan->state != LCHAN_S_ACTIVE) {
+		LOGPC(DRSL, LOGL_NOTICE, "\n");
+		return 0;
+	}
 
 	rsl_tlv_parse(&tp, dh->data, msgb_l2len(msg)-sizeof(*dh));
 
@@ -1587,12 +1593,21 @@ static int rsl_rx_rll_err_ind(struct msgb *msg)
 	}
 
 	rlm_cause = *TLVP_VAL(&tp, RSL_IE_RLM_CAUSE);
-	LOGP(DRLL, LOGL_ERROR, "%s ERROR INDICATION cause=%s\n",
+	LOGP(DRLL, LOGL_ERROR, "%s ERROR INDICATION cause=%s in state=%s\n",
 		gsm_lchan_name(msg->lchan),
-		rsl_rlm_cause_name(rlm_cause));
+		rsl_rlm_cause_name(rlm_cause),
+		gsm_lchans_name(msg->lchan->state));
+
+	/* If the channel is already failing no need to inform anyone. */
+	if (msg->lchan->state != LCHAN_S_ACTIVE)
+		return 0;
 
 	rll_indication(msg->lchan, rllh->link_id, BSC_RLLR_IND_ERR_IND);
 
+	/* The channel might have been released already. */
+	if (msg->lchan->state != LCHAN_S_ACTIVE)
+		return 0;
+
 	if (rlm_cause == RLL_CAUSE_T200_EXPIRED) {
 		osmo_counter_inc(msg->lchan->ts->trx->bts->network->stats.chan.rll_err);
 		return rsl_rf_chan_release(msg->lchan, 1, SACCH_DEACTIVATE);