sgsn: Remove the "permanent" subscriber cachehfreyther/changes/remove-gprs-subscriber-cache

The subscriber cache would help in case:

  * GPRS DETACH, GPRS ATTACH. In that case we might still
  have some cached authentication tuples we avoid another
  sendAuthenticationInfo request.

  * After a detach the cache expiry would make sure to
  eventually send a purgeMS to the HLR (which might be
  ignored).

At the same time to make the cache work we will need to
make sure to start and stop timers. In case we don't
start we might accumulate subscribers. I am afraid that
the above two benefits do not outweight the complexity
of this implementation.

Modify sgsn_mm_ctx_free to remove the entry from the
list as otherwise we might double free the context from
within gprs_subscriber_delete.

3 files changed, 15 insertions, 115 deletions

diff --git a/openbsc/src/gprs/gprs_sgsn.c b/openbsc/src/gprs/gprs_sgsn.c
index 6f706642d..d94ea5d7f 100644
--- a/openbsc/src/gprs/gprs_sgsn.c
+++ b/openbsc/src/gprs/gprs_sgsn.c
@@ -187,6 +187,9 @@ void sgsn_mm_ctx_free(struct sgsn_mm_ctx *mm)
 		osmo_timer_del(&mm->timer);
 	}
 
+	/* Unlink from global list of MM contexts */
+	llist_del(&mm->list);
+
 	/* Detach from subscriber which is possibly freed then */
 	if (mm->subscr) {
 		struct gsm_subscriber *subscr =  mm->subscr;
@@ -195,9 +198,6 @@ void sgsn_mm_ctx_free(struct sgsn_mm_ctx *mm)
 		gprs_subscr_delete(subscr);
 	}
 
-	/* Unlink from global list of MM contexts */
-	llist_del(&mm->list);
-
 	/* Free all PDP contexts */
 	llist_for_each_entry_safe(pdp, pdp2, &mm->pdp_list, list)
 		sgsn_pdp_ctx_free(pdp);
diff --git a/openbsc/src/gprs/gprs_subscriber.c b/openbsc/src/gprs/gprs_subscriber.c
index 5bde6a090..5d0ee1ccb 100644
--- a/openbsc/src/gprs/gprs_subscriber.c
+++ b/openbsc/src/gprs/gprs_subscriber.c
@@ -89,75 +89,25 @@ static int check_blocking(
 
 static void abort_blocking_procedure(struct gsm_subscriber *subscr)
 {
-	/* Best effort, stop retries at least */
-	subscr->sgsn_data->retries = SGSN_SUBSCR_MAX_RETRIES;
+	/* reset something */
 }
 
-static void sgsn_subscriber_timeout_cb(void *subscr_);
 int gprs_subscr_purge(struct gsm_subscriber *subscr);
 
-void gprs_subscr_stop_timer(struct gsm_subscriber *subscr)
+static void sgsn_subscriber_do_delete(struct gsm_subscriber *subscr)
 {
-	if (subscr->sgsn_data->timer.data) {
-		osmo_timer_del(&subscr->sgsn_data->timer);
-		subscr->sgsn_data->timer.cb = NULL;
-		OSMO_ASSERT(subscr->sgsn_data->timer.data == subscr);
-		subscr->sgsn_data->timer.data = NULL;
-		subscr_put(subscr);
-	}
-}
-
-void gprs_subscr_start_timer(struct gsm_subscriber *subscr, unsigned seconds)
-{
-	if (!subscr->sgsn_data->timer.data) {
-		subscr->sgsn_data->timer.cb = sgsn_subscriber_timeout_cb;
-		subscr->sgsn_data->timer.data = subscr_get(subscr);
-		subscr->sgsn_data->retries = 0;
-	}
-
-	osmo_timer_schedule(&subscr->sgsn_data->timer, seconds, 0);
-}
-
-static void sgsn_subscriber_timeout_cb(void *subscr_)
-{
-	struct gsm_subscriber *subscr = subscr_;
-
 	LOGGSUBSCRP(LOGL_INFO, subscr,
-		    "Expired, deleting subscriber entry\n");
-
-	subscr_get(subscr);
-
-	/* Check, whether to cleanup immediately */
-	if (!(subscr->flags & GPRS_SUBSCRIBER_ENABLE_PURGE) ||
-	    subscr->sgsn_data->retries >= SGSN_SUBSCR_MAX_RETRIES)
-		goto force_cleanup;
-
-	/* Send a 'purge MS' message to the HLR */
-	if (gprs_subscr_purge(subscr) < 0)
-		goto force_cleanup;
+		    "Deleting subscriber entry\n");
 
-	/* Purge request has been sent */
 
-	/* Check, whether purge is still enabled */
-	if (!(subscr->flags & GPRS_SUBSCRIBER_ENABLE_PURGE))
-		goto force_cleanup;
-
-	/* Make sure this will be tried again if there is no response in time */
-	subscr->sgsn_data->retries += 1;
-	gprs_subscr_start_timer(subscr, SGSN_SUBSCR_RETRY_INTERVAL);
+	/*
+	 * Send a 'purge MS' message to the HLR. They might just
+	 * ignore it anyway.
+	 */
+	subscr_get(subscr);
+	gprs_subscr_purge(subscr);
 	subscr_put(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);
-	/* The subscr is freed now, if the timer was the last user */
-	subscr_put(subscr);
 }
 
 static struct sgsn_subscriber_data *sgsn_subscriber_data_alloc(void *ctx)
