Commit d252feb5 authored by Olivier Crête's avatar Olivier Crête

discovery: Make sure each candidate has a unique priority

This should fix compliance with RFC 5245 Section 4.1.2

https://phabricator.freedesktop.org/T3324
parent b72b9153
...@@ -391,7 +391,7 @@ nice_component_update_selected_pair (NiceComponent *component, const CandidatePa ...@@ -391,7 +391,7 @@ nice_component_update_selected_pair (NiceComponent *component, const CandidatePa
component->selected_pair.local = pair->local; component->selected_pair.local = pair->local;
component->selected_pair.remote = pair->remote; component->selected_pair.remote = pair->remote;
component->selected_pair.priority = pair->priority; component->selected_pair.priority = pair->priority;
component->selected_pair.prflx_priority = pair->prflx_priority;
} }
/* /*
......
...@@ -83,6 +83,7 @@ struct _CandidatePair ...@@ -83,6 +83,7 @@ struct _CandidatePair
NiceCandidate *local; NiceCandidate *local;
NiceCandidate *remote; NiceCandidate *remote;
guint64 priority; /* candidate pair priority */ guint64 priority; /* candidate pair priority */
guint32 prflx_priority;
CandidatePairKeepalive keepalive; CandidatePairKeepalive keepalive;
}; };
......
...@@ -708,7 +708,6 @@ static gboolean priv_conn_keepalive_tick_unlocked (NiceAgent *agent) ...@@ -708,7 +708,6 @@ static gboolean priv_conn_keepalive_tick_unlocked (NiceAgent *agent)
if (agent->compatibility == NICE_COMPATIBILITY_GOOGLE || if (agent->compatibility == NICE_COMPATIBILITY_GOOGLE ||
agent->keepalive_conncheck) { agent->keepalive_conncheck) {
guint32 priority;
uint8_t uname[NICE_STREAM_MAX_UNAME]; uint8_t uname[NICE_STREAM_MAX_UNAME];
size_t uname_len = size_t uname_len =
priv_create_username (agent, agent_find_stream (agent, stream->id), priv_create_username (agent, agent_find_stream (agent, stream->id),
...@@ -718,8 +717,6 @@ static gboolean priv_conn_keepalive_tick_unlocked (NiceAgent *agent) ...@@ -718,8 +717,6 @@ static gboolean priv_conn_keepalive_tick_unlocked (NiceAgent *agent)
size_t password_len = priv_get_password (agent, size_t password_len = priv_get_password (agent,
agent_find_stream (agent, stream->id), p->remote, &password); agent_find_stream (agent, stream->id), p->remote, &password);
priority = peer_reflexive_candidate_priority (agent, p->local);
if (p->keepalive.stun_message.buffer != NULL) { if (p->keepalive.stun_message.buffer != NULL) {
nice_debug ("Agent %p: Keepalive for s%u:c%u still" nice_debug ("Agent %p: Keepalive for s%u:c%u still"
" retransmitting, not restarting", agent, stream->id, " retransmitting, not restarting", agent, stream->id,
...@@ -735,14 +732,16 @@ static gboolean priv_conn_keepalive_tick_unlocked (NiceAgent *agent) ...@@ -735,14 +732,16 @@ static gboolean priv_conn_keepalive_tick_unlocked (NiceAgent *agent)
"password='%.*s' (%" G_GSIZE_FORMAT "), priority=%u.", agent, "password='%.*s' (%" G_GSIZE_FORMAT "), priority=%u.", agent,
tmpbuf, nice_address_get_port (&p->remote->addr), tmpbuf, nice_address_get_port (&p->remote->addr),
component->id, (int) uname_len, uname, uname_len, component->id, (int) uname_len, uname, uname_len,
(int) password_len, password, password_len, priority); (int) password_len, password, password_len,
p->prflx_priority);
} }
if (uname_len > 0) { if (uname_len > 0) {
buf_len = stun_usage_ice_conncheck_create (&component->stun_agent, buf_len = stun_usage_ice_conncheck_create (&component->stun_agent,
&p->keepalive.stun_message, p->keepalive.stun_buffer, &p->keepalive.stun_message, p->keepalive.stun_buffer,
sizeof(p->keepalive.stun_buffer), sizeof(p->keepalive.stun_buffer),
uname, uname_len, password, password_len, uname, uname_len, password, password_len,
agent->controlling_mode, agent->controlling_mode, priority, agent->controlling_mode, agent->controlling_mode,
p->prflx_priority,
agent->tie_breaker, agent->tie_breaker,
NULL, NULL,
agent_to_ice_compatibility (agent)); agent_to_ice_compatibility (agent));
...@@ -1446,6 +1445,38 @@ static void priv_mark_pair_nominated (NiceAgent *agent, NiceStream *stream, Nice ...@@ -1446,6 +1445,38 @@ static void priv_mark_pair_nominated (NiceAgent *agent, NiceStream *stream, Nice
} }
} }
guint32
ensure_unique_priority (Component *component, guint32 priority)
{
GSList *item;
again:
if (priority == 0)
priority--;
for (item = component->local_candidates; item; item = item->next) {
NiceCandidate *cand = item->data;
if (cand->priority == priority) {
priority--;
goto again;
}
}
for (item = component->stream->conncheck_list; item; item = item->next) {
CandidateCheckPair *p = item->data;
if (p->component_id == component->id &&
p->prflx_priority == priority) {
priority--;
goto again;
}
}
return priority;
}
/* /*
* Creates a new connectivity check pair and adds it to * Creates a new connectivity check pair and adds it to
* the agent's list of checks. * the agent's list of checks.
...@@ -1486,6 +1517,8 @@ static void priv_add_new_check_pair (NiceAgent *agent, guint stream_id, NiceComp ...@@ -1486,6 +1517,8 @@ static void priv_add_new_check_pair (NiceAgent *agent, guint stream_id, NiceComp
} }
pair->nominated = use_candidate; pair->nominated = use_candidate;
pair->controlling = agent->controlling_mode; pair->controlling = agent->controlling_mode;
pair->prflx_priority = ensure_unique_priority (component,
peer_reflexive_candidate_priority (agent, local));
stream->conncheck_list = g_slist_insert_sorted (stream->conncheck_list, pair, stream->conncheck_list = g_slist_insert_sorted (stream->conncheck_list, pair,
(GCompareFunc)conn_check_compare); (GCompareFunc)conn_check_compare);
...@@ -1913,7 +1946,6 @@ int conn_check_send (NiceAgent *agent, CandidateCheckPair *pair) ...@@ -1913,7 +1946,6 @@ int conn_check_send (NiceAgent *agent, CandidateCheckPair *pair)
* - ICE-CONTROLLED/ICE-CONTROLLING (for role conflicts) * - ICE-CONTROLLED/ICE-CONTROLLING (for role conflicts)
* - USE-CANDIDATE (if sent by the controlling agent) * - USE-CANDIDATE (if sent by the controlling agent)
*/ */
guint32 priority;
uint8_t uname[NICE_STREAM_MAX_UNAME]; uint8_t uname[NICE_STREAM_MAX_UNAME];
NiceStream *stream; NiceStream *stream;
...@@ -1935,8 +1967,6 @@ int conn_check_send (NiceAgent *agent, CandidateCheckPair *pair) ...@@ -1935,8 +1967,6 @@ int conn_check_send (NiceAgent *agent, CandidateCheckPair *pair)
pair->remote, pair->local, uname, sizeof (uname), FALSE); pair->remote, pair->local, uname, sizeof (uname), FALSE);
password_len = priv_get_password (agent, stream, pair->remote, &password); password_len = priv_get_password (agent, stream, pair->remote, &password);
priority = peer_reflexive_candidate_priority (agent, pair->local);
if (password != NULL && if (password != NULL &&
(agent->compatibility == NICE_COMPATIBILITY_MSN || (agent->compatibility == NICE_COMPATIBILITY_MSN ||
agent->compatibility == NICE_COMPATIBILITY_OC2007)) { agent->compatibility == NICE_COMPATIBILITY_OC2007)) {
...@@ -1958,8 +1988,7 @@ int conn_check_send (NiceAgent *agent, CandidateCheckPair *pair) ...@@ -1958,8 +1988,7 @@ int conn_check_send (NiceAgent *agent, CandidateCheckPair *pair)
(unsigned long long)agent->tie_breaker, (unsigned long long)agent->tie_breaker,
(int) uname_len, uname, uname_len, (int) uname_len, uname, uname_len,
(int) password_len, password, password_len, (int) password_len, password, password_len,
priority, controlling); pair->prflx_priority, controlling);
} }
if (cand_use) if (cand_use)
...@@ -1969,7 +1998,7 @@ int conn_check_send (NiceAgent *agent, CandidateCheckPair *pair) ...@@ -1969,7 +1998,7 @@ int conn_check_send (NiceAgent *agent, CandidateCheckPair *pair)
buffer_len = stun_usage_ice_conncheck_create (&component->stun_agent, buffer_len = stun_usage_ice_conncheck_create (&component->stun_agent,
&pair->stun_message, pair->stun_buffer, sizeof(pair->stun_buffer), &pair->stun_message, pair->stun_buffer, sizeof(pair->stun_buffer),
uname, uname_len, password, password_len, uname, uname_len, password, password_len,
cand_use, controlling, priority, cand_use, controlling, pair->prflx_priority,
agent->tie_breaker, agent->tie_breaker,
pair->local->foundation, pair->local->foundation,
agent_to_ice_compatibility (agent)); agent_to_ice_compatibility (agent));
...@@ -2290,14 +2319,14 @@ static int priv_store_pending_check (NiceAgent *agent, NiceComponent *component, ...@@ -2290,14 +2319,14 @@ static int priv_store_pending_check (NiceAgent *agent, NiceComponent *component,
* *
* @return created pair, or NULL on fatal (memory allocation) errors * @return created pair, or NULL on fatal (memory allocation) errors
*/ */
static CandidateCheckPair *priv_add_peer_reflexive_pair (NiceAgent *agent, guint stream_id, guint component_id, NiceCandidate *local_cand, CandidateCheckPair *parent_pair) static CandidateCheckPair *priv_add_peer_reflexive_pair (NiceAgent *agent, guint stream_id, Component *component, NiceCandidate *local_cand, CandidateCheckPair *parent_pair)
{ {
CandidateCheckPair *pair = g_slice_new0 (CandidateCheckPair); CandidateCheckPair *pair = g_slice_new0 (CandidateCheckPair);
NiceStream *stream = agent_find_stream (agent, stream_id); NiceStream *stream = agent_find_stream (agent, stream_id);
pair->agent = agent; pair->agent = agent;
pair->stream_id = stream_id; pair->stream_id = stream_id;
pair->component_id = component_id;; pair->component_id = component->id;;
pair->local = local_cand; pair->local = local_cand;
pair->remote = parent_pair->remote; pair->remote = parent_pair->remote;
pair->sockptr = local_cand->sockptr; pair->sockptr = local_cand->sockptr;
...@@ -2322,6 +2351,8 @@ static CandidateCheckPair *priv_add_peer_reflexive_pair (NiceAgent *agent, guint ...@@ -2322,6 +2351,8 @@ static CandidateCheckPair *priv_add_peer_reflexive_pair (NiceAgent *agent, guint
pair->local->priority); pair->local->priority);
pair->nominated = FALSE; pair->nominated = FALSE;
pair->controlling = agent->controlling_mode; pair->controlling = agent->controlling_mode;
pair->prflx_priority = ensure_unique_priority (component,
peer_reflexive_candidate_priority (agent, local_cand));
nice_debug ("Agent %p : added a new peer-discovered pair with foundation of '%s'.", agent, pair->foundation); nice_debug ("Agent %p : added a new peer-discovered pair with foundation of '%s'.", agent, pair->foundation);
stream->conncheck_list = g_slist_insert_sorted (stream->conncheck_list, pair, stream->conncheck_list = g_slist_insert_sorted (stream->conncheck_list, pair,
...@@ -2431,7 +2462,7 @@ static CandidateCheckPair *priv_process_response_check_for_reflexive(NiceAgent * ...@@ -2431,7 +2462,7 @@ static CandidateCheckPair *priv_process_response_check_for_reflexive(NiceAgent *
/* step: add a new discovered pair (see RFC 5245 7.1.3.2.2 /* step: add a new discovered pair (see RFC 5245 7.1.3.2.2
"Constructing a Valid Pair") */ "Constructing a Valid Pair") */
new_pair = priv_add_peer_reflexive_pair (agent, stream->id, component->id, new_pair = priv_add_peer_reflexive_pair (agent, stream->id, component,
local_cand, p); local_cand, p);
nice_debug ("Agent %p : conncheck %p FAILED, %p DISCOVERED.", agent, p, new_pair); nice_debug ("Agent %p : conncheck %p FAILED, %p DISCOVERED.", agent, p, new_pair);
} }
......
...@@ -89,6 +89,7 @@ struct _CandidateCheckPair ...@@ -89,6 +89,7 @@ struct _CandidateCheckPair
gboolean timer_restarted; gboolean timer_restarted;
gboolean valid; gboolean valid;
guint64 priority; guint64 priority;
guint32 prflx_priority;
GTimeVal next_tick; /* next tick timestamp */ GTimeVal next_tick; /* next tick timestamp */
StunTimer timer; StunTimer timer;
uint8_t stun_buffer[STUN_MAX_MESSAGE_SIZE_IPV6]; uint8_t stun_buffer[STUN_MAX_MESSAGE_SIZE_IPV6];
...@@ -110,4 +111,6 @@ void ...@@ -110,4 +111,6 @@ void
conn_check_prune_socket (NiceAgent *agent, NiceStream *stream, NiceComponent *component, conn_check_prune_socket (NiceAgent *agent, NiceStream *stream, NiceComponent *component,
NiceSocket *sock); NiceSocket *sock);
guint32 ensure_unique_priority (Component *component, guint32 priority);
#endif /*_NICE_CONNCHECK_H */ #endif /*_NICE_CONNCHECK_H */
...@@ -549,6 +549,8 @@ HostCandidateResult discovery_add_local_host_candidate ( ...@@ -549,6 +549,8 @@ HostCandidateResult discovery_add_local_host_candidate (
agent->reliable, FALSE); agent->reliable, FALSE);
} }
candidate->priority = ensure_unique_priority (component,
candidate->priority);
priv_generate_candidate_credentials (agent, candidate); priv_generate_candidate_credentials (agent, candidate);
priv_assign_foundation (agent, candidate); priv_assign_foundation (agent, candidate);
...@@ -639,6 +641,8 @@ discovery_add_server_reflexive_candidate ( ...@@ -639,6 +641,8 @@ discovery_add_server_reflexive_candidate (
agent->reliable, nat_assisted); agent->reliable, nat_assisted);
} }
candidate->priority = ensure_unique_priority (component,
candidate->priority);
priv_generate_candidate_credentials (agent, candidate); priv_generate_candidate_credentials (agent, candidate);
priv_assign_foundation (agent, candidate); priv_assign_foundation (agent, candidate);
...@@ -755,6 +759,8 @@ discovery_add_relay_candidate ( ...@@ -755,6 +759,8 @@ discovery_add_relay_candidate (
agent->reliable, FALSE); agent->reliable, FALSE);
} }
candidate->priority = ensure_unique_priority (component,
candidate->priority);
priv_generate_candidate_credentials (agent, candidate); priv_generate_candidate_credentials (agent, candidate);
/* Google uses the turn username as the candidate username */ /* Google uses the turn username as the candidate username */
...@@ -835,6 +841,8 @@ discovery_add_peer_reflexive_candidate ( ...@@ -835,6 +841,8 @@ discovery_add_peer_reflexive_candidate (
agent->reliable, FALSE); agent->reliable, FALSE);
} }
candidate->priority = ensure_unique_priority (component,
candidate->priority);
priv_assign_foundation (agent, candidate); priv_assign_foundation (agent, candidate);
if ((agent->compatibility == NICE_COMPATIBILITY_MSN || if ((agent->compatibility == NICE_COMPATIBILITY_MSN ||
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment