[PATCH] osmo-hlr[master]: move creation of insert subscriber data messages to a common...

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
Tue May 8 13:00:39 UTC 2018


Hello Neels Hofmeyr, Jenkins Builder,

I'd like you to reexamine a change.  Please visit

    https://gerrit.osmocom.org/7992

to look at the new patch set (#4).

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(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/92/7992/4

diff --git a/src/gsup_server.c b/src/gsup_server.c
index 07d4feb..c525495 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[in] gsup  The gsup message to populate.
+ * \param[in] imsi  The subscriber's IMSI.
+ * \param[in] msisdn The subscriber's MSISDN.
+ * \param[in] 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[in] 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, char *imsi, 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..a725521 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, char *imsi, 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,51 +61,30 @@
 		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;
 		}
 
@@ -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,39 +215,26 @@
 /*! 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 */
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, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6a92ca34cdaadca9eacc774bb1ca386c325ba865
Gerrit-PatchSet: 4
Gerrit-Project: osmo-hlr
Gerrit-Branch: master
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>



More information about the gerrit-log mailing list