- OpenBSC - lists.osmocom.org

Re: Lack of maintenance for osmo-bts-trx
by Tom Tsou 16 Apr '16

16 Apr '16

Hi Harald, On Mon, Apr 4, 2016 at 8:10 AM, Harald Welte <laforge(a)gnumonks.org> wrote: > Ever since the merge in August 2015, as far as I recall, I never saw a > single message from any osmo-bts-trx user whether > a) the L1SAP merge back then works for them > b) it introduced any regressions I suspect that there were issues since the beginning of the merge. The issue that I saw with master was reported by Sipos Csaba back in Nov 2015. http://lists.osmocom.org/pipermail/openbsc/2015-November/000856.html Unlike Alexander, I was never able to setup a working version with older commits including the one specified in this post. http://lists.osmocom.org/pipermail/openbsc/2015-September/000398.html I guess there is something wrong with the OML setup since trx->mo.nm_state is reporting NM_OPSTATE_NULL during the post-RACH channel allocation, but I am not familiar enough with those procedures to debug more. > While some people think I sometimes write good code, I fail to believe > that all those intrusive changes didn't break anything for OsmoTRX > users. Still, no feedback and no fall-out was reported. I suspect the case was that very few users, if any, had working setups to break or to report regressions. > Like any larger project, particularly one with different hardware > support, we need somebody with essential interest in that hardware to > maintain the respective back-end. Think of device driver maintainers in > the Linux kernel as an example. Agreed. > We need a sub-maintainer for osmo-bts-trx, one who > 1) consistently follows the developments in master and checks for > regressiosn > 2) fixes osmo-bts-trx specific bugs like the fact that GPRS measurment > values are not passed to the PCU, breaking rate/link adaption for > this platform > 3) tests interoperability with OsmoTRX and other OpenBTS transceivers > out three > 4) ensures that important osmo-bts-trx related patches/branches end up > in master, ensuring that master is what people use. Telling users to > use a branch different than master is *wrong* for anything but the > most bleeding edge / experiemental code which is to be merged ASAP. i I can provide a supporting role in the above tasks on behalf of Ettus but we do need a working starting point - that makes number 4 the most critical item. Unfortunately, I am not able to resolve the patch differences (I painfully tried) to make master the main user branch. I agree that directing users to the Fairwaves branch is the wrong approach, but right now it is the only branch for osmo-bts-trx that works. -TT

4 4

[PATCH] Add function to get osmo_earfcn bit size
by msuraev＠sysmocom.de 16 Apr '16

16 Apr '16

From: Max <msuraev(a)sysmocom.de> This function returns the number of bits necessary to pack osmo_earfcn into Repeated E-UTRAN Neighbour Cells IE (3GPP TS 44.018 Table 10.5.2.33b.1). --- include/osmocom/gsm/sysinfo.h | 1 + src/gsm/libosmogsm.map | 1 + src/gsm/sysinfo.c | 24 ++++++++++++++++++++++++ 3 files changed, 26 insertions(+) diff --git a/include/osmocom/gsm/sysinfo.h b/include/osmocom/gsm/sysinfo.h index cb7e10d..c7972f4 100644 --- a/include/osmocom/gsm/sysinfo.h +++ b/include/osmocom/gsm/sysinfo.h @@ -66,6 +66,7 @@ typedef uint8_t sysinfo_buf_t[GSM_MACBLOCK_LEN]; extern const struct value_string osmo_sitype_strs[_MAX_SYSINFO_TYPE]; int osmo_earfcn_add(struct osmo_earfcn_si2q *e, uint16_t arfcn, uint8_t meas_bw); int osmo_earfcn_del(struct osmo_earfcn_si2q *e, uint16_t arfcn); +size_t osmo_earfcn_bit_size(const struct osmo_earfcn_si2q *e); void osmo_earfcn_init(struct osmo_earfcn_si2q *e); uint8_t osmo_sitype2rsl(enum osmo_sysinfo_type si_type); enum osmo_sysinfo_type osmo_rsl2sitype(uint8_t rsl_si); diff --git a/src/gsm/libosmogsm.map b/src/gsm/libosmogsm.map index a3e4e14..6886a6c 100644 --- a/src/gsm/libosmogsm.map +++ b/src/gsm/libosmogsm.map @@ -31,6 +31,7 @@ dbm2rxlev; osmo_earfcn_add; osmo_earfcn_del; +osmo_earfcn_bit_size; osmo_earfcn_init; gprs_cipher_gen_input_i; diff --git a/src/gsm/sysinfo.c b/src/gsm/sysinfo.c index 42e717f..3ec5444 100644 --- a/src/gsm/sysinfo.c +++ b/src/gsm/sysinfo.c @@ -144,6 +144,30 @@ int osmo_earfcn_add(struct osmo_earfcn_si2q *e, uint16_t arfcn, uint8_t meas_bw) return -ENOMEM; } +/*! \brief Return number of bits necessary to represent earfcn struct as + * Repeated E-UTRAN Neighbour Cells IE from 3GPP TS 44.018 Table 10.5.2.33b.1 + * \param[in,out] e earfcn struct + * \returns number of bits + */ +size_t osmo_earfcn_bit_size(const struct osmo_earfcn_si2q *e) +{ + /* 1 stop bit + 5 bits for THRESH_E-UTRAN_high */ + size_t i, bits = 6; + for (i = 0; i < e->length; i++) { + if (e->arfcn[i] != OSMO_EARFCN_INVALID) { + bits += 17; + if (OSMO_EARFCN_MEAS_INVALID == e->meas_bw[i]) + bits++; + else + bits += 4; + } + } + bits += (e->prio_valid) ? 4 : 1; + bits += (e->thresh_lo_valid) ? 6 : 1; + bits += (e->qrxlm_valid) ? 6 : 1; + return bits; +} + /*! \brief Delete arfcn (and corresponding measurement bandwith) from earfcn * struct * \param[in,out] e earfcn struct -- 2.8.1

2 1

[PATCH] logging.c: assert category_token allocation (CID #57871)
by Neels Hofmeyr 16 Apr '16

16 Apr '16

--- src/logging.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/logging.c b/src/logging.c index 35746e9..b96e64a 100644 --- a/src/logging.c +++ b/src/logging.c @@ -190,6 +190,7 @@ void log_parse_category_mask(struct log_target* target, const char *_mask) target->categories[i].enabled = 0; category_token = strtok(mask, ":"); + OSMO_ASSERT(category_token); do { for (i = 0; i < osmo_log_info->num_cat; ++i) { size_t length, cat_length; -- 2.1.4

2 1

[PATCH] sua_test_common.c: use global RANAP SSN definition
by Neels Hofmeyr 16 Apr '16

16 Apr '16

The test 'hardcoded' a local define for RANAP SSN as 142, instead use the proper OSMO_SCCP_SSN_RANAP define from sccp_sap.h --- tests/sigtran/sua_test_common.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/sigtran/sua_test_common.c b/tests/sigtran/sua_test_common.c index aed4899..66fade0 100644 --- a/tests/sigtran/sua_test_common.c +++ b/tests/sigtran/sua_test_common.c @@ -44,8 +44,6 @@ static void sccp_make_addr_pc_ssn(struct osmo_sccp_addr *addr, uint32_t pc, uint addr->pc = pc; } -#define SSN_RANAP 142 - struct osmo_prim_hdr *make_conn_req(uint32_t conn_id) { struct msgb *msg = msgb_alloc(1024, "conn_req"); @@ -56,8 +54,8 @@ struct osmo_prim_hdr *make_conn_req(uint32_t conn_id) OSMO_SCU_PRIM_N_CONNECT, PRIM_OP_REQUEST, msg); /* Set SSN for calling and called addr */ - sccp_make_addr_pc_ssn(&prim->u.connect.called_addr, 2, SSN_RANAP); - sccp_make_addr_pc_ssn(&prim->u.connect.calling_addr, 1, SSN_RANAP); + sccp_make_addr_pc_ssn(&prim->u.connect.called_addr, 2, OSMO_SCCP_SSN_RANAP); + sccp_make_addr_pc_ssn(&prim->u.connect.calling_addr, 1, OSMO_SCCP_SSN_RANAP); prim->u.connect.sccp_class = 2; prim->u.connect.conn_id = conn_id; -- 2.1.4

2 1

[PATCH] Fix RANAP SSN: it's 142, not 143
by Neels Hofmeyr 16 Apr '16

16 Apr '16

143 is actually the SSN for RNSAP. Wireshark displayed a RNSAP message type and malformed packet warning until I fixed this to 142. Now I get the proper RANAP and id-Paging reported. There has been a reallocation for RANAP and RNSAP SSNs, though the old SSN for RANAP is apparently 32 (seen in a pcap from a real 3G network). When I send 32 instead of 142, wireshark also decodes the message as valid RANAP. --- include/osmocom/sigtran/sccp_sap.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/osmocom/sigtran/sccp_sap.h b/include/osmocom/sigtran/sccp_sap.h index 5696b47..15aa840 100644 --- a/include/osmocom/sigtran/sccp_sap.h +++ b/include/osmocom/sigtran/sccp_sap.h @@ -106,7 +106,7 @@ enum osmo_sccp_ssn { OSMO_SCCP_SSN_RES_INTL = 0x0c, OSMO_SCCP_SSN_BISDN = 0x0d, OSMO_SCCP_SSN_TC_TEST = 0x0e, - OSMO_SCCP_SSN_RANAP = 143, + OSMO_SCCP_SSN_RANAP = 142, }; struct osmo_sccp_gt { -- 2.1.4

2 1

[PATCH] compiler warning: unconstify arg passed to OCTET_STRING_noalloc()
by Neels Hofmeyr 15 Apr '16

15 Apr '16

This is after the patch to libasn1c, removing str arg constness. --- src/ranap_msg_factory.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ranap_msg_factory.c b/src/ranap_msg_factory.c index bdae92e..461df1a 100644 --- a/src/ranap_msg_factory.c +++ b/src/ranap_msg_factory.c @@ -181,7 +181,7 @@ struct msgb *ranap_new_msg_dt(uint8_t sapi, const uint8_t *nas, unsigned int nas ies.sapi = RANAP_SAPI_sapi_0; /* Avoid copying + later freeing of OCTET STRING */ - OCTET_STRING_noalloc(&ies.nas_pdu, nas, nas_len); + OCTET_STRING_noalloc(&ies.nas_pdu, (uint8_t*)nas, nas_len); /* ies -> dt */ rc = ranap_encode_directtransferies(&dt, &ies); -- 2.1.4

3 4

Lack of maintenance for osmo-bts-trx
by Harald Welte 15 Apr '16

15 Apr '16

Dear all, I am growing incredibly frustrated at the fact that osmo-bts-trx is effectively unmaintained, despite the fact that there are plenty of users as well as even commercial users / companies (particularly Fairwaves). I'm not pointing Fairwaves out because I am associated with sysmocom. I'm pointing them out as for me as Osmocom project founder and co-maintainer, I sincerely think something is wrong here. And particuarly commercial vendors of BTSs using the Osmocom stack should at the very absolute minimum ensure proper maintenance of their own hardware support in the respective Osmocom projects. They should equally consider contributing to maintenance and developemnt of the 'core' code of those projects, too, in order to fairly share the burden of that effort. I explicitly asked for proper maintenance of osmo-bts-trx first in August 2014 in this post: http://lists.osmocom.org/pipermail/openbsc/2014-August/007657.html Back in 2014 and the first half of 2015 I spent a lot of time in forward-porting and cleaning up the the (fairwaves-commissioned?) l1sap changes, as well as the associated osmo-bts-trx port. Back then, there were several patches in related (several) branches that I could not merge. See my mail (and follow-up mails) from http://lists.osmocom.org/pipermail/openbsc/2015-September/000379.html Ever since the merge in August 2015, as far as I recall, I never saw a single message from any osmo-bts-trx user whether a) the L1SAP merge back then works for them b) it introduced any regressions The same happened after the phy_link/phy_interface merge. While some people think I sometimes write good code, I fail to believe that all those intrusive changes didn't break anything for OsmoTRX users. Still, no feedback and no fall-out was reported. Like any larger project, particularly one with different hardware support, we need somebody with essential interest in that hardware to maintain the respective back-end. Think of device driver maintainers in the Linux kernel as an example. For osmo-bts-sysmo, it is clear that sysmocom has such a vital interest, and that it is the primary hardware on whcih Holger and I are working on. For osmo-bts-octphy, Octasic funds related maintenance work to be done at sysmocom. We need a sub-maintainer for osmo-bts-trx, one who 1) consistently follows the developments in master and checks for regressiosn 2) fixes osmo-bts-trx specific bugs like the fact that GPRS measurment values are not passed to the PCU, breaking rate/link adaption for this platform 3) tests interoperability with OsmoTRX and other OpenBTS transceivers out three 4) ensures that important osmo-bts-trx related patches/branches end up in master, ensuring that master is what people use. Telling users to use a branch different than master is *wrong* for anything but the most bleeding edge / experiemental code which is to be merged ASAP. I thus strongly suggest that the users and proponents of osmo-bts-trx get together and see how they can ensure the proper maintenance of this port. Thanks for your consideration. Regards, Harald -- - Harald Welte <laforge(a)gnumonks.org> http://laforge.gnumonks.org/ ============================================================================ "Privacy in residential applications is a desirable marketing option." (ETSI EN 300 175-7 Ch. A6)

4 4

[PATCH] compiler warning: unconstify arg passed to OCTET_STRING_noalloc()
by Neels Hofmeyr 14 Apr '16

14 Apr '16

This is after the patch to libasn1c, removing str arg constness. Note the comment added in the code. --- src/ranap_msg_factory.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/ranap_msg_factory.c b/src/ranap_msg_factory.c index bdae92e..e87eeaf 100644 --- a/src/ranap_msg_factory.c +++ b/src/ranap_msg_factory.c @@ -180,8 +180,13 @@ struct msgb *ranap_new_msg_dt(uint8_t sapi, const uint8_t *nas, unsigned int nas else ies.sapi = RANAP_SAPI_sapi_0; - /* Avoid copying + later freeing of OCTET STRING */ - OCTET_STRING_noalloc(&ies.nas_pdu, nas, nas_len); + /* + * Avoid copying + later freeing of OCTET STRING. + * To store the pointer in an OCTET_STRING_t without copying, we need + * to un-const *nas. Note: un-consting *nas is safe because ies.nas_pdu + * is only used to store the IEs in dt. + */ + OCTET_STRING_noalloc(&ies.nas_pdu, (uint8_t*)nas, nas_len); /* ies -> dt */ rc = ranap_encode_directtransferies(&dt, &ies); -- 2.1.4

1 0

iuh: Coverity build breakage because of old sctp
by Holger Freyther 14 Apr '16

14 Apr '16

Hi, sua.c: In function 'rx_inact_tmr_cb': sua.c:288: warning: unused variable 'conn' sua.c: In function 'sua_rx_coref': sua.c:908: warning: unused variable 'cause' sua.c: In function 'sua_rx_codt': sua.c:1059: warning: unused variable 'cur' sua.c: In function 'sua_srv_conn_cb': sua.c:1266: error: 'SCTP_SENDER_DRY_EVENT' undeclared (first use in this function) sua.c:1266: error: (Each undeclared identifier is reported only once sua.c:1266: error: for each function it appears in.) sua.c: In function 'sua_cli_read_cb': sua.c:1451: error: 'SCTP_SENDER_DRY_EVENT' undeclared (first use in this function) can we make the SCTP_SENDER_DRY_EVENT optional or is a strict requirement? If it is strict then maybe we can check for this in autoconf? holger

2 6

[PATCH 1/2] vty_test_runner: update ipa sending code
by msuraev＠sysmocom.de 14 Apr '16

14 Apr '16

From: Max <msuraev(a)sysmocom.de> Factor out 2, add 3 functions. Those functions are simple wrappers around hex strings specific to IPA protocol. Not all of them are utilized at the moment but they were checked with wireshark while working on the tests. It might come in handy if we'd like to further expand related test harness in future. The same goes for optional verbosity argument which is not used right now but will be handy for future debugging. --- openbsc/tests/vty_test_runner.py | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/openbsc/tests/vty_test_runner.py b/openbsc/tests/vty_test_runner.py index ecf5204..d574129 100644 --- a/openbsc/tests/vty_test_runner.py +++ b/openbsc/tests/vty_test_runner.py @@ -736,13 +736,13 @@ class TestVTYNAT(TestVTYGenericBSC): self.assertEqual(data, "\x00\x01\xfe\x04") print "Going to send ID_RESP response" - res = ussdSocket.send("\x00\x07\xfe\x05\x00\x04\x01\x6b\x65\x79") + res = ipa_send_resp(ussdSocket, "\x6b\x65\x79") self.assertEqual(res, 10) # initiating PING/PONG cycle to know, that the ID_RESP message has been processed print "Going to send PING request" - res = ussdSocket.send("\x00\x01\xfe\x00") + res = ipa_send_ping(ussdSocket) self.assertEqual(res, 4) print "Expecting PONG response" @@ -1007,6 +1007,31 @@ def add_nat_test(suite, workdir): test = unittest.TestLoader().loadTestsFromTestCase(TestVTYNAT) suite.addTest(test) +def ipa_send_pong(x, verbose = False): + if (verbose): + print "\tBSC -> NAT: PONG!" + return x.send("\x00\x01\xfe\x01") + +def ipa_send_ping(x, verbose = False): + if (verbose): + print "\tBSC -> NAT: PING?" + return x.send("\x00\x01\xfe\x00") + +def ipa_send_ack(x, verbose = False): + if (verbose): + print "\tBSC -> NAT: IPA ID ACK" + return x.send("\x00\x01\xfe\x06") + +def ipa_send_reset(x, verbose = False): + if (verbose): + print "\tBSC -> NAT: RESET" + return x.send("\x00\x12\xfd\x09\x00\x03\x05\x07\x02\x42\xfe\x02\x42\xfe\x06\x00\x04\x30\x04\x01\x20") + +def ipa_send_resp(x, tk, verbose = False): + if (verbose): + print "\tBSC -> NAT: IPA ID RESP" + return x.send("\x00\x07\xfe\x05\x00\x04\x01" + tk) + def add_bsc_test(suite, workdir): if not os.path.isfile(os.path.join(workdir, "src/osmo-bsc/osmo-bsc")): print("Skipping the BSC test") -- 2.8.1

2 2