[PATCH] openbsc[master]: dyn PDCH: set lchan->state after PDCH DEACT / before PDCH ACT

[Neels Hofmeyr]

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

dyn PDCH: set lchan->state after PDCH DEACT / before PDCH ACT

Do the PDCH DE/ACT before we set the lchan->state to De-/Activation Requested.
It makes more sense semantically to change PDCH mode while the lchan is still
in NONE status. Thus slightly move some invocations:

PDCH ACT: Free the lchan before PDCH activation. Hence remove the lchan_free()
call from the rsl_rx_pdch_act_ack() code path; it used to do the PDCH
activation first and call lchan_free() in the callback.

PDCH DEACT: Set the (TCH) Activation Requested state only within
rsl_chan_activate_lchan(), after the PDCH deact is complete.

Channel allocator: don't pick channels that have a PDCH PENDING flag set, to
avoid using channels that are still in PDCH switchover (despite their state
being NONE).

The lchan_may_change_pdch() sanity checks are becoming a lot simpler.

Change-Id: I4206dd4808e21c3e59393ea7f5ab4f438afff066
---
M openbsc/src/libbsc/abis_rsl.c
M openbsc/src/libbsc/chan_alloc.c
2 files changed, 12 insertions(+), 23 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/84/284/1

diff --git a/openbsc/src/libbsc/abis_rsl.c b/openbsc/src/libbsc/abis_rsl.c
index 3aefd9c..fc56862 100644
--- a/openbsc/src/libbsc/abis_rsl.c
+++ b/openbsc/src/libbsc/abis_rsl.c
@@ -479,6 +479,8 @@
 		return rsl_ipacc_pdch_activate(lchan->ts, 0);
 	}
 
+	rsl_lchan_set_state(lchan, LCHAN_S_ACT_REQ);
+
 	ta = lchan->rqd_ta;
 
 	/* BS11 requires TA shifted by 2 bits */
@@ -756,11 +758,11 @@
 			gsm_lchan_name(lchan),
 			gsm_lchans_name(lchan->state));
 
+	do_lchan_free(lchan);
+
 	/* Put PDCH channel back into PDCH mode first */
 	if (lchan->ts->pchan == GSM_PCHAN_TCH_F_PDCH)
 		return rsl_ipacc_pdch_activate(lchan->ts, 1);
-
-	do_lchan_free(lchan);
 
 	return 0;
 }
@@ -1204,7 +1206,6 @@
 static bool lchan_may_change_pdch(struct gsm_lchan *lchan, bool pdch_act)
 {
 	struct gsm_bts_trx_ts *ts;
-	bool ok;
 
 	OSMO_ASSERT(lchan);
 
@@ -1222,22 +1223,7 @@
 		return false;
 	}
 
-	/* During BTS initialization, we expect to PDCH ACT in channel state ==
-	 * NONE. However, during dynamic channel switching, we will PDCH ACT
-	 * while in state DEACTIVATION REQUESTED (we're deactivating TCH/F
-	 * while activating PDCH); and we will PDCH DEACT while in state
-	 * ACTIVATION REQUESTED (we're activating TCH/F while deactivating
-	 * PDCH). */
-	/* FIXME: rather switch PDCH while in state NONE, i.e. after
-	 * deactivation of TCH/F and before activation of TCH/F? */
-	if (pdch_act)
-		ok = (lchan->state == LCHAN_S_NONE
-		      || lchan->state == LCHAN_S_REL_REQ);
-	else
-		ok = (lchan->state == LCHAN_S_NONE
-		      || lchan->state == LCHAN_S_ACT_REQ);
-
-	if (!ok) {
+	if (lchan->state != LCHAN_S_NONE) {
 		LOGP(DRSL, LOGL_ERROR, "(bts %u, trx %u, ts %u, pchan %s)"
 		     " Rx PDCH %s ACK in unexpected state: %s\n",
 		     ts->trx->bts->nr, ts->trx->nr, ts->nr,
@@ -1256,9 +1242,6 @@
 
 	msg->lchan->ts->flags |= TS_F_PDCH_ACTIVE;
 	msg->lchan->ts->flags &= ~TS_F_PDCH_ACT_PENDING;
-
-	/* We have activated PDCH, so now the channel is available again. */
-	do_lchan_free(msg->lchan);
 
 	return 0;
 }
@@ -1532,7 +1515,6 @@
 		return -ENOMEM;
 	}
 
-	rsl_lchan_set_state(lchan, LCHAN_S_ACT_REQ);
 	memcpy(lchan->rqd_ref, rqd_ref, sizeof(*rqd_ref));
 	lchan->rqd_ta = rqd_ta;
 
diff --git a/openbsc/src/libbsc/chan_alloc.c b/openbsc/src/libbsc/chan_alloc.c
index 4715780..f4ed530 100644
--- a/openbsc/src/libbsc/chan_alloc.c
+++ b/openbsc/src/libbsc/chan_alloc.c
@@ -43,6 +43,13 @@
 			return 0;
 	}
 
+	/* If a dyn PDCH channel is busy changing, it is already taken or not
+	 * yet available. */
+	if (ts->pchan == GSM_PCHAN_TCH_F_PDCH) {
+		if (ts->flags & TS_F_PDCH_PENDING_MASK)
+			return 0;
+	}
+
 	return 1;
 }
 

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

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