| Age | Commit message (Collapse) | Author |
|---|
|
The function already operates on arbitrary terms.
|
|
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.
|
|
This has had a hugely negative impact on performance when decoding
messages containing many AVP: each decode of an AVP having variable
arity computed the length of the list of previously decoded AVPs when
checking that the allowed arity was not exceeded, even if the allowed
arity was infinite, making for O(n^2) cost. Here are some execution
times, for diameter_codec:decode/2 on a representative message with n
integer AVPs in the Common application (on the host at hand):
Before After
------- ---------
n = 1K 5 ms 2 ms
n = 10K 500 ms 25 ms
n = 100K 75 sec 225 ms
n = 1M 2.6 sec
Note the nearly linear increase following the change.
Remove the dire documentation warning for incoming_maxlen as a
consequence. It can still be useful to set, but not doing so won't have
the same consequences as previously.
|
|
Since the list can potentially be long.
|
|
The decode of an incoming Diameter message uses the record
representation to determine whether or not an AVP has been received with
the expected arity, the number of AVPs in each field following decode
being compared with the arity specified in the message grammar. The
problem with this is that decode failure isn't reflected in the record
representation, so that an AVP can be appended to the errors field of a
diameter_packet record despite an entry for the same AVP already
existing. This isn't a fault as much as a misleading error indication,
but now only append AVPs that aren't already represented.
|
|
The warning report was removed in commit 00584303.
|
|
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.
|
|
The suite pretends to be gen_tcp-ish in configuring itself to
diameter_tcp. The function close/1 can be called as a result.
|
|
|
|
OTP-12845
* bruce/change-license:
fix errors caused by changed line numbers
Change license text to APLv2
|
|
* anders/diameter/18/OTP-12588:
vsn -> 1.10
Remove dead upgrade-related code
Update appup for 18
Fix release note typo
Fix comment typo
|
|
* anders/diameter/sctp/OTP-12768:
Fix connection timeouts in test transports
Fix start order of alternate transports
Log discarded answers
Ensure accepting processes are first in, first out
Remove upgrade-related code
Be less parallel in traffic suite
Increase send/receive buffers for testsuite SCTP listeners
Decrease unnecessarily long testsuite timetraps
Simplify accepting transport start
Simplify peeloff signaling
Simplify socket close at terminate
Don't monitor listener after peeloff
Don't receive initial messages out of order
Remove assumption that SCTP association ids will be unique
|
|
* anders/diameter/grouped_errors/OTP-12721:
Fix decode of Grouped AVPs containing errors
Simplify logic
Simplify logic
|
|
Without a timeout, TCP/SCTP connect can take some time to fail, which
resulted in failures in the pool suite after the parent commit fixed the
previously faulty sctp-first-then-tcp connect.
|
|
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.
OTP-12851
|
|
|
|
Not needed with the parent commit's restart_application.
|
|
To diameter_lib:log/4, which was last motivated in commit 39acfdb0.
|
|
A listener process in diameter_sctp starts accepting transport processes
as required, either as associations are established or as diameter asks
for a processes to be started. Since this can happen in any order, the
listener maintains two queues: one for processes that diameter has
requested and which are waiting to be given an association, another for
processes that have been started to become owners of an association but
are waiting for diameter to request them. Only one queue at a time is
non-empty. The first queue's length is bounded by the number of
accepting processes configured as pool_size. Entries in the second queue
are short-lived since diameter starts a replacement transport process
whenever an existing one dies or communicates that it has an
association.
The two queues were previously implemented in an ets ordered_set, whose
keys were the pid() of transport processes. Removing an element from the
queue was then done with ets:first/1. The problem with this it's not
really a queue: there's no guarantee that pid-ordering is the same as
the order in which processes are started. If it isn't then it's possible
that an established association never be given to diameter as a
transport process if there's always a newer association whose pid sorts
first. This isn't a problem in practice since it would require new
associations to be established faster than diameter starts transport
processes, but redo the implementation as a queue, with strict FIFO
semantics.
|
|
The changes in some of the previous commits assume application restart.
|
|
At the current count, there are 128 groups run in parallel, each of
which runs 52 testcases in parallel. That makes for 128*52 = 6656
testcases, which is probably also a factor in the sporadic failures
addressed by the parent commit. Don't run the 128 groups in parallel.
|
|
The defaults result in sporadic timeouts in the traffic suite after
testing over SCTP was added in commit fadf753b. The behaviour looks to
be specific to SLES 11, and is presumably the same resends/congestion
that lead to the buffers being increased in the gen_sctp suite in commit
12febf13 (and commented in commit e931991f). The behaviour hasn't been
seen on SLES 10.
|
|
|
|
Don't pass an association id that's no longer used.
|
|
In particular, don't give the accepting transport process the listening
socket. It was used to match the initial sctp message received in a
peeloff message, but replace the socket in the forwarded message
instead.
|
|
The existing code was a remnant of the pre-peeloff implementation.
There's no need to close anything but the whole socket.
|
|
Listener death should have no effect on a peeled off association.
|
|
Forwarding an sctp message from the listener process at the same time
that the controlling process is changed means there's no guarantee that
the message order will be preserved. Selectively receive the peeloff
message before entering the gen_server loop to ensure the order is
preserved.
|
|
This is not the case under Solaris for one: successive
associations can receive the same association id as a result of peeloff,
the id only being unique for the controlling port, not for the listening
port as is the case under Linux for example. This made for many failures
in the diameter test suites, the traffic suite in particular.
Peeloff in diameter_sctp was introduced in 9a671bf0, before which the
assumption was fine since it was the listening process that owned all
associations. (Which obviously had other drawbacks.) Other remnants of
the pre-peeloff implementation have also been removed: that the listener
process might receive a message on a socket after peeloff for one.
Peeloff in gen_sctp became available in commit 067cfe79, after the
original implementation of diameter_sctp.
This is trace on the unpatched code showing id reuse under Solaris:
+ {trace_ts,<0.103.0>,call,
{diameter_sctp,handle_info,
[{sctp,#Port<0.1625>,
{127,0,0,1},
35904,
{[],{sctp_assoc_change,comm_up,0,32,32,1}}},
{listener,#Ref<0.0.1.948>,#Port<0.1625>,4,
57384,
{-4,61481},
#Ref<0.0.8.12>,
[]}]},
{1432,458752,612168}}
+ {trace_ts,<0.103.0>,call,
{diameter_sctp,handle_info,
[{sctp,#Port<0.1625>,
{127,0,0,1},
35905,
{[],{sctp_assoc_change,comm_up,0,32,32,1}}},
{listener,#Ref<0.0.1.948>,#Port<0.1625>,4,
57384,
{-3,61481},
#Ref<0.0.8.12>,
[]}]},
{1432,458752,613042}}
The result was this, when the second association was incorrectly
forwarded to the first association's controlling process:
** {function_clause,
[{diameter_sctp,transition,
[{peeloff,#Port<0.1635>,
{sctp,#Port<0.1625>,
{127,0,0,1},
35892,
{[],{sctp_assoc_change,comm_up,0,32,32,1}}},
[]},
{transport,<0.107.0>,accept,#Port<0.1634>,1,undefined,{32,32},0}],
[{file,"transport/diameter_sctp.erl"},{line,561}]},
{diameter_sctp,t,2,[{file,"transport/diameter_sctp.erl"},{line,549}]},
{diameter_sctp,handle_info,2,
[{file,"transport/diameter_sctp.erl"},{line,397}]},
{gen_server,try_dispatch,4,[{file,"gen_server.erl"},{line,614}]},
{gen_server,handle_msg,5,[{file,"gen_server.erl"},{line,680}]},
{proc_lib,init_p_do_apply,3,[{file,"proc_lib.erl"},{line,238}]}]}
|
|
|
|
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.
|
|
Testing is_failed() is unnecessary since put/2 a second time will
return a previously put 'true'.
|
|
Failed == undefined implies is_failed() == true. This was true even when
the code was written, in commit c2c00fdd.
|
|
|
|
|
|
|
|
|
|
|
|
* anders/diameter/test/OTP-12767:
Replace config suite call to erlang:now/0
Fix incorrect suite usage of OTP 18 monotonic time
Make tls suite crash more verbosely
|
|
* anders/diameter/17.5.5/OTP-12757:
vsn -> 1.9.2
Update appup for 17.5.5
Fix mangled release note
|
|
* anders/diameter/sctp/OTP-12744:
Fix diameter_sctp listener race
Tweak transport suite failures
Run traffic suite over SCTP
|
|
Commit 4b691d8d made it possible for accepting transport processes to be
started concurrently, and commit 77c1b162 adapted diameter_sctp to this,
but missed that the publication of the listener process in diameter_reg
has to precede the return of its start function. As a result, concurrent
starts could result in multiple listener processes.
|
|
Make anything but a comm_up sctp_assoc_change crash. Make timeouts more
reasonable.
|
|
Previously it was only run over TCP.
Configure a pool of accepting processes since simultaneous connections
are otherwise prone to rejection, as discussed in commit 4b691d8d.
Tweak timeouts to more reasonable values.
|
|
To remove a compilation warning with OTP 18.
|
