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))].