If bsc receives Delete_Ind message from bts, bsc should release allocated channel, which was specified in dropped imm_assign message. --- openbsc/src/libbsc/abis_rsl.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+)
diff --git a/openbsc/src/libbsc/abis_rsl.c b/openbsc/src/libbsc/abis_rsl.c index f53ba84..ed6d8cd 100644 --- a/openbsc/src/libbsc/abis_rsl.c +++ b/openbsc/src/libbsc/abis_rsl.c @@ -1539,6 +1539,41 @@ static int rsl_rx_ccch_load(struct msgb *msg) return 0; }
+/* CCCH overloaded, IMM_ASSIGN was dropped */ +static int rsl_rx_delete_ind(struct gsm_bts_trx *trx, struct msgb *msg) +{ + struct abis_rsl_dchan_hdr *rqd_hdr = msgb_l2(msg); + struct gsm48_imm_ass *ia; + struct gsm_lchan *lchan; + struct gsm_bts_trx *cur_trx; + uint8_t chan_nr; + uint16_t arfcn; + + /* bts didn't send IMM_ASSIGN, so we should release allocated channel */ + if (msgb_l2len(msg) != MACBLOCK_SIZE + 6) + return -EIO; + + ia = (struct gsm48_imm_ass *) (rqd_hdr->data + 2); + + if (ia->msg_type == GSM48_MT_RR_IMM_ASS) { + chan_nr = ia->chan_desc.chan_nr; + arfcn = ia->chan_desc.h0.arfcn_high; + arfcn = (arfcn << 8) | ia->chan_desc.h0.arfcn_low; + llist_for_each_entry(cur_trx, &trx->bts->trx_list, list) { + if (cur_trx->arfcn == arfcn) { + lchan = lchan_lookup(cur_trx, chan_nr); + if (!lchan) + return -EINVAL; + if (lchan->state != LCHAN_S_ACTIVE) + return -EINVAL; + rsl_direct_rf_release(lchan); + } + } + } + + return 0; +} + static int abis_rsl_rx_cchan(struct msgb *msg) { struct e1inp_sign_link *sign_link = msg->dst; @@ -1558,6 +1593,8 @@ static int abis_rsl_rx_cchan(struct msgb *msg) break; case RSL_MT_DELETE_IND: /* CCCH overloaded, IMM_ASSIGN was dropped */ + rc = rsl_rx_delete_ind(sign_link->trx, msg); + break; case RSL_MT_CBCH_LOAD_IND: /* current load on the CBCH */ LOGP(DRSL, LOGL_NOTICE, "Unimplemented Abis RSL TRX message "
On Mon, Feb 17, 2014 at 04:50:42PM +0400, Ivan Kluchnikov wrote:
Good Morning,
llist_for_each_entry(cur_trx, &trx->bts->trx_list, list) {if (cur_trx->arfcn == arfcn) {
I wondered if we want to compare the access reference as well? We do not enforce that the ARFCNs of a TRX are distinct. And besides that I would like you to move the look-up code to a separate function. That way we can at least write a unit test in the future.
I wondered if we want to compare the access reference as well? We do not enforce that the ARFCNs of a TRX are distinct.
It seems, I don't understand what do you mean. What access reference we should compare? As I understand, each trx has one arfcn, so we can find trx by arfcn.
And besides that I would
like you to move the look-up code to a separate function. That way we can at least write a unit test in the future.
Ok, I can move this code to "gsm_bts_trx_by_arfcn" function.
On Tue, Feb 18, 2014 at 12:34:26PM +0400, Ivan Kluchnikov wrote:
It seems, I don't understand what do you mean. What access reference we should compare? As I understand, each trx has one arfcn, so we can find trx by arfcn.
bts 0 trx 0 arfcn 123 trx 1 arfcn 123
it is a config not rejected by OpenBSC. Maybe we should reject it, or we should make the deletion code more robust. I was referring to the request reference (10.5.2.30) of GSM 04.08.
And besides that I would
like you to move the look-up code to a separate function. That way we can at least write a unit test in the future.
Ok, I can move this code to "gsm_bts_trx_by_arfcn" function.
exactly
bts 0 trx 0 arfcn 123 trx 1 arfcn 123
it is a config not rejected by OpenBSC.
Ok, now I understand. But I think the right way is to check duplicate arfcns in bootstrap_bts function, because it allows us to detect this problem earlier.
On Tue, Feb 18, 2014 at 07:03:50PM +0400, Ivan Kluchnikov wrote:
Ok, now I understand. But I think the right way is to check duplicate arfcns in bootstrap_bts function, because it allows us to detect this problem earlier.
Sure, I just highlighted that your code makes an assumption that is not true. I can't think of a valid use-case for re-using the ARFCN across a BTS. We can forbid this kind of configuration and see if somebody complains.
18 февр. 2014 г. 11:02 пользователь "Holger Hans Peter Freyther" < holger@freyther.de> написал:
On Mon, Feb 17, 2014 at 04:50:42PM +0400, Ivan Kluchnikov wrote:
Good Morning,
llist_for_each_entry(cur_trx, &trx->bts->trx_list, list) {if (cur_trx->arfcn == arfcn) {I wondered if we want to compare the access reference as well? We do not enforce that the ARFCNs of a TRX are distinct.
Actually I think we should rather start enforcing that. Or do you know situations when both TRX's could run on the same ARFCN?
And besides that I would like you to move the look-up code to a separate function. That way we can at least write a unit test in the future.
+1
Please excuse typos. Written with a touchscreen keyboard.
-- Regards, Alexander Chemeris CEO/Founder Fairwaves LLC http://fairwaves.ru
17 февр. 2014 г. 16:54 пользователь "Ivan Kluchnikov" kluchnikovi@gmail.com написал:
+/* CCCH overloaded, IMM_ASSIGN was dropped */
You're missing "is" before "overloaded", if I remember English grammar correctly. :)
Please excuse typos. Written with a touchscreen keyboard.
-- Regards, Alexander Chemeris CEO/Founder Fairwaves LLC http://fairwaves.ru
-
Alexander Chemeris -
Holger Hans Peter Freyther -
Ivan Kluchnikov -
Ivan Kluchnikov