fixeria has uploaded this change for review. (
https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/40993?usp=email )
Change subject: s1ap_proxy: erab_fsm_start_reg/2: check if already registered
......................................................................
s1ap_proxy: erab_fsm_start_reg/2: check if already registered
Before this patch, s1ap_proxy would spawn a new erab_fsm process with
a duplicate UID and simply replace the old process in the rigistry.
This would result in stale erab_fsm process, still linked but not
present in the registry, and thus not reachable.
If there's already an E-RAB process for the given UID, simply return
Pid of that existing process. The FSM will most likely reject an
unexpected event, resulting in a negative verdict:
* 'reply' for duplicate E-RAB SETUP REQUEST
* 'drop' for duplicate INITIAL CONTEXT SETUP REQUEST
Change-Id: I8e93492219100496fd8dc69e031310858ba9c7fb
---
M src/s1ap_proxy.erl
M test/s1ap_proxy_test.erl
2 files changed, 42 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-s1gw
refs/changes/93/40993/1
diff --git a/src/s1ap_proxy.erl b/src/s1ap_proxy.erl
index 8111ad6..0195e39 100644
--- a/src/s1ap_proxy.erl
+++ b/src/s1ap_proxy.erl
@@ -1104,8 +1104,14 @@
-spec erab_fsm_start_reg(erab_id(), proxy_state()) -> {pid(), proxy_state()}.
erab_fsm_start_reg(RABId, #proxy_state{erabs = ERABs} = S) ->
UID = erab_uid(RABId, S),
- {ok, Pid} = erab_fsm:start_link(UID),
- {Pid, S#proxy_state{erabs = dict:store(UID, Pid, ERABs)}}.
+ case dict:find(UID, ERABs) of
+ {ok, Pid} ->
+ ?LOG_ERROR("E-RAB ~p is already registered?!?", [UID]),
+ {Pid, S}; %% return Pid of the existing erab_fsm process
+ error ->
+ {ok, Pid} = erab_fsm:start_link(UID),
+ {Pid, S#proxy_state{erabs = dict:store(UID, Pid, ERABs)}}
+ end.
-spec erab_fsm_find(erab_id(), proxy_state()) -> {ok, pid()} | error.
diff --git a/test/s1ap_proxy_test.erl b/test/s1ap_proxy_test.erl
index 1145bc5..8f338c9 100644
--- a/test/s1ap_proxy_test.erl
+++ b/test/s1ap_proxy_test.erl
@@ -52,6 +52,8 @@
?TC(fun test_e_rab_setup/1)},
{"E-RAB SETUP REQUEST (failure)",
?TC(fun test_e_rab_setup_req_fail/1)},
+ {"E-RAB SETUP REQUEST (duplicate)",
+ ?TC(fun test_e_rab_setup_dup/1)},
{"E-RAB RELEASE COMMAND/RESPONSE",
?TC(fun test_e_rab_release_cmd/1)},
{"E-RAB RELEASE INDICATION",
@@ -68,6 +70,8 @@
?TC(fun test_e_rab_modify_ind_cnf_release/1)},
{"INITIAL CONTEXT SETUP REQUEST/RESPONSE",
?TC(fun test_initial_context_setup/1)},
+ {"INITIAL CONTEXT SETUP REQUEST/RESPONSE (duplicate)",
+ ?TC(fun test_initial_context_setup_dup/1)},
{"UE CONTEXT RELEASE REQUEST",
?TC(fun test_ue_ctx_release_req/1)},
{"UE CONTEXT RELEASE COMMAND/COMPLETE",
@@ -147,6 +151,23 @@
?_assertEqual([], s1ap_proxy:fetch_erab_list(Pid))].
+test_e_rab_setup_dup(#{handler := Pid}) ->
+ %% [eNB <- MME] E-RAB SETUP REQUEST
+ SetupReqIn = e_rab_setup_req_pdu(?ADDR_U2C, ?TEID_U2C),
+ %% [eNB -> MME] E-RAB SETUP RESPONSE
+ SetupRspIn = e_rab_setup_rsp_pdu(?ADDR_U2A, ?TEID_U2A),
+ %% eNB <- [S1GW <- MME] E-RAB SETUP REQUEST (duplicate)
+ %% eNB -- [S1GW -> MME] E-RAB SETUP RESPONSE (failure)
+ SetupRspExp = e_rab_setup_rsp_fail_pdu(),
+
+ [?_assertMatch({forward, _}, s1ap_proxy:process_pdu(Pid, SetupReqIn)),
+ ?_assertMatch({forward, _}, s1ap_proxy:process_pdu(Pid, SetupRspIn)),
+ %% duplicate E-RAB SETUP REQUEST triggers a reply
+ ?_assertEqual({reply, SetupRspExp}, s1ap_proxy:process_pdu(Pid,
SetupReqIn)),
+ ?_assertMatch({ok, _}, s1ap_proxy:fetch_erab(Pid, {7, 9, 6})),
+ ?_assertMatch([_], s1ap_proxy:fetch_erab_list(Pid))].
+
+
test_e_rab_release_cmd(#{handler := Pid}) ->
%% [eNB <- MME] E-RAB SETUP REQUEST
SetupReq = e_rab_setup_req_pdu(?ADDR_U2C, ?TEID_U2C),
@@ -357,6 +378,19 @@
?_assertMatch([_], s1ap_proxy:fetch_erab_list(Pid))].
+test_initial_context_setup_dup(#{handler := Pid}) ->
+ %% [eNB <- MME] INITIAL CONTEXT SETUP REQUEST
+ InitCtxSetupReqIn = initial_context_setup_req_pdu(?ADDR_U2C, ?TEID_U2C),
+ %% [eNB -> MME] INITIAL CONTEXT SETUP RESPONSE
+ InitCtxSetupRspIn = initial_context_setup_rsp_pdu(?ADDR_U2A, ?TEID_U2A),
+
+ [?_assertMatch({forward, _}, s1ap_proxy:process_pdu(Pid,
InitCtxSetupReqIn)),
+ ?_assertMatch({forward, _}, s1ap_proxy:process_pdu(Pid,
InitCtxSetupRspIn)),
+ %% duplicate INITIAL CONTEXT SETUP REQUEST results in the PDU being
dropped
+ ?_assertEqual({drop, InitCtxSetupReqIn}, s1ap_proxy:process_pdu(Pid,
InitCtxSetupReqIn)),
+ ?_assertMatch([_], s1ap_proxy:fetch_erab_list(Pid))].
+
+
test_ue_ctx_release_req(#{handler := Pid}) ->
%% [eNB <- MME] INITIAL CONTEXT SETUP REQUEST
InitCtxSetupReq = initial_context_setup_req_pdu(?ADDR_U2C, ?TEID_U2C),
--
To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/40993?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: I8e93492219100496fd8dc69e031310858ba9c7fb
Gerrit-Change-Number: 40993
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <[email protected]>
