dexter has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bts/+/38090?usp=email ) Change subject: pcuif: fix TXT_IND/PCU_VERSION BTS initialization behavior ...................................................................... pcuif: fix TXT_IND/PCU_VERSION BTS initialization behavior The PCU sends a TXT_IND with the PCU_VERSION as first message after the socket connection is established. The BTS uses those messages to trigger some initialization (allocate BTS object inside the PCU, send SI to the PCU). Unfortunately the PCU will send the TXT_IND with the PCU_VERSION always with to BTS 0. The 0 in this case is a hardcoded fake BTS number (There is no BTS object allocated under this number in the PCU at this point). The BTS will then do the initialization process for BTS 0, which is only correct by chance since osmo-bts currently only supports one BTS object with the number 0. Nevertheless it is incorrect to use the BTS number from the TXT_IND to resolve the BTS object. Let's fix this design error by introducing a broadcast BTS number, we then send the TXT_IND with the PCU_VERSION to that broadcast BTS number. The BTS will then do the initialization for all BTS objects it is aware of (BTS 0) Related: OS#6507 Change-Id: I5316d3b7cef416eb19bb256f4ccc1468b3efe1c6 Depends: osmo-bsc.git Ie92f5833a80b06e78c6cec8f03f054e2e2625fad Depends: osmo-pcu.git Id01966a1ee52d0f5e465dc2e0eaf85e5b7942f81 Depends: osmo-ttcn3-hacks.git Idce13d55fc5bdbf5b366e3a60d9b46aacfc701a9 Depends: docker-playground.git I15f7150b51047379a557a1f8df6330eca597f2e3 --- M include/osmo-bts/pcuif_proto.h M src/common/pcu_sock.c 2 files changed, 43 insertions(+), 18 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/90/38090/1 diff --git a/include/osmo-bts/pcuif_proto.h b/include/osmo-bts/pcuif_proto.h index 04936af..5e64980 100644 --- a/include/osmo-bts/pcuif_proto.h +++ b/include/osmo-bts/pcuif_proto.h @@ -7,7 +7,7 @@ #define PCU_SOCK_DEFAULT "/tmp/pcu_bts" -#define PCU_IF_VERSION 0x0c +#define PCU_IF_VERSION 0x0d #define TXT_MAX_LEN 128 /* msg_type */ @@ -257,6 +257,10 @@ bool confirm; } __attribute__((packed)); +/* reserved BTS number to indicate that the PCUIF INDICATION is not targeted to a + * specific BTS. (commonly used with TXT indications to transfer the PCU version number) */ +#define PCU_IF_BTS_NR_BCAST 0xff + struct gsm_pcu_if { /* context based information */ uint8_t msg_type; /* message type */ diff --git a/src/common/pcu_sock.c b/src/common/pcu_sock.c index 048e766..bf2e395 100644 --- a/src/common/pcu_sock.c +++ b/src/common/pcu_sock.c @@ -858,27 +858,40 @@ static int pcu_rx_txt_ind(struct gsm_bts *bts, struct gsm_pcu_if_txt_ind *txt) { - int rc; + int rc = 0; switch (txt->type) { case PCU_VERSION: LOGP(DPCU, LOGL_INFO, "OsmoPCU version %s connected\n", txt->text); - oml_tx_failure_event_rep(&bts->gprs.cell.mo, NM_SEVER_CEASED, OSMO_EVT_PCU_VERS, txt->text); - osmo_strlcpy(bts->pcu_version, txt->text, MAX_VERSION_LENGTH); - /* patch SI to advertise GPRS, *if* the SI sent by BSC said so */ - regenerate_si3_restoctets(bts); - regenerate_si4_restoctets(bts); + /* we use the reception of the PCU_VERSION as a trigger to make the PCU available for + * all BTSs handled by this process (currently this is exactly one BTS, see FIXME notes) */ + llist_for_each_entry(bts, &g_bts_sm->bts_list, list) { + oml_tx_failure_event_rep(&bts->gprs.cell.mo, NM_SEVER_CEASED, OSMO_EVT_PCU_VERS, txt->text); + osmo_strlcpy(bts->pcu_version, txt->text, MAX_VERSION_LENGTH); - rc = pcu_tx_si_all(bts); + /* patch SI to advertise GPRS, *if* the SI sent by BSC said so */ + regenerate_si3_restoctets(bts); + regenerate_si4_restoctets(bts); + + if (pcu_tx_si_all(bts) < 0) + rc = -EINVAL; + } if (rc < 0) - return -EINVAL; - + return rc; break; case PCU_OML_ALERT: - oml_tx_failure_event_rep(&bts->gprs.cell.mo, NM_SEVER_INDETERMINATE, OSMO_EVT_EXT_ALARM, - txt->text); + if (bts) { + /* The PCU_OML_ALERT is directed to a spcific BTS object */ + oml_tx_failure_event_rep(&bts->gprs.cell.mo, NM_SEVER_INDETERMINATE, OSMO_EVT_EXT_ALARM, txt->text); + } else { + /* The PCU_OML_ALERT is directed to all BTS objects (currently this is exactly one BTS, + * see FIXME notes) */ + llist_for_each_entry(bts, &g_bts_sm->bts_list, list) { + oml_tx_failure_event_rep(&bts->gprs.cell.mo, NM_SEVER_INDETERMINATE, OSMO_EVT_EXT_ALARM, txt->text); + } + } break; default: LOGP(DPCU, LOGL_ERROR, "Unknown TXT_IND type %u received\n", @@ -937,35 +950,43 @@ return -EINVAL; \ } \ } while (0) + +#define ENSURE_BTS_OBJECT() \ + if ((bts = gsm_bts_num(g_bts_sm, pcu_prim->bts_nr)) == NULL) { \ + LOGP(DPCU, LOGL_ERROR, "Received PCU Prim for non-existent BTS %u\n", pcu_prim->bts_nr); \ + return -EINVAL; \ + } + static int pcu_rx(uint8_t msg_type, struct gsm_pcu_if *pcu_prim, size_t prim_len) { int rc = 0; - struct gsm_bts *bts; + struct gsm_bts *bts = NULL; size_t exp_len; - if ((bts = gsm_bts_num(g_bts_sm, pcu_prim->bts_nr)) == NULL) { - LOGP(DPCU, LOGL_ERROR, "Received PCU Prim for non-existent BTS %u\n", pcu_prim->bts_nr); - return -EINVAL; - } - switch (msg_type) { case PCU_IF_MSG_DATA_REQ: + ENSURE_BTS_OBJECT(); CHECK_IF_MSG_SIZE(prim_len, pcu_prim->u.data_req); rc = pcu_rx_data_req(bts, msg_type, &pcu_prim->u.data_req); break; case PCU_IF_MSG_PAG_REQ: + ENSURE_BTS_OBJECT(); CHECK_IF_MSG_SIZE(prim_len, pcu_prim->u.pag_req); rc = pcu_rx_pag_req(bts, msg_type, &pcu_prim->u.pag_req); break; case PCU_IF_MSG_ACT_REQ: + ENSURE_BTS_OBJECT(); CHECK_IF_MSG_SIZE(prim_len, pcu_prim->u.act_req); rc = pcu_rx_act_req(bts, &pcu_prim->u.act_req); break; case PCU_IF_MSG_TXT_IND: + if (pcu_prim->bts_nr != PCU_IF_BTS_NR_BCAST) + ENSURE_BTS_OBJECT(); CHECK_IF_MSG_SIZE(prim_len, pcu_prim->u.txt_ind); rc = pcu_rx_txt_ind(bts, &pcu_prim->u.txt_ind); break; case PCU_IF_MSG_CONTAINER: + ENSURE_BTS_OBJECT(); CHECK_IF_MSG_SIZE(prim_len, pcu_prim->u.container); /* ^ check if we can access container fields, v check with container data length */ exp_len = PCUIF_HDR_SIZE + sizeof(pcu_prim->u.container) + osmo_load16be(&pcu_prim->u.container.length); -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/38090?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I5316d3b7cef416eb19bb256f4ccc1468b3efe1c6 Gerrit-Change-Number: 38090 Gerrit-PatchSet: 1 Gerrit-Owner: dexter &lt;pmaier(a)sysmocom.de&gt;