sgsn: Remove the "permanent" 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.
diff --git a/openbsc/include/openbsc/gprs_sgsn.h b/openbsc/include/openbsc/gprs_sgsn.h
index 00cf5cc..6af3985 100644
--- a/openbsc/include/openbsc/gprs_sgsn.h
+++ b/openbsc/include/openbsc/gprs_sgsn.h
@@ -283,8 +283,6 @@
struct gsm_auth_tuple auth_triplets[5];
int auth_triplets_updated;
int error_cause;
- struct osmo_timer_list timer;
- int retries;
enum sgsn_subscriber_proc blocked_by;
};
@@ -316,7 +314,6 @@
#define GPRS_SUBSCRIBER_UPDATE_AUTH_INFO_PENDING (1 << 16)
#define GPRS_SUBSCRIBER_UPDATE_LOCATION_PENDING (1 << 17)
#define GPRS_SUBSCRIBER_CANCELLED (1 << 18)
-#define GPRS_SUBSCRIBER_ENABLE_PURGE (1 << 19)
#define GPRS_SUBSCRIBER_UPDATE_PENDING_MASK ( \
GPRS_SUBSCRIBER_UPDATE_LOCATION_PENDING | \
diff --git a/openbsc/include/openbsc/sgsn.h b/openbsc/include/openbsc/sgsn.h
index 8a45146..78064dd 100644
--- a/openbsc/include/openbsc/sgsn.h
+++ b/openbsc/include/openbsc/sgsn.h
@@ -7,8 +7,6 @@
#include <osmocom/gprs/gprs_ns.h>
#include <openbsc/gprs_sgsn.h>
-#define SGSN_TIMEOUT_NEVER (-1)
-
struct gprs_gsup_client;
enum sgsn_auth_policy {
@@ -33,8 +31,6 @@
struct sockaddr_in gsup_server_addr;
int gsup_server_port;
- int subscriber_expiry_timeout;
-
int require_authentication;
int require_update_location;
};
diff --git a/openbsc/src/gprs/gprs_sgsn.c b/openbsc/src/gprs/gprs_sgsn.c
index 6f70664..d94ea5d 100644
--- a/openbsc/src/gprs/gprs_sgsn.c
+++ b/openbsc/src/gprs/gprs_sgsn.c
@@ -187,6 +187,9 @@
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 @@
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 5bde6a0..5d0ee1c 100644
--- a/openbsc/src/gprs/gprs_subscriber.c
+++ b/openbsc/src/gprs/gprs_subscriber.c
@@ -89,75 +89,25 @@
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");
+ "Deleting subscriber entry\n");
+
+ /*
+ * Send a 'purge MS' message to the HLR. They might just
+ * ignore it anyway.
+ */
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;
-
- /* 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);
+ 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 @@
if (!subscr->sgsn_data)
subscr->sgsn_data = sgsn_subscriber_data_alloc(subscr);
-
- gprs_subscr_stop_timer(subscr);
-
return subscr;
}
@@ -207,11 +154,9 @@
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 @@
{
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 @@
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 @@
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 18d997b..6240446 100644
--- a/openbsc/src/gprs/sgsn_vty.c
+++ b/openbsc/src/gprs/sgsn_vty.c
@@ -151,10 +151,6 @@
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 @@
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 @@
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 @@
"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 @@
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 @@
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;
}
diff --git a/openbsc/tests/sgsn/sgsn_test.c b/openbsc/tests/sgsn/sgsn_test.c
index 1ae94b0..34ad647 100644
--- a/openbsc/tests/sgsn/sgsn_test.c
+++ b/openbsc/tests/sgsn/sgsn_test.c
@@ -46,7 +46,6 @@
.cfg = {
.gtp_statedir = "./",
.auth_policy = SGSN_AUTH_POLICY_CLOSED,
- .subscriber_expiry_timeout = SGSN_TIMEOUT_NEVER,
},
};
struct sgsn_instance *sgsn = &sgsn_inst;
@@ -218,9 +217,8 @@
llist_for_each_entry(subscr, &active_subscribers, entry) {
fprintf(out, " Subscriber: %s, "
- "use count: %d, keep: %d, timer: %d\n",
- subscr->imsi, subscr->use_count, subscr->keep_in_ram,
- osmo_timer_pending(&subscr->sgsn_data->timer));
+ "use count: %d\n",
+ subscr->imsi, subscr->use_count);
}
}
@@ -237,7 +235,6 @@
const char *imsi1 = "1234567890";
const char *imsi2 = "9876543210";
const char *imsi3 = "5656565656";
- int saved_expiry_timeout = sgsn->cfg.subscriber_expiry_timeout;
update_subscriber_data_cb = my_dummy_sgsn_update_subscriber_data;
@@ -271,25 +268,12 @@
gprs_subscr_update(s1);
OSMO_ASSERT(last_updated_subscr == s1);
- /* Because of the update, it won't be freed on delete now */
- gprs_subscr_delete(s1);
- sfound = gprs_subscr_get_by_imsi(imsi1);
- OSMO_ASSERT(sfound != NULL);
- s1 = sfound;
-
- /* Cancel it, so that delete will free it.
- * Refcount it to make sure s1 won't be freed here */
- last_updated_subscr = NULL;
- gprs_subscr_put_and_cancel(subscr_get(s1));
- OSMO_ASSERT(last_updated_subscr == s1);
-
- /* Cancelled entries are still being found */
- assert_subscr(s1, imsi1);
-
- /* Free entry 1 (GPRS_SUBSCRIBER_CANCELLED is set) */
+ /* There is no subscriber cache. Verify it */
gprs_subscr_delete(s1);
s1 = NULL;
- OSMO_ASSERT(gprs_subscr_get_by_imsi(imsi1) == NULL);
+ sfound = gprs_subscr_get_by_imsi(imsi1);
+ OSMO_ASSERT(sfound == NULL);
+
assert_subscr(s2, imsi2);
assert_subscr(s3, imsi3);
@@ -301,24 +285,9 @@
assert_subscr(s3, imsi3);
/* Try to delete entry 3 */
- OSMO_ASSERT(sgsn->cfg.subscriber_expiry_timeout == SGSN_TIMEOUT_NEVER);
gprs_subscr_delete(s3);
- assert_subscr(s3, imsi3);
- /* Process timeouts, this shouldn't delete s3 (SGSN_TIMEOUT_NEVER) */
- osmo_timers_update();
- assert_subscr(s3, imsi3);
- s3 = subscr_get(s3);
-
- /* Free entry 3 (TIMEOUT == 0) */
- sgsn->cfg.subscriber_expiry_timeout = 0;
- gprs_subscr_delete(s3);
- assert_subscr(s3, imsi3);
- /* Process timeouts, this should delete s3 */
- osmo_timers_update();
- OSMO_ASSERT(gprs_subscr_get_by_imsi(imsi1) == NULL);
- OSMO_ASSERT(gprs_subscr_get_by_imsi(imsi2) == NULL);
+ s3 = NULL;
OSMO_ASSERT(gprs_subscr_get_by_imsi(imsi3) == NULL);
- sgsn->cfg.subscriber_expiry_timeout = saved_expiry_timeout;
OSMO_ASSERT(llist_empty(&active_subscribers));
@@ -1053,18 +1022,6 @@
return 0;
};
-static void cleanup_subscr_by_imsi(const char *imsi)
-{
- struct gsm_subscriber *subscr;
-
- subscr = gprs_subscr_get_by_imsi(imsi);
- OSMO_ASSERT(subscr != NULL);
- subscr->keep_in_ram = 0;
- subscr_put(subscr);
- subscr = gprs_subscr_get_by_imsi(imsi);
- OSMO_ASSERT(subscr == NULL);
-}
-
static void test_gmm_attach_subscr(void)
{
const enum sgsn_auth_policy saved_auth_policy = sgsn->cfg.auth_policy;
@@ -1076,13 +1033,10 @@
subscr = gprs_subscr_get_or_create("123456789012345");
subscr->authorized = 1;
- subscr->keep_in_ram = 1;
- subscr_put(subscr);
printf("Auth policy 'remote': ");
test_gmm_attach(0);
-
- cleanup_subscr_by_imsi("123456789012345");
+ subscr_put(subscr);
assert_no_subscrs();
sgsn->cfg.auth_policy = saved_auth_policy;
@@ -1111,15 +1065,12 @@
subscr = gprs_subscr_get_or_create("123456789012345");
subscr->authorized = 1;
- subscr->keep_in_ram = 1;
sgsn->cfg.require_authentication = 1;
sgsn->cfg.require_update_location = 1;
- subscr_put(subscr);
printf("Auth policy 'remote', auth faked: ");
test_gmm_attach(0);
-
- cleanup_subscr_by_imsi("123456789012345");
+ subscr_put(subscr);
assert_no_subscrs();
sgsn->cfg.auth_policy = saved_auth_policy;
@@ -1154,16 +1105,13 @@
subscr = gprs_subscr_get_or_create("123456789012345");
subscr->authorized = 1;
- subscr->keep_in_ram = 1;
sgsn->cfg.require_authentication = 1;
sgsn->cfg.require_update_location = 1;
- subscr_put(subscr);
printf("Auth policy 'remote', triplet based auth: ");
test_gmm_attach(0);
-
- cleanup_subscr_by_imsi("123456789012345");
+ subscr_put(subscr);
assert_no_subscrs();
sgsn->cfg.auth_policy = saved_auth_policy;
@@ -1249,8 +1197,6 @@
printf("Auth policy 'remote', GSUP based auth: ");
test_gmm_attach(retry);
-
- cleanup_subscr_by_imsi("123456789012345");
assert_no_subscrs();
sgsn->cfg.auth_policy = saved_auth_policy;
@@ -1317,7 +1263,6 @@
struct gsm_subscriber *subscr;
sgsn_inst.cfg.auth_policy = SGSN_AUTH_POLICY_REMOTE;
- sgsn_inst.cfg.subscriber_expiry_timeout = 0;
gprs_gsup_client_send_cb = my_gprs_gsup_client_send;
sgsn->gsup_client = talloc_zero(tall_bsc_ctx, struct gprs_gsup_client);
@@ -1331,15 +1276,10 @@
test_gmm_attach(retry);
subscr = gprs_subscr_get_by_imsi("123456789012345");
- OSMO_ASSERT(subscr != NULL);
- gprs_subscr_delete(subscr);
-
- osmo_timers_update();
-
+ OSMO_ASSERT(subscr == NULL);
assert_no_subscrs();
sgsn->cfg.auth_policy = saved_auth_policy;
- sgsn_inst.cfg.subscriber_expiry_timeout = SGSN_TIMEOUT_NEVER;
gprs_gsup_client_send_cb = __real_gprs_gsup_client_send;
upd_loc_skip = 0;
auth_info_skip = 0;