From 0447bd6e8bcd3b0249b9956883c213c434095ec5 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Mon, 28 Aug 2017 23:53:40 +0200 Subject: Use unordered delivery on a lone outbound stream in diameter_sctp RFC 6733 say the below about head-of-line blocking and SCTP, the suggestion of unordered sending being new compared to the RFC 3588. Until now, all delivery in diameter_sctp has been ordered, and roundrobin over available streams unless the user passes a stream identifier in an outgoing diameter_packet record. This commit changes this to use unordered delivery when there's only a single outbound stream to choose from. The special case at capabilities exchange is handled by only starting to send unordered after the second message from the peer has been received. (First message after capabilities exchange, as the RFC probably means.) The special case at DPR isn't handled, since there's no knowing the order in which a peer will answer: a node that sends DPR while it has other requests outstanding can't expect that the latter will be answered before DPR, even if delivery is ordered since incoming requests are handled concurrently. If it wants a guarantee then it simply has to wait for answers before sending DPR. A user can force all delivery to be unordered by specifying {sctp_default_send_params, #sctp_sndrcvinfo{flags = [unordered]}} as a config option to diameter_sctp, but in this case there's no handling of a request being sent directly after CEA since there's no ordered flag to override the default. RFC 6733: 2.1.1. SCTP Guidelines Diameter messages SHOULD be mapped into SCTP streams in a way that avoids head-of-the-line (HOL) blocking. Among different ways of performing the mapping that fulfill this requirement it is RECOMMENDED that a Diameter node send every Diameter message (request or response) over stream zero with the unordered flag set. However, Diameter nodes MAY select and implement other design alternatives for avoiding HOL blocking such as using multiple streams with the unordered flag cleared (as originally instructed in RFC 3588). On the receiving side, a Diameter entity MUST be ready to receive Diameter messages over any stream, and it is free to return responses over a different stream. This way, both sides manage the available streams in the sending direction, independently of the streams chosen by the other side to send a particular Diameter message. These messages can be out-of-order and belong to different Diameter sessions. Out-of-order delivery has special concerns during a connection establishment and termination. When a connection is established, the responder side sends a CEA message and moves to R-Open state as specified in Section 5.6. If an application message is sent shortly after the CEA and delivered out-of-order, the initiator side, still in Wait-I-CEA state, will discard the application message and close the connection. In order to avoid this race condition, the receiver side SHOULD NOT use out-of-order delivery methods until the first message has been received from the initiator, proving that it has moved to I-Open state. To trigger such a message, the receiver side could send a DWR immediately after sending a CEA. Upon reception of the corresponding DWA, the receiver side should start using out-of- order delivery methods to counter the HOL blocking. Another race condition may occur when DPR and DPA messages are used. Both DPR and DPA are small in size; thus, they may be delivered to the peer faster than application messages when an out-of-order delivery mechanism is used. Therefore, it is possible that a DPR/DPA exchange completes while application messages are still in transit, resulting in a loss of these messages. An implementation could mitigate this race condition, for example, using timers, and wait for a short period of time for pending application level messages to arrive before proceeding to disconnect the transport connection. Eventually, lost messages are handled by the retransmission mechanism described in Section 5.5.4. --- lib/diameter/src/transport/diameter_sctp.erl | 30 ++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/lib/diameter/src/transport/diameter_sctp.erl b/lib/diameter/src/transport/diameter_sctp.erl index 4f56475529..0d8fb37629 100644 --- a/lib/diameter/src/transport/diameter_sctp.erl +++ b/lib/diameter/src/transport/diameter_sctp.erl @@ -105,6 +105,7 @@ packet = true :: boolean() %% legacy transport_data? | raw, message_cb = false :: false | diameter:eval(), + unordered = false :: boolean() | 0 | 1, %% send unordered? send = false :: pid() | boolean()}). %% sending process %% Monitor process state. @@ -677,7 +678,11 @@ send(#diameter_packet{transport_data = {outstream, SId}} = S) -> send(SId rem OS, Msg, S); -%% ... or not: rotate through all streams. +%% ... or not: send unordered on a lone stream ... +send(Msg, #transport{unordered = true} = S) -> + send(0, Msg, S); + +%% ... or rotate through all. send(Msg, #transport{streams = {_, OS}, os = N} = S) -> @@ -725,6 +730,7 @@ recv({_, #sctp_assoc_change{state = comm_up, %% Deal with different association id after peeloff on Solaris by %% taking the id from the first reception. up(S#transport{assoc_id = T == accept orelse Id, + unordered = 1 == OS andalso 1, streams = {IS, OS}}); %% ... or not: try the next address. @@ -749,7 +755,7 @@ recv({[#sctp_sndrcvinfo{assoc_id = Id}], _Bin} %% Inbound Diameter message. recv({[#sctp_sndrcvinfo{}], Bin} = Msg, S) when is_binary(Bin) -> - message(recv, Msg, S); + message(recv, Msg, recv(S)); recv({_, #sctp_shutdown_event{}}, _) -> stop; @@ -769,6 +775,26 @@ recv({_, #sctp_paddr_change{}}, _) -> recv({_, #sctp_pdapi_event{}}, _) -> ok. +%% recv/1 +%% +%% Start sending unordered on a lone outbound stream after the second +%% reception, so that an outgoing CER/CEA will arrive at the peer +%% before another request. + +recv(#transport{unordered = B} = S) + when is_boolean(B) -> + S; + +recv(#transport{unordered = 0, socket = Sock} = S) -> + ok = inet:setopts(Sock, [{sctp_default_send_param, + #sctp_sndrcvinfo{flags = [unordered]}}]), + S#transport{unordered = true}; + +recv(#transport{unordered = N} = S) -> + S#transport{unordered = N-1}. + +%% publish/4 + publish(T, Ref, Id, Sock) -> true = diameter_reg:add_new({?MODULE, T, {Ref, {Id, Sock}}}), putr(?INFO_KEY, {gen_sctp, Sock}). %% for info/1 -- cgit v1.2.3 From 549a82df3ae250b7c5598a9451b7b9802073d6f9 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Tue, 29 Aug 2017 09:08:38 +0200 Subject: Exercise unordered delivery in traffic suite By randomly setting the number of outbound streams on associations. --- lib/diameter/test/diameter_traffic_SUITE.erl | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/diameter/test/diameter_traffic_SUITE.erl b/lib/diameter/test/diameter_traffic_SUITE.erl index 1760d7c5dc..f058ed65b8 100644 --- a/lib/diameter/test/diameter_traffic_SUITE.erl +++ b/lib/diameter/test/diameter_traffic_SUITE.erl @@ -112,6 +112,8 @@ %% diameter_{tcp,sctp} callbacks -export([message/3]). +-include_lib("kernel/include/inet_sctp.hrl"). + -include("diameter.hrl"). -include("diameter_gen_base_rfc3588.hrl"). -include("diameter_gen_base_accounting.hrl"). @@ -486,7 +488,7 @@ add_transports(Config) -> | server_apps()] ++ [{spawn_opt, {erlang, spawn, []}} || CS]), Cs = [?util:connect(CN, - [T, {sender, CS}], + [T, {sender, CS} | client_opts(T)], LRef, [{id, Id} | client_apps(R, [{'Origin-State-Id', origin(Id)}])]) @@ -496,6 +498,14 @@ add_transports(Config) -> Id <- [{D,E}]], ?util:write_priv(Config, "transport", [LRef | Cs]). +client_opts(tcp) -> + []; +client_opts(sctp) -> + [{sctp_initmsg, #sctp_initmsg{num_ostreams = N, + max_instreams = 5}} + || N <- [rand:uniform(8)], + N =< 6]. + server_apps() -> B = have_nas(), [{applications, [diameter_gen_base_rfc3588, -- cgit v1.2.3 From 1fd9b919608b54e4d08340c45fe1cabb6975880c Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Tue, 29 Aug 2017 14:33:47 +0200 Subject: Delay rotation of diameter_sctp outbound streams For the same reason as unordered delivery is delayed in the grandparent commit. Delivery is ordered only within a stream, so until a second message is received from the peer, there's no guarantee that a second outgoing request won't be received before the initial capablities exchange message. Rotation begins upon reception of a second message from the peer, messages being sent on stream 0 until then. --- lib/diameter/src/transport/diameter_sctp.erl | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/lib/diameter/src/transport/diameter_sctp.erl b/lib/diameter/src/transport/diameter_sctp.erl index 0d8fb37629..86a98776c5 100644 --- a/lib/diameter/src/transport/diameter_sctp.erl +++ b/lib/diameter/src/transport/diameter_sctp.erl @@ -105,7 +105,7 @@ packet = true :: boolean() %% legacy transport_data? | raw, message_cb = false :: false | diameter:eval(), - unordered = false :: boolean() | 0 | 1, %% send unordered? + unordered = 1 :: boolean() | 0 | 1, %% send unordered? send = false :: pid() | boolean()}). %% sending process %% Monitor process state. @@ -678,15 +678,16 @@ send(#diameter_packet{transport_data = {outstream, SId}} = S) -> send(SId rem OS, Msg, S); -%% ... or not: send unordered on a lone stream ... -send(Msg, #transport{unordered = true} = S) -> - send(0, Msg, S); - -%% ... or rotate through all. -send(Msg, #transport{streams = {_, OS}, +%% ... or not: rotate when sending ordered ... +send(Msg, #transport{unordered = false, + streams = {_, OS}, os = N} = S) -> - send(N, Msg, S#transport{os = (N + 1) rem OS}). + send(N, Msg, S#transport{os = (N + 1) rem OS}); + +%% ... or send only on the first stream, possibly unordered. +send(Msg, S) -> + send(0, Msg, S). %% send/3 @@ -730,7 +731,6 @@ recv({_, #sctp_assoc_change{state = comm_up, %% Deal with different association id after peeloff on Solaris by %% taking the id from the first reception. up(S#transport{assoc_id = T == accept orelse Id, - unordered = 1 == OS andalso 1, streams = {IS, OS}}); %% ... or not: try the next address. @@ -785,11 +785,14 @@ recv(#transport{unordered = B} = S) when is_boolean(B) -> S; -recv(#transport{unordered = 0, socket = Sock} = S) -> +recv(#transport{unordered = 0, streams = {_, 1}, socket = Sock} = S) -> ok = inet:setopts(Sock, [{sctp_default_send_param, #sctp_sndrcvinfo{flags = [unordered]}}]), S#transport{unordered = true}; +recv(#transport{unordered = 0} = S) -> + S#transport{unordered = false}; + recv(#transport{unordered = N} = S) -> S#transport{unordered = N-1}. -- cgit v1.2.3 From a3749fd240260958053f90539b0f7e04e720d070 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Tue, 29 Aug 2017 22:26:51 +0200 Subject: Send unordered on all outbound diameter_sctp streams There's no reason for sending ordered since request handling is concurrent: different processes handling incoming requests can't know in which order they were received on the transport, and different processes sending requests can't know the order in which they're sent. --- lib/diameter/src/transport/diameter_sctp.erl | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/lib/diameter/src/transport/diameter_sctp.erl b/lib/diameter/src/transport/diameter_sctp.erl index 86a98776c5..2af11729a9 100644 --- a/lib/diameter/src/transport/diameter_sctp.erl +++ b/lib/diameter/src/transport/diameter_sctp.erl @@ -102,10 +102,10 @@ streams :: {uint(), uint()} %% {InStream, OutStream} counts | undefined, os = 0 :: uint(), %% next output stream + rotate = 1 :: boolean() | 0 | 1, %% rotate os? packet = true :: boolean() %% legacy transport_data? | raw, message_cb = false :: false | diameter:eval(), - unordered = 1 :: boolean() | 0 | 1, %% send unordered? send = false :: pid() | boolean()}). %% sending process %% Monitor process state. @@ -678,14 +678,14 @@ send(#diameter_packet{transport_data = {outstream, SId}} = S) -> send(SId rem OS, Msg, S); -%% ... or not: rotate when sending ordered ... -send(Msg, #transport{unordered = false, +%% ... or not: rotate when sending on multiple streams ... +send(Msg, #transport{rotate = true, streams = {_, OS}, os = N} = S) -> send(N, Msg, S#transport{os = (N + 1) rem OS}); -%% ... or send only on the first stream, possibly unordered. +%% ... or send on the only stream available. send(Msg, S) -> send(0, Msg, S). @@ -777,24 +777,20 @@ recv({_, #sctp_pdapi_event{}}, _) -> %% recv/1 %% -%% Start sending unordered on a lone outbound stream after the second -%% reception, so that an outgoing CER/CEA will arrive at the peer -%% before another request. +%% Start sending unordered after the second reception, so that an +%% outgoing CER/CEA will arrive at the peer before another request. -recv(#transport{unordered = B} = S) +recv(#transport{rotate = B} = S) when is_boolean(B) -> S; -recv(#transport{unordered = 0, streams = {_, 1}, socket = Sock} = S) -> +recv(#transport{rotate = 0, streams = {_,N}, socket = Sock} = S) -> ok = inet:setopts(Sock, [{sctp_default_send_param, #sctp_sndrcvinfo{flags = [unordered]}}]), - S#transport{unordered = true}; + S#transport{rotate = 1 < N}; -recv(#transport{unordered = 0} = S) -> - S#transport{unordered = false}; - -recv(#transport{unordered = N} = S) -> - S#transport{unordered = N-1}. +recv(#transport{rotate = N} = S) -> + S#transport{rotate = N-1}. %% publish/4 -- cgit v1.2.3