Merge branch 'anders/diameter/capabilities_cb/OTP-9654'

* anders/diameter/capabilities_cb/OTP-9654:
  Update documentation
  Add capx suite
  Accept any 2xxx result code in CEA
  Rename some functions plus comment tweak
  Send events for connection establishment failure
  Use trivial capabilities callback in traffic suite
  Add capabilities_cb transport option
  Minor tls_ack simplification
  diameter_dbg:log -> diameter_lib:log

5 files changed, 272 insertions, 203 deletions

diff --git a/lib/diameter/src/base/diameter_capx.erl b/lib/diameter/src/base/diameter_capx.erl
index 138e76411e..842a9e6103 100644
--- a/lib/diameter/src/base/diameter_capx.erl
+++ b/lib/diameter/src/base/diameter_capx.erl
@@ -57,9 +57,9 @@
 -include("diameter_types.hrl").
 -include("diameter_gen_base_rfc3588.hrl").
 
--define(SUCCESS, ?'DIAMETER_BASE_RESULT-CODE_DIAMETER_SUCCESS').
--define(NOAPP, ?'DIAMETER_BASE_RESULT-CODE_DIAMETER_NO_COMMON_APPLICATION').
--define(NOSECURITY, ?'DIAMETER_BASE_RESULT-CODE_DIAMETER_NO_COMMON_SECURITY').
+-define(SUCCESS,    2001).  %% DIAMETER_SUCCESS
+-define(NOAPP,      5010).  %% DIAMETER_NO_COMMON_APPLICATION
+-define(NOSECURITY, 5017).  %% DIAMETER_NO_COMMON_SECURITY
 
 -define(NO_INBAND_SECURITY, 0).
 -define(TLS, 1).
@@ -96,7 +96,7 @@ try_it([Fun | Args]) ->
     try apply(Fun, Args) of
         T -> {ok, T}
     catch
-        throw: ?FAILURE(Reason) -> {error, {Reason, Args}}
+        throw: ?FAILURE(Reason) -> {error, Reason}
     end.
 
 %% mk_caps/2
