aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAnders Svensson <[email protected]>2011-09-27 11:41:33 +0200
committerAnders Svensson <[email protected]>2011-09-27 11:41:33 +0200
commit578d53ae4ae3ecc1e8f3d05ad78d16a6a19ea890 (patch)
treeec8bd0c751463f4a889ab79045cfe002df270f89
parent1e24333833f49e52ab3235148acf85cfd2c044b3 (diff)
parent7d38b593e8b1624e1ce2cda8fa57a42cdf2d3068 (diff)
downloadotp-578d53ae4ae3ecc1e8f3d05ad78d16a6a19ea890.tar.gz
otp-578d53ae4ae3ecc1e8f3d05ad78d16a6a19ea890.tar.bz2
otp-578d53ae4ae3ecc1e8f3d05ad78d16a6a19ea890.zip
Merge branch 'anders/diameter/peer_filters/OTP-9580' into dev
* anders/diameter/peer_filters/OTP-9580: Fix and clarify semantics of peer filters
-rw-r--r--lib/diameter/doc/src/diameter.xml37
-rw-r--r--lib/diameter/doc/src/diameter_app.xml7
-rw-r--r--lib/diameter/src/app/diameter_service.erl71
3 files changed, 72 insertions, 43 deletions
diff --git a/lib/diameter/doc/src/diameter.xml b/lib/diameter/doc/src/diameter.xml
index 36b6cbf0cf..2cad70e3bc 100644
--- a/lib/diameter/doc/src/diameter.xml
+++ b/lib/diameter/doc/src/diameter.xml
@@ -277,6 +277,10 @@ callback.</p>
</taglist>
+<p>
+An invalid option will cause <seealso marker="#call">call/4</seealso>
+to fail.</p>
+
<marker id="capability"/>
</item>
@@ -405,6 +409,8 @@ sense.</p>
<code>
eval([{M,F,A} | T]) ->
apply(M, F, T ++ A);
+eval([[F|A] | T]) ->
+ eval([F | T ++ A]);
eval([F|A]) ->
apply(F, A);
eval(F) ->
@@ -461,14 +467,14 @@ or any peer if the request does not contain
a <c>Destination-Realm</c> AVP.</p>
</item>
-<tag><c>{host, any|UTF8String()}</c></tag>
+<tag><c>{host, any|DiameterIdentity()}</c></tag>
<item>
<p>
Matches only those peers whose <c>Origin-Host</c> has the
specified value, or all peers if the atom <c>any</c>.</p>
</item>
-<tag><c>{realm, any|UTF8String()</c></tag>
+<tag><c>{realm, any|DiameterIdentity()</c></tag>
<item>
<p>
Matches only those peers whose <c>Origin-Realm</c> has the
@@ -478,8 +484,9 @@ value, or all peers if the atom <c>any</c>.</p>
<tag><c>{eval, evaluable()}</c></tag>
<item>
<p>
-Matches only those peers for which the specified evaluable() evaluates
-to true on the peer's <c>diameter_caps</c> record.</p>
+Matches only those peers for which the specified evaluable() returns
+<c>true</c> on the connection's <c>diameter_caps</c> record.
+Any other return value or exception is equivalent to <c>false</c>.</p>
</item>
<tag><c>{neg, peer_filter()}</c></tag>
@@ -503,6 +510,21 @@ specified list.</p>
</taglist>
+<p>
+Note that the <c>host</c> and <c>realm</c> filters examine the
+outgoing request as passed to <seealso marker="#call">call/4</seealso>,
+assuming that this is a record- or list-valued message() as documented
+in <seealso marker="diameter_app">diameter_app(3)</seealso>, and that
+the message contains at most one of each AVP.
+If this is not the case then the <c>{host|realm, DiameterIdentity()}</c>
+filters must be used to achieve the desired result.
+Note also that an empty host/realm (which should not be typical)
+is equivalent to an unspecified one for the purposes of filtering.</p>
+
+<p>
+An invalid filter is equivalent to <c>{any, []}</c>, a filter
+that matches no peer.</p>
+
<marker id="service_event"/>
</item>
@@ -787,7 +809,7 @@ transports.</p>
<type>
<v>SvcName = service_name()</v>
<v>App = application_alias()</v>
-<v>Request = diameter_app:message()</v>
+<v>Request = diameter_app:message() | term()</v>
<v>Answer = term()</v>
<v>Options = [call_opt()]</v>
</type>
@@ -819,9 +841,8 @@ If there are no suitable peers, or if
<seealso marker="diameter_app#pick_peer">pick_peer/4</seealso>
rejects them by returning 'false', then <c>{error, no_connection}</c>
is returned.
-If <seealso marker="diameter_app#pick_peer">pick_peer/4</seealso>
-selects a candidate peer then a request process is spawned for the
-outgoing request, in which there is a
+Otherwise <seealso marker="diameter_app#pick_peer">pick_peer/4</seealso>
+is followed by a
<seealso
marker="diameter_app#prepare_request">prepare_request/3</seealso>
callback, the message is encoded and sent.</p>
diff --git a/lib/diameter/doc/src/diameter_app.xml b/lib/diameter/doc/src/diameter_app.xml
index fc359b9d1d..f2bada84ae 100644
--- a/lib/diameter/doc/src/diameter_app.xml
+++ b/lib/diameter/doc/src/diameter_app.xml
@@ -269,7 +269,12 @@ The candidate peers list will only include those
which are selected by any <c>filter</c> option specified in the call to
<seealso marker="diameter#call">diameter:call/4</seealso>, and only
those which have indicated support for the Diameter application in
-question.</p>
+question.
+The order of the elements is unspecified except that any
+peers whose Origin-Host and Origin-Realm matches that of the
+outgoing request (in the sense of a <c>{filter, {all, [host, realm]}}</c>
+option to <seealso marker="diameter#call">diameter:call/4</seealso>)
+will be placed at the head of the list.</p>
<p>
The return values <c>false</c> and <c>{false, State}</c> are
diff --git a/lib/diameter/src/app/diameter_service.erl b/lib/diameter/src/app/diameter_service.erl
index a1cf1f6405..0569d595b9 100644
--- a/lib/diameter/src/app/diameter_service.erl
+++ b/lib/diameter/src/app/diameter_service.erl
@@ -1401,15 +1401,15 @@ recv_answer(Timeout,
%% is, from the last peer to which we've transmitted.
receive
- {answer = A, Ref, Rq, Pkt} -> %% Answer from peer.
+ {answer = A, Ref, Rq, Pkt} -> %% Answer from peer
{A, Rq, Pkt};
- {timeout = Reason, TRef, _} -> %% No timely reply
+ {timeout = Reason, TRef, _} -> %% No timely reply
{error, Req, Reason};
- {failover = Reason, TRef, false} -> %% No alternative peer.
+ {failover = Reason, TRef, false} -> %% No alternate peer
{error, Req, Reason};
- {failover, TRef, Transport} -> %% Resend to alternate peer.
+ {failover, TRef, Transport} -> %% Resend to alternate peer
try_retransmit(Timeout, SvcName, Req, Transport);
- {failover, TRef} -> %% May have missed failover notification.
+ {failover, TRef} -> %% May have missed failover notification
Seqs = diameter_codec:sequence_numbers(RPkt),
Pid = whois(SvcName),
is_pid(Pid) andalso (Pid ! {failover, TRef, Seqs}),
@@ -2471,6 +2471,7 @@ rpd(Pid, Alias, PDict) ->
%%%
%%% Output: {TransportPid, #diameter_caps{}, #diameter_app{}}
%%% | false
+%%% | {error, Reason}
%%% ---------------------------------------------------------------------------
%% Initial call, from an arbitrary process.
@@ -2529,22 +2530,12 @@ get_destination(Msg, Dict) ->
[str(get_avp_value(Dict, 'Destination-Realm', Msg)),
str(get_avp_value(Dict, 'Destination-Host', Msg))].
-%% TODO:
-%%
-%% Should add some way of specifying destination directly so that the
-%% only requirement is that the prepare_request callback returns
-%% something specific. (eg. {host, DH}; that is, let the caller specify.)
-%%
-%% Also, there is no longer any need to call get_destination at all in
-%% the default case.
-
-str(T)
- when T == undefined;
- T == [] ->
+%% This is not entirely correct. The avp could have an arity 1, in
+%% which case an empty list is a DiameterIdentity of length 0 rather
+%% than the list of no values we treat it as by mapping to undefined.
+%% This behaviour is documented.
+str([]) ->
undefined;
-str([X])
- when is_list(X) ->
- X;
str(T) ->
T.
@@ -2679,7 +2670,8 @@ peers(Alias, RH, Filter, Peers) ->
end.
%% Place a peer whose Destination-Host/Realm matches those of the
-%% request at the front of the result list.
+%% request at the front of the result list. Could add some sort of
+%% 'sort' option to allow more control.
ps([], _, _, {Ys, Ns}) ->
lists:reverse(Ys, Ns);
@@ -2689,11 +2681,11 @@ ps([{_TPid, #diameter_caps{} = Caps} = TC | Rest], RH, Filter, Acc) ->
TC,
Acc)).
-pacc(true, true, TC, {Ts, Fs}) ->
- {[TC|Ts], Fs};
-pacc(true, false, TC, {Ts, Fs}) ->
- {Ts, [TC|Fs]};
-pacc(false, _, _, Acc) ->
+pacc(true, true, Peer, {Ts, Fs}) ->
+ {[Peer|Ts], Fs};
+pacc(true, false, Peer, {Ts, Fs}) ->
+ {Ts, [Peer|Fs]};
+pacc(_, _, _, Acc) ->
Acc.
%% caps_filter/3
@@ -2701,17 +2693,19 @@ pacc(false, _, _, Acc) ->
caps_filter(C, RH, {neg, F}) ->
not caps_filter(C, RH, F);
-caps_filter(C, RH, {all, L}) ->
+caps_filter(C, RH, {all, L})
+ when is_list(L) ->
lists:all(fun(F) -> caps_filter(C, RH, F) end, L);
-caps_filter(C, RH, {any, L}) ->
+caps_filter(C, RH, {any, L})
+ when is_list(L) ->
lists:any(fun(F) -> caps_filter(C, RH, F) end, L);
-caps_filter(#diameter_caps{origin_host = {_,H}}, [_,DH], host) ->
- eq(undefined, DH, H);
+caps_filter(#diameter_caps{origin_host = {_,OH}}, [_,DH], host) ->
+ eq(undefined, DH, OH);
-caps_filter(#diameter_caps{origin_realm = {_,R}}, [DR,_], realm) ->
- eq(undefined, DR, R);
+caps_filter(#diameter_caps{origin_realm = {_,OR}}, [DR,_], realm) ->
+ eq(undefined, DR, OR);
caps_filter(C, _, Filter) ->
caps_filter(C, Filter).
@@ -2727,6 +2721,9 @@ caps_filter(#diameter_caps{origin_host = {_,OH}}, {host, H}) ->
caps_filter(#diameter_caps{origin_realm = {_,OR}}, {realm, R}) ->
eq(any, R, OR);
+%% Anything else is expected to be an eval filter. Filter failure is
+%% documented as being equivalent to a non-matching filter.
+
caps_filter(C, T) ->
try
{eval, F} = T,
@@ -2735,8 +2732,14 @@ caps_filter(C, T) ->
_:_ -> false
end.
-eq(X, A, B) ->
- X == A orelse A == B.
+eq(Any, Id, PeerId) ->
+ Any == Id orelse try
+ iolist_to_binary(Id) == iolist_to_binary(PeerId)
+ catch
+ _:_ -> false
+ end.
+%% OctetString() can be specified as an iolist() so test for string
+%% rather then term equality.
%% transports/1