[PATCH 2/3] rsl: Avoid double channel release procedure in error conditions

From: Holger Hans Peter Freyther <holger(a)moiji-mobile.com> 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. --- openbsc/src/libbsc/abis_rsl.c | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/openbsc/src/libbsc/abis_rsl.c b/openbsc/src/libbsc/abis_rsl.c index 984fa7e..de76d0f 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); -- 1.9.1