<p>neels <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-msc/+/19024">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, but someone else must approve
pespin: Looks good to me, but someone else must approve
neels: Looks good to me, approved
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">refactor: move RESET Osmux TLV parsing to ran_msg_a.c<br><br>ran_peer.c is not the proper place to parse messages, because it should be RAN<br>agnostic. All parsing and encoding belongs in ran_msg_a.c and ran_msg_iu.c.<br><br>Move the Osmux TLV parsing into the is_reset_msg op: add supports_osmux<br>out-parameter (and add a logging fi pointer). To be able to modify msg->l3h,<br>also make the msgb arg non-const.<br><br>In ranap_is_reset_msg(), always return non-support for Osmux.<br><br>In bssmap_is_reset_msg(), return 0 if no TLVs were parsed, 1/-1 if an Osmux TLV<br>was present/not present.<br><br>Update the osmux support flag directly where the ConnectionLess message is<br>received, so that there is only one place responsible for that.<br><br>Related: OS#4595<br>Change-Id: I1ad4a3f9356216dd4bf8c48fba29fd23438810a7<br>---<br>M include/osmocom/msc/ran_msg_a.h<br>M include/osmocom/msc/ran_msg_iu.h<br>M include/osmocom/msc/sccp_ran.h<br>M src/libmsc/ran_msg_a.c<br>M src/libmsc/ran_msg_iu.c<br>M src/libmsc/ran_peer.c<br>6 files changed, 60 insertions(+), 26 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmocom/msc/ran_msg_a.h b/include/osmocom/msc/ran_msg_a.h</span><br><span>index 3ba081d..2d045b9 100644</span><br><span>--- a/include/osmocom/msc/ran_msg_a.h</span><br><span>+++ b/include/osmocom/msc/ran_msg_a.h</span><br><span>@@ -35,7 +35,8 @@</span><br><span> int ran_a_decode_l2(struct ran_dec *ran_a, struct msgb *bssap);</span><br><span> struct msgb *ran_a_encode(struct osmo_fsm_inst *caller_fi, const struct ran_msg *ran_enc_msg);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-enum reset_msg_type bssmap_is_reset_msg(const struct sccp_ran_inst *sri, const struct msgb *l2);</span><br><span style="color: hsl(120, 100%, 40%);">+enum reset_msg_type bssmap_is_reset_msg(const struct sccp_ran_inst *sri, struct osmo_fsm_inst *log_fi,</span><br><span style="color: hsl(120, 100%, 40%);">+ struct msgb *l2, int *supports_osmux);</span><br><span> struct msgb *bssmap_make_reset_msg(const struct sccp_ran_inst *sri, enum reset_msg_type type);</span><br><span> struct msgb *bssmap_make_paging_msg(const struct sccp_ran_inst *sri, const struct gsm0808_cell_id *page_cell_id,</span><br><span> const char *imsi, uint32_t tmsi, enum paging_cause cause);</span><br><span>diff --git a/include/osmocom/msc/ran_msg_iu.h b/include/osmocom/msc/ran_msg_iu.h</span><br><span>index 316a91c..3f3d61e 100644</span><br><span>--- a/include/osmocom/msc/ran_msg_iu.h</span><br><span>+++ b/include/osmocom/msc/ran_msg_iu.h</span><br><span>@@ -28,7 +28,8 @@</span><br><span> int ran_iu_decode_l2(struct ran_dec *ran_dec_iu, struct msgb *ranap);</span><br><span> struct msgb *ran_iu_encode(struct osmo_fsm_inst *caller_fi, const struct ran_msg *ran_enc_msg);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-enum reset_msg_type ranap_is_reset_msg(const struct sccp_ran_inst *sri, const struct msgb *l2);</span><br><span style="color: hsl(120, 100%, 40%);">+enum reset_msg_type ranap_is_reset_msg(const struct sccp_ran_inst *sri, struct osmo_fsm_inst *log_fi,</span><br><span style="color: hsl(120, 100%, 40%);">+ struct msgb *l2, int *supports_osmux);</span><br><span> struct msgb *ranap_make_reset_msg(const struct sccp_ran_inst *sri, enum reset_msg_type type);</span><br><span> struct msgb *ranap_make_paging_msg(const struct sccp_ran_inst *sri, const struct gsm0808_cell_id *page_cell_id,</span><br><span> const char *imsi, uint32_t tmsi, enum paging_cause cause);</span><br><span>diff --git a/include/osmocom/msc/sccp_ran.h b/include/osmocom/msc/sccp_ran.h</span><br><span>index f84bf61..a4bd4ca 100644</span><br><span>--- a/include/osmocom/msc/sccp_ran.h</span><br><span>+++ b/include/osmocom/msc/sccp_ran.h</span><br><span>@@ -233,8 +233,11 @@</span><br><span> </span><br><span> /* Return whether the given l2_cl message is a RESET, RESET ACKNOWLEDGE, or RESET-unrelated message.</span><br><span> * This callback is stored in struct sccp_ran_inst to provide RESET handling to the caller (ran_peer),</span><br><span style="color: hsl(0, 100%, 40%);">- * it is not used in sccp_ran.c. */</span><br><span style="color: hsl(0, 100%, 40%);">- enum reset_msg_type (* is_reset_msg )(const struct sccp_ran_inst *sri, const struct msgb *l2_cl);</span><br><span style="color: hsl(120, 100%, 40%);">+ * it is not used in sccp_ran.c.</span><br><span style="color: hsl(120, 100%, 40%);">+ * In supports_osmux, return 0 for no information, 1 for support detected, -1 for non-support detected.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+ enum reset_msg_type (* is_reset_msg )(const struct sccp_ran_inst *sri, struct osmo_fsm_inst *log_fi,</span><br><span style="color: hsl(120, 100%, 40%);">+ struct msgb *l2_cl, int *supports_osmux);</span><br><span> </span><br><span> /* Return a RESET or RESET ACK message for this RAN type.</span><br><span> * This callback is stored in struct sccp_ran_inst to provide RESET handling to the caller (ran_peer),</span><br><span>diff --git a/src/libmsc/ran_msg_a.c b/src/libmsc/ran_msg_a.c</span><br><span>index ab58526..1fa8ccb 100644</span><br><span>--- a/src/libmsc/ran_msg_a.c</span><br><span>+++ b/src/libmsc/ran_msg_a.c</span><br><span>@@ -1275,20 +1275,50 @@</span><br><span> return msg;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/* Return 1 for a RESET, 2 for a RESET ACK message, 0 otherwise */</span><br><span style="color: hsl(0, 100%, 40%);">-enum reset_msg_type bssmap_is_reset_msg(const struct sccp_ran_inst *sri, const struct msgb *l2)</span><br><span style="color: hsl(120, 100%, 40%);">+static void cl_parse_osmux(struct osmo_fsm_inst *log_fi, struct msgb *msg, int *supports_osmux)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ struct tlv_parsed tp;</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%);">+ if (supports_osmux == NULL)</span><br><span style="color: hsl(120, 100%, 40%);">+ return;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = tlv_parse(&tp, gsm0808_att_tlvdef(), msgb_l3(msg) + 1, msgb_l3len(msg) - 1, 0, 0);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rc < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPFSMSL(log_fi, DBSSAP, LOGL_ERROR, "BSSMAP: Failed parsing TLV looking for Osmux support\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ return;</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%);">+ if (TLVP_PRESENT(&tp, GSM0808_IE_OSMO_OSMUX_SUPPORT)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ *supports_osmux = true;</span><br><span style="color: hsl(120, 100%, 40%);">+ } else {</span><br><span style="color: hsl(120, 100%, 40%);">+ *supports_osmux = false;</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%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+/* Return 1 for a RESET, 2 for a RESET ACK message, 0 otherwise.</span><br><span style="color: hsl(120, 100%, 40%);">+ * In supports_osmux, return 0 for no information, 1 for support detected, -1 for non-support detected. */</span><br><span style="color: hsl(120, 100%, 40%);">+enum reset_msg_type bssmap_is_reset_msg(const struct sccp_ran_inst *sri, struct osmo_fsm_inst *log_fi,</span><br><span style="color: hsl(120, 100%, 40%);">+ struct msgb *l2, int *supports_osmux)</span><br><span> {</span><br><span> struct bssmap_header *bs = (struct bssmap_header *)msgb_l2(l2);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ if (supports_osmux != NULL)</span><br><span style="color: hsl(120, 100%, 40%);">+ *supports_osmux = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> if (!bs</span><br><span> || msgb_l2len(l2) < (sizeof(*bs) + 1)</span><br><span> || bs->type != BSSAP_MSG_BSS_MANAGEMENT)</span><br><span> return SCCP_RAN_MSG_NON_RESET;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- switch (l2->l2h[sizeof(*bs)]) {</span><br><span style="color: hsl(120, 100%, 40%);">+ l2->l3h = l2->l2h + sizeof(struct bssmap_header);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ switch (l2->l3h[0]) {</span><br><span> case BSS_MAP_MSG_RESET:</span><br><span style="color: hsl(120, 100%, 40%);">+ cl_parse_osmux(log_fi, l2, supports_osmux);</span><br><span> return SCCP_RAN_MSG_RESET;</span><br><span> case BSS_MAP_MSG_RESET_ACKNOWLEDGE:</span><br><span style="color: hsl(120, 100%, 40%);">+ cl_parse_osmux(log_fi, l2, supports_osmux);</span><br><span> return SCCP_RAN_MSG_RESET_ACK;</span><br><span> default:</span><br><span> return SCCP_RAN_MSG_NON_RESET;</span><br><span>diff --git a/src/libmsc/ran_msg_iu.c b/src/libmsc/ran_msg_iu.c</span><br><span>index b17aef8..5d13460 100644</span><br><span>--- a/src/libmsc/ran_msg_iu.c</span><br><span>+++ b/src/libmsc/ran_msg_iu.c</span><br><span>@@ -438,11 +438,15 @@</span><br><span> }</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-enum reset_msg_type ranap_is_reset_msg(const struct sccp_ran_inst *sri, const struct msgb *l2)</span><br><span style="color: hsl(120, 100%, 40%);">+enum reset_msg_type ranap_is_reset_msg(const struct sccp_ran_inst *sri, struct osmo_fsm_inst *log_fi,</span><br><span style="color: hsl(120, 100%, 40%);">+ struct msgb *l2, int *supports_osmux)</span><br><span> {</span><br><span> int ret = SCCP_RAN_MSG_NON_RESET;</span><br><span> int rc;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ if (supports_osmux != NULL)</span><br><span style="color: hsl(120, 100%, 40%);">+ *supports_osmux = -1;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> rc = ranap_cn_rx_cl(ranap_handle_cl, &ret, msgb_l2(l2), msgb_l2len(l2));</span><br><span> if (rc)</span><br><span> return 0;</span><br><span>diff --git a/src/libmsc/ran_peer.c b/src/libmsc/ran_peer.c</span><br><span>index da88873..80ce558 100644</span><br><span>--- a/src/libmsc/ran_peer.c</span><br><span>+++ b/src/libmsc/ran_peer.c</span><br><span>@@ -122,25 +122,19 @@</span><br><span> }</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/* TODO: create an sccp_ran_ops.rx_reset(_ack) to handle this differently on 2g and 3G */</span><br><span style="color: hsl(0, 100%, 40%);">-/* We expect RAN peer to provide use with an Osmocom extension TLV in BSSMAP_RESET to</span><br><span style="color: hsl(0, 100%, 40%);">- * announce Osmux support */</span><br><span style="color: hsl(0, 100%, 40%);">-static void ran_peer_update_osmux_support(struct ran_peer *rp, struct msgb *msg)</span><br><span style="color: hsl(120, 100%, 40%);">+static void ran_peer_update_osmux_support(struct ran_peer *rp, int supports_osmux)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- struct tlv_parsed tp;</span><br><span style="color: hsl(0, 100%, 40%);">- int rc;</span><br><span> bool old_value = rp->remote_supports_osmux;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- OSMO_ASSERT(msg);</span><br><span style="color: hsl(0, 100%, 40%);">- msg->l3h = msg->l2h + sizeof(struct bssmap_header);</span><br><span style="color: hsl(0, 100%, 40%);">- rc = tlv_parse(&tp, gsm0808_att_tlvdef(), msg->l3h + 1, msgb_l3len(msg) - 1, 0, 0);</span><br><span style="color: hsl(0, 100%, 40%);">- if (rc < 0)</span><br><span style="color: hsl(0, 100%, 40%);">- LOG_RAN_PEER(rp, LOGL_NOTICE, "Failed parsing TLV looking for Osmux support\n");</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (TLVP_PRESENT(&tp, GSM0808_IE_OSMO_OSMUX_SUPPORT)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ switch (supports_osmux) {</span><br><span style="color: hsl(120, 100%, 40%);">+ case 1:</span><br><span> rp->remote_supports_osmux = true;</span><br><span style="color: hsl(0, 100%, 40%);">- } else {</span><br><span style="color: hsl(120, 100%, 40%);">+ break;</span><br><span style="color: hsl(120, 100%, 40%);">+ case -1:</span><br><span> rp->remote_supports_osmux = false;</span><br><span style="color: hsl(120, 100%, 40%);">+ break;</span><br><span style="color: hsl(120, 100%, 40%);">+ default:</span><br><span style="color: hsl(120, 100%, 40%);">+ return;</span><br><span> }</span><br><span> </span><br><span> if (old_value != rp->remote_supports_osmux)</span><br><span>@@ -155,8 +149,6 @@</span><br><span> </span><br><span> ran_peer_discard_all_conns(rp);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- ran_peer_update_osmux_support(rp, msg);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> reset_ack = rp->sri->ran->sccp_ran_ops.make_reset_msg(rp->sri, SCCP_RAN_MSG_RESET_ACK);</span><br><span> </span><br><span> if (!reset_ack) {</span><br><span>@@ -183,7 +175,6 @@</span><br><span> static void ran_peer_rx_reset_ack(struct ran_peer *rp, struct msgb* msg)</span><br><span> {</span><br><span> ran_peer_state_chg(rp, RAN_PEER_ST_READY);</span><br><span style="color: hsl(0, 100%, 40%);">- ran_peer_update_osmux_support(rp, msg);</span><br><span> }</span><br><span> </span><br><span> void ran_peer_reset(struct ran_peer *rp)</span><br><span>@@ -213,10 +204,14 @@</span><br><span> struct ran_peer *rp = fi->priv;</span><br><span> struct ran_peer_ev_ctx *ctx = data;</span><br><span> struct msgb *msg = ctx->msg;</span><br><span style="color: hsl(120, 100%, 40%);">+ int rc;</span><br><span style="color: hsl(120, 100%, 40%);">+ int supports_osmux;</span><br><span> </span><br><span> switch (event) {</span><br><span> case RAN_PEER_EV_MSG_UP_CL:</span><br><span style="color: hsl(0, 100%, 40%);">- switch (rp->sri->ran->sccp_ran_ops.is_reset_msg(rp->sri, msg)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = rp->sri->ran->sccp_ran_ops.is_reset_msg(rp->sri, fi, msg, &supports_osmux);</span><br><span style="color: hsl(120, 100%, 40%);">+ ran_peer_update_osmux_support(rp, supports_osmux);</span><br><span style="color: hsl(120, 100%, 40%);">+ switch (rc) {</span><br><span> case 1:</span><br><span> osmo_fsm_inst_dispatch(fi, RAN_PEER_EV_RX_RESET, msg);</span><br><span> return;</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-msc/+/19024">change 19024</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-msc/+/19024"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-msc </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I1ad4a3f9356216dd4bf8c48fba29fd23438810a7 </div>
<div style="display:none"> Gerrit-Change-Number: 19024 </div>
<div style="display:none"> Gerrit-PatchSet: 4 </div>
<div style="display:none"> Gerrit-Owner: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>