From f81cacc6814dde73f203d125b0065d1451a98317 Mon Sep 17 00:00:00 2001
From: Jacob Erlbeck <jerlbeck@sysmocom.de>
Date: Thu, 8 Jan 2015 16:23:25 +0100
Subject: gprs: Block other GSUP procedures during PURGE_MS

GSM 09.02, 19.4.1.4 mandates that no other MAP procedures shall be
started until the PURGE_MS procedure has been completed.

This patch implements this by adding corresponding state and checks
to gprs_subscr_purge, gprs_subscr_location_update, and
gprs_subscr_update_auth_info. If an Update Location or a Send Auth
Info Req procedure is not started because of blocking, the retry
mechanism is aborted to shorten the blocking time. The outstanding
Purge MS procedure itself is not aborted.

Sponsored-by: On-Waves ehf
---
 openbsc/include/openbsc/gprs_sgsn.h | 12 +++++
 openbsc/src/gprs/gprs_subscriber.c  | 56 +++++++++++++++++++++-
 openbsc/tests/sgsn/sgsn_test.c      | 92 +++++++++++++++++++++++++++++++++++++
 openbsc/tests/sgsn/sgsn_test.ok     |  1 +
 4 files changed, 160 insertions(+), 1 deletion(-)

diff --git a/openbsc/include/openbsc/gprs_sgsn.h b/openbsc/include/openbsc/gprs_sgsn.h
index 2b94096f6..d6a9bdada 100644
--- a/openbsc/include/openbsc/gprs_sgsn.h
+++ b/openbsc/include/openbsc/gprs_sgsn.h
@@ -271,6 +271,13 @@ struct imsi_acl_entry {
 	char imsi[16+1];
 };
 
+enum sgsn_subscriber_proc {
+	SGSN_SUBSCR_PROC_NONE = 0,
+	SGSN_SUBSCR_PROC_PURGE,
+	SGSN_SUBSCR_PROC_UPD_LOC,
+	SGSN_SUBSCR_PROC_UPD_AUTH,
+};
+
 struct sgsn_subscriber_data {
 	struct sgsn_mm_ctx	*mm;
 	struct gsm_auth_tuple	auth_triplets[5];
@@ -278,6 +285,7 @@ struct sgsn_subscriber_data {
 	int			error_cause;
 	struct osmo_timer_list	timer;
 	int			retries;
+	enum sgsn_subscriber_proc blocked_by;
 };
 
 #define LOGGSUBSCRP(level, subscr, fmt, args...) \
@@ -324,6 +332,10 @@ void gprs_subscr_update(struct gsm_subscriber *subscr);
 void gprs_subscr_update_auth_info(struct gsm_subscriber *subscr);
 int gprs_subscr_rx_gsup_message(struct msgb *msg);
 
+int gprs_subscr_purge(struct gsm_subscriber *subscr);
+int gprs_subscr_query_auth_info(struct gsm_subscriber *subscr);
+int gprs_subscr_location_update(struct gsm_subscriber *subscr);
+
 /* Called on subscriber data updates */
 void sgsn_update_subscriber_data(struct sgsn_mm_ctx *mmctx,
 				 struct gsm_subscriber *subscr);
diff --git a/openbsc/src/gprs/gprs_subscriber.c b/openbsc/src/gprs/gprs_subscriber.c
index 8399ea137..88e037e9d 100644
--- a/openbsc/src/gprs/gprs_subscriber.c
+++ b/openbsc/src/gprs/gprs_subscriber.c
@@ -76,6 +76,23 @@ static int gsup_read_cb(struct gprs_gsup_client *gsupc, struct msgb *msg)
 	return rc;
 }
 
+static int check_blocking(
+	struct gsm_subscriber *subscr,
+	enum sgsn_subscriber_proc what)
+{
+	if (subscr->sgsn_data->blocked_by == SGSN_SUBSCR_PROC_NONE ||
+	    subscr->sgsn_data->blocked_by == what)
+		return 1;
+
+	return 0;
+}
+
+static void abort_blocking_procedure(struct gsm_subscriber *subscr)
+{
+	/* Best effort, stop retries at least */
+	subscr->sgsn_data->retries = SGSN_SUBSCR_MAX_RETRIES;
+}
+
 static void sgsn_subscriber_timeout_cb(void *subscr_);
 int gprs_subscr_purge(struct gsm_subscriber *subscr);
 
@@ -132,6 +149,10 @@ static void sgsn_subscriber_timeout_cb(void *subscr_)
 	return;
 
 force_cleanup:
+	/* Make sure to clear blocking */
+	if (check_blocking(subscr, SGSN_SUBSCR_PROC_PURGE))
+		subscr->sgsn_data->blocked_by = SGSN_SUBSCR_PROC_NONE;
+
 	/* Make sure, the timer is cleaned up */
 	subscr->keep_in_ram = 0;
 	gprs_subscr_stop_timer(subscr);
