fixeria has uploaded this change for review.
sctp_proxy: use a record for storing state data
Maps are convenient, but they are also more permissive: field names
are unchecked at compile time, making it easier for typos or unexpected
fields to slip through. Records, on the other hand, are stricter and
more declarative. Their structure is explicitly defined, the set of
fields is fixed, and the compiler can validate field names.
Change-Id: Ice9c255c0cf14db0a216bb078198b9b9c76d22a7
---
M src/sctp_proxy.erl
1 file changed, 61 insertions(+), 45 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-s1gw refs/changes/15/41615/1
diff --git a/src/sctp_proxy.erl b/src/sctp_proxy.erl
index 458c53a..5e0d26e 100644
--- a/src/sctp_proxy.erl
+++ b/src/sctp_proxy.erl
@@ -62,6 +62,18 @@
mme_aid => gen_sctp:assoc_id()
}.
+-record(state, {enb_aid :: gen_sctp:assoc_id(),
+ mme_aid :: undefined | gen_sctp:assoc_id(),
+ enb_conn_info :: sctp_server:conn_info(),
+ mme_conn_cfg :: sctp_client:cfg(),
+ tx_queue :: [binary()],
+ sock :: undefined | gen_sctp:sctp_socket(),
+ enb_handle :: enb_registry:enb_handle(),
+ handler :: pid() %% s1ap_proxy process' pid
+ }).
+
+-type state() :: #state{}.
+
-export_type([conn_info/0]).
@@ -100,12 +112,12 @@
{ok, EnbHandle} = enb_registry:enb_register(),
{ok, Pid} = s1ap_proxy:start_link(EnbHandle, EnbConnInfo),
{ok, connecting,
- #{enb_aid => maps:get(aid, EnbConnInfo),
- enb_conn_info => EnbConnInfo,
- mme_conn_cfg => MmeConnCfg,
- tx_queue => [],
- enb_handle => EnbHandle,
- handler => Pid}}.
+ #state{enb_aid = maps:get(aid, EnbConnInfo),
+ enb_conn_info = EnbConnInfo,
+ mme_conn_cfg = MmeConnCfg,
+ tx_queue = [],
+ enb_handle = EnbHandle,
+ handler = Pid}}.
callback_mode() ->
@@ -114,14 +126,14 @@
%% CONNECTING state
connecting(enter, OldState,
- #{enb_conn_info := EnbConnInfo,
- mme_conn_cfg := MmeConnCfg,
- enb_handle := Handle} = S) ->
+ #state{enb_conn_info = EnbConnInfo,
+ mme_conn_cfg = MmeConnCfg,
+ enb_handle = Handle} = 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),
- {next_state, connecting, S#{sock => Sock},
+ {next_state, connecting, S#state{sock = Sock},
[{state_timeout, 2_000, conn_est_timeout}]};
%% Handle connection establishment timeout
@@ -130,10 +142,10 @@
%% Handle an eNB -> MME data forwarding request (queue)
connecting(cast, {send_data, Data},
- #{tx_queue := Pending} = S) ->
+ #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#{tx_queue => [Data | Pending]}};
+ {keep_state, S#state{tx_queue = [Data | Pending]}};
%% Handle an #sctp_assoc_change event (connection state)
connecting(info, {sctp, _Socket, MmeAddr, MmePort,
@@ -143,7 +155,7 @@
comm_up ->
?LOG_NOTICE("MME connection (id=~p, ~p:~p) established",
[Aid, MmeAddr, MmePort]),
- {next_state, connected, S#{mme_aid => Aid}};
+ {next_state, connected, S#state{mme_aid = Aid}};
_ ->
?LOG_NOTICE("MME connection establishment failed: ~p", [ConnState]),
{stop, {shutdown, conn_est_fail}}
@@ -155,7 +167,7 @@
%% CONNECTED state
connected(enter, OldState,
- #{enb_handle := Handle} = S0) ->
+ #state{enb_handle = Handle} = S0) ->
?LOG_INFO("State change: ~p -> ~p", [OldState, ?FUNCTION_NAME]),
MmeConnInfo = conn_info(?FUNCTION_NAME, S0),
ok = enb_registry:enb_event(Handle, {?FUNCTION_NAME, MmeConnInfo}),
@@ -188,7 +200,7 @@
stream = SID,
ssn = SSN,
tsn = TSN}], Data}},
- #{mme_aid := MmeAid} = S) ->
+ #state{mme_aid = MmeAid} = S) ->
?LOG_DEBUG("MME connection (id=~p, ~p:~p) -> eNB: ~p",
[MmeAid, MmeAddr, MmePort,
#{tsn => TSN, sid => SID, ssn => SSN,
@@ -231,19 +243,22 @@
terminate(Reason, State,
- #{handler := Pid,
- enb_handle := Handle} = S) ->
+ #state{handler = Pid,
+ enb_handle = Handle,
+ sock = Sock,
+ mme_aid = MmeAid}) ->
?LOG_NOTICE("Terminating in state ~p, reason ~p", [State, Reason]),
enb_registry:enb_unregister(Handle),
s1ap_proxy:shutdown(Pid),
- case S of
- #{sock := Sock,
- mme_aid := Aid} ->
- sctp_common:shutdown({Sock, Aid}),
- gen_sctp:close(Sock);
- #{sock := Sock} ->
- gen_sctp:close(Sock);
- _ -> ok
+ case Sock of
+ undefined -> ok;
+ _ ->
+ case MmeAid of
+ undefined -> ok;
+ _ ->
+ sctp_common:shutdown({Sock, MmeAid})
+ end,
+ gen_sctp:close(Sock)
end.
@@ -251,17 +266,13 @@
%% private API
%% ------------------------------------------------------------------
-%% XXX: proper type
--type state() :: map().
-
-
%% Send a single message: eNB -> MME
-spec sctp_send_from_enb(binary(), state()) -> ok | {error, term()}.
sctp_send_from_enb(Data,
- #{sock := Sock,
- enb_aid := EnbAid,
- mme_aid := MmeAid,
- handler := Pid}) ->
+ #state{sock = Sock,
+ enb_aid = EnbAid,
+ mme_aid = MmeAid,
+ handler = Pid}) ->
case s1ap_proxy:process_pdu(Pid, Data) of
{forward, FwdData} ->
sctp_common:send_data({Sock, MmeAid}, FwdData);
@@ -275,10 +286,10 @@
%% Send a single message: eNB <- MME
-spec sctp_send_from_mme(binary(), state()) -> ok | {error, term()}.
sctp_send_from_mme(Data,
- #{sock := Sock,
- enb_aid := EnbAid,
- mme_aid := MmeAid,
- handler := Pid}) ->
+ #state{sock = Sock,
+ enb_aid = EnbAid,
+ mme_aid = MmeAid,
+ handler = Pid}) ->
case s1ap_proxy:process_pdu(Pid, Data) of
{forward, FwdData} ->
sctp_server:send_data(EnbAid, FwdData);
@@ -290,23 +301,28 @@
%% Send pending messages to the MME
-sctp_send_pending(#{tx_queue := Pending} = S) ->
+-spec sctp_send_pending(state()) -> state().
+sctp_send_pending(#state{tx_queue = Pending} = S) ->
sctp_send_pending(lists:reverse(Pending), S).
+-spec sctp_send_pending([binary()], state()) -> state().
sctp_send_pending([Data | Pending], S) ->
sctp_send_from_enb(Data, S),
s1gw_metrics:gauge_dec(?S1GW_GAUGE_S1AP_PROXY_UPLINK_PACKETS_QUEUED),
sctp_send_pending(Pending, S);
sctp_send_pending([], S) ->
- S#{tx_queue := []}.
+ S#state{tx_queue = []}.
-conn_info(State, #{mme_conn_cfg := MmeConnCfg} = S0) ->
- S1 = maps:with([enb_aid, mme_aid, handler], S0),
- S1#{state => State,
- mme_addr => maps:get(raddr, MmeConnCfg),
- mme_port => maps:get(rport, MmeConnCfg)}.
-
+-spec conn_info(atom(), state()) -> conn_info().
+conn_info(State, #state{mme_conn_cfg = MmeConnCfg} = S) ->
+ Info = #{state => State,
+ handler => S#state.handler,
+ mme_addr => maps:get(raddr, MmeConnCfg),
+ mme_port => maps:get(rport, MmeConnCfg),
+ enb_aid => S#state.enb_aid,
+ mme_aid => S#state.mme_aid},
+ maps:filter(fun(_K, V) -> V =/= undefined end, Info).
%% vim:set ts=4 sw=4 et:
To view, visit change 41615. To unsubscribe, or for help writing mail filters, visit settings.