diff options
Diffstat (limited to 'lib/diameter')
| -rw-r--r-- | lib/diameter/doc/src/diameter.xml | 43 | ||||
| -rw-r--r-- | lib/diameter/doc/src/notes.xml | 88 | ||||
| -rw-r--r-- | lib/diameter/include/diameter_gen.hrl | 8 | ||||
| -rw-r--r-- | lib/diameter/src/base/diameter.erl | 1 | ||||
| -rw-r--r-- | lib/diameter/src/base/diameter_codec.erl | 10 | ||||
| -rw-r--r-- | lib/diameter/src/base/diameter_config.erl | 3 | ||||
| -rw-r--r-- | lib/diameter/src/base/diameter_service.erl | 13 | ||||
| -rw-r--r-- | lib/diameter/src/base/diameter_traffic.erl | 81 | ||||
| -rw-r--r-- | lib/diameter/src/base/diameter_watchdog.erl | 8 | ||||
| -rw-r--r-- | lib/diameter/src/diameter.appup.src | 12 | ||||
| -rw-r--r-- | lib/diameter/vsn.mk | 2 |
11 files changed, 194 insertions, 75 deletions
diff --git a/lib/diameter/doc/src/diameter.xml b/lib/diameter/doc/src/diameter.xml index 47247bc2ff..61b7fd1337 100644 --- a/lib/diameter/doc/src/diameter.xml +++ b/lib/diameter/doc/src/diameter.xml @@ -913,6 +913,49 @@ Options <c>monitor</c> and <c>link</c> are ignored.</p> Defaults to the empty list.</p> </item> +<marker id="strict_mbit"/> +<tag><c>{strict_mbit, boolean()}</c></tag> +<item> +<p> +Whether or not to regard an AVP setting the M-bit as erroneous when +the command grammar in question does not explicitly allow the AVP. +If <c>true</c> then such AVPs are regarded as 5001 errors, +DIAMETER_AVP_UNSUPPORTED. +If <c>false</c> then the M-bit is ignored and policing +it becomes the receiver's responsibility.</p> + +<p> +Defaults to <c>true</c>.</p> + +<warning> +<p> +RFC 6733 is unclear about the semantics of the M-bit. +One the one hand, the CCF specification in section 3.2 documents AVP +in a command grammar as meaning <b>any</b> arbitrary AVP; on the +other hand, 1.3.4 states that AVPs setting the M-bit cannot be added +to an existing command: the modified command must instead be +placed in a new Diameter application.</p> +<p> +The reason for the latter is presumably interoperability: +allowing arbitrary AVPs setting the M-bit in a command makes its +interpretation implementation-dependent, since there's no +guarantee that all implementations will understand the same set of +arbitrary AVPs in the context of a given command. +However, interpreting <c>AVP</c> in a command grammar as <b>any</b> +AVP, regardless of M-bit, renders 1.3.4 meaningless, since the receiver +can simply ignore any AVP it thinks isn't relevant, regardless of the +sender's intent.</p> +<p> +Beware of confusing mandatory in the sense of the M-bit with mandatory +in the sense of the command grammar. +The former is a semantic requirement: that the receiver understand the +semantics of the AVP in the context in question. +The latter is a syntactic requirement: whether or not the AVP must +occur in the message in question.</p> +</warning> + +</item> + <marker id="string_decode"/> <tag><c>{string_decode, boolean()}</c></tag> <item> diff --git a/lib/diameter/doc/src/notes.xml b/lib/diameter/doc/src/notes.xml index c5f0d66f10..828ade4a71 100644 --- a/lib/diameter/doc/src/notes.xml +++ b/lib/diameter/doc/src/notes.xml @@ -43,21 +43,33 @@ first.</p> <!-- ===================================================================== --> -<section><title>diameter 1.11</title> +<section><title>diameter 1.11.1</title> <section><title>Fixed Bugs and Malfunctions</title> <list> <item> <p> - Don't report 5005 (DIAMETER_AVP_MISSING) errors - unnecessarily.</p> + Fix request table leaks</p> <p> - An AVP whose decode failed was reported as missing, - despite having been reported with another error as a - consequence of the failure.</p> + The End-to-End and Hop-by-Hop identifiers of outgoing + Diameter requests are stored in a table in order for the + caller to be located when the corresponding answer + message is received. Entries were orphaned if the handler + was terminated by an exit signal as a consequence of + actions taken by callback functions, or if callbacks + modified identifiers in retransmission cases.</p> <p> - Own Id: OTP-12871</p> + Own Id: OTP-13137</p> </item> + </list> + </section> + +</section> + +<section><title>diameter 1.11</title> + + <section><title>Fixed Bugs and Malfunctions</title> + <list> <item> <p> Fix relay encode of nested, Grouped AVPs.</p> @@ -69,16 +81,6 @@ first.</p> </item> <item> <p> - Improve decode performance.</p> - <p> - The time required to decode a message increased - quadratically with the number of AVPs in the worst case, - leading to extremely long execution times.</p> - <p> - Own Id: OTP-12891</p> - </item> - <item> - <p> Match acceptable peer addresses case insensitively.</p> <p> Regular expressions passed in an 'accept' tuple to @@ -89,6 +91,43 @@ first.</p> </item> <item> <p> + Fix diameter_watchdog function clause.</p> + <p> + OTP-12912 introduced an error with accepting transports + setting <c>{restrict_connections, false}</c>, causing + processes to fail when peer connections were terminated.</p> + <p> + Own Id: OTP-12969</p> + </item> + </list> + </section> + + + <section><title>Improvements and New Features</title> + <list> + <item> + <p> + Don't report 5005 (DIAMETER_AVP_MISSING) errors + unnecessarily.</p> + <p> + An AVP whose decode failed was reported as missing, + despite having been reported with another error as a + consequence of the failure.</p> + <p> + Own Id: OTP-12871</p> + </item> + <item> + <p> + Improve decode performance.</p> + <p> + The time required to decode a message increased + quadratically with the number of AVPs in the worst case, + leading to extremely long execution times.</p> + <p> + Own Id: OTP-12891</p> + </item> + <item> + <p> Improve watchdog and statistics performance.</p> <p> Inefficient use of timers contributed to poor performance @@ -97,13 +136,24 @@ first.</p> <p> Own Id: OTP-12912</p> </item> + <item> + <p> + Add service_opt() strict_mbit.</p> + <p> + There are differing opinions on whether or not reception + of an arbitrary AVP setting the M-bit is an error. The + default interpretation is strict: if a command grammar + doesn't explicitly allow an AVP setting the M-bit then + reception of such an AVP is regarded as an error. Setting + <c>{strict_mbit, false}</c> disables this check.</p> + <p> + Own Id: OTP-12947</p> + </item> </list> </section> </section> -<!-- ===================================================================== --> - <section><title>diameter 1.10</title> <section><title>Fixed Bugs and Malfunctions</title> diff --git a/lib/diameter/include/diameter_gen.hrl b/lib/diameter/include/diameter_gen.hrl index 5624ee6626..611ad796a9 100644 --- a/lib/diameter/include/diameter_gen.hrl +++ b/lib/diameter/include/diameter_gen.hrl @@ -31,7 +31,10 @@ %% Key to a value in the process dictionary that determines whether or %% not an unrecognized AVP setting the M-bit should be regarded as an -%% error or not. See is_strict/0. +%% error or not. See is_strict/0. This is only used to relax M-bit +%% interpretation inside Grouped AVPs not setting the M-bit. The +%% service_opt() strict_mbit can be used to disable the check +%% globally. -define(STRICT_KEY, strict). %% Key that says whether or not we should do a best-effort decode @@ -448,7 +451,8 @@ relax(_, _) -> false. is_strict() -> - false /= getr(?STRICT_KEY). + diameter_codec:getopt(strict_mbit) + andalso false /= getr(?STRICT_KEY). %% relax/1 %% diff --git a/lib/diameter/src/base/diameter.erl b/lib/diameter/src/base/diameter.erl index e82c2c168c..de88f6befd 100644 --- a/lib/diameter/src/base/diameter.erl +++ b/lib/diameter/src/base/diameter.erl @@ -312,6 +312,7 @@ call(SvcName, App, Message) -> | {sequence, sequence() | evaluable()} | {share_peers, remotes()} | {string_decode, boolean()} + | {strict_mbit, boolean()} | {incoming_maxlen, message_length()} | {use_shared_peers, remotes()} | {spawn_opt, list()}. diff --git a/lib/diameter/src/base/diameter_codec.erl b/lib/diameter/src/base/diameter_codec.erl index bcdc5b3005..1ea5357924 100644 --- a/lib/diameter/src/base/diameter_codec.erl +++ b/lib/diameter/src/base/diameter_codec.erl @@ -77,9 +77,10 @@ setopts(Opts) when is_list(Opts) -> lists:foreach(fun setopt/1, Opts). -%% Decode stringish types to string()? The default true is for -%% backwards compatibility. -setopt({string_decode = K, false = B}) -> +%% The default string_decode true is for backwards compatibility. +setopt({K, false = B}) + when K == string_decode; + K == strict_mbit -> setopt(K, B); %% Regard anything but the generated RFC 3588 dictionary as modern. @@ -97,7 +98,8 @@ setopt(Key, Value) -> getopt(Key) -> case get({diameter, Key}) of - undefined when Key == string_decode -> + undefined when Key == string_decode; + Key == strict_mbit -> true; undefined when Key == rfc -> 6733; diff --git a/lib/diameter/src/base/diameter_config.erl b/lib/diameter/src/base/diameter_config.erl index b7d8345b6c..702f11593a 100644 --- a/lib/diameter/src/base/diameter_config.erl +++ b/lib/diameter/src/base/diameter_config.erl @@ -647,6 +647,7 @@ make_config(SvcName, Opts) -> {?NOMASK, sequence}, {nodes, restrict_connections}, {16#FFFFFF, incoming_maxlen}, + {true, strict_mbit}, {true, string_decode}, {[], spawn_opt}]), @@ -685,12 +686,14 @@ opt(K, false = B) K == use_shared_peers; K == monitor; K == restrict_connections; + K == strict_mbit; K == string_decode -> B; opt(K, true = B) when K == share_peers; K == use_shared_peers; + K == strict_mbit; K == string_decode -> B; diff --git a/lib/diameter/src/base/diameter_service.erl b/lib/diameter/src/base/diameter_service.erl index e47b975768..d49176ec3e 100644 --- a/lib/diameter/src/base/diameter_service.erl +++ b/lib/diameter/src/base/diameter_service.erl @@ -129,6 +129,7 @@ | {share_peers, diameter:remotes()} %% broadcast to | {use_shared_peers, diameter:remotes()} %% use from | {restrict_connections, diameter:restriction()} + | {strict_mbit, boolean()} | {string_decode, boolean()} | {incoming_maxlen, diameter:message_length()}]}). %% shared_peers reflects the peers broadcast from remote nodes. @@ -207,7 +208,7 @@ stop_transport(SvcName, [_|_] = Refs) -> info(SvcName, Item) -> case lookup_state(SvcName) of - [#state{} = S] -> + [S] -> service_info(Item, S); [] -> undefined @@ -216,7 +217,12 @@ info(SvcName, Item) -> %% lookup_state/1 lookup_state(SvcName) -> - ets:lookup(?STATE_TABLE, SvcName). + case ets:lookup(?STATE_TABLE, SvcName) of + [#state{}] = L -> + L; + _ -> + [] + end. %% --------------------------------------------------------------------------- %% # subscribe/1 @@ -698,7 +704,8 @@ service_options(Opts) -> ?RESTRICT)}, {spawn_opt, proplists:get_value(spawn_opt, Opts, [])}, {string_decode, proplists:get_value(string_decode, Opts, true)}, - {incoming_maxlen, proplists:get_value(incoming_maxlen, Opts, 16#FFFFFF)}]. + {incoming_maxlen, proplists:get_value(incoming_maxlen, Opts, 16#FFFFFF)}, + {strict_mbit, proplists:get_value(strict_mbit, Opts, true)}]. %% The order of options is significant since we match against the list. mref(false = No) -> diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl index 692a01e651..07f39c562f 100644 --- a/lib/diameter/src/base/diameter_traffic.erl +++ b/lib/diameter/src/base/diameter_traffic.erl @@ -80,6 +80,7 @@ apps :: [#diameter_app{}], sequence :: diameter:sequence(), codec :: [{string_decode, boolean()} + | {strict_mbit, boolean()} | {incoming_maxlen, diameter:message_length()}]}). %% Note that incoming_maxlen is currently handled in diameter_peer_fsm, %% so that any message exceeding the maximum is discarded. Retain the @@ -106,7 +107,8 @@ make_recvdata([SvcName, PeerT, Apps, SvcOpts | _]) -> sequence = Mask, codec = [T || {K,_} = T <- SvcOpts, lists:member(K, [string_decode, - incoming_maxlen])]}. + incoming_maxlen, + strict_mbit])]}. %% --------------------------------------------------------------------------- %% peer_up/1 @@ -167,7 +169,7 @@ incr_error(Dir, Id, TPid, _) -> incr_error(Dir, Id, TPid) -> incr(TPid, {Id, Dir, error}). - + %% --------------------------------------------------------------------------- %% incr_rc/4 %% --------------------------------------------------------------------------- @@ -1483,16 +1485,12 @@ send_R(Pkt0, caps = Caps, packet = Pkt0}, - try - incr(send, Pkt, TPid, AppDict), - TRef = send_request(TPid, Pkt, Req, SvcName, Timeout), - Pid ! Ref, %% tell caller a send has been attempted - handle_answer(SvcName, - App, - recv_A(Timeout, SvcName, App, Opts, {TRef, Req})) - after - erase_requests(Pkt) - end. + incr(send, Pkt, TPid, AppDict), + TRef = send_request(TPid, Pkt, Req, SvcName, Timeout), + Pid ! Ref, %% tell caller a send has been attempted + handle_answer(SvcName, + App, + recv_A(Timeout, SvcName, App, Opts, {TRef, Req})). %% recv_A/5 @@ -1692,9 +1690,18 @@ encode(_, _, #diameter_packet{} = Pkt) -> send_request(TPid, #diameter_packet{bin = Bin} = Pkt, Req, _SvcName, Timeout) when node() == node(TPid) -> - %% Store the outgoing request before sending to avoid a race with - %% reply reception. - TRef = store_request(TPid, Bin, Req, Timeout), + Seqs = diameter_codec:sequence_numbers(Bin), + TRef = erlang:start_timer(Timeout, self(), TPid), + Entry = {Seqs, Req, TRef}, + + %% Ensure that request table is cleaned even if we receive an exit + %% signal. An alternative would be to simply trap exits, but + %% callbacks are applied in this process, and these could possibly + %% be expecting the prevailing behaviour. + Self = self(), + spawn(fun() -> diameter_lib:wait([Self]), erase_request(Entry) end), + + store_request(Entry, TPid), send(TPid, Pkt), TRef; @@ -1709,31 +1716,21 @@ send_request(TPid, #diameter_packet{} = Pkt, Req, SvcName, Timeout) -> %% send/1 send({TPid, Pkt, #request{handler = Pid} = Req0, SvcName, Timeout, TRef}) -> - Seqs = diameter_codec:sequence_numbers(Pkt), Req = Req0#request{handler = self()}, - Ref = send_request(TPid, Pkt, Req, SvcName, Timeout), - - try - recv(TPid, Pid, TRef, Ref) - after - %% Remove only the entry for this specific send since a resend - %% from the originating node can pick another transport on - %% this one. - ets:delete_object(?REQUEST_TABLE, {Seqs, Req, Ref}) - end. + recv(TPid, Pid, TRef, send_request(TPid, Pkt, Req, SvcName, Timeout)). %% recv/4 %% %% Relay an answer from a remote node. -recv(TPid, Pid, TRef, Ref) -> +recv(TPid, Pid, TRef, LocalTRef) -> receive {answer, _, _, _, _} = A -> Pid ! A; - {failover = T, Ref} -> + {failover = T, LocalTRef} -> Pid ! {T, TRef}; T -> - exit({timeout, Ref, TPid} = T) + exit({timeout, LocalTRef, TPid} = T) end. %% send/2 @@ -1810,17 +1807,21 @@ resend_request(Pkt0, TRef = send_request(TPid, Pkt, Req, SvcName, Tmo), {TRef, Req}. -%% store_request/4 +%% store_request/2 -store_request(TPid, Bin, Req, Timeout) -> - Seqs = diameter_codec:sequence_numbers(Bin), - TRef = erlang:start_timer(Timeout, self(), TPid), - ets:insert(?REQUEST_TABLE, {Seqs, Req, TRef}), +store_request(T, TPid) -> + ets:insert(?REQUEST_TABLE, T), ets:member(?REQUEST_TABLE, TPid) - orelse (self() ! {failover, TRef}), %% failover/1 may have missed - TRef. + orelse begin + {_Seqs, _Req, TRef} = T, + (self() ! {failover, TRef}) %% failover/1 may have missed + end. %% lookup_request/2 +%% +%% Note the match on both the key and transport pid. The latter is +%% necessary since the same Hop-by-Hop and End-to-End identifiers are +%% reused in the case of retransmission. lookup_request(Msg, TPid) -> Seqs = diameter_codec:sequence_numbers(Msg), @@ -1834,10 +1835,10 @@ lookup_request(Msg, TPid) -> false end. -%% erase_requests/1 +%% erase_request/1 -erase_requests(Pkt) -> - ets:delete(?REQUEST_TABLE, diameter_codec:sequence_numbers(Pkt)). +erase_request(T) -> + ets:delete_object(?REQUEST_TABLE, T). %% match_requests/1 @@ -1860,7 +1861,7 @@ failover(TPid) when is_pid(TPid) -> lists:foreach(fun failover/1, match_requests(TPid)); %% Note that a request process can store its request after failover -%% notifications are sent here: store_request/4 sends the notification +%% notifications are sent here: store_request/2 sends the notification %% in that case. %% Failover as a consequence of request_peer_down/1: inform the diff --git a/lib/diameter/src/base/diameter_watchdog.erl b/lib/diameter/src/base/diameter_watchdog.erl index 3c0d8f6f6e..ea8b2fdb0e 100644 --- a/lib/diameter/src/base/diameter_watchdog.erl +++ b/lib/diameter/src/base/diameter_watchdog.erl @@ -541,13 +541,13 @@ set_watchdog(#watchdog{tref = undefined} = S) -> %% Timer already set: start at new one only at expiry. set_watchdog(#watchdog{} = S) -> - S#watchdog{tref = diameter_lib:now()}; - -set_watchdog(stop = No) -> - No. + S#watchdog{tref = diameter_lib:now()}. %% set_watchdog/2 +set_watchdog(_, stop = No) -> + No; + set_watchdog(Ms, #watchdog{tw = TwInit} = S) -> S#watchdog{tref = erlang:start_timer(tw(TwInit, Ms), self(), tw)}. diff --git a/lib/diameter/src/diameter.appup.src b/lib/diameter/src/diameter.appup.src index 788ea790fa..de0f324f47 100644 --- a/lib/diameter/src/diameter.appup.src +++ b/lib/diameter/src/diameter.appup.src @@ -44,6 +44,7 @@ {"1.9.1", [{restart_application, diameter}]}, %% 17.5.3 {"1.9.2", [{restart_application, diameter}]}, %% 17.5.5 {"1.9.2.1", [{restart_application, diameter}]}, %% 17.5.6.3 + {"1.9.2.2", [{restart_application, diameter}]}, %% 17.5.6.7 {"1.10", [{load_module, diameter_codec}, %% 18.0 {load_module, diameter_peer_fsm}, {load_module, diameter_watchdog}, @@ -52,13 +53,16 @@ {load_module, diameter_lib}, {load_module, diameter_peer}, {load_module, diameter_reg}, + {load_module, diameter_traffic}, {load_module, diameter_service}, {load_module, diameter_sync}, + {load_module, diameter}, {load_module, diameter_gen_base_rfc6733}, {load_module, diameter_gen_acct_rfc6733}, {load_module, diameter_gen_base_rfc3588}, {load_module, diameter_gen_base_accounting}, - {load_module, diameter_gen_relay}]} + {load_module, diameter_gen_relay}]}, + {"1.11", [{load_module, diameter_traffic}]} %% 18.1 ], [ {"0.9", [{restart_application, diameter}]}, @@ -84,13 +88,16 @@ {"1.9.1", [{restart_application, diameter}]}, {"1.9.2", [{restart_application, diameter}]}, {"1.9.2.1", [{restart_application, diameter}]}, + {"1.9.2.2", [{restart_application, diameter}]}, {"1.10", [{load_module, diameter_gen_relay}, {load_module, diameter_gen_base_accounting}, {load_module, diameter_gen_base_rfc3588}, {load_module, diameter_gen_acct_rfc6733}, {load_module, diameter_gen_base_rfc6733}, + {load_module, diameter}, {load_module, diameter_sync}, {load_module, diameter_service}, + {load_module, diameter_traffic}, {load_module, diameter_reg}, {load_module, diameter_peer}, {load_module, diameter_lib}, @@ -98,6 +105,7 @@ {load_module, diameter_stats}, {load_module, diameter_watchdog}, {load_module, diameter_peer_fsm}, - {load_module, diameter_codec}]} + {load_module, diameter_codec}]}, + {"1.11", [{load_module, diameter_traffic}]} ] }. diff --git a/lib/diameter/vsn.mk b/lib/diameter/vsn.mk index 041d21b261..7ac4a7adfb 100644 --- a/lib/diameter/vsn.mk +++ b/lib/diameter/vsn.mk @@ -17,5 +17,5 @@ # %CopyrightEnd% APPLICATION = diameter -DIAMETER_VSN = 1.11 +DIAMETER_VSN = 1.11.1 APP_VSN = $(APPLICATION)-$(DIAMETER_VSN)$(PRE_VSN) |