fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/41493?usp=email )
Change subject: erab_fsm: handle E-RAB SETUP Rsp during release ......................................................................
erab_fsm: handle E-RAB SETUP Rsp during release
It is quite common in production for a UE context to be established and then released before the eNB has acknowledged its creation. In the current implementation, this leads to errors and causes the PDU that triggers the late E-RAB SETUP Response event to be dropped.
Make the FSM logic more flexible by handling late erab_setup_rsp event in state erab_wait_release_rsp, i.e. during the release. This way we avoid dropping PDUs and printing errors.
Change-Id: I17414aad42507b64e933c80a220bfbe3798a4ad0 Related: SYS#7738, SYS#7599 --- M src/erab_fsm.erl M test/erab_fsm_test.erl M test/s1ap_proxy_test.erl 3 files changed, 31 insertions(+), 11 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-s1gw refs/changes/93/41493/1
diff --git a/src/erab_fsm.erl b/src/erab_fsm.erl index 213a3c1..1871919 100644 --- a/src/erab_fsm.erl +++ b/src/erab_fsm.erl @@ -547,6 +547,17 @@ ?LOG_INFO("Rx E-RAB RELEASE Ind while releasing (ignored)"), keep_state_and_data;
+%% late E-RAB SETUP Rsp (race condition) +%% see test_initial_context_setup_rsp_during_release +erab_wait_release_rsp({call, From}, + {erab_setup_rsp, U2A}, + #erab_state{u2a = undefined} = S) -> + ?LOG_NOTICE("Rx late E-RAB SETUP Rsp (U2A ~s)", [pp(U2A)]), + Reply = {ok, S#erab_state.c2u}, + {keep_state, + S#erab_state{u2a = U2A}, + {reply, From, Reply}}; + erab_wait_release_rsp(Event, EventData, S) -> handle_event(?FUNCTION_NAME, Event, EventData, S).
diff --git a/test/erab_fsm_test.erl b/test/erab_fsm_test.erl index e49d5cc..1c8f3e5 100644 --- a/test/erab_fsm_test.erl +++ b/test/erab_fsm_test.erl @@ -77,7 +77,9 @@ {"E-RAB RELEASE IND (in state erab_wait_release_rsp) :: success", ?TC(fun test_erab_wait_release_rsp_release_ind_success/1)}, {"E-RAB release :: PFCP session deletion error", - ?TC(fun test_erab_release_pfcp_delete_error/1)}]. + ?TC(fun test_erab_release_pfcp_delete_error/1)}, + {"E-RAB release :: late E-RAB SETUP.rsp", + ?TC(fun test_erab_wait_release_rsp_setup_rsp/1)}].
erab_info_test_() -> @@ -241,6 +243,15 @@ ?_assertNot(erlang:is_process_alive(Pid))].
+%% test E-RAB SETUP.rsp received in state erab_wait_release_rsp +test_erab_wait_release_rsp_setup_rsp(Pid) -> + [?_assertEqual({ok, ?A2U}, erab_fsm:erab_setup_req(Pid, ?U2C)), + ?_assertEqual(ok, erab_fsm:erab_release_cmd(Pid)), + ?_assertEqual({ok, ?C2U}, erab_fsm:erab_setup_rsp(Pid, ?U2A)), + ?_assertEqual(ok, erab_fsm:erab_release_rsp(Pid)), + ?_assertNot(erlang:is_process_alive(Pid))]. + + test_erab_info_state(Pid) -> [?_assertEqual(erab_wait_setup_req, erab_fsm_info(Pid, state)), ?_assertEqual({ok, ?A2U}, erab_fsm:erab_setup_req(Pid, ?U2C)), diff --git a/test/s1ap_proxy_test.erl b/test/s1ap_proxy_test.erl index eebbb49..f21a227 100644 --- a/test/s1ap_proxy_test.erl +++ b/test/s1ap_proxy_test.erl @@ -170,6 +170,7 @@ SetupReqExp = e_rab_setup_req_pdu(?ADDR_A2U, ?TEID_A2U), %% [eNB -> MME] E-RAB SETUP RESPONSE SetupRspIn = e_rab_setup_rsp_pdu(?ADDR_U2A, ?TEID_U2A), + SetupRspExp = e_rab_setup_rsp_pdu(?ADDR_C2U, ?TEID_C2U), %% [eNB <- MME] E-RAB RELEASE COMMAND ReleaseCmd = e_rab_release_cmd_pdu(), %% [eNB -> MME] E-RAB RELEASE RESPONSE @@ -180,8 +181,7 @@ %% MME orders release of an E-RAB, even before the eNB responds ?_assertEqual({forward, ReleaseCmd}, s1ap_proxy:process_pdu(Pid, ReleaseCmd)), %% eNB confirms allocation of an E-RAB - %% XXX: "Failed to process E-RAB SETUP RESPONSE: {unexpected_call, erab_wait_release_rsp}" - ?_assertMatch({drop, _}, s1ap_proxy:process_pdu(Pid, SetupRspIn)), + ?_assertEqual({forward, SetupRspExp}, s1ap_proxy:process_pdu(Pid, SetupRspIn)), %% eNB confirms release of an E-RAB ?_assertEqual({forward, ReleaseRsp}, s1ap_proxy:process_pdu(Pid, ReleaseRsp)), %% E-RAB have been released at this point @@ -192,9 +192,8 @@ ?_assertMetric(?S1GW_CTR_S1AP_PROXY_IN_PKT_ERAB_SETUP_RSP, 1), ?_assertMetric(?S1GW_CTR_S1AP_PROXY_IN_PKT_ERAB_RELEASE_CMD, 1), ?_assertMetric(?S1GW_CTR_S1AP_PROXY_IN_PKT_ERAB_RELEASE_RSP, 1), - ?_assertMetric(?S1GW_CTR_S1AP_PROXY_IN_PKT_DROP_ALL, 1), - ?_assertMetric(?S1GW_CTR_S1AP_PROXY_OUT_PKT_FWD_ALL, 3), - ?_assertMetric(?S1GW_CTR_S1AP_PROXY_OUT_PKT_FWD_PROC, 1), + ?_assertMetric(?S1GW_CTR_S1AP_PROXY_OUT_PKT_FWD_ALL, 4), + ?_assertMetric(?S1GW_CTR_S1AP_PROXY_OUT_PKT_FWD_PROC, 2), ?_assertMetric(?S1GW_CTR_S1AP_PROXY_OUT_PKT_FWD_UNMODIFIED, 2)].
@@ -446,6 +445,7 @@ InitCtxSetupReqExp = initial_context_setup_req_pdu(?ADDR_A2U, ?TEID_A2U), %% [eNB -> MME] INITIAL CONTEXT SETUP RESPONSE InitCtxSetupRsp = initial_context_setup_rsp_pdu(?ADDR_U2A, ?TEID_U2A), + InitCtxSetupRspExp = initial_context_setup_rsp_pdu(?ADDR_C2U, ?TEID_C2U), %% [eNB -> MME] UE CONTEXT RELEASE REQUEST UeCtxReleaseReq = ue_ctx_release_req_pdu(), %% [eNB <- MME] UE CONTEXT RELEASE COMMAND @@ -460,8 +460,7 @@ %% MME orders release of the UE context, as requested ?_assertEqual({forward, UeCtxReleaseCmd}, s1ap_proxy:process_pdu(Pid, UeCtxReleaseCmd)), %% eNB confirms creation of the UE context - %% XXX: "Failed to process INITIAL CONTEXT SETUP RESPONSE: {unexpected_call, erab_wait_release_rsp}" - ?_assertMatch({drop, _}, s1ap_proxy:process_pdu(Pid, InitCtxSetupRsp)), + ?_assertEqual({forward, InitCtxSetupRspExp}, s1ap_proxy:process_pdu(Pid, InitCtxSetupRsp)), %% eNB confirms release of the UE context ?_assertEqual({forward, UeCtxReleaseCompl}, s1ap_proxy:process_pdu(Pid, UeCtxReleaseCompl)), %% the UE context has been released, so no E-RAB FSMs shall remain @@ -473,9 +472,8 @@ ?_assertMetric(?S1GW_CTR_S1AP_PROXY_IN_PKT_RELEASE_CTX_REQ, 1), ?_assertMetric(?S1GW_CTR_S1AP_PROXY_IN_PKT_RELEASE_CTX_CMD, 1), ?_assertMetric(?S1GW_CTR_S1AP_PROXY_IN_PKT_RELEASE_CTX_COMPL, 1), - ?_assertMetric(?S1GW_CTR_S1AP_PROXY_IN_PKT_DROP_ALL, 1), - ?_assertMetric(?S1GW_CTR_S1AP_PROXY_OUT_PKT_FWD_ALL, 4), - ?_assertMetric(?S1GW_CTR_S1AP_PROXY_OUT_PKT_FWD_PROC, 1), + ?_assertMetric(?S1GW_CTR_S1AP_PROXY_OUT_PKT_FWD_ALL, 5), + ?_assertMetric(?S1GW_CTR_S1AP_PROXY_OUT_PKT_FWD_PROC, 2), ?_assertMetric(?S1GW_CTR_S1AP_PROXY_OUT_PKT_FWD_UNMODIFIED, 3)].