Merge branch 'anders/diameter/config_consistency/OTP-14555' into maint

* anders/diameter/config_consistency/OTP-14555:
  Let strict_mbit and incoming_maxlen be configured per transport
  Let a service configure default transport options
  Rename type evaluable -> eval

10 files changed, 332 insertions, 311 deletions

diff --git a/lib/diameter/doc/src/diameter.xml b/lib/diameter/doc/src/diameter.xml
index 8b72736108..b7be184058 100644
--- a/lib/diameter/doc/src/diameter.xml
+++ b/lib/diameter/doc/src/diameter.xml
@@ -397,10 +397,10 @@ from the peer offers it.</p>
 Note that each tuple communicates one or more AVP values.
 It is an error to specify duplicate tuples.</p>
 
-<marker id="evaluable"/>
+<marker id="eval"/>
 </item>
 
-<tag><c>evaluable() = {M,F,A} | fun() | [evaluable() | A]</c></tag>
+<tag><c>eval() = {M,F,A} | fun() | [eval() | A]</c></tag>
 <item>
 <p>
 An expression that can be evaluated as a function in the following
@@ -418,7 +418,7 @@ eval(F) ->
 </pre>
 
 <p>
-Applying an <c>&evaluable;</c>
+Applying an <c>&eval;</c>
 <c>E</c> to an argument list <c>A</c>
 is meant in the sense of <c>eval([E|A])</c>.</p>
 
@@ -484,11 +484,11 @@ Matches only those peers whose Origin-Realm has the
 specified value, or all peers if the atom <c>any</c>.</p>
 </item>
 
-<tag><c>{eval, &evaluable;}</c></tag>
+<tag><c>{eval, &eval;}</c></tag>
 <item>
 <p>
 Matches only those peers for which the specified
-<c>&evaluable;</c> returns
+<c>&eval;</c> returns
 <c>true</c> when applied to the connection's <c>diameter_caps</c>
 record.
 Any other return value or exception is equivalent to <c>false</c>.</p>
@@ -650,7 +650,7 @@ Result = ResultCode | {capabilities_cb, CB, ResultCode|discard}
 Caps = #diameter_caps{}
 Pkt  = #diameter_packet{}
 ResultCode = integer()
-CB = &evaluable;
+CB = &eval;
 </pre>
 
 <p>
@@ -826,24 +826,11 @@ field of diameter_packet records independently of
 
 </item>
 
-<tag>
-<marker id="incoming_maxlen"/><c>{incoming_maxlen, 0..16777215}</c></tag>
-<item>
-<p>
-Bound on the expected size of incoming Diameter messages.
-Messages larger than the specified number of bytes are discarded.</p>
-
-<p>
-Defaults to <c>16777215</c>, the maximum value of the 24-bit Message
-Length field in a Diameter Header.</p>
-
-</item>
-
 <tag><c>{restrict_connections, false
                              | node
                              | nodes
                              | [node()]
-                             | evaluable()}</c></tag>
+                             | eval()}</c></tag>
 <item>
 <p>
 The degree to which the service allows multiple transport
@@ -854,7 +841,7 @@ at capabilities exchange.</p>
 If <c>[node()]</c> then a connection is rejected if another already
 exists on any of the specified nodes.
 Types <c>false</c>, <c>node</c>, <c>nodes</c> and
-&evaluable; are equivalent to
+&eval; are equivalent to
 <c>[]</c>, <c>[node()]</c>, <c>[node()|nodes()]</c> and the
 evaluated value respectively, evaluation of each expression taking
 place whenever a new connection is to be established.
@@ -869,7 +856,7 @@ by their own peer and watchdog state machines.</p>
 Defaults to <c>nodes</c>.</p>
 </item>
 
-<tag><c>{sequence, {H,N} | &evaluable;}</c></tag>
+<tag><c>{sequence, {H,N} | &eval;}</c></tag>
 <item>
 <p>
 A constant value <c>H</c> for the topmost <c>32-N</c> bits of
@@ -904,7 +891,7 @@ outgoing requests.</p>
 </warning>
 </item>
 
-<tag><c>{share_peers, boolean() | [node()] | evaluable()}</c></tag>
+<tag><c>{share_peers, boolean() | [node()] | eval()}</c></tag>
 <item>
 <p>
 Nodes to which peer connections established on the local
@@ -917,7 +904,7 @@ configured to use them: see <c>use_shared_peers</c> below.</p>
 If <c>false</c> then peers are not shared.
 If <c>[node()]</c> then peers are shared with the specified list of
 nodes.
-If <c>evaluable()</c> then peers are shared with the nodes returned
+If <c>eval()</c> then peers are shared with the nodes returned
 by the specified function, evaluated whenever a peer connection
 becomes available or a remote service requests information about local
 connections.
@@ -943,18 +930,6 @@ of a single Diameter node across multiple Erlang nodes.</p>
 </note>
 </item>
 
