| Age | Commit message (Collapse) | Author |
|---|
|
Decode is only a no-op when the AVP is Grouped, and then only on the
Grouped AVP itself, not its components.
|
|
To support specifications like RFC 7683 DOIC, that only define AVPs, not
applications. AVPs that aren't known to the application dictionary in
question could previously not be decoded. Configuring alternate
dictionaries with the new transport/service option avp_dictionaries
changes this, so that AVPs like DOIC's Grouped OC-OLR can presented in
their fully decoded glory. Encode is also extended, allowing things like
the following to be encoded in an outgoing message:
'AVP' => [{'OC-OLR', #{'OC-Sequence-Number' => 1,
'OC-Report-Type' => 0,
'OC-Reduction-Percentage' => [25]}}]
A diameter_gen_doic_rfc7683 dictionary is installed, but
avp_dictionaries isn't specific to DOIC.
This commit also solves the problem demonstrated a few commits back,
that application AVPs aren't decoded in answers setting the E-bit. Test
coverage will come in a subsequent commit.
|
|
To better reflect what the field is: field 'module' is the dictionary
module that's calling diameter_gen to decode a list of AVP, while field
'app_dictionary' is the dictionary module defining the message being
decoded.
|
|
Which motivates the avp_dictionaries config that will be added in
a subsequent commit.
|
|
Function avp/5 isn't exported from dictionary modules. Not necessarily
intentional, but don't just export it since that requires recompilation
of all dictionary modules, since the function is in diameter_gen.hrl.
Not having to recompile was the main motivation for moving most of the
included code to module diameter_gen in commit 205521d3.
This reveals a weakness in the decode of answers setting the E-bit: any
AVP that isn't defined by the common application won't be decoded; the
diameter_avp records that these are packed into (in the 'AVP' field of a
message record, or equivalent) will have value = undefined. This is
nothing new (same in OTP 19), but the values should be decoded. Fix it
(and the lack of test coverage) in a subsequent commit that will add
avp_dictionaries config.
|
|
Adding a second {Vendor-Id} to the common CER definition results in
this error:
** AVP CER at line 85 already referenced at line 84
That is, the error incorrectly refers to the message name (CER) where the
AVP name (Vendor-Id) is expected.
|
|
Which reads better and makes it easier to distinguish this false from
others.
|
|
Represent the decoded message by its atom-valued name in
diameter_packet.msg, which makes trace much more readable. A
diameter_avp.value is untouched (ie. undefined): the AVP name is already
in the name field.
|
|
Which dialyzer hasn't noticed.
|
|
anders/diameter/decode_format/OTP-14511
* anders/diameter/config_consistency/OTP-14555:
Fix strict_arities blunder
Fix minor error-handling blunder
Let strict_mbit and incoming_maxlen be configured per transport
Let a service configure default transport options
Rename type evaluable -> eval
Document transport_opt() strict_capx
Rename transport_opt() capx_strictness to strict_capx
|
|
Remove value from the merged map, not from the maps being merged.
Bundled in commit 5f3becad.
|
|
Leading to this admonition from dialyzer:
diameter_config.erl:670: The variable No can never match since previous
clauses completely covered the type 'ok'
The throw was caught, but resulted in an error return without the
intended information.
|
|
RFC 6733 says this:
6.2. Diameter Answer Processing
When a request is locally processed, the following procedures MUST be
applied to create the associated answer, in addition to any
additional procedures that MAY be discussed in the Diameter
application defining the command:
...
o Any Proxy-Info AVPs in the request MUST be added to the answer
message, in the same order they were present in the request.
This wasn't done when a handle_request callback returned a Result-Code
in an 'answer-message' or protocol_error tuple, causing diameter itself
to construct the answer message. This form of answer is just a
convenience, since the callback can always return an answer that it
constructs itself.
|
|
The introduction of decode_format in commit 722fa415 (and then 55e65b26)
meant the value was not necessarily the intended tuple.
|
|
It's no longer the AVP name as of the parent commit, but the name of
the field/member the value will be stored in. Typically the AVP name,
but possibly 'AVP'.
|
|
That is, when the arity of an 'AVP' field has an upper bound. This
shouldn't happen in practice, but if an AVP is known but its name not
explicit in the message grammar then its count was confused with that
of AVPs packed into the 'AVP' field.
|
|
Commit 96cd627a changed the way the index field was used, but the
enumeration is used in at least one known application (as a pointer from
elements of diameter_packet.errors to elements of diameter_packet.avps)
and the motivation for the change is questionable: the lookup that was
avoided was unnecessary given that it was already performed in
incrementing a counter. Revert to enumerating as before in the non-relay
case, but not in the relay case since there's no corresponding usecase.
|
|
Decoded CER/CEA messages are passed in events messages that can be
subscribed to using diameter:subscribe/1. A configured decode_format was
not reflected in these, messages always being passed as records.
Clarify that strict_arities only applies to message callbacks.
|
|
Since these can make sense per peer. The remaining service-only options
either belong there or make little sense being configured per transport.
|
|
Only a default spawn_opt has been possible to configure, but there's
no reason why most others should need to be configured per transport.
Those options that still only make sense on a transport are
transport_module/config (because of the semantics of multiple values),
applications/capabilities (since these override service options), and
private (since it's only to allow user-specific options in a backwards
compatible way).
|
|
Export the old type as a synonym for backwards compatability. The name
evaluable is a bit too awkward.
|
|
To follow the naming of options like strict_mbit and more. Still accept
capx_strictness since this is known to be used.
Introduced in commit e4f28f3b.
|
|
To be able to disable the counting of messages for which application
callbacks take place. Messages sent/handled by diameter itself are
always counted.
|
|
That dialyzer hasn't noticed is broken.
|
|
Despite what the Efficiency Guide says about match being more efficient,
split_binary appears to be slightly faster. (Although this one
extraction is a drop in the bucket.) Binary optimizations aren't an
issue during decode.
|
|
Since messages are accumulated by appending binaries as of three
commits back, the accumulated binary is prone to being copied, as
discussed in the Efficiency Guide. Matching the Message Length header
as bytes are being accumulated is one cause of this, so work around it
by splitting the binary and extracting the length without a match.
This doesn't feel like something that should be necessary: that matching
a binary would cause an append to copy isn't obvious. The first attempt
at simplifying the accumulation was simply to append an incoming binary
to the current fragment, match against <<_, Len:24, _/binary>> to
extract the length, and then test if there are enough bytes for a
message. This lead to horrible performance (response times for 2 MB
messages approximately 100 times worse than previously), and it wasn't
at all obvious that the reason was the accumulated binary being copied
with each append as a result of the match. Using split_binary avoids the
match context that forces the copying.
|
|
Not with each setopts to re-activate the socket.
|
|
Only reset the fragment after all messages have been extracted.
|
|
Appending to a binary is efficient, so just append message fragments
Only match out the length once per message since doing so for every
packet from TCP causes the binary to be copied.
|
|
Create less garbage.
|
|
That dialyzer hasn't noticed is broken.
|
|
Also inline incr/8 and associated functions that were needed for the
compiler to accept the optimization, since this does make for a
measuable improvement.
|
|
With ERL_COMPILER_OPTIONS=bin_opt_info, before:
base/diameter_codec.erl:508: Warning: NOT OPTIMIZED: the binary matching instruction that follows in the same function have problems that prevent delayed sub binary optimization (probably indicated by INFO warnings)
And after:
base/diameter_codec.erl:508: Warning: OPTIMIZED: creation of sub binary delayed
This has a surprisingly large impact on the performance of
diameter_codec:collect_avps/2: about 15% faster in one testcase on a RAR
with 7 AVPs.
|
|
The index field in record diameter_avp was previously used to enumerate
AVPs so that the list could be returned in some cases, since
diameter_codec:collect_avps/2 (now /1) reversed the order. That's no
longer the case as of the grandparent commit, so use the field to
enumerate instances of the same AVP instead, and only when arities are
being checked, to save having to look them up in the map when checking
for 5009 errors, or counting AVPs at all in diameter_codec:collect_avps/1.
|
|
Extract strict_arities once and pass it through as an argument.
|
|
Decode has previously been two passes: first chunk the message into a
reversed list of toplevel diameter_avp records, then fold through the
reversed list to build the full result. Various workarounds have made it
a bit more convoluted than it should be however. Rework it completely,
turning the previous 2-pass tail-recursive implementation into a 1-pass
body recursive one.
The relay decode still exists in diameter_codec, as a stripped down
version of the full decode in diameter_gen.
|
|
|
|
To be able to disable the relatively expensive check that the number of
AVPs received in a message or grouped AVP agrees with the dictionary in
question. The may well be easier for the user in handle_request/answer
callbacks, when digesting the received message, and in some cases may
not be important.
The check at encode can also be disabled, allowing messages that don't
agree with the dictionary in question to be sent, which can be useful in
test (at least).
|
|
As noted in the parent commit, the wrong AVPs were reported, being
counted from the back of the message instead of the front. Both 5005 and
5009 errors are now detected after the message is decoded.
|
|
Stop counting when there can be no arity errors.
|
|
Count as AVPs are encoded instead.
|
|
Use the same [MsgName | Avps] representation as for the list decode, but
with Avps a map instead of a AVP name/values list. As a result, don't
set the message/AVP name on an additional key in the map, which felt a
bit odd. Messages are [MsgName :: atom() | map()], Grouped AVPs are just
map().
Fix at least one problem in the traffic suite along the way: with
decode_format false, the own decode in to_map/2 didn't know whether or
not to decode strings, resulting on some failures.
|
|
Undocumented, for transforming a map decode to record.
The record decode becomes more expensive the larger the number of AVPs
in the message definition in question, since the record is recreated
each time an AVP value is set in it. The map decode can potentially do
better.
|
|
{record_decode, map} is a bit too quirky.
|
|
That is, decode to the same format that encode already accepts. Only a
message has its name at the head of the list since AVPs are already
name/value pairs.
|
|
With {record_decode, map}. The option name is arguably a bit misleading
now, but not too objectionable given that the encode/decode in question
has historically only been of records.
One advantage of the map decode is that the map only contains values for
those AVPs existing in the message or grouped AVP in question. The name
of the message or grouped AVP is stored in with key ':name', the leading
colon ensuring that the key isn't a diameter-name.
Decoding to maps makes the hrl files generated from dictionary files
largely irrelevant. There are value defines generated into these, but
they're typically so long as to be unusable.
|
|
To control whether or not messages and grouped AVPs are decoded to
records, in #diameter_packet.msg and #diameter_avp.value respectively.
The decode became unnecessary for diameter's needs in parent commit,
which decoupled it from the checking of AVP arities.
|
|
Instead of after, during the check that AVPs have sufficient arity.
This makes the arity checks independent of the record decode, which
will allow the latter to be made optional.
|
|
Most of the contents were moved to module diameter_gen in commit
205521d3.
|
|
|
