[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
Mon May 7 09:00:37 UTC 2018


Hello Jenkins Builder,

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

    https://gerrit.osmocom.org/7992

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

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, 90 insertions(+), 69 deletions(-)


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

diff --git a/src/gsup_server.c b/src/gsup_server.c
index 07d4feb..685633e 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,42 @@
 
 	return 0;
 }
+
+struct osmo_gsup_message *
+osmo_gsup_create_insert_subscriber_data_msg(char *imsi, char *msisdn, enum osmo_gsup_cn_domain cn_domain)
+{
+	struct osmo_gsup_message *gsup;
+	int len;
+	uint8_t *msisdn_enc;
+
+	gsup = talloc_zero(NULL, struct osmo_gsup_message);
+	OSMO_ASSERT(gsup);
+
+	gsup->message_type = OSMO_GSUP_MSGT_INSERT_DATA_REQUEST;
+	osmo_strlcpy(gsup->imsi, imsi, GSM23003_IMSI_MAX_DIGITS + 1);
+
+	msisdn_enc = talloc_size(gsup, OSMO_GSUP_MAX_CALLED_PARTY_BCD_LEN);
+	OSMO_ASSERT(msisdn_enc);
+
+	len = gsm48_encode_bcd_number(msisdn_enc, OSMO_GSUP_MAX_CALLED_PARTY_BCD_LEN, 0, msisdn);
+	if (len < 1) {
+		LOGP(DLGSUP, LOGL_ERROR, "%s: Error: cannot encode MSISDN '%s'\n", imsi, msisdn);
+		talloc_free(gsup);
+		return NULL;
+	}
+	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) {
+		uint8_t *apn = talloc_size(gsup, APN_MAXLEN);
+		OSMO_ASSERT(apn);
+		/* FIXME: PDP infos - use more fine-grained access control
+		   instead of wildcard APN */
+		osmo_gsup_configure_wildcard_apn(gsup, apn, APN_MAXLEN);
+	}
+
+	return gsup;
+}
diff --git a/src/gsup_server.h b/src/gsup_server.h
index 66c1a9c..0945d71 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,5 @@
 
 int osmo_gsup_configure_wildcard_apn(struct osmo_gsup_message *gsup,
 				     uint8_t *apn_buf, size_t apn_buf_size);
+struct osmo_gsup_message *osmo_gsup_create_insert_subscriber_data_msg(char *imsi, char *msisdn,
+								      enum osmo_gsup_cn_domain cn_domain);
diff --git a/src/hlr.c b/src/hlr.c
index 1c72f45..919751c 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,28 @@
 		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;
+		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;
+		}
+
+		gsup = osmo_gsup_create_insert_subscriber_data_msg(subscr->imsi, subscr->msisdn, cn_domain);
+		if (gsup == NULL) {
 			LOGP(DMAIN, LOGL_ERROR,
-			       "IMSI='%s': Cannot notify GSUP client, cannot get peer name "
+			       "IMSI='%s': Cannot notify GSUP client; could not allocate 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;
 		}
 
@@ -118,10 +94,23 @@
 			       "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);
+			talloc_free(gsup);
+			gsup = NULL;
+			continue;
+		}
+		osmo_gsup_encode(msg_out, gsup);
+		talloc_free(gsup);
+		gsup = NULL;
+
+		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;
 		}
 
-		osmo_gsup_encode(msg_out, &gsup);
 		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..40cc6fb 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,43 +215,29 @@
 /*! 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 */
-	uint8_t apn[APN_MAXLEN];
-	int l;
+	struct hlr_subscriber *subscr = &luop->subscr;
+	struct osmo_gsup_message *gsup;
+	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) {
+	gsup = osmo_gsup_create_insert_subscriber_data_msg(subscr->imsi, subscr->msisdn, cn_domain);
+	if (gsup == NULL) {
 		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 allocate 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);
+	_luop_tx_gsup(luop, gsup);
+	talloc_free(gsup);
 
 	lu_op_statechg(luop, LU_S_ISD_SENT);
 	osmo_timer_schedule(&luop->timer, ISD_TIMEOUT_SECS, 0);
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: 2
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 <nhofmeyr at sysmocom.de>



More information about the gerrit-log mailing list