Change in osmo-msc[master]: refactor: move RESET Osmux TLV parsing to ran_msg_a.c

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

neels gerrit-no-reply at lists.osmocom.org
Fri Jun 26 13:32:05 UTC 2020


neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-msc/+/19024 )


Change subject: refactor: move RESET Osmux TLV parsing to ran_msg_a.c
......................................................................

refactor: move RESET Osmux TLV parsing to ran_msg_a.c

ran_peer.c is not the proper place to parse messages, because it should be RAN
agnostic. All parsing and encoding belongs in ran_msg_a.c and ran_msg_iu.c.

Move the Osmux TLV parsing into the is_reset_msg op: add supports_osmux
out-parameter (and add a logging fi pointer). To be able to modify msg->l3h,
also make the msgb arg non-const.

In ranap_is_reset_msg(), always return non-support for Osmux.

In bssmap_is_reset_msg(), return 0 if no TLVs were parsed, 1/-1 if an Osmux TLV
was present/not present.

Update the osmux support flag directly where the ConnectionLess message is
received, so that there is only one place responsible for that.

Related: OS#4595
Change-Id: I1ad4a3f9356216dd4bf8c48fba29fd23438810a7
---
M include/osmocom/msc/ran_msg_a.h
M include/osmocom/msc/ran_msg_iu.h
M include/osmocom/msc/sccp_ran.h
M src/libmsc/ran_msg_a.c
M src/libmsc/ran_msg_iu.c
M src/libmsc/ran_peer.c
6 files changed, 59 insertions(+), 26 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/24/19024/1

diff --git a/include/osmocom/msc/ran_msg_a.h b/include/osmocom/msc/ran_msg_a.h
index 3ba081d..2d045b9 100644
--- a/include/osmocom/msc/ran_msg_a.h
+++ b/include/osmocom/msc/ran_msg_a.h
@@ -35,7 +35,8 @@
 int ran_a_decode_l2(struct ran_dec *ran_a, struct msgb *bssap);
 struct msgb *ran_a_encode(struct osmo_fsm_inst *caller_fi, const struct ran_msg *ran_enc_msg);
 
-enum reset_msg_type bssmap_is_reset_msg(const struct sccp_ran_inst *sri, const struct msgb *l2);
+enum reset_msg_type bssmap_is_reset_msg(const struct sccp_ran_inst *sri, struct osmo_fsm_inst *log_fi,
+					struct msgb *l2, int *supports_osmux);
 struct msgb *bssmap_make_reset_msg(const struct sccp_ran_inst *sri, enum reset_msg_type type);
 struct msgb *bssmap_make_paging_msg(const struct sccp_ran_inst *sri, const struct gsm0808_cell_id *page_cell_id,
 				    const char *imsi, uint32_t tmsi, enum paging_cause cause);
diff --git a/include/osmocom/msc/ran_msg_iu.h b/include/osmocom/msc/ran_msg_iu.h
index 316a91c..3f3d61e 100644
--- a/include/osmocom/msc/ran_msg_iu.h
+++ b/include/osmocom/msc/ran_msg_iu.h
@@ -28,7 +28,8 @@
 int ran_iu_decode_l2(struct ran_dec *ran_dec_iu, struct msgb *ranap);
 struct msgb *ran_iu_encode(struct osmo_fsm_inst *caller_fi, const struct ran_msg *ran_enc_msg);
 
-enum reset_msg_type ranap_is_reset_msg(const struct sccp_ran_inst *sri, const struct msgb *l2);
+enum reset_msg_type ranap_is_reset_msg(const struct sccp_ran_inst *sri, struct osmo_fsm_inst *log_fi,
+				       struct msgb *l2, int *supports_osmux);
 struct msgb *ranap_make_reset_msg(const struct sccp_ran_inst *sri, enum reset_msg_type type);
 struct msgb *ranap_make_paging_msg(const struct sccp_ran_inst *sri, const struct gsm0808_cell_id *page_cell_id,
 				   const char *imsi, uint32_t tmsi, enum paging_cause cause);
