Age | Commit message (Collapse) | Author |
|
|
|
|
|
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.
|
|
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.
|
|
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.
|
|
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.
|
|
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.
|
|
{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.
|
|
Most of the contents were moved to module diameter_gen in commit
205521d3.
|
|
The documentation has been out of date since the string_decode option
was added in commit 1590920c. The optionless decode/2 was removed in the
commit that removed the use of the process dictionary in decode.
|
|
To allow list-valued messaged to be encoded in the specified order,
instead of in the dictionary order by first converting the list to a
record. This is not yet exposed in configuration.
|
|
By passing additional arguments through it.
|
|
|
|
|
|
base/diameter_codec.erl:716: Warning: OPTIMIZED: creation of sub binary delayed
|
|
base/diameter_codec.erl:545: Warning: OPTIMIZED: creation of sub binary delayed
|
|
base/diameter_codec.erl:600: Warning: OPTIMIZED: creation of sub binary delayed
|
|
|
|
|
|
|
|
Dict:avp(encode, Value, Name) no longer needs to return a binary, only
an iolist(). Message encode runs list_to_binary/1 to convert accumulated
lists into a message binary.
|
|
This is a special case to allow encode of something other than an
iolist.
Eg. #diameter_avp{data = {diameter_gen_base_rfc6733,
'Proxy-Info',
[{'Proxy-Host', "HOST"}, {'Proxy-State', "STATE"}]}}
Only worked as expected for AVPs of type other than Grouped.
|
|
On the same theme as the parent commit, building binaries in fewer
steps.
|
|
Prepend the header in a single step.
Before:
{[{{diameter_codec,pack_avp,1}, 7000, 126.074, 51.058}],
{ {diameter_codec,pack_avp,2}, 7000, 126.074, 51.058}, %
[{{diameter_codec,pack_avp,5}, 7000, 51.144, 25.758},
{{diameter_codec,pad,2}, 7000, 23.844, 23.570},
{suspend, 1, 0.028, 0.000}]}.
After:
{[{{diameter_codec,pack_avp,1}, 7000, 78.563, 26.986}],
{ {diameter_codec,pack_avp,2}, 7000, 78.563, 26.986}, %
[{{diameter_codec,pack_avp,6}, 7000, 51.459, 26.381},
{suspend, 4, 0.118, 0.000}]}.
|
|
|
|
Profiling with fprof showed this prior to this commit:
{[{{diameter_codec,decode,3}, 1000, 231.122, 4.092},
{{diameter_codec,collect_avps,1}, 1000, 0.000, 3.929}],
{ {diameter_codec,collect_avps,1}, 2000, 231.122, 8.021}, %
[{{diameter_codec,collect_avps,3}, 1000, 222.932, 11.644},
{garbage_collect, 19, 0.169, 0.169},
{{diameter_codec,collect_avps,1}, 1000, 0.000, 3.929}]}.
{[{{diameter_codec,collect_avps,1}, 1000, 222.932, 11.644},
{{diameter_codec,collect_avps,3}, 7000, 0.000, 68.186}],
{ {diameter_codec,collect_avps,3}, 8000, 222.932, 79.830}, %
[{{diameter_codec,split_avp,1}, 7000, 120.886, 72.382},
{{erlang,setelement,3}, 7000, 21.830, 21.830},
{garbage_collect, 48, 0.386, 0.386},
{{diameter_codec,collect_avps,3}, 7000, 0.000, 68.186}]}.
Note the time consumed in split_avp/1 and erlang:setelement/3. This
commit does more matching in one go, without intermediate results,
giving this:
{[{{diameter_codec,decode,3}, 1000, 42.512, 3.701},
{{diameter_codec,collect_avps,1}, 1000, 0.000, 3.594}],
{ {diameter_codec,collect_avps,1}, 2000, 42.512, 7.295}, %
[{{diameter_codec,collect_avps,3}, 1000, 35.217, 4.577},
{{diameter_codec,collect_avps,1}, 1000, 0.000, 3.594}]}.
{[{{diameter_codec,collect_avps,1}, 1000, 35.217, 4.577},
{{diameter_codec,collect_avps,3}, 7000, 0.000, 27.754}],
{ {diameter_codec,collect_avps,3}, 8000, 35.217, 32.331}, %
[{garbage_collect, 262, 2.647, 2.647},
{suspend, 9, 0.239, 0.000},
{{diameter_codec,collect_avps,3}, 7000, 0.000, 27.754}]}.
|
|
* anders/diameter/M-bit/OTP-12947:
Add service_opt() strict_mbit
|
|
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.
|
|
The diffs are all about adapting to the OTP 18 time interface. The code
was previously backwards compatible, falling back on the erlang:now/0 if
erlang:monotonic_time/0 is unavailable, but this was seen to be a bad
thing in commit 9c0f2f2c. Use of erlang:now/0 is now removed.
|
|
* anders/diameter/grouped_errors/OTP-12930:
Fix decode of Grouped AVPs containing errors
Simplify logic
Simplify logic
|
|
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.
|
|
OTP-12845
* bruce/change-license:
fix errors caused by changed line numbers
Change license text to APLv2
|
|
|
|
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.
|
|
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.
|
|
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.
|
|
* 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.
|
|
6.1.9 of RFC 6733 states this:
A relay or proxy agent MUST append a Route-Record AVP to all requests
forwarded.
The AVP was inserted as the head of the AVP list, not appended, since
the entire AVP list was reversed relative to the received order.
Thanks to Andrzej Trawiński.
|
|
* anders/diameter/grouped_decode/OTP-12475:
Allow encode of decoded diameter_avp list
Add testcases for diameter_avp decode
Fix handling of length errors on Grouped AVPs
Don't discard component diameter_avp list on Grouped AVP decode error
Fix process dictionary manipulation during message decode
|
|
The decode of an incoming request in a non-relay application results in
a deep list of diameter_avp records. Encoding such a list resulted in a
function_clause error in diameter_codec:pack_avp/1, which expected a
flat list. The list is only flat in the relay case, or in the absence of
AVPs of type Grouped.
This is also related to code that exists but isn't documented. It's
documented that a diameter_app(3) handle_request callback can return
{relay, Opts} to relay a request received in the relay application.
What's not documented is that it can also return {proxy|resend, Opts} in
a non-relay application, but this leads to encode failure when there are
Grouped AVPs. This shouldn't be interpreted as meaning that proxy|resend
are now supported: they aren't. The two extra terms are a historical
relic that should probably be removed. Neither are generally usable
since, for example, a proxy agent may want to modify a request before
resending it. A specific handle_request return is not needed to
implement a proxy agent. Even {relay, Opts} isn't strictly necessary.
|
|
The decode of a Grouped AVP ignored the case that extracting component
AVPs with diameter_codec:collect_avps/1 returned a tuple, in the case of
a truncated AVP header.
|
|
Extracting the End-to-End and Hop-by-Hop identifiers resulted in a
function clause error, causing the send to fail.
|
|
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.
|