Good Morning Harald,
do you mind if we drop support for the HSL BTS? The support is spewing out compile warnings, now turns up in coverity reports as well and we don't even know if it is working with the latest version of the software running on these BTS.
holger
From: Holger Hans Peter Freyther holger@moiji-mobile.com
The support has been implemented for an old model, we were told that newer versions would be made incompatible with OpenBSC. Ther are various warnings in the code and coverity has found some new ones.
Just remove the code as we don't know of anyone using this code. --- openbsc/doc/examples/osmo-nitb/hsl/openbsc.cfg | 90 ------- openbsc/include/openbsc/bss.h | 1 - openbsc/include/openbsc/gsm_data_shared.h | 4 - openbsc/osmoappdesc.py | 4 +- openbsc/src/libbsc/Makefile.am | 1 - openbsc/src/libbsc/abis_nm.c | 4 - openbsc/src/libbsc/abis_rsl.c | 6 - openbsc/src/libbsc/bsc_init.c | 25 +- openbsc/src/libbsc/bsc_vty.c | 48 ---- openbsc/src/libbsc/bts_hsl_femtocell.c | 309 ------------------------ openbsc/src/libbsc/bts_init.c | 1 - openbsc/src/libbsc/e1_config.c | 3 +- openbsc/src/libbsc/system_information.c | 4 - openbsc/src/libcommon/gsm_data.c | 4 - 14 files changed, 6 insertions(+), 498 deletions(-) delete mode 100644 openbsc/doc/examples/osmo-nitb/hsl/openbsc.cfg delete mode 100644 openbsc/src/libbsc/bts_hsl_femtocell.c
diff --git a/openbsc/doc/examples/osmo-nitb/hsl/openbsc.cfg b/openbsc/doc/examples/osmo-nitb/hsl/openbsc.cfg deleted file mode 100644 index b0af855..0000000 --- a/openbsc/doc/examples/osmo-nitb/hsl/openbsc.cfg +++ /dev/null @@ -1,90 +0,0 @@ -! -! OpenBSC (0.9.11.261-32c0) configuration saved from vty -!! -password foo -! -line vty - no login -! -e1_input - e1_line 0 driver hsl -network - network country code 262 - mobile network code 42 - short name OpenBSC - long name OpenBSC - auth policy closed - location updating reject cause 13 - encryption a5 0 - neci 1 - paging any use tch 0 - rrlp mode none - mm info 0 - handover 0 - handover window rxlev averaging 10 - handover window rxqual averaging 1 - handover window rxlev neighbor averaging 10 - handover power budget interval 6 - handover power budget hysteresis 3 - handover maximum distance 9999 - timer t3101 10 - timer t3103 0 - timer t3105 0 - timer t3107 0 - timer t3109 4 - timer t3111 0 - timer t3113 60 - timer t3115 0 - timer t3117 0 - timer t3119 0 - timer t3122 0 - timer t3141 0 - dtx-used 1 - subscriber-keep-in-ram 0 - bts 0 - type hsl_femto - band DCS1800 - cell_identity 0 - location_area_code 1 - training_sequence_code 0 - base_station_id_code 0 - ms max power 15 - cell reselection hysteresis 4 - rxlev access min 0 - channel allocator ascending - rach tx integer 9 - rach max transmission 1 - hsl serial-number 8303701 - neighbor-list mode automatic - oml hsl line 0 - gprs mode none - trx 0 - rf_locked 0 - arfcn 871 - nominal power 23 - max_power_red 20 - rsl e1 tei 0 - timeslot 0 - phys_chan_config CCCH+SDCCH4 - hopping enabled 0 - timeslot 1 - phys_chan_config TCH/F - hopping enabled 0 - timeslot 2 - phys_chan_config TCH/F - hopping enabled 0 - timeslot 3 - phys_chan_config TCH/F - hopping enabled 0 - timeslot 4 - phys_chan_config TCH/F - hopping enabled 0 - timeslot 5 - phys_chan_config TCH/F - hopping enabled 0 - timeslot 6 - phys_chan_config TCH/F - hopping enabled 0 - timeslot 7 - phys_chan_config TCH/F - hopping enabled 0 diff --git a/openbsc/include/openbsc/bss.h b/openbsc/include/openbsc/bss.h index 1c6b5c3..49df547 100644 --- a/openbsc/include/openbsc/bss.h +++ b/openbsc/include/openbsc/bss.h @@ -13,7 +13,6 @@ extern int bts_init(void); extern int bts_model_bs11_init(void); extern int bts_model_rbs2k_init(void); extern int bts_model_nanobts_init(void); -extern int bts_model_hslfemto_init(void); extern int bts_model_nokia_site_init(void); extern int bts_model_sysmobts_init(void); #endif diff --git a/openbsc/include/openbsc/gsm_data_shared.h b/openbsc/include/openbsc/gsm_data_shared.h index 8e704d0..d3e5102 100644 --- a/openbsc/include/openbsc/gsm_data_shared.h +++ b/openbsc/include/openbsc/gsm_data_shared.h @@ -390,7 +390,6 @@ enum gsm_bts_type { GSM_BTS_TYPE_BS11, GSM_BTS_TYPE_NANOBTS, GSM_BTS_TYPE_RBS2000, - GSM_BTS_TYPE_HSL_FEMTO, GSM_BTS_TYPE_NOKIA_SITE, GSM_BTS_TYPE_OSMO_SYSMO, _NUM_GSM_BTS_TYPE @@ -628,9 +627,6 @@ struct gsm_bts { } tf; } rbs2000; struct { - unsigned long serno; - } hsl; - struct { uint8_t bts_type; unsigned int configured:1, skip_reset:1, diff --git a/openbsc/osmoappdesc.py b/openbsc/osmoappdesc.py index 19ec6c3..9d6fbe6 100644 --- a/openbsc/osmoappdesc.py +++ b/openbsc/osmoappdesc.py @@ -33,7 +33,7 @@ app_configs = { "mgcp": ["doc/examples/osmo-bsc_mgcp/mgcp.cfg"], "gbproxy": ["doc/examples/osmo-gbproxy/osmo-gbproxy.cfg"], "sgsn": ["doc/examples/osmo-sgsn/osmo-sgsn.cfg"], - "nitb": ["doc/examples/osmo-nitb/hsl/openbsc.cfg", + "nitb": ["doc/examples/osmo-nitb/nanobts/openbsc-multitrx.cfg", "doc/examples/osmo-nitb/nanobts/openbsc.cfg"] }
@@ -47,6 +47,6 @@ apps = [(4242, "src/osmo-bsc/osmo-bsc", "OsmoBSC", "osmo-bsc"), ]
vty_command = ["./src/osmo-nitb/osmo-nitb", "-c", - "doc/examples/osmo-nitb/hsl/openbsc.cfg"] + "doc/examples/osmo-nitb/nanobts/openbsc.cfg"]
vty_app = apps[-1] diff --git a/openbsc/src/libbsc/Makefile.am b/openbsc/src/libbsc/Makefile.am index f5b1ff1..42fabab 100644 --- a/openbsc/src/libbsc/Makefile.am +++ b/openbsc/src/libbsc/Makefile.am @@ -12,7 +12,6 @@ libbsc_a_SOURCES = abis_nm.c abis_nm_vty.c \ bts_ipaccess_nanobts.c \ bts_siemens_bs11.c \ bts_nokia_site.c \ - bts_hsl_femtocell.c \ bts_unknown.c \ bts_sysmobts.c \ chan_alloc.c \ diff --git a/openbsc/src/libbsc/abis_nm.c b/openbsc/src/libbsc/abis_nm.c index adc2362..ee1fc9c 100644 --- a/openbsc/src/libbsc/abis_nm.c +++ b/openbsc/src/libbsc/abis_nm.c @@ -632,10 +632,6 @@ static int abis_nm_rcvmsg_fom(struct msgb *mb) osmo_signal_dispatch(SS_NM, S_NM_IPACC_RESTART_NACK, NULL); break; case NM_MT_SET_BTS_ATTR_ACK: - /* The HSL wants an OPSTART _after_ the SI has been set */ - if (sign_link->trx->bts->type == GSM_BTS_TYPE_HSL_FEMTO) { - abis_nm_opstart(sign_link->trx->bts, NM_OC_BTS, 255, 255, 255); - } break; }
diff --git a/openbsc/src/libbsc/abis_rsl.c b/openbsc/src/libbsc/abis_rsl.c index 7aae590..41bfcdc 100644 --- a/openbsc/src/libbsc/abis_rsl.c +++ b/openbsc/src/libbsc/abis_rsl.c @@ -722,12 +722,6 @@ static int rsl_rf_chan_release(struct gsm_lchan *lchan, int error, rc = abis_rsl_sendmsg(msg);
/* BTS will respond by RF CHAN REL ACK */ -#ifdef HSL_SR_1_0 - /* The HSL Femto seems to 'forget' sending a REL ACK for TS1...TS7 */ - if (lchan->ts->trx->bts->type == GSM_BTS_TYPE_HSL_FEMTO && lchan->ts->nr != 0) - rc = rsl_rx_rf_chan_rel_ack(lchan); -#endif - return rc; }
diff --git a/openbsc/src/libbsc/bsc_init.c b/openbsc/src/libbsc/bsc_init.c index 2fb4f13..8fd72cf 100644 --- a/openbsc/src/libbsc/bsc_init.c +++ b/openbsc/src/libbsc/bsc_init.c @@ -38,7 +38,6 @@
/* global pointer to the gsm network data structure */ extern struct gsm_network *bsc_gsmnet; -extern int hsl_setup(struct gsm_network *gsmnet);
/* Callback function for NACK on the OML NM */ static int oml_msg_nack(struct nm_nack_signal_data *nack) @@ -98,7 +97,7 @@ int bsc_shutdown_net(struct gsm_network *net) static int rsl_si(struct gsm_bts_trx *trx, enum osmo_sysinfo_type i, int si_len) { struct gsm_bts *bts = trx->bts; - int rc, j; + int rc;
DEBUGP(DRR, "SI%s: %s\n", get_value_string(osmo_sitype_strs, i), osmo_hexdump(GSM_BTS_SI(bts, i), GSM_MACBLOCK_LEN)); @@ -108,26 +107,8 @@ static int rsl_si(struct gsm_bts_trx *trx, enum osmo_sysinfo_type i, int si_len) case SYSINFO_TYPE_5bis: case SYSINFO_TYPE_5ter: case SYSINFO_TYPE_6: - if (trx->bts->type == GSM_BTS_TYPE_HSL_FEMTO) { - /* HSL has mistaken SACCH INFO MODIFY for SACCH FILLING, - * so we need a special workaround here */ - /* This assumes a combined BCCH and TCH on TS1...7 */ - for (j = 0; j < 4; j++) - rsl_sacch_info_modify(&trx->ts[0].lchan[j], - osmo_sitype2rsl(i), - GSM_BTS_SI(bts, i), si_len); - for (j = 1; j < 8; j++) { - rsl_sacch_info_modify(&trx->ts[j].lchan[0], - osmo_sitype2rsl(i), - GSM_BTS_SI(bts, i), si_len); - rsl_sacch_info_modify(&trx->ts[j].lchan[1], - osmo_sitype2rsl(i), - GSM_BTS_SI(bts, i), si_len); - } - rc = 0; - } else - rc = rsl_sacch_filling(trx, osmo_sitype2rsl(i), - GSM_BTS_SI(bts, i), si_len); + rc = rsl_sacch_filling(trx, osmo_sitype2rsl(i), + GSM_BTS_SI(bts, i), si_len); break; default: rc = rsl_bcch_info(trx, osmo_sitype2rsl(i), diff --git a/openbsc/src/libbsc/bsc_vty.c b/openbsc/src/libbsc/bsc_vty.c index b98ef15..6c58ff1 100644 --- a/openbsc/src/libbsc/bsc_vty.c +++ b/openbsc/src/libbsc/bsc_vty.c @@ -276,8 +276,6 @@ static void bts_dump_vty(struct vty *vty, struct gsm_bts *bts) vty_out(vty, " Unit ID: %u/%u/0, OML Stream ID 0x%02x%s", bts->ip_access.site_id, bts->ip_access.bts_id, bts->oml_tei, VTY_NEWLINE); - else if (bts->type == GSM_BTS_TYPE_HSL_FEMTO) - vty_out(vty, " Serial Number: %lu%s", bts->hsl.serno, VTY_NEWLINE); else if (bts->type == GSM_BTS_TYPE_NOKIA_SITE) vty_out(vty, " Skip Reset: %d%s", bts->nokia.skip_reset, VTY_NEWLINE); @@ -560,11 +558,6 @@ static void config_write_bts_single(struct vty *vty, struct gsm_bts *bts) vty_out(vty, " oml ip.access stream_id %u line %u%s", bts->oml_tei, bts->oml_e1_link.e1_nr, VTY_NEWLINE); break; - case GSM_BTS_TYPE_HSL_FEMTO: - vty_out(vty, " hsl serial-number %lu%s", bts->hsl.serno, VTY_NEWLINE); - vty_out(vty, " oml hsl line %u%s", - bts->oml_e1_link.e1_nr, VTY_NEWLINE); - break; case GSM_BTS_TYPE_NOKIA_SITE: vty_out(vty, " nokia_site skip-reset %d%s", bts->nokia.skip_reset, VTY_NEWLINE); break; @@ -1665,25 +1658,6 @@ DEFUN(cfg_bts_rsl_ip, }
-DEFUN(cfg_bts_serno, - cfg_bts_serno_cmd, - "hsl serial-number STRING", - "HSL BTS specific options\n" - "Set the HSL Serial Number of this BTS\n" - "Serial Number of this HSL BTS\n") -{ - struct gsm_bts *bts = vty->index; - - if (bts->type != GSM_BTS_TYPE_HSL_FEMTO) { - vty_out(vty, "%% BTS is not of HSL type%s", VTY_NEWLINE); - return CMD_WARNING; - } - - bts->hsl.serno = strtoul(argv[0], NULL, 10); - - return CMD_SUCCESS; -} - DEFUN(cfg_bts_nokia_site_skip_reset, cfg_bts_nokia_site_skip_reset_cmd, "nokia_site skip-reset (0|1)", @@ -1728,26 +1702,6 @@ DEFUN(cfg_bts_stream_id, return CMD_SUCCESS; }
-DEFUN(cfg_bts_hsl_oml, - cfg_bts_hsl_oml_cmd, - "oml hsl line E1_LINE", - OML_STR "HSL femto Specific Options\n" - "Set OML link of this HSL femto BTS\n" - "Virtual E1/T1 line number\n") -{ - struct gsm_bts *bts = vty->index; - int linenr = atoi(argv[0]); - - if (!(bts->type == GSM_BTS_TYPE_HSL_FEMTO)) { - vty_out(vty, "%% BTS is not of HSL type%s", VTY_NEWLINE); - return CMD_WARNING; - } - - bts->oml_e1_link.e1_nr = linenr; - - return CMD_SUCCESS; -} - #define OML_E1_STR OML_STR "OML E1/T1 Configuration\n"
DEFUN(cfg_bts_oml_e1, @@ -3084,10 +3038,8 @@ int bsc_vty_init(const struct log_info *cat) install_element(BTS_NODE, &cfg_bts_rsl_ip_cmd); install_element(BTS_NODE, &cfg_bts_timezone_cmd); install_element(BTS_NODE, &cfg_bts_no_timezone_cmd); - install_element(BTS_NODE, &cfg_bts_serno_cmd); install_element(BTS_NODE, &cfg_bts_nokia_site_skip_reset_cmd); install_element(BTS_NODE, &cfg_bts_stream_id_cmd); - install_element(BTS_NODE, &cfg_bts_hsl_oml_cmd); install_element(BTS_NODE, &cfg_bts_oml_e1_cmd); install_element(BTS_NODE, &cfg_bts_oml_e1_tei_cmd); install_element(BTS_NODE, &cfg_bts_challoc_cmd); diff --git a/openbsc/src/libbsc/bts_hsl_femtocell.c b/openbsc/src/libbsc/bts_hsl_femtocell.c deleted file mode 100644 index 1ce3eaa..0000000 --- a/openbsc/src/libbsc/bts_hsl_femtocell.c +++ /dev/null @@ -1,309 +0,0 @@ -/* OpenBSC support code for HSL Femtocell */ - -/* (C) 2011 by Harald Welte laforge@gnumonks.org - * (C) 2011 by OnWaves - * - * All Rights Reserved - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as published by - * the Free Software Foundation; either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see http://www.gnu.org/licenses/. - * - */ - -#include <inttypes.h> - -#include <arpa/inet.h> - -#include <osmocom/gsm/tlv.h> -#include <openbsc/gsm_data.h> -#include <openbsc/abis_nm.h> -#include <openbsc/abis_rsl.h> -#include <openbsc/signal.h> -#include <openbsc/debug.h> -#include <osmocom/core/logging.h> -#include <osmocom/abis/e1_input.h> -#include <osmocom/abis/ipaccess.h> - -static int bts_model_hslfemto_start(struct gsm_network *net); -static void bts_model_hslfemto_e1line_bind_ops(struct e1inp_line *line); - -static struct gsm_bts_model model_hslfemto = { - .type = GSM_BTS_TYPE_HSL_FEMTO, - .start = bts_model_hslfemto_start, - .e1line_bind_ops = &bts_model_hslfemto_e1line_bind_ops, - .nm_att_tlvdef = { - .def = { - /* no HSL specific OML attributes that we know of */ - }, - }, -}; - - -static const uint8_t l1_msg[] = { -#ifdef HSL_SR_1_0 - 0x80, 0x8a, -#else - 0x81, 0x8a, -#endif - 0xC4, 0x0b, -}; - -static const uint8_t conn_trau_msg[] = { -#ifdef HSL_SR_1_0 - 0x80, 0x81, -#else - 0x81, 0x81, -#endif - 0xC1, 16, - 0x02, 0x00, 0x00, 0x00, 0xC0, 0xA8, 0xEA, 0x01, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 -}; - -static const uint8_t conn_trau_msg2[] = { -#ifdef HSL_SR_1_0 - 0x80, 0x81, -#else - 0x81, 0x81, -#endif - 0xC1, 16, - 0x02, 0x00, 0xd4, 0x07, 0xC0, 0xA8, 0xEA, 0x01, - 0x38, 0xA4, 0x45, 0x00, 0x04, 0x59, 0x40, 0x00 -}; - -static uint8_t oml_arfcn_bsic[] = { -#ifdef HSL_SR_1_0 - 0x81, 0x80, 0x00, 10, -#else - 0x80, 0x80, 0x00, 10, -#endif - NM_MT_SET_BTS_ATTR, NM_OC_BTS, 0xff, 0xff, 0xff, - NM_ATT_BCCH_ARFCN, 0x03, 0x67, - NM_ATT_BSIC, 0x00 -}; - -static inline struct msgb *hsl_alloc_msgb(void) -{ - return msgb_alloc_headroom(1024, 127, "HSL"); -} - -static int hslfemto_bootstrap_om(struct gsm_bts *bts) -{ - struct msgb *msg; - - msg = hsl_alloc_msgb(); - msgb_put(msg, sizeof(l1_msg)); - memcpy(msg->data, l1_msg, sizeof(l1_msg)); - msg->dst = bts->c0->rsl_link; - abis_rsl_sendmsg(msg); - -#if 1 - msg = hsl_alloc_msgb(); - msgb_put(msg, sizeof(conn_trau_msg)); - memcpy(msg->data, conn_trau_msg, sizeof(conn_trau_msg)); - msg->dst = bts->c0->rsl_link; - abis_rsl_sendmsg(msg); -#endif - msg = hsl_alloc_msgb(); - msgb_put(msg, sizeof(conn_trau_msg2)); - memcpy(msg->data, conn_trau_msg2, sizeof(conn_trau_msg2)); - msg->dst = bts->c0->rsl_link; - abis_rsl_sendmsg(msg); - - *((uint16_t *)oml_arfcn_bsic+10) = htons(bts->c0->arfcn); - oml_arfcn_bsic[13] = bts->bsic; - - msg = hsl_alloc_msgb(); - msgb_put(msg, sizeof(oml_arfcn_bsic)); - memcpy(msg->data, oml_arfcn_bsic, sizeof(oml_arfcn_bsic)); - msg->dst = bts->c0->rsl_link; - abis_sendmsg(msg); - - /* Delay the OPSTART until after SI have been set via RSL */ - //abis_nm_opstart(bts, NM_OC_BTS, 255, 255, 255); - - return 0; -} - -/* Callback function to be called every time we receive a signal from INPUT */ -static int inp_sig_cb(unsigned int subsys, unsigned int signal, - void *handler_data, void *signal_data) -{ - struct input_signal_data *isd = signal_data; - - if (subsys != SS_L_INPUT) - return 0; - - switch (signal) { - case S_L_INP_TEI_UP: - switch (isd->link_type) { - case E1INP_SIGN_OML: - if (isd->trx->bts->type == GSM_BTS_TYPE_HSL_FEMTO) - hslfemto_bootstrap_om(isd->trx->bts); - break; - } - } - - return 0; -} - -static struct gsm_network *hsl_gsmnet; - -static int bts_model_hslfemto_start(struct gsm_network *net) -{ - model_hslfemto.features.data = &model_hslfemto._features_data[0]; - model_hslfemto.features.data_len = sizeof(model_hslfemto._features_data); - - gsm_btsmodel_set_feature(&model_hslfemto, BTS_FEAT_GPRS); - gsm_btsmodel_set_feature(&model_hslfemto, BTS_FEAT_EGPRS); - - osmo_signal_register_handler(SS_L_INPUT, inp_sig_cb, NULL); - - hsl_gsmnet = net; - return 0; -} - -int bts_model_hslfemto_init(void) -{ - return gsm_bts_model_register(&model_hslfemto); -} - -#define OML_UP 0x0001 -#define RSL_UP 0x0002 - -struct gsm_bts *find_bts_by_serno(struct gsm_network *net, uint64_t serno) -{ - struct gsm_bts *bts; - - llist_for_each_entry(bts, &net->bts_list, list) { - if (bts->type != GSM_BTS_TYPE_HSL_FEMTO) - continue; - - if (serno == bts->hsl.serno) - return bts; - } - return NULL; -} - -/* This function is called once the OML/RSL link becomes up. */ -static struct e1inp_sign_link * -hsl_sign_link_up(void *unit_data, struct e1inp_line *line, - enum e1inp_sign_type type) -{ - struct hsl_unit *dev = unit_data; - struct gsm_bts *bts; - - bts = find_bts_by_serno(hsl_gsmnet, dev->serno); - if (!bts) { - LOGP(DLINP, LOGL_ERROR, "Unable to find BTS config for " - "serial number %"PRIx64"\n", dev->serno); - return NULL; - } - DEBUGP(DLINP, "Identified HSL BTS Serial Number %"PRIx64"\n", dev->serno); - - /* we shouldn't hardcode it, but HSL femto also hardcodes it... */ - bts->oml_tei = 255; - bts->c0->rsl_tei = 0; - bts->oml_link = e1inp_sign_link_create(&line->ts[E1INP_SIGN_OML-1], - E1INP_SIGN_OML, bts->c0, - bts->oml_tei, 0); - bts->c0->rsl_link = e1inp_sign_link_create(&line->ts[E1INP_SIGN_OML-1], - E1INP_SIGN_RSL, bts->c0, - bts->c0->rsl_tei, 0); - e1inp_event(&line->ts[E1INP_SIGN_OML-1], S_L_INP_TEI_UP, 255, 0); - e1inp_event(&line->ts[E1INP_SIGN_OML-1], S_L_INP_TEI_UP, 0, 0); - bts->ip_access.flags |= OML_UP; - bts->ip_access.flags |= (RSL_UP << 0); - - return bts->oml_link; -} - -void hsl_drop_oml(struct gsm_bts *bts) -{ - if (!bts->oml_link) - return; - - e1inp_sign_link_destroy(bts->oml_link); - bts->oml_link = NULL; - - e1inp_sign_link_destroy(bts->c0->rsl_link); - bts->c0->rsl_link = NULL; - - bts->ip_access.flags = 0; -} - -static void hsl_sign_link_down(struct e1inp_line *line) -{ - /* No matter what link went down, we close both signal links. */ - struct e1inp_ts *ts = &line->ts[E1INP_SIGN_OML-1]; - struct e1inp_sign_link *link; - - llist_for_each_entry(link, &ts->sign.sign_links, list) { - struct gsm_bts *bts = link->trx->bts; - - hsl_drop_oml(bts); - /* Yes, we only use the first element of the list. */ - break; - } -} - -/* This function is called if we receive one OML/RSL message. */ -static int hsl_sign_link(struct msgb *msg) -{ - int ret = 0; - struct e1inp_sign_link *link = msg->dst; - struct e1inp_ts *e1i_ts = link->ts; - - switch (link->type) { - case E1INP_SIGN_OML: - if (!(link->trx->bts->ip_access.flags & OML_UP)) { - e1inp_event(e1i_ts, S_L_INP_TEI_UP, - link->tei, link->sapi); - link->trx->bts->ip_access.flags |= OML_UP; - } - ret = abis_nm_rcvmsg(msg); - break; - case E1INP_SIGN_RSL: - if (!(link->trx->bts->ip_access.flags & - (RSL_UP << link->trx->nr))) { - e1inp_event(e1i_ts, S_L_INP_TEI_UP, - link->tei, link->sapi); - link->trx->bts->ip_access.flags |= - (RSL_UP << link->trx->nr); - } - ret = abis_rsl_rcvmsg(msg); - break; - default: - LOGP(DLINP, LOGL_ERROR, "Unknown signal link type %d\n", - link->type); - msgb_free(msg); - break; - } - return ret; -} - -static struct e1inp_line_ops hsl_e1inp_line_ops = { - .cfg = { - .ipa = { - .addr = "0.0.0.0", - .role = E1INP_LINE_R_BSC, - }, - }, - .sign_link_up = hsl_sign_link_up, - .sign_link_down = hsl_sign_link_down, - .sign_link = hsl_sign_link, -}; - -static void bts_model_hslfemto_e1line_bind_ops(struct e1inp_line *line) -{ - e1inp_line_bind_ops(line, &hsl_e1inp_line_ops); -} diff --git a/openbsc/src/libbsc/bts_init.c b/openbsc/src/libbsc/bts_init.c index ae41ecc..d6b152a 100644 --- a/openbsc/src/libbsc/bts_init.c +++ b/openbsc/src/libbsc/bts_init.c @@ -23,7 +23,6 @@ int bts_init(void) bts_model_bs11_init(); bts_model_rbs2k_init(); bts_model_nanobts_init(); - bts_model_hslfemto_init(); bts_model_nokia_site_init(); bts_model_sysmobts_init(); /* Your new BTS here. */ diff --git a/openbsc/src/libbsc/e1_config.c b/openbsc/src/libbsc/e1_config.c index d3dff48..d82b009 100644 --- a/openbsc/src/libbsc/e1_config.c +++ b/openbsc/src/libbsc/e1_config.c @@ -185,8 +185,7 @@ int e1_reconfig_bts(struct gsm_bts *bts)
/* skip signal link initialization, this is done later for these BTS. */ if (bts->type == GSM_BTS_TYPE_NANOBTS || - bts->type == GSM_BTS_TYPE_OSMO_SYSMO || - bts->type == GSM_BTS_TYPE_HSL_FEMTO) + bts->type == GSM_BTS_TYPE_OSMO_SYSMO) return e1inp_line_update(line);
/* OML link */ diff --git a/openbsc/src/libbsc/system_information.c b/openbsc/src/libbsc/system_information.c index 88b81dc..c901a4a 100644 --- a/openbsc/src/libbsc/system_information.c +++ b/openbsc/src/libbsc/system_information.c @@ -596,7 +596,6 @@ static int generate_si5(uint8_t *output, struct gsm_bts *bts) switch (bts->type) { case GSM_BTS_TYPE_NANOBTS: case GSM_BTS_TYPE_OSMO_SYSMO: - case GSM_BTS_TYPE_HSL_FEMTO: *output++ = (l2_plen << 2) | 1; l2_plen++; break; @@ -632,7 +631,6 @@ static int generate_si5bis(uint8_t *output, struct gsm_bts *bts) switch (bts->type) { case GSM_BTS_TYPE_NANOBTS: case GSM_BTS_TYPE_OSMO_SYSMO: - case GSM_BTS_TYPE_HSL_FEMTO: *output++ = (l2_plen << 2) | 1; l2_plen++; break; @@ -677,7 +675,6 @@ static int generate_si5ter(uint8_t *output, struct gsm_bts *bts) switch (bts->type) { case GSM_BTS_TYPE_NANOBTS: case GSM_BTS_TYPE_OSMO_SYSMO: - case GSM_BTS_TYPE_HSL_FEMTO: *output++ = (l2_plen << 2) | 1; l2_plen++; break; @@ -714,7 +711,6 @@ static int generate_si6(uint8_t *output, struct gsm_bts *bts) switch (bts->type) { case GSM_BTS_TYPE_NANOBTS: case GSM_BTS_TYPE_OSMO_SYSMO: - case GSM_BTS_TYPE_HSL_FEMTO: *output++ = (l2_plen << 2) | 1; l2_plen++; break; diff --git a/openbsc/src/libcommon/gsm_data.c b/openbsc/src/libcommon/gsm_data.c index dd1d93b..5f7e32e 100644 --- a/openbsc/src/libcommon/gsm_data.c +++ b/openbsc/src/libcommon/gsm_data.c @@ -189,7 +189,6 @@ const struct value_string bts_type_names[_NUM_GSM_BTS_TYPE+1] = { { GSM_BTS_TYPE_BS11, "bs11" }, { GSM_BTS_TYPE_NANOBTS, "nanobts" }, { GSM_BTS_TYPE_RBS2000, "rbs2000" }, - { GSM_BTS_TYPE_HSL_FEMTO, "hsl_femto" }, { GSM_BTS_TYPE_NOKIA_SITE, "nokia_site" }, { GSM_BTS_TYPE_OSMO_SYSMO, "sysmobts" }, { 0, NULL } @@ -200,7 +199,6 @@ const struct value_string bts_type_descs[_NUM_GSM_BTS_TYPE+1] = { { GSM_BTS_TYPE_BS11, "Siemens BTS (BS-11 or compatible)" }, { GSM_BTS_TYPE_NANOBTS, "ip.access nanoBTS or compatible" }, { GSM_BTS_TYPE_RBS2000, "Ericsson RBS2000 Series" }, - { GSM_BTS_TYPE_HSL_FEMTO, "HSL 2.75G femto" }, { GSM_BTS_TYPE_NOKIA_SITE, "Nokia {Metro,Ultra,In}Site" }, { GSM_BTS_TYPE_OSMO_SYSMO, "sysmocom sysmoBTS" }, { 0, NULL } @@ -349,8 +347,6 @@ int gsm_set_bts_type(struct gsm_bts *bts, enum gsm_bts_type type) }
switch (bts->type) { - case GSM_BTS_TYPE_HSL_FEMTO: - bts->c0->rsl_tei = 0; case GSM_BTS_TYPE_NANOBTS: case GSM_BTS_TYPE_OSMO_SYSMO: /* Set the default OML Stream ID to 0xff */
Speaking of which - anyone in Berlin have an HSL 2G femtocell which I can buy/borrow?
Hi Holger,
On Wed, Jul 03, 2013 at 10:16:05AM +0200, Holger Hans Peter Freyther wrote:
Good Morning Harald,
if you explicitly want me to respond, a Cc to my personal e-mail account might be a good idea, I'm not always following all mailing lists...
do you mind if we drop support for the HSL BTS? The support is spewing out compile warnings, now turns up in coverity reports as well and we don't even know if it is working with the latest version of the software running on these BTS.
no, please go ahead and remove it. The HSL femto is a discontinued product anyway, and the hostile reaction by the HSL CEO has rendered subsequent HSL femto firmware versions intentionally incompatible with OpenBSC anyway.
So let's just remove some dead code...
Regards, Harald
Hi Harald and Holger,
On Wed, Jul 03, 2013 at 10:01:35PM +0200, Harald Welte wrote:
Hi Holger,
On Wed, Jul 03, 2013 at 10:16:05AM +0200, Holger Hans Peter Freyther wrote:
Good Morning Harald,
if you explicitly want me to respond, a Cc to my personal e-mail account might be a good idea, I'm not always following all mailing lists...
do you mind if we drop support for the HSL BTS? The support is spewing out compile warnings, now turns up in coverity reports as well and we don't even know if it is working with the latest version of the software running on these BTS.
no, please go ahead and remove it. The HSL femto is a discontinued product anyway, and the hostile reaction by the HSL CEO has rendered subsequent HSL femto firmware versions intentionally incompatible with OpenBSC anyway.
So let's just remove some dead code...
I'll proceed to remove HSL support from libosmo-abis as well.
Regards.
On Fri, Jul 05, 2013 at 12:23:54AM +0200, Pablo Neira Ayuso wrote:
Hi,
I'll proceed to remove HSL support from libosmo-abis as well.
thanks, I already removed it (as part of looking through the coverity issues). If there was an issue with my removal please poke me.
In coverity we have four more issues, it would be nice if you could have a look at the following:
src/input/ipaccess.c: static int ipaccess_bts_cb(struct ipa_client_conn *link, struct msgb *msg) ... 862 } else if (link->port == IPA_TCP_PORT_OML) 863 e1i_ts = &link->line->ts[0]; 4. Condition "link->port == 3003", taking false branch 864 else if (link->port == IPA_TCP_PORT_RSL) 865 e1i_ts = &link->line->ts[1]; 866 867 /* look up for some existing signaling link. */ CID 1042352 (#1 of 1): Explicit null dereferenced (FORWARD_NULL) 5. var_deref_model: Passing null pointer "e1i_ts" to function "e1inp_lookup_sign_link(struct e1inp_ts *, uint8_t, uint8_t)", which dereferences it. [show details] 868 sign_link = e1inp_lookup_sign_link(e1i_ts, hh->proto, 0);
port can probably only be IPA_TCP_PORT_OML|IPA_TCP_PORT_RSL. Can you confirm this is a false positive? Shall we add an else branch and add a OSMO_ASSERT(false) there?
src/input/ipa.c:
static int ipa_server_fd_cb(struct osmo_fd *ofd, unsigned int what) ... 4. Condition "link->accept_cb", taking true branch 324 if (link->accept_cb) 325 link->accept_cb(link, ret); 326 CID 1040692 (#1 of 1): Resource leak (RESOURCE_LEAK) 5. leaked_handle: Handle variable "ret" going out of scope leaks the handle. 327 return 0;
Shall we have an else close(ret)? or should we enforce the presence of a accept_cb when the socket is bound?
src/input/misdn.c: static int _mi_e1_line_update(struct e1inp_line *line) ... 649 ret = mi_e1_setup(line, 1); 12. Condition "ret", taking false branch 650 if (ret) CID 1040691 (#3 of 4): Resource leak (RESOURCE_LEAK) [select issue] 651 return ret; 652 CID 1040691 (#4 of 4): Resource leak (RESOURCE_LEAK) 13. leaked_handle: Handle variable "sk" going out of scope leaks the handle. 653 return 0;
I would add a close(sk); in the code but I was not sure if mISDN requires this code to be open?
src/input/ipaccess.c: static struct msgb * 713ipa_bts_id_resp(struct ipaccess_unit *dev, uint8_t *data, int len)
751 case IPAC_IDTAG_SWVERSION: CID 1040690 (#4 of 5): Copy into fixed size buffer (STRING_OVERFLOW) 15. fixed_size_dest: You might overrun the 64 byte fixed-size string "str" by copying "dev->swversion" without checking the length. 16. parameter_as_source: Note: This defect has an elevated risk because the source argument is a parameter of the current function. 752 strcpy(str, dev->swversion); 753 break;
this appears to be a genuine issue.
cheers holger
Hi Holger,
On Fri, Jul 05, 2013 at 07:29:15AM +0200, Holger Hans Peter Freyther wrote:
On Fri, Jul 05, 2013 at 12:23:54AM +0200, Pablo Neira Ayuso wrote:
I'll proceed to remove HSL support from libosmo-abis as well.
thanks, I already removed it (as part of looking through the coverity issues). If there was an issue with my removal please poke me.
Looks good, thanks.
In coverity we have four more issues, it would be nice if you could have a look at the following:
src/input/ipaccess.c: static int ipaccess_bts_cb(struct ipa_client_conn *link, struct msgb *msg) ... 862 } else if (link->port == IPA_TCP_PORT_OML) 863 e1i_ts = &link->line->ts[0]; 4. Condition "link->port == 3003", taking false branch 864 else if (link->port == IPA_TCP_PORT_RSL) 865 e1i_ts = &link->line->ts[1]; 866 867 /* look up for some existing signaling link. */ CID 1042352 (#1 of 1): Explicit null dereferenced (FORWARD_NULL) 5. var_deref_model: Passing null pointer "e1i_ts" to function "e1inp_lookup_sign_link(struct e1inp_ts *, uint8_t, uint8_t)", which dereferences it. [show details] 868 sign_link = e1inp_lookup_sign_link(e1i_ts, hh->proto, 0);
port can probably only be IPA_TCP_PORT_OML|IPA_TCP_PORT_RSL. Can you confirm this is a false positive? Shall we add an else branch and add a OSMO_ASSERT(false) there?
This should not ever happen. Added an assertion as you suggested and pushed the patch.
src/input/ipa.c:
static int ipa_server_fd_cb(struct osmo_fd *ofd, unsigned int what) ... 4. Condition "link->accept_cb", taking true branch 324 if (link->accept_cb) 325 link->accept_cb(link, ret); 326 CID 1040692 (#1 of 1): Resource leak (RESOURCE_LEAK) 5. leaked_handle: Handle variable "ret" going out of scope leaks the handle. 327 return 0;
Shall we have an else close(ret)? or should we enforce the presence of a accept_cb when the socket is bound?
Should not happen either, but added the close as you suggested.
src/input/misdn.c: static int _mi_e1_line_update(struct e1inp_line *line) ... 649 ret = mi_e1_setup(line, 1); 12. Condition "ret", taking false branch 650 if (ret) CID 1040691 (#3 of 4): Resource leak (RESOURCE_LEAK) [select issue] 651 return ret; 652 CID 1040691 (#4 of 4): Resource leak (RESOURCE_LEAK) 13. leaked_handle: Handle variable "sk" going out of scope leaks the handle. 653 return 0;
I would add a close(sk); in the code but I was not sure if mISDN requires this code to be open?
I don't have any msidn card. It seems we don't have any ->close callback in the line set to close that socket, but I prefer to leave as is by now until I/someone else can confirm this.
src/input/ipaccess.c: static struct msgb * 713ipa_bts_id_resp(struct ipaccess_unit *dev, uint8_t *data, int len)
751 case IPAC_IDTAG_SWVERSION: CID 1040690 (#4 of 5): Copy into fixed size buffer (STRING_OVERFLOW) 15. fixed_size_dest: You might overrun the 64 byte fixed-size string "str" by copying "dev->swversion" without checking the length. 16. parameter_as_source: Note: This defect has an elevated risk because the source argument is a parameter of the current function. 752 strcpy(str, dev->swversion); 753 break;
this appears to be a genuine issue.
Those strings are set in the configuration path, I have fix it, no such an "elevated risk" as coverity spotted.
Also pushed a couple of patches to fix compilation warnings.
I have this warning as well:
e1_input.c: In function 'write_pcap_packet': e1_input.c:163:6: warning: variable 'ret' set but not used [-Wunused-but-set-variable]
I can fix it by returning error in abis_sendmsg and e1inp_rx_ts if it cannot write the traces to the pcap file (should only affect isdn-based BTSes) including some log message.
Let me know if you have any issue with those.
Thanks!
On Fri, Jul 05, 2013 at 03:55:22PM +0200, Pablo Neira Ayuso wrote:
Looks good, thanks.
Thanks, thanks a lot for the speedy fix-up of things. Do you follow coverity reports for netfilter too?
Should not happen either, but added the close as you suggested.
OSMO_ASSERT(link->accept_cb) maybe? Coverity is still not happy about the ret (mostly because it doesn't find an assignment but then I would probably need to build every project as one thing).
E.g. we don't check the return value of accept_cb, but if you want to, I can close this as a false positive now.
I don't have any msidn card. It seems we don't have any ->close callback in the line set to close that socket, but I prefer to leave as is by now until I/someone else can confirm this.
I think 'sk' is only used to gain information about the mISDN device (it is a bit racy, as at the time we use it the card might be gone, I assume we can just close the sk after the last ioctl).
Those strings are set in the configuration path, I have fix it, no such an "elevated risk" as coverity spotted.
thanks.
Let me know if you have any issue with those.
Coverity found another thing (so apparently it 'learns')
src/input/dahdi.c
404 if (line->port_nr > ARRAY_SIZE(span_cfgs)) 405 return; 406 CID 1042368 (#1 of 1): Out-of-bounds read (OVERRUN) 3. overrun-local: Overrunning array "span_cfgs" of 128 4-byte elements at element index 128 (byte offset 512) using index "line->port_nr" (which evaluates to 128). 407 scfg = span_cfgs[line->port_nr];
So I think this needs to be a >=. Please use CID in the commit message when fixing it (or in case you are busy and ack that >= is the right fix I will make the commit).
holger
Hi Holger,
On Fri, Jul 05, 2013 at 07:24:49PM +0200, Holger Hans Peter Freyther wrote: [...]
Should not happen either, but added the close as you suggested.
OSMO_ASSERT(link->accept_cb) maybe? Coverity is still not happy about the ret (mostly because it doesn't find an assignment but then I would probably need to build every project as one thing).
E.g. we don't check the return value of accept_cb, but if you want to, I can close this as a false positive now.
Just pushed a patch for this, let's see if it calms down coverity.
I don't have any msidn card. It seems we don't have any ->close callback in the line set to close that socket, but I prefer to leave as is by now until I/someone else can confirm this.
I think 'sk' is only used to gain information about the mISDN device (it is a bit racy, as at the time we use it the card might be gone, I assume we can just close the sk after the last ioctl).
[...]
CID 1042368 (#1 of 1): Out-of-bounds read (OVERRUN) 3. overrun-local: Overrunning array "span_cfgs" of 128 4-byte elements at element index 128 (byte offset 512) using index "line->port_nr" (which evaluates to 128). 407 scfg = span_cfgs[line->port_nr];
So I think this needs to be a >=. Please use CID in the commit message when fixing it (or in case you are busy and ack that >= is the right fix I will make the commit).
I just noticed that you pushed two patches for this. Thanks.
BTW, I'm hitting this compilation warning here:
trau/osmo_ortp.c: In function 'osmo_rtcp_fd_cb': trau/osmo_ortp.c:207:2: warning: implicit declaration of function 'rtp_session_rtcp_recv' [-Wimplicit-function-declaration]
I can find that function defined in a private header of ortp, any clue on why we need it there?
On Mon, Jul 08, 2013 at 01:28:44AM +0200, Pablo Neira Ayuso wrote:
Just pushed a patch for this, let's see if it calms down coverity.
it is still confused as it is not capable of seeing who sets the accept_cb (i would need to build everything at once and it might be different). I have marked this as either false positive or intentional.
libosmo-abis is the first repository with zero coverity issues. Well maybe tnt bet me and libosmo-dsp/osmo-gmr got there first.
thank you very much for having a look and quick responses!
holger
Hi Pablo,
On Mon, Jul 08, 2013 at 01:28:44AM +0200, Pablo Neira Ayuso wrote:
BTW, I'm hitting this compilation warning here:
trau/osmo_ortp.c: In function 'osmo_rtcp_fd_cb': trau/osmo_ortp.c:207:2: warning: implicit declaration of function 'rtp_session_rtcp_recv' [-Wimplicit-function-declaration]
I can find that function defined in a private header of ortp, any clue on why we need it there?
Please read the comment in the source above it:
/* We probably don't need this at all, as * rtp_session_recvm_with_ts() will alway also poll the RTCP * file descriptor for new data */
Theoretically it is the right thing for the application/osmocore select loop handling to detect the arrival of a RTCP message on the socket and let the ortp library know.
However, ortp seems to prefer to simply poll that file descriptor every time a RTP data frame arrives. So if you receive an RTCP message but not an RTP frame in a long time, that RTCP will get delayed witnhout any good reason.
So I thought it might be a good idea to simply pass this information into the library.
RTCP handling of libosmo-trau was never tested so far, AFAIK.
Regards, Harald