laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/38958?usp=email ) Change subject: pcu_sock: do not receive a TXT ind. with PCU_VERSION for a specific BTS ...................................................................... pcu_sock: do not receive a TXT ind. with PCU_VERSION for a specific BTS We receive a TXT indication that contains the PCU_VERSION from the PCU when the connection to the PCU is established. This message contains a BTS number, which is always 0. This is a hard coded value that does not refer to a specific BTS object. Also it is not logical to inform a specific BTS object about the PCU version. This information should be directed to the connecting process as a whole. However, we use this TXT indication to trigger certain initialization processes on the BTS object we manage inside the BTS process (currently this is only 1 bts, but this may change in the future). So instead of using the BTS in the TXT indication, we should iterate of over all BTS objects and trigger the initializations for each of the BTS objects. This change does not have any dependencies, nor does it change the current behavior of osmo-bts. It just cleans up the logic, so that it works by intension and not just by chance. Related: OS#6507 Change-Id: I1bb8d0ec5e8d4b9f822f94249a75d8dc477144a3 --- M src/common/pcu_sock.c 1 file changed, 37 insertions(+), 15 deletions(-) Approvals: laforge: Looks good to me, approved pespin: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/src/common/pcu_sock.c b/src/common/pcu_sock.c index 048e766..3f1689a 100644 --- a/src/common/pcu_sock.c +++ b/src/common/pcu_sock.c @@ -858,25 +858,31 @@ 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: + OSMO_ASSERT(bts); oml_tx_failure_event_rep(&bts->gprs.cell.mo, NM_SEVER_INDETERMINATE, OSMO_EVT_EXT_ALARM, txt->text); break; @@ -937,36 +943,52 @@ return -EINVAL; \ } \ } while (0) + +#define ENSURE_BTS_OBJECT(bts) \ + do { \ + 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; \ + } \ + } while (0) + 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; 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: CHECK_IF_MSG_SIZE(prim_len, pcu_prim->u.data_req); + ENSURE_BTS_OBJECT(bts); rc = pcu_rx_data_req(bts, msg_type, &pcu_prim->u.data_req); break; case PCU_IF_MSG_PAG_REQ: CHECK_IF_MSG_SIZE(prim_len, pcu_prim->u.pag_req); + ENSURE_BTS_OBJECT(bts); rc = pcu_rx_pag_req(bts, msg_type, &pcu_prim->u.pag_req); break; case PCU_IF_MSG_ACT_REQ: CHECK_IF_MSG_SIZE(prim_len, pcu_prim->u.act_req); + ENSURE_BTS_OBJECT(bts); rc = pcu_rx_act_req(bts, &pcu_prim->u.act_req); break; case PCU_IF_MSG_TXT_IND: CHECK_IF_MSG_SIZE(prim_len, pcu_prim->u.txt_ind); - rc = pcu_rx_txt_ind(bts, &pcu_prim->u.txt_ind); + if (pcu_prim->u.txt_ind.type == PCU_VERSION) { + /* A TXT indication that carries the PCU_VERSION is always addressed to the + * receiving process as a whole, which means we will not resolve a specific + * BTS object in this case. */ + rc = pcu_rx_txt_ind(NULL, &pcu_prim->u.txt_ind); + } else { + ENSURE_BTS_OBJECT(bts); + rc = pcu_rx_txt_ind(bts, &pcu_prim->u.txt_ind); + } break; case PCU_IF_MSG_CONTAINER: CHECK_IF_MSG_SIZE(prim_len, pcu_prim->u.container); + ENSURE_BTS_OBJECT(bts); /* ^ 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); if (prim_len < exp_len) { -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/38958?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I1bb8d0ec5e8d4b9f822f94249a75d8dc477144a3 Gerrit-Change-Number: 38958 Gerrit-PatchSet: 4 Gerrit-Owner: dexter &lt;pmaier(a)sysmocom.de&gt; Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge &lt;laforge(a)osmocom.org&gt; Gerrit-Reviewer: pespin &lt;pespin(a)sysmocom.de&gt;