@@ -544,17 +565,42 @@ int gprs_subscr_rx_gsup_message(struct msgb *msg)
 int gprs_subscr_purge(struct gsm_subscriber *subscr)
 {
 	struct gprs_gsup_message gsup_msg = {0};
+	int rc;
+
+	if (!check_blocking(subscr, SGSN_SUBSCR_PROC_PURGE)) {
+		LOGGSUBSCRP(
+			LOGL_NOTICE, subscr,
+			"Cannot purge MS subscriber, blocked\n");
+		return -EAGAIN;
+	}
+
+	/* GSM 09.02, 19.4.1.4 requires other MAP requests to be blocked until
+	 * this procedure is completed
+	 */
+	subscr->sgsn_data->blocked_by = SGSN_SUBSCR_PROC_PURGE;
 
 	LOGGSUBSCRP(LOGL_INFO, subscr, "purging MS subscriber\n");
 
 	gsup_msg.message_type = GPRS_GSUP_MSGT_PURGE_MS_REQUEST;
-	return gprs_subscr_tx_gsup_message(subscr, &gsup_msg);
+	rc = gprs_subscr_tx_gsup_message(subscr, &gsup_msg);
+	if (rc < 0)
+		subscr->sgsn_data->blocked_by = SGSN_SUBSCR_PROC_NONE;
+
+	return rc;
 }
 
 int gprs_subscr_query_auth_info(struct gsm_subscriber *subscr)
 {
 	struct gprs_gsup_message gsup_msg = {0};
 
+	if (!check_blocking(subscr, SGSN_SUBSCR_PROC_UPD_AUTH)) {
+		LOGGSUBSCRP(
+			LOGL_NOTICE, subscr,
+			"Cannot start update auth info request procedure, blocked\n");
+		abort_blocking_procedure(subscr);
+		return -EAGAIN;
+	}
+
 	LOGGSUBSCRP(LOGL_INFO, subscr,
 		"subscriber auth info is not available\n");
 
@@ -566,6 +612,14 @@ int gprs_subscr_location_update(struct gsm_subscriber *subscr)
 {
 	struct gprs_gsup_message gsup_msg = {0};
 
+	if (!check_blocking(subscr, SGSN_SUBSCR_PROC_UPD_LOC)) {
+		LOGGSUBSCRP(
+			LOGL_NOTICE, subscr,
+			"Cannot start update location procedure, blocked\n");
+		abort_blocking_procedure(subscr);
+		return -EAGAIN;
+	}
+
 	LOGGSUBSCRP(LOGL_INFO, subscr,
 		"subscriber data is not available\n");
 
diff --git a/openbsc/tests/sgsn/sgsn_test.c b/openbsc/tests/sgsn/sgsn_test.c
index 818106190..d419f0a92 100644
--- a/openbsc/tests/sgsn/sgsn_test.c
+++ b/openbsc/tests/sgsn/sgsn_test.c
@@ -546,6 +546,97 @@ static void test_subscriber_gsup(void)
 	update_subscriber_data_cb = __real_sgsn_update_subscriber_data;
 }
 
+int my_gprs_gsup_client_send_dummy(struct gprs_gsup_client *gsupc, struct msgb *msg)
+{
+	msgb_free(msg);
+	return 0;
+};
+
+
+static void test_subscriber_blocking(void)
+{
+	struct gsm_subscriber *s1;
+	const char *imsi1 = "1234567890";
+	struct sgsn_mm_ctx *ctx;
+	struct gprs_ra_id raid = { 0, };
+	uint32_t local_tlli = 0xffeeddcc;
+	struct gprs_llc_llme *llme;
+	int rc;
+
+	printf("Testing subcriber procedure blocking\n");
+
+	gprs_gsup_client_send_cb = my_gprs_gsup_client_send_dummy;
+	sgsn->gsup_client = talloc_zero(tall_bsc_ctx, struct gprs_gsup_client);
+
+	/* Check for emptiness */
+	OSMO_ASSERT(gprs_subscr_get_by_imsi(imsi1) == NULL);
+
+	/* Create a context */
+	OSMO_ASSERT(count(gprs_llme_list()) == 0);
+	ctx = alloc_mm_ctx(local_tlli, &raid);
+	llme = ctx->llme;
+	strncpy(ctx->imsi, imsi1, sizeof(ctx->imsi) - 1);
+
+	/* Allocate and attach a subscriber */
+	s1 = gprs_subscr_get_or_create_by_mmctx(ctx);
+	assert_subscr(s1, imsi1);
+
+	/* Start SendAuthInfoRequest procedure */
+	rc = gprs_subscr_query_auth_info(s1);
+	/* Not blocking */
+	OSMO_ASSERT(rc == 0);
+
+	/* Start UpdateLocation procedure */
+	rc = gprs_subscr_location_update(s1);
+	/* Blocking */
+	OSMO_ASSERT(rc == 0);
+
+	/* Start PurgeMS procedure */
+	rc = gprs_subscr_purge(s1);
+	/* Not blocking */
+	OSMO_ASSERT(rc == 0);
+	OSMO_ASSERT(s1->sgsn_data->blocked_by == SGSN_SUBSCR_PROC_PURGE);
+
+	/* Start PurgeMS procedure (retry) */
+	rc = gprs_subscr_purge(s1);
+	/* Not blocking */
+	OSMO_ASSERT(rc == 0);
+
+	/* Start SendAuthInfoRequest procedure */
+	rc = gprs_subscr_query_auth_info(s1);
+	/* Blocking */
+	OSMO_ASSERT(rc == -EAGAIN);
+
+	/* Start UpdateLocation procedure */
+	rc = gprs_subscr_location_update(s1);
+	/* Blocking */
+	OSMO_ASSERT(rc == -EAGAIN);
+
+	/* Unblock manually (normally done by the caller of gprs_subscr_purge) */
+	s1->sgsn_data->blocked_by = SGSN_SUBSCR_PROC_NONE;
+
+	/* Start SendAuthInfoRequest procedure */
+	rc = gprs_subscr_query_auth_info(s1);
+	/* Not blocking */
+	OSMO_ASSERT(rc == 0);
+
+	/* Start UpdateLocation procedure */
+	rc = gprs_subscr_location_update(s1);
+	/* Blocking */
+	OSMO_ASSERT(rc == 0);
+
+	subscr_put(s1);
+	sgsn_mm_ctx_free(ctx);
+	gprs_llgmm_assign(llme, local_tlli, 0xffffffff, GPRS_ALGO_GEA0, NULL);
+
+	assert_no_subscrs();
+
+	gprs_gsup_client_send_cb = __real_gprs_gsup_client_send;
+	talloc_free(sgsn->gsup_client);
+	sgsn->gsup_client = NULL;
+}
+
+
 /*
  * Test that a GMM Detach will remove the MMCTX and the
  * associated LLME.
@@ -1710,6 +1801,7 @@ int main(int argc, char **argv)
 	test_subscriber();
 	test_auth_triplets();
 	test_subscriber_gsup();
+	test_subscriber_blocking();
 	test_gmm_detach();
 	test_gmm_detach_power_off();
 	test_gmm_detach_no_mmctx();
diff --git a/openbsc/tests/sgsn/sgsn_test.ok b/openbsc/tests/sgsn/sgsn_test.ok
index cff29205d..e5df50482 100644
--- a/openbsc/tests/sgsn/sgsn_test.ok
+++ b/openbsc/tests/sgsn/sgsn_test.ok
@@ -2,6 +2,7 @@ Testing LLME allocations
 Testing core subscriber data API
 Testing authentication triplet handling
 Testing subcriber GSUP handling
+Testing subcriber procedure blocking
 Testing GMM detach
 Testing GMM detach (power off)
 Testing GMM detach (no MMCTX)
-- 
cgit v1.2.3


From 9999fd9026fbb3f4a7d07a455698230d209dbcb6 Mon Sep 17 00:00:00 2001
From: Jacob Erlbeck <jerlbeck@sysmocom.de>
Date: Thu, 15 Jan 2015 17:08:30 +0100
Subject: gprs: Add replies for all GSUP requests

Currently, an incoming GSUP request message isn't answered at all if
it is not handled due to an error or missing implementation.

This patch adds GSUP error replies for these requests (and only for
requests). It also adds tests for these cases.

Note that several of these tests check for
GMM_CAUSE_MSGT_NOTEXIST_NOTIMPL, which will have to be changed, when
the features are implemented.

Sponsored-by: On-Waves ehf
---
 openbsc/include/openbsc/gprs_gsup_messages.h |  4 ++
 openbsc/include/openbsc/gprs_sgsn.h          |  3 +-
 openbsc/src/gprs/gprs_subscriber.c           | 59 ++++++++++++++++++++++++----
 openbsc/tests/sgsn/sgsn_test.c               | 47 +++++++++++++++++++++-
 4 files changed, 103 insertions(+), 10 deletions(-)

diff --git a/openbsc/include/openbsc/gprs_gsup_messages.h b/openbsc/include/openbsc/gprs_gsup_messages.h
index b63f74baa..9857b979d 100644
--- a/openbsc/include/openbsc/gprs_gsup_messages.h
+++ b/openbsc/include/openbsc/gprs_gsup_messages.h
@@ -74,6 +74,10 @@ enum gprs_gsup_message_type {
 	GPRS_GSUP_MSGT_LOCATION_CANCEL_RESULT	= 0b00011110,
 };
 
+#define GPRS_GSUP_IS_MSGT_REQUEST(msgt) (((msgt) & 0b00000011) == 0b00)
+#define GPRS_GSUP_IS_MSGT_ERROR(msgt)   (((msgt) & 0b00000011) == 0b01)
+#define GPRS_GSUP_TO_MSGT_ERROR(msgt)   (((msgt) & 0b11111100) | 0b01)
+
 enum gprs_gsup_cancel_type {
 	GPRS_GSUP_CANCEL_TYPE_UPDATE		= 1, /* on wire: 0 */
 	GPRS_GSUP_CANCEL_TYPE_WITHDRAW		= 2, /* on wire: 1 */
diff --git a/openbsc/include/openbsc/gprs_sgsn.h b/openbsc/include/openbsc/gprs_sgsn.h
index d6a9bdada..25810ab3a 100644
--- a/openbsc/include/openbsc/gprs_sgsn.h
+++ b/openbsc/include/openbsc/gprs_sgsn.h
@@ -289,7 +289,8 @@ struct sgsn_subscriber_data {
 };
 
 #define LOGGSUBSCRP(level, subscr, fmt, args...) \
-	LOGP(DGPRS, level, "SUBSCR(%s) " fmt, (subscr)->imsi, \
+	LOGP(DGPRS, level, "SUBSCR(%s) " fmt, \
+	     (subscr) ? (subscr)->imsi : "---", \
 	     ## args)
 
 struct sgsn_config;
diff --git a/openbsc/src/gprs/gprs_subscriber.c b/openbsc/src/gprs/gprs_subscriber.c
index 88e037e9d..e971210ef 100644
--- a/openbsc/src/gprs/gprs_subscriber.c
+++ b/openbsc/src/gprs/gprs_subscriber.c
@@ -231,12 +231,13 @@ static int gprs_subscr_tx_gsup_message(struct gsm_subscriber *subscr,
 {
 	struct msgb *msg = gprs_gsup_msgb_alloc();
 
-	strncpy(gsup_msg->imsi, subscr->imsi, sizeof(gsup_msg->imsi) - 1);
+	if (strlen(gsup_msg->imsi) == 0 && subscr)
+		strncpy(gsup_msg->imsi, subscr->imsi, sizeof(gsup_msg->imsi) - 1);
 
 	gprs_gsup_encode(msg, gsup_msg);
 
 	LOGGSUBSCRP(LOGL_INFO, subscr,
-		"Sending GSUP, will send: %s\n", msgb_hexdump(msg));
+		    "Sending GSUP, will send: %s\n", msgb_hexdump(msg));
 
 	if (!sgsn->gsup_client) {
 		msgb_free(msg);
@@ -246,6 +247,20 @@ static int gprs_subscr_tx_gsup_message(struct gsm_subscriber *subscr,
 	return gprs_gsup_client_send(sgsn->gsup_client, msg);
 }
 
+static int gprs_subscr_tx_gsup_error_reply(struct gsm_subscriber *subscr,
+					   struct gprs_gsup_message *gsup_orig,
+					   enum gsm48_gmm_cause cause)
+{
+	struct gprs_gsup_message gsup_reply = {0};
+
+	strncpy(gsup_reply.imsi, gsup_orig->imsi, sizeof(gsup_reply.imsi) - 1);
+	gsup_reply.cause = cause;
+	gsup_reply.message_type =
+		GPRS_GSUP_TO_MSGT_ERROR(gsup_orig->message_type);
+
+	return gprs_subscr_tx_gsup_message(subscr, &gsup_reply);
+}
+
 static int gprs_subscr_handle_gsup_auth_res(struct gsm_subscriber *subscr,
 					    struct gprs_gsup_message *gsup_msg)
 {
@@ -475,6 +490,32 @@ static int gprs_subscr_handle_gsup_purge_err(struct gsm_subscriber *subscr,
 	return -gsup_msg->cause;
 }
 
+static int gprs_subscr_handle_unknown_imsi(struct gprs_gsup_message *gsup_msg)
+{
+	if (GPRS_GSUP_IS_MSGT_REQUEST(gsup_msg->message_type)) {
+		gprs_subscr_tx_gsup_error_reply(NULL, gsup_msg,
+						GMM_CAUSE_IMSI_UNKNOWN);
+		LOGP(DGPRS, LOGL_NOTICE,
+		     "Unknown IMSI %s, discarding GSUP request "
+		     "of type 0x%02x\n",
+		     gsup_msg->imsi, gsup_msg->message_type);
+	} else if (GPRS_GSUP_IS_MSGT_ERROR(gsup_msg->message_type)) {
+		LOGP(DGPRS, LOGL_NOTICE,
+		     "Unknown IMSI %s, discarding GSUP error "
+		     "of type 0x%02x, cause '%s' (%d)\n",
+		     gsup_msg->imsi, gsup_msg->message_type,
+		     get_value_string(gsm48_gmm_cause_names, gsup_msg->cause),
+		     gsup_msg->cause);
+	} else {
+		LOGP(DGPRS, LOGL_NOTICE,
+		     "Unknown IMSI %s, discarding GSUP response "
+		     "of type 0x%02x\n",
+		     gsup_msg->imsi, gsup_msg->message_type);
+	}
+
+	return -GMM_CAUSE_IMSI_UNKNOWN;
+}
+
 int gprs_subscr_rx_gsup_message(struct msgb *msg)
 {
 	uint8_t *data = msgb_l2(msg);
@@ -500,11 +541,8 @@ int gprs_subscr_rx_gsup_message(struct msgb *msg)
 	else
 		subscr = gprs_subscr_get_by_imsi(gsup_msg.imsi);
 
-	if (!subscr) {
-		LOGP(DGPRS, LOGL_NOTICE,
-		     "Unknown IMSI %s, discarding GSUP message\n", gsup_msg.imsi);
-		return -GMM_CAUSE_IMSI_UNKNOWN;
-	}
+	if (!subscr)
+		return gprs_subscr_handle_unknown_imsi(&gsup_msg);
 
 	LOGGSUBSCRP(LOGL_INFO, subscr,
 		"Received GSUP message of type 0x%02x\n", gsup_msg.message_type);
@@ -545,6 +583,8 @@ int gprs_subscr_rx_gsup_message(struct msgb *msg)
 		LOGGSUBSCRP(LOGL_ERROR, subscr,
 			"Rx GSUP message type %d not yet implemented\n",
 			gsup_msg.message_type);
+		gprs_subscr_tx_gsup_error_reply(subscr, &gsup_msg,
+						GMM_CAUSE_MSGT_NOTEXIST_NOTIMPL);
 		rc = -GMM_CAUSE_MSGT_NOTEXIST_NOTIMPL;
 		break;
 
@@ -552,7 +592,10 @@ int gprs_subscr_rx_gsup_message(struct msgb *msg)
 		LOGGSUBSCRP(LOGL_ERROR, subscr,
 			"Rx GSUP message type %d not valid at SGSN\n",
 			gsup_msg.message_type);
-		rc = -GMM_CAUSE_MSGT_INCOMP_P_STATE;
+		if (GPRS_GSUP_IS_MSGT_REQUEST(gsup_msg.message_type))
+			gprs_subscr_tx_gsup_error_reply(
+				subscr, &gsup_msg, GMM_CAUSE_MSGT_NOTEXIST_NOTIMPL);
+		rc = -GMM_CAUSE_MSGT_NOTEXIST_NOTIMPL;
 		break;
 	};
 
diff --git a/openbsc/tests/sgsn/sgsn_test.c b/openbsc/tests/sgsn/sgsn_test.c
index d419f0a92..7050a16ec 100644
--- a/openbsc/tests/sgsn/sgsn_test.c
+++ b/openbsc/tests/sgsn/sgsn_test.c
@@ -468,6 +468,21 @@ static void test_subscriber_gsup(void)
 		0x06, 0x01, 0x00,
 	};
 
+	static const uint8_t insert_data_req[] = {
+		0x10,
+		TEST_GSUP_IMSI1_IE,
+		0x05, 0x11,
+			0x10, 0x01, 0x03,
+			0x11, 0x02, 0xf1, 0x21, /* IPv4 */
+			0x12, 0x08, 0x03, 'b', 'a', 'r', 0x03, 'a', 'p', 'n',
+	};
+
+	static const uint8_t delete_data_req[] = {
+		0x14,
+		TEST_GSUP_IMSI1_IE,
+		0x10, 0x01, 0x03,
+	};
+
 	printf("Testing subcriber GSUP handling\n");
 
 	update_subscriber_data_cb = my_dummy_sgsn_update_subscriber_data;
@@ -527,7 +542,19 @@ static void test_subscriber_gsup(void)
 	/* Check authorization */
 	OSMO_ASSERT(s1->authorized == 0);
 
-	/* Inject UpdateLocReq GSUP message */
+	/* Inject InsertSubscrData GSUP message */
+	last_updated_subscr = NULL;
+	rc = rx_gsup_message(insert_data_req, sizeof(insert_data_req));
+	OSMO_ASSERT(rc == -GMM_CAUSE_MSGT_NOTEXIST_NOTIMPL);
+	OSMO_ASSERT(last_updated_subscr == NULL);
+
+	/* Inject DeleteSubscrData GSUP message */
+	last_updated_subscr = NULL;
+	rc = rx_gsup_message(delete_data_req, sizeof(delete_data_req));
+	OSMO_ASSERT(rc == -GMM_CAUSE_MSGT_NOTEXIST_NOTIMPL);
+	OSMO_ASSERT(last_updated_subscr == NULL);
+
+	/* Inject LocCancelReq GSUP message */
 	rc = rx_gsup_message(location_cancellation_req,
 			     sizeof(location_cancellation_req));
 	OSMO_ASSERT(rc >= 0);
@@ -543,6 +570,24 @@ static void test_subscriber_gsup(void)
 	OSMO_ASSERT(s1found == NULL);
 	gprs_llgmm_assign(llme, local_tlli, 0xffffffff, GPRS_ALGO_GEA0, NULL);
 
+	/* Inject InsertSubscrData GSUP message (unknown IMSI) */
+	last_updated_subscr = NULL;
+	rc = rx_gsup_message(insert_data_req, sizeof(insert_data_req));
+	/* TODO: Remove the comments when this is fixed */
+	/* OSMO_ASSERT(rc == -GMM_CAUSE_IMSI_UNKNOWN); */
+	OSMO_ASSERT(last_updated_subscr == NULL);
+
+	/* Inject DeleteSubscrData GSUP message (unknown IMSI) */
+	rc = rx_gsup_message(delete_data_req, sizeof(delete_data_req));
+	OSMO_ASSERT(rc == -GMM_CAUSE_IMSI_UNKNOWN);
+	OSMO_ASSERT(last_updated_subscr == NULL);
+
+	/* Inject LocCancelReq GSUP message (unknown IMSI) */
+	rc = rx_gsup_message(location_cancellation_req,
+			     sizeof(location_cancellation_req));
+	OSMO_ASSERT(rc == -GMM_CAUSE_IMSI_UNKNOWN);
+	OSMO_ASSERT(last_updated_subscr == NULL);
+
 	update_subscriber_data_cb = __real_sgsn_update_subscriber_data;
 }
 
-- 
cgit v1.2.3


From 4dedb27d7e7829099bf0873ecd6af3b9b9e570b5 Mon Sep 17 00:00:00 2001
From: Jacob Erlbeck <jerlbeck@sysmocom.de>
Date: Thu, 15 Jan 2015 17:50:16 +0100
Subject: gprs: Don't create a subscr entry on InsertSubscriberData

Currently gprs_subscr_rx_gsup_message creates a subscriber entry if
such an entry doesn't exist for the IMSI within an
InsertSubscriberData GSUP message. This behaviour is not compliant to
GSM 09.02, 20.3.3.2 (Subscriber data management/SGSN) where it is
defined, that an error ("Unidentified subscriber") shall be returned.

This patch removes the case distinction, so that an existing
subscriber entry is required for all incoming GSUP messages.

Sponsored-by: On-Waves ehf
---
 openbsc/src/gprs/gprs_subscriber.c | 5 +----
 openbsc/tests/sgsn/sgsn_test.c     | 3 +--
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/openbsc/src/gprs/gprs_subscriber.c b/openbsc/src/gprs/gprs_subscriber.c
index e971210ef..271cc7832 100644
--- a/openbsc/src/gprs/gprs_subscriber.c
+++ b/openbsc/src/gprs/gprs_subscriber.c
@@ -536,10 +536,7 @@ int gprs_subscr_rx_gsup_message(struct msgb *msg)
 	if (!gsup_msg.imsi[0])
 		return -GMM_CAUSE_INV_MAND_INFO;
 
-	if (gsup_msg.message_type == GPRS_GSUP_MSGT_INSERT_DATA_REQUEST)
-		subscr = gprs_subscr_get_or_create(gsup_msg.imsi);
-	else
-		subscr = gprs_subscr_get_by_imsi(gsup_msg.imsi);
+	subscr = gprs_subscr_get_by_imsi(gsup_msg.imsi);
 
 	if (!subscr)
 		return gprs_subscr_handle_unknown_imsi(&gsup_msg);
diff --git a/openbsc/tests/sgsn/sgsn_test.c b/openbsc/tests/sgsn/sgsn_test.c
index 7050a16ec..c09b16931 100644
--- a/openbsc/tests/sgsn/sgsn_test.c
+++ b/openbsc/tests/sgsn/sgsn_test.c
@@ -573,8 +573,7 @@ static void test_subscriber_gsup(void)
 	/* Inject InsertSubscrData GSUP message (unknown IMSI) */
 	last_updated_subscr = NULL;
 	rc = rx_gsup_message(insert_data_req, sizeof(insert_data_req));
-	/* TODO: Remove the comments when this is fixed */
-	/* OSMO_ASSERT(rc == -GMM_CAUSE_IMSI_UNKNOWN); */
+	OSMO_ASSERT(rc == -GMM_CAUSE_IMSI_UNKNOWN);
 	OSMO_ASSERT(last_updated_subscr == NULL);
 
 	/* Inject DeleteSubscrData GSUP message (unknown IMSI) */
-- 
cgit v1.2.3


From 87c7ffccea5fb8a579ade220e6e4754f857e4ecc Mon Sep 17 00:00:00 2001
From: Jacob Erlbeck <jerlbeck@sysmocom.de>
Date: Thu, 8 Jan 2015 15:29:01 +0100
Subject: gprs: Support the full cancellation procedure

Currently no GSUP LocationCancellationResult message is sent back to
the peer (HLR), if the procedure succeeded at the SGSN's side.

This patch adds the missing message and put the whole request
handling of this procedure into a separate function.

Ticket: OW#1338
Sponsored-by: On-Waves ehf
---
 openbsc/src/gprs/gprs_subscriber.c | 23 ++++++++++++++++++-----
 openbsc/tests/sgsn/sgsn_test.c     | 17 +++++++++++++++++
 2 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/openbsc/src/gprs/gprs_subscriber.c b/openbsc/src/gprs/gprs_subscriber.c
index 271cc7832..a30729d4d 100644
--- a/openbsc/src/gprs/gprs_subscriber.c
+++ b/openbsc/src/gprs/gprs_subscriber.c
@@ -490,6 +490,22 @@ static int gprs_subscr_handle_gsup_purge_err(struct gsm_subscriber *subscr,
 	return -gsup_msg->cause;
 }
 
+static int gprs_subscr_handle_loc_cancel_req(struct gsm_subscriber *subscr,
+					     struct gprs_gsup_message *gsup_msg)
+{
+	struct gprs_gsup_message gsup_reply = {0};
+
+	LOGGSUBSCRP(LOGL_INFO, subscr, "purging MS subscriber\n");
+
+	gsup_reply.message_type = GPRS_GSUP_MSGT_LOCATION_CANCEL_RESULT;
+	gprs_subscr_tx_gsup_message(subscr, &gsup_reply);
+
+	subscr->sgsn_data->error_cause = 0;
+	gprs_subscr_put_and_cancel(subscr_get(subscr));
+
+	return 0;
+}
+
 static int gprs_subscr_handle_unknown_imsi(struct gprs_gsup_message *gsup_msg)
 {
 	if (GPRS_GSUP_IS_MSGT_REQUEST(gsup_msg->message_type)) {
@@ -546,9 +562,7 @@ int gprs_subscr_rx_gsup_message(struct msgb *msg)
 
 	switch (gsup_msg.message_type) {
 	case GPRS_GSUP_MSGT_LOCATION_CANCEL_REQUEST:
-		subscr->sgsn_data->error_cause = 0;
-		gprs_subscr_put_and_cancel(subscr);
-		subscr = NULL;
+		rc = gprs_subscr_handle_loc_cancel_req(subscr, &gsup_msg);
 		break;
 
 	case GPRS_GSUP_MSGT_SEND_AUTH_INFO_RESULT:
@@ -596,8 +610,7 @@ int gprs_subscr_rx_gsup_message(struct msgb *msg)
 		break;
 	};
 
-	if (subscr)
-		subscr_put(subscr);
+	subscr_put(subscr);
 
 	return rc;
 }
diff --git a/openbsc/tests/sgsn/sgsn_test.c b/openbsc/tests/sgsn/sgsn_test.c
index c09b16931..662227d65 100644
--- a/openbsc/tests/sgsn/sgsn_test.c
+++ b/openbsc/tests/sgsn/sgsn_test.c
@@ -468,6 +468,12 @@ static void test_subscriber_gsup(void)
 		0x06, 0x01, 0x00,
 	};
 
+	static const uint8_t location_cancellation_req_other[] = {
+		0x1c,
+		0x01, 0x05, 0x11, 0x11, 0x11, 0x11, 0x01,
+		0x06, 0x01, 0x00,
+	};
+
 	static const uint8_t insert_data_req[] = {
 		0x10,
 		TEST_GSUP_IMSI1_IE,
@@ -554,6 +560,17 @@ static void test_subscriber_gsup(void)
 	OSMO_ASSERT(rc == -GMM_CAUSE_MSGT_NOTEXIST_NOTIMPL);
 	OSMO_ASSERT(last_updated_subscr == NULL);
 
+	/* Inject wrong LocCancelReq GSUP message */
+	last_updated_subscr = NULL;
+	rc = rx_gsup_message(location_cancellation_req_other,
+			     sizeof(location_cancellation_req_other));
+	OSMO_ASSERT(rc == -GMM_CAUSE_IMSI_UNKNOWN);
+	OSMO_ASSERT(last_updated_subscr == NULL);
+
+	/* Check cancellation result */
+	OSMO_ASSERT(!(s1->flags & GPRS_SUBSCRIBER_CANCELLED));
+	OSMO_ASSERT(s1->sgsn_data->mm != NULL);
+
 	/* Inject LocCancelReq GSUP message */
 	rc = rx_gsup_message(location_cancellation_req,
 			     sizeof(location_cancellation_req));
-- 
cgit v1.2.3


From d8a65536ecc6eae026898822b58f520f5ee25ac7 Mon Sep 17 00:00:00 2001
From: Jacob Erlbeck <jerlbeck@sysmocom.de>
Date: Thu, 15 Jan 2015 18:51:31 +0100
Subject: sgsn: Fix P-TMSI generator's distance of equal values

Currently sgsn_alloc_ptmsi uses rand() to get a new P-TMSI and then
sets to upper 2 MSB. Therefore there is no lower limit of the
distance between 2 identical P-TMSI.

This patch changes the implementation to discard any random value
above 2^30 and to generate a new random number in that case until a
fitting number is found (or a repetition limit is reached). This way,
all number below 2^30 within the PRNG's period are used.

Ticket: OW#1362
Sponsored-by: On-Waves ehf
---
 openbsc/src/gprs/gprs_sgsn.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/openbsc/src/gprs/gprs_sgsn.c b/openbsc/src/gprs/gprs_sgsn.c
index f3da0387a..6f706642d 100644
--- a/openbsc/src/gprs/gprs_sgsn.c
+++ b/openbsc/src/gprs/gprs_sgsn.c
@@ -403,10 +403,34 @@ uint32_t sgsn_alloc_ptmsi(void)
 {
 	struct sgsn_mm_ctx *mm;
 	uint32_t ptmsi;
-	int max_retries = 23;
+	int max_retries = 100;
 
 restart:
-	ptmsi = rand() | 0xC0000000;
+	ptmsi = rand();
+	/* Enforce that the 2 MSB are set without loosing the distance between
+	 * identical values. Since rand() has no duplicate values within a
+	 * period (because the size of the state is the same like the size of
+	 * the random value), this leads to a distance of period/4 when the
+	 * distribution of the 2 MSB is uniform. This approach fails with a
+	 * probability of (3/4)^max_retries, only 1% of the approaches will
+	 * need more than 16 numbers (even distribution assumed).
+	 *
+	 * Alternatively, a freeze list could be used if another PRNG is used
+	 * or when this approach proves to be not sufficient.
+	 */
+	if (ptmsi >= 0xC0000000) {
+		if (!max_retries--)
+			goto failed;
+		goto restart;
+	}
+	ptmsi |= 0xC0000000;
+
+	if (ptmsi == GSM_RESERVED_TMSI) {
+		if (!max_retries--)
+			goto failed;
+		goto restart;
+	}
+
 	llist_for_each_entry(mm, &sgsn_mm_ctxts, list) {
 		if (mm->p_tmsi == ptmsi) {
 			if (!max_retries--)
-- 
cgit v1.2.3


From 6be9ffa3b811311043194d04ae06e05d216c57bb Mon Sep 17 00:00:00 2001
From: Jacob Erlbeck <jerlbeck@sysmocom.de>
Date: Mon, 19 Jan 2015 08:57:07 +0100
Subject: sgsn/test: Make assert_substr safer (Coverity)

Currently, if assert_subscr were called with subscr == NULL, the
later call to subscr_put might fail, as Coverity has complained. In
addition, the call to subscr_put would free the subscr object if it
were in the cache with a refcount of 0 at the time assert_substr was
called.

This patch adds a check for the subscr being non-NULL and reorders
the checks, so that the subscr_put comes last.

Fixes: Coverity CID 1264590

Sponsored-by: On-Waves ehf
---
 openbsc/tests/sgsn/sgsn_test.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/openbsc/tests/sgsn/sgsn_test.c b/openbsc/tests/sgsn/sgsn_test.c
index 662227d65..1ae94b0dd 100644
--- a/openbsc/tests/sgsn/sgsn_test.c
+++ b/openbsc/tests/sgsn/sgsn_test.c
@@ -203,12 +203,13 @@ void my_dummy_sgsn_update_subscriber_data(struct sgsn_mm_ctx *mmctx,
 static void assert_subscr(const struct gsm_subscriber *subscr, const char *imsi)
 {
 	struct gsm_subscriber *sfound;
+	OSMO_ASSERT(subscr);
+	OSMO_ASSERT(strcmp(subscr->imsi, imsi) == 0);
 
 	sfound = gprs_subscr_get_by_imsi(imsi);
 	OSMO_ASSERT(sfound == subscr);
-	subscr_put(sfound);
 
-	OSMO_ASSERT(strcmp(subscr->imsi, imsi) == 0);
+	subscr_put(sfound);
 }
 
 static void show_subscrs(FILE *out)
-- 
cgit v1.2.3


From 2585620857a3a6c17b17a65a3d9a863824b8e401 Mon Sep 17 00:00:00 2001
From: Jacob Erlbeck <jerlbeck@sysmocom.de>
Date: Mon, 19 Jan 2015 09:13:05 +0100
Subject: sgsn: Fix access to subscr in sgsn_auth_update (Coverity)

Currently the access to subscr->sgsn_data->error_cause is not
protected against subscr == NULL like it is done in other code paths
of sgsn_auth_update.

This commit adds a conditional to avoid a NULL-dereference.

Fixes: Coverity CID 1264589

Sponsored-by: On-Waves ehf
---
 openbsc/src/gprs/sgsn_auth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/openbsc/src/gprs/sgsn_auth.c b/openbsc/src/gprs/sgsn_auth.c
index 9f526dcc1..41f7c4157 100644
--- a/openbsc/src/gprs/sgsn_auth.c
+++ b/openbsc/src/gprs/sgsn_auth.c
@@ -257,7 +257,7 @@ void sgsn_auth_update(struct sgsn_mm_ctx *mmctx)
 		gsm0408_gprs_access_granted(mmctx);
 		break;
 	case SGSN_AUTH_REJECTED:
-		gmm_cause = subscr->sgsn_data->error_cause;
+		gmm_cause = subscr ? subscr->sgsn_data->error_cause : 0;
 
 		if (subscr && (subscr->flags & GPRS_SUBSCRIBER_CANCELLED) != 0)
 			gsm0408_gprs_access_cancelled(mmctx, gmm_cause);
-- 
cgit v1.2.3


From d6267d12d8bd945f219b3d2f7bf04060fe2d83bd Mon Sep 17 00:00:00 2001
From: Jacob Erlbeck <jerlbeck@sysmocom.de>
Date: Mon, 19 Jan 2015 11:10:04 +0100
Subject: sgsn: Add SGSN_ERROR_CAUSE_NONE and use it instead of 0

Currently an error_cause of 0 is being used to indicate normal
operation. Albeit this is not a defined GMM cause, the value is not
explicitly reserved.

This commit adds the macro SGSN_ERROR_CAUSE_NONE and uses it for
initialisation (instead of relying on talloc_zero) and comparisons.
The value is set to -1 to be on the safe side. The VTY code is
updated to set the error_cause when using the
'update-subscriber imsi IMSI update-location-result CAUSE' command.

Sponsored-by: On-Waves ehf
---
 openbsc/include/openbsc/gprs_sgsn.h |  2 ++
 openbsc/src/gprs/gprs_gmm.c         |  4 ++--
 openbsc/src/gprs/gprs_subscriber.c  | 10 ++++++----
 openbsc/src/gprs/sgsn_auth.c        |  4 +++-
 openbsc/src/gprs/sgsn_vty.c         | 18 ++++++++++++++++--
 5 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/openbsc/include/openbsc/gprs_sgsn.h b/openbsc/include/openbsc/gprs_sgsn.h
index 25810ab3a..00cf5ccef 100644
--- a/openbsc/include/openbsc/gprs_sgsn.h
+++ b/openbsc/include/openbsc/gprs_sgsn.h
@@ -288,6 +288,8 @@ struct sgsn_subscriber_data {
 	enum sgsn_subscriber_proc blocked_by;
 };
 
+#define SGSN_ERROR_CAUSE_NONE (-1)
+
 #define LOGGSUBSCRP(level, subscr, fmt, args...) \
 	LOGP(DGPRS, level, "SUBSCR(%s) " fmt, \
 	     (subscr) ? (subscr)->imsi : "---", \
diff --git a/openbsc/src/gprs/gprs_gmm.c b/openbsc/src/gprs/gprs_gmm.c
index 8b7cc9f17..32fb8e49d 100644
--- a/openbsc/src/gprs/gprs_gmm.c
+++ b/openbsc/src/gprs/gprs_gmm.c
@@ -654,7 +654,7 @@ void gsm0408_gprs_access_granted(struct sgsn_mm_ctx *ctx)
 
 void gsm0408_gprs_access_denied(struct sgsn_mm_ctx *ctx, int gmm_cause)
 {
-	if (gmm_cause == 0)
+	if (gmm_cause == SGSN_ERROR_CAUSE_NONE)
 		gmm_cause = GMM_CAUSE_GPRS_NOTALLOWED;
 
 	switch (ctx->mm_state) {
@@ -690,7 +690,7 @@ void gsm0408_gprs_access_denied(struct sgsn_mm_ctx *ctx, int gmm_cause)
 
 void gsm0408_gprs_access_cancelled(struct sgsn_mm_ctx *ctx, int gmm_cause)
 {
-	if (gmm_cause != 0) {
+	if (gmm_cause != SGSN_ERROR_CAUSE_NONE) {
 		LOGMMCTXP(LOGL_INFO, ctx,
 			  "Cancelled with cause '%s' (%d), deleting context\n",
 			  get_value_string(gsm48_gmm_cause_names, gmm_cause),
diff --git a/openbsc/src/gprs/gprs_subscriber.c b/openbsc/src/gprs/gprs_subscriber.c
index a30729d4d..5bde6a090 100644
--- a/openbsc/src/gprs/gprs_subscriber.c
+++ b/openbsc/src/gprs/gprs_subscriber.c
@@ -167,6 +167,8 @@ static struct sgsn_subscriber_data *sgsn_subscriber_data_alloc(void *ctx)
 
 	sdata = talloc_zero(ctx, struct sgsn_subscriber_data);
 
+	sdata->error_cause = SGSN_ERROR_CAUSE_NONE;
+
 	for (idx = 0; idx < ARRAY_SIZE(sdata->auth_triplets); idx++)
 	     sdata->auth_triplets[idx].key_seq = GSM_KEY_SEQ_INVAL;
 
@@ -292,7 +294,7 @@ static int gprs_subscr_handle_gsup_auth_res(struct gsm_subscriber *subscr,
 	}
 
 	sdata->auth_triplets_updated = 1;
-	sdata->error_cause = 0;
+	sdata->error_cause = SGSN_ERROR_CAUSE_NONE;
 
 	gprs_subscr_update_auth_info(subscr);
 
@@ -322,7 +324,7 @@ static int gprs_subscr_handle_gsup_upd_loc_res(struct gsm_subscriber *subscr,
 	}
 
 	subscr->authorized = 1;
-	subscr->sgsn_data->error_cause = 0;
+	subscr->sgsn_data->error_cause = SGSN_ERROR_CAUSE_NONE;
 
 	subscr->flags |= GPRS_SUBSCRIBER_ENABLE_PURGE;
 
@@ -451,7 +453,7 @@ static int gprs_subscr_handle_gsup_purge_res(struct gsm_subscriber *subscr,
 	LOGGSUBSCRP(LOGL_INFO, subscr, "Completing purge MS\n");
 
 	/* Force silent cancellation */
-	subscr->sgsn_data->error_cause = 0;
+	subscr->sgsn_data->error_cause = SGSN_ERROR_CAUSE_NONE;
 	gprs_subscr_put_and_cancel(subscr_get(subscr));
 
 	return 0;
@@ -500,7 +502,7 @@ static int gprs_subscr_handle_loc_cancel_req(struct gsm_subscriber *subscr,
 	gsup_reply.message_type = GPRS_GSUP_MSGT_LOCATION_CANCEL_RESULT;
 	gprs_subscr_tx_gsup_message(subscr, &gsup_reply);
 
-	subscr->sgsn_data->error_cause = 0;
+	subscr->sgsn_data->error_cause = SGSN_ERROR_CAUSE_NONE;
 	gprs_subscr_put_and_cancel(subscr_get(subscr));
 
 	return 0;
diff --git a/openbsc/src/gprs/sgsn_auth.c b/openbsc/src/gprs/sgsn_auth.c
index 41f7c4157..d77a02194 100644
--- a/openbsc/src/gprs/sgsn_auth.c
+++ b/openbsc/src/gprs/sgsn_auth.c
@@ -257,7 +257,9 @@ void sgsn_auth_update(struct sgsn_mm_ctx *mmctx)
 		gsm0408_gprs_access_granted(mmctx);
 		break;
 	case SGSN_AUTH_REJECTED:
-		gmm_cause = subscr ? subscr->sgsn_data->error_cause : 0;
+		gmm_cause =
+			subscr ? subscr->sgsn_data->error_cause :
+			SGSN_ERROR_CAUSE_NONE;
 
 		if (subscr && (subscr->flags & GPRS_SUBSCRIBER_CANCELLED) != 0)
 			gsm0408_gprs_access_cancelled(mmctx, gmm_cause);
diff --git a/openbsc/src/gprs/sgsn_vty.c b/openbsc/src/gprs/sgsn_vty.c
index ef4c8d82e..84fd5ef52 100644
--- a/openbsc/src/gprs/sgsn_vty.c
+++ b/openbsc/src/gprs/sgsn_vty.c
@@ -644,15 +644,29 @@ DEFUN(update_subscr_update_location_result, update_subscr_update_location_result
 
 	struct gsm_subscriber *subscr;
 
+	const struct value_string cause_mapping[] = {
+		{ GMM_CAUSE_NET_FAIL,		"system-failure" },
+		{ GMM_CAUSE_INV_MAND_INFO,	"data-missing" },
+		{ GMM_CAUSE_PROTO_ERR_UNSPEC,   "unexpected-data-value" },
+		{ GMM_CAUSE_IMSI_UNKNOWN,       "unknown-subscriber" },
+		{ GMM_CAUSE_GPRS_NOTALLOWED,    "roaming-not-allowed" },
+		{ 0, NULL }
+	};
+
 	subscr = gprs_subscr_get_by_imsi(imsi);
 	if (!subscr) {
 		vty_out(vty, "%% unable to get subscriber record for %s\n", imsi);
 		return CMD_WARNING;
 	}
-	if (strcmp(ret_code_str, "ok") == 0)
+
+	if (strcmp(ret_code_str, "ok") == 0) {
+		subscr->sgsn_data->error_cause = SGSN_ERROR_CAUSE_NONE;
 		subscr->authorized = 1;
-	else
+	} else {
+		subscr->sgsn_data->error_cause =
+			get_string_value(cause_mapping, ret_code_str);
 		subscr->authorized = 0;
+	}
 
 	gprs_subscr_update(subscr);
 
-- 
cgit v1.2.3


From 15cc8c812553e60422ce1917ba8e4230d7eab0b4 Mon Sep 17 00:00:00 2001
From: Jacob Erlbeck <jerlbeck@sysmocom.de>
Date: Mon, 19 Jan 2015 14:29:43 +0100
Subject: sgsn: Fix vty_out newlines

Currently '\n' is used to end lines in the VTY output string
constants instead of inserting VTY_NEWLINE. This leads to incorrect
line starts in error messages.

This patch fixes that accordingly.

Sponsored-by: On-Waves ehf
---
 openbsc/src/gprs/sgsn_vty.c | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/openbsc/src/gprs/sgsn_vty.c b/openbsc/src/gprs/sgsn_vty.c
index 84fd5ef52..a22f70c38 100644
--- a/openbsc/src/gprs/sgsn_vty.c
+++ b/openbsc/src/gprs/sgsn_vty.c
@@ -373,7 +373,8 @@ DEFUN(imsi_acl, cfg_imsi_acl_cmd,
 		rc = sgsn_acl_del(imsi, g_cfg);
 
 	if (rc < 0) {
-		vty_out(vty, "%% unable to %s ACL\n", op);
+		vty_out(vty, "%% unable to %s ACL%s", op, VTY_NEWLINE);
+
 		return CMD_WARNING;
 	}
 
@@ -516,7 +517,8 @@ DEFUN(update_subscr_insert, update_subscr_insert_cmd,
 
 	subscr = gprs_subscr_get_or_create(imsi);
 	if (!subscr) {
-		vty_out(vty, "%% unable get subscriber record for %s\n", imsi);
+		vty_out(vty, "%% unable get subscriber record for %s%s",
+			imsi, VTY_NEWLINE);
 		return CMD_WARNING;
 	}
 
@@ -548,22 +550,26 @@ DEFUN(update_subscr_insert_auth_triplet, update_subscr_insert_auth_triplet_cmd,
 
 	subscr = gprs_subscr_get_or_create(imsi);
 	if (!subscr) {
-		vty_out(vty, "%% unable get subscriber record for %s\n", imsi);
+		vty_out(vty, "%% unable get subscriber record for %s%s",
+			imsi, VTY_NEWLINE);
 		return CMD_WARNING;
 	}
 
 	OSMO_ASSERT(subscr->sgsn_data);
 
 	if (osmo_hexparse(sres_str, &at.sres[0], sizeof(at.sres)) < 0) {
-		vty_out(vty, "%% invalid SRES value '%s'\n", sres_str);
+		vty_out(vty, "%% invalid SRES value '%s'%s",
+			sres_str, VTY_NEWLINE);
 		goto failed;
 	}
 	if (osmo_hexparse(rand_str, &at.rand[0], sizeof(at.rand)) < 0) {
-		vty_out(vty, "%% invalid RAND value '%s'\n", rand_str);
+		vty_out(vty, "%% invalid RAND value '%s'%s",
+			rand_str, VTY_NEWLINE);
 		goto failed;
 	}
 	if (osmo_hexparse(kc_str, &at.kc[0], sizeof(at.kc)) < 0) {
-		vty_out(vty, "%% invalid Kc value '%s'\n", kc_str);
+		vty_out(vty, "%% invalid Kc value '%s'%s",
+			kc_str, VTY_NEWLINE);
 		goto failed;
 	}
 	at.key_seq = cksn;
@@ -591,7 +597,8 @@ DEFUN(update_subscr_cancel, update_subscr_cancel_cmd,
 
 	subscr = gprs_subscr_get_by_imsi(imsi);
 	if (!subscr) {
-		vty_out(vty, "%% no subscriber record for %s\n", imsi);
+		vty_out(vty, "%% no subscriber record for %s%s",
+			imsi, VTY_NEWLINE);
 		return CMD_WARNING;
 	}
 
@@ -610,8 +617,9 @@ DEFUN(update_subscr_commit, update_subscr_commit_cmd,
 	struct gsm_subscriber *subscr;
 
 	subscr = gprs_subscr_get_by_imsi(imsi);
-	if (!subscr) {
-		vty_out(vty, "%% unable to get subscriber record for %s\n", imsi);
+	if (subscr) {
+		vty_out(vty, "%% subscriber record already exists for %s%s",
+			imsi, VTY_NEWLINE);
 		return CMD_WARNING;
 	}
 
@@ -655,7 +663,8 @@ DEFUN(update_subscr_update_location_result, update_subscr_update_location_result
 
 	subscr = gprs_subscr_get_by_imsi(imsi);
 	if (!subscr) {
-		vty_out(vty, "%% unable to get subscriber record for %s\n", imsi);
+		vty_out(vty, "%% unable to get subscriber record for %s%s",
+			imsi, VTY_NEWLINE);
 		return CMD_WARNING;
 	}
 
@@ -686,7 +695,8 @@ DEFUN(update_subscr_update_auth_info, update_subscr_update_auth_info_cmd,
 
 	subscr = gprs_subscr_get_by_imsi(imsi);
 	if (!subscr) {
-		vty_out(vty, "%% unable to get subscriber record for %s\n", imsi);
+		vty_out(vty, "%% unable to get subscriber record for %s%s",
+			imsi, VTY_NEWLINE);
 		return CMD_WARNING;
 	}
 
-- 
cgit v1.2.3


From d91934357fe28e5362da600e61fd6473f33ff62b Mon Sep 17 00:00:00 2001
From: Jacob Erlbeck <jerlbeck@sysmocom.de>
Date: Mon, 19 Jan 2015 14:11:46 +0100
Subject: sgsn: Restructure the 'update-subscriber' command

This patch drops the following commands:

 - update-subscriber imsi IMSI insert authorized <0-1>
 - update-subscriber imsi IMSI commit

since they are already covered by the 'update-location-result'
sub-command, except that this command doesn't create an new entry if
none is found with the given IMSI.

It adds the following command:

 - update-subscriber imsi IMSI create

which can be used to create a new entry.

Sponsored-by: On-Waves ehf
---
 openbsc/src/gprs/sgsn_vty.c      | 41 +++++++---------------------------------
 openbsc/tests/vty_test_runner.py | 10 +++++-----
 2 files changed, 12 insertions(+), 39 deletions(-)

diff --git a/openbsc/src/gprs/sgsn_vty.c b/openbsc/src/gprs/sgsn_vty.c
index a22f70c38..18d997b00 100644
--- a/openbsc/src/gprs/sgsn_vty.c
+++ b/openbsc/src/gprs/sgsn_vty.c
@@ -503,32 +503,6 @@ DEFUN(show_subscr_cache,
 
 #define UPDATE_SUBSCR_INSERT_HELP "Insert data into the subscriber record\n"
 
-DEFUN(update_subscr_insert, update_subscr_insert_cmd,
-	UPDATE_SUBSCR_STR "insert authorized <0-1>)",
-	UPDATE_SUBSCR_HELP
-	UPDATE_SUBSCR_INSERT_HELP
-	"Authorize the subscriber to attach\n"
-	"New option value\n")
-{
-	const char *imsi = argv[0];
-	const char *value = argv[1];
-
-	struct gsm_subscriber *subscr;
-
-	subscr = gprs_subscr_get_or_create(imsi);
-	if (!subscr) {
-		vty_out(vty, "%% unable get subscriber record for %s%s",
-			imsi, VTY_NEWLINE);
-		return CMD_WARNING;
-	}
-
-	subscr->authorized = atoi(value);
-
-	subscr_put(subscr);
-
-	return CMD_SUCCESS;
-}
-
 DEFUN(update_subscr_insert_auth_triplet, update_subscr_insert_auth_triplet_cmd,
 	UPDATE_SUBSCR_STR "insert auth-triplet <1-5> sres SRES rand RAND kc KC",
 	UPDATE_SUBSCR_HELP
@@ -548,7 +522,7 @@ DEFUN(update_subscr_insert_auth_triplet, update_subscr_insert_auth_triplet_cmd,
 
 	struct gsm_subscriber *subscr;
 
-	subscr = gprs_subscr_get_or_create(imsi);
+	subscr = gprs_subscr_get_by_imsi(imsi);
 	if (!subscr) {
 		vty_out(vty, "%% unable get subscriber record for %s%s",
 			imsi, VTY_NEWLINE);
@@ -607,10 +581,10 @@ DEFUN(update_subscr_cancel, update_subscr_cancel_cmd,
 	return CMD_SUCCESS;
 }
 
-DEFUN(update_subscr_commit, update_subscr_commit_cmd,
-	UPDATE_SUBSCR_STR "commit",
+DEFUN(update_subscr_create, update_subscr_create_cmd,
+	UPDATE_SUBSCR_STR "create",
 	UPDATE_SUBSCR_HELP
-	"Apply the changes made by the insert commands\n")
+	"Create a subscriber entry\n")
 {
 	const char *imsi = argv[0];
 
@@ -623,8 +597,8 @@ DEFUN(update_subscr_commit, update_subscr_commit_cmd,
 		return CMD_WARNING;
 	}
 
-	gprs_subscr_update(subscr);
-
+	subscr = gprs_subscr_get_or_create(imsi);
+	subscr->keep_in_ram = 1;
 	subscr_put(subscr);
 
 	return CMD_SUCCESS;
@@ -757,10 +731,9 @@ int sgsn_vty_init(void)
 	install_element_ve(&show_pdpctx_all_cmd);
 	install_element_ve(&show_subscr_cache_cmd);
 
-	install_element(ENABLE_NODE, &update_subscr_insert_cmd);
 	install_element(ENABLE_NODE, &update_subscr_insert_auth_triplet_cmd);
+	install_element(ENABLE_NODE, &update_subscr_create_cmd);
 	install_element(ENABLE_NODE, &update_subscr_cancel_cmd);
-	install_element(ENABLE_NODE, &update_subscr_commit_cmd);
 	install_element(ENABLE_NODE, &update_subscr_update_location_result_cmd);
 	install_element(ENABLE_NODE, &update_subscr_update_auth_info_cmd);
 
diff --git a/openbsc/tests/vty_test_runner.py b/openbsc/tests/vty_test_runner.py
index a3cb9e580..31fadf1f8 100644
--- a/openbsc/tests/vty_test_runner.py
+++ b/openbsc/tests/vty_test_runner.py
@@ -760,14 +760,14 @@ class TestVTYSGSN(TestVTYGenericBSC):
         self.vty.enable()
         res = self.vty.command('show subscriber cache')
         self.assert_(res.find('1234567890') < 0)
-        self.assertTrue(self.vty.verify('update-subscriber imsi 1234567890 insert authorized 1', ['']))
+        self.assertTrue(self.vty.verify('update-subscriber imsi 1234567890 create', ['']))
         res = self.vty.command('show subscriber cache')
         self.assert_(res.find('1234567890') >= 0)
-        self.assert_(res.find('Authorized: 1') >= 0)
-        self.assertTrue(self.vty.verify('update-subscriber imsi 1234567890 insert authorized 0', ['']))
-        res = self.vty.command('show subscriber cache')
         self.assert_(res.find('Authorized: 0') >= 0)
-        self.assertTrue(self.vty.verify('update-subscriber imsi 1234567890 commit', ['']))
+        self.assertTrue(self.vty.verify('update-subscriber imsi 1234567890 update-location-result ok', ['']))
+        res = self.vty.command('show subscriber cache')
+        self.assert_(res.find('1234567890') >= 0)
+        self.assert_(res.find('Authorized: 1') >= 0)
         self.assertTrue(self.vty.verify('update-subscriber imsi 1234567890 cancel', ['']))
         res = self.vty.command('show subscriber cache')
         self.assert_(res.find('1234567890') < 0)
-- 
cgit v1.2.3


From 9be675ea52c2da4b7e2de3339e3d600efc8a853b Mon Sep 17 00:00:00 2001
From: Holger Hans Peter Freyther <holger@moiji-mobile.com>
Date: Wed, 21 Jan 2015 11:39:47 +0100
Subject: mgcp: Honor the rtp IP_TOS settings for Osmux

Honor the IP_TOS settings for Osmux as well. Re-use the RTP
setting as it makes sense to classify the audio packets the
same way.

Fixes: OW#1369
---
 openbsc/include/openbsc/mgcp_internal.h | 2 ++
 openbsc/src/libmgcp/mgcp_network.c      | 6 +++---
 openbsc/src/libmgcp/mgcp_osmux.c        | 1 +
 openbsc/src/libmgcp/mgcp_vty.c          | 2 +-
 4 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/openbsc/include/openbsc/mgcp_internal.h b/openbsc/include/openbsc/mgcp_internal.h
index 434b46c1f..6778064b3 100644
--- a/openbsc/include/openbsc/mgcp_internal.h
+++ b/openbsc/include/openbsc/mgcp_internal.h
@@ -253,6 +253,8 @@ void mgcp_rtp_annex_count(struct mgcp_endpoint *endp, struct mgcp_rtp_state *sta
 			const uint16_t seq, const int32_t transit,
 			const uint32_t ssrc);
 
+int mgcp_set_ip_tos(int fd, int tos);
+
 enum {
 	MGCP_DEST_NET = 0,
 	MGCP_DEST_BTS,
diff --git a/openbsc/src/libmgcp/mgcp_network.c b/openbsc/src/libmgcp/mgcp_network.c
index 37fc59aec..c3f43dd39 100644
--- a/openbsc/src/libmgcp/mgcp_network.c
+++ b/openbsc/src/libmgcp/mgcp_network.c
@@ -901,7 +901,7 @@ int mgcp_create_bind(const char *source_addr, struct osmo_fd *fd, int port)
 	return 0;
 }
 
-static int set_ip_tos(int fd, int tos)
+int mgcp_set_ip_tos(int fd, int tos)
 {
 	int ret;
 	ret = setsockopt(fd, IPPROTO_IP, IP_TOS,
@@ -925,8 +925,8 @@ static int bind_rtp(struct mgcp_config *cfg, struct mgcp_rtp_end *rtp_end, int e
 		goto cleanup1;
 	}
 
-	set_ip_tos(rtp_end->rtp.fd, cfg->endp_dscp);
-	set_ip_tos(rtp_end->rtcp.fd, cfg->endp_dscp);
+	mgcp_set_ip_tos(rtp_end->rtp.fd, cfg->endp_dscp);
+	mgcp_set_ip_tos(rtp_end->rtcp.fd, cfg->endp_dscp);
 
 	rtp_end->rtp.when = BSC_FD_READ;
 	if (osmo_fd_register(&rtp_end->rtp) != 0) {
diff --git a/openbsc/src/libmgcp/mgcp_osmux.c b/openbsc/src/libmgcp/mgcp_osmux.c
index d5e671d06..7f61173a1 100644
--- a/openbsc/src/libmgcp/mgcp_osmux.c
+++ b/openbsc/src/libmgcp/mgcp_osmux.c
@@ -420,6 +420,7 @@ int osmux_init(int role, struct mgcp_config *cfg)
 		LOGP(DMGCP, LOGL_ERROR, "cannot bind OSMUX socket\n");
 		return ret;
 	}
+	mgcp_set_ip_tos(osmux_fd.fd, cfg->endp_dscp);
 	osmux_fd.when |= BSC_FD_READ;
 
 	ret = osmo_fd_register(&osmux_fd);
diff --git a/openbsc/src/libmgcp/mgcp_vty.c b/openbsc/src/libmgcp/mgcp_vty.c
index 5124ca670..3d99c83b7 100644
--- a/openbsc/src/libmgcp/mgcp_vty.c
+++ b/openbsc/src/libmgcp/mgcp_vty.c
@@ -378,7 +378,7 @@ DEFUN(cfg_mgcp_rtp_ip_dscp,
       cfg_mgcp_rtp_ip_dscp_cmd,
       "rtp ip-dscp <0-255>",
       RTP_STR
-      "Apply IP_TOS to the audio stream\n" "The DSCP value\n")
+      "Apply IP_TOS to the audio stream (including Osmux)\n" "The DSCP value\n")
 {
 	int dscp = atoi(argv[0]);
 	g_cfg->endp_dscp = dscp;
-- 
cgit v1.2.3