| Age | Commit message (Collapse) | Author |
|---|
|
Not needed as of commit 6c9cbd96.
|
|
The export of diameter_traffic:failover/1 happened with the creation of
the module in commit e49e7acc, but was never needed since the calling
code was also moved into diameter_traffic.
|
|
Each service process maintains a dictionary of peers, mapping an
application alias to a {pid(), #diameter_caps{}} list of connected
peers. These lists are potentially large, peers were appended to the end
of the list for no particular reason, and these long lists were
constructed/deconstructed when filtering them for pick_peer callbacks.
Many simultaneous outgoing request could then slow the VM to a crawl,
with many scheduled processes mired in list manipulation.
The pseudo-dicts are now replaced by plain ets tables. The reason for
them was (once upon a time) to have an interface interchangeable with a
plain dict for debugging purposes, but strict swapablity hasn't been the
case for some time now, and in practice a swap has never taken place.
Additional tables mapping Origin-Host/Realm have also been introduced,
to minimize the size of the peers lists when peers are filtered on
host/realm. For example, a filter like
{any, [{all, [realm, host]}, realm]}
is probably a very common case: preferring a Destination-Realm/Host
match before falling back on Destination-Realm alone. This is now more
efficiently (but not equivalently) expressed as
{first, [{all, [realm, host]}, realm]}
to stop the search when the best match is made, and extracts peers from
host/realm tables instead of searching through the list of all peers
supporting the application in question. The code to try and start with a
lookup isn't exhaustive, and the 'any' filter is still as inefficient as
previously.
|
|
See commit 862af31d.
|
|
* anders/diameter/17.5.6.7/OTP-13211:
vsn -> 1.9.2.2
Update/fix appup for 17.5.6.7
Be resilient to diameter_service state upgrades
|
|
* anders/diameter/request_leak/OTP-13137:
Fix request table leak at retransmission
Fix request table leak at exit signal
|
|
By not failing in code that looks up state: pick_peer and service_info.
|
|
In the case of retranmission, a prepare_retransmit callback could modify
End-to-End and/or Hop-by-Hop identifiers so that the resulting
diameter_request entry was not removed, since the removal was of entries
with the identifiers of the original request. The chances someone doing
this in practice are probably minimal.
|
|
The storing of request records in the ets table diameter_request was
wrapped in a try/after so that the latter would unconditionally remove
written entries. The problem is that it didn't deal with the process
exiting as a result of an exit signal, since this doesn't raise in an
exception. Since the process in question applies callbacks to user code,
we can potentially be linked to other process and exit as a result.
Trapping exits changes the current behaviour of the process, so spawn a
monitoring process that cleans up upon reception of 'DOWN'.
|
|
Commit 4f365c07 introduced the error on set_watchdog/2, as a consequence
of timeout/1 returning stop, which only happens with accepting
transports with {restrict_connections, false}.
|
|
There are differing opinions on whether or not reception of an arbitrary
AVP setting the M-bit is an error. 1.3.4 of RFC 6733 says this about
how an existing Diameter application may be modified:
o The M-bit allows the sender to indicate to the receiver whether or
not understanding the semantics of an AVP and its content is
mandatory. If the M-bit is set by the sender and the receiver
does not understand the AVP or the values carried within that AVP,
then a failure is generated (see Section 7).
It is the decision of the protocol designer when to develop a new
Diameter application rather than extending Diameter in other ways.
However, a new Diameter application MUST be created when one or more
of the following criteria are met:
M-bit Setting
An AVP with the M-bit in the MUST column of the AVP flag table is
added to an existing Command/Application. An AVP with the M-bit
in the MAY column of the AVP flag table is added to an existing
Command/Application.
The point here is presumably interoperability: that the command grammar
should specify explicitly what mandatory AVPs much be understood, and
that anything more is an error.
On the other hand, 3.2 says thus about command grammars:
avp-name = avp-spec / "AVP"
; The string "AVP" stands for *any* arbitrary AVP
; Name, not otherwise listed in that Command Code
; definition. The inclusion of this string
; is recommended for all CCFs to allow for
; extensibility.
This renders 1.3.4 pointless unless "*any* AVP" is qualified by "not
setting the M-bit", since the sender can effectively violate 1.3.4
without this necessitating an error at the receiver. If clients add
arbitrary AVPs setting the M-bit then request handling becomes more
implementation-dependent.
The current interpretation in diameter 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. The strict_mbit option now allows this
behaviour to be changed, false turning all responsibility for the M-bit
over to the user.
|
|
* anders/diameter/17/time/OTP-12926:
Simplify time manipulation
Remove use of monotonic time in pre-18 code
Remove unnecessary redefinition of erlang:max/2
|
|
* anders/diameter/grouped_errors/OTP-12930:
Fix decode of Grouped AVPs containing errors
Simplify logic
Simplify logic
|
|
* anders/diameter/transport/OTP-12929:
Fix start order of alternate transports
Log discarded answers
|
|
* anders/diameter/lcnt/OTP-12912:
Make ets diameter_stats a set
Remove unnecessary sorting in stats suite
Set ets {write_concurrency, true} on diameter_stats
Don't start watchdog timers unnecessarily
Remove unnecessary erlang:monitor/2 qualification
Add missing watchdog suite clause
|
|
* anders/diameter/caseless/OTP-12902:
Match allowable peer addresses case insensitively
Replace calls to module inet_parse to equivalents in inet
|
|
* anders/diameter/grouped_decode/OTP-12879:
Fix relay encode of decoded diameter_avp lists
|
|
There's no need for it to be ordered, and the ordering has been seen to
have an unexpectedly negative impact on performance in some cases. Order
when retrieving statistics instead, so as not to change the
presentation in diameter:service_info/2.
|
|
By doing away with more wrapping that the parent commit started to
remove.
|
|
This has been seen to be a bottleneck at high load: each undef results
in a loop out to the code server.
Originally implemented as suggested in the erts user's guide, in commits
e6d19a18 and d4386254.
|
|
A transport configured with diameter:add_transport/2 can be passed
multiple transport_module/transport_config tuples in order to specify
alternate configuration, modules being attempted in order until one
succeeds. This is primarily for the connecting case, to allow a
transport to be configured to first attempt connection over SCTP, and
then TCP in case SCTP fails, with configuration like that documented:
{transport_module, diameter_sctp},
{transport_config, [...], 5000},
{transport_module, diameter_tcp},
{transport_config, [...]}
If the options are the same in both cases, another possibility would be
configuration like this, which attaches the same transport_config to
both modules:
{transport_module, diameter_sctp},
{transport_module, diameter_tcp},
{transport_config, [...], 5000},
However, in this case the start order was reversed relative to the
documented order: first tcp, then sctp. This commit restores the
intended order.
|
|
To diameter_lib:log/4, which was last motivated in commit 39acfdb0.
|
|
Commit c74b593a fixed the problem that a decoded deep diameter_avp list
couldn't be encoded, but did so in the wrong way: there's no need to
reencode component AVPs since the Grouped AVP itself already contains
the encoded binary. The blunder caused diameter_codec:pack_avp/1 to fail
if the first element of the AVP list to be encoded was itself a list.
Thanks to Andrzej Trawiński for reporting the problem.
|
|
Both diameter_tcp and diameter_sctp can be configured with one or more
IP addresses from which connections should be accepted (an 'accept'
tuple), specified either as a tuple-valued address or as a regular
expression. In the latter case, peer addresses are mapped to string
using inet:ntoa/1 and the result matched against the regexp. Since
(ipv6) addresses are case insensitive, this should also be the case with
the match, but was not.
|
|
Commits b563c796 (R16B) and 0fad6449 (R16B02) added parse_address/1 and
ntoa/1 to module inet, providing documented alternatives to address/1
and ntoa/1 in the undocumented (save comments in inet(3)) inet_parse.
|
|
Last visited in commit 00584303.
|
|
lcnt:inspect/1 recently showed this:
lock id #tries collisions [%] time [us]
----- --- ------- --------------- ----------
db_tab diameter_stats 932920 92.9326 330332554
|
|
In particular, restart the timer with each incoming Diameter message,
only when the previous timer has expired. Doing so has been seen to
result in high lock contention at load, as in the example below:
(diameter@test)9> lcnt:conflicts([{print, [name, tries, ratio, time]}]).
lock #tries collisions [%] time [us]
----- ------- --------------- ----------
bif_timers 7844528 99.4729 1394434884
db_tab 17240988 1.7947 6286664
timeofday 7358692 5.6729 1399624
proc_link 4814938 2.2736 482985
drv_ev_state 2324012 0.5951 98920
run_queue 21768213 0.2091 63516
pollset 1190174 1.7170 42499
pix_lock 1956 2.5562 39770
make_ref 4697067 0.3669 20211
proc_msgq 9475944 0.0295 5200
timer_wheel 5325966 0.0568 2654
proc_main 10005332 2.8190 1079
pollset_rm_list 59768 1.7752 480
|
|
The function has been auto-exported since R14B.
|
|
RFC 6733 says this of Failed-AVP in 7.5:
In the case where the offending AVP is embedded within a Grouped AVP,
the Failed-AVP MAY contain the grouped AVP, which in turn contains
the single offending AVP. The same method MAY be employed if the
grouped AVP itself is embedded in yet another grouped AVP and so on.
In this case, the Failed-AVP MAY contain the grouped AVP hierarchy up
to the single offending AVP. This enables the recipient to detect
the location of the offending AVP when embedded in a group.
It says this of DIAMETER_INVALID_AVP_LENGTH in 7.1.5:
The request contained an AVP with an invalid length. A Diameter
message indicating this error MUST include the offending AVPs
within a Failed-AVP AVP. In cases where the erroneous AVP length
value exceeds the message length or is less than the minimum AVP
header length, it is sufficient to include the offending AVP
header and a zero filled payload of the minimum required length
for the payloads data type. If the AVP is a Grouped AVP, the
Grouped AVP header with an empty payload would be sufficient to
indicate the offending AVP. In the case where the offending AVP
header cannot be fully decoded when the AVP length is less than
the minimum AVP header length, it is sufficient to include an
offending AVP header that is formulated by padding the incomplete
AVP header with zero up to the minimum AVP header length.
The AVPs placed in the errors field of a diameter_packet record are
intended to be appropriate for inclusion in a Failed-AVP, but neither of
the above paragraphs has been followed in the Grouped case: the entire
faulty AVP (non-faulty components and all) has been included. This made
it impossible to identify the actual faulty AVP in all but simple case.
This commit adapts the decode to the RFC, and implements the suggested
single faulty AVP, nested in as many Grouped containers as required.
The best-effort decode of Failed-AVP in answer messages, initially
implemented in commit 0f9cdbaf, is also applied.
|
|
The message was regarded as unknown if the answer message in question
set the E-bit and the application dictionary was not the common
dictionary.
|
|
That is, outgoing answer messages received in response to a
handle_request callback having returned {relay, Opts}.
|
|
To clarify what it is that's being computed, which isn't entirely
obvious. No functional change, just renaming.
|
|
As the first step in starting to count outgoing, relayed answer
messages.
|
|
An incoming Diameter message is either a request, an answer to an
outstanding request, or an unexpected answer. The latter weren't
counted, but are now counted on keys of this form:
{pid(), {{unknown, 0}, recv, discarded}}
The form of the second element is similar to those of other counters,
like:
{{relay, 0|1}, send|recv, invalid_error_bit}
Compare this to the key used when counting known answers:
{{ApplicationId, CommandCode, 0}, recv}
The application id and command code aren't included so as not to count
on arbitrary keys, a topic last visited in commit 49e8b11c.
|
|
To differentiate between requests and answers, in analogy with relay
counters. This isn't backwards compatible, but these counters aren't yet
documented.
|
|
Commit 49e8b11c broke the counting of relayed message, causing them to
be accumulated as unknown messages.
|
|
Commit a1df50b3 broke result code counters in the case of answer
messages sent as a header/avp lists (unless the avps, untypically, set
the name field), and for answers sent/received in the relay application.
|
|
* anders/diameter/counters/OTP-12701:
Add counters testcase to 3xxx suite
Fix counting error with unknown application id
Add missing doc wording
|
|
* anders/diameter/result_codes/OTP-12654:
Fix broken traffic testcase
Match harder in traffic suite
Don't confuse Result-Code and Experimental-Result
|
|
Decode of an answer message not setting the E-bit, and containing
Experiment-Result but not Result-Code, identified Result-Code as the
erroneous when Erroneous-Result-Code was 3xxx. Here's an example (from
trace) of a the errors field after decode:
[{5004,
{diameter_avp,undefined,undefined,false,false,undefined,'Result-Code',
3001,undefined,undefined}}],
The diameter_avp was just constructed from the AVP name and decoded
result, without regard for which result code AVP contained the value.
Fix by extracting the AVP from the incoming message.
|
|
Statistics could be accumulated on a key like {{23,275,0}, recv} even
though 23 was not the application id of the dictionary in question.
Missed in commits df19c272 and 7816ab2f.
|
|
In the case of a faulty AVP Length (pointing past the end of a message
or not spanning the header), an extra bit is prepended to data bytes in
diameter_avp:collect_avps/1 in order to force a 5014 decode error. The
bit is supposed to be removed as part of the decode in diameter_gen.hrl
but this didn't happen in case of an AVP that unknown to the dictionary
in question.
|
|
As for the port number in the parent commit, a FQDN can't be arbitrarily
long, at most 255 octets. Make decode fail if it's more.
|
|
A port number is a 16-bit integer, but the regexp used to parse it in
commit 1590920 slavishly followed the RFC 6733 grammar in matching an
arbitrary number of digits. Make decode fail if it's anything more than
5, to avoid doing erlang:list_to_integer/1 on arbitrarily large lists.
Also make it fail if the resulting integer is outside of the expected
range.
|
|
To bound the length of incoming messages that will be decoded. A message
longer than the specified number of bytes is discarded. An
incoming_maxlen_exceeded counter is incremented to make note of the
occurrence.
The motivation is to prevent a sufficiently malicious peer from
generating significant load by sending long messages with many AVPs for
diameter to decode. The 24-bit message length header accomodates
(16#FFFFFF - 20) div 12 = 1398099
Unsigned32 AVPs for example, which the current record-valued decode is
too slow with in practice. A bound of 16#FFFF bytes allows for 5461
small AVPs, which is probably more than enough for the majority of
applications, but the default is the full 16#FFFFFF.
|
|
It was possible to configure the option, but doing so caused the service
to fail when starting a watchdog process:
{function_clause,
[{diameter_service,'-spawn_opts/1-lc$^0/1-0-',
[false],
[{file,"base/diameter_service.erl"},{line,846}]},
{diameter_service,start,5,
[{file,"base/diameter_service.erl"},{line,820}]},
{diameter_service,start,3,
[{file,"base/diameter_service.erl"},{line,782}]},
{diameter_service,handle_call,3,
[{file,"base/diameter_service.erl"},{line,385}]},
{gen_server,try_handle_call,4,[{file,"gen_server.erl"},{line,607}]},
{gen_server,handle_msg,5,[{file,"gen_server.erl"},{line,639}]},
{proc_lib,init_p_do_apply,3,[{file,"proc_lib.erl"},{line,237}]}]}
Tests for the option in the config suite were also missing.
Bungled in commit 78b3dc6.
|
|
* anders/diameter/dpr/OTP-12609:
Discard incoming/outgoing requests after incoming DPR
Add transport_opt() dpr_timeout
Be lenient with errors in incoming DPR
|
|
Despite claims of full backwards compatibility, the text of RFC 6733
changes the interpretation of unspecified values in a DiameterURI. In
particular, 3588 says that the default port and transport are 3868 and
sctp respectively, while 6733 says it's either 3868/tcp (aaa) or
5658/tcp (aaas). The 3588 defaults were used regardless, but now use
them only if the common dictionary is diameter_gen_base_rfc3588. The
6733 defaults are used otherwise.
This kind of change in the standard can lead to interop problems, since
a node has to know which RFC its peer is following to know that it will
properly interpret missing URI components. Encode of a URI includes all
components to avoid such confusion.
That said, note that the defaults in the diameter_uri record have *not*
been changed. This avoids breaking code that depends on them, but the
risk is that such code sends inappropriate values. The record defaults
may be changed in a future release, to force values to be explicitly
specified.
|
|
Both RFC 3588 and 6733 disallow the combination. Make its encode fail.
|
