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

Change subject: enb_proxy: split conn_info() into mme_conn_info() and proxy_info() ......................................................................

enb_proxy: split conn_info() into mme_conn_info() and proxy_info()

The old conn_info() conflated two distinct concerns: the MME SCTP connection info stored in enb_registry (aid, saddr, sport) and the broader operational state used for introspection (handler pid, enb connection info, etc.). Mixing them forced enb_registry to hold a handler pid it has no business knowing about, and required rest_server to extract that pid just to reach s1ap_proxy for E-RAB listing.

Split into two distinct types:

* mme_conn_info() - pure MME SCTP connection info (aid, saddr, sport), stored in the enb_registry and signalled via notify_mme_comm_up/2. The `mme_` prefix is dropped from field names as the type name provides the context.

* proxy_info() - richer operational snapshot (handler, enb_handle, enb_conn_info, mme_conn_info, genb_id_str, mme_info), returned by fetch_info/1 for introspection/debugging purposes.

Additionally:

* Add fetch_erab_list/1 to enb_proxy, delegating internally to s1ap_proxy:fetch_erab_list/1 via the cached handler pid. This allows the rest_server to obtain a list of E-RAB without having to obtain pid of the s1ap_proxy and interact with it.

* Remove separate enb_aid/mme_aid/mme_saddr/mme_sport state fields; enb_aid is now read directly from enb_conn_info, and the MME fields are grouped in mme_conn_info.

Change-Id: Ia428ceb4762f972211e9b790688dc89fb5b8a274 Related: SYS#7066 --- M src/enb_proxy.erl M src/enb_registry.erl M src/rest_server.erl 3 files changed, 66 insertions(+), 68 deletions(-)

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

diff --git a/src/enb_proxy.erl b/src/enb_proxy.erl index 6a7254e..c281b33 100644 --- a/src/enb_proxy.erl +++ b/src/enb_proxy.erl @@ -47,6 +47,7 @@ -export([start_link/2, send_data/2, fetch_info/1, + fetch_erab_list/1, shutdown/1]).

-include_lib("kernel/include/logger.hrl"). @@ -58,18 +59,21 @@ -include("S1AP-Constants.hrl").

--type conn_info() :: #{handler := pid(), - enb_aid := gen_sctp:assoc_id(), - mme_aid => gen_sctp:assoc_id(), - mme_saddr => inet:ip_address(), - mme_sport => inet:port_number() - }. +-type mme_conn_info() :: #{aid := gen_sctp:assoc_id(), + saddr := inet:ip_address(), + sport := inet:port_number() + }.

