<p>neels has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/23358">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">refactor handover penalty timers<br><br>So far the list of penalty timers was stored for an opaque target<br>pointer. That was either a gsm_bts pointer for a local BTS, or a cell<br>identifier list pointer for a remote-BSS cell.<br><br>Reasons to refactor penalty timers:<br><br>- The cell identifier list pointer came from the neighbor configuration<br> storage, but the way cell neighbor config is stored will change in a<br> subsequent patch. There will be no more cell identifier lists there.<br><br>- Storing object pointers is inherently unsafe -- if an object gets<br> removed and another gets allocated, the penalty timer could<br> theoretically remain in force for an unrelated object.<br><br>Rather store penalty timers for specific Cell IDs. Since remote-BSS<br>neighbors can be requested by a cell identifier *list*, use a<br>gsm0808_cell_id_list2 as key in the list of penalty timers.<br><br>Fix handover_test.c: have different CI for each local BTS. So far it was<br>the same LAC+CI for all BTSes, which now would make the test fail,<br>because any penalty timer would appear to apply to all local cells.<br><br>Related: OS#5018<br>Change-Id: I72dd6226a6d69c3f653a3174c6f55bf4eecc6885<br>---<br>M include/osmocom/bsc/bts.h<br>M include/osmocom/bsc/gsm_data.h<br>M include/osmocom/bsc/penalty_timers.h<br>M src/osmo-bsc/bsc_subscr_conn_fsm.c<br>M src/osmo-bsc/bts.c<br>M src/osmo-bsc/handover_decision_2.c<br>M src/osmo-bsc/handover_fsm.c<br>M src/osmo-bsc/penalty_timers.c<br>M tests/handover/handover_test.c<br>9 files changed, 135 insertions(+), 131 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/58/23358/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmocom/bsc/bts.h b/include/osmocom/bsc/bts.h</span><br><span>index 6b58c7e..3e3c73e 100644</span><br><span>--- a/include/osmocom/bsc/bts.h</span><br><span>+++ b/include/osmocom/bsc/bts.h</span><br><span>@@ -620,6 +620,8 @@</span><br><span> </span><br><span> bool gsm_bts_matches_lai(const struct gsm_bts *bts, const struct osmo_location_area_id *lai);</span><br><span> bool gsm_bts_matches_cell_id(const struct gsm_bts *bts, const struct gsm0808_cell_id *cell_id);</span><br><span style="color: hsl(120, 100%, 40%);">+void gsm_bts_cell_id(struct gsm0808_cell_id *cell_id, const struct gsm_bts *bts);</span><br><span style="color: hsl(120, 100%, 40%);">+void gsm_bts_cell_id_list(struct gsm0808_cell_id_list2 *cell_id_list, const struct gsm_bts *bts);</span><br><span> </span><br><span> int gsm_bts_local_neighbor_add(struct gsm_bts *bts, struct gsm_bts *neighbor);</span><br><span> int gsm_bts_local_neighbor_del(struct gsm_bts *bts, const struct gsm_bts *neighbor);</span><br><span>diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h</span><br><span>index af1c8e8..1bf21ae 100644</span><br><span>--- a/include/osmocom/bsc/gsm_data.h</span><br><span>+++ b/include/osmocom/bsc/gsm_data.h</span><br><span>@@ -207,6 +207,10 @@</span><br><span> enum gsm_chan_t new_lchan_type;</span><br><span> struct neighbor_ident_key target_cell;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ /* For inter-BSC handover, this may reflect more than one Cell ID. Must also be set for intra-BSC handover,</span><br><span style="color: hsl(120, 100%, 40%);">+ * because it is used as key for penalty timers (e.g. in handover decision 2). */</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm0808_cell_id_list2 target_cell_ids;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> uint8_t ho_ref;</span><br><span> struct gsm_bts *new_bts;</span><br><span> struct gsm_lchan *new_lchan;</span><br><span>@@ -248,7 +252,7 @@</span><br><span> </span><br><span> struct {</span><br><span> int failures;</span><br><span style="color: hsl(0, 100%, 40%);">- struct penalty_timers *penalty_timers;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct llist_head penalty_timers;</span><br><span> } hodec2;</span><br><span> </span><br><span> /* "Codec List (MSC Preferred)" as received by the BSSAP Assignment Request. 3GPP 48.008</span><br><span>diff --git a/include/osmocom/bsc/penalty_timers.h b/include/osmocom/bsc/penalty_timers.h</span><br><span>index f5d1778..d662b3c 100644</span><br><span>--- a/include/osmocom/bsc/penalty_timers.h</span><br><span>+++ b/include/osmocom/bsc/penalty_timers.h</span><br><span>@@ -2,40 +2,23 @@</span><br><span> * initially used by handover algorithm 2 to keep per-BTS timers for each subscriber connection. */</span><br><span> #pragma once</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/* Opaque struct to manage penalty timers */</span><br><span style="color: hsl(0, 100%, 40%);">-struct penalty_timers;</span><br><span style="color: hsl(120, 100%, 40%);">+#include <osmocom/gsm/gsm0808_utils.h></span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/* Initialize a list of penalty timers.</span><br><span style="color: hsl(0, 100%, 40%);">- * param ctx: talloc context to allocate in.</span><br><span style="color: hsl(0, 100%, 40%);">- * returns an empty struct penalty_timers. */</span><br><span style="color: hsl(0, 100%, 40%);">-struct penalty_timers *penalty_timers_init(void *ctx);</span><br><span style="color: hsl(120, 100%, 40%);">+struct penalty_timer {</span><br><span style="color: hsl(120, 100%, 40%);">+ struct llist_head entry;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/* Add a penalty timer for an arbitrary object.</span><br><span style="color: hsl(0, 100%, 40%);">- * Note: the ownership of for_object remains with the caller; it is handled as a mere void* value, so</span><br><span style="color: hsl(0, 100%, 40%);">- * invalid pointers can be handled without problems, while common sense dictates that invalidated</span><br><span style="color: hsl(0, 100%, 40%);">- * pointers (freed objects) should probably be removed from this list. More importantly, the pointer must</span><br><span style="color: hsl(0, 100%, 40%);">- * match any pointers used to query penalty timers, so for_object should reference some global/singleton</span><br><span style="color: hsl(0, 100%, 40%);">- * object that tends to stay around longer than the penalty timers.</span><br><span style="color: hsl(0, 100%, 40%);">- * param pt: penalty timers list as from penalty_timers_init().</span><br><span style="color: hsl(0, 100%, 40%);">- * param for_object: arbitrary pointer reference to store a penalty timer for (passing NULL is possible,</span><br><span style="color: hsl(0, 100%, 40%);">- * but note that penalty_timers_clear() will clear all timers if given for_object=NULL).</span><br><span style="color: hsl(0, 100%, 40%);">- * param timeout: penalty time in seconds. */</span><br><span style="color: hsl(0, 100%, 40%);">-void penalty_timers_add(struct penalty_timers *pt, const void *for_object, int timeout);</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm0808_cell_id for_target_cell;</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int timeout;</span><br><span style="color: hsl(120, 100%, 40%);">+};</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/* Return the amount of penalty time remaining for an object.</span><br><span style="color: hsl(0, 100%, 40%);">- * param pt: penalty timers list as from penalty_timers_init().</span><br><span style="color: hsl(0, 100%, 40%);">- * param for_object: arbitrary pointer reference to query penalty timers for.</span><br><span style="color: hsl(0, 100%, 40%);">- * returns seconds remaining until all penalty time has expired. */</span><br><span style="color: hsl(0, 100%, 40%);">-unsigned int penalty_timers_remaining(struct penalty_timers *pt, const void *for_object);</span><br><span style="color: hsl(120, 100%, 40%);">+void penalty_timers_add(void *ctx, struct llist_head *penalty_timers,</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct gsm0808_cell_id *for_target_cell, int timeout);</span><br><span style="color: hsl(120, 100%, 40%);">+void penalty_timers_add_list(void *ctx, struct llist_head *penalty_timers,</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct gsm0808_cell_id_list2 *for_target_cells, int timeout);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/* Clear penalty timers for one or all objects.</span><br><span style="color: hsl(0, 100%, 40%);">- * param pt: penalty timers list as from penalty_timers_init().</span><br><span style="color: hsl(0, 100%, 40%);">- * param for_object: arbitrary pointer reference to clear penalty time for,</span><br><span style="color: hsl(0, 100%, 40%);">- * or NULL to clear all timers. */</span><br><span style="color: hsl(0, 100%, 40%);">-void penalty_timers_clear(struct penalty_timers *pt, const void *for_object);</span><br><span style="color: hsl(120, 100%, 40%);">+unsigned int penalty_timers_remaining(struct llist_head *penalty_timers,</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct gsm0808_cell_id *for_target_cell);</span><br><span style="color: hsl(120, 100%, 40%);">+unsigned int penalty_timers_remaining_list(struct llist_head *penalty_timers,</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct gsm0808_cell_id_list2 *for_target_cells);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/* Free a struct as returned from penalty_timers_init().</span><br><span style="color: hsl(0, 100%, 40%);">- * Clear all timers from the list, deallocate the list and set the pointer to NULL.</span><br><span style="color: hsl(0, 100%, 40%);">- * param pt: pointer-to-pointer which references a struct penalty_timers as returned by</span><br><span style="color: hsl(0, 100%, 40%);">- * penalty_timers_init(); *pt_p will be set to NULL. */</span><br><span style="color: hsl(0, 100%, 40%);">-void penalty_timers_free(struct penalty_timers **pt_p);</span><br><span style="color: hsl(120, 100%, 40%);">+void penalty_timers_clear(struct llist_head *penalty_timers, const struct gsm0808_cell_id *for_target_cell);</span><br><span>diff --git a/src/osmo-bsc/bsc_subscr_conn_fsm.c b/src/osmo-bsc/bsc_subscr_conn_fsm.c</span><br><span>index ed08e86..954c6a5 100644</span><br><span>--- a/src/osmo-bsc/bsc_subscr_conn_fsm.c</span><br><span>+++ b/src/osmo-bsc/bsc_subscr_conn_fsm.c</span><br><span>@@ -933,7 +933,7 @@</span><br><span> /* drop pending messages */</span><br><span> gscon_dtap_queue_flush(conn, 0);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- penalty_timers_free(&conn->hodec2.penalty_timers);</span><br><span style="color: hsl(120, 100%, 40%);">+ penalty_timers_clear(&conn->hodec2.penalty_timers, NULL);</span><br><span> }</span><br><span> </span><br><span> static int gscon_timer_cb(struct osmo_fsm_inst *fi)</span><br><span>@@ -1004,7 +1004,7 @@</span><br><span> </span><br><span> conn->network = net;</span><br><span> INIT_LLIST_HEAD(&conn->dtap_queue);</span><br><span style="color: hsl(0, 100%, 40%);">- /* BTW, penalty timers will be initialized on-demand. */</span><br><span style="color: hsl(120, 100%, 40%);">+ INIT_LLIST_HEAD(&conn->hodec2.penalty_timers);</span><br><span> conn->sccp.conn_id = -1;</span><br><span> </span><br><span> /* don't allocate from 'conn' context, as gscon_cleanup() will call talloc_free(conn) before</span><br><span>diff --git a/src/osmo-bsc/bts.c b/src/osmo-bsc/bts.c</span><br><span>index 1d0979d..3bb4f8a 100644</span><br><span>--- a/src/osmo-bsc/bts.c</span><br><span>+++ b/src/osmo-bsc/bts.c</span><br><span>@@ -414,6 +414,34 @@</span><br><span> }</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/* Return a LAC+CI cell identity for the given BTS.</span><br><span style="color: hsl(120, 100%, 40%);">+ * (For matching a BTS within the local BSS, the PLMN code is not important.) */</span><br><span style="color: hsl(120, 100%, 40%);">+void gsm_bts_cell_id(struct gsm0808_cell_id *cell_id, const struct gsm_bts *bts)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ *cell_id = (struct gsm0808_cell_id){</span><br><span style="color: hsl(120, 100%, 40%);">+ .id_discr = CELL_IDENT_LAC_AND_CI,</span><br><span style="color: hsl(120, 100%, 40%);">+ .id.lac_and_ci = {</span><br><span style="color: hsl(120, 100%, 40%);">+ .lac = bts->location_area_code,</span><br><span style="color: hsl(120, 100%, 40%);">+ .ci = bts->cell_identity,</span><br><span style="color: hsl(120, 100%, 40%);">+ },</span><br><span style="color: hsl(120, 100%, 40%);">+ };</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+/* Same as gsm_bts_cell_id(), but return in a single-entry gsm0808_cell_id_list2. Useful for e.g.</span><br><span style="color: hsl(120, 100%, 40%);">+ * gsm0808_cell_id_list_add() and gsm0808_cell_id_lists_same(). */</span><br><span style="color: hsl(120, 100%, 40%);">+void gsm_bts_cell_id_list(struct gsm0808_cell_id_list2 *cell_id_list, const struct gsm_bts *bts)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm0808_cell_id cell_id;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm0808_cell_id_list2 add;</span><br><span style="color: hsl(120, 100%, 40%);">+ int rc;</span><br><span style="color: hsl(120, 100%, 40%);">+ gsm_bts_cell_id(&cell_id, bts);</span><br><span style="color: hsl(120, 100%, 40%);">+ gsm0808_cell_id_to_list(&add, &cell_id);</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Since the target list is empty, this should always succeed. */</span><br><span style="color: hsl(120, 100%, 40%);">+ (*cell_id_list) = (struct gsm0808_cell_id_list2){};</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = gsm0808_cell_id_list_add(cell_id_list, &add);</span><br><span style="color: hsl(120, 100%, 40%);">+ OSMO_ASSERT(rc > 0);</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> static struct gsm_bts_ref *gsm_bts_ref_find(const struct llist_head *list, const struct gsm_bts *bts)</span><br><span> {</span><br><span> struct gsm_bts_ref *ref;</span><br><span>diff --git a/src/osmo-bsc/handover_decision_2.c b/src/osmo-bsc/handover_decision_2.c</span><br><span>index 134f502..0d0391c 100644</span><br><span>--- a/src/osmo-bsc/handover_decision_2.c</span><br><span>+++ b/src/osmo-bsc/handover_decision_2.c</span><br><span>@@ -215,49 +215,6 @@</span><br><span> reinit_congestion_timer(net);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-static void _conn_penalty_time_add(struct gsm_subscriber_connection *conn,</span><br><span style="color: hsl(0, 100%, 40%);">- const void *for_object,</span><br><span style="color: hsl(0, 100%, 40%);">- int penalty_time)</span><br><span style="color: hsl(0, 100%, 40%);">-{</span><br><span style="color: hsl(0, 100%, 40%);">- if (!for_object) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DHODEC, LOGL_ERROR, "%s Unable to set Handover-2 penalty timer:"</span><br><span style="color: hsl(0, 100%, 40%);">- " no target cell pointer\n",</span><br><span style="color: hsl(0, 100%, 40%);">- bsc_subscr_name(conn->bsub));</span><br><span style="color: hsl(0, 100%, 40%);">- return;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (!conn->hodec2.penalty_timers) {</span><br><span style="color: hsl(0, 100%, 40%);">- conn->hodec2.penalty_timers = penalty_timers_init(conn);</span><br><span style="color: hsl(0, 100%, 40%);">- OSMO_ASSERT(conn->hodec2.penalty_timers);</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- penalty_timers_add(conn->hodec2.penalty_timers, for_object, penalty_time);</span><br><span style="color: hsl(0, 100%, 40%);">-}</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-static void nik_penalty_time_add(struct gsm_subscriber_connection *conn,</span><br><span style="color: hsl(0, 100%, 40%);">- struct neighbor_ident_key *nik,</span><br><span style="color: hsl(0, 100%, 40%);">- int penalty_time)</span><br><span style="color: hsl(0, 100%, 40%);">-{</span><br><span style="color: hsl(0, 100%, 40%);">- _conn_penalty_time_add(conn,</span><br><span style="color: hsl(0, 100%, 40%);">- neighbor_ident_get(conn->network->neighbor_bss_cells, nik),</span><br><span style="color: hsl(0, 100%, 40%);">- penalty_time);</span><br><span style="color: hsl(0, 100%, 40%);">-}</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-static void bts_penalty_time_add(struct gsm_subscriber_connection *conn,</span><br><span style="color: hsl(0, 100%, 40%);">- struct gsm_bts *bts,</span><br><span style="color: hsl(0, 100%, 40%);">- int penalty_time)</span><br><span style="color: hsl(0, 100%, 40%);">-{</span><br><span style="color: hsl(0, 100%, 40%);">- _conn_penalty_time_add(conn, bts, penalty_time);</span><br><span style="color: hsl(0, 100%, 40%);">-}</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-static unsigned int conn_penalty_time_remaining(struct gsm_subscriber_connection *conn,</span><br><span style="color: hsl(0, 100%, 40%);">- const void *for_object)</span><br><span style="color: hsl(0, 100%, 40%);">-{</span><br><span style="color: hsl(0, 100%, 40%);">- if (!conn->hodec2.penalty_timers)</span><br><span style="color: hsl(0, 100%, 40%);">- return 0;</span><br><span style="color: hsl(0, 100%, 40%);">- return penalty_timers_remaining(conn->hodec2.penalty_timers, for_object);</span><br><span style="color: hsl(0, 100%, 40%);">-}</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> /* did we get a RXLEV for a given cell in the given report? Mark matches as MRC_F_PROCESSED. */</span><br><span> static struct gsm_meas_rep_cell *cell_in_rep(struct gsm_meas_rep *mr, uint16_t arfcn, uint8_t bsic)</span><br><span> {</span><br><span>@@ -491,6 +448,7 @@</span><br><span> uint8_t requirement = 0;</span><br><span> unsigned int penalty_time;</span><br><span> int32_t current_overbooked;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm0808_cell_id target_cell_id;</span><br><span> c->requirements = 0;</span><br><span> </span><br><span> /* Requirement A */</span><br><span>@@ -510,7 +468,8 @@</span><br><span> }</span><br><span> </span><br><span> /* the handover penalty timer must not run for this bts */</span><br><span style="color: hsl(0, 100%, 40%);">- penalty_time = conn_penalty_time_remaining(c->current.lchan->conn, c->target.bts);</span><br><span style="color: hsl(120, 100%, 40%);">+ gsm_bts_cell_id(&target_cell_id, c->target.bts);</span><br><span style="color: hsl(120, 100%, 40%);">+ penalty_time = penalty_timers_remaining(&c->current.lchan->conn->hodec2.penalty_timers, &target_cell_id);</span><br><span> if (penalty_time) {</span><br><span> LOGPHOLCHANTOBTS(c->current.lchan, c->target.bts, LOGL_DEBUG, "not a candidate, target BTS still in penalty time"</span><br><span> " (%u seconds left)\n", penalty_time);</span><br><span>@@ -788,7 +747,7 @@</span><br><span> /* Requirement A */</span><br><span> </span><br><span> /* the handover penalty timer must not run for this bts */</span><br><span style="color: hsl(0, 100%, 40%);">- penalty_time = conn_penalty_time_remaining(c->current.lchan->conn, c->target.cil);</span><br><span style="color: hsl(120, 100%, 40%);">+ penalty_time = penalty_timers_remaining_list(&c->current.lchan->conn->hodec2.penalty_timers, c->target.cil);</span><br><span> if (penalty_time) {</span><br><span> LOGPHOLCHANTOREMOTE(c->current.lchan, c->target.cil, LOGL_DEBUG,</span><br><span> "not a candidate, target BSS still in penalty time"</span><br><span>@@ -1544,6 +1503,7 @@</span><br><span> /* Max Distance */</span><br><span> if (lchan->meas_rep_count > 0</span><br><span> && lchan->last_ta > ho_get_hodec2_max_distance(bts->ho)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm0808_cell_id bts_id;</span><br><span> global_ho_reason = HO_REASON_MAX_DISTANCE;</span><br><span> LOGPHOLCHAN(lchan, LOGL_NOTICE, "TA is TOO HIGH: %u > %d\n",</span><br><span> lchan->last_ta, ho_get_hodec2_max_distance(bts->ho));</span><br><span>@@ -1551,7 +1511,9 @@</span><br><span> * early. it must be started before selecting a better cell,</span><br><span> * so there is no assignment selected, due to running</span><br><span> * penalty timer. */</span><br><span style="color: hsl(0, 100%, 40%);">- bts_penalty_time_add(lchan->conn, bts, ho_get_hodec2_penalty_max_dist(bts->ho));</span><br><span style="color: hsl(120, 100%, 40%);">+ gsm_bts_cell_id(&bts_id, bts);</span><br><span style="color: hsl(120, 100%, 40%);">+ penalty_timers_add(lchan->conn, &lchan->conn->hodec2.penalty_timers, &bts_id,</span><br><span style="color: hsl(120, 100%, 40%);">+ ho_get_hodec2_penalty_max_dist(bts->ho));</span><br><span> find_alternative_lchan(lchan, true);</span><br><span> return;</span><br><span> }</span><br><span>@@ -1989,7 +1951,6 @@</span><br><span> static void on_handover_end(struct gsm_subscriber_connection *conn, enum handover_result result)</span><br><span> {</span><br><span> struct gsm_bts *old_bts = NULL;</span><br><span style="color: hsl(0, 100%, 40%);">- struct gsm_bts *new_bts = NULL;</span><br><span> int penalty;</span><br><span> struct handover *ho = &conn->ho;</span><br><span> </span><br><span>@@ -1999,8 +1960,6 @@</span><br><span> </span><br><span> if (conn->lchan)</span><br><span> old_bts = conn->lchan->ts->trx->bts;</span><br><span style="color: hsl(0, 100%, 40%);">- if (ho->new_lchan)</span><br><span style="color: hsl(0, 100%, 40%);">- new_bts = ho->new_lchan->ts->trx->bts;</span><br><span> </span><br><span> /* Only interested in handovers within this BSS or going out into another BSS. Incoming handovers</span><br><span> * from another BSS are accounted for in the other BSS. */</span><br><span>@@ -2027,11 +1986,7 @@</span><br><span> </span><br><span> LOG_HO(conn, LOGL_NOTICE, "Failed, starting penalty timer (%d s)\n", penalty);</span><br><span> conn->hodec2.failures = 0;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (new_bts)</span><br><span style="color: hsl(0, 100%, 40%);">- bts_penalty_time_add(conn, new_bts, penalty);</span><br><span style="color: hsl(0, 100%, 40%);">- else</span><br><span style="color: hsl(0, 100%, 40%);">- nik_penalty_time_add(conn, &ho->target_cell, penalty);</span><br><span style="color: hsl(120, 100%, 40%);">+ penalty_timers_add_list(conn, &conn->hodec2.penalty_timers, &ho->target_cell_ids, penalty);</span><br><span> }</span><br><span> </span><br><span> static struct handover_decision_callbacks hodec2_callbacks = {</span><br><span>diff --git a/src/osmo-bsc/handover_fsm.c b/src/osmo-bsc/handover_fsm.c</span><br><span>index 87359dc..5270152 100644</span><br><span>--- a/src/osmo-bsc/handover_fsm.c</span><br><span>+++ b/src/osmo-bsc/handover_fsm.c</span><br><span>@@ -375,6 +375,7 @@</span><br><span> ho->scope = (ho->new_bts == bts) ? HO_INTRA_CELL : HO_INTRA_BSC;</span><br><span> ho->ho_ref = g_next_ho_ref++;</span><br><span> ho->async = true;</span><br><span style="color: hsl(120, 100%, 40%);">+ gsm_bts_cell_id_list(&ho->target_cell_ids, ho->new_bts);</span><br><span> </span><br><span> ho->new_lchan = lchan_select_by_type(ho->new_bts, ho->new_lchan_type);</span><br><span> </span><br><span>diff --git a/src/osmo-bsc/penalty_timers.c b/src/osmo-bsc/penalty_timers.c</span><br><span>index 02cf246..6890061 100644</span><br><span>--- a/src/osmo-bsc/penalty_timers.c</span><br><span>+++ b/src/osmo-bsc/penalty_timers.c</span><br><span>@@ -28,16 +28,6 @@</span><br><span> #include <osmocom/bsc/penalty_timers.h></span><br><span> #include <osmocom/bsc/gsm_data.h></span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-struct penalty_timers {</span><br><span style="color: hsl(0, 100%, 40%);">- struct llist_head timers;</span><br><span style="color: hsl(0, 100%, 40%);">-};</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-struct penalty_timer {</span><br><span style="color: hsl(0, 100%, 40%);">- struct llist_head entry;</span><br><span style="color: hsl(0, 100%, 40%);">- const void *for_object;</span><br><span style="color: hsl(0, 100%, 40%);">- unsigned int timeout;</span><br><span style="color: hsl(0, 100%, 40%);">-};</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> static unsigned int time_now(void)</span><br><span> {</span><br><span> time_t now;</span><br><span>@@ -46,16 +36,14 @@</span><br><span> return (unsigned int)now;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-struct penalty_timers *penalty_timers_init(void *ctx)</span><br><span style="color: hsl(0, 100%, 40%);">-{</span><br><span style="color: hsl(0, 100%, 40%);">- struct penalty_timers *pt = talloc_zero(ctx, struct penalty_timers);</span><br><span style="color: hsl(0, 100%, 40%);">- if (!pt)</span><br><span style="color: hsl(0, 100%, 40%);">- return NULL;</span><br><span style="color: hsl(0, 100%, 40%);">- INIT_LLIST_HEAD(&pt->timers);</span><br><span style="color: hsl(0, 100%, 40%);">- return pt;</span><br><span style="color: hsl(0, 100%, 40%);">-}</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-void penalty_timers_add(struct penalty_timers *pt, const void *for_object, int timeout)</span><br><span style="color: hsl(120, 100%, 40%);">+/* Add a penalty timer for a target cell ID.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param ctx talloc context to allocate new struct penalty_timer from.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param penalty_timers llist head to add penalty timer to.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param for_target_cell Which handover target to penalize.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param timeout Penalty time in seconds.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+void penalty_timers_add(void *ctx, struct llist_head *penalty_timers,</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct gsm0808_cell_id *for_target_cell, int timeout)</span><br><span> {</span><br><span> struct penalty_timer *timer;</span><br><span> unsigned int now;</span><br><span>@@ -67,9 +55,9 @@</span><br><span> </span><br><span> then = now + timeout;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* timer already running for that BTS? */</span><br><span style="color: hsl(0, 100%, 40%);">- llist_for_each_entry(timer, &pt->timers, entry) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (timer->for_object != for_object)</span><br><span style="color: hsl(120, 100%, 40%);">+ /* timer already running for that target cell? */</span><br><span style="color: hsl(120, 100%, 40%);">+ llist_for_each_entry(timer, penalty_timers, entry) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!gsm0808_cell_ids_match(&timer->for_target_cell, for_target_cell, true))</span><br><span> continue;</span><br><span> /* raise, if running timer will timeout earlier or has timed</span><br><span> * out already, otherwise keep later timeout */</span><br><span>@@ -79,24 +67,49 @@</span><br><span> }</span><br><span> </span><br><span> /* add new timer */</span><br><span style="color: hsl(0, 100%, 40%);">- timer = talloc_zero(pt, struct penalty_timer);</span><br><span style="color: hsl(120, 100%, 40%);">+ timer = talloc_zero(ctx, struct penalty_timer);</span><br><span> if (!timer)</span><br><span> return;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- timer->for_object = for_object;</span><br><span style="color: hsl(120, 100%, 40%);">+ timer->for_target_cell = *for_target_cell;</span><br><span> timer->timeout = then;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- llist_add_tail(&timer->entry, &pt->timers);</span><br><span style="color: hsl(120, 100%, 40%);">+ llist_add_tail(&timer->entry, penalty_timers);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-unsigned int penalty_timers_remaining(struct penalty_timers *pt, const void *for_object)</span><br><span style="color: hsl(120, 100%, 40%);">+/* Add a penalty timer for each target cell ID in the given list.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param ctx talloc context to allocate new struct penalty_timer from.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param penalty_timers llist head to add penalty timer to.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param for_target_cells Which handover targets to penalize.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param timeout Penalty time in seconds.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+void penalty_timers_add_list(void *ctx, struct llist_head *penalty_timers,</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct gsm0808_cell_id_list2 *for_target_cells, int timeout)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ int i;</span><br><span style="color: hsl(120, 100%, 40%);">+ for (i = 0; i < for_target_cells->id_list_len; i++) {</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm0808_cell_id add = {</span><br><span style="color: hsl(120, 100%, 40%);">+ .id_discr = for_target_cells->id_discr,</span><br><span style="color: hsl(120, 100%, 40%);">+ .id = for_target_cells->id_list[i],</span><br><span style="color: hsl(120, 100%, 40%);">+ };</span><br><span style="color: hsl(120, 100%, 40%);">+ penalty_timers_add(ctx, penalty_timers, &add, timeout);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+/* Return the amount of penalty time in seconds remaining for a target cell.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param penalty_timers llist head to look up penalty time in.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param for_target_cell Which handover target to query.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \returns seconds remaining until all penalty time has expired.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+unsigned int penalty_timers_remaining(struct llist_head *penalty_timers,</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct gsm0808_cell_id *for_target_cell)</span><br><span> {</span><br><span> struct penalty_timer *timer;</span><br><span> unsigned int now = time_now();</span><br><span> unsigned int max_remaining = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- llist_for_each_entry(timer, &pt->timers, entry) {</span><br><span style="color: hsl(120, 100%, 40%);">+ llist_for_each_entry(timer, penalty_timers, entry) {</span><br><span> unsigned int remaining;</span><br><span style="color: hsl(0, 100%, 40%);">- if (timer->for_object != for_object)</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!gsm0808_cell_ids_match(&timer->for_target_cell, for_target_cell, true))</span><br><span> continue;</span><br><span> if (now >= timer->timeout)</span><br><span> continue;</span><br><span>@@ -107,23 +120,39 @@</span><br><span> return max_remaining;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-void penalty_timers_clear(struct penalty_timers *pt, const void *for_object)</span><br><span style="color: hsl(120, 100%, 40%);">+/* Return the largest amount of penalty time in seconds remaining for any one of the given target cells.</span><br><span style="color: hsl(120, 100%, 40%);">+ * Call penalty_timers_remaining() for each entry of for_target_cells and return the largest value encountered.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param penalty_timers llist head to look up penalty time in.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param for_target_cells Which handover targets to query.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \returns seconds remaining until all penalty time has expired.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+unsigned int penalty_timers_remaining_list(struct llist_head *penalty_timers,</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct gsm0808_cell_id_list2 *for_target_cells)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ int i;</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int max_remaining = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ for (i = 0; i < for_target_cells->id_list_len; i++) {</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int remaining;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm0808_cell_id query = {</span><br><span style="color: hsl(120, 100%, 40%);">+ .id_discr = for_target_cells->id_discr,</span><br><span style="color: hsl(120, 100%, 40%);">+ .id = for_target_cells->id_list[i],</span><br><span style="color: hsl(120, 100%, 40%);">+ };</span><br><span style="color: hsl(120, 100%, 40%);">+ remaining = penalty_timers_remaining(penalty_timers, &query);</span><br><span style="color: hsl(120, 100%, 40%);">+ max_remaining = OSMO_MAX(max_remaining, remaining);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ return max_remaining;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+/* Clear penalty timers for one target cell, or completely clear the entire list.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param penalty_timers llist head to add penalty timer to.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param for_target_cell Which handover target to clear timers for, or NULL to clear all timers. */</span><br><span style="color: hsl(120, 100%, 40%);">+void penalty_timers_clear(struct llist_head *penalty_timers, const struct gsm0808_cell_id *for_target_cell)</span><br><span> {</span><br><span> struct penalty_timer *timer, *timer2;</span><br><span style="color: hsl(0, 100%, 40%);">- llist_for_each_entry_safe(timer, timer2, &pt->timers, entry) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (for_object && timer->for_object != for_object)</span><br><span style="color: hsl(120, 100%, 40%);">+ llist_for_each_entry_safe(timer, timer2, penalty_timers, entry) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (for_target_cell && !gsm0808_cell_ids_match(&timer->for_target_cell, for_target_cell, true))</span><br><span> continue;</span><br><span> llist_del(&timer->entry);</span><br><span> talloc_free(timer);</span><br><span> }</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-void penalty_timers_free(struct penalty_timers **pt_p)</span><br><span style="color: hsl(0, 100%, 40%);">-{</span><br><span style="color: hsl(0, 100%, 40%);">- struct penalty_timers *pt = *pt_p;</span><br><span style="color: hsl(0, 100%, 40%);">- if (!pt)</span><br><span style="color: hsl(0, 100%, 40%);">- return;</span><br><span style="color: hsl(0, 100%, 40%);">- penalty_timers_clear(pt, NULL);</span><br><span style="color: hsl(0, 100%, 40%);">- talloc_free(pt);</span><br><span style="color: hsl(0, 100%, 40%);">- *pt_p = NULL;</span><br><span style="color: hsl(0, 100%, 40%);">-}</span><br><span>diff --git a/tests/handover/handover_test.c b/tests/handover/handover_test.c</span><br><span>index dff1390..332c94c 100644</span><br><span>--- a/tests/handover/handover_test.c</span><br><span>+++ b/tests/handover/handover_test.c</span><br><span>@@ -205,6 +205,7 @@</span><br><span> static struct gsm_bts *_create_bts(int num_trx, const char * const *ts_args, int ts_args_count)</span><br><span> {</span><br><span> static int arfcn = 870;</span><br><span style="color: hsl(120, 100%, 40%);">+ static int ci = 0;</span><br><span> struct gsm_bts *bts;</span><br><span> struct e1inp_sign_link *rsl_link;</span><br><span> int i;</span><br><span>@@ -220,6 +221,7 @@</span><br><span> }</span><br><span> </span><br><span> bts->location_area_code = 23;</span><br><span style="color: hsl(120, 100%, 40%);">+ bts->cell_identity = ci++;</span><br><span> bts->c0->arfcn = arfcn++;</span><br><span> </span><br><span> bts->codec.efr = 1;</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-bsc/+/23358">change 23358</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/osmo-bsc/+/23358"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-bsc </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I72dd6226a6d69c3f653a3174c6f55bf4eecc6885 </div>
<div style="display:none"> Gerrit-Change-Number: 23358 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>