Change in osmo-hlr[master]: move creation of insert subscriber data messages to a common function

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/.

Stefan Sperling gerrit-no-reply at lists.osmocom.org
Fri May 18 12:50:46 UTC 2018


Stefan Sperling has submitted this change and it was merged. ( https://gerrit.osmocom.org/7992 )

Change subject: move creation of insert subscriber data messages to a common function
......................................................................

move creation of insert subscriber data messages to a common function

Move code to create an Insert Subscriber Data message into a common
function which can be shared by hlr.c and luop.c.

As a consequence, we always encode gsup.cn_domain in the corresponding
msgb and must adjust expected output of the 'gsup' test accordingly.

Change-Id: I6a92ca34cdaadca9eacc774bb1ca386c325ba865
Requested-by: neels
Related: OS#2785
---
M src/gsup_server.c
M src/gsup_server.h
M src/hlr.c
M src/luop.c
M tests/gsup/gsup_test.err
5 files changed, 103 insertions(+), 67 deletions(-)

Approvals:
  Jenkins Builder: Verified
  Neels Hofmeyr: Looks good to me, approved



diff --git a/src/gsup_server.c b/src/gsup_server.c
index 07d4feb..4b8a0fa 100644
--- a/src/gsup_server.c
+++ b/src/gsup_server.c
@@ -24,6 +24,7 @@
 #include <osmocom/core/linuxlist.h>
 #include <osmocom/abis/ipa.h>
 #include <osmocom/abis/ipaccess.h>
+#include <osmocom/gsm/gsm48_ie.h>
 #include <osmocom/gsm/apn.h>
 
 #include "gsup_server.h"
@@ -357,3 +358,56 @@
 
 	return 0;
 }
+
+/**
+ * Populate a gsup message structure with an Insert Subscriber Data Message.
+ * All required memory buffers for data pointed to by pointers in struct omso_gsup_message
+ * must be allocated by the caller and should have the same lifetime as the gsup parameter.
+ *
+ * \param[out] gsup  The gsup message to populate.
+ * \param[in] imsi  The subscriber's IMSI.
+ * \param[in] msisdn The subscriber's MSISDN.
+ * \param[out] msisdn_enc A buffer large enough to store the MSISDN in encoded form.
+ * \param[in] msisdn_enc_size Size of the buffer (must be >= OSMO_GSUP_MAX_CALLED_PARTY_BCD_LEN).
+ * \param[out] apn_buf A buffer large enough to store an APN (required if cn_domain is OSMO_GSUP_CN_DOMAIN_PS).
+ * \param[in] apn_buf_size Size of APN buffer (must be >= APN_MAXLEN).
+ * \param[in] cn_domain The CN Domain of the subscriber connection.
+ * \returns 0 on success, and negative on error.
+ */
+int osmo_gsup_create_insert_subscriber_data_msg(struct osmo_gsup_message *gsup, const char *imsi, const char *msisdn,
+						uint8_t *msisdn_enc, size_t msisdn_enc_size,
+						uint8_t *apn_buf, size_t apn_buf_size,
+						enum osmo_gsup_cn_domain cn_domain)
+{
+	int len;
+
+	OSMO_ASSERT(gsup);
+
+	gsup->message_type = OSMO_GSUP_MSGT_INSERT_DATA_REQUEST;
+	osmo_strlcpy(gsup->imsi, imsi, sizeof(gsup->imsi));
+
+	if (msisdn_enc_size < OSMO_GSUP_MAX_CALLED_PARTY_BCD_LEN)
+		return -ENOSPC;
+
+	OSMO_ASSERT(msisdn_enc);
+	len = gsm48_encode_bcd_number(msisdn_enc, msisdn_enc_size, 0, msisdn);
+	if (len < 1) {
+		LOGP(DLGSUP, LOGL_ERROR, "%s: Error: cannot encode MSISDN '%s'\n", imsi, msisdn);
+		return -ENOSPC;
+	}
+	gsup->msisdn_enc = msisdn_enc;
+	gsup->msisdn_enc_len = len;
+
+	#pragma message "FIXME: deal with encoding the following data: gsup.hlr_enc"
+
+	gsup->cn_domain = cn_domain;
+	if (gsup->cn_domain == OSMO_GSUP_CN_DOMAIN_PS) {
+		OSMO_ASSERT(apn_buf_size >= APN_MAXLEN);
+		OSMO_ASSERT(apn_buf);
+		/* FIXME: PDP infos - use more fine-grained access control
+		   instead of wildcard APN */
+		osmo_gsup_configure_wildcard_apn(gsup, apn_buf, apn_buf_size);
+	}
+
+	return 0;
+}
diff --git a/src/gsup_server.h b/src/gsup_server.h
index 66c1a9c..e49d283 100644
--- a/src/gsup_server.h
+++ b/src/gsup_server.h
@@ -6,6 +6,10 @@
 #include <osmocom/abis/ipaccess.h>
 #include <osmocom/gsm/gsup.h>
 
