fixeria has submitted this change. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/38298?usp=email )

Change subject: s1ap_proxy: move sctp_proxy:handle_pdu() to process_pdu_safe() ......................................................................

s1ap_proxy: move sctp_proxy:handle_pdu() to process_pdu_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(-)

Approvals: pespin: Looks good to me, approved Jenkins Builder: Verified

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 68d0d0e..6ec3d10 100644 --- a/src/s1ap_proxy.erl +++ b/src/s1ap_proxy.erl @@ -37,6 +37,7 @@ -export([init/0, deinit/1, process_pdu/2, + process_pdu_safe/2, handle_exit/2, encode_pdu/1, decode_pdu/1]). @@ -90,7 +91,8 @@

%% Process an S1AP PDU --spec process_pdu(binary(), proxy_state()) -> {{proxy_action(), binary()}, proxy_state()}. +-type process_pdu_result() :: {{proxy_action(), binary()}, proxy_state()}. +-spec process_pdu(binary(), proxy_state()) -> process_pdu_result(). process_pdu(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 process_pdu_safe(binary(), proxy_state()) -> process_pdu_result(). +process_pdu_safe(OrigData, S) -> + try process_pdu(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 c901b80..9bec272 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:process_pdu_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:process_pdu(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:process_pdu_safe(Data, Priv), case Action of {forward, FwdData} -> ok = sctp_client:send_data({Sock, Aid}, FwdData);