osmith has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc-nat/+/28249 ) Change subject: subscr_conn_fsm: refactor timeouts ...................................................................... subscr_conn_fsm: refactor timeouts Use osmo_tdef_state_timeout instead of passing timers every time. Rename g_mgw_tdefs to g_bsc_nat_tdefs and add the new TDEF_ASS_COMPL there, so the TDEF_MGCP can be used in subscr_conn_fsm and by mgcp client without duplicating it. Related: SYS#5560 Change-Id: Ib34e6ccc34901ebc37d2dbe347d9644cb70921ca --- M include/osmocom/bsc_nat/bsc_nat.h M src/osmo-bsc-nat/bsc_nat.c M src/osmo-bsc-nat/subscr_conn_fsm.c 3 files changed, 42 insertions(+), 28 deletions(-) Approvals: Jenkins Builder: Verified pespin: Looks good to me, but someone else must approve fixeria: Looks good to me, approved diff --git a/include/osmocom/bsc_nat/bsc_nat.h b/include/osmocom/bsc_nat/bsc_nat.h index fd2c3fb..f7dafbb 100644 --- a/include/osmocom/bsc_nat/bsc_nat.h +++ b/include/osmocom/bsc_nat/bsc_nat.h @@ -24,6 +24,9 @@ #include <osmocom/mgcp_client/mgcp_client_pool.h> #include <osmocom/sigtran/sccp_sap.h> +#define BSC_NAT_TDEF_ASS_COMPL (-1) +#define BSC_NAT_TDEF_MGCP (-2427) + enum bsc_nat_net { BSC_NAT_NET_CN = 0, BSC_NAT_NET_RAN @@ -39,11 +42,11 @@ struct bsc_nat { struct osmo_fsm_inst *fi; + struct osmo_tdef *tdefs; struct llist_head subscr_conns; /* list of struct subscr_conn */ struct { struct mgcp_client_pool *pool; - struct osmo_tdef *tdefs; uint32_t call_id_next; } mgw; diff --git a/src/osmo-bsc-nat/bsc_nat.c b/src/osmo-bsc-nat/bsc_nat.c index b847679..3ae7881 100644 --- a/src/osmo-bsc-nat/bsc_nat.c +++ b/src/osmo-bsc-nat/bsc_nat.c @@ -28,13 +28,14 @@ #include <osmocom/bsc_nat/msc.h> #include <osmocom/bsc_nat/subscr_conn.h> -struct osmo_tdef g_mgw_tdefs[] = { - { .T = -2427, .default_val = 5, .desc = "timeout for MGCP response from MGW" }, +static struct osmo_tdef g_bsc_nat_tdefs[] = { + { .T = BSC_NAT_TDEF_ASS_COMPL, .default_val = 20, .desc = "Timeout for BSSMAP Assignment Complete from BSC" }, + { .T = BSC_NAT_TDEF_MGCP, .default_val = 5, .desc = "Timeout for MGCP response from MGW" }, {} }; struct osmo_tdef_group g_bsc_nat_tdef_group[] = { - { .name = "mgw", .tdefs = g_mgw_tdefs, .desc = "MGW (Media Gateway) interface" }, + { .name = "bsc-nat", .tdefs = g_bsc_nat_tdefs, .desc = "BSCNAT" }, {} }; @@ -45,9 +46,10 @@ bsc_nat = talloc_zero(tall_ctx, struct bsc_nat); OSMO_ASSERT(bsc_nat); + bsc_nat->tdefs = g_bsc_nat_tdefs; + osmo_tdefs_reset(bsc_nat->tdefs); + bsc_nat->mgw.pool = mgcp_client_pool_alloc(bsc_nat); - bsc_nat->mgw.tdefs = g_mgw_tdefs; - osmo_tdefs_reset(bsc_nat->mgw.tdefs); bsc_nat->cn.sccp_inst = talloc_zero(bsc_nat, struct bsc_nat_sccp_inst); OSMO_ASSERT(bsc_nat->cn.sccp_inst); diff --git a/src/osmo-bsc-nat/subscr_conn_fsm.c b/src/osmo-bsc-nat/subscr_conn_fsm.c index 848204d..8a58ac1 100644 --- a/src/osmo-bsc-nat/subscr_conn_fsm.c +++ b/src/osmo-bsc-nat/subscr_conn_fsm.c @@ -31,8 +31,6 @@ #include <osmocom/bsc_nat/subscr_conn_fsm.h> #define X(s) (1 << (s)) -#define TIMEOUT_MGW 10 -#define TIMEOUT_BSC 20 enum subscr_conn_fsm_states { SUBSCR_CONN_FSM_ST_IDLE, @@ -44,6 +42,19 @@ SUBSCR_CONN_FSM_ST_WAITING_FOR_CLEAR_COMMAND, }; +static const struct osmo_tdef_state_timeout subscr_conn_fsm_timeouts[32] = { + [SUBSCR_CONN_FSM_ST_IDLE] = {}, + [SUBSCR_CONN_FSM_ST_PROCESSING_ASSIGNMENT_REQUEST_CRCX_CN] = { .T = BSC_NAT_TDEF_MGCP }, + [SUBSCR_CONN_FSM_ST_PROCESSING_ASSIGNMENT_REQUEST_MDCX_CN] = { .T = BSC_NAT_TDEF_MGCP }, + [SUBSCR_CONN_FSM_ST_PROCESSING_ASSIGNMENT_REQUEST_CRCX_RAN] = { .T = BSC_NAT_TDEF_MGCP }, + [SUBSCR_CONN_FSM_ST_WAITING_FOR_ASSIGNMENT_COMPLETE] = { .T = BSC_NAT_TDEF_ASS_COMPL }, + [SUBSCR_CONN_FSM_ST_PROCESSING_ASSIGNMENT_COMPLETE_MDCX_RAN] = { .T = BSC_NAT_TDEF_MGCP }, + [SUBSCR_CONN_FSM_ST_WAITING_FOR_CLEAR_COMMAND] = {}, /* Call in progress, no timeout */ +}; + +#define subscr_conn_fsm_state_chg(fi, NEXT_STATE) \ + osmo_tdef_fsm_inst_state_chg(fi, NEXT_STATE, subscr_conn_fsm_timeouts, g_bsc_nat->tdefs, -1) + static int tx_ass_req_to_ran(struct subscr_conn *subscr_conn) { const struct mgcp_conn_peer *rtp_info; @@ -122,7 +133,7 @@ { switch (event) { case SUBSCR_CONN_FSM_EV_BSSMAP_ASSIGNMENT_REQUEST: - osmo_fsm_inst_state_chg(fi, SUBSCR_CONN_FSM_ST_PROCESSING_ASSIGNMENT_REQUEST_CRCX_CN, TIMEOUT_MGW, 0); + subscr_conn_fsm_state_chg(fi, SUBSCR_CONN_FSM_ST_PROCESSING_ASSIGNMENT_REQUEST_CRCX_CN); break; case SUBSCR_CONN_FSM_EV_MGCP_EP_TERM: /* This event is expected, as we terminate the MGCP EP in @@ -148,14 +159,14 @@ if (call_id < 0) { LOGPFSML(fi, LOGL_ERROR, "Failed to get next_id_mgw, aborting assignment request processing\n"); bssmap_tx_assignment_failure_cn(subscr_conn, GSM0808_CAUSE_PROTOCOL_ERROR_BETWEEN_BSS_AND_MSC); - osmo_fsm_inst_state_chg(fi, SUBSCR_CONN_FSM_ST_IDLE, 0, 0); + subscr_conn_fsm_state_chg(fi, SUBSCR_CONN_FSM_ST_IDLE); } /* MGCP EP */ mgcp_client = mgcp_client_pool_get(g_bsc_nat->mgw.pool); OSMO_ASSERT(mgcp_client); subscr_conn->ep = osmo_mgcpc_ep_alloc(subscr_conn->fi, SUBSCR_CONN_FSM_EV_MGCP_EP_TERM, mgcp_client, - g_bsc_nat->mgw.tdefs, "SUBSCR-CONN-EP", + g_bsc_nat->tdefs, "SUBSCR-CONN-EP", mgcp_client_rtpbridge_wildcard(mgcp_client)); /* MGCP CRCX CN */ @@ -173,12 +184,12 @@ switch (event) { case SUBSCR_CONN_FSM_EV_MGCP_EP_OK: LOGPFSML(fi, LOGL_DEBUG, "Rx MGCP OK\n"); - osmo_fsm_inst_state_chg(fi, SUBSCR_CONN_FSM_ST_PROCESSING_ASSIGNMENT_REQUEST_MDCX_CN, TIMEOUT_MGW, 0); + subscr_conn_fsm_state_chg(fi, SUBSCR_CONN_FSM_ST_PROCESSING_ASSIGNMENT_REQUEST_MDCX_CN); break; case SUBSCR_CONN_FSM_EV_MGCP_EP_FAIL: LOGPFSML(fi, LOGL_ERROR, "MGCP failure, aborting assignment request processing\n"); bssmap_tx_assignment_failure_cn(subscr_conn, GSM0808_CAUSE_PROTOCOL_ERROR_BETWEEN_BSS_AND_MSC); - osmo_fsm_inst_state_chg(fi, SUBSCR_CONN_FSM_ST_IDLE, 0, 0); + subscr_conn_fsm_state_chg(fi, SUBSCR_CONN_FSM_ST_IDLE); break; default: OSMO_ASSERT(false); @@ -208,12 +219,12 @@ switch (event) { case SUBSCR_CONN_FSM_EV_MGCP_EP_OK: LOGPFSML(fi, LOGL_DEBUG, "Rx MGCP OK\n"); - osmo_fsm_inst_state_chg(fi, SUBSCR_CONN_FSM_ST_PROCESSING_ASSIGNMENT_REQUEST_CRCX_RAN, TIMEOUT_MGW, 0); + subscr_conn_fsm_state_chg(fi, SUBSCR_CONN_FSM_ST_PROCESSING_ASSIGNMENT_REQUEST_CRCX_RAN); break; case SUBSCR_CONN_FSM_EV_MGCP_EP_FAIL: LOGPFSML(fi, LOGL_ERROR, "MGCP failure, aborting assignment request processing\n"); bssmap_tx_assignment_failure_cn(subscr_conn, GSM0808_CAUSE_PROTOCOL_ERROR_BETWEEN_BSS_AND_MSC); - osmo_fsm_inst_state_chg(fi, SUBSCR_CONN_FSM_ST_IDLE, 0, 0); + subscr_conn_fsm_state_chg(fi, SUBSCR_CONN_FSM_ST_IDLE); break; default: OSMO_ASSERT(false); @@ -240,15 +251,15 @@ LOGPFSML(fi, LOGL_DEBUG, "Rx MGCP OK\n"); if (tx_ass_req_to_ran(subscr_conn) < 0) { bssmap_tx_assignment_failure_cn(subscr_conn, GSM0808_CAUSE_PROTOCOL_ERROR_BETWEEN_BSS_AND_MSC); - osmo_fsm_inst_state_chg(fi, SUBSCR_CONN_FSM_ST_IDLE, 0, 0); + subscr_conn_fsm_state_chg(fi, SUBSCR_CONN_FSM_ST_IDLE); return; } - osmo_fsm_inst_state_chg(fi, SUBSCR_CONN_FSM_ST_WAITING_FOR_ASSIGNMENT_COMPLETE, TIMEOUT_BSC, 0); + subscr_conn_fsm_state_chg(fi, SUBSCR_CONN_FSM_ST_WAITING_FOR_ASSIGNMENT_COMPLETE); break; case SUBSCR_CONN_FSM_EV_MGCP_EP_FAIL: LOGPFSML(fi, LOGL_ERROR, "MGCP failure, aborting assignment request processing\n"); bssmap_tx_assignment_failure_cn(subscr_conn, GSM0808_CAUSE_PROTOCOL_ERROR_BETWEEN_BSS_AND_MSC); - osmo_fsm_inst_state_chg(fi, SUBSCR_CONN_FSM_ST_IDLE, 0, 0); + subscr_conn_fsm_state_chg(fi, SUBSCR_CONN_FSM_ST_IDLE); break; default: OSMO_ASSERT(false); @@ -259,13 +270,13 @@ { switch (event) { case SUBSCR_CONN_FSM_EV_BSSMAP_ASSIGNMENT_COMPLETE: - osmo_fsm_inst_state_chg(fi, SUBSCR_CONN_FSM_ST_PROCESSING_ASSIGNMENT_COMPLETE_MDCX_RAN, TIMEOUT_MGW, 0); + subscr_conn_fsm_state_chg(fi, SUBSCR_CONN_FSM_ST_PROCESSING_ASSIGNMENT_COMPLETE_MDCX_RAN); break; case SUBSCR_CONN_FSM_EV_BSSMAP_ASSIGNMENT_FAILURE: /* The original bssmap message gets forwarded from RAN to CN by * bssmap_ran_handle_assignment_failure() already, so just * reset the FSM to idle here (clears the mgw endpoint). */ - osmo_fsm_inst_state_chg(fi, SUBSCR_CONN_FSM_ST_IDLE, 0, 0); + subscr_conn_fsm_state_chg(fi, SUBSCR_CONN_FSM_ST_IDLE); break; case SUBSCR_CONN_FSM_EV_BSSMAP_CLEAR_COMMAND: /* Dialing an invalid number results in receiving a clear @@ -273,7 +284,7 @@ * The original message gets forwarded from CN to RAN by * bssmap_cn_handle_clear_cmd() already, so just reset the FSM * to idle here (clears the mgw endpoint). */ - osmo_fsm_inst_state_chg(fi, SUBSCR_CONN_FSM_ST_IDLE, 0, 0); + subscr_conn_fsm_state_chg(fi, SUBSCR_CONN_FSM_ST_IDLE); break; default: OSMO_ASSERT(false); @@ -306,18 +317,16 @@ if (tx_ass_compl_to_cn(subscr_conn) < 0) { bssmap_tx_assignment_failure_cn(subscr_conn, GSM0808_CAUSE_PROTOCOL_ERROR_BETWEEN_BSS_AND_MSC); bssmap_tx_assignment_failure_ran(subscr_conn, GSM0808_CAUSE_PROTOCOL_ERROR_BETWEEN_BSS_AND_MSC); - osmo_fsm_inst_state_chg(fi, SUBSCR_CONN_FSM_ST_IDLE, 0, 0); + subscr_conn_fsm_state_chg(fi, SUBSCR_CONN_FSM_ST_IDLE); return; } - /* No timeout for ST_WAITING_FOR_CLEAR_COMMAND, as the FSM will - * stay in this state until the call is done. */ - osmo_fsm_inst_state_chg(fi, SUBSCR_CONN_FSM_ST_WAITING_FOR_CLEAR_COMMAND, 0, 0); + subscr_conn_fsm_state_chg(fi, SUBSCR_CONN_FSM_ST_WAITING_FOR_CLEAR_COMMAND); break; case SUBSCR_CONN_FSM_EV_MGCP_EP_FAIL: LOGPFSML(fi, LOGL_ERROR, "MGCP failure, aborting assignment complete processing\n"); bssmap_tx_assignment_failure_cn(subscr_conn, GSM0808_CAUSE_PROTOCOL_ERROR_BETWEEN_BSS_AND_MSC); bssmap_tx_assignment_failure_ran(subscr_conn, GSM0808_CAUSE_PROTOCOL_ERROR_BETWEEN_BSS_AND_MSC); - osmo_fsm_inst_state_chg(fi, SUBSCR_CONN_FSM_ST_IDLE, 0, 0); + subscr_conn_fsm_state_chg(fi, SUBSCR_CONN_FSM_ST_IDLE); break; default: OSMO_ASSERT(false); @@ -331,7 +340,7 @@ /* The original bssmap message gets forwarded from CN to RAN by * bssmap_cn_handle_clear_cmd() already, so just reset the FSM * to idle here (clears the mgw endpoint). */ - osmo_fsm_inst_state_chg(fi, SUBSCR_CONN_FSM_ST_IDLE, 0, 0); + subscr_conn_fsm_state_chg(fi, SUBSCR_CONN_FSM_ST_IDLE); break; default: OSMO_ASSERT(false); @@ -341,7 +350,7 @@ int subscr_conn_fsm_timer_cb(struct osmo_fsm_inst *fi) { LOGPFSML(fi, LOGL_ERROR, "Timeout reached, reset FSM to idle\n"); - osmo_fsm_inst_state_chg(fi, SUBSCR_CONN_FSM_ST_IDLE, 0, 0); + subscr_conn_fsm_state_chg(fi, SUBSCR_CONN_FSM_ST_IDLE); return 0; } -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc-nat/+/28249 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc-nat Gerrit-Branch: master Gerrit-Change-Id: Ib34e6ccc34901ebc37d2dbe347d9644cb70921ca Gerrit-Change-Number: 28249 Gerrit-PatchSet: 3 Gerrit-Owner: osmith &lt;osmith(a)sysmocom.de&gt; Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria &lt;vyanitskiy(a)sysmocom.de&gt; Gerrit-Reviewer: osmith &lt;osmith(a)sysmocom.de&gt; Gerrit-Reviewer: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-MessageType: merged