fixeria has submitted this change. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/38790?usp=email )
Change subject: sctp_server: make it abstract from sctp_proxy logic ......................................................................
sctp_server: make it abstract from sctp_proxy logic
Instead of using the sctp_proxy API directly, let the caller pass a module name (handler), which implements the sctp_client behavior.
This makes the interaction between modules cleaner, allows to reuse the sctp_server module in other places, and makes testing easier.
Change-Id: Iee4202dd4d4e9762fe74ead985aa43343ebdc524 --- M src/osmo_s1gw_sup.erl M src/sctp_client.erl M src/sctp_proxy.erl M src/sctp_server.erl 4 files changed, 53 insertions(+), 23 deletions(-)
Approvals: fixeria: Looks good to me, approved pespin: Looks good to me, but someone else must approve Jenkins Builder: Verified laforge: Looks good to me, but someone else must approve
diff --git a/src/osmo_s1gw_sup.erl b/src/osmo_s1gw_sup.erl index 32272bf..5f332ed 100644 --- a/src/osmo_s1gw_sup.erl +++ b/src/osmo_s1gw_sup.erl @@ -68,7 +68,7 @@ PfcpRemAddr = get_env(pfcp_rem_addr, ?ENV_DEFAULT_PFCP_REM_ADDR),
SctpServer = {sctp_server, {sctp_server, start_link, - [S1GWBindAddr, S1GWBindPort, + [S1GWBindAddr, S1GWBindPort, sctp_proxy, {{MmeLocAddr, MmeRemAddr}, MmeRemPort}]}, permanent, 5000, diff --git a/src/sctp_client.erl b/src/sctp_client.erl index ab02791..72d5bd1 100644 --- a/src/sctp_client.erl +++ b/src/sctp_client.erl @@ -60,6 +60,28 @@ connect_result/0, sock_aid/0]).
+ +%% ------------------------------------------------------------------ +%% behavior callbacks +%% ------------------------------------------------------------------ + +-callback start_link(AID, Priv) -> Result + when AID :: gen_sctp:assoc_id(), + Priv :: term(), + Result :: {ok, pid()} | term(). + + +-callback send_data(Pid, Data) -> Result + when Pid :: pid(), + Data :: binary(), + Result :: term(). + + +-callback shutdown(Pid) -> Result + when Pid :: pid(), + Result :: term(). + + %% ------------------------------------------------------------------ %% public API %% ------------------------------------------------------------------ diff --git a/src/sctp_proxy.erl b/src/sctp_proxy.erl index 72f7980..fc33f70 100644 --- a/src/sctp_proxy.erl +++ b/src/sctp_proxy.erl @@ -34,6 +34,7 @@
-module(sctp_proxy). -behaviour(gen_statem). +-behaviour(sctp_client).
-export([init/1, callback_mode/0, diff --git a/src/sctp_server.erl b/src/sctp_server.erl index 2617375..68d1799 100644 --- a/src/sctp_server.erl +++ b/src/sctp_server.erl @@ -40,7 +40,7 @@ handle_call/3, handle_cast/2, terminate/2]). --export([start_link/3, +-export([start_link/4, send_data/2, shutdown/0]).
@@ -56,6 +56,7 @@
-record(server_state, {sock :: gen_sctp:sctp_socket(), clients :: dict:dict(), + handler :: module(), priv :: term() }).
@@ -68,9 +69,10 @@ %% public API %% ------------------------------------------------------------------
-start_link(BindAddr, BindPort, Priv) -> +start_link(BindAddr, BindPort, Handler, Priv) -> gen_server:start_link({local, ?MODULE}, ?MODULE, - [BindAddr, BindPort, Priv], + [BindAddr, BindPort, + Handler, Priv], []).
@@ -86,11 +88,11 @@ %% gen_server API %% ------------------------------------------------------------------
-init([BindAddrStr, BindPort, Priv]) when is_list(BindAddrStr) -> +init([BindAddrStr, BindPort, Handler, Priv]) when is_list(BindAddrStr) -> {ok, BindAddr} = inet:parse_address(BindAddrStr), - init([BindAddr, BindPort, Priv]); + init([BindAddr, BindPort, Handler, Priv]);
-init([BindAddr, BindPort, Priv]) -> +init([BindAddr, BindPort, Handler, Priv]) -> process_flag(trap_exit, true), {ok, Sock} = gen_sctp:open([{ip, BindAddr}, {port, BindPort}, @@ -101,6 +103,7 @@ ok = gen_sctp:listen(Sock, true), {ok, #server_state{sock = Sock, clients = dict:new(), + handler = Handler, priv = Priv}}.
@@ -180,12 +183,13 @@ %% Handle an #sctp_sndrcvinfo event (incoming data) sctp_recv({FromAddr, FromPort, [#sctp_sndrcvinfo{assoc_id = Aid}], Data}, - #server_state{clients = Clients} = S) -> + #server_state{clients = Clients, + handler = Handler} = S) -> ?LOG_DEBUG("eNB connection (id=~p, ~p:~p) -> MME: ~p", [Aid, FromAddr, FromPort, Data]), s1gw_metrics:ctr_inc(?S1GW_CTR_S1AP_ENB_ALL_RX), case dict:find(Aid, Clients) of {ok, #client_state{pid = Pid}} -> - sctp_proxy:send_data(Pid, Data); + Handler:send_data(Pid, Data); error -> ?LOG_ERROR("eNB connection (id=~p, ~p:~p) is not known to us?!?", [Aid, FromAddr, FromPort]), @@ -201,24 +205,26 @@ S.
-%% Add a new client to the list, spawning a proxy process +%% Add a new client to the list, spawning a handler process client_add(Aid, FromAddr, FromPort, #server_state{clients = Clients, + handler = Handler, priv = Priv} = S) -> - {ok, Pid} = sctp_proxy:start_link(Aid, Priv), + {ok, Pid} = Handler:start_link(Aid, Priv), s1gw_metrics:gauge_inc(?S1GW_GAUGE_S1AP_ENB_NUM_SCTP_CONNECTIONS), NewClient = #client_state{addr_port = {FromAddr, FromPort}, pid = Pid}, S#server_state{clients = dict:store(Aid, NewClient, Clients)}.
-%% Delete an existing client from the list, stopping the proxy process +%% Delete an existing client from the list, stopping the handler process client_del(Aid, - #server_state{clients = Clients} = S) -> + #server_state{clients = Clients, + handler = Handler} = S) -> case dict:find(Aid, Clients) of {ok, Client} -> - %% the proxy process might be already dead, so we guard + %% the handler process might be already dead, so we guard %% against exceptions like noproc or {nodedown,Node}. - catch sctp_proxy:shutdown(Client#client_state.pid), + catch Handler:shutdown(Client#client_state.pid), s1gw_metrics:gauge_dec(?S1GW_GAUGE_S1AP_ENB_NUM_SCTP_CONNECTIONS), S#server_state{clients = dict:erase(Aid, Clients)}; error -> @@ -243,22 +249,23 @@
%% Gracefully terminate client connections -close_conns(#server_state{sock = Sock, clients = Clients}) -> - close_conns(Sock, dict:to_list(Clients)). +close_conns(#server_state{sock = Sock, + clients = Clients, + handler = Handler}) -> + close_conns(Sock, Handler, dict:to_list(Clients)).
-close_conns(Sock, [{Aid, Client} | Clients]) -> +close_conns(Sock, Handler, [{Aid, Client} | Clients]) -> {FromAddr, FromPort} = Client#client_state.addr_port, ?LOG_NOTICE("Terminating eNB connection (id=~p, ~p:~p)", [Aid, FromAddr, FromPort]), - %% request to terminate an MME connection - %% the proxy process might be already dead, so we guard + %% the handler process might be already dead, so we guard %% against exceptions like noproc or {nodedown,Node}. - catch sctp_proxy:shutdown(Client#client_state.pid), + catch Handler:shutdown(Client#client_state.pid), %% gracefully close an eNB connection gen_sctp:eof(Sock, #sctp_assoc_change{assoc_id = Aid}), %% ... and so for the remaining clients - close_conns(Sock, Clients); + close_conns(Sock, Handler, Clients);
-close_conns(_Sock, []) -> +close_conns(_Sock, _Handler, []) -> ok.
%% vim:set ts=4 sw=4 et:
-
fixeria