diff --git a/include/osmocom/msc/sccp_ran.h b/include/osmocom/msc/sccp_ran.h
index f84bf61..a4bd4ca 100644
--- a/include/osmocom/msc/sccp_ran.h
+++ b/include/osmocom/msc/sccp_ran.h
@@ -233,8 +233,11 @@
 
 	/* Return whether the given l2_cl message is a RESET, RESET ACKNOWLEDGE, or RESET-unrelated message.
 	 * This callback is stored in struct sccp_ran_inst to provide RESET handling to the caller (ran_peer),
-	 * it is not used in sccp_ran.c. */
-	enum reset_msg_type (* is_reset_msg )(const struct sccp_ran_inst *sri, const struct msgb *l2_cl);
+	 * it is not used in sccp_ran.c.
+	 * In supports_osmux, return 0 for no information, 1 for support detected, -1 for non-support detected.
+	 */
+	enum reset_msg_type (* is_reset_msg )(const struct sccp_ran_inst *sri, struct osmo_fsm_inst *log_fi,
+					      struct msgb *l2_cl, int *supports_osmux);
 
 	/* Return a RESET or RESET ACK message for this RAN type.
 	 * This callback is stored in struct sccp_ran_inst to provide RESET handling to the caller (ran_peer),
diff --git a/src/libmsc/ran_msg_a.c b/src/libmsc/ran_msg_a.c
index ab58526..937e283 100644
--- a/src/libmsc/ran_msg_a.c
+++ b/src/libmsc/ran_msg_a.c
@@ -1254,7 +1254,6 @@
 struct msgb *ran_a_encode(struct osmo_fsm_inst *caller_fi, const struct ran_msg *ran_enc_msg)
 {
 	struct msgb *msg = _ran_a_encode(caller_fi, ran_enc_msg);
-
 	if (!msg)
 		return NULL;
 
@@ -1275,20 +1274,50 @@
 	return msg;
 }
 
-/* Return 1 for a RESET, 2 for a RESET ACK message, 0 otherwise */
-enum reset_msg_type bssmap_is_reset_msg(const struct sccp_ran_inst *sri, const struct msgb *l2)
+static void cl_parse_osmux(struct osmo_fsm_inst *log_fi, struct msgb *msg, int *supports_osmux)
+{
+	struct tlv_parsed tp;
+	int rc;
+
+	if (supports_osmux == NULL)
+		return;
+
+	rc = tlv_parse(&tp, gsm0808_att_tlvdef(), msgb_l3(msg) + 1, msgb_l3len(msg) - 1, 0, 0);
+	if (rc < 0) {
+		LOGPFSMSL(log_fi, DBSSAP, LOGL_ERROR, "BSSMAP: Failed parsing TLV looking for Osmux support\n");
+		return;
+	}
+
+	if (TLVP_PRESENT(&tp, GSM0808_IE_OSMO_OSMUX_SUPPORT)) {
+		*supports_osmux = true;
+	} else {
+		*supports_osmux = false;
+	}
+}
+
+/* Return 1 for a RESET, 2 for a RESET ACK message, 0 otherwise.
+ * In supports_osmux, return 0 for no information, 1 for support detected, -1 for non-support detected. */
+enum reset_msg_type bssmap_is_reset_msg(const struct sccp_ran_inst *sri, struct osmo_fsm_inst *log_fi,
+					struct msgb *l2, int *supports_osmux)
 {
 	struct bssmap_header *bs = (struct bssmap_header *)msgb_l2(l2);
 
+	if (supports_osmux)
+		*supports_osmux = 0;
+
 	if (!bs
 	    || msgb_l2len(l2) < (sizeof(*bs) + 1)
 	    || bs->type != BSSAP_MSG_BSS_MANAGEMENT)
 		return SCCP_RAN_MSG_NON_RESET;
 
+	l2->l3h = msgb_l2(l2) + sizeof(struct bssmap_header);
+
 	switch (l2->l2h[sizeof(*bs)]) {
 	case BSS_MAP_MSG_RESET:
+		cl_parse_osmux(log_fi, l2, supports_osmux);
 		return SCCP_RAN_MSG_RESET;
 	case BSS_MAP_MSG_RESET_ACKNOWLEDGE:
+		cl_parse_osmux(log_fi, l2, supports_osmux);
 		return SCCP_RAN_MSG_RESET_ACK;
 	default:
 		return SCCP_RAN_MSG_NON_RESET;
diff --git a/src/libmsc/ran_msg_iu.c b/src/libmsc/ran_msg_iu.c
index b17aef8..5d13460 100644
--- a/src/libmsc/ran_msg_iu.c
+++ b/src/libmsc/ran_msg_iu.c
@@ -438,11 +438,15 @@
 	}
 }
 