+#ifndef OSMO_GSUP_MAX_CALLED_PARTY_BCD_LEN
+#define OSMO_GSUP_MAX_CALLED_PARTY_BCD_LEN	43 /* TS 24.008 10.5.4.7 */
+#endif
+
 struct osmo_gsup_conn;
 
 /* Expects message in msg->l2h */
@@ -55,3 +59,7 @@
 
 int osmo_gsup_configure_wildcard_apn(struct osmo_gsup_message *gsup,
 				     uint8_t *apn_buf, size_t apn_buf_size);
+int osmo_gsup_create_insert_subscriber_data_msg(struct osmo_gsup_message *gsup, const char *imsi, const char *msisdn,
+					    uint8_t *msisdn_enc, size_t msisdn_enc_size,
+				            uint8_t *apn_buf, size_t apn_buf_size,
+					    enum osmo_gsup_cn_domain cn_domain);
diff --git a/src/hlr.c b/src/hlr.c
index 1c72f45..4da7b9b 100644
--- a/src/hlr.c
+++ b/src/hlr.c
@@ -26,7 +26,6 @@
 #include <osmocom/core/logging.h>
 #include <osmocom/core/application.h>
 #include <osmocom/gsm/gsup.h>
-#include <osmocom/gsm/gsm48_ie.h>
 #include <osmocom/vty/vty.h>
 #include <osmocom/vty/command.h>
 #include <osmocom/vty/telnet_interface.h>
