From 5bd2d72eeede1ddc13fcaf483d48d88833c691da Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Fri, 6 Mar 2015 02:24:28 +0100 Subject: Discard outgoing requests after outgoing DPR RFC 6733 isn't terribly clear about what should happen to incoming or outgoing messages once DPR is sent and the Peer State Machine transitions into state Closing. There's no event for this in section 5.6, Peer State Machine, and no clarification in section 5.4, Disconnecting Peer Connections. There is a little bit of discussion in 2.1.1, SCTP Guidelines, in relation to unordered message delivery, but the tone there is that messages might be received after DPR because of unordered delivery, not because they were actually sent after DPR. Discarding outgoing answers may do more harm than good, but requests are more likely to be unexpected, as has been seen to be the case with DWR following DPR. DPR indicates a desire to close the connection: discard any subsequent outgoing requests. --- lib/diameter/src/base/diameter_peer_fsm.erl | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl index ee6e7dd89e..e733e973ab 100644 --- a/lib/diameter/src/base/diameter_peer_fsm.erl +++ b/lib/diameter/src/base/diameter_peer_fsm.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2010-2014. All Rights Reserved. +%% Copyright Ericsson AB 2010-2015. All Rights Reserved. %% %% The contents of this file are subject to the Erlang Public License, %% Version 1.1, (the "License"); you may not use this file except in @@ -397,8 +397,8 @@ transition({timeout, _}, _) -> ok; %% Outgoing message. -transition({send, Msg}, #state{transport = TPid}) -> - send(TPid, Msg), +transition({send, Msg}, S) -> + outgoing(Msg, S), ok; %% Request for graceful shutdown at remove_transport, stop_service of @@ -640,6 +640,27 @@ incr_error(Dir, Pkt, Dict0) -> send(Pid, Msg) -> diameter_peer:send(Pid, Msg). +%% outgoing/2 + +%% DPR not sent: send. +outgoing(Msg, #state{transport = TPid, dpr = false}) -> + send(TPid, Msg); + +%% Outgoing answer: send. +outgoing(#diameter_packet{header = #diameter_header{is_request = false}} + = Pkt, + #state{transport = TPid}) -> + send(TPid, Pkt); + +%% Outgoing request: discard. +outgoing(Msg, #state{dpr = {_,_}}) -> + invalid(false, send_after_dpr, header(Msg)). + +header(#diameter_packet{header = H}) -> + H; +header(Bin) -> %% DWR + diameter_codec:decode_header(Bin). + %% handle_request/3 %% %% Incoming CER or DPR. -- cgit v1.2.3 From b1fd629917c2dc4563038f21e0727e2c85b6d07c Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Fri, 6 Mar 2015 08:51:23 +0100 Subject: Discard incoming requests after outgoing DPR Since there's a race between an answer being sent and the connection being closed upon the reception of DPA that's likely to be lost, and because of the questionability of sending messages after DPR, as discussed in the parent commit. An exception is made for DPR so that simultaneous DPR in both directions doesn't result in it being discarded on both ends. --- lib/diameter/src/base/diameter_peer_fsm.erl | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl index e733e973ab..23bba701eb 100644 --- a/lib/diameter/src/base/diameter_peer_fsm.erl +++ b/lib/diameter/src/base/diameter_peer_fsm.erl @@ -516,12 +516,9 @@ encode(Rec, Dict) -> recv(#diameter_packet{header = #diameter_header{} = Hdr} = Pkt, - #state{parent = Pid, - dictionary = Dict0} + #state{dictionary = Dict0} = S) -> - Name = diameter_codec:msg_name(Dict0, Hdr), - Pid ! {recv, self(), Name, Pkt}, - rcv(Name, Pkt, S); + recv1(diameter_codec:msg_name(Dict0, Hdr), Pkt, S); recv(#diameter_packet{header = undefined, bin = Bin} @@ -532,6 +529,22 @@ recv(#diameter_packet{header = undefined, recv(Bin, S) -> recv(#diameter_packet{bin = Bin}, S). +%% recv1/3 + +%% Incoming request after DPR has been sent: discard. Don't discard +%% DPR, so both ends don't do so when sending simultaneously. +recv1(Name, + #diameter_packet{header = #diameter_header{is_request = true} = H}, + #state{dpr = {_,_}}) + when Name /= 'DPR' -> + invalid(false, recv_after_dpr, H); + +%% Any other message with a header and no length errors: send to the +%% parent. +recv1(Name, Pkt, #state{parent = Pid} = S) -> + Pid ! {recv, self(), Name, Pkt}, + rcv(Name, Pkt, S). + %% recv/3 recv(#diameter_header{length = Len} -- cgit v1.2.3 From c0687e03e538daab2f8fddda6cd3a235fc6ed447 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Sat, 7 Mar 2015 15:50:05 +0100 Subject: Add transport_opt() dpa_timeout To make the default DPA timeout configurable. The timeout say how many milliseconds to wait for DPA in response to an outgoing DPR before terminating the transport process regardless. --- lib/diameter/src/base/diameter.erl | 1 + lib/diameter/src/base/diameter_config.erl | 6 ++++-- lib/diameter/src/base/diameter_peer_fsm.erl | 14 ++++++++++++-- 3 files changed, 17 insertions(+), 4 deletions(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter.erl b/lib/diameter/src/base/diameter.erl index d74e091e11..ab4be232e2 100644 --- a/lib/diameter/src/base/diameter.erl +++ b/lib/diameter/src/base/diameter.erl @@ -342,6 +342,7 @@ call(SvcName, App, Message) -> | {capabilities_cb, evaluable()} | {capx_timeout, 'Unsigned32'()} | {disconnect_cb, evaluable()} + | {dpa_timeout, 'Unsigned32'()} | {length_errors, exit | handle | discard} | {connect_timer, 'Unsigned32'()} | {watchdog_timer, 'Unsigned32'() | {module(), atom(), list()}} diff --git a/lib/diameter/src/base/diameter_config.erl b/lib/diameter/src/base/diameter_config.erl index dd1c9b73bb..c9a5beaf55 100644 --- a/lib/diameter/src/base/diameter_config.erl +++ b/lib/diameter/src/base/diameter_config.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2010-2014. All Rights Reserved. +%% Copyright Ericsson AB 2010-2015. All Rights Reserved. %% %% The contents of this file are subject to the Erlang Public License, %% Version 1.1, (the "License"); you may not use this file except in @@ -531,7 +531,9 @@ opt({applications, As}) -> opt({capabilities, Os}) -> is_list(Os) andalso ok == encode_CER(Os); -opt({capx_timeout, Tmo}) -> +opt({K, Tmo}) + when K == capx_timeout; + K == dpa_timeout -> ?IS_UINT32(Tmo); opt({length_errors, T}) -> diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl index 23bba701eb..434524040f 100644 --- a/lib/diameter/src/base/diameter_peer_fsm.erl +++ b/lib/diameter/src/base/diameter_peer_fsm.erl @@ -63,6 +63,7 @@ %% Keys in process dictionary. -define(CB_KEY, cb). %% capabilities callback -define(DPR_KEY, dpr). %% disconnect callback +-define(DPA_KEY, dpa). %% timeout for DPA reception -define(REF_KEY, ref). %% transport_ref() -define(Q_KEY, q). %% transport start queue -define(START_KEY, start). %% start of connected transport @@ -187,6 +188,7 @@ i({Ack, WPid, {M, Ref} = T, Opts, {Mask, Nodes, Dict0, Svc}}) -> putr(?REF_KEY, Ref), putr(?SEQUENCE_KEY, Mask), putr(?RESTRICT_KEY, Nodes), + putr(?DPA_KEY, proplists:get_value(dpa_timeout, Opts, ?DPA_TIMEOUT)), Tmo = proplists:get_value(capx_timeout, Opts, ?EVENT_TIMEOUT), OnLengthErr = proplists:get_value(length_errors, Opts, exit), @@ -1107,7 +1109,7 @@ dpr([CB|Rest], [Reason | _] = Args, S) -> dpr([], [Reason | _], S) -> send_dpr(Reason, [], S). --record(opts, {cause, timeout = ?DPA_TIMEOUT}). +-record(opts, {cause, timeout}). send_dpr(Reason, Opts, #state{transport = TPid, dictionary = Dict, @@ -1119,7 +1121,7 @@ send_dpr(Reason, Opts, #state{transport = TPid, transport -> ?GOAWAY; _ -> ?REBOOT end, - timeout = ?DPA_TIMEOUT}, + timeout = dpa_timeout()}, Opts), #diameter_caps{origin_host = {OH, _}, origin_realm = {OR, _}} @@ -1159,6 +1161,14 @@ cause(N) -> dpa_timer(Tmo) -> erlang:send_after(Tmo, self(), dpa_timeout). +dpa_timeout() -> + dpa_timeout(getr(?DPA_KEY)). + +dpa_timeout(undefined) -> + ?DPA_TIMEOUT; +dpa_timeout(Tmo) -> + Tmo. + %% register_everywhere/1 %% %% Register a term and ensure it's not registered elsewhere. Note that -- cgit v1.2.3 From c0ae6c1bb77e69d69d898965e472ce7633a13404 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Sat, 7 Mar 2015 12:50:21 +0100 Subject: Allow DPR to be sent with diameter:call/4 DPR is sent by diameter at application shutdown, service stop, or transport removal. It has been possible to send the request with diameter:call/4, but the answer was discarded, instead of the transport process being terminated. This commit causes DPR to be handled in the same way regardless of whether it's sent by diameter or by diameter:call/4. Note that the behaviour subsequent to DPA is unchanged. In particular, in the connecting case, the closed connection will be reestablished after a connect_timer expiry unless the transport is removed. The more probable use case is the listening case, to disconnect a single peer associated with a listening transport. --- lib/diameter/src/base/diameter_peer_fsm.erl | 104 ++++++++++++++++++++++------ lib/diameter/src/base/diameter_watchdog.erl | 14 ++-- 2 files changed, 89 insertions(+), 29 deletions(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl index 434524040f..dcfcb63808 100644 --- a/lib/diameter/src/base/diameter_peer_fsm.erl +++ b/lib/diameter/src/base/diameter_peer_fsm.erl @@ -108,7 +108,8 @@ transport :: pid(), %% transport process dictionary :: module(), %% common dictionary service :: #diameter_service{}, - dpr = false :: false | {uint32(), uint32()}, + dpr = false :: false | {uint32(), uint32()} %% set in old code + | {boolean(), uint32(), uint32()}, %% | hop by hop and end to end identifiers length_errors :: exit | handle | discard}). @@ -214,9 +215,12 @@ wait(Ref, Pid) -> Ref -> ok; {'DOWN', _, process, Pid, _} = D -> - exit({shutdown, D}) + x(D) end. +x(T) -> + exit({shutdown, T}). + start_transport(T, Opts, #diameter_service{capabilities = LCaps} = Svc) -> Addrs0 = LCaps#diameter_caps.host_ip_address, start_transport(Addrs0, {T, Opts, Svc}). @@ -228,7 +232,7 @@ start_transport(Addrs0, T) -> q_next(TPid, Addrs0, Tmo, Data), {TPid, Addrs}; {error, No} -> - exit({shutdown, {no_connection, No}}) + x({no_connection, No}) end. svc(#diameter_service{capabilities = LCaps0} = Svc, Addrs) -> @@ -335,6 +339,9 @@ eraser(Key) -> %% transition/2 +transition(T, #state{dpr = {Hid, Eid}} = S) -> %% DPR sent from old code + transition(T, S#state{dpr = {false, Hid, Eid}}); + %% Connection to peer. transition({diameter, {TPid, connected, Remote}}, #state{transport = TPid, @@ -400,8 +407,7 @@ transition({timeout, _}, _) -> %% Outgoing message. transition({send, Msg}, S) -> - outgoing(Msg, S), - ok; + outgoing(Msg, S); %% Request for graceful shutdown at remove_transport, stop_service of %% application shutdown. @@ -537,10 +543,23 @@ recv(Bin, S) -> %% DPR, so both ends don't do so when sending simultaneously. recv1(Name, #diameter_packet{header = #diameter_header{is_request = true} = H}, - #state{dpr = {_,_}}) + #state{dpr = {_,_,_}}) when Name /= 'DPR' -> invalid(false, recv_after_dpr, H); +%% DPA with identifier mismatch, or in response to a DPR initiated by +%% the service. +recv1('DPA' = N, + #diameter_packet{header = #diameter_header{hop_by_hop_id = Hid, + end_to_end_id = Eid}} + = Pkt, + #state{dpr = {X,H,E}} + = S) + when H /= Hid; + E /= Eid; + not X -> + rcv(N, Pkt, S); + %% Any other message with a header and no length errors: send to the %% parent. recv1(Name, Pkt, #state{parent = Pid} = S) -> @@ -603,7 +622,7 @@ rcv(Name, _, #state{state = PS}) rcv('DPR' = N, Pkt, S) -> handle_request(N, Pkt, S); -%% DPA in response to DPR and with the expected identifiers. +%% DPA in response to DPR, with the expected identifiers. rcv('DPA' = N, #diameter_packet{header = #diameter_header{end_to_end_id = Eid, hop_by_hop_id = Hid} @@ -611,10 +630,15 @@ rcv('DPA' = N, = Pkt, #state{dictionary = Dict0, transport = TPid, - dpr = {Hid, Eid}}) -> + dpr = {X, Hid, Eid}}) -> ?LOG(recv, N), - incr(recv, H, Dict0), - incr_rc(recv, diameter_codec:decode(Dict0, Pkt), Dict0), + X orelse begin + %% Only count DPA in response to a DPR sent by the + %% service: explicit DPR is counted in the same way + %% as other explicitly sent requests. + incr(recv, H, Dict0), + incr_rc(recv, diameter_codec:decode(Dict0, Pkt), Dict0) + end, diameter_peer:close(TPid), {stop, N}; @@ -652,23 +676,42 @@ incr_error(Dir, Pkt, Dict0) -> %% Msg here could be a #diameter_packet or a binary depending on who's %% sending. In particular, the watchdog will send DWR as a binary %% while messages coming from clients will be in a #diameter_packet. + send(Pid, Msg) -> diameter_peer:send(Pid, Msg). %% outgoing/2 +%% Explicit DPR. +outgoing(#diameter_packet{header = #diameter_header{application_id = 0, + cmd_code = 282, + is_request = true} + = H} + = Pkt, + #state{dpr = T, + parent = Pid} + = S) -> + if T == false -> + inform_dpr(Pid), + send_dpr(true, Pkt, dpa_timeout(), S); + true -> + invalid(false, dpr_after_dpr, H) %% already sent: discard + end; + %% DPR not sent: send. outgoing(Msg, #state{transport = TPid, dpr = false}) -> - send(TPid, Msg); + send(TPid, Msg), + ok; %% Outgoing answer: send. outgoing(#diameter_packet{header = #diameter_header{is_request = false}} = Pkt, #state{transport = TPid}) -> - send(TPid, Pkt); + send(TPid, Pkt), + ok; %% Outgoing request: discard. -outgoing(Msg, #state{dpr = {_,_}}) -> +outgoing(Msg, #state{dpr = {_,_,_}}) -> invalid(false, send_after_dpr, header(Msg)). header(#diameter_packet{header = H}) -> @@ -784,8 +827,11 @@ cea(CEA, RC, Dict0) -> post('CER' = T, RC, Pkt, S) -> {T, caps(S), {RC, Pkt}}; -post('DPR' = T, _, _, #state{parent = Pid}) -> - [fun(S) -> Pid ! {T, self()}, S end]. +post('DPR', _, _, #state{parent = Pid}) -> + [fun(S) -> inform_dpr(Pid), S end]. + +inform_dpr(Pid) -> + Pid ! {'DPR', self()}. %% tell watchdog to die with us rejected({capabilities_cb, _F, Reason}, T, S) -> rejected(Reason, T, S); @@ -1111,8 +1157,7 @@ dpr([], [Reason | _], S) -> -record(opts, {cause, timeout}). -send_dpr(Reason, Opts, #state{transport = TPid, - dictionary = Dict, +send_dpr(Reason, Opts, #state{dictionary = Dict, service = #diameter_service{capabilities = Caps}} = S) -> #opts{cause = Cause, timeout = Tmo} @@ -1127,18 +1172,31 @@ send_dpr(Reason, Opts, #state{transport = TPid, origin_realm = {OR, _}} = Caps, - #diameter_packet{header = #diameter_header{end_to_end_id = Eid, - hop_by_hop_id = Hid}} - = Pkt - = encode(['DPR', {'Origin-Host', OH}, + Pkt = encode(['DPR', {'Origin-Host', OH}, {'Origin-Realm', OR}, {'Disconnect-Cause', Cause}], Dict), - incr(send, Pkt, Dict), + send_dpr(false, Pkt, Tmo, S). + +%% send_dpr/4 + +send_dpr(X, + #diameter_packet{header = #diameter_header{end_to_end_id = Eid, + hop_by_hop_id = Hid}} + = Pkt, + Tmo, + #state{transport = TPid, + dictionary = Dict} + = S) -> + %% Only count DPR sent by the service: explicit DPR is counted in + %% the same way as other explicitly sent requests. + X orelse incr(send, Pkt, Dict), send(TPid, Pkt), dpa_timer(Tmo), ?LOG(send, 'DPR'), - S#state{dpr = {Hid, Eid}}. + S#state{dpr = {X, Hid, Eid}}. + +%% opt/2 opt({timeout, Tmo}, Rec) when ?IS_TIMEOUT(Tmo) -> diff --git a/lib/diameter/src/base/diameter_watchdog.erl b/lib/diameter/src/base/diameter_watchdog.erl index b7f2d24941..6d7f6d7675 100644 --- a/lib/diameter/src/base/diameter_watchdog.erl +++ b/lib/diameter/src/base/diameter_watchdog.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2010-2014. All Rights Reserved. +%% Copyright Ericsson AB 2010-2015. All Rights Reserved. %% %% The contents of this file are subject to the Erlang Public License, %% Version 1.1, (the "License"); you may not use this file except in @@ -332,8 +332,9 @@ transition({shutdown = T, Pid, Reason}, #watchdog{parent = Pid, send(TPid, {T, self(), Reason}), S#watchdog{shutdown = true}; -%% Transport is telling us that DPA has been sent in response to DPR: -%% its death should lead to ours. +%% Transport is telling us that DPA has been sent in response to DPR, +%% or that DPR has been explicitly sent: transport death should lead +%% to ours. transition({'DPR', TPid}, #watchdog{transport = TPid} = S) -> S#watchdog{shutdown = true}; @@ -549,7 +550,7 @@ send_watchdog(#watchdog{pending = false, ?LOG(send, 'DWR'), S#watchdog{pending = true}. -%% Dont' count encode errors since we don't expect any on DWR/DWA. +%% Don't count encode errors since we don't expect any on DWR/DWA. %% recv/3 @@ -590,9 +591,10 @@ rcv('DWA', Pkt, #watchdog{transport = TPid, rcv(N, _, _) when N == 'CER'; N == 'CEA'; - N == 'DPR'; - N == 'DPA' -> + N == 'DPR' -> false; +%% DPR can be sent explicitly with diameter:call/4. Only the +%% corresponding DPAs arrive here. rcv(_, Pkt, #watchdog{transport = TPid, dictionary = Dict0, -- cgit v1.2.3 From 09e51b177e0b2c1528c99bdd1c354319e07bc421 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Sat, 7 Mar 2015 14:22:11 +0100 Subject: Discard CER or DWR sent with diameter:call/4 These are requests that diameter itself sends. It's previously been possible to send them, but answers timed out at the caller since they were discarded in diameter_watchdog. Answers will still timeout, but now the requests are discarded before being sent. --- lib/diameter/src/base/diameter_peer_fsm.erl | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl index dcfcb63808..9ff6845ab7 100644 --- a/lib/diameter/src/base/diameter_peer_fsm.erl +++ b/lib/diameter/src/base/diameter_peer_fsm.erl @@ -698,6 +698,16 @@ outgoing(#diameter_packet{header = #diameter_header{application_id = 0, invalid(false, dpr_after_dpr, H) %% already sent: discard end; +%% Explict CER or DWR: discard. These are sent by us. +outgoing(#diameter_packet{header = #diameter_header{application_id = 0, + cmd_code = C, + is_request = true} + = H}, + _) + when 257 == C; %% CER + 280 == C -> %% DWR + invalid(false, invalid_request, H); + %% DPR not sent: send. outgoing(Msg, #state{transport = TPid, dpr = false}) -> send(TPid, Msg), -- cgit v1.2.3