From ebc3fe5b00e7e8c95d33ec3ddb0dd486b503d933 Mon Sep 17 00:00:00 2001 From: Neels Hofmeyr Date: Thu, 22 Feb 2018 16:48:17 +0100 Subject: WIP: suggestion to change triggering of HO: store ho info at conn Change-Id: Iebf9d1ce7df3c1a3b8cd80e1233a2891d9d2aa39 --- include/osmocom/bsc/gsm_data.h | 6 +- include/osmocom/bsc/handover.h | 22 +++-- src/libbsc/bsc_api.c | 15 ++- src/libbsc/handover_decision_2.c | 6 +- src/libbsc/handover_logic.c | 191 ++++++++++++++++--------------------- src/osmo-bsc/bsc_subscr_conn_fsm.c | 10 +- src/osmo-bsc/osmo_bsc_audio.c | 6 +- tests/bsc-nat/bsc_nat_test.c | 2 + tests/channel/channel_test.c | 1 + 9 files changed, 120 insertions(+), 139 deletions(-) diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h index 314eaa6d3..c79b6d79c 100644 --- a/include/osmocom/bsc/gsm_data.h +++ b/include/osmocom/bsc/gsm_data.h @@ -18,6 +18,7 @@ #include #include +#include #include #include @@ -105,8 +106,9 @@ struct gsm_subscriber_connection { /* the primary / currently active lchan to the BTS/subscriber */ struct gsm_lchan *lchan; - /* the future/allocated but not yet used lchan during HANDOVER */ - struct gsm_lchan *ho_lchan; + + /* handover information, if a handover is pending for this conn. */ + struct bsc_handover *ho; /* timer for assignment handling */ struct osmo_timer_list T10; diff --git a/include/osmocom/bsc/handover.h b/include/osmocom/bsc/handover.h index bb324343b..eb03f6a7b 100644 --- a/include/osmocom/bsc/handover.h +++ b/include/osmocom/bsc/handover.h @@ -4,10 +4,12 @@ #include #include +#include struct gsm_lchan; struct gsm_bts; struct gsm_subscriber_connection; +struct gsm_meas_rep mr; #define LOGPHOLCHANTOLCHAN(old_lchan, new_lchan, level, fmt, args...) \ LOGP(DHODEC, level, "(BTS %u trx %u arfcn %u ts %u lchan %u %s)->(BTS %u trx %u arfcn %u ts %u lchan %u %s) (subscr %s) " fmt, \ @@ -38,23 +40,23 @@ enum hodec_id { struct bsc_handover { struct llist_head list; - enum hodec_id from_hodec_id; - + /* Initial details of what is requested */ struct gsm_lchan *old_lchan; - struct gsm_lchan *new_lchan; - - struct osmo_timer_list T3103; - - uint8_t ho_ref; + struct gsm_bts *new_bts; + enum gsm_chan_t new_lchan_type; + bool async; + /* Derived and resulting state */ bool inter_cell; - bool async; + uint8_t ho_ref; + enum hodec_id from_hodec_id; + struct gsm_lchan *new_lchan; + struct osmo_timer_list T3103; }; int bsc_handover_start(enum hodec_id from_hodec_id, struct gsm_lchan *old_lchan, struct gsm_bts *new_bts, enum gsm_chan_t new_lchan_type); -int bsc_handover_start_gscon(struct gsm_lchan *old_lchan, struct gsm_bts *new_bts, - enum gsm_chan_t new_lchan_typ); +int bsc_handover_start_gscon(struct gsm_subscriber_connection *conn); void bsc_clear_handover(struct gsm_subscriber_connection *conn, int free_lchan); struct gsm_lchan *bsc_handover_pending(struct gsm_lchan *new_lchan); diff --git a/src/libbsc/bsc_api.c b/src/libbsc/bsc_api.c index 3c812d183..c47654778 100644 --- a/src/libbsc/bsc_api.c +++ b/src/libbsc/bsc_api.c @@ -267,7 +267,7 @@ void ho_dtap_cache_flush(struct gsm_subscriber_connection *conn, int send) struct msgb *msg; unsigned int flushed_count = 0; - if (conn->secondary_lchan || conn->ho_lchan) { + if (conn->secondary_lchan || conn->ho) { LOGP(DHO, LOGL_ERROR, "%s: Cannot send cached DTAP messages, handover/assignment is still ongoing\n", bsc_subscr_name(conn->bsub)); send = 0; @@ -308,7 +308,7 @@ int gsm0808_submit_dtap(struct gsm_subscriber_connection *conn, } /* buffer message during assignment / handover */ - if (conn->secondary_lchan || conn->ho_lchan) { + if (conn->secondary_lchan || conn->ho) { ho_dtap_cache_add(conn, msg, link_id, !! allow_sacch); return 0; } @@ -427,7 +427,7 @@ static void handle_ass_compl(struct gsm_subscriber_connection *conn, struct gsm48_hdr *gh; struct bsc_api *api = conn->network->bsc_api; - if (conn->ho_lchan) { + if (conn->ho) { struct lchan_signal_data sig; struct gsm48_hdr *gh = msgb_l3(msg); @@ -483,7 +483,7 @@ static void handle_ass_fail(struct gsm_subscriber_connection *conn, uint8_t *rr_failure; struct gsm48_hdr *gh; - if (conn->ho_lchan) { + if (conn->ho) { struct lchan_signal_data sig; struct gsm48_hdr *gh = msgb_l3(msg); @@ -772,7 +772,7 @@ int gsm0808_cipher_mode(struct gsm_subscriber_connection *conn, int cipher, */ int gsm0808_clear(struct gsm_subscriber_connection *conn) { - if (conn->ho_lchan) + if (conn->ho) bsc_clear_handover(conn, 1); if (conn->secondary_lchan) @@ -783,7 +783,6 @@ int gsm0808_clear(struct gsm_subscriber_connection *conn) conn->lchan = NULL; conn->secondary_lchan = NULL; - conn->ho_lchan = NULL; osmo_timer_del(&conn->T10); @@ -885,10 +884,8 @@ static void handle_release(struct gsm_subscriber_connection *conn, /* now give up all channels */ if (conn->lchan == lchan) conn->lchan = NULL; - if (conn->ho_lchan == lchan) { + if (conn->ho && conn->ho->new_lchan == lchan) bsc_clear_handover(conn, 0); - conn->ho_lchan = NULL; - } lchan->conn = NULL; } diff --git a/src/libbsc/handover_decision_2.c b/src/libbsc/handover_decision_2.c index 0135020a6..efce95f37 100644 --- a/src/libbsc/handover_decision_2.c +++ b/src/libbsc/handover_decision_2.c @@ -1149,7 +1149,7 @@ static void on_measurement_report(struct gsm_meas_rep *mr) LOGPHOLCHAN(lchan, LOGL_INFO, "Skipping, Initial Assignment is still ongoing\n"); return; } - if (lchan->conn->ho_lchan) { + if (lchan->conn->ho) { LOGPHOLCHAN(lchan, LOGL_INFO, "Skipping, Handover already triggered\n"); return; } @@ -1348,7 +1348,7 @@ static int bts_resolve_congestion(struct gsm_bts *bts, int tchf_congestion, int break; /* omit if there is an ongoing ho/as */ if (!lc->conn || lc->conn->secondary_lchan - || lc->conn->ho_lchan) + || lc->conn->ho) break; /* We desperately want to resolve congestion, ignore rxlev when * collecting candidates by passing include_weaker_rxlev=true. */ @@ -1364,7 +1364,7 @@ static int bts_resolve_congestion(struct gsm_bts *bts, int tchf_congestion, int /* omit of there is an ongoing ho/as */ if (!lc->conn || lc->conn->secondary_lchan - || lc->conn->ho_lchan) + || lc->conn->ho) continue; /* We desperately want to resolve congestion, ignore rxlev when * collecting candidates by passing include_weaker_rxlev=true. */ diff --git a/src/libbsc/handover_logic.c b/src/libbsc/handover_logic.c index ca29290be..8de0fd6b8 100644 --- a/src/libbsc/handover_logic.c +++ b/src/libbsc/handover_logic.c @@ -82,103 +82,105 @@ static struct bsc_handover *bsc_ho_by_old_lchan(struct gsm_lchan *old_lchan) int bsc_handover_start(enum hodec_id from_hodec_id, struct gsm_lchan *old_lchan, struct gsm_bts *new_bts, enum gsm_chan_t new_lchan_type) { - /*! Note: Due to architectural changes this function now acts as a - * wrapper function that dispatches a signal to the GSCON FSM, - * which then calls bsc_handover_start_fsm() in case handover is - * actually permitted. The FSM will then supervise the handover - * process and (if necessary) communicate changes of IP-Addresses - * back to the MGW. - * - * The implementation of a wrapper function had been prefered over - * API changes in order to keep code that makes use of - * bsc_handover_start() unchanged */ - struct gsm_subscriber_connection *conn; - - OSMO_ASSERT(old_lchan); - OSMO_ASSERT(new_bts); - - conn = old_lchan->conn; - OSMO_ASSERT(conn); - - conn->user_plane.ho_from_hodec_id = from_hodec_id; - conn->user_plane.ho_old_lchan = old_lchan; - conn->user_plane.ho_new_bts = new_bts; - conn->user_plane.ho_new_lchan_type = new_lchan_type; - - return osmo_fsm_inst_dispatch(conn->fi, GSCON_EV_HO_START, NULL); -} - -/*! \brief Actual implementation of bsc_handover_start(). This function must - * not be called directly. The only legal caller is the GSCON FSM in - * bsc_subscr_conn_fsm.c. */ -int bsc_handover_start_gscon(struct gsm_lchan *old_lchan, struct gsm_bts *new_bts, - enum gsm_chan_t new_lchan_type) -{ - struct gsm_network *network; - struct gsm_lchan *new_lchan; struct bsc_handover *ho; static uint8_t ho_ref = 0; - int rc; - bool do_assignment = false; + bool do_assignment; + + OSMO_ASSERT(old_lchan); /* don't attempt multiple handovers for the same lchan at * the same time */ if (bsc_ho_by_old_lchan(old_lchan)) return -EBUSY; + conn = old_lchan->conn; + if (!conn) { + LOGP(DHO, LOGL_ERROR, "Old lchan lacks connection data.\n"); + return -ENOSPC; + } + if (!new_bts) new_bts = old_lchan->ts->trx->bts; - do_assignment = (new_bts == old_lchan->ts->trx->bts); - - network = new_bts->network; + OSMO_ASSERT(new_bts); - rate_ctr_inc(&network->bsc_ctrs->ctr[BSC_CTR_HANDOVER_ATTEMPTED]); + do_assignment = (new_bts == old_lchan->ts->trx->bts); - if (!old_lchan->conn) { - LOGP(DHO, LOGL_ERROR, "Old lchan lacks connection data.\n"); - return -ENOSPC; + ho = talloc_zero(conn, struct bsc_handover); + if (!ho) { + LOGP(DHO, LOGL_FATAL, "Out of Memory\n"); + return -ENOMEM; } + ho->from_hodec_id = from_hodec_id; + ho->old_lchan = old_lchan; + ho->new_bts = new_bts; + ho->new_lchan_type = new_lchan_type; + ho->ho_ref = ho_ref++; + ho->inter_cell = !do_assignment; + ho->async = true; + llist_add(&ho->list, &bsc_handovers); + + conn->ho = ho; - DEBUGP(DHO, "(BTS %u trx %u ts %u lchan %u %s)->(BTS %u lchan %s) Beginning with handover operation...\n", + DEBUGP(DHO, "(BTS %u trx %u ts %u lchan %u %s)->(BTS %u lchan %s) Initiating %s...\n", old_lchan->ts->trx->bts->nr, old_lchan->ts->trx->nr, old_lchan->ts->nr, old_lchan->nr, gsm_pchan_name(old_lchan->ts->pchan), new_bts->nr, - gsm_lchant_name(new_lchan_type)); + gsm_lchant_name(new_lchan_type), + do_assignment ? "Assignment" : "Handover"); - new_lchan = lchan_alloc(new_bts, new_lchan_type, 0); - if (!new_lchan) { - LOGP(DHO, LOGL_NOTICE, "No free channel for %s\n", gsm_lchant_name(new_lchan_type)); - rate_ctr_inc(&network->bsc_ctrs->ctr[BSC_CTR_HANDOVER_NO_CHANNEL]); - return -ENOSPC; - } + return osmo_fsm_inst_dispatch(conn->fi, GSCON_EV_HO_START, NULL); +} + +/*! Start actual handover. Call bsc_handover_start() instead; The only legal caller is the GSCON FSM in + * bsc_subscr_conn_fsm.c. */ +int bsc_handover_start_gscon(struct gsm_subscriber_connection *conn) +{ + int rc; + struct gsm_network *network = conn->network; + struct bsc_handover *ho = conn->ho; + struct gsm_lchan *old_lchan; + struct gsm_lchan *new_lchan; - ho = talloc_zero(tall_bsc_ctx, struct bsc_handover); if (!ho) { - LOGP(DHO, LOGL_FATAL, "Out of Memory\n"); - lchan_free(new_lchan); - return -ENOMEM; + LOGP(DHO, LOGL_ERROR, "%s: Requested to start handover, but conn->ho is NULL\n", + bsc_subscr_name(conn->bsub)); + return -EINVAL; + } + + OSMO_ASSERT(ho->old_lchan && ho->new_bts); + + if (ho->old_lchan->conn != conn) { + LOGP(DHO, LOGL_ERROR, + "%s: Requested to start handover, but the lchan does not belong to this conn\n", + bsc_subscr_name(conn->bsub)); + return -EINVAL; + } + + rate_ctr_inc(&network->bsc_ctrs->ctr[BSC_CTR_HANDOVER_ATTEMPTED]); + + ho->new_lchan = lchan_alloc(ho->new_bts, ho->new_lchan_type, 0); + if (!ho->new_lchan) { + LOGP(DHO, LOGL_NOTICE, "No free channel for %s\n", gsm_lchant_name(ho->new_lchan_type)); + rate_ctr_inc(&network->bsc_ctrs->ctr[BSC_CTR_HANDOVER_NO_CHANNEL]); + return -ENOSPC; } - ho->from_hodec_id = from_hodec_id; - ho->old_lchan = old_lchan; - ho->new_lchan = new_lchan; - ho->ho_ref = ho_ref++; - ho->inter_cell = !do_assignment; - ho->async = true; - LOGPHO(ho, LOGL_INFO, "Triggering %s\n", do_assignment? "Assignment" : "Handover"); + LOGPHO(ho, LOGL_INFO, "Triggering %s\n", ho->inter_cell? "Handover" : "Assignment"); /* copy some parameters from old lchan */ + old_lchan = ho->old_lchan; + new_lchan = ho->new_lchan; memcpy(&new_lchan->encr, &old_lchan->encr, sizeof(new_lchan->encr)); - if (do_assignment) { + if (!ho->inter_cell) { new_lchan->ms_power = old_lchan->ms_power; new_lchan->rqd_ta = old_lchan->rqd_ta; } else { new_lchan->ms_power = - ms_pwr_ctl_lvl(new_bts->band, new_bts->ms_max_power); + ms_pwr_ctl_lvl(ho->new_bts->band, ho->new_bts->ms_max_power); /* FIXME: do we have a better idea of the timing advance? */ //new_lchan->rqd_ta = old_lchan->rqd_ta; } @@ -188,24 +190,21 @@ int bsc_handover_start_gscon(struct gsm_lchan *old_lchan, struct gsm_bts *new_bt memcpy(&new_lchan->mr_ms_lv, &old_lchan->mr_ms_lv, sizeof(new_lchan->mr_ms_lv)); memcpy(&new_lchan->mr_bts_lv, &old_lchan->mr_bts_lv, sizeof(new_lchan->mr_bts_lv)); - new_lchan->conn = old_lchan->conn; - new_lchan->conn->ho_lchan = new_lchan; + new_lchan->conn = conn; rc = rsl_chan_activate_lchan(new_lchan, ho->async ? RSL_ACT_INTER_ASYNC : RSL_ACT_INTER_SYNC, ho->ho_ref); if (rc < 0) { LOGPHO(ho, LOGL_INFO, "%s Failure: activate lchan rc = %d\n", - do_assignment? "Assignment" : "Handover", rc); - new_lchan->conn->ho_lchan = NULL; - new_lchan->conn = NULL; - talloc_free(ho); + ho->inter_cell? "Handover" : "Assignment", rc); lchan_free(new_lchan); + ho->new_lchan = NULL; + bsc_clear_handover(conn, 0); return rc; } rsl_lchan_set_state(new_lchan, LCHAN_S_ACT_REQ); - llist_add(&ho->list, &bsc_handovers); /* we continue in the SS_LCHAN handler / ho_chan_activ_ack */ return 0; @@ -214,26 +213,20 @@ int bsc_handover_start_gscon(struct gsm_lchan *old_lchan, struct gsm_bts *new_bt /* clear any operation for this connection */ void bsc_clear_handover(struct gsm_subscriber_connection *conn, int free_lchan) { - struct bsc_handover *ho; - - ho = bsc_ho_by_new_lchan(conn->ho_lchan); - - - if (!ho && conn->ho_lchan) - LOGP(DHO, LOGL_ERROR, "BUG: We lost some state.\n"); + struct bsc_handover *ho = conn->ho; - if (!ho) { - LOGP(DHO, LOGL_ERROR, "unable to find HO record\n"); + if (!ho) return; - } - - conn->ho_lchan->conn = NULL; - conn->ho_lchan = NULL; - if (free_lchan) - lchan_release(ho->new_lchan, 0, RSL_REL_LOCAL_END); + if (ho->new_lchan) { + ho->new_lchan->conn = NULL; + if (free_lchan) + lchan_release(ho->new_lchan, 0, RSL_REL_LOCAL_END); + ho->new_lchan = NULL; + } handover_free(ho); + conn->ho = NULL; } /* T3103 expired: Handover has failed without HO COMPLETE or HO FAIL */ @@ -248,10 +241,7 @@ static void ho_T3103_cb(void *_ho) /* Inform the GSCON FSM about the timed out handover */ osmo_fsm_inst_dispatch(ho->old_lchan->conn->fi, GSCON_EV_HO_TIMEOUT, NULL); - ho->new_lchan->conn->ho_lchan = NULL; - ho->new_lchan->conn = NULL; - lchan_release(ho->new_lchan, 0, RSL_REL_LOCAL_END); - handover_free(ho); + bsc_clear_handover(ho->old_lchan->conn, 1); } /* RSL has acknowledged activation of the new lchan */ @@ -300,12 +290,7 @@ static int ho_chan_activ_nack(struct gsm_lchan *new_lchan) if (hdc && hdc->on_ho_chan_activ_nack) hdc->on_ho_chan_activ_nack(ho); - new_lchan->conn->ho_lchan = NULL; - new_lchan->conn = NULL; - handover_free(ho); - - /* FIXME: maybe we should try to allocate a new LCHAN here? */ - + bsc_clear_handover(new_lchan->conn, 0); return 0; } @@ -333,16 +318,16 @@ static int ho_gsm48_ho_compl(struct gsm_lchan *new_lchan) if (ho->old_lchan != new_lchan->conn->lchan) LOGPHO(ho, LOGL_ERROR, "Primary lchan changed during handover.\n"); - if (new_lchan != new_lchan->conn->ho_lchan) + if (new_lchan->conn->ho != ho) LOGPHO(ho, LOGL_ERROR, "Handover channel changed during this handover.\n"); - new_lchan->conn->ho_lchan = NULL; new_lchan->conn->lchan = new_lchan; ho->old_lchan->conn = NULL; lchan_release(ho->old_lchan, 0, RSL_REL_LOCAL_END); handover_free(ho); + new_lchan->conn->ho = NULL; /* Inform the GSCON FSM that the handover is complete */ osmo_fsm_inst_dispatch(new_lchan->conn->fi, GSCON_EV_HO_COMPL, NULL); @@ -354,7 +339,6 @@ static int ho_gsm48_ho_fail(struct gsm_lchan *old_lchan) { struct gsm_network *net = old_lchan->ts->trx->bts->network; struct bsc_handover *ho; - struct gsm_lchan *new_lchan; struct handover_decision_callbacks *hdc; ho = bsc_ho_by_old_lchan(old_lchan); @@ -369,14 +353,7 @@ static int ho_gsm48_ho_fail(struct gsm_lchan *old_lchan) rate_ctr_inc(&net->bsc_ctrs->ctr[BSC_CTR_HANDOVER_FAILED]); - new_lchan = ho->new_lchan; - - /* release the channel and forget about it */ - ho->new_lchan->conn->ho_lchan = NULL; - ho->new_lchan->conn = NULL; - handover_free(ho); - - lchan_release(new_lchan, 0, RSL_REL_LOCAL_END); + bsc_clear_handover(ho->new_lchan->conn, 1); /* Inform the GSCON FSM that the handover failed */ osmo_fsm_inst_dispatch(old_lchan->conn->fi, GSCON_EV_HO_FAIL, NULL); diff --git a/src/osmo-bsc/bsc_subscr_conn_fsm.c b/src/osmo-bsc/bsc_subscr_conn_fsm.c index 6b8825874..569921342 100644 --- a/src/osmo-bsc/bsc_subscr_conn_fsm.c +++ b/src/osmo-bsc/bsc_subscr_conn_fsm.c @@ -31,6 +31,7 @@ #include #include #include +#include #include #include @@ -373,8 +374,7 @@ static void gscon_fsm_active(struct osmo_fsm_inst *fi, uint32_t event, void *dat } break; case GSCON_EV_HO_START: - rc = bsc_handover_start_gscon(conn->user_plane.ho_old_lchan, conn->user_plane.ho_new_bts, - conn->user_plane.ho_new_lchan_type); + rc = bsc_handover_start_gscon(conn); if (rc) { resp = gsm0808_create_clear_rqst(GSM0808_CAUSE_EQUIPMENT_FAILURE); sigtran_send(conn, resp, fi); @@ -928,10 +928,10 @@ static void gscon_cleanup(struct osmo_fsm_inst *fi, enum osmo_fsm_term_cause cau { struct gsm_subscriber_connection *conn = fi->priv; - if (conn->ho_lchan) { - LOGPFSML(fi, LOGL_DEBUG, "Releasing ho_lchan\n"); + if (conn->ho) { + LOGPFSML(fi, LOGL_DEBUG, "Releasing handover state\n"); bsc_clear_handover(conn, 1); - conn->ho_lchan = NULL; + conn->ho = NULL; } if (conn->secondary_lchan) { diff --git a/src/osmo-bsc/osmo_bsc_audio.c b/src/osmo-bsc/osmo_bsc_audio.c index f821e221f..23fd8c913 100644 --- a/src/osmo-bsc/osmo_bsc_audio.c +++ b/src/osmo-bsc/osmo_bsc_audio.c @@ -30,6 +30,7 @@ #include #include #include +#include #include @@ -71,9 +72,8 @@ static int handle_abisip_signal(unsigned int subsys, unsigned int signal, break; case S_ABISIP_MDCX_ACK: - if (con->ho_lchan) { - LOGP(DHO, LOGL_DEBUG, "%s -> %s BTS sent MDCX ACK\n", gsm_lchan_name(lchan), - gsm_lchan_name(con->ho_lchan)); + if (con->ho) { + LOGPHO(con->ho, LOGL_DEBUG, "BTS sent MDCX ACK\n"); /* No need to do anything for handover here. As soon as a HANDOVER DETECT * happens, osmo_bsc_mgcp.c will trigger the MGCP MDCX towards MGW by * receiving an S_LCHAN_HANDOVER_DETECT signal. diff --git a/tests/bsc-nat/bsc_nat_test.c b/tests/bsc-nat/bsc_nat_test.c index 09378ae6a..bb287ec4c 100644 --- a/tests/bsc-nat/bsc_nat_test.c +++ b/tests/bsc-nat/bsc_nat_test.c @@ -1590,3 +1590,5 @@ void bsc_nat_send_mgcp_to_msc(struct bsc_nat *nat, struct msgb *msg) { abort(); } + +struct gsm_subscriber_connection *bsc_subscr_con_allocate(struct gsm_network *network) { return NULL; } diff --git a/tests/channel/channel_test.c b/tests/channel/channel_test.c index b41e3d6a7..78db1d4e3 100644 --- a/tests/channel/channel_test.c +++ b/tests/channel/channel_test.c @@ -119,5 +119,6 @@ void ipa_client_conn_open() {} void ipa_client_conn_send() {} void ipa_msg_push_header() {} void ipaccess_bts_handle_ccm() {} +struct gsm_subscriber_connection *bsc_subscr_con_allocate(struct gsm_network *network) { return NULL; } struct tlv_definition nm_att_tlvdef; -- cgit v1.2.3