-<tag><c>{spawn_opt, [term()]}</c></tag>
-<item>
-<p>
-Options list passed to &spawn_opt; when spawning a process for an
-incoming Diameter request, unless the transport in question
-specifies another value.
-Options <c>monitor</c> and <c>link</c> are ignored.</p>
-
-<p>
-Defaults to the empty list.</p>
-</item>
-
 <tag>
 <marker id="strict_arities"/><c>{strict_arities, boolean()
                                                | encode
@@ -988,49 +963,6 @@ of arity 1 as bare values, not wrapped in a list.</p>
 </item>
 
 <tag>
-<marker id="strict_mbit"/><c>{strict_mbit, boolean()}</c></tag>
-<item>
-<p>
-Whether or not to regard an AVP setting the M-bit as erroneous when
-the command grammar in question does not explicitly allow the AVP.
-If <c>true</c> then such AVPs are regarded as 5001 errors,
-DIAMETER_AVP_UNSUPPORTED.
-If <c>false</c> then the M-bit is ignored and policing
-it becomes the receiver's responsibility.</p>
-
-<p>
-Defaults to <c>true</c>.</p>
-
-<warning>
-<p>
-RFC 6733 is unclear about the semantics of the M-bit.
-One the one hand, the CCF specification in section 3.2 documents AVP
-in a command grammar as meaning <em>any</em> arbitrary AVP; on the
-other hand, 1.3.4 states that AVPs setting the M-bit cannot be added
-to an existing command: the modified command must instead be
-placed in a new Diameter application.</p>
-<p>
-The reason for the latter is presumably interoperability:
-allowing arbitrary AVPs setting the M-bit in a command makes its
-interpretation implementation-dependent, since there's no
-guarantee that all implementations will understand the same set of
-arbitrary AVPs in the context of a given command.
-However, interpreting <c>AVP</c> in a command grammar as any
-AVP, regardless of M-bit, renders 1.3.4 meaningless, since the receiver
-can simply ignore any AVP it thinks isn't relevant, regardless of the
-sender's intent.</p>
-<p>
-Beware of confusing mandatory in the sense of the M-bit with mandatory
-in the sense of the command grammar.
-The former is a semantic requirement: that the receiver understand the
-semantics of the AVP in the context in question.
-The latter is a syntactic requirement: whether or not the AVP must
-occur in the message in question.</p>
-</warning>
-
-</item>
-
-<tag>
 <marker id="string_decode"/><c>{string_decode, boolean()}</c></tag>
 <item>
 <p>
@@ -1074,7 +1006,7 @@ omitted counters are not returned by &service_info;.</p>
 
 </item>
 
-<tag><c>{use_shared_peers, boolean() | [node()] | evaluable()}</c></tag>
+<tag><c>{use_shared_peers, boolean() | [node()] | eval()}</c></tag>
 <item>
 <p>
 Nodes from which communicated peers are made available in
@@ -1084,7 +1016,7 @@ the remote candidates list of &app_pick_peer; callbacks.</p>
 If <c>false</c> then remote peers are not used.
 If <c>[node()]</c> then only peers from the specified list of nodes
 are used.
-If <c>evaluable()</c> then only peers returned by the specified
+If <c>eval()</c> then only peers returned by the specified
 function are used, evaluated whenever a remote service communicates
 information about an available peer connection.
 The value <c>true</c> is equivalent to <c>fun &nodes;</c>.
@@ -1109,6 +1041,15 @@ each node from which requests are sent.</p>
 </warning>
 </item>
 
+<tag><c>&transport_opt;</c></tag>
+<item>
+<p>
+Any transport option except <c>applications</c> or
+<c>capabilities</c>.
+Used as defaults for transport configuration, values passed to
+&add_transport; overriding values configured on the service.</p>
+</item>
+
 </taglist>
 
 <marker id="transport_opt"/>
@@ -1156,7 +1097,7 @@ TLS is desired over TCP as implemented by &man_tcp;.</p>
 </item>
 
 <tag>
-<marker id="capabilities_cb"/><c>{capabilities_cb, &evaluable;}</c></tag>
+<marker id="capabilities_cb"/><c>{capabilities_cb, &eval;}</c></tag>
 <item>
 <p>
 Callback invoked upon reception of CER/CEA during capabilities
@@ -1250,7 +1191,7 @@ transport.</p>
 </item>
 
 <tag>
-<marker id="disconnect_cb"/><c>{disconnect_cb, &evaluable;}</c></tag>
+<marker id="disconnect_cb"/><c>{disconnect_cb, &eval;}</c></tag>
 <item>
 <p>
 Callback invoked prior to terminating the transport process of a
@@ -1350,6 +1291,19 @@ Defaults to 5000.</p>
 </item>
 
 <tag>
+<marker id="incoming_maxlen"/><c>{incoming_maxlen, 0..16777215}</c></tag>
+<item>
+<p>
+Bound on the expected size of incoming Diameter messages.
+Messages larger than the specified number of bytes are discarded.</p>
+
+<p>
+Defaults to <c>16777215</c>, the maximum value of the 24-bit Message
+Length field in a Diameter Header.</p>
+
+</item>
+
+<tag>
 <marker id="length_errors"/><c>{length_errors, exit|handle|discard}</c></tag>
 <item>
 <p>
@@ -1407,7 +1361,7 @@ incoming Diameter request.
 Options <c>monitor</c> and <c>link</c> are ignored.</p>
 
 <p>
-Defaults to the list configured on the service if not specified.</p>
+Defaults to the empty list.</p>
 </item>
 
 <tag>
@@ -1425,6 +1379,49 @@ case peers are known to be behave badly.</p>
 </item>
 
 <tag>
+<marker id="strict_mbit"/><c>{strict_mbit, boolean()}</c></tag>
+<item>
+<p>
+Whether or not to regard an AVP setting the M-bit as erroneous when
+the command grammar in question does not explicitly allow the AVP.
+If <c>true</c> then such AVPs are regarded as 5001 errors,
+DIAMETER_AVP_UNSUPPORTED.
+If <c>false</c> then the M-bit is ignored and policing
+it becomes the receiver's responsibility.</p>
+
+<p>
+Defaults to <c>true</c>.</p>
+
+<warning>
+<p>
+RFC 6733 is unclear about the semantics of the M-bit.
+One the one hand, the CCF specification in section 3.2 documents AVP
+in a command grammar as meaning <em>any</em> arbitrary AVP; on the
+other hand, 1.3.4 states that AVPs setting the M-bit cannot be added
+to an existing command: the modified command must instead be
+placed in a new Diameter application.</p>
+<p>
+The reason for the latter is presumably interoperability:
+allowing arbitrary AVPs setting the M-bit in a command makes its
+interpretation implementation-dependent, since there's no
+guarantee that all implementations will understand the same set of
+arbitrary AVPs in the context of a given command.
+However, interpreting <c>AVP</c> in a command grammar as any
+AVP, regardless of M-bit, renders 1.3.4 meaningless, since the receiver
+can simply ignore any AVP it thinks isn't relevant, regardless of the
+sender's intent.</p>
+<p>
+Beware of confusing mandatory in the sense of the M-bit with mandatory
+in the sense of the command grammar.
+The former is a semantic requirement: that the receiver understand the
+semantics of the AVP in the context in question.
+The latter is a syntactic requirement: whether or not the AVP must
+occur in the message in question.</p>
+</warning>
+
+</item>
+
+<tag>
 <marker id="transport_config"/><c>{transport_config, term()}</c></tag><item/>
 <tag><c>{transport_config, term(), &dict_Unsigned32; | infinity}</c></tag>
 <item>
diff --git a/lib/diameter/doc/src/diameter_app.xml b/lib/diameter/doc/src/diameter_app.xml
index dfcd00975b..aa334beb21 100644
--- a/lib/diameter/doc/src/diameter_app.xml
+++ b/lib/diameter/doc/src/diameter_app.xml
@@ -13,7 +13,8 @@
 <header>
 
 <copyright>
-<year>2011</year><year>2016</year>
+<year>2011</year>
+<year>2017</year>
 <holder>Ericsson AB. All Rights Reserved.</holder>
 </copyright>
 <legalnotice>
@@ -319,7 +320,7 @@ or &peer_down; callback.</p>
 <v>Action = Send | Discard | {eval_packet, Action, PostF}</v>
 <v>Send = {send, &packet; | &message;}</v>
 <v>Discard = {discard, Reason} | discard</v>
-<v>PostF = &mod_evaluable;}</v>
+<v>PostF = &mod_eval;}</v>
 </type>
 <desc>
 <p>
