aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorNeels Hofmeyr <nhofmeyr@sysmocom.de>2016-08-24 16:57:31 +0200
committerHarald Welte <laforge@gnumonks.org>2016-08-27 02:23:47 +0000
commitb74a2c8e290e58a371c07b9d8a82872e54de6a2f (patch)
treeb58ef2e5ccbd2fe51d32c821aed85c40d1777c35
parentcd150a8f7473347b17728d0797bc6464441dde9a (diff)
dyn TS: clearly use lchan[0], fixing minor confusion
The dyn_ts_switchover_*() functions made the impression that they act on a specific lchan of a timeslot. The assumption that we would remember to use e.g. lchan[1] across a PDCH deactivation is brain damaged to begin with; and factually we always use lchan[0] anyway (the only case for using lchan[1] would be when switching to TCH/H, but the channel allocator will always return lchan[0] for that). Instead of the brain damaged lchan args, use a ts arg across all dyn_ts_switchover_*() functions, with one exception: The dyn_ts_switchover_complete() actually receives an RSL activation ack message on a specific lchan and needs to evaluate its lchan type. This will always be lchan[0] as it is now, but we should stick with the lchan the message was sent for. For PDCH, a check to use lchan[0] already existed, when composing the ACT message in rsl_chan_activate_lchan_as_pdch(). Replace with an assertion. Adjust all callers to pass ts instead of lchan. In dyn_ts_switchover_start(), there was a dead code check that jumps to switchover_complete() in case the pchan already matches. This never hits, because we only call dyn_ts_switchover_start() when pchans mismatch. So avoid guessing at passing lchan[0] to dyn_ts_switchover_complete() by not calling it at all but logging an error instead. In rsl_chan_activate_lchan(), we remember some values before going into switchover from PDCH. Explicitly store them in lchan[0], because after a PDCH release we have always and will activate no other than lchan[0]. In dyn_ts_switchover_continue(), move the check for any existing lchan->rqd_ref further above, and more correctly check all lchans that were so far valid on the TS, instead of just one. This partly prepares for a subsequent commit to fix the act_timer use for dyn TS: with the old lchan arg, we might schedule an activation timer on lchan[1] but receive an ack on lchan[0] (for PDCH), leading to an act_timer expiry. Change-Id: I3f5d48a9bdaa49a42a1908d4a03744638c59796a
-rw-r--r--openbsc/include/openbsc/abis_rsl.h2
-rw-r--r--openbsc/src/libbsc/abis_rsl.c94
-rw-r--r--openbsc/src/libbsc/bsc_dyn_ts.c2
3 files changed, 56 insertions, 42 deletions
diff --git a/openbsc/include/openbsc/abis_rsl.h b/openbsc/include/openbsc/abis_rsl.h
index 8313f489e..758c5557a 100644
--- a/openbsc/include/openbsc/abis_rsl.h
+++ b/openbsc/include/openbsc/abis_rsl.h
@@ -108,7 +108,7 @@ int rsl_start_t3109(struct gsm_lchan *lchan);
int rsl_direct_rf_release(struct gsm_lchan *lchan);
void dyn_ts_init(struct gsm_bts_trx_ts *ts);
-int dyn_ts_switchover_start(struct gsm_lchan *lchan,
+int dyn_ts_switchover_start(struct gsm_bts_trx_ts *ts,
enum gsm_phys_chan_config to_pchan);
#endif /* RSL_MT_H */
diff --git a/openbsc/src/libbsc/abis_rsl.c b/openbsc/src/libbsc/abis_rsl.c
index d04b70739..42281ba8f 100644
--- a/openbsc/src/libbsc/abis_rsl.c
+++ b/openbsc/src/libbsc/abis_rsl.c
@@ -55,8 +55,8 @@ enum sacch_deact {
static int rsl_send_imm_assignment(struct gsm_lchan *lchan);
static void error_timeout_cb(void *data);
-static int dyn_ts_switchover_continue(struct gsm_lchan *lchan);
-static int dyn_ts_switchover_failed(struct gsm_lchan *lchan, int rc);
+static int dyn_ts_switchover_continue(struct gsm_bts_trx_ts *ts);
+static int dyn_ts_switchover_failed(struct gsm_bts_trx_ts *ts, int rc);
static void dyn_ts_switchover_complete(struct gsm_lchan *lchan);
static void send_lchan_signal(int sig_no, struct gsm_lchan *lchan,
@@ -460,9 +460,9 @@ static int rsl_chan_activate_lchan_as_pdch(struct gsm_lchan *lchan)
struct abis_rsl_dchan_hdr *dh;
/* This might be called after release of the second lchan of a TCH/H,
- * but PDCH activation should always happen on the first lchan. So
- * switch to lchan->nr == 0. */
- lchan = lchan->ts->lchan;
+ * but PDCH activation must always happen on the first lchan. Make sure
+ * the calling code passes the correct lchan. */
+ OSMO_ASSERT(lchan == lchan->ts->lchan);
rsl_lchan_set_state(lchan, LCHAN_S_ACT_REQ);
@@ -521,16 +521,21 @@ int rsl_chan_activate_lchan(struct gsm_lchan *lchan, uint8_t act_type,
enum gsm_phys_chan_config pchan_want;
pchan_want = pchan_for_lchant(lchan->type);
if (lchan->ts->dyn.pchan_is != pchan_want) {
- lchan->dyn.act_type = act_type,
- lchan->dyn.ho_ref = ho_ref;
- lchan->dyn.rqd_ref = lchan->rqd_ref;
- lchan->dyn.rqd_ta = lchan->rqd_ta;
+ /*
+ * Make sure to record on lchan[0] so that we'll find
+ * it after the PDCH release.
+ */
+ struct gsm_lchan *lchan0 = lchan->ts->lchan;
+ lchan0->dyn.act_type = act_type,
+ lchan0->dyn.ho_ref = ho_ref;
+ lchan0->dyn.rqd_ref = lchan->rqd_ref;
+ lchan0->dyn.rqd_ta = lchan->rqd_ta;
lchan->rqd_ref = NULL;
lchan->rqd_ta = 0;
DEBUGP(DRSL, "%s saved rqd_ref=%p ta=%u\n",
- gsm_lchan_name(lchan), lchan->rqd_ref,
- lchan->rqd_ta);
- return dyn_ts_switchover_start(lchan, pchan_want);
+ gsm_lchan_name(lchan0), lchan0->rqd_ref,
+ lchan0->rqd_ta);
+ return dyn_ts_switchover_start(lchan->ts, pchan_want);
}
}
@@ -774,7 +779,7 @@ static void error_timeout_cb(void *data)
rsl_ipacc_pdch_activate(lchan->ts, 1);
if (dyn_ts_should_switch_to_pdch(lchan->ts))
- dyn_ts_switchover_start(lchan, GSM_PCHAN_PDCH);
+ dyn_ts_switchover_start(lchan->ts, GSM_PCHAN_PDCH);
}
static int rsl_rx_rf_chan_rel_ack(struct gsm_lchan *lchan);
@@ -910,11 +915,11 @@ static int rsl_rx_rf_chan_rel_ack(struct gsm_lchan *lchan)
/* (a) */
if (ts->dyn.pchan_is != ts->dyn.pchan_want)
- return dyn_ts_switchover_continue(lchan);
+ return dyn_ts_switchover_continue(ts);
/* (b) */
if (dyn_ts_should_switch_to_pdch(ts))
- return dyn_ts_switchover_start(lchan, GSM_PCHAN_PDCH);
+ return dyn_ts_switchover_start(ts, GSM_PCHAN_PDCH);
}
/*
@@ -2338,11 +2343,10 @@ static int abis_rsl_rx_ipacc(struct msgb *msg)
return rc;
}
-int dyn_ts_switchover_start(struct gsm_lchan *lchan,
+int dyn_ts_switchover_start(struct gsm_bts_trx_ts *ts,
enum gsm_phys_chan_config to_pchan)
{
int ss;
- struct gsm_bts_trx_ts *ts = lchan->ts;
int rc = -EIO;
OSMO_ASSERT(ts->pchan == GSM_PCHAN_TCH_F_TCH_H_PDCH);
@@ -2360,15 +2364,14 @@ int dyn_ts_switchover_start(struct gsm_lchan *lchan,
if (ts->dyn.pchan_is == to_pchan) {
LOGP(DRSL, LOGL_INFO,
- "%s %s Already is in %s mode, skipping switchover.\n",
+ "%s %s Already is in %s mode, cannot switchover.\n",
gsm_ts_name(ts), gsm_pchan_name(ts->pchan),
gsm_pchan_name(to_pchan));
- dyn_ts_switchover_complete(lchan);
- return 0;
+ return -EINVAL;
}
/* Paranoia: let's make sure all is indeed released. */
- for (ss = 0; ss < ts_subslots(lchan->ts); ss++) {
+ for (ss = 0; ss < ts_subslots(ts); ss++) {
struct gsm_lchan *lc = &ts->lchan[ss];
if (lc->state != LCHAN_S_NONE) {
LOGP(DRSL, LOGL_ERROR,
@@ -2386,16 +2389,16 @@ int dyn_ts_switchover_start(struct gsm_lchan *lchan,
/*
* To switch from PDCH, we need to initiate the release from the BSC
* side. dyn_ts_switchover_continue() will be called from
- * rsl_rx_rf_chan_rel_ack().
+ * rsl_rx_rf_chan_rel_ack(). PDCH is always on lchan[0].
*/
if (ts->dyn.pchan_is == GSM_PCHAN_PDCH) {
- rsl_lchan_set_state(lchan, LCHAN_S_REL_REQ);
- rc = rsl_rf_chan_release(lchan, 0, SACCH_NONE);
+ rsl_lchan_set_state(ts->lchan, LCHAN_S_REL_REQ);
+ rc = rsl_rf_chan_release(ts->lchan, 0, SACCH_NONE);
if (rc) {
LOGP(DRSL, LOGL_ERROR,
"%s RSL RF Chan Release failed\n",
gsm_ts_and_pchan_name(ts));
- return dyn_ts_switchover_failed(lchan, rc);
+ return dyn_ts_switchover_failed(ts, rc);
}
return 0;
}
@@ -2405,15 +2408,16 @@ int dyn_ts_switchover_start(struct gsm_lchan *lchan,
* rsl_rx_rf_chan_rel_ack(), i.e. release is complete. Go ahead and
* activate as new type. This will always be PDCH.
*/
- return dyn_ts_switchover_continue(lchan);
+ return dyn_ts_switchover_continue(ts);
}
-static int dyn_ts_switchover_continue(struct gsm_lchan *lchan)
+static int dyn_ts_switchover_continue(struct gsm_bts_trx_ts *ts)
{
int rc;
uint8_t act_type;
uint8_t ho_ref;
- struct gsm_bts_trx_ts *ts = lchan->ts;
+ int ss;
+ struct gsm_lchan *lchan;
OSMO_ASSERT(ts->pchan == GSM_PCHAN_TCH_F_TCH_H_PDCH);
DEBUGP(DRSL, "%s switchover: release complete,"
@@ -2427,6 +2431,25 @@ static int dyn_ts_switchover_continue(struct gsm_lchan *lchan)
gsm_ts_and_pchan_name(ts));
return 0;
}
+
+ for (ss = 0; ss < ts_subslots(ts); ss++) {
+ lchan = &ts->lchan[ss];
+ if (lchan->rqd_ref) {
+ LOGP(DRSL, LOGL_ERROR,
+ "%s During dyn TS switchover, expecting no"
+ " Request Reference to be pending. Discarding!\n",
+ gsm_lchan_name(lchan));
+ talloc_free(lchan->rqd_ref);
+ lchan->rqd_ref = NULL;
+ }
+ }
+
+ /*
+ * When switching pchan modes, all lchans are unused. So always
+ * activate whatever wants to be activated on the first lchan. (We
+ * wouldn't remember to use lchan[1] across e.g. a PDCH deact anyway)
+ */
+ lchan = ts->lchan;
/*
* For TCH/x, the lchan->type has been set in lchan_alloc(), but it may
@@ -2459,14 +2482,6 @@ static int dyn_ts_switchover_continue(struct gsm_lchan *lchan)
: lchan->dyn.ho_ref;
/* Fetch the rqd_ref back from before switchover started. */
- if (lchan->rqd_ref) {
- LOGP(DRSL, LOGL_ERROR,
- "%s During dyn TS switchover, expecting no"
- " Request Reference to be pending. Discarding!\n",
- gsm_lchan_name(lchan));
- talloc_free(lchan->rqd_ref);
- lchan->rqd_ref = NULL;
- }
lchan->rqd_ref = lchan->dyn.rqd_ref;
lchan->rqd_ta = lchan->dyn.rqd_ta;
lchan->dyn.rqd_ref = NULL;
@@ -2477,14 +2492,13 @@ static int dyn_ts_switchover_continue(struct gsm_lchan *lchan)
LOGP(DRSL, LOGL_ERROR,
"%s RSL Chan Activate failed\n",
gsm_ts_and_pchan_name(ts));
- return dyn_ts_switchover_failed(lchan, rc);
+ return dyn_ts_switchover_failed(ts, rc);
}
return 0;
}
-static int dyn_ts_switchover_failed(struct gsm_lchan *lchan, int rc)
+static int dyn_ts_switchover_failed(struct gsm_bts_trx_ts *ts, int rc)
{
- struct gsm_bts_trx_ts *ts = lchan->ts;
ts->dyn.pchan_want = ts->dyn.pchan_is;
LOGP(DRSL, LOGL_ERROR, "%s Error %d during dynamic channel switchover."
" Going back to previous pchan.\n", gsm_ts_and_pchan_name(ts),
@@ -2510,7 +2524,7 @@ static void dyn_ts_switchover_complete(struct gsm_lchan *lchan)
LOGP(DRSL, LOGL_ERROR,
"%s Requested transition does not match lchan type %s\n",
gsm_ts_and_pchan_name(ts),
- gsm_lchant_name(ts->lchan[0].type));
+ gsm_lchant_name(lchan->type));
pchan_was = ts->dyn.pchan_is;
ts->dyn.pchan_is = ts->dyn.pchan_want = pchan_act;
diff --git a/openbsc/src/libbsc/bsc_dyn_ts.c b/openbsc/src/libbsc/bsc_dyn_ts.c
index 456500a89..e5422fc5c 100644
--- a/openbsc/src/libbsc/bsc_dyn_ts.c
+++ b/openbsc/src/libbsc/bsc_dyn_ts.c
@@ -52,7 +52,7 @@ void tchf_tchh_pdch_ts_init(struct gsm_bts_trx_ts *ts)
return;
}
- dyn_ts_switchover_start(ts->lchan, GSM_PCHAN_PDCH);
+ dyn_ts_switchover_start(ts, GSM_PCHAN_PDCH);
}
void dyn_ts_init(struct gsm_bts_trx_ts *ts)