fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/37803?usp=email ) Change subject: erab_fsm: fix message flow misconceptions ...................................................................... erab_fsm: fix message flow misconceptions Change-Id: Ie6f65516fd5621acf002de97eeeb2b9b0b4fae4a Fixes: 9f2a172 "erab_fsm: E-RAB FSM implementation" --- M src/erab_fsm.erl M test/erab_fsm_test.erl M test/pfcp_mock.erl M test/pfcp_mock.hrl 4 files changed, 48 insertions(+), 48 deletions(-) git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-s1gw refs/changes/03/37803/1 diff --git a/src/erab_fsm.erl b/src/erab_fsm.erl index 8a9d0ec..abc25b1 100644 --- a/src/erab_fsm.erl +++ b/src/erab_fsm.erl @@ -66,10 +66,10 @@ -type teid() :: 0..16#ffffffff. -record(erab_state, {from :: undefined | gen_statem:from(), %% destination to use when replying - teid_from_access :: undefined | teid(), %% TEID to be used in eNB -> UPF - teid_to_access :: undefined | teid(), %% TEID to be used in UPF -> eNB - teid_from_core :: undefined | teid(), %% TEID to be used in PGW -> UPF - teid_to_core :: undefined | teid(), %% TEID to be used in UPF -> PGW + teid_upf2core :: undefined | teid(), %% TEID to be used in UPF -> Core + teid_core2upf :: undefined | teid(), %% TEID to be used in UPF <- Core + teid_acc2upf :: undefined | teid(), %% TEID to be used in UPF <- Access + teid_upf2acc :: undefined | teid(), %% TEID to be used in UPF -> Access seid_loc :: undefined | pfcp_peer:pfcp_seid(), %% local SEID, chosen by us seid_rem :: undefined | pfcp_peer:pfcp_seid() %% remote SEID, chosen by the UPF }). @@ -89,10 +89,10 @@ gen_statem:start_link(?MODULE, [UID], []). -%% @doc Indicate reception of E-RAB setup request (from the eNB). +%% @doc Indicate reception of E-RAB setup request (from core). %% @param Pid PID of an erab_fsm. -%% @param TEID TEID chosen by the eNB. -%% @returns TEID to be sent to the core; an error otherwise. +%% @param TEID TEID indicated by the MME. +%% @returns TEID to be sent to the eNB; an error otherwise. %% @end -spec erab_setup_req(pid(), teid()) -> {ok, teid()} | {error, term()}. @@ -100,10 +100,10 @@ gen_statem:call(Pid, {?FUNCTION_NAME, TEID}). -%% @doc Indicate reception of E-RAB setup response (from the core). +%% @doc Indicate reception of E-RAB setup response (from access). %% @param Pid PID of an erab_fsm. -%% @param TEID TEID chosen by the core. -%% @returns TEID to be sent to the eNB; an error otherwise. +%% @param TEID TEID indicated by the eNB. +%% @returns TEID to be sent to the MME; an error otherwise. %% @end -spec erab_setup_rsp(pid(), teid()) -> {ok, teid()} | {error, term()}. @@ -142,18 +142,18 @@ [state_functions, state_enter]. -%% state WAIT_SETUP_REQ :: wait E-RAB SETUP Req from the Access +%% state WAIT_SETUP_REQ :: wait E-RAB SETUP Req from core erab_wait_setup_req(enter, ?FUNCTION_NAME, S) -> ?LOG_DEBUG("State enter: ~p", [?FUNCTION_NAME]), {keep_state, S}; erab_wait_setup_req({call, From}, - {erab_setup_req, TEID_FromAcc}, + {erab_setup_req, TEID_U2C}, #erab_state{} = S) -> - ?LOG_DEBUG("Rx E-RAB SETUP Req (Access TEID ~p)", [TEID_FromAcc]), + ?LOG_DEBUG("Rx E-RAB SETUP Req (TEID UPF -> Core ~p)", [TEID_U2C]), {next_state, session_establish, S#erab_state{from = From, - teid_from_access = TEID_FromAcc}}; + teid_upf2core = TEID_U2C}}; erab_wait_setup_req(Event, EventData, S) -> ?LOG_ERROR("Unexpected event ~p: ~p", [Event, EventData]), @@ -182,15 +182,15 @@ seid = SEID_Rsp, %% matches F-SEID we sent in the ESTABLISH Req ie = #{pfcp_cause := 'Request accepted', f_seid := #f_seid{seid = F_SEID}, - created_pdr := [#{f_teid := #f_teid{teid = TEID_ToCore}}, - #{f_teid := #f_teid{teid = TEID_ToAcc}}]}} -> + created_pdr := [#{f_teid := #f_teid{teid = TEID_C2U}}, + #{f_teid := #f_teid{teid = TEID_A2U}}]}} -> ?LOG_DEBUG("PFCP session established"), {next_state, erab_wait_setup_rsp, S#erab_state{from = undefined, seid_rem = F_SEID, %% SEID to be used in further requests from us - teid_to_core = TEID_ToCore, - teid_to_access = TEID_ToAcc}, - [{reply, From, {ok, TEID_ToCore}}]}; + teid_core2upf = TEID_C2U, + teid_acc2upf = TEID_A2U}, + [{reply, From, {ok, TEID_A2U}}]}; _ -> ?LOG_ERROR("Rx unexpected PFCP PDU: ~p", [PDU]), {stop_and_reply, @@ -203,7 +203,7 @@ {keep_state, S}. -%% state WAIT_SETUP_RSP :: wait E-RAB SETUP Rsp from the Core +%% state WAIT_SETUP_RSP :: wait E-RAB SETUP Rsp from access erab_wait_setup_rsp(enter, OldState, S) -> ?LOG_DEBUG("State change: ~p -> ~p", [OldState, ?FUNCTION_NAME]), {next_state, ?FUNCTION_NAME, S, %% loop transition to enable state_timeout @@ -213,12 +213,12 @@ {stop, {shutdown, timeout}}; erab_wait_setup_rsp({call, From}, - {erab_setup_rsp, TEID_FromCore}, + {erab_setup_rsp, TEID_U2A}, #erab_state{} = S) -> - ?LOG_DEBUG("Rx E-RAB SETUP Rsp (Core TEID ~p)", [TEID_FromCore]), + ?LOG_DEBUG("Rx E-RAB SETUP Rsp (TEID UPF -> Access ~p)", [TEID_U2A]), {next_state, session_modify, S#erab_state{from = From, - teid_from_core = TEID_FromCore}}; + teid_upf2acc = TEID_U2A}}; %% Catch-all handler for this state erab_wait_setup_rsp(Event, EventData, S) -> @@ -243,7 +243,7 @@ session_modify(info, #pfcp{} = PDU, #erab_state{from = From, seid_loc = SEID_Rsp, - teid_to_access = TEID_ToAcc} = S) -> + teid_core2upf = TEID_C2U} = S) -> case PDU of #pfcp{type = session_modification_response, seid = SEID_Rsp, %% matches F-SEID we sent in the ESTABLISH Req @@ -251,7 +251,7 @@ ?LOG_DEBUG("PFCP session modified"), {next_state, erab_setup, S#erab_state{from = undefined}, - [{reply, From, {ok, TEID_ToAcc}}]}; + [{reply, From, {ok, TEID_C2U}}]}; _ -> ?LOG_ERROR("Rx unexpected PFCP PDU: ~p", [PDU]), {stop_and_reply, @@ -322,7 +322,7 @@ {keep_state, S}. -%% state WAIT_RELEASE_RSP :: wait E-RAB RELEASE Rsp from the Core +%% state WAIT_RELEASE_RSP :: wait E-RAB RELEASE Rsp erab_wait_release_rsp(enter, OldState, S) -> ?LOG_DEBUG("State change: ~p -> ~p", [OldState, ?FUNCTION_NAME]), {next_state, ?FUNCTION_NAME, S, %% loop transition to enable state_timeout @@ -371,7 +371,7 @@ -spec session_establish_req(erab_state()) -> pfcp_peer:pfcp_session_rsp(). session_establish_req(#erab_state{seid_loc = F_SEID, %% used as F-SEID - teid_from_access = TEID_FromAcc}) -> + teid_upf2core = TEID_U2C}) -> %% Packet Detection Rules OHR = #outer_header_removal{header = 'GTP-U/UDP/IPv4'}, PDRs = [#{pdr_id => {pdr_id, 1}, %% -- for Core -> Access @@ -392,10 +392,10 @@ OHC = #outer_header_creation{n6 = false, n19 = false, type = 'GTP-U', - teid = TEID_FromAcc, + teid = TEID_U2C, ipv4 = <<127,0,0,1>>}, %% XXX: Core facing addr of the UPF FARs = [#{far_id => {far_id, 1}, %% -- for Core -> Access - %% We don't know the Core side TEID / GTP-U address yet, so we set + %% We don't know the Access side TEID / GTP-U address yet, so we set %% this FAR to DROP and modify it when we get E-RAB SETUP RESPONSE. apply_action => #{'DROP' => []}}, #{far_id => {far_id, 2}, %% -- for Access -> Core @@ -408,12 +408,12 @@ -spec session_modify_req(erab_state()) -> pfcp_peer:pfcp_session_rsp(). session_modify_req(#erab_state{seid_rem = SEID, %% SEID allocated to us - teid_from_core = TEID_FromCore}) -> + teid_upf2acc = TEID_U2A}) -> %% Forwarding Action Rules OHC = #outer_header_creation{n6 = false, n19 = false, type = 'GTP-U', - teid = TEID_FromCore, + teid = TEID_U2A, ipv4 = <<127,0,0,1>>}, %% XXX: Access facing addr of the UPF FARs = [#{far_id => {far_id, 1}, %% -- for Core -> Access %% Now we know the Core side TEID / GTP-U address, so we modify diff --git a/test/erab_fsm_test.erl b/test/erab_fsm_test.erl index fce03fd..5c42f61 100644 --- a/test/erab_fsm_test.erl +++ b/test/erab_fsm_test.erl @@ -57,8 +57,8 @@ %% ------------------------------------------------------------------ test_erab_setup_success(Pid) -> - [?_assertEqual({ok, ?TEID_ToCore}, erab_fsm:erab_setup_req(Pid, ?TEID_FromAcc)), - ?_assertEqual({ok, ?TEID_ToAcc}, erab_fsm:erab_setup_rsp(Pid, ?TEID_FromCore)), + [?_assertEqual({ok, ?TEID_A2U}, erab_fsm:erab_setup_req(Pid, ?TEID_U2C)), + ?_assertEqual({ok, ?TEID_C2U}, erab_fsm:erab_setup_rsp(Pid, ?TEID_U2A)), ?_assertEqual(ok, erab_fsm:shutdown(Pid)), ?_assertNot(erlang:is_process_alive(Pid))]. @@ -71,7 +71,7 @@ pfcp_mock:mock_req(session_establish_req, PDU), unlink(Pid), %% we expect the FSM to terminate abnormally Error = {unexp_pdu, session_establish}, - [?_assertEqual({error, Error}, erab_fsm:erab_setup_req(Pid, ?TEID_FromAcc)), + [?_assertEqual({error, Error}, erab_fsm:erab_setup_req(Pid, ?TEID_U2C)), ?_assertNot(erlang:is_process_alive(Pid))]. @@ -81,14 +81,14 @@ pfcp_mock:mock_req(session_modify_req, PDU), unlink(Pid), %% we expect the FSM to terminate abnormally Error = {unexp_pdu, session_modify}, - [?_assertEqual({ok, ?TEID_ToCore}, erab_fsm:erab_setup_req(Pid, ?TEID_FromAcc)), - ?_assertEqual({error, Error}, erab_fsm:erab_setup_rsp(Pid, ?TEID_FromCore)), + [?_assertEqual({ok, ?TEID_A2U}, erab_fsm:erab_setup_req(Pid, ?TEID_U2C)), + ?_assertEqual({error, Error}, erab_fsm:erab_setup_rsp(Pid, ?TEID_U2A)), ?_assertNot(erlang:is_process_alive(Pid))]. test_erab_release_success(Pid) -> - [?_assertEqual({ok, ?TEID_ToCore}, erab_fsm:erab_setup_req(Pid, ?TEID_FromAcc)), - ?_assertEqual({ok, ?TEID_ToAcc}, erab_fsm:erab_setup_rsp(Pid, ?TEID_FromCore)), + [?_assertEqual({ok, ?TEID_A2U}, erab_fsm:erab_setup_req(Pid, ?TEID_U2C)), + ?_assertEqual({ok, ?TEID_C2U}, erab_fsm:erab_setup_rsp(Pid, ?TEID_U2A)), ?_assertEqual(ok, erab_fsm:erab_release_req(Pid)), ?_assertEqual(ok, erab_fsm:erab_release_rsp(Pid)), ?_assertNot(erlang:is_process_alive(Pid))]. @@ -100,8 +100,8 @@ pfcp_mock:mock_req(session_delete_req, PDU), unlink(Pid), %% we expect the FSM to terminate abnormally Error = {unexp_pdu, session_delete}, - [?_assertEqual({ok, ?TEID_ToCore}, erab_fsm:erab_setup_req(Pid, ?TEID_FromAcc)), - ?_assertEqual({ok, ?TEID_ToAcc}, erab_fsm:erab_setup_rsp(Pid, ?TEID_FromCore)), + [?_assertEqual({ok, ?TEID_A2U}, erab_fsm:erab_setup_req(Pid, ?TEID_U2C)), + ?_assertEqual({ok, ?TEID_C2U}, erab_fsm:erab_setup_rsp(Pid, ?TEID_U2A)), ?_assertEqual({error, Error}, erab_fsm:erab_release_req(Pid)), ?_assertNot(erlang:is_process_alive(Pid))]. @@ -114,8 +114,8 @@ test_erab_shutdown_session_del(Pid) -> - [?_assertEqual({ok, ?TEID_ToCore}, erab_fsm:erab_setup_req(Pid, ?TEID_FromAcc)), - ?_assertEqual({ok, ?TEID_ToAcc}, erab_fsm:erab_setup_rsp(Pid, ?TEID_FromCore)), + [?_assertEqual({ok, ?TEID_A2U}, erab_fsm:erab_setup_req(Pid, ?TEID_U2C)), + ?_assertEqual({ok, ?TEID_C2U}, erab_fsm:erab_setup_rsp(Pid, ?TEID_U2A)), ?_assertEqual(ok, erab_fsm:shutdown(Pid)), ?_assertNot(erlang:is_process_alive(Pid))]. diff --git a/test/pfcp_mock.erl b/test/pfcp_mock.erl index c9f8297..635223a 100644 --- a/test/pfcp_mock.erl +++ b/test/pfcp_mock.erl @@ -89,8 +89,8 @@ pdu_session_establish_rsp() -> IEs = #{pfcp_cause => 'Request accepted', f_seid => #f_seid{seid = ?SEID_Rem}, - created_pdr => [#{f_teid => #f_teid{teid = ?TEID_ToCore}}, - #{f_teid => #f_teid{teid = ?TEID_ToAcc}}]}, + created_pdr => [#{f_teid => #f_teid{teid = ?TEID_C2U}}, + #{f_teid => #f_teid{teid = ?TEID_A2U}}]}, pdu_rsp(session_establishment_response, ?SEID_Loc, IEs). diff --git a/test/pfcp_mock.hrl b/test/pfcp_mock.hrl index 06b5c5a..3a54ade 100644 --- a/test/pfcp_mock.hrl +++ b/test/pfcp_mock.hrl @@ -1,9 +1,9 @@ -define(SEID_Loc, 16#dead). -define(SEID_Rem, 16#beef). --define(TEID_FromAcc, 16#0001). --define(TEID_FromCore, 16#0002). --define(TEID_ToAcc, 16#0003). --define(TEID_ToCore, 16#0004). +-define(TEID_U2C, 16#0001). %% UPF -> Core +-define(TEID_C2U, 16#0101). %% UPF <- Core +-define(TEID_A2U, 16#0202). %% UPF <- Access +-define(TEID_U2A, 16#0002). %% UPF -> Access %% vim:set ts=4 sw=4 et: -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/37803?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: erlang/osmo-s1gw Gerrit-Branch: master Gerrit-Change-Id: Ie6f65516fd5621acf002de97eeeb2b9b0b4fae4a Gerrit-Change-Number: 37803 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria &lt;vyanitskiy(a)sysmocom.de&gt;