@@ -371,7 +372,7 @@ discarded}</c>.</p>
 <v>Action = Send | Discard | {eval_packet, Action, PostF}</v>
 <v>Send = {send, &packet; | &message;}</v>
 <v>Discard = {discard, Reason} | discard</v>
-<v>PostF = &mod_evaluable;}</v>
+<v>PostF = &mod_eval;}</v>
 </type>
 <desc>
 <p>
@@ -478,7 +479,7 @@ not selected.</p>
            | {answer_message, 3000..3999|5000..5999}
            | {protocol_error, 3000..3999}</v>
 <v>Opt     = &mod_call_opt;</v>
-<v>PostF   = &mod_evaluable;</v>
+<v>PostF   = &mod_eval;</v>
 </type>
 <desc>
 <p>
diff --git a/lib/diameter/doc/src/seealso.ent b/lib/diameter/doc/src/seealso.ent
index e5c284c6e8..ef6af1a3d0 100644
--- a/lib/diameter/doc/src/seealso.ent
+++ b/lib/diameter/doc/src/seealso.ent
@@ -4,7 +4,7 @@
 
 %CopyrightBegin%
 
-Copyright Ericsson AB 2012-2015. All Rights Reserved.
+Copyright Ericsson AB 2012-2017. All Rights Reserved.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -53,7 +53,7 @@ significant.
 <!ENTITY mod_application_opt '<seealso marker="diameter#application_opt">diameter:application_opt()</seealso>'>
 <!ENTITY mod_call_opt '<seealso marker="diameter#call_opt">diameter:call_opt()</seealso>'>
 <!ENTITY mod_capability '<seealso marker="diameter#capability">diameter:capability()</seealso>'>
-<!ENTITY mod_evaluable '<seealso marker="diameter#evaluable">diameter:evaluable()</seealso>'>
+<!ENTITY mod_eval '<seealso marker="diameter#eval">diameter:eval()</seealso>'>
 <!ENTITY mod_peer_filter '<seealso marker="diameter#peer_filter">diameter:peer_filter()</seealso>'>
 <!ENTITY mod_service_event '<seealso marker="diameter#service_event">diameter:service_event()</seealso>'>
 <!ENTITY mod_service_event_info '<seealso marker="diameter#service_event_info">diameter:service_event_info()</seealso>'>
diff --git a/lib/diameter/src/base/diameter.erl b/lib/diameter/src/base/diameter.erl
index 888a108f8b..3b41feac0d 100644
--- a/lib/diameter/src/base/diameter.erl
+++ b/lib/diameter/src/base/diameter.erl
@@ -46,7 +46,8 @@
 -export([start/0,
          stop/0]).
 
