Holger Hans Peter Freyther wrote: > Here you point out that the range of 4 to UINT8_MAX is coming from the > specification. Which is very good. The way it is done is a violation of > the principle of least surprise though. If a BSC sends the value '2' it > doesn't make sense that '32' is used. The configurator/implementor > certainly wanted to have a low value. > > IMHO the right thing to do is to NACK the set bts attributes with > a descriptive error message. The same goes for the conn fail crit > being present but not of the type we support. > hi holger,
the range is actually 4..64, so i send a NACK now, if the given attribute is not is that range. also the counting of S value is moved to a seperate function. check out the attached patch. regards, andreas
>From e501ea2c24f922275514d8149ae75750d79ec2d0 Mon Sep 17 00:00:00 2001 From: Andreas Eversberg <[email protected]> Date: Thu, 14 Mar 2013 11:38:11 +0100 Subject: [PATCH] Fix: Stop RADIO LINK TIMEOUT couter S from counting, if it has reached 0 In case that the counter S reached 0, it will stay 0. Subsequent received good and bad SACCH frames must not cause to trigger radio link failure again. Once the BSC has been indicated about link failure, it will release channel. The counting of S has been moved to a seperate function. This patch will ensure that the link failure is indicated only once. But even if the link failure would be sent multiple times, the BSC should ignore it. The BSC releases the channel and may only reuse it after confirm from BTS. (There cannot be any link failure indications after confirm of channel release.) The allowed timeout value range is 4..64, as defined in TS 05.08, so if the BSC sends an attribute with value out of range or other failure criterion, the Set BTS Attributes message is NACKed. --- src/common/oml.c | 12 ++++++++-- src/osmo-bts-sysmo/l1_if.c | 50 +++++++++++++++++++++++++++---------------- 2 files changed, 40 insertions(+), 22 deletions(-) diff --git a/src/common/oml.c b/src/common/oml.c index 4e2dead..bf90ff1 100644 --- a/src/common/oml.c +++ b/src/common/oml.c @@ -446,10 +446,16 @@ static int oml_rx_set_bts_attr(struct gsm_bts *bts, struct msgb *msg) btsb->interference.intave = *TLVP_VAL(&tp, NM_ATT_INTAVE_PARAM); /* 9.4.14 Connection Failure Criterion */ - if (TLVP_PRESENT(&tp, NM_ATT_CONN_FAIL_CRIT) && - (TLVP_LEN(&tp, NM_ATT_CONN_FAIL_CRIT) >= 2) && - *TLVP_VAL(&tp, NM_ATT_CONN_FAIL_CRIT) == 0x01) { + if (TLVP_PRESENT(&tp, NM_ATT_CONN_FAIL_CRIT)) { const uint8_t *val = TLVP_VAL(&tp, NM_ATT_CONN_FAIL_CRIT); + + if (TLVP_LEN(&tp, NM_ATT_CONN_FAIL_CRIT) < 2 + || val[0] != 0x01 || val[1] < 4 || val[1] > 64) { + LOGP(DOML, LOGL_NOTICE, "Given Conn. Failure Criterion " + "not supported. Please use critetion 0x01 with " + "RADIO_LINK_TIMEOUT value of 4..64\n"); + return oml_fom_ack_nack(msg, NM_NACK_PARAM_RANGE); + } btsb->radio_link_timeout = val[1]; } /* if val[0] != 0x01: can be 'operator dependent' and needs to diff --git a/src/osmo-bts-sysmo/l1_if.c b/src/osmo-bts-sysmo/l1_if.c index df660c5..bdba4c2 100644 --- a/src/osmo-bts-sysmo/l1_if.c +++ b/src/osmo-bts-sysmo/l1_if.c @@ -647,11 +647,39 @@ static int process_meas_res(struct gsm_lchan *lchan, GsmL1_MeasParam_t *m) return lchan_new_ul_meas(lchan, &ulm); } +/* process radio link timeout counter S */ +static void radio_link_timeout(struct gsm_lchan *lchan, int bad_frame) +{ + struct gsm_bts_role_bts *btsb = lchan->ts->trx->bts->role; + + /* if link loss criterion already reached */ + if (lchan->s == 0) + return; + + if (bad_frame) { + /* count down radio link counter S */ + lchan->s--; + DEBUGP(DMEAS, "counting down radio link counter S=%d\n", + lchan->s); + if (lchan->s == 0) + rsl_tx_conn_fail(lchan, RSL_ERR_RADIO_LINK_FAIL); + return; + } + + if (lchan->s < btsb->radio_link_timeout) { + /* count up radio link counter S */ + lchan->s += 2; + if (lchan->s > btsb->radio_link_timeout) + lchan->s = btsb->radio_link_timeout; + DEBUGP(DMEAS, "counting up radio link counter S=%d\n", + lchan->s); + } +} + static int handle_ph_data_ind(struct femtol1_hdl *fl1, GsmL1_PhDataInd_t *data_ind, struct msgb *l1p_msg) { struct gsm_bts_trx *trx = fl1->priv; - struct gsm_bts_role_bts *btsb = trx->bts->role; struct osmo_phsap_prim pp; struct gsm_lchan *lchan; struct lapdm_entity *le; @@ -681,25 +709,9 @@ static int handle_ph_data_ind(struct femtol1_hdl *fl1, GsmL1_PhDataInd_t *data_i switch (data_ind->sapi) { case GsmL1_Sapi_Sacch: - /* process radio link timeout coniter S */ - if (data_ind->msgUnitParam.u8Size == 0) { - /* count down radio link counter S */ - lchan->s--; - DEBUGP(DMEAS, "counting down radio link counter S=%d\n", - lchan->s); - if (lchan->s == 0) - rsl_tx_conn_fail(lchan, - RSL_ERR_RADIO_LINK_FAIL); + radio_link_timeout(lchan, (data_ind->msgUnitParam.u8Size == 0)); + if (data_ind->msgUnitParam.u8Size == 0) break; - } - if (lchan->s < btsb->radio_link_timeout) { - /* count up radio link counter S */ - lchan->s += 2; - if (lchan->s > btsb->radio_link_timeout) - lchan->s = btsb->radio_link_timeout; - DEBUGP(DMEAS, "counting up radio link counter S=%d\n", - lchan->s); - } /* save the SACCH L1 header in the lchan struct for RSL MEAS RES */ if (data_ind->msgUnitParam.u8Size < 2) { LOGP(DL1C, LOGL_NOTICE, "SACCH with size %u<2 !?!\n", -- 1.7.3.4
