fixeria has uploaded this change for review.
s1ap_proxy: drop PDUs that we failed to process
TODO: extend unit test
Change-Id: I6345b4ce63e8fc5588de02fe921fa61807d97d91
Related: SYS#7352
---
M include/s1gw_metrics.hrl
M src/s1ap_proxy.erl
M src/s1gw_metrics.erl
M src/sctp_proxy.erl
4 files changed, 36 insertions(+), 23 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-s1gw refs/changes/67/40767/1
diff --git a/include/s1gw_metrics.hrl b/include/s1gw_metrics.hrl
index 83ab46d..db087c5 100644
--- a/include/s1gw_metrics.hrl
+++ b/include/s1gw_metrics.hrl
@@ -14,6 +14,7 @@
-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_DROP_ALL, [ctr, s1ap, proxy, in_pkt, drop, 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]).
-define(S1GW_CTR_S1AP_PROXY_IN_PKT_ERAB_SETUP_REQ, [ctr, s1ap, proxy, in_pkt, erab_setup_req]).
diff --git a/src/s1ap_proxy.erl b/src/s1ap_proxy.erl
index 953cf66..bfa5502 100644
--- a/src/s1ap_proxy.erl
+++ b/src/s1ap_proxy.erl
@@ -82,7 +82,7 @@
}).
-type proxy_state() :: #proxy_state{}.
--type proxy_action() :: forward | reply.
+-type proxy_action() :: forward | reply | drop.
-export_type([proxy_action/0]).
@@ -324,33 +324,40 @@
?LOG_DEBUG("Tx (forward) S1AP PDU unmodified"),
ctr_inc(?S1GW_CTR_S1AP_PROXY_OUT_PKT_FWD_ALL, S1),
ctr_inc(?S1GW_CTR_S1AP_PROXY_OUT_PKT_FWD_UNMODIFIED, S1),
+ {{forward, OrigData}, S1};
+ {drop, S1} ->
+ ?LOG_NOTICE("Drop received S1AP PDU"),
+ ctr_inc(?S1GW_CTR_S1AP_PROXY_IN_PKT_DROP_ALL, S1),
{{forward, OrigData}, S1}
catch
Exception:Reason:StackTrace ->
?LOG_ERROR("An exception occurred: ~p, ~p, ~p", [Exception, Reason, StackTrace]),
ctr_inc(?S1GW_CTR_S1AP_PROXY_EXCEPTION, S0),
ctr_inc(?S1GW_CTR_S1AP_PROXY_IN_PKT_PROC_ERROR, S0),
- ctr_inc(?S1GW_CTR_S1AP_PROXY_OUT_PKT_FWD_UNMODIFIED, S0),
- {{forward, OrigData}, S0} %% XXX: proxy as-is or drop?
+ ctr_inc(?S1GW_CTR_S1AP_PROXY_IN_PKT_DROP_ALL, S0),
+ {{drop, OrigData}, S0}
end;
{error, {asn1, Error}} ->
?LOG_ERROR("S1AP PDU decoding failed: ~p", [Error]),
ctr_inc(?S1GW_CTR_S1AP_PROXY_IN_PKT_DECODE_ERROR, S0),
- ctr_inc(?S1GW_CTR_S1AP_PROXY_OUT_PKT_FWD_UNMODIFIED, S0),
- {{forward, OrigData}, S0} %% XXX: forward as-is or drop?
+ ctr_inc(?S1GW_CTR_S1AP_PROXY_IN_PKT_DROP_ALL, S0),
+ {{drop, OrigData}, S0}
catch
Exception:Reason:StackTrace ->
?LOG_ERROR("An exception occurred: ~p, ~p, ~p", [Exception, Reason, StackTrace]),
ctr_inc(?S1GW_CTR_S1AP_PROXY_EXCEPTION, S0),
ctr_inc(?S1GW_CTR_S1AP_PROXY_IN_PKT_DECODE_ERROR, S0),
- ctr_inc(?S1GW_CTR_S1AP_PROXY_OUT_PKT_FWD_UNMODIFIED, S0),
- {{forward, OrigData}, S0} %% XXX: proxy as-is or drop?
+ ctr_inc(?S1GW_CTR_S1AP_PROXY_IN_PKT_DROP_ALL, S0),
+ {{drop, OrigData}, S0}
end.
%% Process an S1AP PDU (decoded)
--spec handle_pdu(s1ap_pdu(), proxy_state()) -> {{proxy_action(), s1ap_pdu()}, proxy_state()} |
- {forward, proxy_state()}.
+-spec handle_pdu(PDU, S0) -> {Result, S1}
+ when PDU :: s1ap_pdu(),
+ S0 :: proxy_state(),
+ S1 :: proxy_state(),
+ Result :: {forward | reply, s1ap_pdu()} | forward | drop.
%% 9.1.8.4 S1 SETUP REQUEST
handle_pdu({initiatingMessage,
@@ -407,7 +414,7 @@
{{error, Reason}, S1} ->
?LOG_NOTICE("Failed to process E-RAB SETUP RESPONSE: ~p", [Reason]),
ctr_inc(?S1GW_CTR_S1AP_PROXY_IN_PKT_PROC_ERROR, S1),
- {forward, S1} %% XXX: forward as-is or drop?
+ {drop, S1} %% drop this PDU
end;
%% 9.1.3.3 E-RAB MODIFY REQUEST
@@ -425,7 +432,7 @@
{{error, Reason}, S1} ->
?LOG_NOTICE("Failed to process E-RAB MODIFY REQUEST: ~p", [Reason]),
ctr_inc(?S1GW_CTR_S1AP_PROXY_IN_PKT_PROC_ERROR, S1),
- {forward, S1} %% XXX: forward as-is or drop?
+ {drop, S1} %% drop this PDU
end;
%% 9.1.3.4 E-RAB MODIFY RESPONSE
@@ -448,15 +455,15 @@
value = C0}}, S0) ->
?LOG_DEBUG("Processing E-RAB RELEASE COMMAND"),
ctr_inc(?S1GW_CTR_S1AP_PROXY_IN_PKT_ERAB_RELEASE_CMD, S0),
+ %% there's nothing to patch in this PDU, so we forward it as-is
case handle_ies(?'id-E-RABToBeReleasedList',
C0#'E-RABReleaseCommand'.protocolIEs, S0) of
{{ok, _}, S1} ->
- %% there's nothing to patch in this PDU, so we forward it as-is
- {forward, S1}; %% forward patched PDU
+ {forward, S1};
{{error, Reason}, S1} ->
?LOG_NOTICE("Failed to process E-RAB RELEASE COMMAND: ~p", [Reason]),
ctr_inc(?S1GW_CTR_S1AP_PROXY_IN_PKT_PROC_ERROR, S1),
- {forward, S1} %% XXX: forward as-is or drop?
+ {forward, S1}
end;
%% 9.1.3.6 E-RAB RELEASE RESPONSE
@@ -465,15 +472,15 @@
value = C0}}, S0) ->
?LOG_DEBUG("Processing E-RAB RELEASE RESPONSE"),
ctr_inc(?S1GW_CTR_S1AP_PROXY_IN_PKT_ERAB_RELEASE_RSP, S0),
+ %% there's nothing to patch in this PDU, so we forward it as-is
case handle_ies(?'id-E-RABReleaseListBearerRelComp',
C0#'E-RABReleaseResponse'.protocolIEs, S0) of
{{ok, _}, S1} ->
- %% there's nothing to patch in this PDU, so we forward it as-is
- {forward, S1}; %% forward patched PDU
+ {forward, S1};
{{error, Reason}, S1} ->
?LOG_NOTICE("Failed to process E-RAB RELEASE RESPONSE: ~p", [Reason]),
ctr_inc(?S1GW_CTR_S1AP_PROXY_IN_PKT_PROC_ERROR, S1),
- {forward, S1} %% XXX: forward as-is or drop?
+ {forward, S1}
end;
%% 9.1.3.7 E-RAB RELEASE INDICATION
@@ -482,15 +489,15 @@
value = C0}}, S0) ->
?LOG_DEBUG("Processing E-RAB RELEASE INDICATION"),
ctr_inc(?S1GW_CTR_S1AP_PROXY_IN_PKT_ERAB_RELEASE_IND, S0),
+ %% there's nothing to patch in this PDU, so we forward it as-is
case handle_ies(?'id-E-RABReleasedList',
C0#'E-RABReleaseIndication'.protocolIEs, S0) of
{{ok, _}, S1} ->
- %% there's nothing to patch in this PDU, so we forward it as-is
{forward, S1};
{{error, Reason}, S1} ->
?LOG_NOTICE("Failed to process E-RAB RELEASE INDICATION: ~p", [Reason]),
ctr_inc(?S1GW_CTR_S1AP_PROXY_IN_PKT_PROC_ERROR, S1),
- {forward, S1} %% XXX: forward as-is or drop?
+ {forward, S1}
end;
%% 9.1.3.8 E-RAB MODIFICATION INDICATION
@@ -544,7 +551,7 @@
{{error, Reason}, S1} ->
?LOG_NOTICE("Failed to process INITIAL CONTEXT SETUP REQUEST: ~p", [Reason]),
ctr_inc(?S1GW_CTR_S1AP_PROXY_IN_PKT_PROC_ERROR, S1),
- {forward, S1} %% XXX: forward as-is or drop?
+ {drop, S1} %% drop this PDU
end;
%% 9.1.4.3 INITIAL CONTEXT SETUP RESPONSE
@@ -563,7 +570,7 @@
{{error, Reason}, S1} ->
?LOG_NOTICE("Failed to process INITIAL CONTEXT SETUP RESPONSE: ~p", [Reason]),
ctr_inc(?S1GW_CTR_S1AP_PROXY_IN_PKT_PROC_ERROR, S1),
- {forward, S1} %% XXX: forward as-is or drop?
+ {drop, S1} %% drop this PDU
end;
%% 9.1.4.5 UE CONTEXT RELEASE REQUEST
diff --git a/src/s1gw_metrics.erl b/src/s1gw_metrics.erl
index 9272033..1c3d76e 100644
--- a/src/s1gw_metrics.erl
+++ b/src/s1gw_metrics.erl
@@ -69,6 +69,7 @@
?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_DROP_ALL, %% dropped total
?S1GW_CTR_S1AP_PROXY_IN_PKT_DECODE_ERROR, %% failed to decode
?S1GW_CTR_S1AP_PROXY_IN_PKT_PROC_ERROR, %% failed to process
?S1GW_CTR_S1AP_PROXY_IN_PKT_ERAB_SETUP_REQ, %% E-RAB SETUP.req PDUs
diff --git a/src/sctp_proxy.erl b/src/sctp_proxy.erl
index d475b20..e8460a4 100644
--- a/src/sctp_proxy.erl
+++ b/src/sctp_proxy.erl
@@ -186,7 +186,9 @@
{forward, FwdData} ->
sctp_server:send_data(EnbAid, FwdData);
{reply, ReData} ->
- ok = sctp_common:send_data({Sock, Aid}, ReData)
+ ok = sctp_common:send_data({Sock, Aid}, ReData);
+ {drop, Data} ->
+ ok %% no-op
end,
{keep_state, S};
@@ -251,7 +253,9 @@
{forward, FwdData} ->
ok = sctp_common:send_data({Sock, Aid}, FwdData);
{reply, ReData} ->
- sctp_server:send_data(EnbAid, ReData)
+ sctp_server:send_data(EnbAid, ReData);
+ {drop, Data} ->
+ ok %% no-op
end.
To view, visit change 40767. To unsubscribe, or for help writing mail filters, visit settings.