-enum reset_msg_type ranap_is_reset_msg(const struct sccp_ran_inst *sri, const struct msgb *l2)
+enum reset_msg_type ranap_is_reset_msg(const struct sccp_ran_inst *sri, struct osmo_fsm_inst *log_fi,
+				       struct msgb *l2, int *supports_osmux)
 {
 	int ret = SCCP_RAN_MSG_NON_RESET;
 	int rc;
 
+	if (supports_osmux != NULL)
+		*supports_osmux = -1;
+
 	rc = ranap_cn_rx_cl(ranap_handle_cl, &ret, msgb_l2(l2), msgb_l2len(l2));
 	if (rc)
 		return 0;
diff --git a/src/libmsc/ran_peer.c b/src/libmsc/ran_peer.c
index da88873..80ce558 100644
--- a/src/libmsc/ran_peer.c
+++ b/src/libmsc/ran_peer.c
@@ -122,25 +122,19 @@
 	}
 }
 
-/* TODO: create an sccp_ran_ops.rx_reset(_ack) to handle this differently on 2g and 3G */
-/* We expect RAN peer to provide use with an Osmocom extension TLV in BSSMAP_RESET to
- * announce Osmux support */
-static void ran_peer_update_osmux_support(struct ran_peer *rp, struct msgb *msg)
+static void ran_peer_update_osmux_support(struct ran_peer *rp, int supports_osmux)
 {
-	struct tlv_parsed tp;
-	int rc;
 	bool old_value = rp->remote_supports_osmux;
 
-	OSMO_ASSERT(msg);
-	msg->l3h = msg->l2h + sizeof(struct bssmap_header);
-	rc = tlv_parse(&tp, gsm0808_att_tlvdef(), msg->l3h + 1, msgb_l3len(msg) - 1, 0, 0);
-	if (rc < 0)
-		LOG_RAN_PEER(rp, LOGL_NOTICE, "Failed parsing TLV looking for Osmux support\n");
-
-	if (TLVP_PRESENT(&tp, GSM0808_IE_OSMO_OSMUX_SUPPORT)) {
+	switch (supports_osmux) {
+	case 1:
 		rp->remote_supports_osmux = true;
-	} else {
+		break;
+	case -1:
 		rp->remote_supports_osmux = false;
+		break;
+	default:
+		return;
 	}
 
 	if (old_value != rp->remote_supports_osmux)
@@ -155,8 +149,6 @@
 
 	ran_peer_discard_all_conns(rp);
 
-	ran_peer_update_osmux_support(rp, msg);
-
 	reset_ack = rp->sri->ran->sccp_ran_ops.make_reset_msg(rp->sri, SCCP_RAN_MSG_RESET_ACK);
 
 	if (!reset_ack) {
@@ -183,7 +175,6 @@
 static void ran_peer_rx_reset_ack(struct ran_peer *rp, struct msgb* msg)
 {
 	ran_peer_state_chg(rp, RAN_PEER_ST_READY);
-	ran_peer_update_osmux_support(rp, msg);
 }
 
 void ran_peer_reset(struct ran_peer *rp)
@@ -213,10 +204,14 @@
 	struct ran_peer *rp = fi->priv;
 	struct ran_peer_ev_ctx *ctx = data;
 	struct msgb *msg = ctx->msg;
+	int rc;
+	int supports_osmux;
 
 	switch (event) {
 	case RAN_PEER_EV_MSG_UP_CL:
-		switch (rp->sri->ran->sccp_ran_ops.is_reset_msg(rp->sri, msg)) {
+		rc = rp->sri->ran->sccp_ran_ops.is_reset_msg(rp->sri, fi, msg, &supports_osmux);
+		ran_peer_update_osmux_support(rp, supports_osmux);
+		switch (rc) {
 		case 1:
 			osmo_fsm_inst_dispatch(fi, RAN_PEER_EV_RX_RESET, msg);
 			return;

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/19024
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I1ad4a3f9356216dd4bf8c48fba29fd23438810a7
Gerrit-Change-Number: 19024
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr at sysmocom.de>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200626/4c2d8b36/attachment.htm>


More information about the gerrit-log mailing list