fixeria has submitted this change. (
https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/40994?usp=email )
Change subject: s1ap_proxy: remove eNB-ID from erab_uid()
......................................................................
s1ap_proxy: remove eNB-ID from erab_uid()
This patch prepares for a follow-up change implementing handling of
the HANDOVER related PDUs. The problem is that the HANDOVER REQUEST
PDU contains MME-UE-S1AP-ID, but not eNB-UE-S1AP-ID, while the E-RAB
registry requires both to be part of the erab_uid() tuple.
In the scope of an S1-connection, a pair of MME-UE-S1AP-ID and
E-RAB-ID values is enough to identify an E-RAB unambiguously,
thus we can safely remove eNB-UE-S1AP-ID from erab_uid().
Change-Id: I7277d19080795600252306dbfa0c733f996e026e
---
M src/s1ap_proxy.erl
M test/s1ap_proxy_test.erl
2 files changed, 30 insertions(+), 43 deletions(-)
Approvals:
pespin: Looks good to me, but someone else must approve
jolly: Looks good to me, but someone else must approve
Jenkins Builder: Verified
fixeria: Looks good to me, approved
diff --git a/src/s1ap_proxy.erl b/src/s1ap_proxy.erl
index 0195e39..0a08707 100644
--- a/src/s1ap_proxy.erl
+++ b/src/s1ap_proxy.erl
@@ -66,7 +66,8 @@
-type mme_ue_id() :: 0..16#ffffffff.
-type enb_ue_id() :: 0..16#ffffff.
-type erab_id() :: 0..16#ff.
--type erab_uid() :: {mme_ue_id(), enb_ue_id(), erab_id()}.
+-type erab_uid() :: {mme_ue_id(), erab_id()}.
+
-type plmn_id() :: {MCC :: nonempty_string(),
MNC :: nonempty_string()}.
@@ -185,30 +186,24 @@
-spec erab_uid(erab_id(), proxy_state()) -> erab_uid().
-erab_uid(ERABId, #proxy_state{mme_ue_id = MmeUeId,
- enb_ue_id = EnbUeId}) ->
- {MmeUeId, EnbUeId, ERABId}.
+erab_uid(ERABId, #proxy_state{mme_ue_id = MmeUeId}) ->
+ {MmeUeId, ERABId}.
--spec erab_for_each(UID, Fun, ERABs) -> ok
- when UID :: {mme_ue_id(), enb_ue_id()} | {mme_ue_id()},
+-spec erab_for_each(MMEUeId, Fun, ERABs) -> ok
+ when MMEUeId :: mme_ue_id(),
Fun :: fun((pid()) -> term()),
ERABs :: [{erab_uid(), pid()}].
-erab_for_each({MMEUEId, ENBUEId} = UID, Fun,
- [{{MMEUEId, ENBUEId, _}, Pid} | ERABs]) ->
- Fun(Pid), %% matched by a pair of {MME,eNB}-UE-S1AP-ID
- erab_for_each(UID, Fun, ERABs);
+erab_for_each(MMEUEId, Fun,
+ [{{MMEUEId, _}, Pid} | ERABs]) ->
+ Fun(Pid), %% it's a match!
+ erab_for_each(MMEUEId, Fun, ERABs);
-erab_for_each({MMEUEId} = UID, Fun,
- [{{MMEUEId, _, _}, Pid} | ERABs]) ->
- Fun(Pid), %% matched by an MME-UE-S1AP-ID
- erab_for_each(UID, Fun, ERABs);
-
-erab_for_each(UID, Fun,
+erab_for_each(MMEUeId, Fun,
[_ERAB | ERABs]) ->
- erab_for_each(UID, Fun, ERABs);
+ erab_for_each(MMEUeId, Fun, ERABs);
-erab_for_each(_UID, _Fun, []) ->
+erab_for_each(_MMEUeId, _Fun, []) ->
ok.
@@ -585,13 +580,11 @@
#proxy_state{erabs = ERABs} = S) ->
?LOG_DEBUG("Processing UE CONTEXT RELEASE REQUEST"),
ctr_inc(?S1GW_CTR_S1AP_PROXY_IN_PKT_RELEASE_CTX_REQ, S),
- %% fetch {MME,eNB}-UE-S1AP-ID values (mandatory IEs)
+ %% fetch MME-UE-S1AP-ID value (mandatory IE)
#'ProtocolIE-Field'{id = ?'id-MME-UE-S1AP-ID',
value = MMEUEId} = lists:nth(1, IEs),
- #'ProtocolIE-Field'{id = ?'id-eNB-UE-S1AP-ID',
- value = ENBUEId} = lists:nth(2, IEs),
- %% poke E-RAB FSMs with the matching {MME,eNB}-UE-S1AP-ID
- erab_for_each({MMEUEId, ENBUEId},
+ %% poke E-RAB FSMs with the matching MME-UE-S1AP-ID
+ erab_for_each(MMEUEId,
fun erab_fsm:erab_release_ind/1,
dict:to_list(ERABs)),
{forward, S}; %% forward as-is, there's nothing to patch
@@ -603,21 +596,17 @@
#proxy_state{erabs = ERABs} = S) ->
?LOG_DEBUG("Processing UE CONTEXT RELEASE COMMAND"),
ctr_inc(?S1GW_CTR_S1AP_PROXY_IN_PKT_RELEASE_CTX_CMD, S),
+ %% fetch MME-UE-S1AP-ID value from UE-S1AP-IDs (mandatory IE)
#'ProtocolIE-Field'{id = ?'id-UE-S1AP-IDs',
value = S1APIDs} = lists:nth(1, IEs),
- case S1APIDs of
- {'uE-S1AP-ID-pair', #'UE-S1AP-ID-pair'{'mME-UE-S1AP-ID' = MMEUEId,
- 'eNB-UE-S1AP-ID' = ENBUEId}} ->
- %% poke E-RAB FSMs with the matching {MME,eNB}-UE-S1AP-ID
- erab_for_each({MMEUEId, ENBUEId},
- fun erab_fsm:erab_release_cmd/1,
- dict:to_list(ERABs));
- {'mME-UE-S1AP-ID', MMEUEId} ->
- %% poke E-RAB FSMs with the matching MME-UE-S1AP-ID
- erab_for_each({MMEUEId},
- fun erab_fsm:erab_release_cmd/1,
- dict:to_list(ERABs))
+ MMEUEId = case S1APIDs of
+ {'uE-S1AP-ID-pair', #'UE-S1AP-ID-pair'{'mME-UE-S1AP-ID' = Val}} -> Val;
+ {'mME-UE-S1AP-ID', Val} -> Val
end,
+ %% poke E-RAB FSMs with the matching MME-UE-S1AP-ID
+ erab_for_each(MMEUEId,
+ fun erab_fsm:erab_release_cmd/1,
+ dict:to_list(ERABs)),
{forward, S}; %% forward as-is, there's nothing to patch
%% 9.1.4.7 UE CONTEXT RELEASE COMPLETE
@@ -627,13 +616,11 @@
#proxy_state{erabs = ERABs} = S) ->
?LOG_DEBUG("Processing UE CONTEXT RELEASE COMPLETE"),
ctr_inc(?S1GW_CTR_S1AP_PROXY_IN_PKT_RELEASE_CTX_COMPL, S),
- %% fetch {MME,eNB}-UE-S1AP-ID values (mandatory IEs)
+ %% fetch MME-UE-S1AP-ID value (mandatory IE)
#'ProtocolIE-Field'{id = ?'id-MME-UE-S1AP-ID',
value = MMEUEId} = lists:nth(1, IEs),
- #'ProtocolIE-Field'{id = ?'id-eNB-UE-S1AP-ID',
- value = ENBUEId} = lists:nth(2, IEs),
- %% poke E-RAB FSMs with the matching {MME,eNB}-UE-S1AP-ID
- erab_for_each({MMEUEId, ENBUEId},
+ %% poke E-RAB FSMs with the matching MME-UE-S1AP-ID
+ erab_for_each(MMEUEId,
fun erab_fsm:erab_release_rsp/1,
dict:to_list(ERABs)),
{forward, S}; %% forward as-is, there's nothing to patch
@@ -1079,7 +1066,7 @@
enb_ue_id = EnbUeId}) ->
%% FIXME: Currently we respond with E-RAB-ID of the first E-RAB in the
registry.
%% Instead, we need to iterate over E-RABs in the REQUEST and reject them
all.
- [{_, _, FirstERABid}|_] = dict:fetch_keys(ERABs),
+ [{_, FirstERABid}|_] = dict:fetch_keys(ERABs),
Cause = {transport, 'transport-resource-unavailable'},
ERABitem = #'E-RABItem'{'e-RAB-ID' = FirstERABid,
cause = Cause},
diff --git a/test/s1ap_proxy_test.erl b/test/s1ap_proxy_test.erl
index 8f338c9..60b09dc 100644
--- a/test/s1ap_proxy_test.erl
+++ b/test/s1ap_proxy_test.erl
@@ -128,7 +128,7 @@
?_assertMetricENB(?S1GW_CTR_S1AP_PROXY_OUT_PKT_FWD_ALL, 2 + 2),
?_assertMetricENB(?S1GW_CTR_S1AP_PROXY_OUT_PKT_FWD_PROC, 2),
?_assertMetricENB(?S1GW_CTR_S1AP_PROXY_OUT_PKT_FWD_UNMODIFIED, 2),
- ?_assertMatch({ok, _}, s1ap_proxy:fetch_erab(Pid, {7, 9, 6})),
+ ?_assertMatch({ok, _}, s1ap_proxy:fetch_erab(Pid, {7, 6})),
?_assertMatch([_], s1ap_proxy:fetch_erab_list(Pid))].
@@ -164,7 +164,7 @@
?_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({ok, _}, s1ap_proxy:fetch_erab(Pid, {7, 6})),
?_assertMatch([_], s1ap_proxy:fetch_erab_list(Pid))].
--
To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/40994?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: erlang/osmo-s1gw
Gerrit-Branch: master
Gerrit-Change-Id: I7277d19080795600252306dbfa0c733f996e026e
Gerrit-Change-Number: 40994
Gerrit-PatchSet: 3
Gerrit-Owner: fixeria <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-Reviewer: jolly <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
