pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31874 )
Change subject: bsc_subscriber: Introduce bsc_subscriber_store object ......................................................................
bsc_subscriber: Introduce bsc_subscriber_store object
This allows keeping the bsc_subscriber storage internals outside of main gsm_network code, while easily allowing making the internal implementation more complex (in order to optimize it in a follow-up commit). It is also nice since we get rid of uncommon procedures being used in this code, like allocating an llist directly as a talloc context, etc.
Change-Id: I616e8872af4ac63a6985f8900909e324ba889192 --- M include/osmocom/bsc/bsc_subscriber.h M include/osmocom/bsc/gsm_data.h M src/osmo-bsc/bsc_subscriber.c M src/osmo-bsc/bsc_vty.c M src/osmo-bsc/net_init.c M tests/subscr/bsc_subscr_test.c M tests/subscr/bsc_subscr_test.ok 7 files changed, 87 insertions(+), 57 deletions(-)
Approvals: laforge: Looks good to me, approved osmith: Looks good to me, but someone else must approve Jenkins Builder: Verified
diff --git a/include/osmocom/bsc/bsc_subscriber.h b/include/osmocom/bsc/bsc_subscriber.h index a86ea5f..7ec457b 100644 --- a/include/osmocom/bsc/bsc_subscriber.h +++ b/include/osmocom/bsc/bsc_subscriber.h @@ -12,8 +12,15 @@ struct log_target; struct gsm_bts;
+struct bsc_subscr_store { + struct llist_head bsub_list; /* list containing "struct bsc_subscr" */ +}; + +struct bsc_subscr_store *bsc_subscr_store_alloc(void *ctx); + struct bsc_subscr { - struct llist_head entry; + struct bsc_subscr_store *store; /* backpointer to "struct bsc_subscr_store" */ + struct llist_head entry; /* entry in (struct bsc_subscr_store)->bsub_list */ struct osmo_use_count use_count;
char imsi[GSM23003_IMSI_MAX_DIGITS+1]; @@ -28,16 +35,17 @@ const char *bsc_subscr_name(struct bsc_subscr *bsub); const char *bsc_subscr_id(struct bsc_subscr *bsub);
-struct bsc_subscr *bsc_subscr_find_or_create_by_imsi(struct llist_head *list, +struct bsc_subscr *bsc_subscr_find_or_create_by_imsi(struct bsc_subscr_store *bsubst, const char *imsi, const char *use_token); -struct bsc_subscr *bsc_subscr_find_or_create_by_tmsi(struct llist_head *list, +struct bsc_subscr *bsc_subscr_find_or_create_by_tmsi(struct bsc_subscr_store *bsubst, uint32_t tmsi, const char *use_token); -struct bsc_subscr *bsc_subscr_find_or_create_by_mi(struct llist_head *list, const struct osmo_mobile_identity *mi, +struct bsc_subscr *bsc_subscr_find_or_create_by_mi(struct bsc_subscr_store *bsubst, + const struct osmo_mobile_identity *mi, const char *use_token);
-struct bsc_subscr *bsc_subscr_find_by_imsi(struct llist_head *list, +struct bsc_subscr *bsc_subscr_find_by_imsi(struct bsc_subscr_store *bsubst, const char *imsi, const char *use_token);
diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h index 0c5b485..dc7ee3f 100644 --- a/include/osmocom/bsc/gsm_data.h +++ b/include/osmocom/bsc/gsm_data.h @@ -954,13 +954,8 @@ * OsmoMSC, this should be tied to the location area code (LAC). */ struct gsm_tz tz;
- /* List of all struct bsc_subscr used in libbsc. This llist_head is - * allocated so that the llist_head pointer itself can serve as a - * talloc context (useful to not have to pass the entire gsm_network - * struct to the bsc_subscr_* API, and for bsc_susbscr unit tests to - * not require gsm_data.h). In an MSC-without-BSC environment, this - * pointer is NULL to indicate absence of a bsc_subscribers list. */ - struct llist_head *bsc_subscribers; + /* Keeps track of struct bsc_subcr. */ + struct bsc_subscr_store *bsc_subscribers;
/* Timer for periodic channel load measurements to maintain each BTS's T3122. */ struct osmo_timer_list t3122_chan_load_timer; diff --git a/src/osmo-bsc/bsc_subscriber.c b/src/osmo-bsc/bsc_subscriber.c index 6815ab5..65c5c87 100644 --- a/src/osmo-bsc/bsc_subscriber.c +++ b/src/osmo-bsc/bsc_subscriber.c @@ -65,14 +65,27 @@ return 0; }
-static struct bsc_subscr *bsc_subscr_alloc(struct llist_head *list) +struct bsc_subscr_store *bsc_subscr_store_alloc(void *ctx) +{ + struct bsc_subscr_store *bsubst; + + bsubst = talloc_zero(ctx, struct bsc_subscr_store); + if (!bsubst) + return NULL; + + INIT_LLIST_HEAD(&bsubst->bsub_list); + return bsubst; +} + +static struct bsc_subscr *bsc_subscr_alloc(struct bsc_subscr_store *bsubst) { struct bsc_subscr *bsub;
- bsub = talloc_zero(list, struct bsc_subscr); + bsub = talloc_zero(bsubst, struct bsc_subscr); if (!bsub) return NULL;
+ bsub->store = bsubst; bsub->tmsi = GSM_RESERVED_TMSI; bsub->use_count = (struct osmo_use_count){ .talloc_object = bsub, @@ -80,12 +93,12 @@ }; INIT_LLIST_HEAD(&bsub->active_paging_requests);
- llist_add_tail(&bsub->entry, list); + llist_add_tail(&bsub->entry, &bsubst->bsub_list);
return bsub; }
-struct bsc_subscr *bsc_subscr_find_by_imsi(struct llist_head *list, +struct bsc_subscr *bsc_subscr_find_by_imsi(struct bsc_subscr_store *bsubst, const char *imsi, const char *use_token) { @@ -94,7 +107,7 @@ if (!imsi || !*imsi) return NULL;
- llist_for_each_entry(bsub, list, entry) { + llist_for_each_entry(bsub, &bsubst->bsub_list, entry) { if (!strcmp(bsub->imsi, imsi)) { bsc_subscr_get(bsub, use_token); return bsub; @@ -103,7 +116,7 @@ return NULL; }
-static struct bsc_subscr *bsc_subscr_find_by_imei(struct llist_head *list, +static struct bsc_subscr *bsc_subscr_find_by_imei(struct bsc_subscr_store *bsubst, const char *imei, const char *use_token) { @@ -112,7 +125,7 @@ if (!imei || !*imei) return NULL;
- llist_for_each_entry(bsub, list, entry) { + llist_for_each_entry(bsub, &bsubst->bsub_list, entry) { if (!strcmp(bsub->imei, imei)) { bsc_subscr_get(bsub, use_token); return bsub; @@ -121,7 +134,7 @@ return NULL; }
-static struct bsc_subscr *bsc_subscr_find_by_tmsi(struct llist_head *list, +static struct bsc_subscr *bsc_subscr_find_by_tmsi(struct bsc_subscr_store *bsubst, uint32_t tmsi, const char *use_token) { @@ -130,7 +143,7 @@ if (tmsi == GSM_RESERVED_TMSI) return NULL;
- llist_for_each_entry(bsub, list, entry) { + llist_for_each_entry(bsub, &bsubst->bsub_list, entry) { if (bsub->tmsi == tmsi) { bsc_subscr_get(bsub, use_token); return bsub; @@ -153,15 +166,15 @@ osmo_strlcpy(bsub->imei, imei, sizeof(bsub->imei)); }
-struct bsc_subscr *bsc_subscr_find_or_create_by_imsi(struct llist_head *list, +struct bsc_subscr *bsc_subscr_find_or_create_by_imsi(struct bsc_subscr_store *bsubst, const char *imsi, const char *use_token) { struct bsc_subscr *bsub; - bsub = bsc_subscr_find_by_imsi(list, imsi, use_token); + bsub = bsc_subscr_find_by_imsi(bsubst, imsi, use_token); if (bsub) return bsub; - bsub = bsc_subscr_alloc(list); + bsub = bsc_subscr_alloc(bsubst); if (!bsub) return NULL; bsc_subscr_set_imsi(bsub, imsi); @@ -169,15 +182,15 @@ return bsub; }
-static struct bsc_subscr *bsc_subscr_find_or_create_by_imei(struct llist_head *list, +static struct bsc_subscr *bsc_subscr_find_or_create_by_imei(struct bsc_subscr_store *bsubst, const char *imei, const char *use_token) { struct bsc_subscr *bsub; - bsub = bsc_subscr_find_by_imei(list, imei, use_token); + bsub = bsc_subscr_find_by_imei(bsubst, imei, use_token); if (bsub) return bsub; - bsub = bsc_subscr_alloc(list); + bsub = bsc_subscr_alloc(bsubst); if (!bsub) return NULL; bsc_subscr_set_imei(bsub, imei); @@ -185,15 +198,15 @@ return bsub; }
-struct bsc_subscr *bsc_subscr_find_or_create_by_tmsi(struct llist_head *list, +struct bsc_subscr *bsc_subscr_find_or_create_by_tmsi(struct bsc_subscr_store *bsubst, uint32_t tmsi, const char *use_token) { struct bsc_subscr *bsub; - bsub = bsc_subscr_find_by_tmsi(list, tmsi, use_token); + bsub = bsc_subscr_find_by_tmsi(bsubst, tmsi, use_token); if (bsub) return bsub; - bsub = bsc_subscr_alloc(list); + bsub = bsc_subscr_alloc(bsubst); if (!bsub) return NULL; bsub->tmsi = tmsi; @@ -201,18 +214,18 @@ return bsub; }
-struct bsc_subscr *bsc_subscr_find_or_create_by_mi(struct llist_head *list, const struct osmo_mobile_identity *mi, +struct bsc_subscr *bsc_subscr_find_or_create_by_mi(struct bsc_subscr_store *bsubst, const struct osmo_mobile_identity *mi, const char *use_token) { if (!mi) return NULL; switch (mi->type) { case GSM_MI_TYPE_IMSI: - return bsc_subscr_find_or_create_by_imsi(list, mi->imsi, use_token); + return bsc_subscr_find_or_create_by_imsi(bsubst, mi->imsi, use_token); case GSM_MI_TYPE_IMEI: - return bsc_subscr_find_or_create_by_imei(list, mi->imei, use_token); + return bsc_subscr_find_or_create_by_imei(bsubst, mi->imei, use_token); case GSM_MI_TYPE_TMSI: - return bsc_subscr_find_or_create_by_tmsi(list, mi->tmsi, use_token); + return bsc_subscr_find_or_create_by_tmsi(bsubst, mi->tmsi, use_token); default: return NULL; } diff --git a/src/osmo-bsc/bsc_vty.c b/src/osmo-bsc/bsc_vty.c index c219ff4..e860888 100644 --- a/src/osmo-bsc/bsc_vty.c +++ b/src/osmo-bsc/bsc_vty.c @@ -3285,7 +3285,7 @@ vty_out(vty, " IMSI TMSI Use%s", VTY_NEWLINE); /* " 001010123456789 ffffffff 1" */
- llist_for_each_entry(bsc_subscr, bsc_gsmnet->bsc_subscribers, entry) + llist_for_each_entry(bsc_subscr, &bsc_gsmnet->bsc_subscribers->bsub_list, entry) dump_one_sub(vty, bsc_subscr);
return CMD_SUCCESS; diff --git a/src/osmo-bsc/net_init.c b/src/osmo-bsc/net_init.c index 8a508dd..5a7422c 100644 --- a/src/osmo-bsc/net_init.c +++ b/src/osmo-bsc/net_init.c @@ -119,8 +119,7 @@
INIT_LLIST_HEAD(&net->subscr_conns);
- net->bsc_subscribers = talloc_zero(net, struct llist_head); - INIT_LLIST_HEAD(net->bsc_subscribers); + net->bsc_subscribers = bsc_subscr_store_alloc(net);
INIT_LLIST_HEAD(&net->bts_list); net->num_bts = 0; diff --git a/tests/subscr/bsc_subscr_test.c b/tests/subscr/bsc_subscr_test.c index 14e069d..27dc93e 100644 --- a/tests/subscr/bsc_subscr_test.c +++ b/tests/subscr/bsc_subscr_test.c @@ -29,7 +29,7 @@ #include <stdlib.h> #include <inttypes.h>
-struct llist_head *bsc_subscribers; +struct bsc_subscr_store *bsc_subscribers;
#define VERBOSE_ASSERT(val, expect_op, fmt) \ do { \ @@ -61,26 +61,26 @@ printf("Test BSC subscriber allocation and deletion\n");
/* Check for emptiness */ - VERBOSE_ASSERT(llist_count(bsc_subscribers), == 0, "%d"); + VERBOSE_ASSERT(llist_count(&bsc_subscribers->bsub_list), == 0, "%d"); OSMO_ASSERT(bsc_subscr_find_by_imsi(bsc_subscribers, imsi1, BSUB_USE) == NULL); OSMO_ASSERT(bsc_subscr_find_by_imsi(bsc_subscribers, imsi2, BSUB_USE) == NULL); OSMO_ASSERT(bsc_subscr_find_by_imsi(bsc_subscribers, imsi3, BSUB_USE) == NULL);
/* Allocate entry 1 */ s1 = bsc_subscr_find_or_create_by_imsi(bsc_subscribers, imsi1, BSUB_USE); - VERBOSE_ASSERT(llist_count(bsc_subscribers), == 1, "%d"); + VERBOSE_ASSERT(llist_count(&bsc_subscribers->bsub_list), == 1, "%d"); assert_bsc_subscr(s1, imsi1); - VERBOSE_ASSERT(llist_count(bsc_subscribers), == 1, "%d"); + VERBOSE_ASSERT(llist_count(&bsc_subscribers->bsub_list), == 1, "%d"); OSMO_ASSERT(bsc_subscr_find_by_imsi(bsc_subscribers, imsi2, BSUB_USE) == NULL);
/* Allocate entry 2 */ s2 = bsc_subscr_find_or_create_by_imsi(bsc_subscribers, imsi2, BSUB_USE); s2->tmsi = 0x73517351; - VERBOSE_ASSERT(llist_count(bsc_subscribers), == 2, "%d"); + VERBOSE_ASSERT(llist_count(&bsc_subscribers->bsub_list), == 2, "%d");
/* Allocate entry 3 */ s3 = bsc_subscr_find_or_create_by_imsi(bsc_subscribers, imsi3, BSUB_USE); - VERBOSE_ASSERT(llist_count(bsc_subscribers), == 3, "%d"); + VERBOSE_ASSERT(llist_count(&bsc_subscribers->bsub_list), == 3, "%d");
/* Check entries */ assert_bsc_subscr(s1, imsi1); @@ -90,7 +90,7 @@ /* Free entry 1 */ bsc_subscr_put(s1, BSUB_USE); s1 = NULL; - VERBOSE_ASSERT(llist_count(bsc_subscribers), == 2, "%d"); + VERBOSE_ASSERT(llist_count(&bsc_subscribers->bsub_list), == 2, "%d"); OSMO_ASSERT(bsc_subscr_find_by_imsi(bsc_subscribers, imsi1, BSUB_USE) == NULL);
assert_bsc_subscr(s2, imsi2); @@ -99,7 +99,7 @@ /* Free entry 2 */ bsc_subscr_put(s2, BSUB_USE); s2 = NULL; - VERBOSE_ASSERT(llist_count(bsc_subscribers), == 1, "%d"); + VERBOSE_ASSERT(llist_count(&bsc_subscribers->bsub_list), == 1, "%d"); OSMO_ASSERT(bsc_subscr_find_by_imsi(bsc_subscribers, imsi1, BSUB_USE) == NULL); OSMO_ASSERT(bsc_subscr_find_by_imsi(bsc_subscribers, imsi2, BSUB_USE) == NULL); assert_bsc_subscr(s3, imsi3); @@ -107,10 +107,10 @@ /* Free entry 3 */ bsc_subscr_put(s3, BSUB_USE); s3 = NULL; - VERBOSE_ASSERT(llist_count(bsc_subscribers), == 0, "%d"); + VERBOSE_ASSERT(llist_count(&bsc_subscribers->bsub_list), == 0, "%d"); OSMO_ASSERT(bsc_subscr_find_by_imsi(bsc_subscribers, imsi3, BSUB_USE) == NULL);
- OSMO_ASSERT(llist_empty(bsc_subscribers)); + OSMO_ASSERT(llist_empty(&bsc_subscribers->bsub_list)); }
static const struct log_info_cat log_categories[] = { @@ -137,8 +137,7 @@ log_set_print_category_hex(osmo_stderr_target, 0); log_set_print_category(osmo_stderr_target, 1);
- bsc_subscribers = talloc_zero(ctx, struct llist_head); - INIT_LLIST_HEAD(bsc_subscribers); + bsc_subscribers = bsc_subscr_store_alloc(ctx);
test_bsc_subscr();
diff --git a/tests/subscr/bsc_subscr_test.ok b/tests/subscr/bsc_subscr_test.ok index 0f6a8be..b82b096 100644 --- a/tests/subscr/bsc_subscr_test.ok +++ b/tests/subscr/bsc_subscr_test.ok @@ -1,11 +1,11 @@ Testing BSC subscriber core code. Test BSC subscriber allocation and deletion -llist_count(bsc_subscribers) == 0 -llist_count(bsc_subscribers) == 1 -llist_count(bsc_subscribers) == 1 -llist_count(bsc_subscribers) == 2 -llist_count(bsc_subscribers) == 3 -llist_count(bsc_subscribers) == 2 -llist_count(bsc_subscribers) == 1 -llist_count(bsc_subscribers) == 0 +llist_count(&bsc_subscribers->bsub_list) == 0 +llist_count(&bsc_subscribers->bsub_list) == 1 +llist_count(&bsc_subscribers->bsub_list) == 1 +llist_count(&bsc_subscribers->bsub_list) == 2 +llist_count(&bsc_subscribers->bsub_list) == 3 +llist_count(&bsc_subscribers->bsub_list) == 2 +llist_count(&bsc_subscribers->bsub_list) == 1 +llist_count(&bsc_subscribers->bsub_list) == 0 Done
-
pespin