Commit d88379ee authored by Kai Vehmanen's avatar Kai Vehmanen

Fixed agent code bugs: correct handling of restarts and role conflict corner cases.

darcs-hash:20070720213534-77cd4-d76a3872d2d9b2a56b28eee31d8bc747513ac14b.gz
parent 6d8a0c84
...@@ -1254,7 +1254,9 @@ nice_agent_poll_read ( ...@@ -1254,7 +1254,9 @@ nice_agent_poll_read (
/** /**
* Sends a data payload over a stream component. * Sends a data payload over a stream component.
* *
* @pre component state MUST be NICE_COMPONENT_STATE_READY * @pre component state MUST be NICE_COMPONENT_STATE_READY,
* or as a special case, in any state if component was
* in READY state before and was then restarted
* *
* @return number of bytes sent, or negative error code * @return number of bytes sent, or negative error code
*/ */
...@@ -1365,13 +1367,17 @@ nice_agent_restart ( ...@@ -1365,13 +1367,17 @@ nice_agent_restart (
GSList *i; GSList *i;
gboolean res = TRUE; gboolean res = TRUE;
/* clean up all connectivity checks */ /* step: clean up all connectivity checks */
conn_check_free (agent); conn_check_free (agent);
/* step: regenerate tie-breaker value */
priv_generate_tie_breaker (agent);
for (i = agent->streams; i && res; i = i->next) { for (i = agent->streams; i && res; i = i->next) {
Stream *stream = i->data; Stream *stream = i->data;
/* reset local credentials */ /* step: reset local credentials for the stream and
* clean up the list of remote candidates */
res = stream_restart (stream, agent->rng); res = stream_restart (stream, agent->rng);
} }
......
...@@ -381,7 +381,7 @@ static gboolean priv_conn_keepalive_tick (gpointer pointer) ...@@ -381,7 +381,7 @@ static gboolean priv_conn_keepalive_tick (gpointer pointer)
res = stun_bind_keepalive (p->local->sockptr->fileno, res = stun_bind_keepalive (p->local->sockptr->fileno,
&sockaddr, sizeof (sockaddr)); &sockaddr, sizeof (sockaddr));
g_debug ("stun_bind_keepalive for pair %p res %d.", p, res); g_debug ("stun_bind_keepalive for pair %p res %d (%s).", p, res, strerror (res));
if (res < 0) if (res < 0)
++errors; ++errors;
} }
...@@ -475,6 +475,7 @@ static gboolean priv_add_new_check_pair (NiceAgent *agent, guint stream_id, Comp ...@@ -475,6 +475,7 @@ static gboolean priv_add_new_check_pair (NiceAgent *agent, guint stream_id, Comp
pair->priority = nice_candidate_pair_priority (remote->priority, local->priority); pair->priority = nice_candidate_pair_priority (remote->priority, local->priority);
pair->state = initial_state; pair->state = initial_state;
pair->nominated = use_candidate; pair->nominated = use_candidate;
pair->controlling = agent->controlling_mode;
/* note: for the first added check */ /* note: for the first added check */
if (!agent->conncheck_list) if (!agent->conncheck_list)
...@@ -1033,6 +1034,7 @@ static CandidateCheckPair *priv_add_peer_reflexive_pair (NiceAgent *agent, guint ...@@ -1033,6 +1034,7 @@ static CandidateCheckPair *priv_add_peer_reflexive_pair (NiceAgent *agent, guint
else else
pair->priority = nice_candidate_pair_priority (parent_pair->remote->priority, local_cand->priority); pair->priority = nice_candidate_pair_priority (parent_pair->remote->priority, local_cand->priority);
pair->nominated = FALSE; pair->nominated = FALSE;
pair->controlling = agent->controlling_mode;
g_debug ("added a new peer-discovered pair with foundation of '%s'.", pair->foundation); g_debug ("added a new peer-discovered pair with foundation of '%s'.", pair->foundation);
return pair; return pair;
} }
...@@ -1041,6 +1043,42 @@ static CandidateCheckPair *priv_add_peer_reflexive_pair (NiceAgent *agent, guint ...@@ -1041,6 +1043,42 @@ static CandidateCheckPair *priv_add_peer_reflexive_pair (NiceAgent *agent, guint
return NULL; return NULL;
} }
/**
* Recalculates priorities of all candidate pairs. This
* is required after a conflict in ICE roles.
*/
static void priv_recalculate_pair_priorities (NiceAgent *agent)
{
GSList *i;
for (i = agent->conncheck_list; i; i = i->next) {
CandidateCheckPair *p = i->data;
if (agent->controlling_mode == TRUE)
p->priority = nice_candidate_pair_priority (p->local->priority, p->remote->priority);
else
p->priority = nice_candidate_pair_priority (p->remote->priority, p->local->priority);
}
}
/**
* Change the agent role if different from 'control'. Can be
* initiated both by handling of incoming connectivity checks,
* and by processing the responses to checks sent by us.
*/
static void priv_check_for_role_conflict (NiceAgent *agent, gboolean control)
{
/* role conflict, change mode; wait for a new conn. check */
if (control != agent->controlling_mode) {
g_debug ("Role conflict, changing agent role to %d.", control);
agent->controlling_mode = control;
/* the pair priorities depend on the roles, so recalculation
* is needed */
priv_recalculate_pair_priorities (agent);
}
else
g_debug ("Role conflict, agent role already changed to %d.", control);
}
/** /**
* Tries to match STUN reply in 'buf' to an existing STUN connectivity * Tries to match STUN reply in 'buf' to an existing STUN connectivity
* check transaction. If found, the reply is processed. Implements * check transaction. If found, the reply is processed. Implements
...@@ -1060,7 +1098,7 @@ static gboolean priv_map_reply_to_conn_check_request (NiceAgent *agent, Stream * ...@@ -1060,7 +1098,7 @@ static gboolean priv_map_reply_to_conn_check_request (NiceAgent *agent, Stream *
CandidateCheckPair *p = i->data; CandidateCheckPair *p = i->data;
if (p->stun_ctx) { if (p->stun_ctx) {
res = stun_bind_process (p->stun_ctx, buf, len, &sockaddr, &socklen); res = stun_bind_process (p->stun_ctx, buf, len, &sockaddr, &socklen);
g_debug ("stun_bind_process/conncheck for %p res %d (controlling=%d).", p, res, agent->controlling_mode); g_debug ("stun_bind_process/conncheck for %p res %d (%s) (controlling=%d).", p, res, strerror (res), agent->controlling_mode);
if (res == 0) { if (res == 0) {
/* case: found a matching connectivity check request */ /* case: found a matching connectivity check request */
...@@ -1136,6 +1174,19 @@ static gboolean priv_map_reply_to_conn_check_request (NiceAgent *agent, Stream * ...@@ -1136,6 +1174,19 @@ static gboolean priv_map_reply_to_conn_check_request (NiceAgent *agent, Stream *
trans_found = TRUE; trans_found = TRUE;
} }
else if (res == ECONNRESET) {
/* case: role conflict error, need to restart with new role */
g_debug ("conncheck %p ROLE CONFLICT, restarting", p);
/* note: our role might already have changed due to an
* incoming request, but if not, change role now;
* follows ICE 7.1.2.1 "Failure Cases" (ID-17) */
priv_check_for_role_conflict (agent, !p->controlling);
p->stun_ctx = NULL;
p->state = NICE_CHECK_WAITING;
trans_found = TRUE;
}
else if (res != EAGAIN) { else if (res != EAGAIN) {
/* case: STUN error, the check STUN context was freed */ /* case: STUN error, the check STUN context was freed */
g_debug ("conncheck %p FAILED.", p); g_debug ("conncheck %p FAILED.", p);
...@@ -1143,15 +1194,11 @@ static gboolean priv_map_reply_to_conn_check_request (NiceAgent *agent, Stream * ...@@ -1143,15 +1194,11 @@ static gboolean priv_map_reply_to_conn_check_request (NiceAgent *agent, Stream *
trans_found = TRUE; trans_found = TRUE;
} }
else { else {
/* case: STUN could not parse, skip */
/* XXX: added by Remi, needs review */
g_assert (res == EAGAIN); g_assert (res == EAGAIN);
/* XXX: stun won't restart automatically, so cancel and g_debug ("conncheck %p SKIPPED", p);
* restart, should do this only for 487 errors (role conflict) */
g_debug ("cancelling and restarting check for pair %p", p);
stun_bind_cancel (p->stun_ctx),
p->stun_ctx = NULL;
p->state = NICE_CHECK_WAITING;
} }
} }
} }
...@@ -1177,7 +1224,7 @@ static gboolean priv_map_reply_to_discovery_request (NiceAgent *agent, gchar *bu ...@@ -1177,7 +1224,7 @@ static gboolean priv_map_reply_to_discovery_request (NiceAgent *agent, gchar *bu
CandidateDiscovery *d = i->data; CandidateDiscovery *d = i->data;
if (d->stun_ctx) { if (d->stun_ctx) {
res = stun_bind_process (d->stun_ctx, buf, len, &sockaddr, &socklen); res = stun_bind_process (d->stun_ctx, buf, len, &sockaddr, &socklen);
g_debug ("stun_bind_process/disc for %p res %d.", d, res); g_debug ("stun_bind_process/disc for %p res %d (%s).", d, res, strerror (res));
if (res == 0) { if (res == 0) {
/* case: succesful binding discovery, create a new local candidate */ /* case: succesful binding discovery, create a new local candidate */
NiceAddress niceaddr; NiceAddress niceaddr;
...@@ -1229,23 +1276,6 @@ static gboolean priv_verify_inbound_username (Stream *stream, const char *uname) ...@@ -1229,23 +1276,6 @@ static gboolean priv_verify_inbound_username (Stream *stream, const char *uname)
return FALSE; return FALSE;
} }
/**
* Recalculates priorities of all candidate pairs. This
* is required after a conflict in ICE roles.
*/
static void priv_recalculate_pair_priorities (NiceAgent *agent)
{
GSList *i;
for (i = agent->conncheck_list; i; i = i->next) {
CandidateCheckPair *p = i->data;
if (agent->controlling_mode == TRUE)
p->priority = nice_candidate_pair_priority (p->local->priority, p->remote->priority);
else
p->priority = nice_candidate_pair_priority (p->remote->priority, p->local->priority);
}
}
/** /**
* Processing an incoming STUN message. * Processing an incoming STUN message.
* *
...@@ -1274,25 +1304,18 @@ gboolean conn_check_handle_inbound_stun (NiceAgent *agent, Stream *stream, Compo ...@@ -1274,25 +1304,18 @@ gboolean conn_check_handle_inbound_stun (NiceAgent *agent, Stream *stream, Compo
/* note: contents of 'buf' already validated, so it is /* note: contents of 'buf' already validated, so it is
* a valid and fully received STUN message */ * a valid and fully received STUN message */
g_debug ("inbound STUN packet for stream/component %u/%u:", stream->id, component->id); g_debug ("inbound STUN packet for %p/%u/%u (agent/stream/component):", agent, stream->id, component->id);
/* note: ICE 7.2. "STUN Server Procedures" (ID-17) */ /* note: ICE 7.2. "STUN Server Procedures" (ID-17) */
res = stun_conncheck_reply (rbuf, &rbuf_len, (const uint8_t*)buf, &sockaddr, sizeof (sockaddr), res = stun_conncheck_reply (rbuf, &rbuf_len, (const uint8_t*)buf, &sockaddr, sizeof (sockaddr),
stream->local_password, &control, agent->tie_breaker); stream->local_password, &control, agent->tie_breaker);
if (res == EACCES) {
/* role conflict, change mode and regenarate reply */ if (res == EACCES)
if (control != agent->controlling_mode) { priv_check_for_role_conflict (agent, control);
g_debug ("Conflict in controller selection, switching to mode %d.", control);
agent->controlling_mode = control;
/* the pair priorities depend on the roles, so recalculation
* is needed */
priv_recalculate_pair_priorities (agent);
}
}
if (res == 0 || res == EACCES) { if (res == 0 || res == EACCES) {
/* case 1: valid incoming request, send a reply */ /* case 1: valid incoming request, send a reply/error */
GSList *i; GSList *i;
bool use_candidate = bool use_candidate =
......
...@@ -78,6 +78,7 @@ struct _CandidateCheckPair ...@@ -78,6 +78,7 @@ struct _CandidateCheckPair
gchar foundation[NICE_CANDIDATE_PAIR_MAX_FOUNDATION]; gchar foundation[NICE_CANDIDATE_PAIR_MAX_FOUNDATION];
NiceCheckState state; NiceCheckState state;
gboolean nominated; gboolean nominated;
gboolean controlling;
guint64 priority; guint64 priority;
GTimeVal next_tick; /* next tick timestamp */ GTimeVal next_tick; /* next tick timestamp */
gboolean traffic_after_tick; gboolean traffic_after_tick;
......
...@@ -404,6 +404,9 @@ NiceCandidate *discovery_learn_remote_peer_reflexive_candidate ( ...@@ -404,6 +404,9 @@ NiceCandidate *discovery_learn_remote_peer_reflexive_candidate (
{ {
NiceCandidate *candidate; NiceCandidate *candidate;
/* XXX: for use compiler */
(void)udp_socket;
candidate = nice_candidate_new (NICE_CANDIDATE_TYPE_PEER_REFLEXIVE); candidate = nice_candidate_new (NICE_CANDIDATE_TYPE_PEER_REFLEXIVE);
if (candidate) { if (candidate) {
GSList *modified_list; GSList *modified_list;
......
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