--record(state, {enb_aid :: gen_sctp:assoc_id(), - mme_aid :: undefined | gen_sctp:assoc_id(), - mme_saddr :: undefined | inet:ip_address(), - mme_sport :: undefined | inet:port_number(), - enb_conn_info :: sctp_server:conn_info(), +-type proxy_info() :: #{handler := pid(), + enb_handle := enb_registry:enb_handle(), + enb_conn_info := sctp_server:conn_info(), + mme_conn_info => mme_conn_info(), + mme_info => mme_registry:mme_info(), + genb_id_str => string() + }. + +-record(state, {enb_conn_info :: sctp_server:conn_info(), + mme_conn_info :: undefined | mme_conn_info(), s1setup_req :: undefined | binary(), tried_mmes :: [mme_registry:mme_name()], enb_tacs :: undefined | [s1ap_utils:tac()], @@ -81,7 +85,8 @@

-type state() :: #state{}.

--export_type([conn_info/0]). +-export_type([mme_conn_info/0, + proxy_info/0]).

%% ------------------------------------------------------------------ @@ -101,11 +106,16 @@ gen_statem:cast(Pid, {send_data, Data}).

--spec fetch_info(pid()) -> conn_info(). +-spec fetch_info(pid()) -> proxy_info(). fetch_info(Pid) -> gen_statem:call(Pid, ?FUNCTION_NAME).

+-spec fetch_erab_list(pid()) -> [{term(), pid()}]. +fetch_erab_list(Pid) -> + gen_statem:call(Pid, ?FUNCTION_NAME). + + -spec shutdown(pid()) -> ok. shutdown(Pid) -> gen_statem:stop(Pid). @@ -119,10 +129,7 @@ {ok, EnbHandle} = enb_registry:enb_register(), {ok, Pid} = s1ap_proxy:start_link(self()), {ok, wait_s1setup_req, - #state{enb_aid = maps:get(aid, EnbConnInfo), - mme_saddr = undefined, - mme_sport = undefined, - enb_conn_info = EnbConnInfo, + #state{enb_conn_info = EnbConnInfo, enb_handle = EnbHandle, tried_mmes = [], handler = Pid}}. @@ -210,9 +217,7 @@ enb_registry:notify_mme_connecting(S#state.enb_handle, MmeInfo), {next_state, ?FUNCTION_NAME, %% loop transition to enable state_timeout S#state{sock = Sock, - mme_aid = undefined, - mme_saddr = undefined, - mme_sport = undefined, + mme_conn_info = undefined, tried_mmes = [MmeName | TriedMMEs]}, [{state_timeout, 2_000, conn_est_timeout}]}; error -> @@ -251,14 +256,15 @@ {ok, {MmeLAddr, MmeLPort}} = inet:sockname(Socket), ?LOG_NOTICE("MME ~p: connection (id=~p, ~p:~p) established", [MmeName, Aid, MmeAddr, MmePort]), - NewS = S#state{mme_aid = Aid, - mme_saddr = MmeLAddr, - mme_sport = MmeLPort}, + MmeConnInfo = #{aid => Aid, + saddr => MmeLAddr, + sport => MmeLPort}, %% send the S1 SETUP REQUEST PDU to the MME - sctp_send_from_enb(S#state.s1setup_req, NewS), + sctp_send_from_enb(S#state.s1setup_req, + S#state{mme_conn_info = MmeConnInfo}), %% surface MME connection details before S1 Setup completes - enb_registry:notify_mme_comm_up(S#state.enb_handle, conn_info(NewS)), - {next_state, wait_s1setup_rsp, NewS}; + enb_registry:notify_mme_comm_up(S#state.enb_handle, MmeConnInfo), + {next_state, wait_s1setup_rsp, S#state{mme_conn_info = MmeConnInfo}}; _ -> ?LOG_NOTICE("MME ~p: connection establishment failed: ~p", [MmeName, ConnState]), @@ -299,7 +305,7 @@ stream = SID, ssn = SSN, tsn = TSN}], Data}}, - #state{mme_aid = MmeAid} = S) -> + #state{mme_conn_info = #{aid := MmeAid}} = S) -> ?LOG_DEBUG("MME connection (id=~p, ~p:~p) -> eNB: ~p", [MmeAid, MmeAddr, MmePort, #{tsn => TSN, sid => SID, ssn => SSN, @@ -382,7 +388,7 @@ stream = SID, ssn = SSN, tsn = TSN}], Data}}, - #state{mme_aid = MmeAid} = S) -> + #state{mme_conn_info = #{aid := MmeAid}} = S) -> ?LOG_DEBUG("MME connection (id=~p, ~p:~p) -> eNB: ~p", [MmeAid, MmeAddr, MmePort, #{tsn => TSN, sid => SID, ssn => SSN, @@ -401,7 +407,19 @@

%% Event handler for all states handle_event(_State, {call, From}, fetch_info, S) -> - {keep_state_and_data, {reply, From, conn_info(S)}}; + Info = #{handler => S#state.handler, + enb_handle => S#state.enb_handle, + enb_conn_info => S#state.enb_conn_info, + mme_conn_info => S#state.mme_conn_info, + %% TODO: mme_info => S#state.mme_info, + genb_id_str => S#state.genb_id_str + }, + Reply = maps:filter(fun(_K, V) -> V =/= undefined end, Info), + {keep_state_and_data, {reply, From, Reply}}; + +handle_event(_State, {call, From}, fetch_erab_list, S) -> + ERABs = s1ap_proxy:fetch_erab_list(S#state.handler), + {keep_state_and_data, {reply, From, ERABs}};

handle_event(State, {call, From}, EventData, _S) -> ?LOG_ERROR("Unexpected call in state ~p: ~p", [State, EventData]), @@ -445,8 +463,8 @@ -spec sctp_send_from_enb(binary(), state()) -> ok | {error, term()}. sctp_send_from_enb(Data, #state{sock = Sock, - enb_aid = EnbAid, - mme_aid = MmeAid, + enb_conn_info = #{aid := EnbAid}, + mme_conn_info = #{aid := MmeAid}, handler = Pid}) -> case s1ap_proxy:process_pdu(Pid, Data) of {forward, FwdData} -> @@ -462,8 +480,8 @@ -spec sctp_send_from_mme(binary(), state()) -> ok | {error, term()}. sctp_send_from_mme(Data, #state{sock = Sock, - enb_aid = EnbAid, - mme_aid = MmeAid, + enb_conn_info = #{aid := EnbAid}, + mme_conn_info = #{aid := MmeAid}, handler = Pid}) -> case s1ap_proxy:process_pdu(Pid, Data) of {forward, FwdData} -> @@ -475,16 +493,6 @@ end.

--spec conn_info(state()) -> conn_info(). -conn_info(S) -> - Info = #{handler => S#state.handler, - enb_aid => S#state.enb_aid, - mme_aid => S#state.mme_aid, - mme_saddr => S#state.mme_saddr, - mme_sport => S#state.mme_sport}, - maps:filter(fun(_K, V) -> V =/= undefined end, Info). - - -spec gtpu_kpi_enb_register(state()) -> ok. gtpu_kpi_enb_register(#state{genb_id_str = GlobalENBId, enb_conn_info = EnbConnInfo}) -> @@ -566,9 +574,9 @@

-spec close_conn(state()) -> ok | {error, term()}. -close_conn(#state{mme_aid = undefined}) -> ok; +close_conn(#state{mme_conn_info = undefined}) -> ok;

-close_conn(#state{sock = Sock, mme_aid = MmeAid}) -> +close_conn(#state{sock = Sock, mme_conn_info = #{aid := MmeAid}}) -> sctp_common:shutdown({Sock, MmeAid}).

diff --git a/src/enb_registry.erl b/src/enb_registry.erl index 2bf6e47..a65ecbf 100644 --- a/src/enb_registry.erl +++ b/src/enb_registry.erl @@ -66,7 +66,7 @@

-type enb_prop() :: {state, atom()} | {enb_conn_info, sctp_server:conn_info()} - | {mme_conn_info, undefined | enb_proxy:conn_info()} + | {mme_conn_info, undefined | enb_proxy:mme_conn_info()} | {mme_info, mme_registry:mme_info()} | {genb_id, s1ap_utils:genb_id()}. -type enb_proplist() :: [enb_prop()]. @@ -86,7 +86,7 @@ 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 => enb_proxy:conn_info(), %% S1GW -> MME connection info + mme_conn_info => enb_proxy:mme_conn_info(), %% S1GW -> MME connection info mme_info => mme_registry:mme_info() %% selected MME configuration }.

@@ -139,7 +139,7 @@

%% MME SCTP comm_up; connection details (aid, saddr, sport) are now known --spec notify_mme_comm_up(enb_handle(), enb_proxy:conn_info()) -> ok. +-spec notify_mme_comm_up(enb_handle(), enb_proxy:mme_conn_info()) -> ok. notify_mme_comm_up(Handle, ConnInfo) -> enb_update(Handle, [{mme_conn_info, ConnInfo}]).

@@ -381,7 +381,7 @@ fun(_, Item) -> enb_filter_by_sub_field({enb_conn_info, aid, Aid}, Item) end;

enb_filter({mme_sctp_aid, Aid}) -> - fun(_, Item) -> enb_filter_by_sub_field({mme_conn_info, mme_aid, Aid}, Item) end; + fun(_, Item) -> enb_filter_by_sub_field({mme_conn_info, aid, Aid}, Item) end;

enb_filter({enb_addr_port, {Addr, Port}}) -> fun(_, Item) -> enb_filter_by_sub_field({enb_conn_info, addr, Addr}, Item) andalso diff --git a/src/rest_server.erl b/src/rest_server.erl index 0107427..5f6257e 100644 --- a/src/rest_server.erl +++ b/src/rest_server.erl @@ -427,11 +427,12 @@

-spec enb_item(enb_registry:enb_info()) -> map(). enb_item(EnbInfo) -> + ERABs = enb_proxy:fetch_erab_list(maps:get(pid, EnbInfo)), M0 = #{handle => maps:get(handle, EnbInfo), pid => pid_to_list(maps:get(pid, EnbInfo)), state => maps:get(state, EnbInfo), uptime => maps:get(uptime, EnbInfo), - erab_count => 0}, + erab_count => length(ERABs)}, M1 = enb_item_add_enb_info(M0, EnbInfo), M2 = enb_item_add_enb_conn_info(M1, EnbInfo), M3 = enb_item_add_mme_conn_info(M2, EnbInfo), @@ -459,18 +460,10 @@

-spec enb_item_add_mme_conn_info(map(), enb_registry:enb_info()) -> map(). enb_item_add_mme_conn_info(M0, #{mme_conn_info := ConnInfo}) -> - Pid = maps:get(handler, ConnInfo), - ERABs = s1ap_proxy:fetch_erab_list(Pid), - M1 = M0#{erab_count => length(ERABs)}, - %% mme_aid/mme_saddr/mme_sport are only present once the MME SCTP - %% connection is fully established (comm_up); handle them together. - case maps:find(mme_aid, ConnInfo) of - {ok, Aid} -> - M1#{mme_sctp_aid => Aid, - mme_saddr => inet:ntoa(maps:get(mme_saddr, ConnInfo)), - mme_sport => maps:get(mme_sport, ConnInfo)}; - error -> M1 - end; + M0#{mme_saddr => inet:ntoa(maps:get(saddr, ConnInfo)), + mme_sport => maps:get(sport, ConnInfo), + mme_sctp_aid => maps:get(aid, ConnInfo) + };

enb_item_add_mme_conn_info(M0, _) -> M0.

@@ -543,12 +536,9 @@

-spec fetch_erab_list(enb_registry:enb_info()) -> [map()]. -fetch_erab_list(#{mme_conn_info := ConnInfo}) -> - Pid = maps:get(handler, ConnInfo), %% s1ap_proxy process pid - ERABs = s1ap_proxy:fetch_erab_list(Pid), - lists:map(fun erab_list_item/1, ERABs); - -fetch_erab_list(_) -> []. +fetch_erab_list(#{pid := EnbPid}) -> + ERABs = enb_proxy:fetch_erab_list(EnbPid), + lists:map(fun erab_list_item/1, ERABs).

-spec erab_list_item({term(), pid()}) -> map().