<p>pespin <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-pcu/+/24169">View Change</a></p><div style="white-space:pre-wrap">Approvals:
pespin: Looks good to me, approved
osmith: Looks good to me, approved
Jenkins Builder: Verified
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">RIM: Refactor Rx path to decode stack in proper order<br><br>Previous implementation of the Rx path was first checking the APP ID<br>before checking the lower layer (container type), which was confusing<br>because the information is then not verified in ascending order in the<br>protocol stack.<br><br>Let's instead, first, pass the pdu to the correct container type<br>handler, and only once there, let each container type handler verify the<br>available applications.<br><br>Change-Id: Ibe017c1a6e789f45d74c4a5f5f4608298c8c9f91<br>---<br>M src/gprs_bssgp_rim.c<br>1 file changed, 47 insertions(+), 63 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/gprs_bssgp_rim.c b/src/gprs_bssgp_rim.c</span><br><span>index e9b9ef8..c19ed81 100644</span><br><span>--- a/src/gprs_bssgp_rim.c</span><br><span>+++ b/src/gprs_bssgp_rim.c</span><br><span>@@ -136,34 +136,6 @@</span><br><span> resp_pdu->rim_cont_iei = BSSGP_IE_RI_ERROR_RIM_COINTAINER;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/* Check if the application ID in the request PDU is actually BSSGP_RAN_INF_APP_ID_NACC */</span><br><span style="color: hsl(0, 100%, 40%);">-static const enum bssgp_ran_inf_app_id *get_app_id(const struct bssgp_ran_information_pdu *pdu)</span><br><span style="color: hsl(0, 100%, 40%);">-{</span><br><span style="color: hsl(0, 100%, 40%);">- switch (pdu->rim_cont_iei) {</span><br><span style="color: hsl(0, 100%, 40%);">- case BSSGP_IE_RI_REQ_RIM_CONTAINER:</span><br><span style="color: hsl(0, 100%, 40%);">- return &pdu->decoded.req_rim_cont.app_id;</span><br><span style="color: hsl(0, 100%, 40%);">- case BSSGP_IE_RI_RIM_CONTAINER:</span><br><span style="color: hsl(0, 100%, 40%);">- return &pdu->decoded.rim_cont.app_id;</span><br><span style="color: hsl(0, 100%, 40%);">- case BSSGP_IE_RI_APP_ERROR_RIM_CONT:</span><br><span style="color: hsl(0, 100%, 40%);">- return &pdu->decoded.app_err_rim_cont.app_id;</span><br><span style="color: hsl(0, 100%, 40%);">- case BSSGP_IE_RI_ACK_RIM_CONTAINER:</span><br><span style="color: hsl(0, 100%, 40%);">- return &pdu->decoded.ack_rim_cont.app_id;</span><br><span style="color: hsl(0, 100%, 40%);">- case BSSGP_IE_RI_ERROR_RIM_COINTAINER:</span><br><span style="color: hsl(0, 100%, 40%);">- return &pdu->decoded.err_rim_cont.app_id;</span><br><span style="color: hsl(0, 100%, 40%);">- default:</span><br><span style="color: hsl(0, 100%, 40%);">- return NULL;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-}</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-/* Check if the application ID in the request PDU is of a certain type */</span><br><span style="color: hsl(0, 100%, 40%);">-static bool match_app_id(const struct bssgp_ran_information_pdu *pdu, enum bssgp_ran_inf_app_id exp_app_id)</span><br><span style="color: hsl(0, 100%, 40%);">-{</span><br><span style="color: hsl(0, 100%, 40%);">- const enum bssgp_ran_inf_app_id *app_id = get_app_id(pdu);</span><br><span style="color: hsl(0, 100%, 40%);">- if (app_id && *app_id == exp_app_id)</span><br><span style="color: hsl(0, 100%, 40%);">- return true;</span><br><span style="color: hsl(0, 100%, 40%);">- return false;</span><br><span style="color: hsl(0, 100%, 40%);">-}</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> static int handle_ran_info_response_nacc(const struct bssgp_ran_inf_app_cont_nacc *nacc, struct gprs_rlcmac_bts *bts)</span><br><span> {</span><br><span> struct si_cache_value val;</span><br><span>@@ -197,11 +169,56 @@</span><br><span> return 0;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+static int handle_ran_info_request(const struct bssgp_ran_information_pdu *pdu,</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gprs_rlcmac_bts *bts, uint16_t nsei)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct bssgp_ran_inf_req_rim_cont *ri_req = &pdu->decoded.req_rim_cont;</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct bssgp_ran_inf_req_app_cont_nacc *nacc_req;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct bssgp_ran_information_pdu resp_pdu;</span><br><span style="color: hsl(120, 100%, 40%);">+ int rc;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Check if we support the application ID */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ri_req->app_id != BSSGP_RAN_INF_APP_ID_NACC) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPRIM(nsei, LOGL_ERROR,</span><br><span style="color: hsl(120, 100%, 40%);">+ "Rx RAN-INFO-REQ for cell %s with unknown/wrong application ID %s -- reject.\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_cgi_ps_name(&bts->cgi_ps), bssgp_ran_inf_app_id_str(ri_req->app_id));</span><br><span style="color: hsl(120, 100%, 40%);">+ format_response_pdu_err(&resp_pdu, pdu);</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = -ENOTSUP;</span><br><span style="color: hsl(120, 100%, 40%);">+ goto tx_ret;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ nacc_req = &ri_req->u.app_cont_nacc;</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = osmo_cgi_ps_cmp(&bts->cgi_ps, &nacc_req->reprt_cell);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rc != 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPRIM(nsei, LOGL_ERROR, "reporting cell in RIM application container %s "</span><br><span style="color: hsl(120, 100%, 40%);">+ "does not match destination cell in RIM routing info %s -- rejected.\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_cgi_ps_name(&nacc_req->reprt_cell),</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_cgi_ps_name2(&bts->cgi_ps));</span><br><span style="color: hsl(120, 100%, 40%);">+ format_response_pdu_err(&resp_pdu, pdu);</span><br><span style="color: hsl(120, 100%, 40%);">+ } else {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPRIM(nsei, LOGL_INFO, "Responding to RAN INFORMATION REQUEST %s ...\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_cgi_ps_name(&nacc_req->reprt_cell));</span><br><span style="color: hsl(120, 100%, 40%);">+ format_response_pdu(&resp_pdu, pdu, bts);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+tx_ret:</span><br><span style="color: hsl(120, 100%, 40%);">+ bssgp_tx_rim(&resp_pdu, nsei);</span><br><span style="color: hsl(120, 100%, 40%);">+ return rc;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> static int handle_ran_info_response(const struct bssgp_ran_information_pdu *pdu, struct gprs_rlcmac_bts *bts)</span><br><span> {</span><br><span> const struct bssgp_ran_inf_rim_cont *ran_info = &pdu->decoded.rim_cont;</span><br><span> char ri_src_str[64];</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ /* Check if we support the application ID */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ran_info->app_id != BSSGP_RAN_INF_APP_ID_NACC) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DRIM, LOGL_ERROR,</span><br><span style="color: hsl(120, 100%, 40%);">+ "Rx RAN-INFO for cell %s with unknown/wrong application ID %s received -- reject.\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_cgi_ps_name(&bts->cgi_ps), bssgp_ran_inf_app_id_str(ran_info->app_id));</span><br><span style="color: hsl(120, 100%, 40%);">+ return -1;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> if (ran_info->app_err) {</span><br><span> LOGP(DRIM, LOGL_ERROR,</span><br><span> "%s Rx RAN-INFO with an app error! cause: %s\n",</span><br><span>@@ -210,16 +227,7 @@</span><br><span> return -1;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- switch (pdu->decoded.rim_cont.app_id) {</span><br><span style="color: hsl(0, 100%, 40%);">- case BSSGP_RAN_INF_APP_ID_NACC:</span><br><span style="color: hsl(0, 100%, 40%);">- handle_ran_info_response_nacc(&ran_info->u.app_cont_nacc, bts);</span><br><span style="color: hsl(0, 100%, 40%);">- break;</span><br><span style="color: hsl(0, 100%, 40%);">- default:</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DRIM, LOGL_ERROR, "%s Rx RAN-INFO with unknown/wrong application ID %s received\n",</span><br><span style="color: hsl(0, 100%, 40%);">- bssgp_rim_ri_name_buf(ri_src_str, sizeof(ri_src_str), &pdu->routing_info_src),</span><br><span style="color: hsl(0, 100%, 40%);">- bssgp_ran_inf_app_id_str(pdu->decoded.rim_cont.app_id));</span><br><span style="color: hsl(0, 100%, 40%);">- return -1;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(120, 100%, 40%);">+ handle_ran_info_response_nacc(&ran_info->u.app_cont_nacc, bts);</span><br><span> return 0;</span><br><span> }</span><br><span> </span><br><span>@@ -231,7 +239,6 @@</span><br><span> struct bssgp_ran_information_pdu resp_pdu;</span><br><span> struct osmo_cell_global_id_ps dst_addr;</span><br><span> struct gprs_rlcmac_bts *bts;</span><br><span style="color: hsl(0, 100%, 40%);">- int rc;</span><br><span> </span><br><span> OSMO_ASSERT (bp->oph.sap == SAP_BSSGP_RIM);</span><br><span> </span><br><span>@@ -265,33 +272,10 @@</span><br><span> return 0;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* Check if the RIM container inside the incoming RIM PDU has the correct</span><br><span style="color: hsl(0, 100%, 40%);">- * application ID */</span><br><span style="color: hsl(0, 100%, 40%);">- if (!match_app_id(pdu, BSSGP_RAN_INF_APP_ID_NACC)) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGPRIM(nsei, LOGL_ERROR,</span><br><span style="color: hsl(0, 100%, 40%);">- "RIM PDU for cell %s with unknown/wrong application ID received -- reject.\n",</span><br><span style="color: hsl(0, 100%, 40%);">- osmo_cgi_ps_name(&dst_addr));</span><br><span style="color: hsl(0, 100%, 40%);">- format_response_pdu_err(&resp_pdu, pdu);</span><br><span style="color: hsl(0, 100%, 40%);">- return 0;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> /* Handle incoming RIM container */</span><br><span> switch (pdu->rim_cont_iei) {</span><br><span> case BSSGP_IE_RI_REQ_RIM_CONTAINER:</span><br><span style="color: hsl(0, 100%, 40%);">- rc = osmo_cgi_ps_cmp(&dst_addr, &pdu->decoded.req_rim_cont.u.app_cont_nacc.reprt_cell);</span><br><span style="color: hsl(0, 100%, 40%);">- if (rc != 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGPRIM(nsei, LOGL_ERROR, "reporting cell in RIM application container %s "</span><br><span style="color: hsl(0, 100%, 40%);">- "does not match destination cell in RIM routing info %s -- rejected.\n",</span><br><span style="color: hsl(0, 100%, 40%);">- osmo_cgi_ps_name(&pdu->decoded.req_rim_cont.u.app_cont_nacc.reprt_cell),</span><br><span style="color: hsl(0, 100%, 40%);">- osmo_cgi_ps_name2(&dst_addr));</span><br><span style="color: hsl(0, 100%, 40%);">- format_response_pdu_err(&resp_pdu, pdu);</span><br><span style="color: hsl(0, 100%, 40%);">- } else {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGPRIM(nsei, LOGL_INFO, "Responding to RAN INFORMATION REQUEST %s ...\n",</span><br><span style="color: hsl(0, 100%, 40%);">- osmo_cgi_ps_name(&pdu->decoded.req_rim_cont.u.app_cont_nacc.reprt_cell));</span><br><span style="color: hsl(0, 100%, 40%);">- format_response_pdu(&resp_pdu, pdu, bts);</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- bssgp_tx_rim(&resp_pdu, nsei);</span><br><span style="color: hsl(0, 100%, 40%);">- break;</span><br><span style="color: hsl(120, 100%, 40%);">+ return handle_ran_info_request(pdu, bts, nsei);</span><br><span> case BSSGP_IE_RI_RIM_CONTAINER:</span><br><span> return handle_ran_info_response(pdu, bts);</span><br><span> case BSSGP_IE_RI_APP_ERROR_RIM_CONT:</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-pcu/+/24169">change 24169</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-pcu/+/24169"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-pcu </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Ibe017c1a6e789f45d74c4a5f5f4608298c8c9f91 </div>
<div style="display:none"> Gerrit-Change-Number: 24169 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: dexter <pmaier@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: osmith <osmith@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>