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
This commit is contained in:
Neels Hofmeyr 2016-08-24 16:57:31 +02:00 committed by Harald Welte
parent cd150a8f74
commit b74a2c8e29
3 changed files with 56 additions and 42 deletions

View File

@ -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 */

View File

@ -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;

View File

@ -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)