fixeria has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/41622?usp=email )


Change subject: s1ap_proxy: move S1 SETUP handling to sctp_proxy
......................................................................

s1ap_proxy: move S1 SETUP handling to sctp_proxy

This is a prerequisite for the MME pooling support.  We need to receive
the S1 SETUP REQUEST PDU, extract the Global-eNB-ID and TAC(s), and use
this information to select an appropriate MME.  The PDU must also be
cached so it can be sent (or re-sent) to the selected MME(s).

The s1ap_proxy module no longer needs to parse S1 SETUP PDUs, as this
logic has been moved to the sctp_proxy and implemented using the newly
introduced s1ap_utils API.  However, s1ap_proxy still requires the
Global-eNB-ID for logging and per-eNB counters, so sctp_proxy now
provides it through the new s1ap_proxy:set_genb_id/2 API.

Change-Id: I9aa67732b418bcdf3f10b2db89a41dda26ee3d4e
Related: SYS#7052
---
M src/enb_registry.erl
M src/s1ap_proxy.erl
M src/sctp_proxy.erl
M test/s1ap_proxy_test.erl
4 files changed, 151 insertions(+), 113 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-s1gw 
refs/changes/22/41622/1

diff --git a/src/enb_registry.erl b/src/enb_registry.erl
index 90c3376..80d264b 100644
--- a/src/enb_registry.erl
+++ b/src/enb_registry.erl
@@ -65,7 +65,7 @@

 -type enb_event() :: {connecting, sctp_server:conn_info()}
                    | {connected, sctp_proxy:conn_info()}
-                   | {s1setup, s1ap_proxy:enb_info()}.
+                   | {s1setup, s1ap_utils:genb_id()}.

 -type enb_filter() :: {genb_id_str, string()}
                     | {enb_sctp_aid, gen_sctp:assoc_id()}
@@ -79,9 +79,8 @@
                       state := enb_state(),                        %% 
connection state
                       reg_time := integer(),                       %% 
registration time (monotonic)
                       uptime := non_neg_integer(),                 %% seconds 
since reg_time
-                      genb_id_str => string(),                     %% 
Global-eNB-ID
-                      enb_id => s1ap_utils:enb_id(),               %% eNB-ID
-                      plmn_id => s1ap_utils:plmn_id(),             %% PLMN-ID
+                      genb_id => s1ap_utils:genb_id(),             %% 
Global-eNB-ID
+                      genb_id_str => string(),                     %% 
Global-eNB-ID string
                       enb_conn_info => sctp_server:conn_info(),    %% eNB -> 
S1GW connection info
                       mme_conn_info => sctp_proxy:conn_info()      %% S1GW -> 
MME connection info
                      }.
@@ -307,8 +306,10 @@
     EnbInfo#{state => connected,
              mme_conn_info => ConnInfo};

-enb_handle_event(EnbInfo, {s1setup, Info}) ->
-    maps:merge(EnbInfo#{state => s1setup}, Info);
+enb_handle_event(EnbInfo, {s1setup, GENBId}) ->
+    EnbInfo#{state => s1setup,
+             genb_id => GENBId,
+             genb_id_str => s1ap_utils:genb_id_str(GENBId)};

 enb_handle_event(EnbInfo, Event) ->
     ?LOG_ERROR("Unhandled event: ~p", [Event]),