@@ -185,9 +135,6 @@ struct gsm_subscriber *gprs_subscr_get_or_create(const char *imsi)
 
 	if (!subscr->sgsn_data)
 		subscr->sgsn_data = sgsn_subscriber_data_alloc(subscr);
-
-	gprs_subscr_stop_timer(subscr);
-
 	return subscr;
 }
 
@@ -207,11 +154,9 @@ void gprs_subscr_delete(struct gsm_subscriber *subscr)
 	if ((subscr->flags & GPRS_SUBSCRIBER_CANCELLED) ||
 	    (subscr->flags & GSM_SUBSCRIBER_FIRST_CONTACT)) {
 		subscr->keep_in_ram = 0;
-		gprs_subscr_stop_timer(subscr);
-	} else if (sgsn->cfg.subscriber_expiry_timeout != SGSN_TIMEOUT_NEVER) {
-		gprs_subscr_start_timer(subscr, sgsn->cfg.subscriber_expiry_timeout);
 	} else {
-		subscr->keep_in_ram = 1;
+		subscr->keep_in_ram = 0;
+		sgsn_subscriber_do_delete(subscr);
 	}
 
 	subscr_put(subscr);
@@ -221,10 +166,8 @@ void gprs_subscr_put_and_cancel(struct gsm_subscriber *subscr)
 {
 	subscr->authorized = 0;
 	subscr->flags |= GPRS_SUBSCRIBER_CANCELLED;
-	subscr->flags &= ~GPRS_SUBSCRIBER_ENABLE_PURGE;
 
 	gprs_subscr_update(subscr);
-
 	gprs_subscr_delete(subscr);
 }
 
@@ -325,9 +268,6 @@ static int gprs_subscr_handle_gsup_upd_loc_res(struct gsm_subscriber *subscr,
 
 	subscr->authorized = 1;
 	subscr->sgsn_data->error_cause = SGSN_ERROR_CAUSE_NONE;
-
-	subscr->flags |= GPRS_SUBSCRIBER_ENABLE_PURGE;
-
 	gprs_subscr_update(subscr);
 	return 0;
 }
