Commit ae3174d8 authored by Fabrice Bellet's avatar Fabrice Bellet

conncheck: remove cancelled pair state

Pairs with the state NICE_CHECK_CANCELLED are the pairs targeted for
removal after the nomination of a pair with an higher priority,
described in Section 8.1.2 "Updating States", item 2 of RFC 5245. They
include also pairs that overflow the conncheck list size, but this is a
somewhat more marginal situation. So we are mainly interested in the
first use case of this state.

This state mixes two different situations, that deserves a distinct
handling : on one side, there are waiting or frozen pairs that must be
removed, this is an immediate action that doesn't need a dedicated state
for that. And on the other side, there are in-progress pairs that
should no longer be retransmitted.

This patch removes the cancelled state, and adds a flag
stop_retransmit_on_timeout to deal with this last situation.

Differential Revision: https://phabricator.freedesktop.org/D1114
parent 39761e79
......@@ -100,8 +100,6 @@ priv_state_to_gchar (NiceCheckState state)
return 'F';
case NICE_CHECK_FROZEN:
return 'Z';
case NICE_CHECK_CANCELLED:
return 'C';
case NICE_CHECK_DISCOVERED:
return 'D';
default:
......@@ -649,6 +647,7 @@ static gboolean priv_conn_check_tick_stream (NiceStream *stream, NiceAgent *agen
gchar tmpbuf1[INET6_ADDRSTRLEN], tmpbuf2[INET6_ADDRSTRLEN];
NiceComponent *component;
timer_timeout:
/* case: conncheck cancelled due to in-progress incoming
* check, requeing the pair, ICE spec, sect 7.2.1.4
* "Triggered Checks", "If the state of that pair is
......@@ -684,6 +683,13 @@ static gboolean priv_conn_check_tick_stream (NiceStream *stream, NiceAgent *agen
{
unsigned int timeout = stun_timer_remainder (&p->timer);
/* case: retransmission stopped, due to the nomination of
* a pair with a higher priority than this in-progress pair,
* ICE spec, sect 8.1.2 "Updating States", item 2.2
*/
if (p->stop_retransmit_on_timeout)
goto timer_timeout;
/* case: conncheck cancelled due to in-progress incoming
* check, requeing the pair, ICE spec, sect 7.2.1.4
* "Triggered Checks", "If the state of that pair is
......@@ -1620,26 +1626,6 @@ static void priv_preprocess_conn_check_pending_data (NiceAgent *agent, NiceStrea
}
static GSList *prune_cancelled_conn_check (GSList *conncheck_list)
{
GSList *item = conncheck_list;
while (item) {
CandidateCheckPair *pair = item->data;
GSList *next = item->next;
if (pair->state == NICE_CHECK_CANCELLED) {
conn_check_free_item (pair);
conncheck_list = g_slist_delete_link (conncheck_list, item);
}
item = next;
}
return conncheck_list;
}
/*
* Handle any processing steps for connectivity checks after
* remote credentials have been set. This function handles
......@@ -1774,9 +1760,6 @@ void conn_check_remote_credentials_set(NiceAgent *agent, NiceStream *stream)
(GDestroyNotify) incoming_check_free);
component->incoming_checks = NULL;
}
stream->conncheck_list =
prune_cancelled_conn_check (stream->conncheck_list);
}
/*
......@@ -1784,7 +1767,7 @@ void conn_check_remote_credentials_set(NiceAgent *agent, NiceStream *stream)
* in ICE spec section 5.7.3 (ID-19). See also
* conn_check_add_for_candidate().
*/
static void priv_limit_conn_check_list_size (GSList *conncheck_list, guint upper_limit)
static GSList *priv_limit_conn_check_list_size (GSList *conncheck_list, guint upper_limit)
{
guint valid = 0;
guint cancelled = 0;
......@@ -1792,22 +1775,22 @@ static void priv_limit_conn_check_list_size (GSList *conncheck_list, guint upper
while (item) {
CandidateCheckPair *pair = item->data;
GSList *next = item->next;
if (pair->state != NICE_CHECK_CANCELLED) {
valid++;
if (valid > upper_limit) {
pair->state = NICE_CHECK_CANCELLED;
valid++;
if (valid > upper_limit) {
conn_check_free_item (pair);
conncheck_list = g_slist_delete_link (conncheck_list, item);
cancelled++;
}
}
item = item->next;
item = next;
}
if (cancelled > 0)
nice_debug ("Agent : Pruned %d candidates. Conncheck list has %d elements"
" left. Maximum connchecks allowed : %d", cancelled, valid,
upper_limit);
return conncheck_list;
}
/*
......@@ -2126,7 +2109,8 @@ static CandidateCheckPair *priv_add_new_check_pair (NiceAgent *agent,
/* implement the hard upper limit for number of
checks (see sect 5.7.3 ICE ID-19): */
if (agent->compatibility == NICE_COMPATIBILITY_RFC5245) {
priv_limit_conn_check_list_size (stream->conncheck_list, agent->max_conn_checks);
stream->conncheck_list = priv_limit_conn_check_list_size (
stream->conncheck_list, agent->max_conn_checks);
}
return pair;
......@@ -2766,8 +2750,10 @@ static guint priv_prune_pending_checks (NiceAgent *agent, NiceStream *stream, gu
* the triggered check list should also be treated like an in-progress
* pair.
*/
for (i = stream->conncheck_list; i; i = i->next) {
i = stream->conncheck_list;
while (i) {
CandidateCheckPair *p = i->data;
GSList *next = i->next;
if (p->component_id == component_id) {
gboolean like_in_progress =
......@@ -2776,19 +2762,19 @@ static guint priv_prune_pending_checks (NiceAgent *agent, NiceStream *stream, gu
if (p->state == NICE_CHECK_FROZEN ||
(p->state == NICE_CHECK_WAITING && !like_in_progress)) {
p->state = NICE_CHECK_CANCELLED;
nice_debug ("Agent %p : pair %p state CANCELED", agent, p);
nice_debug ("Agent %p : pair %p removed.", agent, p);
conn_check_free_item (p);
stream->conncheck_list = g_slist_delete_link(stream->conncheck_list, i);
}
/* note: a SHOULD level req. in ICE 8.1.2. "Updating States" (ID-19) */
if (p->state == NICE_CHECK_IN_PROGRESS ||
else if (p->state == NICE_CHECK_IN_PROGRESS ||
(p->state == NICE_CHECK_WAITING && like_in_progress)) {
if (highest_nominated_priority != 0 &&
p->priority < highest_nominated_priority) {
p->stun_message.buffer = NULL;
p->stun_message.buffer_len = 0;
p->state = NICE_CHECK_CANCELLED;
nice_debug ("Agent %p : pair %p state CANCELED", agent, p);
p->stop_retransmit_on_timeout = TRUE;
nice_debug ("Agent %p : pair %p will not be retransmitted.",
agent, p);
} else {
/* We must keep the higher priority pairs running because if a udp
* packet was lost, we might end up using a bad candidate */
......@@ -2800,6 +2786,7 @@ static guint priv_prune_pending_checks (NiceAgent *agent, NiceStream *stream, gu
}
}
}
i = next;
}
return in_progress;
......@@ -3415,10 +3402,6 @@ static gboolean priv_map_reply_to_conn_check_request (NiceAgent *agent, NiceStre
}
}
stream->conncheck_list =
prune_cancelled_conn_check (stream->conncheck_list);
return trans_found;
}
......
......@@ -56,7 +56,6 @@
* @NICE_CHECK_SUCCEEDED: Connection successfully checked.
* @NICE_CHECK_FAILED: No connectivity; retransmissions ceased.
* @NICE_CHECK_FROZEN: Waiting to be scheduled to %NICE_CHECK_WAITING.
* @NICE_CHECK_CANCELLED: Check cancelled.
* @NICE_CHECK_DISCOVERED: A valid candidate pair not on the check list.
*
* States for checking a candidate pair.
......@@ -68,7 +67,6 @@ typedef enum
NICE_CHECK_SUCCEEDED,
NICE_CHECK_FAILED,
NICE_CHECK_FROZEN,
NICE_CHECK_CANCELLED,
NICE_CHECK_DISCOVERED,
} NiceCheckState;
......@@ -90,6 +88,7 @@ struct _CandidateCheckPair
gboolean mark_nominated_on_response_arrival;
gboolean recheck_on_timeout;
guint recheck_on_timeout_count;
gboolean stop_retransmit_on_timeout;
struct _CandidateCheckPair *discovered_pair;
struct _CandidateCheckPair *succeeded_pair;
guint64 priority;
......
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