@@ -224,10 +224,6 @@ rCER(CER, #diameter_service{capabilities = LCaps} = Svc) ->
                RCaps,
                CEA#diameter_base_CEA{'Result-Code' = ?SUCCESS})}.
 
-%% TODO: 5.3 of RFC 3588 says we MUST return DIAMETER_NO_COMMON_APPLICATION
-%%       in the CEA and SHOULD disconnect the transport. However, we have
-%%       no way to guarantee the send before disconnecting.
-
 build_CEA([], _, _, CEA) ->
     CEA#diameter_base_CEA{'Result-Code' = ?NOAPP};
 
@@ -292,25 +288,12 @@ to_cea(CER, Field, CEA) ->
         
 %% rCEA/2
 
-rCEA(#diameter_base_CEA{'Result-Code' = RC}
-     = CEA,
-     #diameter_service{capabilities = LCaps}
-     = Svc) ->
-    RC == ?SUCCESS orelse ?THROW({'Result-Code', RC}),
-
+rCEA(CEA, #diameter_service{capabilities = LCaps} = Svc) ->
     RCaps = capx_to_caps(CEA),
     SApps = common_applications(LCaps, RCaps, Svc),
-
-    [] == SApps andalso ?THROW(no_common_applications),
-
     IS = common_security(LCaps, RCaps),
 
-    [] == IS andalso ?THROW(no_common_security),
-
-    {SApps, IS, RCaps};
-
-rCEA(CEA, _Svc) ->
-    ?THROW({invalid, CEA}).
+    {SApps, IS, RCaps}.
 
 %% capx_to_caps/1
 
diff --git a/lib/diameter/src/base/diameter_codec.erl b/lib/diameter/src/base/diameter_codec.erl
index d88f42fb7c..fe1212b7e0 100644
--- a/lib/diameter/src/base/diameter_codec.erl
+++ b/lib/diameter/src/base/diameter_codec.erl
@@ -190,26 +190,13 @@ encode_avps(Avps) ->
 
 %% msg_header/3
 
-msg_header(Mod, MsgName, Header) ->
-    {Code, Flags, ApplId} = h(Mod, MsgName, Header),
-    {Code, p(Flags, Header), ApplId}.
-
-%% 6.2 of 3588 requires the same 'P' bit on an answer as on the
-%% request.
-
-p(Flags, #diameter_header{is_request = true,
-                          is_proxiable = P}) ->
-    Flags band (2#10110000 bor choose(P, 2#01000000, 0));
-p(Flags, _) ->
-    Flags.
-
-h(Mod, 'answer-message' = MsgName, Header) ->
+msg_header(Mod, 'answer-message' = MsgName, Header) ->
     ?BASE = Mod,
     #diameter_header{cmd_code = Code} = Header,
     {_, Flags, ApplId} = ?BASE:msg_header(MsgName),
     {Code, Flags, ApplId};
 
-h(Mod, MsgName, _) ->
+msg_header(Mod, MsgName, _) ->
     Mod:msg_header(MsgName).
 
 %% rec2msg/2
@@ -554,8 +541,3 @@ pack_avp(Code, Flags, Vid, Sz, Bin) ->
 pack_avp(Code, Flags, Sz, Bin) ->
     Length = Sz + 8,
     <<Code:32, Flags:8, Length:24, Bin/binary>>.
-
-%% ===========================================================================
-
-choose(true, X, _)  -> X;
-choose(false, _, X) -> X.
diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl
index 282fa2742f..fae5d763dc 100644
--- a/lib/diameter/src/base/diameter_peer_fsm.erl
+++ b/lib/diameter/src/base/diameter_peer_fsm.erl
@@ -55,7 +55,8 @@
 -define(NO_INBAND_SECURITY, 0).
 -define(TLS, 1).
 
--define(LOOP_TIMEOUT, 2000).
+%% A 2xxx series Result-Code. Not necessarily 2001.
+-define(IS_SUCCESS(N), 2 == (N) div 1000).
 
 %% RFC 3588:
 %%
@@ -142,9 +143,12 @@ init(T) ->
     proc_lib:init_ack({ok, self()}),
     gen_server:enter_loop(?MODULE, [], i(T)).
 
-i({WPid, {M, _} = T, Opts, #diameter_service{capabilities = Caps} = Svc0}) ->
+i({WPid, T, Opts, #diameter_service{capabilities = Caps} = Svc0}) ->
     putr(dwa, dwa(Caps)),
-    {ok, TPid, Svc} = start_transport(T, Opts, Svc0),
+    {M, Ref} = T,
+    {[Ts], Rest} = proplists:split(Opts, [capabilities_cb]),
+    putr(capabilities_cb, {Ref, [F || {_,F} <- Ts]}),
+    {ok, TPid, Svc} = start_transport(T, Rest, Svc0),
     erlang:monitor(process, TPid),
     erlang:monitor(process, WPid),
     #state{parent = WPid,
@@ -198,10 +202,13 @@ handle_info(T, #state{} = State) ->
             ?LOG(stop, T),
             x(T, State)
     catch
-        throw: {?MODULE, Tag, Reason}  ->
+        {?MODULE, Tag, Reason}  ->
             ?LOG(Tag, {Reason, T}),
             {stop, {shutdown, Reason}, State}
     end.
+%% The form of the exception caught here is historical. It's
+%% significant that it's not a 2-tuple, as in ?FAILURE(Reason),
+%% since these are caught elsewhere.
 
 x(Reason, #state{} = S) ->
     close_wd(Reason, S),
@@ -226,6 +233,9 @@ putr(Key, Val) ->
 getr(Key) ->
     get({?MODULE, Key}).
 
+eraser(Key) ->
+    erase({?MODULE, Key}).
+
 %% transition/2
 
 %% Connection to peer.
@@ -316,9 +326,10 @@ send_CER(#state{mode = {connect, Remote},
                 service = #diameter_service{capabilities = Caps},
                 transport = TPid}
          = S) ->
-    req_send_CER(Caps#diameter_caps.origin_host, Remote)
+    OH = Caps#diameter_caps.origin_host,
+    req_send_CER(OH, Remote)
         orelse
-        close(connected, S),
+        close({already_connected, Remote, Caps}, S),
     CER = build_CER(S),
     ?LOG(send, 'CER'),
     send(TPid, encode(CER)),
@@ -460,20 +471,20 @@ handle_request(Type, #diameter_packet{} = Pkt, S) ->
 %% send_answer/3
 
 send_answer(Type, ReqPkt, #state{transport = TPid} = S) ->
-    #diameter_packet{header = #diameter_header{version = V,
-                                               end_to_end_id = Eid,
-                                               hop_by_hop_id = Hid,
-                                               is_proxiable = P},
+    #diameter_packet{header = H,
                      transport_data = TD}
         = ReqPkt,
 
-    {Answer, PostF} = build_answer(Type, V, ReqPkt, S),
+    {Msg, PostF} = build_answer(Type, ReqPkt, S),
 
-    Pkt = #diameter_packet{header = #diameter_header{version = V,
-                                                     end_to_end_id = Eid,
-                                                     hop_by_hop_id = Hid,
-                                                     is_proxiable = P},
-                           msg = Answer,
+    %% An answer message clears the R and T flags and retains the P
+    %% flag. The E flag is set at encode.
+    Pkt = #diameter_packet{header
+                           = H#diameter_header{version = ?DIAMETER_VERSION,
+                                               is_request = false,
+                                               is_error = undefined,
+                                               is_retransmitted = false},
+                           msg = Msg,
                            transport_data = TD},
 
     send(TPid, diameter_codec:encode(?BASE, Pkt)),
@@ -484,56 +495,104 @@ eval([F|A], S) ->
 eval(ok, S) ->
     S.
 
-%% build_answer/4
+%% build_answer/3
 
 build_answer('CER',
-             ?DIAMETER_VERSION,
              #diameter_packet{msg = CER,
-                              header = #diameter_header{is_error = false},
+                              header = #diameter_header{version
+                                                        = ?DIAMETER_VERSION,
+                                                        is_error = false},
                               errors = []}
              = Pkt,
-             #state{service = Svc}
-             = S) ->
-    #diameter_service{capabilities = #diameter_caps{origin_host = OH}}
-        = Svc,
-
-    {SupportedApps,
-     #diameter_caps{origin_host = DH} = RCaps,
-     #diameter_base_CEA{'Result-Code' = RC}
-     = CEA}
+             S) ->
+    {SupportedApps, RCaps, #diameter_base_CEA{'Result-Code' = RC,
+                                              'Inband-Security-Id' = IS}
+                           = CEA}
         = recv_CER(CER, S),
 
+    #diameter_caps{origin_host = {OH, DH}}
+        = Caps
+        = capz(caps(S), RCaps),
+
     try
         2001 == RC  %% DIAMETER_SUCCESS
-            orelse ?THROW({sent_CEA, RC}),
+            orelse ?THROW(RC),
         register_everywhere({?MODULE, connection, OH, DH})
-            orelse ?THROW({election_lost, 4003}),
-        #diameter_base_CEA{'Inband-Security-Id' = [IS]}
-            = CEA,
-        {CEA, [fun open/5, Pkt, SupportedApps, RCaps, {accept, IS}]}
+            orelse ?THROW(4003),  %% DIAMETER_ELECTION_LOST
+        caps_cb(Caps)
+    of
+        N -> {cea(CEA, N), [fun open/5, Pkt,
+                                        SupportedApps,
+                                        Caps,
+                                        {accept, hd([_] = IS)}]}
     catch
-        ?FAILURE({Reason, RC}) ->
-            {answer('CER', S) ++ [{'Result-Code', RC}],
-             [fun close/2, {'CER', Reason, DH}]}
+        ?FAILURE(Reason) ->
+            rejected(Reason, {'CER', Reason, Caps, Pkt}, S)
     end;
 
 %% The error checks below are similar to those in diameter_service for
 %% other messages. Should factor out the commonality.
 
-build_answer(Type, V, #diameter_packet{header = H, errors = Es} = Pkt, S) ->
-    FailedAvp = failed_avp([A || {_,A} <- Es]),
-    Ans = answer(answer(Type, S), V, H, Es),
-    {set(Ans, FailedAvp), if 'CER' == Type ->
-                                  [fun close/2, {Type, V, Pkt}];
-                             true ->
-                                  ok
-                          end}.
+build_answer(Type,
+             #diameter_packet{header = H,
+                              errors = Es}
+             = Pkt,
+             S) ->
+    RC = rc(H, Es),
+    {answer(Type, RC, Es, S), post(Type, RC, Pkt, S)}.
+
+cea(CEA, ok) ->
+    CEA;
+cea(CEA, 2001) ->
+    CEA;
+cea(CEA, RC) ->
+    CEA#diameter_base_CEA{'Result-Code' = RC}.
+
+post('CER' = T, RC, Pkt, S) ->
+    [fun close/2, {T, caps(S), {RC, Pkt}}];
+post(_, _, _, _) ->
+    ok.
+
+rejected({capabilities_cb, _F, Reason}, T, S) ->
+    rejected(Reason, T, S);
+
+rejected(discard, T, S) ->
+    close(T, S);
+rejected({N, Es}, T, S) ->
+    {answer('CER', N, Es, S), [fun close/2, T]};
+rejected(N, T, S) ->
+    rejected({N, []}, T, S).
+
+answer(Type, RC, Es, S) ->
+    set(answer(Type, RC, S), failed_avp([A || {_,A} <- Es])).
+
+answer(Type, RC, S) ->
+    answer_message(answer(Type, S), RC).
 
+%% answer_message/2
+
+answer_message([_ | Avps], RC)
+  when 3000 =< RC, RC < 4000 ->
+    ['answer-message', {'Result-Code', RC}
+                     | lists:filter(fun is_origin/1, Avps)];
+
+answer_message(Msg, RC) ->
+    Msg ++ [{'Result-Code', RC}].
+
+is_origin({N, _}) ->
+    N == 'Origin-Host'
+        orelse N == 'Origin-Realm'
+        orelse N == 'Origin-State-Id'.
+
+%% failed_avp/1
+    
 failed_avp([] = No) ->
     No;
 failed_avp(Avps) ->
     [{'Failed-AVP', [[{'AVP', Avps}]]}].
 
+%% set/2
+
 set(Ans, []) ->
     Ans;
 set(['answer-message' | _] = Ans, FailedAvp) ->
@@ -541,18 +600,22 @@ set(['answer-message' | _] = Ans, FailedAvp) ->
 set([_|_] = Ans, FailedAvp) ->
     Ans ++ FailedAvp.
 
-answer([_, OH, OR | _], _, #diameter_header{is_error = true}, _) ->
-    ['answer-message', OH, OR, {'Result-Code', 3008}];
+%% rc/2
+
+rc(#diameter_header{is_error = true}, _) ->
+    3008;  %% DIAMETER_INVALID_HDR_BITS
 
-answer([_, OH, OR | _], _, _, [Bs|_])
+rc(_, [Bs|_])
   when is_bitstring(Bs) ->
-    ['answer-message', OH, OR, {'Result-Code', 3009}];
+    3009;  %% DIAMETER_INVALID_HDR_BITS
 
-answer(Ans, ?DIAMETER_VERSION, _, Es) ->
-    Ans ++ [{'Result-Code', rc(Es)}];
+rc(#diameter_header{version = ?DIAMETER_VERSION}, Es) ->
+    rc(Es);
 
-answer(Ans, _, _, _) ->
-    Ans ++ [{'Result-Code', 5011}].  %% DIAMETER_UNSUPPORTED_VERSION
+rc(_, _) ->
+    5011.  %% DIAMETER_UNSUPPORTED_VERSION
+
+%% rc/1
 
 rc([]) ->
     2001;  %% DIAMETER_SUCCESS
@@ -595,12 +658,14 @@ a('CER', #diameter_caps{vendor_id = Vid,
                         origin_host = Host,
                         origin_realm = Realm,
                         host_ip_address = Addrs,
-                        product_name = Name}) ->
+                        product_name = Name,
+                        origin_state_id = OSI}) ->
     ['CEA', {'Origin-Host', Host},
             {'Origin-Realm', Realm},
             {'Host-IP-Address', Addrs},
             {'Vendor-Id', Vid},
-            {'Product-Name', Name}];
+            {'Product-Name', Name},
+            {'Origin-State-Id', OSI}];
 
 a('DPR', #diameter_caps{origin_host = Host,
                         origin_realm = Realm}) ->
