diff options
author | Neels Hofmeyr <neels@hofmeyr.de> | 2021-03-09 17:10:18 +0100 |
---|---|---|
committer | Neels Hofmeyr <neels@hofmeyr.de> | 2021-03-24 10:02:29 +0100 |
commit | d88f9a538408c69c3a39b5370197aedc8492c3fc (patch) | |
tree | abed300bdb92553a1e503d1b6c6c47d1db55d5ad /src/osmo-bsc | |
parent | 3ac1278dcfba946c67329060341385854ddd31a5 (diff) |
refactor handover penalty timers
So far the list of penalty timers was stored for an opaque target
pointer. That was either a gsm_bts pointer for a local BTS, or a cell
identifier list pointer for a remote-BSS cell.
Reasons to refactor penalty timers:
- The cell identifier list pointer came from the neighbor configuration
storage, but the way cell neighbor config is stored will change in a
subsequent patch. There will be no more cell identifier lists there.
- Storing object pointers is inherently unsafe -- if an object gets
removed and another gets allocated, the penalty timer could
theoretically remain in force for an unrelated object.
Rather store penalty timers for specific Cell IDs. Since remote-BSS
neighbors can be requested by a cell identifier *list*, use a
gsm0808_cell_id_list2 as key in the list of penalty timers.
Fix handover_test.c: have different CI for each local BTS. So far it was
the same LAC+CI for all BTSes, which now would make the test fail,
because any penalty timer would appear to apply to all local cells.
Related: OS#5018
Change-Id: I72dd6226a6d69c3f653a3174c6f55bf4eecc6885
Diffstat (limited to 'src/osmo-bsc')
-rw-r--r-- | src/osmo-bsc/bsc_subscr_conn_fsm.c | 4 | ||||
-rw-r--r-- | src/osmo-bsc/bts.c | 28 | ||||
-rw-r--r-- | src/osmo-bsc/handover_decision_2.c | 63 | ||||
-rw-r--r-- | src/osmo-bsc/handover_fsm.c | 1 | ||||
-rw-r--r-- | src/osmo-bsc/penalty_timers.c | 113 |
5 files changed, 111 insertions, 98 deletions
diff --git a/src/osmo-bsc/bsc_subscr_conn_fsm.c b/src/osmo-bsc/bsc_subscr_conn_fsm.c index ed08e86ad..954c6a5d7 100644 --- a/src/osmo-bsc/bsc_subscr_conn_fsm.c +++ b/src/osmo-bsc/bsc_subscr_conn_fsm.c @@ -933,7 +933,7 @@ static void gscon_pre_term(struct osmo_fsm_inst *fi, enum osmo_fsm_term_cause ca /* drop pending messages */ gscon_dtap_queue_flush(conn, 0); - penalty_timers_free(&conn->hodec2.penalty_timers); + penalty_timers_clear(&conn->hodec2.penalty_timers, NULL); } static int gscon_timer_cb(struct osmo_fsm_inst *fi) @@ -1004,7 +1004,7 @@ struct gsm_subscriber_connection *bsc_subscr_con_allocate(struct gsm_network *ne conn->network = net; INIT_LLIST_HEAD(&conn->dtap_queue); - /* BTW, penalty timers will be initialized on-demand. */ + INIT_LLIST_HEAD(&conn->hodec2.penalty_timers); conn->sccp.conn_id = -1; /* don't allocate from 'conn' context, as gscon_cleanup() will call talloc_free(conn) before diff --git a/src/osmo-bsc/bts.c b/src/osmo-bsc/bts.c index 950af5fb3..5a7229872 100644 --- a/src/osmo-bsc/bts.c +++ b/src/osmo-bsc/bts.c @@ -414,6 +414,34 @@ bool gsm_bts_matches_cell_id(const struct gsm_bts *bts, const struct gsm0808_cel } } +/* Return a LAC+CI cell identity for the given BTS. + * (For matching a BTS within the local BSS, the PLMN code is not important.) */ +void gsm_bts_cell_id(struct gsm0808_cell_id *cell_id, const struct gsm_bts *bts) +{ + *cell_id = (struct gsm0808_cell_id){ + .id_discr = CELL_IDENT_LAC_AND_CI, + .id.lac_and_ci = { + .lac = bts->location_area_code, + .ci = bts->cell_identity, + }, + }; +} + +/* Same as gsm_bts_cell_id(), but return in a single-entry gsm0808_cell_id_list2. Useful for e.g. + * gsm0808_cell_id_list_add() and gsm0808_cell_id_lists_same(). */ +void gsm_bts_cell_id_list(struct gsm0808_cell_id_list2 *cell_id_list, const struct gsm_bts *bts) +{ + struct gsm0808_cell_id cell_id; + struct gsm0808_cell_id_list2 add; + int rc; + gsm_bts_cell_id(&cell_id, bts); + gsm0808_cell_id_to_list(&add, &cell_id); + /* Since the target list is empty, this should always succeed. */ + (*cell_id_list) = (struct gsm0808_cell_id_list2){}; + rc = gsm0808_cell_id_list_add(cell_id_list, &add); + OSMO_ASSERT(rc > 0); +} + static struct gsm_bts_ref *gsm_bts_ref_find(const struct llist_head *list, const struct gsm_bts *bts) { struct gsm_bts_ref *ref; diff --git a/src/osmo-bsc/handover_decision_2.c b/src/osmo-bsc/handover_decision_2.c index 134f50250..0d0391cc2 100644 --- a/src/osmo-bsc/handover_decision_2.c +++ b/src/osmo-bsc/handover_decision_2.c @@ -215,49 +215,6 @@ void hodec2_on_change_congestion_check_interval(struct gsm_network *net, unsigne reinit_congestion_timer(net); } -static void _conn_penalty_time_add(struct gsm_subscriber_connection *conn, - const void *for_object, - int penalty_time) -{ - if (!for_object) { - LOGP(DHODEC, LOGL_ERROR, "%s Unable to set Handover-2 penalty timer:" - " no target cell pointer\n", - bsc_subscr_name(conn->bsub)); - return; - } - - if (!conn->hodec2.penalty_timers) { - conn->hodec2.penalty_timers = penalty_timers_init(conn); - OSMO_ASSERT(conn->hodec2.penalty_timers); - } - - penalty_timers_add(conn->hodec2.penalty_timers, for_object, penalty_time); -} - -static void nik_penalty_time_add(struct gsm_subscriber_connection *conn, - struct neighbor_ident_key *nik, - int penalty_time) -{ - _conn_penalty_time_add(conn, - neighbor_ident_get(conn->network->neighbor_bss_cells, nik), - penalty_time); -} - -static void bts_penalty_time_add(struct gsm_subscriber_connection *conn, - struct gsm_bts *bts, - int penalty_time) -{ - _conn_penalty_time_add(conn, bts, penalty_time); -} - -static unsigned int conn_penalty_time_remaining(struct gsm_subscriber_connection *conn, - const void *for_object) -{ - if (!conn->hodec2.penalty_timers) - return 0; - return penalty_timers_remaining(conn->hodec2.penalty_timers, for_object); -} - /* did we get a RXLEV for a given cell in the given report? Mark matches as MRC_F_PROCESSED. */ static struct gsm_meas_rep_cell *cell_in_rep(struct gsm_meas_rep *mr, uint16_t arfcn, uint8_t bsic) { @@ -491,6 +448,7 @@ static void check_requirements(struct ho_candidate *c) uint8_t requirement = 0; unsigned int penalty_time; int32_t current_overbooked; + struct gsm0808_cell_id target_cell_id; c->requirements = 0; /* Requirement A */ @@ -510,7 +468,8 @@ static void check_requirements(struct ho_candidate *c) } /* the handover penalty timer must not run for this bts */ - penalty_time = conn_penalty_time_remaining(c->current.lchan->conn, c->target.bts); + gsm_bts_cell_id(&target_cell_id, c->target.bts); + penalty_time = penalty_timers_remaining(&c->current.lchan->conn->hodec2.penalty_timers, &target_cell_id); if (penalty_time) { LOGPHOLCHANTOBTS(c->current.lchan, c->target.bts, LOGL_DEBUG, "not a candidate, target BTS still in penalty time" " (%u seconds left)\n", penalty_time); @@ -788,7 +747,7 @@ static void check_requirements_remote_bss(struct ho_candidate *c) /* Requirement A */ /* the handover penalty timer must not run for this bts */ - penalty_time = conn_penalty_time_remaining(c->current.lchan->conn, c->target.cil); + penalty_time = penalty_timers_remaining_list(&c->current.lchan->conn->hodec2.penalty_timers, c->target.cil); if (penalty_time) { LOGPHOLCHANTOREMOTE(c->current.lchan, c->target.cil, LOGL_DEBUG, "not a candidate, target BSS still in penalty time" @@ -1544,6 +1503,7 @@ static void on_measurement_report(struct gsm_meas_rep *mr) /* Max Distance */ if (lchan->meas_rep_count > 0 && lchan->last_ta > ho_get_hodec2_max_distance(bts->ho)) { + struct gsm0808_cell_id bts_id; global_ho_reason = HO_REASON_MAX_DISTANCE; LOGPHOLCHAN(lchan, LOGL_NOTICE, "TA is TOO HIGH: %u > %d\n", lchan->last_ta, ho_get_hodec2_max_distance(bts->ho)); @@ -1551,7 +1511,9 @@ static void on_measurement_report(struct gsm_meas_rep *mr) * early. it must be started before selecting a better cell, * so there is no assignment selected, due to running * penalty timer. */ - bts_penalty_time_add(lchan->conn, bts, ho_get_hodec2_penalty_max_dist(bts->ho)); + gsm_bts_cell_id(&bts_id, bts); + penalty_timers_add(lchan->conn, &lchan->conn->hodec2.penalty_timers, &bts_id, + ho_get_hodec2_penalty_max_dist(bts->ho)); find_alternative_lchan(lchan, true); return; } @@ -1989,7 +1951,6 @@ static void congestion_check_cb(void *arg) static void on_handover_end(struct gsm_subscriber_connection *conn, enum handover_result result) { struct gsm_bts *old_bts = NULL; - struct gsm_bts *new_bts = NULL; int penalty; struct handover *ho = &conn->ho; @@ -1999,8 +1960,6 @@ static void on_handover_end(struct gsm_subscriber_connection *conn, enum handove if (conn->lchan) old_bts = conn->lchan->ts->trx->bts; - if (ho->new_lchan) - new_bts = ho->new_lchan->ts->trx->bts; /* Only interested in handovers within this BSS or going out into another BSS. Incoming handovers * from another BSS are accounted for in the other BSS. */ @@ -2027,11 +1986,7 @@ static void on_handover_end(struct gsm_subscriber_connection *conn, enum handove LOG_HO(conn, LOGL_NOTICE, "Failed, starting penalty timer (%d s)\n", penalty); conn->hodec2.failures = 0; - - if (new_bts) - bts_penalty_time_add(conn, new_bts, penalty); - else - nik_penalty_time_add(conn, &ho->target_cell, penalty); + penalty_timers_add_list(conn, &conn->hodec2.penalty_timers, &ho->target_cell_ids, penalty); } static struct handover_decision_callbacks hodec2_callbacks = { diff --git a/src/osmo-bsc/handover_fsm.c b/src/osmo-bsc/handover_fsm.c index 87359dc74..527015236 100644 --- a/src/osmo-bsc/handover_fsm.c +++ b/src/osmo-bsc/handover_fsm.c @@ -375,6 +375,7 @@ static void handover_start_intra_bsc(struct gsm_subscriber_connection *conn) ho->scope = (ho->new_bts == bts) ? HO_INTRA_CELL : HO_INTRA_BSC; ho->ho_ref = g_next_ho_ref++; ho->async = true; + gsm_bts_cell_id_list(&ho->target_cell_ids, ho->new_bts); ho->new_lchan = lchan_select_by_type(ho->new_bts, ho->new_lchan_type); diff --git a/src/osmo-bsc/penalty_timers.c b/src/osmo-bsc/penalty_timers.c index 02cf2468a..689006114 100644 --- a/src/osmo-bsc/penalty_timers.c +++ b/src/osmo-bsc/penalty_timers.c @@ -28,16 +28,6 @@ #include <osmocom/bsc/penalty_timers.h> #include <osmocom/bsc/gsm_data.h> -struct penalty_timers { - struct llist_head timers; -}; - -struct penalty_timer { - struct llist_head entry; - const void *for_object; - unsigned int timeout; -}; - static unsigned int time_now(void) { time_t now; @@ -46,16 +36,14 @@ static unsigned int time_now(void) return (unsigned int)now; } -struct penalty_timers *penalty_timers_init(void *ctx) -{ - struct penalty_timers *pt = talloc_zero(ctx, struct penalty_timers); - if (!pt) - return NULL; - INIT_LLIST_HEAD(&pt->timers); - return pt; -} - -void penalty_timers_add(struct penalty_timers *pt, const void *for_object, int timeout) +/* Add a penalty timer for a target cell ID. + * \param ctx talloc context to allocate new struct penalty_timer from. + * \param penalty_timers llist head to add penalty timer to. + * \param for_target_cell Which handover target to penalize. + * \param timeout Penalty time in seconds. + */ +void penalty_timers_add(void *ctx, struct llist_head *penalty_timers, + const struct gsm0808_cell_id *for_target_cell, int timeout) { struct penalty_timer *timer; unsigned int now; @@ -67,9 +55,9 @@ void penalty_timers_add(struct penalty_timers *pt, const void *for_object, int t then = now + timeout; - /* timer already running for that BTS? */ - llist_for_each_entry(timer, &pt->timers, entry) { - if (timer->for_object != for_object) + /* timer already running for that target cell? */ + llist_for_each_entry(timer, penalty_timers, entry) { + if (!gsm0808_cell_ids_match(&timer->for_target_cell, for_target_cell, true)) continue; /* raise, if running timer will timeout earlier or has timed * out already, otherwise keep later timeout */ @@ -79,24 +67,49 @@ void penalty_timers_add(struct penalty_timers *pt, const void *for_object, int t } /* add new timer */ - timer = talloc_zero(pt, struct penalty_timer); + timer = talloc_zero(ctx, struct penalty_timer); if (!timer) return; - timer->for_object = for_object; + timer->for_target_cell = *for_target_cell; timer->timeout = then; - llist_add_tail(&timer->entry, &pt->timers); + llist_add_tail(&timer->entry, penalty_timers); } -unsigned int penalty_timers_remaining(struct penalty_timers *pt, const void *for_object) +/* Add a penalty timer for each target cell ID in the given list. + * \param ctx talloc context to allocate new struct penalty_timer from. + * \param penalty_timers llist head to add penalty timer to. + * \param for_target_cells Which handover targets to penalize. + * \param timeout Penalty time in seconds. + */ +void penalty_timers_add_list(void *ctx, struct llist_head *penalty_timers, + const struct gsm0808_cell_id_list2 *for_target_cells, int timeout) +{ + int i; + for (i = 0; i < for_target_cells->id_list_len; i++) { + struct gsm0808_cell_id add = { + .id_discr = for_target_cells->id_discr, + .id = for_target_cells->id_list[i], + }; + penalty_timers_add(ctx, penalty_timers, &add, timeout); + } +} + +/* Return the amount of penalty time in seconds remaining for a target cell. + * \param penalty_timers llist head to look up penalty time in. + * \param for_target_cell Which handover target to query. + * \returns seconds remaining until all penalty time has expired. + */ +unsigned int penalty_timers_remaining(struct llist_head *penalty_timers, + const struct gsm0808_cell_id *for_target_cell) { struct penalty_timer *timer; unsigned int now = time_now(); unsigned int max_remaining = 0; - llist_for_each_entry(timer, &pt->timers, entry) { + llist_for_each_entry(timer, penalty_timers, entry) { unsigned int remaining; - if (timer->for_object != for_object) + if (!gsm0808_cell_ids_match(&timer->for_target_cell, for_target_cell, true)) continue; if (now >= timer->timeout) continue; @@ -107,23 +120,39 @@ unsigned int penalty_timers_remaining(struct penalty_timers *pt, const void *for return max_remaining; } -void penalty_timers_clear(struct penalty_timers *pt, const void *for_object) +/* Return the largest amount of penalty time in seconds remaining for any one of the given target cells. + * Call penalty_timers_remaining() for each entry of for_target_cells and return the largest value encountered. + * \param penalty_timers llist head to look up penalty time in. + * \param for_target_cells Which handover targets to query. + * \returns seconds remaining until all penalty time has expired. + */ +unsigned int penalty_timers_remaining_list(struct llist_head *penalty_timers, + const struct gsm0808_cell_id_list2 *for_target_cells) +{ + int i; + unsigned int max_remaining = 0; + for (i = 0; i < for_target_cells->id_list_len; i++) { + unsigned int remaining; + struct gsm0808_cell_id query = { + .id_discr = for_target_cells->id_discr, + .id = for_target_cells->id_list[i], + }; + remaining = penalty_timers_remaining(penalty_timers, &query); + max_remaining = OSMO_MAX(max_remaining, remaining); + } + return max_remaining; +} + +/* Clear penalty timers for one target cell, or completely clear the entire list. + * \param penalty_timers llist head to add penalty timer to. + * \param for_target_cell Which handover target to clear timers for, or NULL to clear all timers. */ +void penalty_timers_clear(struct llist_head *penalty_timers, const struct gsm0808_cell_id *for_target_cell) { struct penalty_timer *timer, *timer2; - llist_for_each_entry_safe(timer, timer2, &pt->timers, entry) { - if (for_object && timer->for_object != for_object) + llist_for_each_entry_safe(timer, timer2, penalty_timers, entry) { + if (for_target_cell && !gsm0808_cell_ids_match(&timer->for_target_cell, for_target_cell, true)) continue; llist_del(&timer->entry); talloc_free(timer); } } - -void penalty_timers_free(struct penalty_timers **pt_p) -{ - struct penalty_timers *pt = *pt_p; - if (!pt) - return; - penalty_timers_clear(pt, NULL); - talloc_free(pt); - *pt_p = NULL; -} |