aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--lib/diameter/doc/src/diameter.xml12
-rw-r--r--lib/diameter/src/base/diameter.erl1
-rw-r--r--lib/diameter/src/base/diameter_config.erl1
-rw-r--r--lib/diameter/src/base/diameter_peer_fsm.erl101
-rw-r--r--lib/diameter/test/diameter_config_SUITE.erl3
5 files changed, 96 insertions, 22 deletions
diff --git a/lib/diameter/doc/src/diameter.xml b/lib/diameter/doc/src/diameter.xml
index d55b761860..bb281bc0dc 100644
--- a/lib/diameter/doc/src/diameter.xml
+++ b/lib/diameter/doc/src/diameter.xml
@@ -1186,6 +1186,18 @@ terminated following an outgoing DPR if DPA is not received.</p>
Defaults to 1000.</p>
</item>
+<marker id="dpr_timeout"/>
+<tag><c>{dpr_timeout, &dict_Unsigned32;}</c></tag>
+<item>
+<p>
+Number of milliseconds after which a transport connection is
+terminated following an incoming DPR if the peer does not close the
+connection.</p>
+
+<p>
+Defaults to 5000.</p>
+</item>
+
<marker id="length_errors"/>
<tag><c>{length_errors, exit|handle|discard}</c></tag>
<item>
diff --git a/lib/diameter/src/base/diameter.erl b/lib/diameter/src/base/diameter.erl
index 8e3e9c7e23..67dfc7bdbf 100644
--- a/lib/diameter/src/base/diameter.erl
+++ b/lib/diameter/src/base/diameter.erl
@@ -344,6 +344,7 @@ call(SvcName, App, Message) ->
| {capabilities_cb, evaluable()}
| {capx_timeout, 'Unsigned32'()}
| {disconnect_cb, evaluable()}
+ | {dpr_timeout, 'Unsigned32'()}
| {dpa_timeout, 'Unsigned32'()}
| {length_errors, exit | handle | discard}
| {connect_timer, 'Unsigned32'()}
diff --git a/lib/diameter/src/base/diameter_config.erl b/lib/diameter/src/base/diameter_config.erl
index 05e9ab88fd..0d0304bf33 100644
--- a/lib/diameter/src/base/diameter_config.erl
+++ b/lib/diameter/src/base/diameter_config.erl
@@ -534,6 +534,7 @@ opt({capabilities, Os}) ->
opt({K, Tmo})
when K == capx_timeout;
+ K == dpr_timeout;
K == dpa_timeout ->
?IS_UINT32(Tmo);
diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl
index 0ee3986b97..aac2685514 100644
--- a/lib/diameter/src/base/diameter_peer_fsm.erl
+++ b/lib/diameter/src/base/diameter_peer_fsm.erl
@@ -63,7 +63,8 @@
%% 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(DPA_KEY, dpa). %% timeout for incoming DPA, or shutdown after
+ %% outgoing DPA
-define(REF_KEY, ref). %% transport_ref()
-define(Q_KEY, q). %% transport start queue
-define(START_KEY, start). %% start of connected transport
@@ -83,18 +84,26 @@
N == ?GOAWAY; N == goaway;
N == ?BUSY; N == busy).
-%% RFC 3588:
+%% RFC 6733:
%%
%% Timeout An application-defined timer has expired while waiting
%% for some event.
%%
--define(EVENT_TIMEOUT, 10000).
+
%% Default timeout for reception of CER/CEA.
+-define(CAPX_TIMEOUT, 10000).
-%% Default timeout for DPA in response to DPR. A bit short but the
-%% timeout used to be hardcoded. (So it could be worse.)
+%% Default timeout for DPA to be received in response to an outgoing
+%% DPR. A bit short but the timeout used to be hardcoded. (So it could
+%% be worse.)
-define(DPA_TIMEOUT, 1000).
+%% Default timeout for the connection to be closed by the peer
+%% following an outgoing DPA in response to an incoming DPR. It's the
+%% recipient of DPA that should close the connection according to the
+%% RFC.
+-define(DPR_TIMEOUT, 5000).
+
-type uint32() :: diameter:'Unsigned32'().
-record(state,
@@ -108,9 +117,14 @@
transport :: pid(), %% transport process
dictionary :: module(), %% common dictionary
service :: #diameter_service{},
- dpr = false :: false | {uint32(), uint32()} %% set in old code
- | {boolean(), uint32(), uint32()},
- %% | hop by hop and end to end identifiers
+ dpr = false :: false
+ | true %% DPR received, DPA sent
+ | {uint32(), uint32()} %% set in old code
+ | {boolean(), uint32(), uint32()},
+ %% hop by hop and end to end identifiers in
+ %% outgoing DPR; boolean says whether or not
+ %% the request was sent explicitly with
+ %% diameter:call/4.
length_errors :: exit | handle | discard}).
%% There are non-3588 states possible as a consequence of 5.6.1 of the
@@ -195,9 +209,10 @@ i({Ack, WPid, {M, Ref} = T, Opts, {SvcOpts, 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)),
+ putr(?DPA_KEY, {proplists:get_value(dpr_timeout, Opts, ?DPR_TIMEOUT),
+ proplists:get_value(dpa_timeout, Opts, ?DPA_TIMEOUT)}),
- Tmo = proplists:get_value(capx_timeout, Opts, ?EVENT_TIMEOUT),
+ Tmo = proplists:get_value(capx_timeout, Opts, ?CAPX_TIMEOUT),
OnLengthErr = proplists:get_value(length_errors, Opts, exit),
{TPid, Addrs} = start_transport(T, Rest, Svc),
@@ -422,7 +437,8 @@ transition({shutdown, Pid, Reason}, #state{parent = Pid, dpr = false} = S) ->
transition({shutdown, Pid, _}, #state{parent = Pid}) ->
ok;
-%% DPA reception has timed out.
+%% DPA reception has timed out, or peer has not closed the connection
+%% as a result of outgoing DPA.
transition(dpa_timeout, _) ->
stop;
@@ -545,13 +561,19 @@ recv(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.
+%% Incoming request after outgoing DPR: 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);
+ invalid(false, recv_after_outgoing_dpr, H);
+
+%% Incoming request after incoming DPR: discard.
+recv1(_,
+ #diameter_packet{header = #diameter_header{is_request = true} = H},
+ #state{dpr = true}) ->
+ invalid(false, recv_after_incoming_dpr, H);
%% DPA with identifier mismatch, or in response to a DPR initiated by
%% the service.
@@ -648,7 +670,9 @@ rcv('DPA' = N,
diameter_peer:close(TPid),
{stop, N};
-%% Ignore anything else, an unsolicited DPA in particular.
+%% Ignore anything else, an unsolicited DPA in particular. Note that
+%% dpa_timeout deals with the case in which the peer sends the wrong
+%% identifiers in DPA.
rcv(N, #diameter_packet{header = H}, _)
when N == 'CER';
N == 'CEA';
@@ -700,8 +724,10 @@ outgoing(#diameter_packet{header = #diameter_header{application_id = 0,
if T == false ->
inform_dpr(Pid),
send_dpr(true, Pkt, dpa_timeout(), S);
+ T == true ->
+ invalid(false, dpr_after_dpa, H); %% DPA sent: discard
true ->
- invalid(false, dpr_after_dpr, H) %% already sent: discard
+ invalid(false, dpr_after_dpr, H) %% DPR sent: discard
end;
%% Explict CER or DWR: discard. These are sent by us.
@@ -828,7 +854,7 @@ build_answer(Type,
errors = Es}
= Pkt,
S) ->
- {RC, FailedAVP} = result_code(H, Es),
+ {RC, FailedAVP} = result_code(Type, H, Es),
{answer(Type, RC, FailedAVP, S), post(Type, RC, Pkt, S)}.
inband_security([]) ->
@@ -846,7 +872,12 @@ cea(CEA, RC, Dict0) ->
post('CER' = T, RC, Pkt, S) ->
{T, caps(S), {RC, Pkt}};
post('DPR', _, _, #state{parent = Pid}) ->
- [fun(S) -> inform_dpr(Pid), S end].
+ [fun(S) -> dpr_timer(), inform_dpr(Pid), dpr(S) end].
+
+dpr(#state{dpr = false} = S) -> %% not awaiting DPA
+ S#state{dpr = true}; %% DPR received
+dpr(S) -> %% DPR already sent or received
+ S.
inform_dpr(Pid) ->
Pid ! {'DPR', self()}. %% tell watchdog to die with us
@@ -898,6 +929,19 @@ set(['answer-message' | _] = Ans, FailedAvp) ->
set([_|_] = Ans, FailedAvp) ->
Ans ++ FailedAvp.
+%% result_code/3
+
+%% Be lenient with errors in DPR since there's no reason to be
+%% otherwise. Rejecting may cause the peer to missinterpret the error
+%% as meaning that the connection should not be closed, which may well
+%% lead to more problems than any errors in the DPR.
+
+result_code('DPR', _, _) ->
+ {2001, []};
+
+result_code('CER', H, Es) ->
+ result_code(H, Es).
+
%% result_code/2
result_code(#diameter_header{is_error = true}, _) ->
@@ -1128,7 +1172,7 @@ close(Reason) ->
%% dpr/2
%%
-%% The RFC isn't clear on whether DPR should be send in a non-Open
+%% The RFC isn't clear on whether DPR should be sent in a non-Open
%% state. The Peer State Machine transitions it documents aren't
%% exhaustive (no Stop in Wait-I-CEA for example) so assume it's up to
%% the implementation and transition to Closed (ie. die) if we haven't
@@ -1144,7 +1188,7 @@ dpr(Reason, #state{state = 'Open',
Peer = {self(), Caps},
dpr(CBs, [Reason, Ref, Peer], S);
-%% Connection is open, DPR already sent.
+%% Connection is open, DPR already sent or received.
dpr(_, #state{state = 'Open'}) ->
ok;
@@ -1242,11 +1286,24 @@ dpa_timer(Tmo) ->
dpa_timeout() ->
dpa_timeout(getr(?DPA_KEY)).
-dpa_timeout(undefined) ->
+dpa_timeout({_, Tmo}) ->
+ Tmo;
+dpa_timeout(undefined) -> %% set in old code
?DPA_TIMEOUT;
-dpa_timeout(Tmo) ->
+dpa_timeout(Tmo) -> %% ditto
Tmo.
+dpr_timer() ->
+ dpa_timer(dpr_timeout()).
+
+dpr_timeout() ->
+ dpr_timeout(getr(?DPA_KEY)).
+
+dpr_timeout({Tmo, _}) ->
+ Tmo;
+dpr_timeout(_) -> %% set in old code
+ ?DPR_TIMEOUT.
+
%% register_everywhere/1
%%
%% Register a term and ensure it's not registered elsewhere. Note that
diff --git a/lib/diameter/test/diameter_config_SUITE.erl b/lib/diameter/test/diameter_config_SUITE.erl
index e2ac2cedcb..77f7aace1b 100644
--- a/lib/diameter/test/diameter_config_SUITE.erl
+++ b/lib/diameter/test/diameter_config_SUITE.erl
@@ -160,6 +160,9 @@
{length_errors,
[[exit], [handle], [discard]],
[[x]]},
+ {dpr_timeout,
+ [[0], [3000], [16#FFFFFFFF]],
+ [[infinity], [-1], [1 bsl 32], [x]]},
{dpa_timeout,
[[0], [3000], [16#FFFFFFFF]],
[[infinity], [-1], [1 bsl 32], [x]]},
generated by cgit v1.2.3 (git 2.25.1) at 2024-11-11 15:36:03 +0000