@@ -615,27 +680,25 @@ recv_CER(CER, #state{service = Svc}) ->
 
 %% handle_CEA/1
 
-handle_CEA(#diameter_packet{header = #diameter_header{version = V},
-                            bin = Bin}
+handle_CEA(#diameter_packet{bin = Bin}
            = Pkt,
            #state{service = #diameter_service{capabilities = LCaps}}
            = S)
   when is_binary(Bin) ->
     ?LOG(recv, 'CEA'),
 
-    ?DIAMETER_VERSION == V orelse close({version, V}, S),
-
-    #diameter_packet{msg = CEA, errors = Errors}
+    #diameter_packet{msg = CEA}
         = DPkt
         = diameter_codec:decode(?BASE, Pkt),
 
-    [] == Errors orelse close({errors, Errors}, S),
+    {SApps, IS, RCaps} = recv_CEA(DPkt, S),
 
-    {SApps, [IS], #diameter_caps{origin_host = DH} = RCaps}
-        = recv_CEA(CEA, S),
+    #diameter_caps{origin_host = {OH, DH}}
+        = Caps
+        = capz(LCaps, RCaps),
 
-    #diameter_caps{origin_host = OH}
-        = LCaps,
+    #diameter_base_CEA{'Result-Code' = RC}
+        = CEA,
 
     %% Ensure that we don't already have a connection to the peer in
     %% question. This isn't the peer election of 3588 except in the
@@ -643,60 +706,101 @@ handle_CEA(#diameter_packet{header = #diameter_header{version = V},
     %% receive a CER/CEA, the first that arrives wins the right to a
     %% connection with the peer.
 
-    register_everywhere({?MODULE, connection, OH, DH})
-        orelse close({'CEA', DH}, S),
-
-    open(DPkt, SApps, RCaps, {connect, IS}, S).
+    try
+        ?IS_SUCCESS(RC)
+            orelse ?THROW(RC),
+        [] == SApps
+            andalso ?THROW(no_common_application),
+        [] == IS
+            andalso ?THROW(no_common_security),
+        register_everywhere({?MODULE, connection, OH, DH})
+            orelse ?THROW(election_lost),
+        caps_cb(Caps)
+    of
+        _ -> open(DPkt, SApps, Caps, {connect, hd([_] = IS)}, S)
+    catch
+        ?FAILURE(Reason) -> close({'CEA', Reason, Caps, DPkt}, S)
+    end.
+%% Check more than the result code since the peer could send success
+%% regardless. If not 2001 then a peer_up callback could do anything
+%% required. It's not unimaginable that a peer agreeing to TLS after
+%% capabilities exchange could send DIAMETER_LIMITED_SUCCESS = 2002,
+%% even if this isn't required by RFC 3588.
 
 %% recv_CEA/2
 
-recv_CEA(CEA, #state{service = Svc} = S) ->
-    case diameter_capx:recv_CEA(CEA, Svc) of
-        {ok, {_,_}} -> %% return from old code
-            close({'CEA', update}, S);
-        {ok, {[], _, _}} ->
-            close({'CEA', no_common_application}, S);
-        {ok, {_, [], _}} ->
-            close({'CEA', no_common_security}, S);
-        {ok, {_,_,_} = T} ->
-            T;
-        {error, Reason} ->
-            close({'CEA', Reason}, S)
+recv_CEA(#diameter_packet{header = #diameter_header{version
+                                                    = ?DIAMETER_VERSION,
+                                                    is_error = false},
+                          msg = CEA,
+                          errors = []},
+         #state{service = Svc}) ->
+    {ok, T} = diameter_capx:recv_CEA(CEA, Svc),
+    T;
+
+recv_CEA(Pkt, S) ->
+    close({'CEA', caps(S), Pkt}, S).
+
+caps(#diameter_service{capabilities = Caps}) ->
+    Caps;
+caps(#state{service = Svc}) ->
+    caps(Svc).
+
+%% caps_cb/1
+
+caps_cb(Caps) ->
+    {Ref, Ts} = eraser(capabilities_cb),
+    ccb(Ts, [Ref, Caps]).
+
+ccb([], _) ->
+    ok;
+ccb([F | Rest], T) ->
+    case diameter_lib:eval([F|T]) of
+        ok ->
+            ccb(Rest, T);
+        N when ?IS_SUCCESS(N) ->  %% 2xxx result code: accept immediately
+            N;
+        Res ->
+            ?THROW({capabilities_cb, F, rejected(Res)})
     end.
+%% Note that returning 2xxx causes the capabilities exchange to be
+%% accepted directly, without further callbacks.
+
+rejected(discard = T) ->
+    T;
+rejected(unknown) ->
+    3010;  %% DIAMETER_UNKNOWN_PEER
+rejected(N)
+  when is_integer(N) ->
+    N.
 
 %% open/5
 
-open(Pkt, SupportedApps, RCaps, {Type, IS}, #state{parent = Pid,
-                                                   service = Svc}
-                                            = S) ->
-    #diameter_service{capabilities = #diameter_caps{origin_host = OH,
-                                                    inband_security_id = LS}
-                                   = LCaps}
-        = Svc,
-    #diameter_caps{origin_host = DH}
-        = RCaps,
+open(Pkt, SupportedApps, Caps, {Type, IS}, #state{parent = Pid} = S) ->
+    #diameter_caps{origin_host = {_,_} = H,
+                   inband_security_id = {LS,_}}
+        = Caps,
 
-    tls_ack(lists:member(?TLS, LS), Type, IS, S),
-    Pid ! {open, self(), {OH,DH}, {capz(LCaps, RCaps), SupportedApps, Pkt}},
+    tls_ack(lists:member(?TLS, LS), Caps, Type, IS, S),
+    Pid ! {open, self(), H, {Caps, SupportedApps, Pkt}},
 
     S#state{state = 'Open'}.
 
 %% We've advertised TLS support: tell the transport the result
 %% and expect a reply when the handshake is complete.
-tls_ack(true, Type, IS, #state{transport = TPid} = S) ->
+tls_ack(true, Caps, Type, IS, #state{transport = TPid} = S) ->
     Ref = make_ref(),
-    MRef = erlang:monitor(process, TPid),
     TPid ! {diameter, {tls, Ref, Type, IS == ?TLS}},
     receive
         {diameter, {tls, Ref}} ->
-            erlang:demonitor(MRef, [flush]);
-        {'DOWN', MRef, process, _, _} = T ->
-            close({tls_ack, T}, S)
+            ok;
+        {'DOWN', _, process, TPid, Reason} ->
+            close({tls_ack, Reason, Caps}, S)
     end;
 
 %% Or not. Don't send anything to the transport so that transports
 %% not supporting TLS work as before without modification.
-tls_ack(false, _, _, _) ->
+tls_ack(false, _, _, _, _) ->
     ok.
 
 capz(#diameter_caps{} = L, #diameter_caps{} = R) ->
diff --git a/lib/diameter/src/base/diameter_service.erl b/lib/diameter/src/base/diameter_service.erl
index 421e36ccf5..7adcf1c265 100644
--- a/lib/diameter/src/base/diameter_service.erl
+++ b/lib/diameter/src/base/diameter_service.erl
@@ -983,7 +983,8 @@ peer_cb(MFA, Alias) ->
 connection_down(Pid, #state{peerT = PeerT,
                             connT = ConnT}
                      = S) ->
-    #peer{conn = TPid}
+    #peer{op_state = ?STATE_UP,  %% assert
+          conn = TPid}
         = P
         = fetch(PeerT, Pid),
 
@@ -993,6 +994,9 @@ connection_down(Pid, #state{peerT = PeerT,
 
 %% connection_down/3
 
+connection_down(#peer{op_state = ?STATE_DOWN}, _, S) ->
+    S;
+
 connection_down(#peer{conn = TPid,
                       op_state = ?STATE_UP}
                 = P,
@@ -1034,13 +1038,23 @@ down_conn(Id, Alias, TC, {SvcName, Apps}) ->
 
 %% Peer process has died.
 
-peer_down(Pid, _Reason, #state{peerT = PeerT} = S) ->
+peer_down(Pid, Reason, #state{peerT = PeerT} = S) ->
     P = fetch(PeerT, Pid),
     ets:delete_object(PeerT, P),
+    closed(Reason, P, S),
     restart(P,S),
     peer_down(P,S).
 
-%% peer_down/2
+%% Send an event at connection establishment failure.
+closed({shutdown, {close, _TPid, Reason}},
+       #peer{op_state = ?STATE_DOWN,
+             ref = Ref,
+             type = Type,
+             options = Opts},
+       #state{service_name = SvcName}) ->
+    send_event(SvcName, {closed, Ref, Reason, {type(Type), Opts}});
+closed(_, _, _) ->
+    ok.
 
 %% The peer has never come up ...
 peer_down(#peer{conn = B}, S)
@@ -1048,27 +1062,9 @@ peer_down(#peer{conn = B}, S)
     S;
 
 %% ... or it has.
-peer_down(#peer{ref = Ref,
-                conn = TPid,
-                type = Type,
-                options = Opts}
-          = P,
-          #state{service_name = SvcName,
-                 connT = ConnT}
-          = S) ->
-    #conn{caps = Caps}
-        = C
-        = fetch(ConnT, TPid),
+peer_down(#peer{conn = TPid} = P, #state{connT = ConnT} = S) ->
+    #conn{} = C = fetch(ConnT, TPid),
     ets:delete_object(ConnT, C),
-    try
-        pd(P,C,S)
-    after
-        send_event(SvcName, {closed, Ref, {TPid, Caps}, {type(Type), Opts}})
-    end.
-
-pd(#peer{op_state = ?STATE_DOWN}, _, S) ->
-    S;
-pd(#peer{op_state = ?STATE_UP} = P, C, S) ->
     connection_down(P,C,S).
 
 %% restart/2
@@ -1259,11 +1255,11 @@ send_request({TPid, Caps, App}, Msg, Opts, Caller, SvcName) ->
     #diameter_app{module = ModX}
         = App,
 
-    Pkt = make_packet(Msg),
+    Pkt = make_request_packet(Msg),
 
     case cb(ModX, prepare_request, [Pkt, SvcName, {TPid, Caps}]) of
         {send, P} ->
-            send_request(make_packet(P, Pkt),
+            send_request(make_request_packet(P, Pkt),
                          TPid,
                          Caps,
                          App,
@@ -1278,70 +1274,73 @@ send_request({TPid, Caps, App}, Msg, Opts, Caller, SvcName) ->
             ?ERROR({invalid_return, prepare_request, App, T})
     end.
 
-%% make_packet/1
+%% make_request_packet/1
 %%
 %% Turn an outgoing request as passed to call/4 into a diameter_packet
 %% record in preparation for a prepare_request callback.
 
-make_packet(Bin)
+make_request_packet(Bin)
   when is_binary(Bin) ->
     #diameter_packet{header = diameter_codec:decode_header(Bin),
                      bin = Bin};
 
-make_packet(#diameter_packet{msg = [#diameter_header{} = Hdr | Avps]} = Pkt) ->
-    Pkt#diameter_packet{msg = [make_header(Hdr) | Avps]};
+make_request_packet(#diameter_packet{msg = [#diameter_header{} = Hdr | Avps]}
+                    = Pkt) ->
+    Pkt#diameter_packet{msg = [make_request_header(Hdr) | Avps]};
 
-make_packet(#diameter_packet{header = Hdr} = Pkt) ->
-    Pkt#diameter_packet{header = make_header(Hdr)};
+make_request_packet(#diameter_packet{header = Hdr} = Pkt) ->
+    Pkt#diameter_packet{header = make_request_header(Hdr)};
 
-make_packet(Msg) ->
-    make_packet(#diameter_packet{msg = Msg}).
+make_request_packet(Msg) ->
+    make_request_packet(#diameter_packet{msg = Msg}).
 
-%% make_header/1
+%% make_request_header/1
 
-make_header(undefined) ->
+make_request_header(undefined) ->
     Seq = diameter_session:sequence(),
-    make_header(#diameter_header{end_to_end_id = Seq,
-                                 hop_by_hop_id = Seq});
+    make_request_header(#diameter_header{end_to_end_id = Seq,
+                                         hop_by_hop_id = Seq});
 
-make_header(#diameter_header{version = undefined} = Hdr) ->
-    make_header(Hdr#diameter_header{version = ?DIAMETER_VERSION});
+make_request_header(#diameter_header{version = undefined} = Hdr) ->
+    make_request_header(Hdr#diameter_header{version = ?DIAMETER_VERSION});
 
-make_header(#diameter_header{end_to_end_id = undefined} = H) ->
+make_request_header(#diameter_header{end_to_end_id = undefined} = H) ->
     Seq = diameter_session:sequence(),
-    make_header(H#diameter_header{end_to_end_id = Seq});
+    make_request_header(H#diameter_header{end_to_end_id = Seq});
 
-make_header(#diameter_header{hop_by_hop_id = undefined} = H) ->
+make_request_header(#diameter_header{hop_by_hop_id = undefined} = H) ->
     Seq = diameter_session:sequence(),
-    make_header(H#diameter_header{hop_by_hop_id = Seq});
+    make_request_header(H#diameter_header{hop_by_hop_id = Seq});
 
-make_header(#diameter_header{} = Hdr) ->
+make_request_header(#diameter_header{} = Hdr) ->
     Hdr;
 
-make_header(T) ->
+make_request_header(T) ->
     ?ERROR({invalid_header, T}).
 
-%% make_packet/2
+%% make_request_packet/2
 %%
 %% Reconstruct a diameter_packet from the return value of
 %% prepare_request or prepare_retransmit callback.
 
-make_packet(Bin, _)
+make_request_packet(Bin, _)
   when is_binary(Bin) ->
-    make_packet(Bin);
+    make_request_packet(Bin);
 
-make_packet(#diameter_packet{msg = [#diameter_header{} | _]} = Pkt, _) ->
+make_request_packet(#diameter_packet{msg = [#diameter_header{} | _]}
+                    = Pkt,
+                    _) ->
     Pkt;
 
 %% Returning a diameter_packet with no header from a prepare_request
 %% or prepare_retransmit callback retains the header passed into it.
 %% This is primarily so that the end to end and hop by hop identifiers
 %% are retained.
-make_packet(#diameter_packet{header = Hdr} = Pkt,
+make_request_packet(#diameter_packet{header = Hdr} = Pkt,
             #diameter_packet{header = Hdr0}) ->
     Pkt#diameter_packet{header = fold_record(Hdr0, Hdr)};
 
-make_packet(Msg, Pkt) ->
+make_request_packet(Msg, Pkt) ->
     Pkt#diameter_packet{msg = Msg}.
 
 %% fold_record/2
@@ -1533,7 +1532,7 @@ retransmit({TPid, Caps, #diameter_app{alias = Alias} = App},
 
     case cb(App, prepare_retransmit, [Pkt, SvcName, {TPid, Caps}]) of
         {send, P} ->
-            retransmit(make_packet(P, Pkt), TPid, Caps, Req, Timeout);
+            retransmit(make_request_packet(P, Pkt), TPid, Caps, Req, Timeout);
         {discard, Reason} ->
             ?THROW(Reason);
         discard ->
@@ -1946,7 +1945,7 @@ reply(Msg, Dict, TPid, #diameter_packet{errors = Es,
                        = ReqPkt)
   when [] == Es;
        is_record(hd(Msg), diameter_header) ->
-    Pkt = diameter_codec:encode(Dict, make_reply_packet(Msg, ReqPkt)),
+    Pkt = diameter_codec:encode(Dict, make_answer_packet(Msg, ReqPkt)),
     incr(send, Pkt, Dict, TPid),  %% count result codes in sent answers
     send(TPid, Pkt#diameter_packet{transport_data = TD});
 
@@ -1957,18 +1956,19 @@ reply(Msg, Dict, TPid, #diameter_packet{errors = [H|_] = Es} = Pkt) ->
           TPid,
           Pkt#diameter_packet{errors = []}).
 
-%% make_reply_packet/2
+%% make_answer_packet/2
 
 %% Binaries and header/avp lists are sent as-is.
-make_reply_packet(Bin, _)
+make_answer_packet(Bin, _)
   when is_binary(Bin) ->
     #diameter_packet{bin = Bin};
-make_reply_packet([#diameter_header{} | _] = Msg, _) ->
+make_answer_packet([#diameter_header{} | _] = Msg, _) ->
     #diameter_packet{msg = Msg};
 
 %% Otherwise a reply message clears the R and T flags and retains the
-%% P flag. The E flag will be set at encode.
-make_reply_packet(Msg, #diameter_packet{header = ReqHdr}) ->
+%% P flag. The E flag will be set at encode. 6.2 of 3588 requires the
+%% same P flag on an answer as on the request.
+make_answer_packet(Msg, #diameter_packet{header = ReqHdr}) ->
     Hdr = ReqHdr#diameter_header{version = ?DIAMETER_VERSION,
                                  is_request = false,
                                  is_error = undefined,
diff --git a/lib/diameter/src/base/diameter_watchdog.erl b/lib/diameter/src/base/diameter_watchdog.erl
index b7c1491f4b..6dc53d9f31 100644
--- a/lib/diameter/src/base/diameter_watchdog.erl
+++ b/lib/diameter/src/base/diameter_watchdog.erl
@@ -179,11 +179,11 @@ transition({close, TPid, _Reason}, #watchdog{transport = TPid}) ->
 %% state okay as the result of the Peer State Machine reaching the
 %% Open state.
 %%
-%% If we're an acceptor then we may be resuming a connection that went
-%% down in another acceptor process, in which case this is the
-%% transition below, from down into reopen. That is, it's not until
-%% we know the identity of the peer (ie. now) that we know that we're
-%% in state down rather than initial.
+%% If we're accepting then we may be resuming a connection that went
+%% down in another watchdog process, in which case this is the
+%% transition below, from down to reopen. That is, it's not until we
+%% know the identity of the peer (ie. now) that we know that we're in
+%% state down rather than initial.
 
 transition({open, TPid, Hosts, T} = Open,
            #watchdog{transport = TPid,