@@ -716,7 +656,6 @@ struct gsm_subscriber *gprs_subscr_get_or_create_by_mmctx(struct sgsn_mm_ctx *mm
 	if (!subscr) {
 		subscr = gprs_subscr_get_or_create(mmctx->imsi);
 		subscr->flags |= GSM_SUBSCRIBER_FIRST_CONTACT;
-		subscr->flags &= ~GPRS_SUBSCRIBER_ENABLE_PURGE;
 	}
 
 	if (strcpy(subscr->equipment.imei, mmctx->imei) != 0) {
diff --git a/openbsc/src/gprs/sgsn_vty.c b/openbsc/src/gprs/sgsn_vty.c
index 18d997b00..624044666 100644
--- a/openbsc/src/gprs/sgsn_vty.c
+++ b/openbsc/src/gprs/sgsn_vty.c
@@ -151,10 +151,6 @@ static int config_write_sgsn(struct vty *vty)
 	llist_for_each_entry(acl, &g_cfg->imsi_acl, list)
 		vty_out(vty, " imsi-acl add %s%s", acl->imsi, VTY_NEWLINE);
 
-	if (g_cfg->subscriber_expiry_timeout != SGSN_TIMEOUT_NEVER)
-		vty_out(vty, " subscriber-expiry-timeout %d%s",
-			g_cfg->subscriber_expiry_timeout, VTY_NEWLINE);
-
 	return CMD_SUCCESS;
 }
 
@@ -406,7 +402,6 @@ static void subscr_dump_full_vty(struct vty *vty, struct gsm_subscriber *subscr,
 	char expire_time[200];
 	struct gsm_auth_tuple *at;
 	int at_idx;
-	struct timeval tv;
 
 	vty_out(vty, "    ID: %llu, Authorized: %d%s", subscr->id,
 		subscr->authorized, VTY_NEWLINE);
@@ -452,19 +447,8 @@ static void subscr_dump_full_vty(struct vty *vty, struct gsm_subscriber *subscr,
 		vty_out(vty, "    Expiration Time: %s%s", expire_time, VTY_NEWLINE);
 	}
 
-	/* print the expiration time if the timer is active */
-	if (osmo_timer_pending(&subscr->sgsn_data->timer)) {
-		osmo_timer_remaining(&subscr->sgsn_data->timer, NULL, &tv);
-		strftime(expire_time, sizeof(expire_time),
-			 "%a, %d %b %Y %T %z",
-			 localtime(&subscr->sgsn_data->timer.timeout.tv_sec));
-		expire_time[sizeof(expire_time) - 1] = '\0';
-		vty_out(vty, "    Expires in: %ds (%s)%s",
-			(int)tv.tv_sec, expire_time, VTY_NEWLINE);
-	}
-
 	if (subscr->flags)
-		vty_out(vty, "    Flags: %s%s%s%s%s%s",
+		vty_out(vty, "    Flags: %s%s%s%s%s",
 			subscr->flags & GSM_SUBSCRIBER_FIRST_CONTACT ?
 			"FIRST_CONTACT " : "",
 			subscr->flags & GPRS_SUBSCRIBER_CANCELLED ?
@@ -473,8 +457,6 @@ static void subscr_dump_full_vty(struct vty *vty, struct gsm_subscriber *subscr,
 			"UPDATE_LOCATION_PENDING " : "",
 			subscr->flags & GPRS_SUBSCRIBER_UPDATE_AUTH_INFO_PENDING ?
 			"AUTH_INFO_PENDING " : "",
-			subscr->flags & GPRS_SUBSCRIBER_ENABLE_PURGE ?
-			"ENABLE_PURGE " : "",
 			VTY_NEWLINE);
 
 	vty_out(vty, "    Use count: %u%s", subscr->use_count, VTY_NEWLINE);
@@ -703,25 +685,6 @@ DEFUN(cfg_gsup_remote_port, cfg_gsup_remote_port_cmd,
 	return CMD_SUCCESS;
 }
 
-DEFUN(cfg_subscriber_expiry_timeout, cfg_subscriber_expiry_timeout_cmd,
-	"subscriber-expiry-timeout <0-999999>",
-	"Set the expiry time for unused subscriber entries\n"
-	"Expiry time in seconds\n")
-{
-	g_cfg->subscriber_expiry_timeout = atoi(argv[0]);
-
-	return CMD_SUCCESS;
-}
-
-DEFUN(cfg_no_subscriber_expiry_timeout, cfg_no_subscriber_expiry_timeout_cmd,
-	"no subscriber-expiry-timeout",
-	NO_STR "Set the expiry time for unused subscriber entries\n")
-{
-	g_cfg->subscriber_expiry_timeout = atoi(argv[0]);
-
-	return CMD_SUCCESS;
-}
-
 int sgsn_vty_init(void)
 {
 	install_element_ve(&show_sgsn_cmd);
@@ -748,8 +711,6 @@ int sgsn_vty_init(void)
 	install_element(SGSN_NODE, &cfg_auth_policy_cmd);
 	install_element(SGSN_NODE, &cfg_gsup_remote_ip_cmd);
 	install_element(SGSN_NODE, &cfg_gsup_remote_port_cmd);
-	install_element(SGSN_NODE, &cfg_subscriber_expiry_timeout_cmd);
-	install_element(SGSN_NODE, &cfg_no_subscriber_expiry_timeout_cmd);
 
 	return 0;
 }