@@ -62,54 +61,33 @@
 		return;
 
 	llist_for_each_entry(co, &g_hlr->gs->clients, list) {
-		struct osmo_gsup_message gsup = {
-			.message_type = OSMO_GSUP_MSGT_INSERT_DATA_REQUEST
-		};
+		struct osmo_gsup_message gsup = { };
+		uint8_t msisdn_enc[OSMO_GSUP_MAX_CALLED_PARTY_BCD_LEN];
+		uint8_t apn[APN_MAXLEN];
+		struct msgb *msg_out;
 		uint8_t *peer;
 		int peer_len;
-		uint8_t msisdn_enc[43]; /* TODO use constant; TS 24.008 10.5.4.7 */
-		uint8_t apn[APN_MAXLEN];
-		int len;
-		struct msgb *msg_out;
+		enum osmo_gsup_cn_domain cn_domain;
 
-		peer_len = osmo_gsup_conn_ccm_get(co, &peer, IPAC_IDTAG_SERNR);
-		if (peer_len < 0) {
+		if (co->supports_ps)
+			cn_domain = OSMO_GSUP_CN_DOMAIN_PS;
+		else if (co->supports_cs)
+			cn_domain = OSMO_GSUP_CN_DOMAIN_CS;
+		else {
+			/* We have not yet received a location update from this subscriber .*/
+			continue;
+		}
+
+		if (osmo_gsup_create_insert_subscriber_data_msg(&gsup, subscr->imsi, subscr->msisdn, msisdn_enc,
+								sizeof(msisdn_enc), apn, sizeof(apn), cn_domain) != 0) {
 			LOGP(DMAIN, LOGL_ERROR,
-			       "IMSI='%s': Cannot notify GSUP client, cannot get peer name "
+			       "IMSI='%s': Cannot notify GSUP client; could not create gsup message "
 			       "for %s:%u\n", subscr->imsi,
 			       co && co->conn && co->conn->server? co->conn->server->addr : "unset",
 			       co && co->conn && co->conn->server? co->conn->server->port : 0);
 			continue;
 		}
 
-		osmo_strlcpy(gsup.imsi, subscr->imsi, GSM23003_IMSI_MAX_DIGITS + 1);
-
-		len = gsm48_encode_bcd_number(msisdn_enc, sizeof(msisdn_enc), 0, subscr->msisdn);
-		if (len < 1) {
-			LOGP(DMAIN, LOGL_ERROR, "%s: Error: cannot encode MSISDN '%s'\n",
-			     subscr->imsi, subscr->msisdn);
-			continue;
-		}
-		gsup.msisdn_enc = msisdn_enc;
-		gsup.msisdn_enc_len = len;
-
-		if (co->supports_ps) {
-			gsup.cn_domain = OSMO_GSUP_CN_DOMAIN_PS;
-
-			/* FIXME: PDP infos - use more fine-grained access control
-			   instead of wildcard APN */
-			if (osmo_gsup_configure_wildcard_apn(&gsup, apn, sizeof(apn))) {
-				LOGP(DMAIN, LOGL_ERROR, "%s: Error: cannot encode wildcard APN\n",
-				     subscr->imsi);
-				continue;
-			}
-		} else if (co->supports_cs) {
-			gsup.cn_domain = OSMO_GSUP_CN_DOMAIN_CS;
-		} else {
-			/* We have not yet received a location update from this subscriber .*/
-			continue;
-		}
-
 		/* Send ISD to MSC/SGSN */
 		msg_out = msgb_alloc_headroom(1024+16, 16, "GSUP ISD UPDATE");
 		if (msg_out == NULL) {
@@ -120,8 +98,17 @@
 			       co && co->conn && co->conn->server? co->conn->server->port : 0);
 			continue;
 		}
-
 		osmo_gsup_encode(msg_out, &gsup);
+
+		peer_len = osmo_gsup_conn_ccm_get(co, &peer, IPAC_IDTAG_SERNR);
+		if (peer_len < 0) {
+			LOGP(DMAIN, LOGL_ERROR,
+			       "IMSI='%s': cannot get peer name for connection %s:%u\n", subscr->imsi,
+			       co && co->conn && co->conn->server? co->conn->server->addr : "unset",
+			       co && co->conn && co->conn->server? co->conn->server->port : 0);
+			continue;
+		}
+
 		if (osmo_gsup_addr_send(g_hlr->gs, peer, peer_len, msg_out) < 0) {
 			LOGP(DMAIN, LOGL_ERROR,
 			       "IMSI='%s': Cannot notify GSUP client; send operation failed "
diff --git a/src/luop.c b/src/luop.c
index db7b3c9..aff4d81 100644
--- a/src/luop.c
+++ b/src/luop.c
@@ -25,7 +25,6 @@
 #include <errno.h>
 
 #include <osmocom/core/logging.h>
-#include <osmocom/gsm/gsm48_ie.h>
 #include <osmocom/gsm/gsup.h>
 #include <osmocom/gsm/apn.h>
 
@@ -52,6 +51,7 @@
 	struct msgb *msg_out;
 
 	msg_out = msgb_alloc_headroom(1024+16, 16, "GSUP LUOP");
+	OSMO_ASSERT(msg_out);
 	osmo_gsup_encode(msg_out, gsup);
 
 	osmo_gsup_addr_send(luop->gsup_server, luop->peer,
@@ -215,40 +215,27 @@
 /*! Transmit Insert Subscriber Data to new VLR/SGSN */
 void lu_op_tx_insert_subscr_data(struct lu_operation *luop)
 {
-	struct osmo_gsup_message gsup;
-	uint8_t msisdn_enc[43]; /* TODO use constant; TS 24.008 10.5.4.7 */
+	struct hlr_subscriber *subscr = &luop->subscr;
+	struct osmo_gsup_message gsup = { };
+	uint8_t msisdn_enc[OSMO_GSUP_MAX_CALLED_PARTY_BCD_LEN];
 	uint8_t apn[APN_MAXLEN];
-	int l;
+	enum osmo_gsup_cn_domain cn_domain;
 
 	OSMO_ASSERT(luop->state == LU_S_LU_RECEIVED ||
 		    luop->state == LU_S_CANCEL_ACK_RECEIVED);
 
-	fill_gsup_msg(&gsup, luop, OSMO_GSUP_MSGT_INSERT_DATA_REQUEST);
+	if (luop->is_ps)
+		cn_domain = OSMO_GSUP_CN_DOMAIN_PS;
+	else
+		cn_domain = OSMO_GSUP_CN_DOMAIN_CS;
 
-	l = gsm48_encode_bcd_number(msisdn_enc, sizeof(msisdn_enc), 0,
-				    luop->subscr.msisdn);
-	if (l < 1) {
+	if (osmo_gsup_create_insert_subscriber_data_msg(&gsup, subscr->imsi, subscr->msisdn, msisdn_enc,
+							sizeof(msisdn_enc), apn, sizeof(apn), cn_domain) != 0) {
 		LOGP(DMAIN, LOGL_ERROR,
-		     "%s: Error: cannot encode MSISDN '%s'\n",
-		     luop->subscr.imsi, luop->subscr.msisdn);
-		lu_op_tx_error(luop, GMM_CAUSE_PROTO_ERR_UNSPEC);
+		       "IMSI='%s': Cannot notify GSUP client; could not create gsup message "
+		       "for %s\n", subscr->imsi, luop->peer);
 		return;
 	}
-	gsup.msisdn_enc = msisdn_enc;
-	gsup.msisdn_enc_len = l;
-
-	#pragma message "FIXME: deal with encoding the following data: gsup.hlr_enc"
-
-	if (luop->is_ps) {
-		/* FIXME: PDP infos - use more fine-grained access control
-		   instead of wildcard APN */
-		if (osmo_gsup_configure_wildcard_apn(&gsup, apn, sizeof(apn))) {
-			LOGP(DMAIN, LOGL_ERROR, "%s: Error: cannot encode wildcard APN\n",
-			     luop->subscr.imsi);
-			lu_op_tx_error(luop, GMM_CAUSE_PROTO_ERR_UNSPEC);
-			return;
-		}
-	}
 
 	/* Send ISD to new VLR/SGSN */
 	_luop_tx_gsup(luop, &gsup);
diff --git a/tests/gsup/gsup_test.err b/tests/gsup/gsup_test.err
index d9d272a..0aeae30 100644
--- a/tests/gsup/gsup_test.err
+++ b/tests/gsup/gsup_test.err
@@ -1,2 +1,2 @@
-DMAIN 10 01 08 21 43 65 87 09 21 43 f5 08 09 08 89 67 45 23 01 89 67 f5 05 07 10 01 01 12 02 01 2a 
+DMAIN 10 01 08 21 43 65 87 09 21 43 f5 08 09 08 89 67 45 23 01 89 67 f5 05 07 10 01 01 12 02 01 2a 28 01 01 
 DMAIN LU OP state change: LU RECEIVED -> ISD SENT

-- 
To view, visit https://gerrit.osmocom.org/7992
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I6a92ca34cdaadca9eacc774bb1ca386c325ba865
Gerrit-Change-Number: 7992
Gerrit-PatchSet: 6
Gerrit-Owner: Stefan Sperling <ssperling at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Stefan Sperling <ssperling at sysmocom.de>
Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20180518/448d2584/attachment.htm>


More information about the gerrit-log mailing list