| Age | Commit message (Collapse) | Author |
|---|
|
Since there's a race between an answer being sent and the connection
being closed upon the reception of DPA that's likely to be lost, and
because of the questionability of sending messages after DPR, as
discussed in the parent commit. An exception is made for DPR so that
simultaneous DPR in both directions doesn't result in it being discarded
on both ends.
|
|
RFC 6733 isn't terribly clear about what should happen to incoming or
outgoing messages once DPR is sent and the Peer State Machine
transitions into state Closing. There's no event for this in section
5.6, Peer State Machine, and no clarification in section 5.4,
Disconnecting Peer Connections. There is a little bit of discussion in
2.1.1, SCTP Guidelines, in relation to unordered message delivery, but
the tone there is that messages might be received after DPR because of
unordered delivery, not because they were actually sent after DPR.
Discarding outgoing answers may do more harm than good, but requests are
more likely to be unexpected, as has been seen to be the case with DWR
following DPR. DPR indicates a desire to close the connection: discard
any subsequent outgoing requests.
|
|
* anders/diameter/filters/OTP-12308:
Order peers in pick_peer callbacks
|
|
* anders/diameter/connect_timer/OTP-12281:
Tweak reason in closed event
Fix ignored connect timer
Check {connect,watchdog}_timer distinction in event testcases
Rename reconnect_timer to connect_timer in examples and suites
|
|
* anders/diameter/3xxx/OTP-12233:
Fix handling of 3xxx Result-Code without E-bit
|
|
The order of peers presented to a diameter_app(3) pick_peer callback has
previously not been documented, but there are use cases that are
simplified by an ordering. For example, consider preferring a direct
connection to a specified Destination-Host/Realm to any host in the
realm. The implementation previously treated this as a special case by
placing matching hosts at the head of the peers list, but the
documentation made no guarantees. Now present peers in match-order, so
that the desired sorting is the result of the following filter.
{any, [{all, [host, realm]}, realm]}
The implementation is not backwards compatible in the sense that a realm
filter alone is no longer equivalent in this case. However, as stated,
the documentation never made any guarantees regarding the sorting.
|
|
From {error, Reason} to {no_connection, Reason} when a connection can't
be established. The exit reason of a diameter_peer_fsm process is turned
into a message from the corresponding diameter_watchdog process to the
relevant diameter_service process, the latter sending a 'closed' event
including the reason to any subscribers. Reason = [] when none of the
configured transport modules succeeds in establishing a connection,
which admittedly isn't terribly descriptive. (The lists is of error
reasons from transport start functions, which is empty as long as
transport processes start successfully.)
Note that this form of the closed event is undocumented, aside from the
documentation saying that one should expect undocumented events. The
explicitly documented forms are currently specific to CER/CEA failures.
|
|
There are two timers governing the establishment of peer connections:
connect_timer and watchdog_timer. The former is the RFC 6733 Tc timer
and is used by diameter_service to establish an initial connection. The
latter is RFC 3539 TwInit and is used by diameter_watchdog for
connection reestablishment after the watchdog leaves state INITIAL. A
connecting transport ignored the connect timer since the watchdog
process never died, regardless of the watchdog state, causing the
watchdog timer to handle reconnection.
This seems to have been broken for some time.
|
|
Commit 00584303 broke the population of the errors field of the
diameter_packet record when an incoming request with an
E-bit/Result-Code mismatch was decoded. Instead of the intended
{5004, #diameter_avp{value = integer()}},
the value was a 4-tuple containing the integer Result-Code.
|
|
An outgoing request whose pick_peer callback selected a transport on
another node resulted in an orphaned diameter_request entry on that
node.
|
|
* anders/diameter/Failed-AVP/OTP-12094:
Fix best effort decode of Failed-AVP
Fix decode of Failed-AVP in RFC 3588 answer-message
|
|
* anders/diameter/counters/OTP-12080:
Fix counters for answer-message
Count relayed messages on {relay, Rbit}
Count request retransmissions
Fix counting of outgoing requests
|
|
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.
|
|
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.
|
|
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.
|
|
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.
|
|
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.
|
|
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.
|
|
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.
|
|
To show process_info of interest. This is not yet documented since it
may well change.
|
|
* anders/diameter/hardening/OTP-11721:
Change answer_errors default from report to discard
|
|
In the same vein as commit 00584303, to avoid logging traffic-related
happenings.
Not that the value in diameter.hrl is just documentation: the value is
set explicitly when diameter:start_service/2 creates diameter_app
records.
|
|
* anders/diameter/Failed-AVP/OTP-11936:
Do best-effort decode of Failed-AVP
Add a testcase that expects a decoded value in Failed-AVP
|
|
* anders/diameter/5014/OTP-11946:
Fix handling of AVP length errors (5014) in unknown AVPs
Add testcases that send unknown AVPs with a bad AVP Length
|
|
* anders/diameter/hardening/OTP-11721:
Simplify example server
Make example server answer unsupported requests with 3001
Make example code quiet
Don't count messages on arbitrary keys
Replace traffic-related log reports with no-op function calls
|
|
Commit 4ce2d3a6 (diameter-1.4.2, OTP-11007) disabled the decode of
values in Failed-AVP components since any error caused the decode of
Failed-AVP itself to fail. This is less than useful since (1) we should
be able to decode it given that we've sent it (modulo mangling on the
way to the peer and back), and (2) it's not unheard of to examine
Failed-AVP to see what the peer objected to.
This commits adds a best-effort decode: decode if possible, otherwise
not, using the same abuse of the process dictionary as commit bbdb027c.
|
|
Commit 4ce2d3a6 added the insertion of a single bit into binary AVP data
to induce an encode error in the case of a header length that pointed
past the available bytes: a 5014 = DIAMETER_INVALID_AVP_LENGTH error.
Commit 838856b fixed this for stringish Diameter types, but both commits
neglected the case in which the offending AVP isn't known to the
dictionary in question. Unless the AVP was regarded as erroneous for
other reasons (eg. an M-bit resulting in 5001) it would be happily be
packed into an 'AVP' field. If it was regarded as an error, the record
could be passed back to diameter_codec:pack_avp/1, and if the record
contained header data then there was no clause to deal with the
unpleasantry.
Deal with it by having the dictionary module strip the extra bit and
flag the AVP as 5014, and by having diameter_codec handle any extra bit
coming from an dictionary compiled against an old diameter_gen. An old
dictionary won't detect 5014 however, so dictionaries should be
recompiled.
Change most of the guards in diameter_codec from is_bitstring/1 to
is_binary/1. What's being passed to the decode functions are binaries
received other the network. The only case in which a non-binary
bitstring is when we've placed an extra bit there ourselves. (Modulo
someone doing something they shouldn't.)
|
|
That is, don't use a key constructed from an incoming Diameter header
unless the message is known to the dictionary in question. Otherwise
there are 2^32 application ids, 2^24 command codes, and 2 R-bits for an
ill-willed peer to choose from, each resulting in new keys in the
counter table (diameter_stats).
The usual {ApplicationId, CommandCode, Rbit} in a key is replaced by the
atom 'unknown' if the message in question is unknown to the decoding
dictionary.
Counters for messages sent and received by a relay are (still) not
implemented.
|
|
The former were a little over-enthusiastic and could cause a node to be
logged to death if a peer Diameter node was sufficiently ill-willed.
The function calls are to diameter_lib:log/4, the arguments of which
identify the happening in question, and which does nothing but provide a
function to trace on. Many existing log calls have been shrunk.
The only remaining traffic-related report (hopefully) is that resulting
from {answer_errors, report} config, and this has been slimmed.
|
|
* anders/diameter/dpr/OTP-11938:
Ensure watchdog dies with transport if DPA was sent
|
|
* anders/diameter/rc_counters/OTP-11937:
Count encode errors in outgoing messages
Count decode errors in incoming requests
Count decode errors independently of result codes
|
|
* anders/diameter/rc_counters/OTP-11891:
Count result codes in CEA/DWA/DPA
|
|
* anders/diameter/watchdog_leak/OTP-11934:
Simplify sending of 'close' to watchdog
Fix watchdog table leak
|
|
* anders/diameter/request_leak/OTP-11893:
Fix leaking request table
Add check that request table is empty to failover suite
Comment fix
|
|
A DPR/DPA exchange should always cause the watchdog process in question
to die with the transport, so that a subsequent connection with the same
peer doesn't result in a 3 x DWR/DWA exchange. Commit 5903d6db saw to
this for the sending of DPR but neglected the corresponding problem for
DPA.
In the case of sending DPR (the aforementioned commit), note that
there's no distinction between receiving DPA as expected and not: the
watchdog dies with the transport regardless.
diameter_watchdog must be loaded first at upgrade.
|
|
Only decode errors were counted previously. Keys are of the form
{Id, send, error}, where Id is:
{ApplicationId, CommandCode, Rbit} | unknown
The latter will be the case if not even a #diameter_header{} can be
constructed.
|
|
Errors were only counted in incoming answers. Counters are keyed on
tuples of the same form:
{{ApplicationId, CommandCode, Rbit}, recv, error}
|
|
Since the former doesn't exclude the latter.
Counter values are returned by diameter:service_info/2. They can
currently only be retrieved for a service, not for individual transports
or peer connections.
|
|
Corresponding counters for other answer messages have been counted
previously, but those for CEA, DWA, and DPA have been missing since
diameter itself sends these messages and the implementation is as bit
more separate than it might be. The counters are keyed on values of the
following form.
{{ApplicationId, CommandCode, 0 = Rbit}, send|recv, {'Result-Code', RC}}
|
|
There's no need to send the message immediately if there's no transport
configuration since that in itself means the service process will tell
the watchdogs to die.
|
|
Commit ef5fddcb (diameter-1.4.1, R16B) caused the leak in the case of an
accepting watchdog with restrict_connections = false. It (correctly)
ensured the state remained at INITIAL but a subsequent 'close' message
to terminate the process was ignored since the state was not DOWN. In
fact, no 'close' was sent since there was no state transition or
previous connection: the former triggers the message from
diameter_service, the latter from diameter_watchdog. The message is now
sent to self() from the watchdog itself.
Send 'close' in the same way when multiple connections to the same peer
are allowed, to avoid waiting for a watchdog timer expiry for the
process to terminate in this case.
|
|
A new connection writes the pid to the table diameter_request. The
normal handling is that loss of a connection leads to a watchdog state
change in the service process, which removes the entry, but this usually
won't happen in the case of diameter:stop_service/1 since the service
process is terminated without waiting for watchdog transitions.
The request table should really be service-specific, so that the table
is deleted when the service is stopped, which requires passing the table
identifier into request processes and handling that the table may not
exist. Just clear out the service-specific entries at service process
termination for now.
|
|
* anders/diameter/pick_peer/OTP-11789:
Fix pick_peer case clause failure
|
|
Possibly overkill for two modules but it mirrors their different
treatment by the makefile.
|
|
It was intended to replace diameter_lib:log/4 at some point but that was
a bad idea since diameter_dbg isn't included in the app file.
|
|
The documented return value changed in commit c37a9761.
|
|
In the case of {call_mutates_state, true} configuration on the service
in question, any peer selection that failed to select a peer resulted in
a case clause failure in diameter_service:pick_peer/5, when the call to
the service process returned false. This was noticed in the case of a
peer failover in which an alternate peer wasn't available.
The explicit matching is intentional, to match exactly what's expected.
|
|
No longer needed to update code in runtime since the emulator is
restarted at a major release.
|
|
* anders/diameter/timer_confusion/OTP-11168:
Rename reconnect_timer -> connect_timer
|
|
The former was misleading since the timer only applies to initial
connection attempts, reconnection attempts being governed by
watchdog_timer. The name is a historic remnant from a (dark, pre-OTP)
time in which RFC 3539 was followed less slavishly than it is now, and
the timer actually did apply to reconnection attempts.
Note that connect_timer corresponds to RFC 6733 Tc, while watchdog_timer
corresponds to RFC 3539 TwInit. The latter RFC makes clear that TwInit
should apply to reconnection attempts. It's less clear if only RFC 6733
is read.
Note also that reconnect_timer is still accepted for backwards
compatibility. It would be possible to add an option to make
reconnect_timer behave strictly as the name suggests (ie. ignore RFC 3539
and interpret RFC 6733 at face value; something that has some value for
testing at least) but no such option is implemented in this commit.
|
