discovery: add a unique local preference value per turn server

This value is built from the position in the component turn servers
list, and from the base address network interface position in the list
of network interfaces. This value is used to ensure a unique candidate
priority for each one. Also ensure that the fields that compose the
local preference don't overlap, by checking their maximum value.  See
RFC-8445, section 5.1.2.2 "Guidelines for Choosing Type and Local
Preferences".

diff --git a/agent/agent.c b/agent/agent.c
index f7f0d73..ed0d6b8 100644 (file)
--- a/agent/agent.c
+++ b/agent/agent.c
@@ -2820,6 +2820,7 @@ nice_agent_set_relay_info(NiceAgent *agent,
   NiceStream *stream = NULL;
   gboolean ret = TRUE;
   TurnServer *turn;
+  guint length;
 
   g_return_val_if_fail (NICE_IS_AGENT (agent), FALSE);
   g_return_val_if_fail (stream_id >= 1, FALSE);
@@ -2838,6 +2839,14 @@ nice_agent_set_relay_info(NiceAgent *agent,
     goto done;
   }
 
+  length = g_list_length (component->turn_servers);
+  if (length == NICE_CANDIDATE_MAX_TURN_SERVERS) {
+    nice_debug ("Agent %p : cannot have more than %d turn servers.",
+        agent, length);
+    ret = FALSE;
+    goto done;
+  }
+
   turn = turn_server_new (server_ip, server_port, username, password, type);
 
   if (!turn) {
@@ -2850,6 +2859,11 @@ nice_agent_set_relay_info(NiceAgent *agent,
       stream_id, component_id, username,
       nice_debug_is_verbose() ? password : "****");
 
+  /* The turn server preference (used to setup its priority in the
+   * conncheck) is simply its position in the list. The preference must
+   * be unique for each one.
+   */
+  turn->preference = length;
   component->turn_servers = g_list_append (component->turn_servers, turn);
 
  if (stream->gathering_started) {
@@ -3034,6 +3048,7 @@ nice_agent_gather_candidates (
   NiceStream *stream;
   GSList *local_addresses = NULL;
   gboolean ret = TRUE;
+  guint length;
 
   g_return_val_if_fail (NICE_IS_AGENT (agent), FALSE);
   g_return_val_if_fail (stream_id >= 1, FALSE);
@@ -3101,6 +3116,12 @@ nice_agent_gather_candidates (
     }
   }
 
+  length = g_slist_length (local_addresses);
+  if (length > NICE_CANDIDATE_MAX_LOCAL_ADDRESSES) {
+    nice_debug ("Agent %p : cannot have more than %d local addresses.",
+        agent, length);
+  }
+
   for (cid = 1; cid <= stream->n_components; cid++) {
     NiceComponent *component = nice_stream_find_component_by_id (stream, cid);
     gboolean found_local_address = FALSE;
@@ -3116,7 +3137,10 @@ nice_agent_gather_candidates (
       continue;
 
     /* generate a local host candidate for each local address */
-    for (i = local_addresses; i; i = i->next) {
+    length = 0;
+    for (i = local_addresses;
+        i && length <= NICE_CANDIDATE_MAX_LOCAL_ADDRESSES;
+        i = i->next, length++) {
       NiceAddress *addr = i->data;
       NiceCandidate *host_candidate;
 

diff --git a/agent/candidate.c b/agent/candidate.c
index c1f9dec..49b8ffe 100644 (file)
--- a/agent/candidate.c
+++ b/agent/candidate.c
@@ -137,18 +137,29 @@ nice_candidate_ice_priority_full (
       (0x100 - component_id));
 }
 
-static guint32
+static guint16
 nice_candidate_ice_local_preference_full (guint direction_preference,
-    guint other_preference)
+    guint turn_preference, guint other_preference)
 {
-  return (0x2000 * direction_preference +
-      other_preference);
+  /*
+   * bits  0- 5: other_preference (ip local preference)
+   *       6- 8: turn_preference
+   *       9-12: <unused>
+   *      13-15: direction_preference
+   */
+  g_assert (other_preference <= NICE_CANDIDATE_MAX_LOCAL_ADDRESSES);
+  g_assert (turn_preference <= NICE_CANDIDATE_MAX_TURN_SERVERS);
+  g_assert (direction_preference < 8);
+
+  return (direction_preference << 13) +
+      (turn_preference << 6) +
+      other_preference;
 }
 
-static guint8
+static guint
 nice_candidate_ip_local_preference (const NiceCandidate *candidate)
 {
-  guint8 preference = 0;
+  guint preference = 0;
   gchar ip_string[INET6_ADDRSTRLEN];
   GList/*<owned gchar*>*/ *ips = NULL;
   GList/*<unowned gchar*>*/ *iter;
@@ -188,7 +199,8 @@ nice_candidate_ip_local_preference (const NiceCandidate *candidate)
 static guint16
 nice_candidate_ice_local_preference (const NiceCandidate *candidate)
 {
-  guint direction_preference;
+  guint direction_preference = 0;
+  guint turn_preference = 0;
 
   switch (candidate->transport)
     {
@@ -219,24 +231,45 @@ nice_candidate_ice_local_preference (const NiceCandidate *candidate)
         break;
     }
 
+  /* Relay candidates are assigned a unique local preference at
+   * creation time.
+   */
+  if (candidate->type == NICE_CANDIDATE_TYPE_RELAYED) {
+    g_assert (candidate->turn);
+    turn_preference = candidate->turn->preference;
+  }
+
   return nice_candidate_ice_local_preference_full (direction_preference,
-      nice_candidate_ip_local_preference (candidate));
+      turn_preference, nice_candidate_ip_local_preference (candidate));
 }
 
-static guint32
+static guint16
 nice_candidate_ms_ice_local_preference_full (guint transport_preference,
-    guint direction_preference, guint other_preference)
+    guint direction_preference, guint turn_preference, guint other_preference)
 {
-  return 0x1000 * transport_preference +
-      0x200 * direction_preference +
-      0x1 * other_preference;
+  /*
+   * bits 0- 5: other_preference (ip local preference)
+   *      6- 8: turn_preference
+   *      9-11: direction_preference
+   *     12-15: transport_preference
+   */
+  g_assert (other_preference <= NICE_CANDIDATE_MAX_LOCAL_ADDRESSES);
+  g_assert (turn_preference <= NICE_CANDIDATE_MAX_TURN_SERVERS);
+  g_assert (direction_preference < 8);
+  g_assert (transport_preference < 16);
+
+  return (transport_preference << 12) +
+      (direction_preference << 9) +
+      (turn_preference << 6) +
+      other_preference;
 }
 
 static guint32
 nice_candidate_ms_ice_local_preference (const NiceCandidate *candidate)
 {
-  guint8 transport_preference = 0;
-  guint8 direction_preference = 0;
+  guint transport_preference = 0;
+  guint direction_preference = 0;
+  guint turn_preference = 0;
 
   switch (candidate->transport)
     {
@@ -255,8 +288,17 @@ nice_candidate_ms_ice_local_preference (const NiceCandidate *candidate)
       break;
     }
 
+  /* Relay candidates are assigned a unique local preference at
+   * creation time.
+   */
+  if (candidate->type == NICE_CANDIDATE_TYPE_RELAYED) {
+    g_assert (candidate->turn);
+    turn_preference = candidate->turn->preference;
+  }
+
   return nice_candidate_ms_ice_local_preference_full(transport_preference,
-      direction_preference, nice_candidate_ip_local_preference (candidate));
+      direction_preference, turn_preference,
+      nice_candidate_ip_local_preference (candidate));
 }
 
 static guint8

diff --git a/agent/candidate.h b/agent/candidate.h
index 5292227..d06aab7 100644 (file)
--- a/agent/candidate.h
+++ b/agent/candidate.h
@@ -81,6 +81,23 @@ G_BEGIN_DECLS
  */
 #define NICE_CANDIDATE_MAX_FOUNDATION                (32+1)
 
+/**
+ * NICE_CANDIDATE_MAX_TURN_SERVERS
+ *
+ * The maximum number of turns servers.
+ */
+#define NICE_CANDIDATE_MAX_TURN_SERVERS              7
+
+/**
+ * NICE_CANDIDATE_MAX_LOCAL_ADDRESSES
+ *
+ * The maximum number of local addresses. The constraint is that the
+ * maximum number of local addresses and number of turn servers must
+ * fit on 9 bits, to ensure candidate priority uniqueness. See also
+ * @NICE_CANDIDATE_MAX_TURN_SERVERS. We choose 6 bits for the number of
+ * local addresses, and 3 bits for the number of turn servers.
+ */
+#define NICE_CANDIDATE_MAX_LOCAL_ADDRESSES           63
 
 /**
  * NiceCandidateType:
@@ -146,6 +163,7 @@ typedef struct _TurnServer TurnServer;
  * @decoded_username_len: The length of @decoded_username
  * @decoded_password_len: The length of @decoded_password
  * @type: The #NiceRelayType of the server
+ * @preference: A unique identifier used to compute priority
  *
  * A structure to store the TURN relay settings
  */
@@ -161,6 +179,7 @@ struct _TurnServer
   gsize decoded_username_len;
   gsize decoded_password_len;
   NiceRelayType type;
+  guint preference;
 };
 
 /**

diff --git a/docs/reference/libnice/libnice-sections.txt b/docs/reference/libnice/libnice-sections.txt
index 4be4f1a..0e73585 100644 (file)
--- a/docs/reference/libnice/libnice-sections.txt
+++ b/docs/reference/libnice/libnice-sections.txt
@@ -91,6 +91,8 @@ NiceCandidateTransport
 TurnServer
 NiceRelayType
 NICE_CANDIDATE_MAX_FOUNDATION
+NICE_CANDIDATE_MAX_TURN_SERVERS
+NICE_CANDIDATE_MAX_LOCAL_ADDRESSES
 nice_candidate_new
 nice_candidate_free
 nice_candidate_copy

diff --git a/tests/test-priority.c b/tests/test-priority.c
index 87b2ea4..2ffcd24 100644 (file)
--- a/tests/test-priority.c
+++ b/tests/test-priority.c
@@ -41,6 +41,7 @@
 #include "agent.h"
 #include "agent-priv.h"
 #include "interfaces.h"
+#include "candidate.h"
 
 
 int
@@ -62,6 +63,9 @@ main (void)
   }
   g_list_free_full (ips, g_free);
 
+  /* test 0 */
+  g_assert (ip_local_preference <= NICE_CANDIDATE_MAX_LOCAL_ADDRESSES);
+
   /* test 1 */
   g_assert_cmpuint (nice_candidate_jingle_priority (candidate), ==, 1000);
   /* Host UDP */