diff --git a/src/s1ap_proxy.erl b/src/s1ap_proxy.erl
index 43624d2..0903906 100644
--- a/src/s1ap_proxy.erl
+++ b/src/s1ap_proxy.erl
@@ -41,6 +41,7 @@
          handle_cast/2,
          terminate/2]).
 -export([start_link/1,
+         set_genb_id/2,
          process_pdu/2,
          fetch_erab/2,
          fetch_erab_list/1,
@@ -68,8 +69,6 @@
 -record(proxy_state, {owner :: pid(),
                       erabs :: dict:dict(K :: erab_uid(),
                                          V :: pid()),
-                      enb_id :: undefined | s1ap_utils:enb_id(),
-                      plmn_id :: undefined | s1ap_utils:plmn_id(),
                       genb_id_str :: undefined | string(),
                       mme_ue_id :: undefined | mme_ue_id(),
                       enb_ue_id :: undefined | enb_ue_id(),
@@ -80,13 +79,7 @@
 -type proxy_state() :: #proxy_state{}.
 -type proxy_action() :: forward | reply | drop.

--type enb_info() :: #{enb_id => s1ap_utils:enb_id(),
-                      plmn_id => s1ap_utils:plmn_id(),
-                      genb_id_str => string()
-                     }.
-
--export_type([proxy_action/0,
-              enb_info/0]).
+-export_type([proxy_action/0]).


 %% ------------------------------------------------------------------
@@ -98,6 +91,11 @@
     gen_server:start_link(?MODULE, [Owner], []).


+-spec set_genb_id(pid(), string()) -> ok | {error, term()}.
+set_genb_id(Pid, GlobalENBId) ->
+    gen_server:call(Pid, {?FUNCTION_NAME, GlobalENBId}).
+
+
 -type process_pdu_result() :: {proxy_action(), binary()}.
 -spec process_pdu(pid(), binary()) -> process_pdu_result().
 process_pdu(Pid, PDU) ->
@@ -145,6 +143,20 @@
             #proxy_state{erabs = ERABs} = S) ->
     {reply, dict:to_list(ERABs), S};

+handle_call({set_genb_id, GlobalENBId}, _From,
+            #proxy_state{genb_id_str = undefined} = S) ->
+    ?LOG_DEBUG("Global-eNB-ID is set: ~s", [GlobalENBId]),
+    %% use that as a context for logging
+    osmo_s1gw:set_log_prefix("eNB " ++ GlobalENBId),
+    %% register per-eNB metrics
+    ctr_reg_all(GlobalENBId),
+    {reply, ok, S#proxy_state{genb_id_str = GlobalENBId}};
+
+handle_call({set_genb_id, _GlobalENBId}, _From,
+            #proxy_state{genb_id_str = GlobalENBId} = S) ->
+    ?LOG_ERROR("Global-eNB-ID is already set: ~s", [GlobalENBId]),
+    {reply, {error, ealready}, S};
+
 handle_call(Info, From,
             #proxy_state{} = S) ->
     ?LOG_ERROR("unknown ~p() from ~p: ~p", [?FUNCTION_NAME, From, Info]),
@@ -198,15 +210,6 @@
     ok.


--spec enb_info(proxy_state()) -> enb_info().
-enb_info(S) ->
-    Info = #{enb_id => S#proxy_state.enb_id,
-             plmn_id => S#proxy_state.plmn_id,
-             genb_id_str => S#proxy_state.genb_id_str},
-    %% omit fields with Value =:= undefined
-    maps:filter(fun(_K, V) -> V =/= undefined end, Info).
-
-
 %% register a single per-eNB counter
 -spec ctr_reg(C, GlobalENBId) -> C
     when C :: [ctr | _],
@@ -308,25 +311,6 @@
          S1 :: proxy_state(),
          Result :: {forward | reply, s1ap_utils:s1ap_pdu()} | forward | drop.

-%% 9.1.8.4 S1 SETUP REQUEST
-handle_pdu({initiatingMessage,
-            #'InitiatingMessage'{procedureCode = ?'id-S1Setup',
-                                 value = C0}}, S0) ->
-    ?LOG_DEBUG("Processing S1 SETUP REQUEST"),
-    %% there's nothing to patch in this PDU, so we forward it as-is
-    %% TODO: check result of handle_ies(), inc. 
?S1GW_CTR_S1AP_PROXY_IN_PKT_PROC_ERROR
-    {_, S1} = handle_ies(?'id-Global-ENB-ID',
-                         C0#'S1SetupRequest'.protocolIEs, S0),
-    {forward, S1};
-
-%% 9.1.8.5 S1 SETUP RESPONSE
-handle_pdu({successfulOutcome,
-            #'SuccessfulOutcome'{procedureCode = ?'id-S1Setup'}}, S) ->
-    ?LOG_DEBUG("Processing S1 SETUP RESPONSE"),
-    signal_enb_info(S),
-    %% there's nothing to patch in this PDU, so we forward it as-is
-    {forward, S};
-

 %% 9.1.3.1 E-RAB SETUP REQUEST
 handle_pdu({Outcome = initiatingMessage,
@@ -642,32 +626,6 @@
          Result :: {handle_ie_result(),
                     proxy_state()}.

-handle_ie([?'id-Global-ENB-ID'],
-          #'Global-ENB-ID'{'pLMNidentity' = PLMNId,
-                           'eNB-ID' = ENBId} = C, S0) ->
-    %% store PLMNId/ENBId
-    S1 = S0#proxy_state{plmn_id = s1ap_utils:parse_plmn_id(PLMNId),
-                        enb_id = s1ap_utils:parse_enb_id(ENBId)},
-    ?LOG_INFO("Global-ENB-ID: PLMN-ID=~p, eNB-ID=~p",
-              [S1#proxy_state.plmn_id,
-               S1#proxy_state.enb_id]),
-    %% use that as a context for logging
-    GlobalENBId = s1ap_utils:genb_id_str(#{plmn_id => S1#proxy_state.plmn_id,
-                                           enb_id => S1#proxy_state.enb_id}),
-    osmo_s1gw:set_log_prefix("eNB " ++ GlobalENBId),
-    %% register per-eNB metrics
-    ctr_reg_all(GlobalENBId),
-    %% increment per-eNB ?S1GW_CTR_S1AP_PROXY_IN_PKT_ALL, if needed
-    %% this is needed to count S1 Setup Req correctly
-    S2 = case S0#proxy_state.genb_id_str of
-        GlobalENBId -> S1;
-        _ ->
-            Ctr = s1gw_metrics:enb_metric(?S1GW_CTR_S1AP_PROXY_IN_PKT_ALL, 
GlobalENBId),
-            s1gw_metrics:ctr_inc(Ctr),
-            S1#proxy_state{genb_id_str = GlobalENBId}
-    end,
-    {{ok, C}, S2};
-
 %% E-RAB SETUP REQUEST related IEs
 handle_ie([?'id-E-RABToBeSetupListBearerSUReq'], C, S) ->
     %% This IE contains a list of BearerSUReq, so patch inner IEs
@@ -1196,11 +1154,4 @@
     ok.


-%% Signal eNB info to the parent process
--spec signal_enb_info(proxy_state()) -> ok.
-signal_enb_info(#proxy_state{owner = Pid} = S) ->
-    Pid ! {?MODULE, {enb_info, enb_info(S)}},
-    ok.
-
-
 %% vim:set ts=4 sw=4 et:
diff --git a/src/sctp_proxy.erl b/src/sctp_proxy.erl
index 60f0c11..305d7d7 100644
--- a/src/sctp_proxy.erl
+++ b/src/sctp_proxy.erl
@@ -1,4 +1,4 @@
-%% Copyright (C) 2024 by sysmocom - s.f.m.c. GmbH <[email protected]>
+%% Copyright (C) 2024-2025 by sysmocom - s.f.m.c. GmbH <[email protected]>
 %% Author: Vadim Yanitskiy <[email protected]>
 %%
 %% All Rights Reserved
@@ -38,6 +38,8 @@

 -export([init/1,
          callback_mode/0,
+         wait_s1setup_req/3,
+         wait_s1setup_rsp/3,
          connecting/3,
          connected/3,
          code_change/4,
@@ -53,6 +55,8 @@

 -include("s1gw_metrics.hrl").

+-include("S1AP-Constants.hrl").
+

 -type conn_info() :: #{state := atom(),
                        handler := pid(),
@@ -66,7 +70,7 @@
                 mme_aid :: undefined | gen_sctp:assoc_id(),
                 enb_conn_info :: sctp_server:conn_info(),
                 mme_conn_cfg :: sctp_client:cfg(),
-                tx_queue :: [binary()],
+                s1setup_req :: undefined | binary(),
                 sock :: undefined | gen_sctp:sctp_socket(),
                 enb_handle :: enb_registry:enb_handle(),
                 genb_id_str :: undefined | string(),
@@ -111,12 +115,12 @@

 init([EnbConnInfo, MmeConnCfg]) ->
     {ok, EnbHandle} = enb_registry:enb_register(),
+    enb_registry:enb_event(EnbHandle, {connecting, EnbConnInfo}),
     {ok, Pid} = s1ap_proxy:start_link(self()),
-    {ok, connecting,
+    {ok, wait_s1setup_req,
      #state{enb_aid = maps:get(aid, EnbConnInfo),
             enb_conn_info = EnbConnInfo,
             mme_conn_cfg = MmeConnCfg,
-            tx_queue = [],
             enb_handle = EnbHandle,
             handler = Pid}}.

@@ -125,13 +129,50 @@
     [state_functions, state_enter].


+%% WAIT_S1SETUP_REQ state
+wait_s1setup_req(enter, _OldState, S) ->
+    ?LOG_INFO("State enter: ~p", [?FUNCTION_NAME]),
+    {next_state, ?FUNCTION_NAME, S,
+     [{state_timeout, 5_000, s1setup_req_timeout}]};
+
+%% Handle S1 SETUP REQUEST timeout
+wait_s1setup_req(state_timeout, s1setup_req_timeout, _S) ->
+    ?LOG_ERROR("Timeout waiting for S1 SETUP REQUEST from eNB"),
+    {stop, {shutdown, s1setup_req_timeout}};
+
+%% Handle PDUs coming from the eNB
+wait_s1setup_req(cast, {send_data, Data}, S) ->
+    ?LOG_DEBUG("Rx S1AP PDU from eNB: ~p", [Data]),
+    case s1ap_utils:parse_pdu(Data) of
+        {{?'id-S1Setup', initiatingMessage}, IEs} ->
+            %% fetch the Global-eNB-ID IE, convert it to a string
+            GENBId = proplists:get_value(?'id-Global-ENB-ID', IEs),
+            GlobalENBId = s1ap_utils:genb_id_str(GENBId),
+            %% use it as the logging prefix
+            osmo_s1gw:set_log_prefix("eNB " ++ GlobalENBId),
+            ?LOG_INFO("Rx S1 SETUP REQUEST from eNB"),
+            %% signal the Global-eNB-ID to other modules
+            s1ap_proxy:set_genb_id(S#state.handler, GlobalENBId),
+            enb_registry:enb_event(S#state.enb_handle, {s1setup, GENBId}),
+            gtpu_kpi_enb_register(S#state{genb_id_str = GlobalENBId}),
+            {next_state, connecting,
+             S#state{s1setup_req = Data,
+                     genb_id_str = GlobalENBId}};
+        {{Proc, Type}, IEs} ->
+            ?LOG_ERROR("Rx unexpected S1AP PDU from eNB: ~p/~p, ~p", [Proc, 
Type, IEs]),
+            {stop, {shutdown, s1setup_error}};
+        {error, _Error} ->
+            {stop, {shutdown, s1setup_error}}
+    end;
+
+wait_s1setup_req(Event, EventData, S) ->
+    handle_event(?FUNCTION_NAME, Event, EventData, S).
+
+
 %% CONNECTING state
 connecting(enter, OldState,
-           #state{enb_conn_info = EnbConnInfo,
-                  mme_conn_cfg = MmeConnCfg,
-                  enb_handle = Handle} = S) ->
+           #state{mme_conn_cfg = MmeConnCfg} = S) ->
     ?LOG_INFO("State change: ~p -> ~p", [OldState, ?FUNCTION_NAME]),
-    ok = enb_registry:enb_event(Handle, {?FUNCTION_NAME, EnbConnInfo}),
     %% Initiate connection establishment with the MME
     {ok, Sock} = sctp_client:connect(MmeConnCfg),
     %% loop transition to enable state_timeout
@@ -142,12 +183,10 @@
 connecting(state_timeout, conn_est_timeout, _S) ->
     {stop, {shutdown, conn_est_timeout}};

-%% Handle an eNB -> MME data forwarding request (queue)
-connecting(cast, {send_data, Data},
-           #state{tx_queue = Pending} = S) ->
-    s1gw_metrics:ctr_inc(?S1GW_CTR_S1AP_PROXY_UPLINK_PACKETS_QUEUED),
-    s1gw_metrics:gauge_inc(?S1GW_GAUGE_S1AP_PROXY_UPLINK_PACKETS_QUEUED),
-    {keep_state, S#state{tx_queue = [Data | Pending]}};
+%% Handle PDUs coming from the eNB
+connecting(cast, {send_data, Data}, _S) ->
+    ?LOG_ERROR("Rx unexpected S1AP PDU from eNB: ~p", [Data]),
+    keep_state_and_data;

 %% Handle an #sctp_assoc_change event (connection state)
 connecting(info, {sctp, _Socket, MmeAddr, MmePort,
@@ -157,7 +196,10 @@
         comm_up ->
             ?LOG_NOTICE("MME connection (id=~p, ~p:~p) established",
                         [Aid, MmeAddr, MmePort]),
-            {next_state, connected, S#state{mme_aid = Aid}};
+            %% send the S1 SETUP REQUEST PDU to the MME
+            sctp_send_from_enb(S#state.s1setup_req,
+                               S#state{mme_aid = Aid}),
+            {next_state, wait_s1setup_rsp, S#state{mme_aid = Aid}};
         _ ->
             ?LOG_NOTICE("MME connection establishment failed: ~p", 
[ConnState]),
             {stop, {shutdown, conn_est_fail}}
@@ -167,15 +209,75 @@
     handle_event(?FUNCTION_NAME, Event, EventData, S).


+%% WAIT_S1SETUP_RSP state
+wait_s1setup_rsp(enter, OldState, S) ->
+    ?LOG_INFO("State change: ~p -> ~p", [OldState, ?FUNCTION_NAME]),
+    {next_state, ?FUNCTION_NAME, S,
+     [{state_timeout, 5_000, s1setup_rsp_timeout}]};
+
+%% Handle S1 SETUP RESPONSE timeout
+wait_s1setup_rsp(state_timeout, s1setup_rsp_timeout, _S) ->
+    ?LOG_ERROR("Timeout waiting for S1 SETUP RESPONSE from MME"),
+    {stop, {shutdown, s1setup_rsp_timeout}};
+
+%% Handle PDUs coming from the eNB
+wait_s1setup_rsp(cast, {send_data, Data}, _S) ->
+    ?LOG_ERROR("Rx unexpected S1AP PDU from eNB: ~p", [Data]),
+    keep_state_and_data;
+
+%% Handle PDUs coming from the MME
+wait_s1setup_rsp(info, {sctp, _Socket, MmeAddr, MmePort,
+                        {[#sctp_sndrcvinfo{assoc_id = MmeAid,
+                                           stream = SID,
+                                           ssn = SSN,
+                                           tsn = TSN}], Data}},
+                 #state{mme_aid = MmeAid} = S) ->
+    ?LOG_DEBUG("MME connection (id=~p, ~p:~p) -> eNB: ~p",
+               [MmeAid, MmeAddr, MmePort,
+                #{tsn => TSN, sid => SID, ssn => SSN,
+                  len => byte_size(Data), data => Data}]),
+    case s1ap_utils:parse_pdu(Data) of
+        {{?'id-S1Setup', successfulOutcome}, _IEs} ->
+            ?LOG_INFO("Rx S1 SETUP RESPONSE from MME"),
+            sctp_send_from_mme(Data, S),
+            {next_state, connected, S};
+        {{?'id-S1Setup', unsuccessfulOutcome}, _IEs} ->
+            ?LOG_NOTICE("Rx S1 SETUP FAILURE from MME"),
+            sctp_send_from_mme(Data, S),
+            {stop, {shutdown, s1setup_error}};
+        {{Proc, Type}, IEs} ->
+            ?LOG_ERROR("Rx unexpected S1AP PDU from MME: ~p/~p, ~p", [Proc, 
Type, IEs]),
+            {stop, {shutdown, s1setup_error}};
+        {error, _Error} ->
+            {stop, {shutdown, s1setup_error}}
+    end;
+
+%% Handle an #sctp_assoc_change event (MME connection state)
+%% We may loose connection while waiting for the S1 SETUP RESPONSE
+wait_s1setup_rsp(info, {sctp, _Socket, MmeAddr, MmePort,
+                 {[], #sctp_assoc_change{state = ConnState,
+                                         assoc_id = Aid}}}, S) ->
+    case ConnState of
+        comm_up ->
+            ?LOG_NOTICE("MME connection (id=~p, ~p:~p) is already 
established?!?",
+                        [Aid, MmeAddr, MmePort]),
+            {keep_state, S};
+        _ ->
+            ?LOG_NOTICE("MME connection state: ~p", [ConnState]),
+            {stop, {shutdown, conn_fail}}
+    end;
+
+wait_s1setup_rsp(Event, EventData, S) ->
+    handle_event(?FUNCTION_NAME, Event, EventData, S).
+
+
 %% CONNECTED state
 connected(enter, OldState,
-          #state{enb_handle = Handle} = S0) ->
+          #state{enb_handle = Handle} = S) ->
     ?LOG_INFO("State change: ~p -> ~p", [OldState, ?FUNCTION_NAME]),
-    MmeConnInfo = conn_info(?FUNCTION_NAME, S0),
+    MmeConnInfo = conn_info(?FUNCTION_NAME, S),
     ok = enb_registry:enb_event(Handle, {?FUNCTION_NAME, MmeConnInfo}),
-    %% Send pending eNB -> MME messages (if any)
-    S1 = sctp_send_pending(S0),
-    {keep_state, S1};
+    {keep_state, S};

 %% Handle an eNB -> MME data forwarding request (forward)
 connected(cast, {send_data, Data}, S0) ->
@@ -210,14 +312,6 @@
     sctp_send_from_mme(Data, S),
     {keep_state, S};

-%% eNB info indication from s1ap_proxy
-connected(info, {s1ap_proxy, {enb_info, EnbInfo}},
-          #state{enb_handle = EnbHandle} = S0) ->
-    enb_registry:enb_event(EnbHandle, {s1setup, EnbInfo}),
-    S1 = S0#state{genb_id_str = maps:get(genb_id_str, EnbInfo)},
-    gtpu_kpi_enb_register(S1),
-    {keep_state, S1};
-
 %% eNB's GTP-U address indication from s1ap_proxy
 connected(info, {s1ap_proxy, {enb_addr, Addr}}, S) ->
     gtpu_kpi_enb_set_addr({s1ap, Addr}),
@@ -315,15 +409,6 @@
     end.


-%% Send pending messages to the MME
--spec sctp_send_pending(state()) -> state().
-sctp_send_pending(#state{tx_queue = Pending} = S) ->
-    [] = lists:filter(fun(Data) -> sctp_send_from_enb(Data, S) =/= ok end,
-                      lists:reverse(Pending)),
-    s1gw_metrics:gauge_set(?S1GW_GAUGE_S1AP_PROXY_UPLINK_PACKETS_QUEUED, 0),
-    S#state{tx_queue = []}.
-
-
 -spec conn_info(atom(), state()) -> conn_info().
 conn_info(State, #state{mme_conn_cfg = MmeConnCfg} = S) ->
     Info = #{state => State,
diff --git a/test/s1ap_proxy_test.erl b/test/s1ap_proxy_test.erl
index e376304..2d748bf 100644
--- a/test/s1ap_proxy_test.erl
+++ b/test/s1ap_proxy_test.erl
@@ -32,6 +32,7 @@
     gtpu_kpi:start_link(#{enable => false}),
     {ok, EnbHandle} = enb_registry:enb_register(),
     {ok, Pid} = s1ap_proxy:start_link(self()),
+    s1ap_proxy:set_genb_id(Pid, ?GlobalENBId),
     ok = enb_registry:enb_unregister(EnbHandle),
     #{handler => Pid}.


--
To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/41622?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: I9aa67732b418bcdf3f10b2db89a41dda26ee3d4e
Gerrit-Change-Number: 41622
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <[email protected]>

Reply via email to