--export_type([evaluable/0,
+-export_type([eval/0,
+              evaluable/0,  %% deprecated
               decode_format/0,
               strict_arities/0,
               restriction/0,
@@ -301,7 +302,7 @@ call(SvcName, App, Message) ->
     | realm
     | {host,  any|'DiameterIdentity'()}
     | {realm, any|'DiameterIdentity'()}
-    | {eval, evaluable()}
+    | {eval, eval()}
     | {neg, peer_filter()}
     | {all, [peer_filter()]}
     | {any, [peer_filter()]}.
@@ -309,10 +310,13 @@ call(SvcName, App, Message) ->
 -opaque peer_ref()
    :: pid().
 
--type evaluable()
+-type eval()
    :: {module(), atom(), list()}
     | fun()
-    | maybe_improper_list(evaluable(), list()).
+    | maybe_improper_list(eval(), list()).
+
+-type evaluable()
+   :: eval().
 
 -type sequence()
    :: {'Unsigned32'(), 0..32}.
@@ -322,12 +326,12 @@ call(SvcName, App, Message) ->
     | node
     | nodes
     | [node()]
-    | evaluable().
+    | eval().
 
 -type remotes()
    :: boolean()
     | [node()]
-    | evaluable().
+    | eval().
 
 -type message_length()
    :: 0..16#FFFFFF.
@@ -344,22 +348,38 @@ call(SvcName, App, Message) ->
     | encode
     | decode.
 
+%% Options common to both start_service/2 and add_transport/2.
+
+-type common_opt()
+   :: {pool_size, pos_integer()}
+    | {capabilities_cb, eval()}
+    | {capx_timeout, 'Unsigned32'()}
+    | {strict_capx, boolean()}
+    | {strict_mbit, boolean()}
+    | {disconnect_cb, eval()}
+    | {dpr_timeout, 'Unsigned32'()}
+    | {dpa_timeout, 'Unsigned32'()}
+    | {incoming_maxlen, message_length()}
+    | {length_errors, exit | handle | discard}
+    | {connect_timer, 'Unsigned32'()}
+    | {watchdog_timer, 'Unsigned32'() | {module(), atom(), list()}}
+    | {watchdog_config, [{okay|suspect, non_neg_integer()}]}
+    | {spawn_opt, list()}.
+
 %% Options passed to start_service/2
 
 -type service_opt()
    :: capability()
     | {application, [application_opt()]}
     | {restrict_connections, restriction()}
-    | {sequence, sequence() | evaluable()}
+    | {sequence, sequence() | eval()}
     | {share_peers, remotes()}
     | {decode_format, decode_format()}
     | {traffic_counters, boolean()}
     | {string_decode, boolean()}
     | {strict_arities, true | strict_arities()}
-    | {strict_mbit, boolean()}
-    | {incoming_maxlen, message_length()}
     | {use_shared_peers, remotes()}
-    | {spawn_opt, list()}.
+    | common_opt().
 
 -type application_opt()
    :: {alias, app_alias()}
@@ -389,20 +409,9 @@ call(SvcName, App, Message) ->
    :: {transport_module, atom()}
     | {transport_config, any()}
     | {transport_config, any(), 'Unsigned32'() | infinity}
-    | {pool_size, pos_integer()}
     | {applications, [app_alias()]}
     | {capabilities, [capability()]}
-    | {capabilities_cb, evaluable()}
-    | {capx_timeout, 'Unsigned32'()}
-    | {strict_capx, boolean()}
-    | {disconnect_cb, evaluable()}
-    | {dpr_timeout, 'Unsigned32'()}
-    | {dpa_timeout, 'Unsigned32'()}
-    | {length_errors, exit | handle | discard}
-    | {connect_timer, 'Unsigned32'()}
-    | {watchdog_timer, 'Unsigned32'() | {module(), atom(), list()}}
-    | {watchdog_config, [{okay|suspect, non_neg_integer()}]}
-    | {spawn_opt, list()}
+    | common_opt()
     | {private, any()}.
 
 %% Predicate passed to remove_transport/2
diff --git a/lib/diameter/src/base/diameter_callback.erl b/lib/diameter/src/base/diameter_callback.erl
index f9cdc66c70..d04a416bef 100644
--- a/lib/diameter/src/base/diameter_callback.erl
+++ b/lib/diameter/src/base/diameter_callback.erl
@@ -26,16 +26,16 @@
 %% as the Diameter application callback in question. The record has
 %% one field for each callback function as well as 'default' and
 %% 'extra' fields. A function-specific field can be set to a
-%% diameter:evaluable() in order to redirect the callback
+%% diameter:eval() in order to redirect the callback
 %% corresponding to that field, or to 'false' to request the default
 %% callback implemented in this module. If neither of these fields are
 %% set then the 'default' field determines the form of the callback: a
 %% module name results in the usual callback as if the module had been
-%% configured directly as the callback module, a diameter_evaluable()
+%% configured directly as the callback module, a diameter_eval()
 %% in a callback applied to the atom-valued callback name and argument
 %% list. For all callbacks not to this module, the 'extra' field is a
 %% list of additional arguments, following arguments supplied by
-%% diameter but preceding those of the diameter:evaluable() being
+%% diameter but preceding those of the diameter:eval() being
 %% applied.
 %%
 %% For example, the following config to diameter:start_service/2, in
diff --git a/lib/diameter/src/base/diameter_config.erl b/lib/diameter/src/base/diameter_config.erl
index d54dbd9976..4721be1ca0 100644
--- a/lib/diameter/src/base/diameter_config.erl
+++ b/lib/diameter/src/base/diameter_config.erl
@@ -102,9 +102,6 @@
 -record(monitor, {mref = make_ref() :: reference(),
                   service}). %% name
 
-%% The default sequence mask.
--define(NOMASK, {0,32}).
-
 %% Time to lay low before restarting a dead service.
 -define(RESTART_SLEEP, 2000).
 
@@ -560,89 +557,184 @@ add(SvcName, Type, Opts0) ->
     end.
 
 transport_opts(Opts) ->
-    lists:map(fun topt/1, Opts).
+    [setopt(transport, T) || T <- Opts].
+
+%% setopt/2
 
-topt(T) ->
-    case opt(T) of
+setopt(K, T) ->
+    case opt(K, T) of
         {value, X} ->
             X;
         true ->
             T;
         false ->
-            ?THROW({invalid, T})
+            ?THROW({invalid, T});
+        {error, Reason} ->
+            ?THROW({invalid, T, Reason})
     end.
 
-opt({transport_module, M}) ->
+%% opt/2
+
+opt(_, {incoming_maxlen, N}) ->
+    is_integer(N) andalso 0 =< N andalso N < 1 bsl 24;
+
+opt(service, {K, B})
+  when K == string_decode;
+       K == traffic_counters ->
+    is_boolean(B);
+
+opt(service, {K, false})
+  when K == share_peers;
+       K == use_shared_peers;
+       K == monitor;
+       K == restrict_connections;
+       K == strict_arities;
+       K == decode_format ->
+    true;
+
+opt(service, {K, true})
+  when K == share_peers;
+       K == use_shared_peers;
+       K == strict_arities ->
+    true;
+
+opt(service, {decode_format, T})
+  when T == record;
+       T == list;
+       T == map;
+       T == record_from_map ->
+    true;
+
+opt(service, {strict_arities, T})
+  when T == encode;
+       T == decode ->
+    true;
+
+opt(service, {restrict_connections, T})
+  when T == node;
+       T == nodes ->
+    true;
+
+opt(service, {K, T})
+  when (K == share_peers
+        orelse K == use_shared_peers
+        orelse K == restrict_connections), ([] == T
+                                            orelse is_atom(hd(T))) ->
+    true;
+
+opt(service, {monitor, P}) ->
+    is_pid(P);
+
+opt(service, {K, F})
+  when K == restrict_connections;
+       K == share_peers;
+       K == use_shared_peers ->
+    try diameter_lib:eval(F) of  %% but no guarantee that it won't fail later
+        Nodes ->
+            is_list(Nodes) orelse {error, Nodes}
+    catch
+        E:R ->
+            {error, {E, R, ?STACK}}
+    end;
+
+opt(service, {sequence, {H,N}}) ->
+    0 =< N andalso N =< 32
+        andalso is_integer(H)
+        andalso 0 =< H
+        andalso 0 == H bsr (32-N);
+
+opt(service = S, {sequence = K, F}) ->
+    try diameter_lib:eval(F) of
+        {_,_} = T ->
+            KT = {K,T},
+            opt(S, KT) andalso {value, KT};
+        V ->
+            {error, V}
+    catch
+        E:R ->
+            {error, {E, R, ?STACK}}
+    end;
+
+opt(transport, {transport_module, M}) ->
     is_atom(M);
 
-opt({transport_config, _, Tmo}) ->
+opt(transport, {transport_config, _, Tmo}) ->
     ?IS_UINT32(Tmo) orelse Tmo == infinity;
 
-opt({applications, As}) ->
+opt(transport, {applications, As}) ->
     is_list(As);
 
-opt({capabilities, Os}) ->
-    is_list(Os) andalso ok == encode_CER(Os);
+opt(transport, {capabilities, Os}) ->
+    is_list(Os) andalso case encode_CER(Os) of
+                            ok -> true;
+                            No -> {error, No}
+                        end;
 
-opt({K, Tmo})
+opt(_, {K, Tmo})
   when K == capx_timeout;
        K == dpr_timeout;
        K == dpa_timeout ->
     ?IS_UINT32(Tmo);
 
-opt({capx_strictness, B}) ->
+opt(_, {capx_strictness, B}) ->
     is_boolean(B) andalso {value, {strict_capx, B}};
-opt({strict_capx, B}) ->
+opt(_, {K, B})
+  when K == strict_capx;
+       K == strict_mbit ->
     is_boolean(B);
 
-opt({length_errors, T}) ->
+opt(_, {length_errors, T}) ->
     lists:member(T, [exit, handle, discard]);
 
-opt({K, Tmo})
-  when K == reconnect_timer;  %% deprecated
-       K == connect_timer ->
+opt(transport, {reconnect_timer, Tmo}) ->  %% deprecated
+    ?IS_UINT32(Tmo) andalso {value, {connect_timer, Tmo}};
+opt(_, {connect_timer, Tmo}) ->
     ?IS_UINT32(Tmo);
 
-opt({watchdog_timer, {M,F,A}})
+opt(_, {watchdog_timer, {M,F,A}})
   when is_atom(M), is_atom(F), is_list(A) ->
     true;
-opt({watchdog_timer, Tmo}) ->
+opt(_, {watchdog_timer, Tmo}) ->
     ?IS_UINT32(Tmo);
 
-opt({watchdog_config, L}) ->
-    is_list(L) andalso lists:all(fun wdopt/1, L);
+opt(_, {watchdog_config, L}) ->
+    is_list(L) andalso lists:all(fun wd/1, L);
 
-opt({spawn_opt, {M,F,A}})
+opt(_, {spawn_opt, {M,F,A}})
   when is_atom(M), is_atom(F), is_list(A) ->
     true;
-opt({spawn_opt = K, Opts}) ->
+opt(_, {spawn_opt = K, Opts}) ->
     if is_list(Opts) ->
             {value, {K, spawn_opts(Opts)}};
        true ->
             false
     end;
 
-opt({pool_size, N}) ->
+opt(_, {pool_size, N}) ->
     is_integer(N) andalso 0 < N;
 
-%% Options that we can't validate.
-opt({K, _})
+%% Options we can't validate.
+opt(_, {K, _})
+  when K == disconnect_cb;
+       K == capabilities_cb ->
+    true;
+opt(transport, {K, _})
   when K == transport_config;
-       K == capabilities_cb;
-       K == disconnect_cb;
        K == private ->
     true;
 
-%% Anything else, which is ignored by us. This makes options sensitive
-%% to spelling mistakes but arbitrary options are passed by some users
-%% as a way to identify transports. (That is, can't just do away with
-%% it.)
-opt(_) ->
-    true.
+%% Anything else, which is ignored in transport config. This makes
+%% options sensitive to spelling mistakes, but arbitrary options are
+%% passed by some users as a way to identify transports so can't just
+%% do away with it.
+opt(K, _) ->
+    K == transport.
+
+%% wd/1
 
-wdopt({K,N}) ->
+wd({K,N}) ->
     (K == okay orelse K == suspect) andalso is_integer(N) andalso 0 =< N;
-wdopt(_) ->
+wd(_) ->
     false.
 
 %% start_transport/2
@@ -707,19 +799,7 @@ make_config(SvcName, Opts) ->
 
     ok = encode_CER(CapOpts),
 
-    SvcOpts = make_opts((Opts -- AppOpts) -- CapOpts,
-                        [{false, share_peers},
-                         {false, use_shared_peers},
-                         {false, monitor},
-                         {?NOMASK, sequence},
-                         {nodes, restrict_connections},
-                         {16#FFFFFF, incoming_maxlen},
-                         {true, strict_arities},
-                         {true, strict_mbit},
-                         {record, decode_format},
-                         {true, traffic_counters},
-                         {true, string_decode},
-                         {[], spawn_opt}]),
+    SvcOpts = service_opts((Opts -- AppOpts) -- CapOpts),
 
     D = proplists:get_value(string_decode, SvcOpts, true),
 
@@ -733,115 +813,22 @@ binary_caps(Caps, true) ->
 binary_caps(Caps, false) ->
     diameter_capx:binary_caps(Caps).
 
-%% make_opts/2
-
-make_opts(Opts, Defs) ->
-    Known = [{K, get_opt(K, Opts, D)} || {D,K} <- Defs],
-    Unknown = Opts -- Known,
+%% service_opts/1
 
-    [] == Unknown orelse ?THROW({invalid, hd(Unknown)}),
-
-    [{K, opt(K,V)} || {K,V} <- Known].
-
-opt(incoming_maxlen, N)
-  when 0 =< N, N < 1 bsl 24 ->
-    N;
-
-opt(spawn_opt, {M,F,A} = T)
-  when is_atom(M), is_atom(F), is_list(A) ->
-    T;
-
-opt(spawn_opt, L)
-  when is_list(L) ->
-    spawn_opts(L);
-
-opt(K, false = B)
-  when K == share_peers;
-       K == use_shared_peers;
-       K == monitor;
-       K == restrict_connections;
-       K == strict_arities;
-       K == strict_mbit;
-       K == decode_format;
-       K == traffic_counters;
-       K == string_decode ->
-    B;
-
-opt(K, true = B)
-  when K == share_peers;
-       K == use_shared_peers;
-       K == strict_arities;
-       K == strict_mbit;
-       K == traffic_counters;
-       K == string_decode ->
-    B;
-
-opt(decode_format, T)
-  when T == record;
-       T == list;
-       T == map;
-       T == record_from_map ->
-    T;
-
-opt(strict_arities, T)
-  when T == encode;
-       T == decode ->
-    T;
-
-opt(restrict_connections, T)
-  when T == node;
-       T == nodes ->
-    T;
-
-opt(K, T)
-  when (K == share_peers
-        orelse K == use_shared_peers
-        orelse K == restrict_connections), ([] == T
-                                            orelse is_atom(hd(T))) ->
-    T;
-
-opt(monitor, P)
-  when is_pid(P) ->
-    P;
-
-opt(K, F)
-  when K == restrict_connections;
-       K == share_peers;
-       K == use_shared_peers ->
-    try diameter_lib:eval(F) of  %% but no guarantee that it won't fail later
-        Nodes when is_list(Nodes) ->
-            F;
-        V ->
-            ?THROW({value, {K,V}})
-    catch
-        E:R ->
-            ?THROW({value, {K, E, R, ?STACK}})
-    end;
-
-opt(sequence, {_,_} = T) ->
-    sequence(T);
-
-opt(sequence = K, F) ->
-    try diameter_lib:eval(F) of
-        T -> sequence(T)
-    catch
-        E:R ->
-            ?THROW({value, {K, E, R, ?STACK}})
-    end;
-
-opt(K, _) ->
-    ?THROW({value, K}).
+service_opts(Opts) ->
+    Res = [setopt(service, T) || T <- Opts],
+    Keys = sets:to_list(sets:from_list([K || {K,_} <- Res])), %% unique
+    Dups = lists:foldl(fun(K,A) -> lists:keydelete(K, 1, A) end, Res, Keys),
+    [] == Dups orelse ?THROW({duplicate, Dups}),
+    Res.
+%% Reject duplicates on a service, but not on a transport. There's no
+%% particular reason for the inconsistency, but the historic behaviour
+%% ignores all but the first of a transport_opt(), and there's no real
+%% reason to change it.
 
 spawn_opts(L) ->
     [T || T <- L, T /= link, T /= monitor].
 
-sequence({H,N} = T)
-  when 0 =< N, N =< 32, 0 =< H, 0 == H bsr (32-N) ->
-    T;
-
-sequence(_) ->
-    ?THROW({value, sequence}).
-
 make_caps(Caps, Opts) ->
     case diameter_capx:make_caps(Caps, Opts) of
         {ok, T} ->
diff --git a/lib/diameter/src/base/diameter_service.erl b/lib/diameter/src/base/diameter_service.erl
index 208e2e8cb8..802e6e7243 100644
--- a/lib/diameter/src/base/diameter_service.erl
+++ b/lib/diameter/src/base/diameter_service.erl
@@ -117,6 +117,18 @@
                       decode_format := diameter:decode_format(),
                       traffic_counters := boolean(),
                       string_decode := boolean(),
+                      capabilities_cb => diameter:evaluable(),
+                      pool_size => pos_integer(),
+                      capx_timeout => diameter:'Unsigned32'(),
+                      strict_capx => boolean(),
+                      disconnect_cb => diameter:evaluable(),
+                      dpr_timeout => diameter:'Unsigned32'(),
+                      dpa_timeout => diameter:'Unsigned32'(),
+                      length_errors => exit | handle | discard,
+                      connect_timer => diameter:'Unsigned32'(),
+                      watchdog_timer => diameter:'Unsigned32'()
+                                      | {module(), atom(), list()},
+                      watchdog_config => [{okay|suspect, non_neg_integer()}],
                       spawn_opt := list() | {module(), atom(), list()}}}).
 
 %% Record representing an RFC 3539 watchdog process implemented by
@@ -689,12 +701,15 @@ i(SvcName) ->
 cfg_acc({SvcName, #diameter_service{applications = Apps} = Rec, Opts},
         {false, Acc}) ->
     lists:foreach(fun init_mod/1, Apps),
+    #{monitor := M}
+        = SvcOpts
+        = service_opts(Opts),
     S = #state{service_name = SvcName,
                service = Rec#diameter_service{pid = self()},
                local   = init_peers(),
                remote  = init_peers(),
-               monitor = mref(get_value(monitor, Opts)),
-               options = service_options(lists:keydelete(monitor, 1, Opts))},
+               monitor = mref(M),
+               options = maps:remove(monitor, SvcOpts)},
     {S, Acc};
 
 cfg_acc({_Ref, Type, _Opts} = T, {S, Acc})
@@ -709,8 +724,23 @@ init_peers() ->
                                %%  Alias,
                                %%  TPid}
 
