| Age | Commit message (Collapse) | Author |
|---|
|
* 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
|
|
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
|
|
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.
|
|
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.
|
|
|
|
* 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.
|
|
- OTP-12741: disfunctional counters
- OTP-12744: diameter_sctp race
No load order requirements.
|
|
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.
|
|
Conflicts:
OTP_VERSION
erts/vsn.mk
lib/test_server/src/erl2html2.erl
|
|
* anders/diameter/17.5.3/OTP-12702:
Fix broken pre-17.4 appup
Update appup for 17.5.3
vsn -> 1.9.1
|
|
* 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.
|
|
Upgrade instructions have been added for each 17.X release without
adjusting the instructions for preceeding releases: the instructions
have only been sufficient to upgrading one release at a time: 17.0 to
17.1, 17.1 to 17.2, etc.
Conficting load order requirements make smooth upgrade from an
arbitrarily old release impossible. In this case, 17.3 looks to be as
far back as we can go, so require restart from 17.[0-2] or older.
Update the app suite to deal with binary regexps in appup, and to match
version numbers harder.
|
|
Required load order by ticket.
- OTP-12642, extra bit in diameter_avp.data
- OTP-12654, Result-Code/Experimental-Result confusion
- OTP-12701, counting error with unknown Application Id
none
|
|
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.
|
|
|
|
Required load order by ticket.
- OTP-11492, answer messages discarded
- OTP-12415, retransmission failure
- OTP-12475, grouped AVP decode
- OTP-12543, no requests after DPR
none
- OTP-12412, shutdown issues
diameter_lib
diameter_service
- OTP-12428, transport_opt() pool_size
diameter_lib
diameter_service
diameter, diameter_config
diameter_{tcp,sctp}
diameter, diameter_config
- OTP-12439, new time api in Erlang/OTP 18
diameter_lib
diameter_{config,peer,reg,service,session,stats,sync,watchdog,sctp}
- OTP-11952, service_opt() decode_string
- OTP-12589, DiameterURI encode/decode
diameter_{capx,codec,peer}
diameter_types
diameter_traffic
diameter_{service,peer_fsm}
diameter_watchdog
diameter, diameter_config
- OTP-12542, DPR with diameter:call/4
diameter_{peer_fsm,watchdog}
diameter, diameter_config
- OTP-12609, transport_opt() dpr_timeout
diameter_peer_fsm
diameter, diameter_config
|
|
|
|
* 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.
|
|
* anders/diameter/string_decode/OTP-11952:
Let examples override default service options
Set {restrict_connections, false} in example server
Set {string_decode, false} in examples
Test {string_decode, false} in traffic suite
Add service_opt() string_decode
Strip potentially large terms when sending outgoing Diameter messages
Improve language consistency in diameter(1)
|
|
* anders/diameter/route_record/OTP-12551:
Fix ordering of AVPs in relayed messages
|
|
To control whether stringish Diameter types are decoded to string or
left as binary. The motivation is the same as in the parent commit: to
avoid large strings being copied when incoming Diameter messages are
passed between processes; or *if* in the case of messages destined for
handle_request and handle_answer callbacks, since these are decoded in
the dedicated processes that the callbacks take place in. It would be
possible to do something about other messages without requiring an
option, but disabling the decode is the most effective.
The value is a boolean(), true being the default for backwards
compatibility. Setting false causes both diameter_caps records and
decoded messages to contain binary() in relevant places that previously
had string(): diameter_app(3) callbacks need to be prepared for the
change.
The Diameter types affected are OctetString and the derived types that
can contain arbitrarily large values: OctetString, UTF8String,
DiameterIdentity, DiameterURI, IPFilterRule, and QoSFilterRule. Time and
Address are unaffected.
The DiameterURI decode has been redone using re(3), which both
simplifies and does away with a vulnerability resulting from the
conversion of arbitrary strings to atom.
The solution continues the use and abuse of the process dictionary for
encode/decode purposes, last seen in commit 0f9cdba.
|
