fixeria has uploaded this change for review.

View Change

s1ap_proxy: move sctp_proxy:handle_pdu() to proc_safe()

This way we can have more precise counting of PDUs forwarded as-is
due to exceptions, for which this patch also adds a counter.

Change-Id: I16d4cf279a930d35ca179bd9b49234d10180e5c5
---
M include/s1gw_metrics.hrl
M src/s1ap_proxy.erl
M src/s1gw_metrics.erl
M src/sctp_proxy.erl
4 files changed, 21 insertions(+), 15 deletions(-)

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

diff --git a/include/s1gw_metrics.hrl b/include/s1gw_metrics.hrl
index 551c136..bb7043f 100644
--- a/include/s1gw_metrics.hrl
+++ b/include/s1gw_metrics.hrl
@@ -6,6 +6,7 @@
 -define(S1GW_CTR_S1AP_ENB_ALL_RX, [ctr, s1ap, enb, all, rx]).
 -define(S1GW_CTR_S1AP_ENB_ALL_RX_UNKNOWN_ENB, [ctr, s1ap, enb, all, rx_unknown_enb]).
 -define(S1GW_CTR_S1AP_PROXY_UPLINK_PACKETS_QUEUED, [ctr, s1ap, proxy, uplink_packets_queued]).
+-define(S1GW_CTR_S1AP_PROXY_EXCEPTION, [ctr, s1ap, proxy, exception]).
 -define(S1GW_CTR_S1AP_PROXY_IN_PKT_ALL, [ctr, s1ap, proxy, in_pkt, all]).
 -define(S1GW_CTR_S1AP_PROXY_IN_PKT_DECODE_ERROR, [ctr, s1ap, proxy, in_pkt, decode_error]).
 -define(S1GW_CTR_S1AP_PROXY_IN_PKT_PROC_ERROR, [ctr, s1ap, proxy, in_pkt, proc_error]).
diff --git a/src/s1ap_proxy.erl b/src/s1ap_proxy.erl
index baf3216..27bbde5 100644
--- a/src/s1ap_proxy.erl
+++ b/src/s1ap_proxy.erl
@@ -37,6 +37,7 @@
 -export([init/0,
          deinit/1,
          proc/2,
+         proc_safe/2,
          handle_exit/2,
          encode_pdu/1,
          decode_pdu/1]).
@@ -90,7 +91,8 @@
 
 
 %% Process an S1AP PDU
--spec proc(binary(), proxy_state()) -> {{proxy_action(), binary()}, proxy_state()}.
+-type proc_result() :: {{proxy_action(), binary()}, proxy_state()}.
+-spec proc(binary(), proxy_state()) -> proc_result().
 proc(OrigData, S0) ->
     s1gw_metrics:ctr_inc(?S1GW_CTR_S1AP_PROXY_IN_PKT_ALL),
     case decode_pdu(OrigData) of
@@ -122,6 +124,20 @@
     end.
 
 
+%% A safe wrapper for proc/2
+-spec proc_safe(binary(), proxy_state()) -> proc_result().
+proc_safe(OrigData, S) ->
+    try proc(OrigData, S) of
+        Result -> Result
+    catch
+        Exception:Reason:StackTrace ->
+            ?LOG_ERROR("An exception occurred: ~p, ~p, ~p", [Exception, Reason, StackTrace]),
+            s1gw_metrics:ctr_inc(?S1GW_CTR_S1AP_PROXY_EXCEPTION),
+            s1gw_metrics:ctr_inc(?S1GW_CTR_S1AP_PROXY_OUT_PKT_FWD_UNMODIFIED),
+            {{forward, OrigData}, S} %% XXX: proxy as-is or drop?
+    end.
+
+
 %% Handle an exit event
 -spec handle_exit(pid(), proxy_state()) -> proxy_state().
 handle_exit(Pid, #proxy_state{erabs = ERABs} = S) ->
diff --git a/src/s1gw_metrics.erl b/src/s1gw_metrics.erl
index ee14d08..e065a4a 100644
--- a/src/s1gw_metrics.erl
+++ b/src/s1gw_metrics.erl
@@ -56,6 +56,7 @@
     ?S1GW_CTR_S1AP_ENB_ALL_RX,
     ?S1GW_CTR_S1AP_ENB_ALL_RX_UNKNOWN_ENB,
     ?S1GW_CTR_S1AP_PROXY_UPLINK_PACKETS_QUEUED,
+    ?S1GW_CTR_S1AP_PROXY_EXCEPTION,                         %% exception(s) occurred
     %% s1ap_proxy: INcoming PDU counters
     ?S1GW_CTR_S1AP_PROXY_IN_PKT_ALL,                        %% received total
     ?S1GW_CTR_S1AP_PROXY_IN_PKT_DECODE_ERROR,               %% failed to decode
diff --git a/src/sctp_proxy.erl b/src/sctp_proxy.erl
index 9f9b746..f771788 100644
--- a/src/sctp_proxy.erl
+++ b/src/sctp_proxy.erl
@@ -171,7 +171,7 @@
             priv := Priv} = S) ->
     ?LOG_DEBUG("MME connection (id=~p, ~p:~p) -> eNB: ~p",
                [Aid, MmeAddr, MmePort, Data]),
-    {Action, NewPriv} = handle_pdu(Data, Priv),
+    {Action, NewPriv} = s1ap_proxy:proc_safe(Data, Priv),
     case Action of
         {forward, FwdData} ->
             sctp_server:send_data(EnbAid, FwdData);
@@ -224,25 +224,13 @@
 %% private API
 %% ------------------------------------------------------------------
 
-%% A safe wrapper for s1ap_proxy:proc/2
--spec handle_pdu(binary(), term()) -> {{s1ap_proxy:proxy_action(), binary()}, term()}.
-handle_pdu(OrigData, Priv) when is_binary(OrigData) ->
-    try s1ap_proxy:proc(OrigData, Priv) of
-        Result -> Result %% {Action, NewPriv}
-    catch
-        Exception:Reason:StackTrace ->
-            ?LOG_ERROR("An exception occurred: ~p, ~p, ~p", [Exception, Reason, StackTrace]),
-            {{forward, OrigData}, Priv} %% XXX: proxy as-is or drop?
-    end.
-
-
 %% Send a single message to the MME
 sctp_send(Data,
           #{sock := Sock,
             enb_aid := EnbAid,
             mme_aid := Aid,
             priv := Priv} = S) ->
-    {Action, NewPriv} = handle_pdu(Data, Priv),
+    {Action, NewPriv} = s1ap_proxy:proc_safe(Data, Priv),
     case Action of
         {forward, FwdData} ->
             ok = sctp_client:send_data({Sock, Aid}, FwdData);

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