fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/40767?usp=email ) Change subject: s1ap_proxy: drop PDUs that we failed to process ...................................................................... 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 https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/40767?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: erlang/osmo-s1gw Gerrit-Branch: master Gerrit-Change-Id: I6345b4ce63e8fc5588de02fe921fa61807d97d91 Gerrit-Change-Number: 40767 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>