fixeria submitted this change.

View Change

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
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(-)

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:

To view, visit change 38790. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-MessageType: merged
Gerrit-Project: erlang/osmo-s1gw
Gerrit-Branch: master
Gerrit-Change-Id: Iee4202dd4d4e9762fe74ead985aa43343ebdc524
Gerrit-Change-Number: 38790
Gerrit-PatchSet: 2
Gerrit-Owner: fixeria <vyanitskiy@sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de>
Gerrit-Reviewer: laforge <laforge@osmocom.org>
Gerrit-Reviewer: pespin <pespin@sysmocom.de>