Commit 2118cbae authored by Fabrice Bellet's avatar Fabrice Bellet Committed by Olivier Crête

conncheck: fix pair priorities uniqueness

This patch fixes the priority assigned to a peer reflexive discovered
local candidate, when the agent has the stun client role and receives an
stun reply. This priority must be the value put in the stun request, ie
the pair->rflx_priority from the parent pair. This ensures two similar
ordered pairs, will generate discovered pairs ordered in the same way
for the stun client, and also for the stun server on the other side.
Without this identical ordered on both sides of the connections, the two
agents may nominate a different pair with the aggresive nomination
scenario, since both are valid.

The other fix concerns the function that ensures local candidates
priority uniqueness, that breaks the assumption that "two local
candidates having the same priority should generate the same
prflx_priority in the pairs they contribute". Respecting this assumption
is important to stay coherent with the behavior of the other agent, that
considers that two stun requests coming from the same peer-reflexive
remote candidate will have the same remote priority (once a remote
candidate is added to the component remote_candidates list, its priority
is not supposed to change).
parent 628fc393
......@@ -2071,17 +2071,47 @@ ensure_unique_priority (NiceStream *stream, NiceComponent *component,
}
}
return priority;
}
static guint32
ensure_unique_prflx_priority (NiceStream *stream, NiceComponent *component,
guint32 local_priority, guint32 prflx_priority)
{
GSList *item;
/* First, ensure we provide the same value for pairs having
* the same local candidate, ie the same local candidate priority
* for the sake of coherency with the stun server behaviour that
* stores a unique priority value per remote candidate, from the
* first stun request it receives (it depends on the kind of NAT
* typically, but for NAT that preserves the binding this is required).
*/
for (item = stream->conncheck_list; item; item = item->next) {
CandidateCheckPair *p = item->data;
if (p->component_id == component->id &&
p->prflx_priority == priority) {
priority--;
p->local->priority == local_priority) {
return p->prflx_priority;
}
}
/* Second, ensure uniqueness across all other prflx_priority values */
again:
if (prflx_priority == 0)
prflx_priority--;
for (item = stream->conncheck_list; item; item = item->next) {
CandidateCheckPair *p = item->data;
if (p->component_id == component->id &&
p->prflx_priority == prflx_priority) {
prflx_priority--;
goto again;
}
}
return priority;
return prflx_priority;
}
......@@ -2128,8 +2158,8 @@ static CandidateCheckPair *priv_add_new_check_pair (NiceAgent *agent,
tmpbuf1, nice_address_get_port (&pair->local->addr),
tmpbuf2, nice_address_get_port (&pair->remote->addr));
}
pair->prflx_priority = ensure_unique_priority (stream, component,
peer_reflexive_candidate_priority (agent, local));
pair->prflx_priority = ensure_unique_prflx_priority (stream, component,
local->priority, peer_reflexive_candidate_priority (agent, local));
stream->conncheck_list = g_slist_insert_sorted (stream->conncheck_list, pair,
(GCompareFunc)conn_check_compare);
......@@ -3049,6 +3079,7 @@ static CandidateCheckPair *priv_add_peer_reflexive_pair (NiceAgent *agent, guint
}
g_snprintf (pair->foundation, NICE_CANDIDATE_PAIR_MAX_FOUNDATION, "%s:%s",
local_cand->foundation, parent_pair->remote->foundation);
if (agent->controlling_mode == TRUE)
pair->priority = nice_candidate_pair_priority (pair->local->priority,
pair->remote->priority);
......@@ -3056,7 +3087,8 @@ static CandidateCheckPair *priv_add_peer_reflexive_pair (NiceAgent *agent, guint
pair->priority = nice_candidate_pair_priority (pair->remote->priority,
pair->local->priority);
pair->nominated = parent_pair->nominated;
pair->prflx_priority = ensure_unique_priority (stream, component,
pair->prflx_priority = ensure_unique_prflx_priority (stream, component,
local_cand->priority,
peer_reflexive_candidate_priority (agent, local_cand));
nice_debug ("Agent %p : added a new peer-discovered pair %p with "
"foundation '%s' and transport %s:%s to stream %u component %u",
......@@ -3172,10 +3204,14 @@ static CandidateCheckPair *priv_process_response_check_for_reflexive(NiceAgent *
if (!agent->force_relay) {
/* step: find a new local candidate, see RFC 5245 7.1.3.2.1.
* "Discovering Peer Reflexive Candidates"
*
* The priority equal to the value of the PRIORITY attribute
* in the Binding request is taken from the "parent" pair p
*/
local_cand = discovery_add_peer_reflexive_candidate (agent,
stream->id,
component->id,
p->prflx_priority,
&mapped,
sockptr,
local_candidate,
......
......@@ -897,6 +897,7 @@ discovery_add_peer_reflexive_candidate (
NiceAgent *agent,
guint stream_id,
guint component_id,
guint32 priority,
NiceAddress *address,
NiceSocket *base_socket,
NiceCandidate *local,
......@@ -927,22 +928,13 @@ discovery_add_peer_reflexive_candidate (
candidate->addr = *address;
candidate->sockptr = base_socket;
candidate->base_addr = base_socket->addr;
if (agent->compatibility == NICE_COMPATIBILITY_GOOGLE) {
candidate->priority = nice_candidate_jingle_priority (candidate);
} else if (agent->compatibility == NICE_COMPATIBILITY_MSN ||
agent->compatibility == NICE_COMPATIBILITY_OC2007) {
candidate->priority = nice_candidate_msn_priority (candidate);
} else if (agent->compatibility == NICE_COMPATIBILITY_OC2007R2) {
candidate->priority = nice_candidate_ms_ice_priority (candidate,
agent->reliable, FALSE);
} else {
candidate->priority = nice_candidate_ice_priority (candidate,
agent->reliable, FALSE);
}
candidate->priority = ensure_unique_priority (stream, component,
candidate->priority);
/* We don't ensure priority uniqueness in this case, since the
* discovered candidate receives the same priority than its
* parent pair, by design, RFC 5245, sect 7.1.3.2.1.
* Discovering Peer Reflexive Candidates (the priority from the
* STUN Request)
*/
candidate->priority = priority;
priv_assign_foundation (agent, candidate);
if ((agent->compatibility == NICE_COMPATIBILITY_MSN ||
......
......@@ -148,6 +148,7 @@ discovery_add_peer_reflexive_candidate (
NiceAgent *agent,
guint stream_id,
guint component_id,
guint32 priority,
NiceAddress *address,
NiceSocket *base_socket,
NiceCandidate *local,
......
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