Commit 7037ab4c authored by Fabrice Bellet's avatar Fabrice Bellet Committed by Philip Withnall

conncheck: implement a "triggered queue" list

The checks should not be sent immediately in priv_conn_check_initiate(),
but be put into the "triggered queue", see  "7.2.1.4 Triggered Checks".
This patch implements this triggered checks list, and uses it to enforce a
pacing of STUN transactions, no more than one per Ta ms, according to
"B.1. Pacing of STUN Transactions".
Reviewed-by: default avatarPhilip Withnall <philip.withnall@collabora.co.uk>
Reviewed-by: default avatarPhilip Withnall <philip@tecnocode.co.uk>
Differential Revision: https://phabricator.freedesktop.org/D802
parent 1732c7d6
...@@ -139,6 +139,7 @@ struct _NiceAgent ...@@ -139,6 +139,7 @@ struct _NiceAgent
guint next_stream_id; /* id of next created candidate */ guint next_stream_id; /* id of next created candidate */
NiceRNG *rng; /* random number generator */ NiceRNG *rng; /* random number generator */
GSList *discovery_list; /* list of CandidateDiscovery items */ GSList *discovery_list; /* list of CandidateDiscovery items */
GSList *triggered_check_queue; /* pairs in the triggered check list */
guint discovery_unsched_items; /* number of discovery items unscheduled */ guint discovery_unsched_items; /* number of discovery items unscheduled */
GSource *discovery_timer_source; /* source of discovery timer */ GSource *discovery_timer_source; /* source of discovery timer */
GSource *conncheck_timer_source; /* source of conncheck timer */ GSource *conncheck_timer_source; /* source of conncheck timer */
......
...@@ -3272,9 +3272,7 @@ _set_remote_candidates_locked (NiceAgent *agent, NiceStream *stream, ...@@ -3272,9 +3272,7 @@ _set_remote_candidates_locked (NiceAgent *agent, NiceStream *stream,
conn_check_remote_candidates_set(agent); conn_check_remote_candidates_set(agent);
if (added > 0) { if (added > 0) {
gboolean res = conn_check_schedule_next (agent); conn_check_schedule_next (agent);
if (res != TRUE)
nice_debug ("Agent %p : Warning: unable to schedule any conn checks!", agent);
} }
return added; return added;
......
...@@ -81,6 +81,41 @@ static int priv_timer_expired (GTimeVal *timer, GTimeVal *now) ...@@ -81,6 +81,41 @@ static int priv_timer_expired (GTimeVal *timer, GTimeVal *now)
now->tv_sec >= timer->tv_sec; now->tv_sec >= timer->tv_sec;
} }
/* Add the pair to the triggered checks list, if not already present
*/
static void
priv_add_pair_to_triggered_check_queue (NiceAgent *agent, CandidateCheckPair *pair)
{
g_assert (pair);
if (agent->triggered_check_queue == NULL ||
g_slist_find (agent->triggered_check_queue, pair) == NULL)
agent->triggered_check_queue = g_slist_append (agent->triggered_check_queue, pair);
}
/* Remove the pair from the triggered checks list
*/
static void
priv_remove_pair_from_triggered_check_queue (NiceAgent *agent, CandidateCheckPair *pair)
{
g_assert (pair);
agent->triggered_check_queue = g_slist_remove (agent->triggered_check_queue, pair);
}
/* Get the pair from the triggered checks list
*/
static CandidateCheckPair *
priv_get_pair_from_triggered_check_queue (NiceAgent *agent)
{
CandidateCheckPair *pair = NULL;
if (agent->triggered_check_queue) {
pair = (CandidateCheckPair *)agent->triggered_check_queue->data;
priv_remove_pair_from_triggered_check_queue (agent, pair);
}
return pair;
}
/* /*
* Finds the next connectivity check in WAITING state. * Finds the next connectivity check in WAITING state.
*/ */
...@@ -107,10 +142,6 @@ static CandidateCheckPair *priv_conn_check_find_next_waiting (GSList *conn_check ...@@ -107,10 +142,6 @@ static CandidateCheckPair *priv_conn_check_find_next_waiting (GSList *conn_check
*/ */
static gboolean priv_conn_check_initiate (NiceAgent *agent, CandidateCheckPair *pair) static gboolean priv_conn_check_initiate (NiceAgent *agent, CandidateCheckPair *pair)
{ {
/* XXX: from ID-16 onwards, the checks should not be sent
* immediately, but be put into the "triggered queue",
* see "7.2.1.4 Triggered Checks"
*/
g_get_current_time (&pair->next_tick); g_get_current_time (&pair->next_tick);
g_time_val_add (&pair->next_tick, agent->timer_ta * 1000); g_time_val_add (&pair->next_tick, agent->timer_ta * 1000);
pair->state = NICE_CHECK_IN_PROGRESS; pair->state = NICE_CHECK_IN_PROGRESS;
...@@ -269,7 +300,7 @@ candidate_check_pair_fail (NiceStream *stream, NiceAgent *agent, CandidateCheckP ...@@ -269,7 +300,7 @@ candidate_check_pair_fail (NiceStream *stream, NiceAgent *agent, CandidateCheckP
* *
* @return will return FALSE when no more pending timers. * @return will return FALSE when no more pending timers.
*/ */
static gboolean priv_conn_check_tick_stream (NiceStream *stream, NiceAgent *agent, GTimeVal *now) static gboolean priv_conn_check_tick_stream (NiceStream *stream, NiceAgent *agent, GTimeVal *now, gboolean *stun_transmitted)
{ {
gboolean keep_timer_going = FALSE; gboolean keep_timer_going = FALSE;
guint s_inprogress = 0, s_succeeded = 0, s_discovered = 0, guint s_inprogress = 0, s_succeeded = 0, s_discovered = 0,
...@@ -318,8 +349,8 @@ static gboolean priv_conn_check_tick_stream (NiceStream *stream, NiceAgent *agen ...@@ -318,8 +349,8 @@ static gboolean priv_conn_check_tick_stream (NiceStream *stream, NiceAgent *agen
p->next_tick = *now; p->next_tick = *now;
g_time_val_add (&p->next_tick, timeout * 1000); g_time_val_add (&p->next_tick, timeout * 1000);
keep_timer_going = TRUE; *stun_transmitted = TRUE;
break; return TRUE;
} }
case STUN_USAGE_TIMER_RETURN_SUCCESS: case STUN_USAGE_TIMER_RETURN_SUCCESS:
{ {
...@@ -382,7 +413,7 @@ static gboolean priv_conn_check_tick_stream (NiceStream *stream, NiceAgent *agen ...@@ -382,7 +413,7 @@ static gboolean priv_conn_check_tick_stream (NiceStream *stream, NiceAgent *agen
p->state == NICE_CHECK_DISCOVERED)) { p->state == NICE_CHECK_DISCOVERED)) {
nice_debug ("Agent %p : restarting check %p as the nominated pair.", agent, p); nice_debug ("Agent %p : restarting check %p as the nominated pair.", agent, p);
p->nominated = TRUE; p->nominated = TRUE;
priv_conn_check_initiate (agent, p); priv_add_pair_to_triggered_check_queue (agent, p);
break; /* move to the next component */ break; /* move to the next component */
} }
} }
...@@ -416,13 +447,37 @@ static gboolean priv_conn_check_tick_unlocked (NiceAgent *agent) ...@@ -416,13 +447,37 @@ static gboolean priv_conn_check_tick_unlocked (NiceAgent *agent)
{ {
CandidateCheckPair *pair = NULL; CandidateCheckPair *pair = NULL;
gboolean keep_timer_going = FALSE; gboolean keep_timer_going = FALSE;
gboolean res;
/* note: we try to only generate a single stun transaction per timer
* callback, to respect some pacing of STUN transaction, as per
* appendix B.1 of ICE spec.
*/
gboolean stun_transmitted = FALSE;
GSList *i, *j; GSList *i, *j;
GTimeVal now; GTimeVal now;
/* step: process ongoing STUN transactions */ /* step: process ongoing STUN transactions */
g_get_current_time (&now); g_get_current_time (&now);
/* step: find the highest priority waiting check and send it */ for (j = agent->streams; j; j = j->next) {
NiceStream *stream = j->data;
res = priv_conn_check_tick_stream (stream, agent, &now, &stun_transmitted);
if (res)
keep_timer_going = res;
if (stun_transmitted)
return TRUE;
}
/* step: first initiate a conncheck with a pair from the triggered list */
pair = priv_get_pair_from_triggered_check_queue (agent);
if (pair) {
priv_conn_check_initiate (agent, pair);
return TRUE;
}
/* step: when the triggered list is empty,
* find the highest priority waiting check and send it */
for (i = agent->streams; i ; i = i->next) { for (i = agent->streams; i ; i = i->next) {
NiceStream *stream = i->data; NiceStream *stream = i->data;
...@@ -433,17 +488,25 @@ static gboolean priv_conn_check_tick_unlocked (NiceAgent *agent) ...@@ -433,17 +488,25 @@ static gboolean priv_conn_check_tick_unlocked (NiceAgent *agent)
if (pair) { if (pair) {
priv_conn_check_initiate (agent, pair); priv_conn_check_initiate (agent, pair);
keep_timer_going = TRUE; return TRUE;
} else {
keep_timer_going = priv_conn_check_unfreeze_next (agent);
} }
for (j = agent->streams; j; j = j->next) { /* step: when there's no pair in the Waiting state,
NiceStream *stream = j->data; * unfreeze a new pair and check it
gboolean res = */
priv_conn_check_tick_stream (stream, agent, &now); res = priv_conn_check_unfreeze_next (agent);
if (res)
keep_timer_going = res; for (i = agent->streams; i ; i = i->next) {
NiceStream *stream = i->data;
pair = priv_conn_check_find_next_waiting (stream->conncheck_list);
if (pair)
break;
}
if (pair) {
priv_conn_check_initiate (agent, pair);
return TRUE;
} }
/* step: stop timer if no work left */ /* step: stop timer if no work left */
...@@ -940,23 +1003,14 @@ static gboolean priv_turn_allocate_refresh_tick (gpointer pointer) ...@@ -940,23 +1003,14 @@ static gboolean priv_turn_allocate_refresh_tick (gpointer pointer)
/* /*
* Initiates the next pending connectivity check. * Initiates the next pending connectivity check.
*
* @return TRUE if a pending check was scheduled
*/ */
gboolean conn_check_schedule_next (NiceAgent *agent) void conn_check_schedule_next (NiceAgent *agent)
{ {
gboolean res = priv_conn_check_unfreeze_next (agent);
nice_debug ("Agent %p : priv_conn_check_unfreeze_next returned %d", agent, res);
if (agent->discovery_unsched_items > 0) if (agent->discovery_unsched_items > 0)
nice_debug ("Agent %p : WARN: starting conn checks before local candidate gathering is finished.", agent); nice_debug ("Agent %p : WARN: starting conn checks before local candidate gathering is finished.", agent);
/* step: call once imediately */
res = priv_conn_check_tick_unlocked (agent);
nice_debug ("Agent %p : priv_conn_check_tick_unlocked returned %d", agent, res);
/* step: schedule timer if not running yet */ /* step: schedule timer if not running yet */
if (res && agent->conncheck_timer_source == NULL) { if (agent->conncheck_timer_source == NULL) {
agent_timeout_add_with_context (agent, &agent->conncheck_timer_source, agent_timeout_add_with_context (agent, &agent->conncheck_timer_source,
"Connectivity check schedule", agent->timer_ta, "Connectivity check schedule", agent->timer_ta,
priv_conn_check_tick, agent); priv_conn_check_tick, agent);
...@@ -968,9 +1022,6 @@ gboolean conn_check_schedule_next (NiceAgent *agent) ...@@ -968,9 +1022,6 @@ gboolean conn_check_schedule_next (NiceAgent *agent)
"Connectivity keepalive timeout", NICE_AGENT_TIMER_TR_DEFAULT, "Connectivity keepalive timeout", NICE_AGENT_TIMER_TR_DEFAULT,
priv_conn_keepalive_tick, agent); priv_conn_keepalive_tick, agent);
} }
nice_debug ("Agent %p : conn_check_schedule_next returning %d", agent, res);
return res;
} }
/* /*
...@@ -1573,6 +1624,8 @@ static void conn_check_free_item (gpointer data) ...@@ -1573,6 +1624,8 @@ static void conn_check_free_item (gpointer data)
{ {
CandidateCheckPair *pair = data; CandidateCheckPair *pair = data;
if (pair->agent)
priv_remove_pair_from_triggered_check_queue (pair->agent, pair);
pair->stun_message.buffer = NULL; pair->stun_message.buffer = NULL;
pair->stun_message.buffer_len = 0; pair->stun_message.buffer_len = 0;
g_slice_free (CandidateCheckPair, pair); g_slice_free (CandidateCheckPair, pair);
...@@ -1803,23 +1856,24 @@ size_t priv_get_password (NiceAgent *agent, NiceStream *stream, ...@@ -1803,23 +1856,24 @@ size_t priv_get_password (NiceAgent *agent, NiceStream *stream,
/* Implement the computation specific in RFC 5245 section 16 */ /* Implement the computation specific in RFC 5245 section 16 */
static unsigned int priv_compute_conncheck_timer (NiceAgent *agent, static unsigned int priv_compute_conncheck_timer (NiceAgent *agent)
NiceStream *stream)
{ {
GSList *item; GSList *item1, *item2;
guint waiting_and_in_progress = 0; guint waiting_and_in_progress = 0;
unsigned int rto = 0; unsigned int rto = 0;
for (item = stream->conncheck_list; item; item = item->next) {
CandidateCheckPair *pair = item->data; for (item1 = agent->streams; item1; item1 = item1->next) {
NiceStream *stream = item1->data;;
for (item2 = stream->conncheck_list; item2; item2 = item2->next) {
CandidateCheckPair *pair = item2->data;
if (pair->state == NICE_CHECK_IN_PROGRESS || if (pair->state == NICE_CHECK_IN_PROGRESS ||
pair->state == NICE_CHECK_WAITING) pair->state == NICE_CHECK_WAITING)
waiting_and_in_progress++; waiting_and_in_progress++;
} }
}
/* FIXME: This should also be multiple by "N", which I believe is the
* number of NiceStreams currently in the conncheck state. */
rto = agent->timer_ta * waiting_and_in_progress; rto = agent->timer_ta * waiting_and_in_progress;
/* We assume non-reliable streams are RTP, so we use 100 as the max */ /* We assume non-reliable streams are RTP, so we use 100 as the max */
...@@ -1921,7 +1975,7 @@ int conn_check_send (NiceAgent *agent, CandidateCheckPair *pair) ...@@ -1921,7 +1975,7 @@ int conn_check_send (NiceAgent *agent, CandidateCheckPair *pair)
stun_timer_start_reliable(&pair->timer, STUN_TIMER_DEFAULT_RELIABLE_TIMEOUT); stun_timer_start_reliable(&pair->timer, STUN_TIMER_DEFAULT_RELIABLE_TIMEOUT);
} else { } else {
stun_timer_start (&pair->timer, stun_timer_start (&pair->timer,
priv_compute_conncheck_timer (agent, stream), priv_compute_conncheck_timer (agent),
STUN_TIMER_DEFAULT_MAX_RETRANSMISSIONS); STUN_TIMER_DEFAULT_MAX_RETRANSMISSIONS);
} }
...@@ -2071,7 +2125,7 @@ static gboolean priv_schedule_triggered_check (NiceAgent *agent, NiceStream *str ...@@ -2071,7 +2125,7 @@ static gboolean priv_schedule_triggered_check (NiceAgent *agent, NiceStream *str
if (p->state == NICE_CHECK_WAITING || if (p->state == NICE_CHECK_WAITING ||
p->state == NICE_CHECK_FROZEN) p->state == NICE_CHECK_FROZEN)
priv_conn_check_initiate (agent, p); priv_add_pair_to_triggered_check_queue (agent, p);
else if (p->state == NICE_CHECK_IN_PROGRESS) { else if (p->state == NICE_CHECK_IN_PROGRESS) {
/* XXX: according to ICE 7.2.1.4 "Triggered Checks" (ID-19), /* XXX: according to ICE 7.2.1.4 "Triggered Checks" (ID-19),
* we should cancel the existing one, instead we reset our timer, so * we should cancel the existing one, instead we reset our timer, so
...@@ -2082,7 +2136,7 @@ static gboolean priv_schedule_triggered_check (NiceAgent *agent, NiceStream *str ...@@ -2082,7 +2136,7 @@ static gboolean priv_schedule_triggered_check (NiceAgent *agent, NiceStream *str
p->timer_restarted ? "no" : "yes"); p->timer_restarted ? "no" : "yes");
if (!nice_socket_is_reliable (p->sockptr) && !p->timer_restarted) { if (!nice_socket_is_reliable (p->sockptr) && !p->timer_restarted) {
stun_timer_start (&p->timer, stun_timer_start (&p->timer,
priv_compute_conncheck_timer (agent, stream), priv_compute_conncheck_timer (agent),
STUN_TIMER_DEFAULT_MAX_RETRANSMISSIONS); STUN_TIMER_DEFAULT_MAX_RETRANSMISSIONS);
p->timer_restarted = TRUE; p->timer_restarted = TRUE;
} }
...@@ -2100,15 +2154,17 @@ static gboolean priv_schedule_triggered_check (NiceAgent *agent, NiceStream *str ...@@ -2100,15 +2154,17 @@ static gboolean priv_schedule_triggered_check (NiceAgent *agent, NiceStream *str
if ((agent->compatibility == NICE_COMPATIBILITY_RFC5245 || if ((agent->compatibility == NICE_COMPATIBILITY_RFC5245 ||
agent->compatibility == NICE_COMPATIBILITY_WLM2009 || agent->compatibility == NICE_COMPATIBILITY_WLM2009 ||
agent->compatibility == NICE_COMPATIBILITY_OC2007R2) && agent->compatibility == NICE_COMPATIBILITY_OC2007R2) &&
agent->controlling_mode) agent->controlling_mode) {
priv_conn_check_initiate (agent, p); priv_add_pair_to_triggered_check_queue (agent, p);
conn_check_schedule_next(agent);
}
} else if (p->state == NICE_CHECK_FAILED) { } else if (p->state == NICE_CHECK_FAILED) {
/* 7.2.1.4 Triggered Checks /* 7.2.1.4 Triggered Checks
* If the state of the pair is Failed, it is changed to Waiting * If the state of the pair is Failed, it is changed to Waiting
and the agent MUST create a new connectivity check for that and the agent MUST create a new connectivity check for that
pair (representing a new STUN Binding request transaction), by pair (representing a new STUN Binding request transaction), by
enqueueing the pair in the triggered check queue. */ enqueueing the pair in the triggered check queue. */
priv_conn_check_initiate (agent, p); priv_add_pair_to_triggered_check_queue (agent, p);
} }
/* note: the spec says the we SHOULD retransmit in-progress /* note: the spec says the we SHOULD retransmit in-progress
...@@ -2493,6 +2549,7 @@ static gboolean priv_map_reply_to_conn_check_request (NiceAgent *agent, NiceStre ...@@ -2493,6 +2549,7 @@ static gboolean priv_map_reply_to_conn_check_request (NiceAgent *agent, NiceStre
p->stun_message.buffer = NULL; p->stun_message.buffer = NULL;
p->stun_message.buffer_len = 0; p->stun_message.buffer_len = 0;
p->state = NICE_CHECK_WAITING; p->state = NICE_CHECK_WAITING;
priv_add_pair_to_triggered_check_queue (agent, p);
nice_debug ("Agent %p : pair %p state WAITING", agent, p); nice_debug ("Agent %p : pair %p state WAITING", agent, p);
trans_found = TRUE; trans_found = TRUE;
} else { } else {
......
...@@ -98,7 +98,7 @@ int conn_check_add_for_candidate (NiceAgent *agent, guint stream_id, NiceCompone ...@@ -98,7 +98,7 @@ int conn_check_add_for_candidate (NiceAgent *agent, guint stream_id, NiceCompone
int conn_check_add_for_local_candidate (NiceAgent *agent, guint stream_id, NiceComponent *component, NiceCandidate *local); int conn_check_add_for_local_candidate (NiceAgent *agent, guint stream_id, NiceComponent *component, NiceCandidate *local);
gboolean conn_check_add_for_candidate_pair (NiceAgent *agent, guint stream_id, NiceComponent *component, NiceCandidate *local, NiceCandidate *remote); gboolean conn_check_add_for_candidate_pair (NiceAgent *agent, guint stream_id, NiceComponent *component, NiceCandidate *local, NiceCandidate *remote);
void conn_check_free (NiceAgent *agent); void conn_check_free (NiceAgent *agent);
gboolean conn_check_schedule_next (NiceAgent *agent); void conn_check_schedule_next (NiceAgent *agent);
int conn_check_send (NiceAgent *agent, CandidateCheckPair *pair); int conn_check_send (NiceAgent *agent, CandidateCheckPair *pair);
void conn_check_prune_stream (NiceAgent *agent, NiceStream *stream); void conn_check_prune_stream (NiceAgent *agent, NiceStream *stream);
gboolean conn_check_handle_inbound_stun (NiceAgent *agent, NiceStream *stream, NiceComponent *component, NiceSocket *udp_socket, const NiceAddress *from, gchar *buf, guint len); gboolean conn_check_handle_inbound_stun (NiceAgent *agent, NiceStream *stream, NiceComponent *component, NiceSocket *udp_socket, const NiceAddress *from, gchar *buf, guint len);
......
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