From f3d0347a6707149cc9e016c85f54142fe99e25d1 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Mon, 21 Jul 2014 09:51:37 +0200 Subject: Tweak comments --- lib/diameter/src/info/diameter_dbg.erl | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) (limited to 'lib') diff --git a/lib/diameter/src/info/diameter_dbg.erl b/lib/diameter/src/info/diameter_dbg.erl index b5b3983afa..68b3d0f973 100644 --- a/lib/diameter/src/info/diameter_dbg.erl +++ b/lib/diameter/src/info/diameter_dbg.erl @@ -69,7 +69,7 @@ -define(VALUES(Rec), tl(tuple_to_list(Rec))). %% ---------------------------------------------------------- -%% # table(TableName) +%% # table/1 %% %% Pretty-print a diameter table. Returns the number of records %% printed, or undefined. @@ -97,7 +97,7 @@ split([F|Fs], [V|Vs]) -> {F, Fs, V, Vs}. %% ---------------------------------------------------------- -%% # TableName() +%% # TableName/0 %% ---------------------------------------------------------- -define(TABLE(Name), Name() -> table(Name)). @@ -111,7 +111,7 @@ split([F|Fs], [V|Vs]) -> ?TABLE(diameter_stats). %% ---------------------------------------------------------- -%% # tables() +%% # tables/0 %% %% Pretty-print diameter tables from all nodes. Returns the number of %% records printed. @@ -127,7 +127,7 @@ split(_, Fs, Vs) -> split(Fs, Vs). %% ---------------------------------------------------------- -%% # modules() +%% # modules/0 %% ---------------------------------------------------------- modules() -> @@ -140,49 +140,49 @@ appdir() -> [_|_] = code:lib_dir(?APP, ebin). %% ---------------------------------------------------------- -%% # versions() +%% # versions/0 %% ---------------------------------------------------------- versions() -> ?I:versions(modules()). %% ---------------------------------------------------------- -%% # versions() +%% # version_info/0 %% ---------------------------------------------------------- version_info() -> ?I:version_info(modules()). %% ---------------------------------------------------------- -%% # compiled() +%% # compiled/0 %% ---------------------------------------------------------- compiled() -> ?I:compiled(modules()). %% ---------------------------------------------------------- -%% procs() +%% # procs/0 %% ---------------------------------------------------------- procs() -> ?I:procs(?APP). %% ---------------------------------------------------------- -%% # latest() +%% # latest/0 %% ---------------------------------------------------------- latest() -> ?I:latest(modules()). %% ---------------------------------------------------------- -%% # nl() +%% # nl/0 %% ---------------------------------------------------------- nl() -> lists:foreach(fun(M) -> abcast = c:nl(M) end, modules()). %% ---------------------------------------------------------- -%% # pp(Bin) +%% # pp/1 %% %% Description: Pretty-print a message binary. %% ---------------------------------------------------------- @@ -317,7 +317,7 @@ ppp({Field, Value}) -> io:format(": ~-22s : ~p~n", [Field, Value]). %% ---------------------------------------------------------- -%% # subscriptions() +%% # subscriptions/0 %% %% Returns a list of {SvcName, Pid}. %% ---------------------------------------------------------- @@ -326,7 +326,7 @@ subscriptions() -> diameter_service:subscriptions(). %% ---------------------------------------------------------- -%% # children() +%% # children/0 %% ---------------------------------------------------------- children() -> -- cgit v1.2.3 From 0631af51f406139302f32bae84be5788b26c25d2 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Mon, 21 Jul 2014 09:12:47 +0200 Subject: Add diameter_dbg:sizes/0 To return sizes of named ets tables. --- lib/diameter/src/info/diameter_dbg.erl | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/diameter/src/info/diameter_dbg.erl b/lib/diameter/src/info/diameter_dbg.erl index 68b3d0f973..b536e5e80b 100644 --- a/lib/diameter/src/info/diameter_dbg.erl +++ b/lib/diameter/src/info/diameter_dbg.erl @@ -32,7 +32,8 @@ compiled/0, procs/0, latest/0, - nl/0]). + nl/0, + sizes/0]). -export([diameter_config/0, diameter_peer/0, @@ -68,6 +69,15 @@ -define(VALUES(Rec), tl(tuple_to_list(Rec))). +%% ---------------------------------------------------------- +%% # sizes/0 +%% +%% Return sizes of named tables. +%% ---------------------------------------------------------- + +sizes() -> + [{T, ets:info(T, size)} || T <- ?LOCAL, T /= diameter_peer]. + %% ---------------------------------------------------------- %% # table/1 %% -- cgit v1.2.3 From 3f090a60f737aa2410b47460d070401d9dd5179e Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Mon, 21 Jul 2014 10:26:10 +0200 Subject: Add (process) info tuple to diameter:service_info/2 To show process_info of interest. This is not yet documented since it may well change. --- lib/diameter/src/base/diameter_service.erl | 39 +++++++++++++++++++++++++----- 1 file changed, 33 insertions(+), 6 deletions(-) (limited to 'lib') diff --git a/lib/diameter/src/base/diameter_service.erl b/lib/diameter/src/base/diameter_service.erl index b7cd311e02..8b25802568 100644 --- a/lib/diameter/src/base/diameter_service.erl +++ b/lib/diameter/src/base/diameter_service.erl @@ -1745,12 +1745,11 @@ peer_acc(PeerT, Acc, #watchdog{pid = Pid, state = WS, started = At, peer = TPid}) -> - dict:append(Ref, - [{type, Type}, - {options, Opts}, - {watchdog, {Pid, At, WS}} - | info_peer(PeerT, TPid, WS)], - Acc). + Info = [{type, Type}, + {options, Opts}, + {watchdog, {Pid, At, WS}} + | info_peer(PeerT, TPid, WS)], + dict:append(Ref, Info ++ [{info, info_process_info(Info)}], Acc). info_peer(PeerT, TPid, WS) when is_pid(TPid), WS /= ?WD_DOWN -> @@ -1762,6 +1761,34 @@ info_peer(PeerT, TPid, WS) info_peer(_, _, _) -> []. +info_process_info(Info) -> + lists:flatmap(fun ipi/1, Info). + +ipi({watchdog, {Pid, _, _}}) -> + info_pid(Pid); + +ipi({peer, {Pid, _}}) -> + info_pid(Pid); + +ipi({port, [{owner, Pid} | _]}) -> + info_pid(Pid); + +ipi(_) -> + []. + +info_pid(Pid) -> + case process_info(Pid, [message_queue_len, memory, binary]) of + undefined -> + []; + L -> + [{Pid, lists:map(fun({K,V}) -> {K, map_info(K,V)} end, L)}] + end. + +map_info(binary, L) -> + lists:reverse(lists:keysort(2, L)); +map_info(_, T) -> + T. + %% The point of extracting the config here is so that 'transport' info %% has one entry for each transport ref, the peer table only %% containing entries that have a living watchdog. -- cgit v1.2.3 From 66833b6dc19dd7fd8a19862e7815c6c3c1b4f230 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Mon, 21 Jul 2014 11:03:54 +0200 Subject: Add info item for diameter:service_info/2 To extract only process info from connections info, which can be useful to reduce the amount of information returned. Choose 'info' for the item since process_info is more than one word: all others are one. Don't choose memory since it's too specific: might want to use it for more. --- lib/diameter/src/base/diameter_service.erl | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/diameter/src/base/diameter_service.erl b/lib/diameter/src/base/diameter_service.erl index 8b25802568..44c2b707b7 100644 --- a/lib/diameter/src/base/diameter_service.erl +++ b/lib/diameter/src/base/diameter_service.erl @@ -1573,7 +1573,8 @@ transports(#state{watchdogT = WatchdogT}) -> -define(OTHER_INFO, [connections, name, peers, - statistics]). + statistics, + info]). service_info(Item, S) when is_atom(Item) -> @@ -1663,6 +1664,7 @@ complete_info(Item, #state{service = Svc} = S) -> keys -> ?ALL_INFO ++ ?CAP_INFO ++ ?OTHER_INFO; all -> service_info(?ALL_INFO, S); statistics -> info_stats(S); + info -> info_info(S); connections -> info_connections(S); peers -> info_peers(S) end. @@ -1846,6 +1848,13 @@ mk_app(#diameter_app{} = A) -> info_pending(#state{} = S) -> diameter_traffic:pending(transports(S)). +%% info_info/1 +%% +%% Extract process_info from connections info. + +info_info(S) -> + [I || L <- conn_list(S), {info, I} <- L]. + %% info_connections/1 %% %% One entry per transport connection. Statistics for each entry are -- cgit v1.2.3 From e2d4c35bb9ac11b5039514276f73dce4d2332100 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Tue, 5 Aug 2014 12:28:04 +0200 Subject: Fix counting of outgoing requests Commit df19c272 broke this in avoiding counting on arbitrary keys. It didn't break it sufficiently for the only counters usage in the test suites to fail however: watchdog counters worked as intended, but no others, not even CER and DPR. More testcases are needed. This commit does change/fix the previous semantics somewhat: - Retransmissions are no longer counted. This previously made it impossible to distinguish between these and unanswered requests, since both counted as an outgoing request. There should probably be a retransmission counter but it should be distinct from the sent request counter. - The counting is always on the node from which diameter:call/4 is invoked, not the node on which the transport resides, as was previously the case. (Although they're typically one and the same.) Note that none of these semantics are documented as yet, so we're not changing a documented interface. --- lib/diameter/src/base/diameter_peer_fsm.erl | 2 ++ lib/diameter/src/base/diameter_traffic.erl | 1 + 2 files changed, 3 insertions(+) (limited to 'lib') diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl index 31e570ae20..86fc43cdc5 100644 --- a/lib/diameter/src/base/diameter_peer_fsm.erl +++ b/lib/diameter/src/base/diameter_peer_fsm.erl @@ -477,6 +477,7 @@ send_CER(#state{state = {'Wait-Conn-Ack', Tmo}, hop_by_hop_id = Hid}} = Pkt = encode(CER, Dict), + incr(send, Pkt, Dict), send(TPid, Pkt), ?LOG(send, 'CER'), start_timer(Tmo, S#state{state = {'Wait-CEA', Hid, Eid}}). @@ -1100,6 +1101,7 @@ send_dpr(Reason, Opts, #state{transport = TPid, {'Origin-Realm', OR}, {'Disconnect-Cause', Cause}], Dict), + incr(send, Pkt, Dict), send(TPid, Pkt), dpa_timer(Tmo), ?LOG(send, 'DPR'), diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl index 5fac61f416..937d08dc4f 100644 --- a/lib/diameter/src/base/diameter_traffic.erl +++ b/lib/diameter/src/base/diameter_traffic.erl @@ -1396,6 +1396,7 @@ send_R(Pkt0, packet = Pkt0}, try + incr(send, Pkt, TPid, Dict), TRef = send_request(TPid, Pkt, Req, SvcName, Timeout), Pid ! Ref, %% tell caller a send has been attempted handle_answer(SvcName, -- cgit v1.2.3 From 50356d68b321f959fda4b2bf5c77453cd1fb6890 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Tue, 5 Aug 2014 14:48:53 +0200 Subject: Count request retransmissions As mentioned in the parent commit. The {Id, send, retransmission} key is of the same form as the {Id, send|recv, error} key used for encode/decode errors. --- lib/diameter/src/base/diameter_traffic.erl | 1 + 1 file changed, 1 insertion(+) (limited to 'lib') diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl index 937d08dc4f..33062e928c 100644 --- a/lib/diameter/src/base/diameter_traffic.erl +++ b/lib/diameter/src/base/diameter_traffic.erl @@ -1684,6 +1684,7 @@ resend_request(Pkt0, caps = Caps}, ?LOG(retransmission, Pkt#diameter_packet.header), + incr(TPid, {msg_id(Pkt, Dict), send, retransmission}), TRef = send_request(TPid, Pkt, Req, SvcName, Tmo), {TRef, Req}. -- cgit v1.2.3 From 7816ab2fc42307bcffd1cfbd40bbbab30c226fb9 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Tue, 5 Aug 2014 14:57:56 +0200 Subject: Count relayed messages on {relay, Rbit} Instead of grouping them with 'unknown'. These messages were keyed on {ApplicationId, CommandCode, Rbit} prior to commit df19c272, but distinguishing between the relay application and others is probably more useful. The only reason for not including the R-bit in the unknown key is that the key is also used elsewhere, and relay is an expected case while unknown isn't. --- lib/diameter/src/base/diameter_traffic.erl | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) (limited to 'lib') diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl index 33062e928c..f3c4e63afc 100644 --- a/lib/diameter/src/base/diameter_traffic.erl +++ b/lib/diameter/src/base/diameter_traffic.erl @@ -1074,12 +1074,22 @@ incr_rc(Dir, Pkt, Dict, TPid, Dict0) -> incr(TPid, {Id, Dir, Ctr}), Ctr. -%% Only count on known keeps so as not to be vulnerable to attack: -%% there are 2^32 (application ids) * 2^24 (command codes) * 2 (R-bits) -%% = 2^57 Ids for an attacker to choose from. +%% Only count on known keys so as not to be vulnerable to attack: +%% there are 2^32 (application ids) * 2^24 (command codes) = 2^56 +%% pairs for an attacker to choose from. msg_id(Hdr, Dict) -> {_ApplId, Code, R} = Id = diameter_codec:msg_id(Hdr), - choose('' == Dict:msg_name(Code, 0 == R), unknown, Id). + case Dict:msg_name(Code, 0 == R) of + '' -> + unknown(Dict:id(), R); + _ -> + Id + end. + +unknown(?APP_ID_RELAY, R) -> + {relay, R}; +unknown(_, _) -> + unknown. %% No E-bit: can't be 3xxx. is_result(RC, false, _Dict0) -> -- cgit v1.2.3 From ea2f229a0335aaaa1a5fcfb8eec0c7881cfdf7bb Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Tue, 5 Aug 2014 17:22:15 +0200 Subject: Map binary process info to a reference/byte count That is, instead of including the list in a diameter:service_info/2 info tuple, only include the number of references and the number of bytes referenced. The list itself can be quite large and typically isn't that interesting, at least not to a diameter user. --- lib/diameter/src/base/diameter_service.erl | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/diameter/src/base/diameter_service.erl b/lib/diameter/src/base/diameter_service.erl index 44c2b707b7..ab56ca9cef 100644 --- a/lib/diameter/src/base/diameter_service.erl +++ b/lib/diameter/src/base/diameter_service.erl @@ -1786,8 +1786,23 @@ info_pid(Pid) -> [{Pid, lists:map(fun({K,V}) -> {K, map_info(K,V)} end, L)}] end. +%% The binary list consists of 3-tuples {Ptr, Size, Count}, where Ptr +%% is a C pointer value, Size is the size of a referenced binary in +%% bytes, and Count is a global reference count. The same Ptr can +%% occur multiple times, once for each reference on the process heap. +%% In this case, the corresponding tuples will have Size in common but +%% Count may differ just because no global lock is taken when the +%% value is retrieved. +%% +%% The list can be quite large, and we aren't often interested in the +%% pointers or counts, so whittle this down to the number of binaries +%% referenced and their total byte count. map_info(binary, L) -> - lists:reverse(lists:keysort(2, L)); + SzD = lists:foldl(fun({P,S,_}, D) -> dict:store(P,S,D) end, + dict:new(), + L), + {dict:size(SzD), dict:fold(fun(_,S,N) -> S + N end, 0, SzD)}; + map_info(_, T) -> T. -- cgit v1.2.3 From ef1064f917b6bdc6814dd579e364ac3868f2f7aa Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Mon, 8 Sep 2014 11:56:17 +0200 Subject: Fix counters for answer-message An answer message that sets the E-bit is encoded/decoded with Diameter common dictionary, using the answer-message grammar specified in the RFC. However, the dictionary of the application in question is the one that knows the command code of the message. Commit df19c272 didn't make this distinction when incrementing counters for an answer-message, using the common dictionary for both purposes, causing the message to be counted as unknown. This commit remedies that. --- lib/diameter/src/base/diameter_traffic.erl | 71 +++++++++++++++++++----------- 1 file changed, 45 insertions(+), 26 deletions(-) (limited to 'lib') diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl index f3c4e63afc..c4875c5975 100644 --- a/lib/diameter/src/base/diameter_traffic.erl +++ b/lib/diameter/src/base/diameter_traffic.erl @@ -129,6 +129,11 @@ incr(Dir, #diameter_header{} = H, TPid, Dict) -> %% incr_error/4 %% --------------------------------------------------------------------------- +%% Identify messages using the application dictionary, not the encode +%% dictionary, which may differ in the case of answer-message. +incr_error(Dir, T, Pid, {_Dict, AppDict}) -> + incr_error(Dir, T, Pid, AppDict); + %% Decoded message without errors. incr_error(recv, #diameter_packet{errors = []}, _, _) -> ok; @@ -169,7 +174,7 @@ incr_error(Dir, Id, TPid) -> incr_rc(Dir, Pkt, TPid, Dict0) -> try - incr_rc(Dir, Pkt, Dict0, TPid, Dict0) + incr_result(Dir, Pkt, TPid, {Dict0, Dict0, Dict0}) catch exit: {E,_} when E == no_result_code; E == invalid_error_bit -> @@ -479,7 +484,7 @@ send_A({Caps, Pkt}, TPid, Dict0, _RecvData) -> %% unsupported application send_A({Caps, Pkt, App, {T, EvalPktFs, EvalFs}}, TPid, Dict0, RecvData) -> send_A(answer(T, Caps, Pkt, App, Dict0, RecvData), TPid, - Dict0, + {App#diameter_app.dictionary, Dict0}, Pkt, EvalPktFs, EvalFs); @@ -489,8 +494,8 @@ send_A(_, _, _, _) -> %% send_A/6 -send_A(T, TPid, Dict0, ReqPkt, EvalPktFs, EvalFs) -> - reply(T, TPid, Dict0, EvalPktFs, ReqPkt), +send_A(T, TPid, DictT, ReqPkt, EvalPktFs, EvalFs) -> + reply(T, TPid, DictT, EvalPktFs, ReqPkt), lists:foreach(fun diameter_lib:eval/1, EvalFs). %% answer/6 @@ -648,32 +653,32 @@ is_loop(Code, Vid, OH, Dict0, Avps) -> %% reply/5 %% Local answer ... -reply({Dict, Ans}, TPid, Dict0, Fs, ReqPkt) -> - reply(Ans, Dict, TPid, Dict0, Fs, ReqPkt); +reply({Dict, Ans}, TPid, {AppDict, Dict0}, Fs, ReqPkt) -> + local(Ans, TPid, {Dict, AppDict, Dict0}, Fs, ReqPkt); %% ... or relayed. reply(#diameter_packet{} = Pkt, TPid, _Dict0, Fs, _ReqPkt) -> eval_packet(Pkt, Fs), send(TPid, Pkt). -%% reply/6 +%% local/5 %% %% Send a locally originating reply. %% Skip the setting of Result-Code and Failed-AVP's below. This is %% undocumented and shouldn't be relied on. -reply([Msg], Dict, TPid, Dict0, Fs, ReqPkt) +local([Msg], TPid, DictT, Fs, ReqPkt) when is_list(Msg); is_tuple(Msg) -> - reply(Msg, Dict, TPid, Dict0, Fs, ReqPkt#diameter_packet{errors = []}); + local(Msg, TPid, DictT, Fs, ReqPkt#diameter_packet{errors = []}); -reply(Msg, Dict, TPid, Dict0, Fs, ReqPkt) -> - Pkt = encode(Dict, +local(Msg, TPid, {Dict, AppDict, Dict0} = DictT, Fs, ReqPkt) -> + Pkt = encode({Dict, AppDict}, TPid, reset(make_answer_packet(Msg, ReqPkt), Dict, Dict0), Fs), - incr(send, Pkt, TPid, Dict), - incr_rc(send, Pkt, Dict, TPid, Dict0), %% count outgoing + incr(send, Pkt, TPid, AppDict), + incr_result(send, Pkt, TPid, DictT), %% count outgoing send(TPid, Pkt). %% reset/3 @@ -1038,29 +1043,29 @@ find(Pred, [H|T]) -> %% code, the missing vendor id, and a zero filled payload of the minimum %% required length for the omitted AVP will be added. -%% incr_rc/5 +%% incr_result/5 %% %% Increment a stats counter for result codes in incoming and outgoing %% answers. %% Outgoing message as binary: don't count. (Sending binaries is only %% partially supported.) -incr_rc(_, #diameter_packet{msg = undefined = No}, _, _, _) -> +incr_result(_, #diameter_packet{msg = undefined = No}, _, _) -> No; %% Incoming or outgoing. Outgoing with encode errors never gets here %% since encode fails. -incr_rc(Dir, Pkt, Dict, TPid, Dict0) -> +incr_result(Dir, Pkt, TPid, {Dict, AppDict, Dict0}) -> #diameter_packet{header = #diameter_header{is_error = E} = Hdr, msg = Msg, errors = Es} = Pkt, - Id = msg_id(Hdr, Dict), + Id = msg_id(Hdr, AppDict), %% Count incoming decode errors. - recv /= Dir orelse [] == Es orelse incr_error(Dir, Id, TPid, Dict), + recv /= Dir orelse [] == Es orelse incr_error(Dir, Id, TPid, AppDict), %% Exit on a missing result code. T = rc_counter(Dict, Msg), @@ -1074,6 +1079,11 @@ incr_rc(Dir, Pkt, Dict, TPid, Dict0) -> incr(TPid, {Id, Dir, Ctr}), Ctr. +%% msg_id/2 + +msg_id(#diameter_packet{header = H}, Dict) -> + msg_id(H, Dict); + %% Only count on known keys so as not to be vulnerable to attack: %% there are 2^32 (application ids) * 2^24 (command codes) = 2^56 %% pairs for an attacker to choose from. @@ -1442,14 +1452,14 @@ handle_answer(SvcName, App, {error, Req, Reason}) -> handle_error(App, Req, Reason, SvcName); handle_answer(SvcName, - #diameter_app{dictionary = Dict, + #diameter_app{dictionary = AppDict, id = Id} = App, {answer, Req, Dict0, Pkt}) -> - Mod = dict(Dict, Dict0, Pkt), - handle_A(errors(Id, diameter_codec:decode(Mod, Pkt)), + Dict = dict(AppDict, Dict0, Pkt), + handle_A(errors(Id, diameter_codec:decode(Dict, Pkt)), SvcName, - Mod, + Dict, Dict0, App, Req). @@ -1459,10 +1469,12 @@ handle_answer(SvcName, %% want to examine the answer? handle_A(Pkt, SvcName, Dict, Dict0, App, #request{transport = TPid} = Req) -> - incr(recv, Pkt, TPid, Dict), + AppDict = App#diameter_app.dictionary, + + incr(recv, Pkt, TPid, AppDict), try - incr_rc(recv, Pkt, Dict, TPid, Dict0) %% count incoming + incr_result(recv, Pkt, TPid, {Dict, AppDict, Dict0}) %% count incoming of _ -> answer(Pkt, SvcName, App, Req) catch @@ -1479,6 +1491,8 @@ handle_A(Pkt, SvcName, Dict, Dict0, App, #request{transport = TPid} = Req) -> answer(Pkt#diameter_packet{errors = [E|Es]}, SvcName, App, Req) end. +%% answer/4 + answer(Pkt, SvcName, #diameter_app{module = ModX, @@ -1579,13 +1593,18 @@ encode(Dict, TPid, Pkt, Fs) -> %% an encoded binary. This isn't the usual case and doesn't properly %% support retransmission but is useful for test. +encode(Dict, TPid, Pkt) + when is_atom(Dict) -> + encode({Dict, Dict}, TPid, Pkt); + %% A message to be encoded. -encode(Dict, TPid, #diameter_packet{bin = undefined} = Pkt) -> +encode(DictT, TPid, #diameter_packet{bin = undefined} = Pkt) -> + {Dict, AppDict} = DictT, try diameter_codec:encode(Dict, Pkt) catch exit: {diameter_codec, encode, T} = Reason -> - incr_error(send, T, TPid, Dict), + incr_error(send, T, TPid, AppDict), exit(Reason) end; -- cgit v1.2.3 From deb951bf382edaf7e8fcec7f57a94d798d2a460f Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Thu, 14 Aug 2014 21:07:07 +0200 Subject: Fix decode of Failed-AVP in RFC 3588 answer-message Commit 066544fa had the unintended consequence of breaking the decode of Failed-AVP in answer-message as defined in the RFC 3588, since the grammar doesn't list Failed-AVP as an explicit component AVP, in contrast to the RFC 6733 grammar, which does. Handle this case explicitly, as an exception, just as with Failed-AVP as parent AVP. --- lib/diameter/include/diameter_gen.hrl | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) (limited to 'lib') diff --git a/lib/diameter/include/diameter_gen.hrl b/lib/diameter/include/diameter_gen.hrl index 7e91ce375f..97cf0c7e29 100644 --- a/lib/diameter/include/diameter_gen.hrl +++ b/lib/diameter/include/diameter_gen.hrl @@ -453,8 +453,8 @@ pack_AVP(_, #diameter_avp{data = <<0:1, Data/binary>>} = Avp, Acc) -> {Rec, Failed} = Acc, {Rec, [{5014, Avp#diameter_avp{data = Data}} | Failed]}; -pack_AVP(Name, #diameter_avp{is_mandatory = M} = Avp, Acc) -> - case pack_arity(Name, M) of +pack_AVP(Name, #diameter_avp{is_mandatory = M, name = AvpName} = Avp, Acc) -> + case pack_arity(Name, AvpName, M) of 0 -> {Rec, Failed} = Acc, {Rec, [{if M -> 5001; true -> 5008 end, Avp} | Failed]}; @@ -462,10 +462,13 @@ pack_AVP(Name, #diameter_avp{is_mandatory = M} = Avp, Acc) -> pack(Arity, 'AVP', Avp, Acc) end. -%% Give Failed-AVP special treatment since it'll contain any -%% unrecognized mandatory AVP's. -pack_arity(Name, M) -> - NF = Name /= 'Failed-AVP' andalso not is_failed(), +%% Give Failed-AVP special treatment since (1) it'll contain any +%% unrecognized mandatory AVP's and (2) the RFC 3588 grammar failed to +%% allow for Failed-AVP in an answer-message. + +pack_arity(Name, AvpName, M) -> + IsFailed = Name == 'Failed-AVP' orelse is_failed(), + %% Not testing just Name /= 'Failed-AVP' means we're changing the %% packing of AVPs nested within Failed-AVP, but the point of %% ignoring errors within Failed-AVP is to decode as much as @@ -473,12 +476,18 @@ pack_arity(Name, M) -> %% packed into a dedicated field defeats that point. Note that we %% can't just test not is_failed() since this will be 'true' when %% packing an unknown AVP directly within Failed-AVP. - case NF andalso M andalso is_strict() of - true -> - 0; - false -> - avp_arity(Name, 'AVP') - end. + + pack_arity(IsFailed + orelse {Name, AvpName} == {'answer-message', 'Failed-AVP'} + orelse not M + orelse not is_strict(), + Name). + +pack_arity(true, Name) -> + avp_arity(Name, 'AVP'); + +pack_arity(false, _) -> + 0. %% 3588: %% -- cgit v1.2.3 From 0f9cdbaf4d7fde93d319be7789dd4119092d91c6 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Wed, 20 Aug 2014 22:44:28 +0200 Subject: Fix best effort decode of Failed-AVP Commit c2c00fdd didn't get it quite right: it only decoded failed AVPs in the common dictionary since it's this dictionary an answer-message is decoded in. An extra dictionary isn't something that's easily passed through the decode without rewriting dictionary compilation however, and that's no small job, so continue with the use/abuse of the process dictionary by storing the dictionary module for the decode to retrieve. This is one step worse than previous uses since the dictionary is put in one module (diameter_codec) and got in another (the dictionary module), but it's the lesser of two evils. --- lib/diameter/include/diameter_gen.hrl | 38 ++++++++++++++++++++++++------ lib/diameter/src/base/diameter_codec.erl | 30 +++++++++++++++++++---- lib/diameter/src/base/diameter_traffic.erl | 4 ++-- 3 files changed, 59 insertions(+), 13 deletions(-) (limited to 'lib') diff --git a/lib/diameter/include/diameter_gen.hrl b/lib/diameter/include/diameter_gen.hrl index 97cf0c7e29..233eb8dd15 100644 --- a/lib/diameter/include/diameter_gen.hrl +++ b/lib/diameter/include/diameter_gen.hrl @@ -311,7 +311,9 @@ d(Name, Avp, Acc) -> Failed = relax(Name), %% Not AvpName or else a failed Failed-AVP %% decode is packed into 'AVP'. - try avp(decode, Data, AvpName) of + Mod = dict(Failed), %% Dictionary to decode in. + + try Mod:avp(decode, Data, AvpName) of V -> {Avps, T} = Acc, {H, A} = ungroup(V, Avp), @@ -324,6 +326,25 @@ d(Name, Avp, Acc) -> reset(?FAILED_KEY, Failed) end. +%% dict/1 +%% +%% Retrieve the dictionary for the best-effort decode of Failed-AVP, +%% as put by diameter_codec:decode/2. See that function for the +%% explanation. + +dict(true) -> + case get({diameter_codec, dictionary}) of + undefined -> + ?MODULE; + Mod -> + Mod + end; + +dict(_) -> + ?MODULE. + +%% d/5 + %% Ignore a decode error within Failed-AVP ... d(true, _, Name, Avp, Acc) -> decode_AVP(Name, Avp, Acc); @@ -341,6 +362,8 @@ d(false, Reason, Name, Avp, {Avps, Acc}) -> {Rec, Failed} = Acc, {[Avp|Avps], {Rec, [rc(Reason, Avp) | Failed]}}. +%% relax/2 + %% Set false in the process dictionary as soon as we see a Grouped AVP %% that doesn't set the M-bit, so that is_strict() can say whether or %% not to ignore the M-bit on an encapsulated AVP. @@ -357,22 +380,23 @@ relax(_, _) -> is_strict() -> false /= getr(?STRICT_KEY). +%% relax/1 +%% %% Set true in the process dictionary as soon as we see Failed-AVP. %% Matching on 'Failed-AVP' assumes that this is the RFC AVP. %% Strictly, this doesn't need to be the case. + relax('Failed-AVP') -> - case getr(?FAILED_KEY) of - undefined -> - putr(?FAILED_KEY, true); - true = Yes -> - Yes - end; + is_failed() orelse putr(?FAILED_KEY, true); + relax(_) -> is_failed(). is_failed() -> true == getr(?FAILED_KEY). +%% reset/2 + reset(Key, undefined) -> eraser(Key); reset(_, _) -> diff --git a/lib/diameter/src/base/diameter_codec.erl b/lib/diameter/src/base/diameter_codec.erl index 06a4f5de64..a2b04bfd63 100644 --- a/lib/diameter/src/base/diameter_codec.erl +++ b/lib/diameter/src/base/diameter_codec.erl @@ -237,15 +237,35 @@ rec2msg(Mod, Rec) -> %% Unsuccessfully decoded AVPs will be placed in #diameter_packet.errors. --spec decode(module(), #diameter_packet{} | binary()) +-spec decode(module() | {module(), module()}, #diameter_packet{} | binary()) -> #diameter_packet{}. +%% An Answer setting the E-bit. The application dictionary is needed +%% for the best-effort decode of Failed-AVP, and the best way to make +%% this available to the AVP decode in diameter_gen.hrl, without +%% having to rewrite the entire codec generation, is to place it in +%% the process dictionary. It's the code in diameter_gen.hrl (that's +%% included by every generated codec module) that looks for the entry. +%% Not ideal, but it solves the problem relatively simply. +decode({Mod, Mod}, Pkt) -> + decode(Mod, Pkt); +decode({Mod, AppMod}, Pkt) -> + Key = {?MODULE, dictionary}, + put(Key, AppMod), + try + decode(Mod, Pkt) + after + erase(Key) + end; + +%% Or not: a request, or an answer not setting the E-bit. decode(Mod, Pkt) -> decode(Mod:id(), Mod, Pkt). -%% If we're a relay application then just extract the avp's without -%% any decoding of their data since we don't know the application in -%% question. +%% decode/3 + +%% Relay application: just extract the avp's without any decoding of +%% their data since we don't know the application in question. decode(?APP_ID_RELAY, _, #diameter_packet{} = Pkt) -> case collect_avps(Pkt) of {E, As} -> @@ -274,6 +294,8 @@ decode(Id, Mod, Bin) when is_binary(Bin) -> decode(Id, Mod, #diameter_packet{header = decode_header(Bin), bin = Bin}). +%% decode_avps/4 + decode_avps(MsgName, Mod, Pkt, {E, Avps}) -> ?LOG(invalid_avp_length, Pkt#diameter_packet.header), #diameter_packet{errors = Failed} diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl index c4875c5975..280d09d7e8 100644 --- a/lib/diameter/src/base/diameter_traffic.erl +++ b/lib/diameter/src/base/diameter_traffic.erl @@ -476,7 +476,7 @@ send_A({Caps, Pkt}, TPid, Dict0, _RecvData) -> %% unsupported application #diameter_packet{errors = [RC|_]} = Pkt, send_A(answer_message(RC, Caps, Dict0, Pkt), TPid, - Dict0, + {Dict0, Dict0}, Pkt, [], []); @@ -1457,7 +1457,7 @@ handle_answer(SvcName, = App, {answer, Req, Dict0, Pkt}) -> Dict = dict(AppDict, Dict0, Pkt), - handle_A(errors(Id, diameter_codec:decode(Dict, Pkt)), + handle_A(errors(Id, diameter_codec:decode({Dict, AppDict}, Pkt)), SvcName, Dict, Dict0, -- cgit v1.2.3 From 1030c02a7f44f307de003b3c36c61f00080a62da Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Mon, 8 Sep 2014 15:04:20 +0200 Subject: Update appup for OTP-12069 --- lib/diameter/src/diameter.appup.src | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/diameter/src/diameter.appup.src b/lib/diameter/src/diameter.appup.src index 3b6e259f5a..dd9f21c866 100644 --- a/lib/diameter/src/diameter.appup.src +++ b/lib/diameter/src/diameter.appup.src @@ -46,7 +46,8 @@ {load_module, diameter_gen_base_accounting}, {load_module, diameter_gen_relay}, {load_module, diameter_codec}, - {load_module, diameter_sctp}]} + {load_module, diameter_sctp}]}, + {"1.7", [{load_module, diameter_service}]} %% 17.1 ], [ {"0.9", [{restart_application, diameter}]}, @@ -75,6 +76,7 @@ {load_module, diameter_peer_fsm}, {load_module, diameter_watchdog}, {load_module, diameter_traffic}, - {load_module, diameter_lib}]} + {load_module, diameter_lib}]}, + {"1.7", [{load_module, diameter_service}]} ] }. -- cgit v1.2.3 From 4b6061836a0d5634f0b1b0a28d16930bd7013c66 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Mon, 8 Sep 2014 15:07:28 +0200 Subject: Update appup for OTP-12080 --- lib/diameter/src/diameter.appup.src | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/diameter/src/diameter.appup.src b/lib/diameter/src/diameter.appup.src index dd9f21c866..a75b36b5ce 100644 --- a/lib/diameter/src/diameter.appup.src +++ b/lib/diameter/src/diameter.appup.src @@ -47,7 +47,9 @@ {load_module, diameter_gen_relay}, {load_module, diameter_codec}, {load_module, diameter_sctp}]}, - {"1.7", [{load_module, diameter_service}]} %% 17.1 + {"1.7", [{load_module, diameter_service}, %% 17.1 + {load_module, diameter_traffic}, + {load_module, diameter_peer_fsm}]} ], [ {"0.9", [{restart_application, diameter}]}, @@ -77,6 +79,8 @@ {load_module, diameter_watchdog}, {load_module, diameter_traffic}, {load_module, diameter_lib}]}, - {"1.7", [{load_module, diameter_service}]} + {"1.7", [{load_module, diameter_peer_fsm}, + {load_module, diameter_traffic}, + {load_module, diameter_service}]} ] }. -- cgit v1.2.3 From 08fd726f1490fbbd4dc31d40a9e73a00e10dbf96 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Mon, 8 Sep 2014 15:13:16 +0200 Subject: Update appup for OTP-12094 diameter_codec must be loaded before diameter_traffic. --- lib/diameter/src/diameter.appup.src | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'lib') diff --git a/lib/diameter/src/diameter.appup.src b/lib/diameter/src/diameter.appup.src index a75b36b5ce..40580e3ce6 100644 --- a/lib/diameter/src/diameter.appup.src +++ b/lib/diameter/src/diameter.appup.src @@ -48,6 +48,12 @@ {load_module, diameter_codec}, {load_module, diameter_sctp}]}, {"1.7", [{load_module, diameter_service}, %% 17.1 + {load_module, diameter_codec}, + {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_traffic}, {load_module, diameter_peer_fsm}]} ], @@ -81,6 +87,12 @@ {load_module, diameter_lib}]}, {"1.7", [{load_module, diameter_peer_fsm}, {load_module, diameter_traffic}, + {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_codec}, {load_module, diameter_service}]} ] }. -- cgit v1.2.3 From 156b9c67d9d0fc7e4b21e5b7dc4b6f36b05775cb Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Mon, 8 Sep 2014 19:36:18 +0200 Subject: vsn -> 1.7.1 --- lib/diameter/vsn.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/diameter/vsn.mk b/lib/diameter/vsn.mk index 560c2aed50..4e54e4eafc 100644 --- a/lib/diameter/vsn.mk +++ b/lib/diameter/vsn.mk @@ -18,5 +18,5 @@ # %CopyrightEnd% APPLICATION = diameter -DIAMETER_VSN = 1.7 +DIAMETER_VSN = 1.7.1 APP_VSN = $(APPLICATION)-$(DIAMETER_VSN)$(PRE_VSN) -- cgit v1.2.3 From f1c76cec3d025df75bf76c0c5f596a5b00e24e41 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Mon, 8 Sep 2014 20:15:36 +0200 Subject: Don't leave extra bit in decoded AVP data The bit is added in diameter_codec to induce a decode error in the case of 5014 errors, but was not removed before returning the decoded result. Code examining the binary data in a diameter_avp record would then see the extra bit. --- lib/diameter/include/diameter_gen.hrl | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/diameter/include/diameter_gen.hrl b/lib/diameter/include/diameter_gen.hrl index 233eb8dd15..bc25f7d472 100644 --- a/lib/diameter/include/diameter_gen.hrl +++ b/lib/diameter/include/diameter_gen.hrl @@ -320,12 +320,27 @@ d(Name, Avp, Acc) -> {[H | Avps], pack_avp(Name, A, T)} catch error: Reason -> - d(undefined == Failed orelse is_failed(), Reason, Name, Avp, Acc) + d(undefined == Failed orelse is_failed(), + Reason, + Name, + trim(Avp), + Acc) after reset(?STRICT_KEY, Strict), reset(?FAILED_KEY, Failed) end. +%% trim/1 +%% +%% Remove any extra bit that was added in diameter_codec to induce a +%% 5014 error. + +trim(#diameter_avp{data = <<0:1, Bin/binary>>} = Avp) -> + Avp#diameter_avp{data = Bin}; + +trim(Avp) -> + Avp. + %% dict/1 %% %% Retrieve the dictionary for the best-effort decode of Failed-AVP, -- cgit v1.2.3