fixeria has submitted this change. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/39523?usp=email )

(

1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: erab_fsm: make erab_release_{cmd,ind}/1 non-blocking ......................................................................

erab_fsm: make erab_release_{cmd,ind}/1 non-blocking

There's no practical benefit(s) in blocking the caller while waiting for the PFCP Session Deletion Response from the UPF. The response does not contain any useful information for the caller.

Make the release related API non-blocking by using gen_statem:cast/2 instead of gen_statem:call/2. Adjust state functions to not reply.

Change-Id: If519334359c3ef331720cb87bdba2d969601816e Related: SYS#7318 --- M src/erab_fsm.erl M test/erab_fsm_test.erl 2 files changed, 15 insertions(+), 27 deletions(-)

Approvals: pespin: Looks good to me, approved Jenkins Builder: Verified

diff --git a/src/erab_fsm.erl b/src/erab_fsm.erl index 9ee9abe..bbb6dad 100644 --- a/src/erab_fsm.erl +++ b/src/erab_fsm.erl @@ -136,7 +136,7 @@

-spec erab_release_cmd(pid()) -> ok. erab_release_cmd(Pid) -> - gen_statem:call(Pid, ?FUNCTION_NAME). + gen_statem:cast(Pid, ?FUNCTION_NAME).

-spec erab_release_rsp(pid()) -> ok. @@ -146,7 +146,7 @@

-spec erab_release_ind(pid()) -> ok. erab_release_ind(Pid) -> - gen_statem:call(Pid, ?FUNCTION_NAME). + gen_statem:cast(Pid, ?FUNCTION_NAME).

-spec fetch_info(pid()) -> proplists:proplist(). @@ -305,21 +305,17 @@ ?LOG_DEBUG("State change: ~p -> ~p", [OldState, ?FUNCTION_NAME]), {keep_state, S};

-erab_setup({call, From}, - erab_release_cmd, +erab_setup(cast, erab_release_cmd, #erab_state{} = S) -> ?LOG_DEBUG("Rx E-RAB RELEASE Cmd"), {next_state, session_delete, - S#erab_state{from = From, - rel_kind = cmd}}; + S#erab_state{rel_kind = cmd}};

-erab_setup({call, From}, - erab_release_ind, +erab_setup(cast, erab_release_ind, #erab_state{} = S) -> ?LOG_DEBUG("Rx E-RAB RELEASE Ind"), {next_state, session_delete, - S#erab_state{from = From, - rel_kind = ind}}; + S#erab_state{rel_kind = ind}};

erab_setup(Event, EventData, S) -> handle_event(?FUNCTION_NAME, Event, EventData, S). @@ -335,36 +331,27 @@ S#erab_state{seid_rem = undefined}, [{state_timeout, ?ERAB_T_SESSION_DELETE, ?FUNCTION_NAME}]};

-session_delete(state_timeout, _Timeout, - #erab_state{from = From}) -> +session_delete(state_timeout, _Timeout, _S) -> ?LOG_ERROR("PFCP session deletion timeout"), - {stop_and_reply, - {shutdown, timeout}, - {reply, From, {error, {timeout, ?FUNCTION_NAME}}}}; + {stop, {shutdown, timeout}};

session_delete(info, #pfcp{} = PDU, - #erab_state{from = From, - seid_loc = SEID_Rsp, + #erab_state{seid_loc = SEID_Rsp, rel_kind = RelKind} = S) -> case PDU of #pfcp{type = session_deletion_response, seid = SEID_Rsp, %% matches F-SEID we sent in the ESTABLISH Req ie = #{pfcp_cause := 'Request accepted'}} -> ?LOG_DEBUG("PFCP session deleted"), - Reply = {reply, From, ok}, case RelKind of cmd -> %% E-RAB RELEASE CMD => wait for the RSP - {next_state, erab_wait_release_rsp, - S#erab_state{from = undefined}, - [Reply]}; + {next_state, erab_wait_release_rsp, S}; ind -> %% E-RAB RELEASE IND => terminate immediately - {stop_and_reply, normal, [Reply]} + {stop, normal} end; _ -> ?LOG_ERROR("Rx unexpected PFCP PDU: ~p", [PDU]), - {stop_and_reply, - {shutdown, unexp_pdu}, - {reply, From, {error, {unexp_pdu, ?FUNCTION_NAME}}}} + {stop, {shutdown, unexp_pdu}} end;

session_delete(Event, EventData, S) -> diff --git a/test/erab_fsm_test.erl b/test/erab_fsm_test.erl index e406e88..ec40e2e 100644 --- a/test/erab_fsm_test.erl +++ b/test/erab_fsm_test.erl @@ -125,10 +125,11 @@ PDU = pfcp_mock:pdu_rsp_reject(session_deletion_response, ?SEID_Loc), pfcp_mock:mock_req(session_delete_req, PDU), unlink(Pid), %% we expect the FSM to terminate abnormally - Error = {unexp_pdu, session_delete}, [?_assertEqual({ok, ?A2U}, erab_fsm:erab_setup_req(Pid, ?U2C)), ?_assertEqual({ok, ?C2U}, erab_fsm:erab_setup_rsp(Pid, ?U2A)), - ?_assertEqual({error, Error}, erab_fsm:erab_release_cmd(Pid)), + %% erab_fsm:erab_release_cmd/1 is non-blocking, + %% so we get ok before the error happens + ?_assertEqual(ok, erab_fsm:erab_release_cmd(Pid)), ?_assertNot(erlang:is_process_alive(Pid))].