<p>laforge <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/19264">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Jenkins Builder: Verified
fixeria: Looks good to me, approved
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">bts_nokia_site: Clean up logging<br><br>We don't want to fprintf directly, and we want to make sure to always<br>log as much context as possible.<br><br>Change-Id: I29ec935669175a08cb42e1666559b681c50a6e72<br>---<br>M src/osmo-bsc/bts_nokia_site.c<br>1 file changed, 37 insertions(+), 53 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/osmo-bsc/bts_nokia_site.c b/src/osmo-bsc/bts_nokia_site.c</span><br><span>index 0d0dbb5..51710b8 100644</span><br><span>--- a/src/osmo-bsc/bts_nokia_site.c</span><br><span>+++ b/src/osmo-bsc/bts_nokia_site.c</span><br><span>@@ -59,7 +59,7 @@</span><br><span> </span><br><span> static void bootstrap_om_bts(struct gsm_bts *bts)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DNM, LOGL_NOTICE, "bootstrapping OML for BTS %u\n", bts->nr);</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_BTS(bts, DNM, LOGL_NOTICE, "bootstrapping OML\n");</span><br><span> </span><br><span> if (!bts->nokia.skip_reset) {</span><br><span> if (!bts->nokia.did_reset)</span><br><span>@@ -72,8 +72,7 @@</span><br><span> </span><br><span> static void bootstrap_om_trx(struct gsm_bts_trx *trx)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DNM, LOGL_NOTICE, "bootstrapping OML for TRX %u/%u\n",</span><br><span style="color: hsl(0, 100%, 40%);">- trx->bts->nr, trx->nr);</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_TRX(trx, DNM, LOGL_NOTICE, "bootstrapping OML\n");</span><br><span> </span><br><span> gsm_trx_all_ts_dispatch(trx, TS_EV_OML_READY, NULL);</span><br><span> }</span><br><span>@@ -762,7 +761,7 @@</span><br><span> /* set CA */</span><br><span> </span><br><span> if (generate_cell_chan_list(&fu_config[38], trx->bts) != 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- fprintf(stderr, "generate_cell_chan_list failed\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_TRX(trx, DNM, LOGL_ERROR, "generate_cell_chan_list failed\n");</span><br><span> return 0;</span><br><span> }</span><br><span> </span><br><span>@@ -810,8 +809,7 @@</span><br><span> chan_config = 11;</span><br><span> break;</span><br><span> default:</span><br><span style="color: hsl(0, 100%, 40%);">- fprintf(stderr,</span><br><span style="color: hsl(0, 100%, 40%);">- "unsupported channel config %s for timeslot %d\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_TRX(trx, DNM, LOGL_ERROR, "unsupported channel config %s for timeslot %d\n",</span><br><span> gsm_pchan_name(ts->pchan_from_config), i);</span><br><span> return 0;</span><br><span> }</span><br><span>@@ -1000,17 +998,17 @@</span><br><span> build the configuration data</span><br><span> */</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-static int make_bts_config(uint8_t bts_type, int n_trx, uint8_t * fu_config,</span><br><span style="color: hsl(120, 100%, 40%);">+static int make_bts_config(struct gsm_bts *bts, uint8_t bts_type, int n_trx, uint8_t * fu_config,</span><br><span> int need_hopping)</span><br><span> {</span><br><span> /* is it an InSite BTS ? */</span><br><span> if (bts_type == 0x0E || bts_type == 0x0F || bts_type == 0x10) { /* TODO */</span><br><span> if (n_trx != 1) {</span><br><span style="color: hsl(0, 100%, 40%);">- fprintf(stderr, "InSite has only one TRX\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_BTS(bts, DNM, LOGL_ERROR, "InSite has only one TRX\n");</span><br><span> return 0;</span><br><span> }</span><br><span> if (need_hopping != 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- fprintf(stderr, "InSite does not support hopping\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_BTS(bts, DNM, LOGL_ERROR, "InSite does not support hopping\n");</span><br><span> return 0;</span><br><span> }</span><br><span> memcpy(fu_config, bts_config_insite, sizeof(bts_config_insite));</span><br><span>@@ -1090,7 +1088,7 @@</span><br><span> noh->reference = htons(ref);</span><br><span> memcpy(noh->data, data, len_data);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- DEBUGPC(DNM, "Sending %s\n", get_msg_type_name_string(msg_type));</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_BTS(bts, DNM, LOGL_DEBUG, "Sending %s\n", get_msg_type_name_string(msg_type));</span><br><span> </span><br><span> return abis_nm_sendmsg(bts, msg);</span><br><span> }</span><br><span>@@ -1165,7 +1163,7 @@</span><br><span> {</span><br><span> uint8_t *data = reset;</span><br><span> int len_data = sizeof(reset);</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DLINP, LOGL_INFO, "Nokia BTS reset timer: %d\n", bts->nokia.bts_reset_timer_cnf);</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_BTS(bts, DLINP, LOGL_INFO, "Nokia BTS reset timer: %d\n", bts->nokia.bts_reset_timer_cnf);</span><br><span> return abis_nm_send(bts, NOKIA_MSG_RESET_REQ, ref, data, len_data);</span><br><span> }</span><br><span> </span><br><span>@@ -1252,7 +1250,7 @@</span><br><span> memcpy(oh->data, data, len_to_send);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- DEBUGPC(DNM, "Sending multi-segment %d\n", seq);</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_BTS(bts, DNM, LOGL_DEBUG, "Sending multi-segment %d\n", seq);</span><br><span> </span><br><span> ret = abis_nm_sendmsg(bts, msg);</span><br><span> if (ret < 0)</span><br><span>@@ -1306,7 +1304,7 @@</span><br><span> idx++;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- ret = make_bts_config(bts_type, idx, config + len, need_hopping);</span><br><span style="color: hsl(120, 100%, 40%);">+ ret = make_bts_config(bts, bts_type, idx, config + len, need_hopping);</span><br><span> len += ret;</span><br><span> </span><br><span> #if 0 /* debugging */</span><br><span>@@ -1502,8 +1500,8 @@</span><br><span> /* OML link */</span><br><span> line = e1inp_line_find(e1_link->e1_nr);</span><br><span> if (!line) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DLINP, LOGL_ERROR, "BTS %u OML link referring to non-existing E1 line %u\n",</span><br><span style="color: hsl(0, 100%, 40%);">- bts->nr, e1_link->e1_nr);</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_BTS(bts, DLINP, LOGL_ERROR, "BTS OML link referring to non-existing E1 line %u\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ e1_link->e1_nr);</span><br><span> return;</span><br><span> }</span><br><span> </span><br><span>@@ -1555,18 +1553,17 @@</span><br><span> int len_data;</span><br><span> </span><br><span> if (bts->nokia.wait_reset) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DNM, LOGL_INFO,</span><br><span style="color: hsl(0, 100%, 40%);">- "Ignore message while waiting for reset\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_BTS(bts, DNM, LOGL_INFO, "Ignoring message while waiting for reset: %s\n", msgb_hexdump(mb));</span><br><span> return ret;</span><br><span> }</span><br><span> </span><br><span> if (oh->length < sizeof(struct abis_om_nokia_hdr)) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DNM, LOGL_ERROR, "Message too short\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_BTS(bts, DNM, LOGL_ERROR, "Message too short: %s\n", msgb_hexdump(mb));</span><br><span> return -EINVAL;</span><br><span> }</span><br><span> </span><br><span> len_data = oh->length - sizeof(struct abis_om_nokia_hdr);</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DNM, LOGL_INFO, "(0x%02X) %s\n", mt, get_msg_type_name_string(mt));</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_BTS(bts, DNM, LOGL_INFO, "Rx (0x%02X) %s\n", mt, get_msg_type_name_string(mt));</span><br><span> #if 0 /* debugging */</span><br><span> dump_elements(noh->data, len_data);</span><br><span> #endif</span><br><span>@@ -1576,11 +1573,10 @@</span><br><span> if (find_element(noh->data, len_data,</span><br><span> NOKIA_EI_BTS_TYPE, &bts->nokia.bts_type,</span><br><span> sizeof(uint8_t)) == sizeof(uint8_t))</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DNM, LOGL_INFO, "BTS type = %d (%s)\n",</span><br><span style="color: hsl(0, 100%, 40%);">- bts->nokia.bts_type,</span><br><span style="color: hsl(0, 100%, 40%);">- get_bts_type_string(bts->nokia.bts_type));</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_BTS(bts, DNM, LOGL_INFO, "Rx BTS type = %d (%s)\n", bts->nokia.bts_type,</span><br><span style="color: hsl(120, 100%, 40%);">+ get_bts_type_string(bts->nokia.bts_type));</span><br><span> else</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DNM, LOGL_ERROR, "BTS type not found\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_BTS(bts, DNM, LOGL_ERROR, "BTS type not found in NOKIA_MSG_OMU_STARTED\n");</span><br><span> /* send START_DOWNLOAD_REQ */</span><br><span> abis_nm_download_req(bts, ref);</span><br><span> break;</span><br><span>@@ -1598,14 +1594,13 @@</span><br><span> if (find_element</span><br><span> (noh->data, len_data, NOKIA_EI_ACK, &ack,</span><br><span> sizeof(uint8_t)) == sizeof(uint8_t)) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DNM, LOGL_INFO, "ACK = %d\n", ack);</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_BTS(bts, DNM, LOGL_INFO, "Rx ACK = %u\n", ack);</span><br><span> if (ack != 1) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DNM, LOGL_ERROR, "No ACK received (%d)\n",</span><br><span style="color: hsl(0, 100%, 40%);">- ack);</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_BTS(bts, DNM, LOGL_ERROR, "Rx No ACK (%u): don't know how to proceed\n", ack);</span><br><span> /* TODO: properly handle failures (NACK) */</span><br><span> }</span><br><span> } else</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DNM, LOGL_ERROR, "ACK not found\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_BTS(bts, DNM, LOGL_ERROR, "Rx MSG_ACK but no EI_ACK found: %s\n", msgb_hexdump(mb));</span><br><span> </span><br><span> /* TODO: the assumption for the following is that no NACK was received */</span><br><span> </span><br><span>@@ -1647,11 +1642,8 @@</span><br><span> /* RSL Link */</span><br><span> line = e1inp_line_find(e1_link->e1_nr);</span><br><span> if (!line) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DLINP, LOGL_ERROR,</span><br><span style="color: hsl(0, 100%, 40%);">- "TRX (%u/%u) RSL link referring "</span><br><span style="color: hsl(0, 100%, 40%);">- "to non-existing E1 line %u\n",</span><br><span style="color: hsl(0, 100%, 40%);">- sign_link->trx->bts->nr, sign_link->trx->nr,</span><br><span style="color: hsl(0, 100%, 40%);">- e1_link->e1_nr);</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_BTS(bts, DLINP, LOGL_ERROR, "RSL link referring to "</span><br><span style="color: hsl(120, 100%, 40%);">+ "non-existing E1 line %u\n", e1_link->e1_nr);</span><br><span> return -ENOMEM;</span><br><span> }</span><br><span> /* start TRX */</span><br><span>@@ -1681,21 +1673,17 @@</span><br><span> sizeof(info));</span><br><span> if (str_len > 0) {</span><br><span> info[str_len] = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DNM, LOGL_INFO, "ALARM Severity %s (%d) : %s\n",</span><br><span style="color: hsl(0, 100%, 40%);">- get_severity_string(severity), severity, info);</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_BTS(bts, DNM, LOGL_NOTICE, "Rx ALARM Severity %s (%d) : %s\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ get_severity_string(severity), severity, info);</span><br><span> } else { /* nothing found, try details */</span><br><span> str_len =</span><br><span style="color: hsl(0, 100%, 40%);">- find_element(noh->data, len_data,</span><br><span style="color: hsl(0, 100%, 40%);">- NOKIA_EI_ALARM_DETAIL, info,</span><br><span style="color: hsl(0, 100%, 40%);">- sizeof(info));</span><br><span style="color: hsl(120, 100%, 40%);">+ find_element(noh->data, len_data, NOKIA_EI_ALARM_DETAIL, info, sizeof(info));</span><br><span> if (str_len > 0) {</span><br><span> uint16_t code;</span><br><span> info[str_len] = 0;</span><br><span> code = (info[0] << 8) + info[1];</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DNM, LOGL_INFO,</span><br><span style="color: hsl(0, 100%, 40%);">- "ALARM Severity %s (%d), code 0x%X : %s\n",</span><br><span style="color: hsl(0, 100%, 40%);">- get_severity_string(severity), severity,</span><br><span style="color: hsl(0, 100%, 40%);">- code, info + 2);</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_BTS(bts, DNM, LOGL_NOTICE, "Rx ALARM Severity %s (%d), code 0x%X : %s\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ get_severity_string(severity), severity, code, info + 2);</span><br><span> }</span><br><span> }</span><br><span> /* send ACK */</span><br><span>@@ -1712,41 +1700,37 @@</span><br><span> </span><br><span> int abis_nokia_rcvmsg(struct msgb *msg)</span><br><span> {</span><br><span style="color: hsl(120, 100%, 40%);">+ struct e1inp_sign_link *sign_link = (struct e1inp_sign_link *)msg->dst;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm_bts *bts = sign_link->trx->bts;</span><br><span> struct abis_om_hdr *oh = msgb_l2(msg);</span><br><span> int rc = 0;</span><br><span> </span><br><span> /* Various consistency checks */</span><br><span> if (oh->placement != ABIS_OM_PLACEMENT_ONLY) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DNM, LOGL_ERROR, "ABIS OML placement 0x%x not supported\n",</span><br><span style="color: hsl(0, 100%, 40%);">- oh->placement);</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_BTS(bts, DNM, LOGL_ERROR, "Rx ABIS OML placement 0x%x not supported\n", oh->placement);</span><br><span> if (oh->placement != ABIS_OM_PLACEMENT_FIRST)</span><br><span> return -EINVAL;</span><br><span> }</span><br><span> if (oh->sequence != 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DNM, LOGL_ERROR, "ABIS OML sequence 0x%x != 0x00\n",</span><br><span style="color: hsl(0, 100%, 40%);">- oh->sequence);</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_BTS(bts, DNM, LOGL_ERROR, "Rx ABIS OML sequence 0x%x != 0x00\n", oh->sequence);</span><br><span> return -EINVAL;</span><br><span> }</span><br><span> msg->l3h = (unsigned char *)oh + sizeof(*oh);</span><br><span> </span><br><span> switch (oh->mdisc) {</span><br><span> case ABIS_OM_MDISC_FOM:</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DNM, LOGL_INFO, "ABIS_OM_MDISC_FOM\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_BTS(bts, DNM, LOGL_INFO, "Rx ABIS_OM_MDISC_FOM\n");</span><br><span> rc = abis_nm_rcvmsg_fom(msg);</span><br><span> break;</span><br><span> case ABIS_OM_MDISC_MANUF:</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DNM, LOGL_INFO, "ABIS_OM_MDISC_MANUF\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_BTS(bts, DNM, LOGL_INFO, "Rx ABIS_OM_MDISC_MANUF: ignoring\n");</span><br><span> break;</span><br><span> case ABIS_OM_MDISC_MMI:</span><br><span> case ABIS_OM_MDISC_TRAU:</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DNM, LOGL_ERROR,</span><br><span style="color: hsl(0, 100%, 40%);">- "unimplemented ABIS OML message discriminator 0x%x\n",</span><br><span style="color: hsl(0, 100%, 40%);">- oh->mdisc);</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_BTS(bts, DNM, LOGL_ERROR, "Rx unimplemented ABIS OML message discriminator 0x%x\n", oh->mdisc);</span><br><span> break;</span><br><span> default:</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DNM, LOGL_ERROR,</span><br><span style="color: hsl(0, 100%, 40%);">- "unknown ABIS OML message discriminator 0x%x\n",</span><br><span style="color: hsl(0, 100%, 40%);">- oh->mdisc);</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_BTS(bts, DNM, LOGL_ERROR, "Rx unknown ABIS OML message discriminator 0x%x\n", oh->mdisc);</span><br><span> return -EINVAL;</span><br><span> }</span><br><span> </span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-bsc/+/19264">change 19264</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/osmo-bsc/+/19264"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-bsc </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I29ec935669175a08cb42e1666559b681c50a6e72 </div>
<div style="display:none"> Gerrit-Change-Number: 19264 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>