-service_options(Opts) ->
-    maps:from_list(lists:delete({strict_arities, true}, Opts)).
+service_opts(Opts) ->
+    T = {strict_arities, true},
+    maps:merge(maps:from_list([{monitor, false} | def_opts() -- [T]]),
+               maps:from_list(Opts -- [T])).
+
+def_opts() ->  %% defaults on the service map
+    [{share_peers, false},
+     {use_shared_peers, false},
+     {sequence, {0,32}},
+     {restrict_connections, nodes},
+     {incoming_maxlen, 16#FFFFFF},
+     {strict_arities, true},
+     {strict_mbit, true},
+     {decode_format, record},
+     {traffic_counters, true},
+     {string_decode, true},
+     {spawn_opt, []}].
 
 mref(false = No) ->
     No;
@@ -730,10 +760,6 @@ init_mod(#diameter_app{alias = Alias,
 start_fsm({Ref, Type, Opts}, S) ->
     start(Ref, {Type, Opts}, S).
 
-get_value(Key, Vs) ->
-    {_, V} = lists:keyfind(Key, 1, Vs),
-    V.
-
 notify(Share, SvcName, T) ->
     Nodes = remotes(Share),
     [] /= Nodes andalso diameter_peer:notify(Nodes, SvcName, T).
@@ -823,7 +849,7 @@ start(Ref, Type, Opts, State) ->
 start(Ref, Type, Opts, N, #state{watchdogT = WatchdogT,
                                  local = {PeerT, _, _},
                                  options = #{string_decode := SD}
-                                 = SvcOpts0,
+                                 = SvcOpts,
                                  service_name = SvcName,
                                  service = Svc0})
   when Type == connect;
@@ -832,12 +858,12 @@ start(Ref, Type, Opts, N, #state{watchdogT = WatchdogT,
         = Svc1
         = merge_service(Opts, Svc0),
     Svc = binary_caps(Svc1, SD),
-    SvcOpts = merge_options(Opts, SvcOpts0),
-    RecvData = diameter_traffic:make_recvdata([SvcName, PeerT, Apps, SvcOpts]),
-    T = {Opts, SvcOpts, RecvData, Svc},
+    {SOpts, TOpts} = merge_opts(SvcOpts, Opts),
+    RecvData = diameter_traffic:make_recvdata([SvcName, PeerT, Apps, SOpts]),
+    T = {TOpts, SOpts, RecvData, Svc},
     Rec = #watchdog{type = Type,
                     ref = Ref,
-                    options = Opts},
+                    options = TOpts},
 
     diameter_lib:fold_n(fun(_,A) ->
                                 [wd(Type, Ref, T, WatchdogT, Rec) | A]
@@ -845,10 +871,14 @@ start(Ref, Type, Opts, N, #state{watchdogT = WatchdogT,
                         [],
                         N).
 
-merge_options(Opts, SvcOpts) ->
-    Keys = maps:keys(SvcOpts),
-    Map = maps:from_list([KV || {K,_} = KV <- Opts, lists:member(K, Keys)]),
-    maps:merge(SvcOpts, Map).
+merge_opts(SvcOpts, Opts) ->
+    Keys = [K || {K,_} <- def_opts()],
+    SO = [T || {K,_} = T <- Opts, lists:member(K, Keys)],
+    TO = Opts -- SO,
+    {maps:merge(maps:with(Keys, SvcOpts), maps:from_list(SO)),
+     TO ++ [T || {K,_} = T <- maps:to_list(SvcOpts),
+                 not lists:member(K, Keys),
+                 not lists:keymember(K, 1, Opts)]}.
 
 binary_caps(Svc, true) ->
     Svc;
diff --git a/lib/diameter/src/base/diameter_watchdog.erl b/lib/diameter/src/base/diameter_watchdog.erl
index b2172356ee..bb671e9860 100644
--- a/lib/diameter/src/base/diameter_watchdog.erl
+++ b/lib/diameter/src/base/diameter_watchdog.erl
@@ -76,10 +76,8 @@
                     string_decode := false,
                     strict_arities => diameter:strict_arities(),
                     strict_mbit := boolean(),
-                    failed_avp := false,
                     rfc := 3588 | 6733,
-                    ordered_encode := false,
-                    incoming_maxlen := diameter:message_length()},
+                    ordered_encode := false},
          shutdown = false :: boolean()}).
 
 %% ---------------------------------------------------------------------------
