[PATCH] openbsc[master]: lchan: Release channel in case of late activation ack

[Neels Hofmeyr]

Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

    https://gerrit.osmocom.org/713

to look at the new patch set (#5).

lchan: Release channel in case of late activation ack

In case of the sysmoBTS and receiving a channel activation ack on a channel
that was marked as broken, release it again.

Use a normal release without SACCH deactivation and release the rqd_ta data.

Also add a local variable 'ts' to shorten some lines.

The typical situation where this would occur is with high latency between BTS
and BSC (or NITB). If a channel activation ack does not arrive in time, a
channel is marked broken, and never recovers after that. This patch will
release the channel again, which will remove the BROKEN_UNUSABLE state and
makes lchan available again. Reported by Rhizomatica.

However, in case of packet loss, i.e. when the channel activation ack never
arrives at the BSC, this patch does not provide a resolution of the
BROKEN_UNUSABLE state.

On dynamic timeslots: clearing the dyn ts state could possibly happen in
lchan_free() instead of in rsl_rx_chan_act_ack(). That's to be done in a
separate patch, if at all.

Tweaked-By: nhofmeyr
Change-Id: I63dc0deaf15ba7c21e20b1e0c7b85f0437e183ed
---
M openbsc/src/libbsc/abis_rsl.c
1 file changed, 28 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/13/713/5

diff --git a/openbsc/src/libbsc/abis_rsl.c b/openbsc/src/libbsc/abis_rsl.c
index f672750..e3a98f5 100644
--- a/openbsc/src/libbsc/abis_rsl.c
+++ b/openbsc/src/libbsc/abis_rsl.c
@@ -1175,6 +1175,7 @@
 {
 	struct abis_rsl_dchan_hdr *rslh = msgb_l2(msg);
 	struct gsm_lchan *lchan = msg->lchan;
+	struct gsm_bts_trx_ts *ts = lchan->ts;
 
 	/* BTS has confirmed channel activation, we now need
 	 * to assign the activated channel to the MS */
@@ -1184,8 +1185,32 @@
 	osmo_timer_del(&lchan->act_timer);
 
 	if (lchan->state == LCHAN_S_BROKEN) {
-		LOGP(DRSL, LOGL_NOTICE, "%s CHAN ACT ACK for broken channel.\n",
-			gsm_lchan_name(lchan));
+		int do_release = is_sysmobts_v2(ts->trx->bts);
+		LOGP(DRSL, LOGL_NOTICE, "%s CHAN ACT ACK for broken channel. %s\n",
+			gsm_lchan_name(lchan),
+			do_release ? "Releasing it" : "Keeping it broken");
+		if (do_release) {
+			talloc_free(lchan->rqd_ref);
+			lchan->rqd_ref = NULL;
+			lchan->rqd_ta = 0;
+			rsl_lchan_set_state(msg->lchan, LCHAN_S_ACTIVE);
+			if (ts->pchan == GSM_PCHAN_TCH_F_TCH_H_PDCH) {
+				/*
+				 * lchan_act_tmr_cb() already called
+				 * lchan_free() and cleared the lchan->type, so
+				 * calling dyn_ts_switchover_complete() here
+				 * would not have the desired effect of
+				 * mimicking an activated lchan that we can
+				 * release. Instead hack the dyn ts state to
+				 * make sure that rsl_rx_rf_chan_rel_ack() will
+				 * switch back to PDCH, i.e. have pchan_is ==
+				 * pchan_want, both != GSM_PCHAN_PDCH:
+				 */
+				ts->dyn.pchan_is = GSM_PCHAN_NONE;
+				ts->dyn.pchan_want = GSM_PCHAN_NONE;
+			}
+			rsl_rf_chan_release(msg->lchan, 0, SACCH_NONE);
+		}
 		return 0;
 	}
 
@@ -1195,7 +1220,7 @@
 			gsm_lchans_name(lchan->state));
 	rsl_lchan_set_state(lchan, LCHAN_S_ACTIVE);
 
-	if (lchan->ts->pchan == GSM_PCHAN_TCH_F_TCH_H_PDCH)
+	if (ts->pchan == GSM_PCHAN_TCH_F_TCH_H_PDCH)
 		dyn_ts_switchover_complete(lchan);
 
 	if (lchan->rqd_ref) {

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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I63dc0deaf15ba7c21e20b1e0c7b85f0437e183ed
Gerrit-PatchSet: 5
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Holger Freyther <holger at freyther.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Holger Freyther <holger at freyther.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: keith <keith at rhizomatica.org>