@@ -137,15 +135,6 @@ i({Ack, T, Pid, {Opts,
     putr(restart, {T, Opts, Svc, SvcOpts}),  %% save seeing it in trace
     putr(dwr, dwr(Caps)),                    %%
     Nodes = restrict_nodes(Restrict),
-    CodecKeys = [decode_format,
-                 string_decode,
-                 strict_arities,
-                 strict_mbit,
-                 incoming_maxlen,
-                 spawn_opt,
-                 rfc,
-                 ordered_encode],
-
     #watchdog{parent = Pid,
               transport = start(T, Opts, SvcOpts, Nodes, Dict0, Svc),
               tw = proplists:get_value(watchdog_timer,
@@ -153,13 +142,21 @@ i({Ack, T, Pid, {Opts,
                                        ?DEFAULT_TW_INIT),
               receive_data = RecvData,
               dictionary = Dict0,
-              config =
-                  maps:without([traffic_counters | CodecKeys],
-                               config(SvcOpts#{restrict => restrict(Nodes),
-                                               suspect => 1,
-                                               okay => 3},
-                                      Opts)),
-              codec = maps:with(CodecKeys -- [strict_arities],
+              config = maps:with([sequence,
+                                  restrict_connections,
+                                  restrict,
+                                  suspect,
+                                  okay],
+                                 config(SvcOpts#{restrict => restrict(Nodes),
+                                                 suspect => 1,
+                                                 okay => 3},
+                                        Opts)),
+              codec = maps:with([decode_format,
+                                 strict_arities,
+                                 strict_mbit,
+                                 string_decode,
+                                 rfc,
+                                 ordered_encode],
                                 SvcOpts#{decode_format := false,
                                          string_decode := false,
                                          ordered_encode => false})}.
diff --git a/lib/diameter/src/transport/diameter_sctp.erl b/lib/diameter/src/transport/diameter_sctp.erl
index e30d7f5cd7..8de19e904e 100644
--- a/lib/diameter/src/transport/diameter_sctp.erl
+++ b/lib/diameter/src/transport/diameter_sctp.erl
@@ -79,7 +79,7 @@
 -type option() :: {sender, boolean()}
                 | sender
                 | {packet, boolean() | raw}
-                | {message_cb, false | diameter:evaluable()}.
+                | {message_cb, false | diameter:eval()}.
 
 -type uint() :: non_neg_integer().
 
@@ -104,7 +104,7 @@
          os = 0   :: uint(),               %% next output stream
          packet = true :: boolean()        %% legacy transport_data?
                         | raw,
-         message_cb = false :: false | diameter:evaluable(),
+         message_cb = false :: false | diameter:eval(),
          send = false :: pid() | boolean()}).      %% sending process
 
 %% Monitor process state.
@@ -120,7 +120,7 @@
          socket    :: gen_sctp:sctp_socket(),
          service   :: pid(), %% service process
          pending = {0, queue:new()},
-         opts      :: [[match()] | boolean() | diameter:evaluable()]}).
+         opts      :: [[match()] | boolean() | diameter:eval()]}).
 %% Field pending implements two queues: the first of transport-to-be
 %% processes to which an association has been assigned but for which
 %% diameter hasn't yet spawned a transport process, a short-lived
diff --git a/lib/diameter/src/transport/diameter_tcp.erl b/lib/diameter/src/transport/diameter_tcp.erl
index 2b7371df22..a8639baa11 100644
--- a/lib/diameter/src/transport/diameter_tcp.erl
+++ b/lib/diameter/src/transport/diameter_tcp.erl
@@ -110,7 +110,7 @@
 -type option() :: {port, non_neg_integer()}
                 | {sender, boolean()}
                 | sender
-                | {message_cb, false | diameter:evaluable()}
+                | {message_cb, false | diameter:eval()}
                 | {fragment_timer, 0..16#FFFFFFFF}.
 
 %% Accepting/connecting transport process state.
@@ -125,7 +125,7 @@
          timeout :: infinity | 0..16#FFFFFFFF,  %% fragment timeout
          tref = false  :: false | reference(),  %% fragment timer reference
          flush = false :: boolean(),            %% flush fragment at timeout?
-         message_cb  :: false | diameter:evaluable(),
+         message_cb  :: false | diameter:eval(),
          send        :: pid() | false}).         %% sending process
 
 %% The usual transport using gen_tcp can be replaced by anything