From 0512ecaa2d29b901e67a41b04f46175af0c193f1 Mon Sep 17 00:00:00 2001 From: Fabrice Bellet Date: Fri, 28 Jun 2019 13:45:11 +0200 Subject: conncheck: define a property for a final idle timeout This final idle timeout is renamed from the NICE_AGENT_MAX_TIMER_GRACE_PERIOD macro, and keeps its semantic. We also increase the default value of this timeout from 1 second to 5 seconds. This is useful for the sipe pidgin plugin that has to deal with SfB agents, that may take some time in controlling mode before choosing and testing the nominated pair --- agent/agent-priv.h | 15 ++------------ agent/agent.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ agent/conncheck.c | 12 ++++++------ 3 files changed, 65 insertions(+), 19 deletions(-) diff --git a/agent/agent-priv.h b/agent/agent-priv.h index ee1000e..4b47252 100644 --- a/agent/agent-priv.h +++ b/agent/agent-priv.h @@ -122,18 +122,6 @@ nice_input_message_iter_compare (const NiceInputMessageIter *a, ((obj)->compatibility == NICE_COMPATIBILITY_RFC5245 || \ (obj)->compatibility == NICE_COMPATIBILITY_OC2007R2) -/* A grace period before declaring a component as failed, in msecs. This - * delay is added to reduce the chance to see the agent receiving new - * stun activity just after the conncheck list has been declared failed, - * reactiviting conncheck activity, and causing a (valid) state - * transitions like that: connecting -> failed -> connecting -> - * connected -> ready. - * Such transitions are not buggy per-se, but may break the - * test-suite, that counts precisely the number of time each state - * has been set, and doesnt expect these transcient failed states. - */ -#define NICE_AGENT_MAX_TIMER_GRACE_PERIOD 1000 - struct _NiceAgent { GObject parent; /* gobject pointer */ @@ -157,6 +145,7 @@ struct _NiceAgent guint stun_reliable_timeout; /* property: stun reliable timeout */ NiceNominationMode nomination_mode; /* property: Nomination mode */ gboolean support_renomination; /* property: support RENOMINATION STUN attribute */ + guint idle_timeout; /* property: conncheck timeout before stop */ GSList *local_addresses; /* list of NiceAddresses for local interfaces */ @@ -193,7 +182,7 @@ struct _NiceAgent gboolean use_ice_tcp; gboolean use_ice_trickle; - guint conncheck_timer_grace_period; /* ongoing delay before timer stop */ + guint conncheck_ongoing_idle_delay; /* ongoing delay before timer stop */ gboolean controlling_mode; /* controlling mode used by the conncheck */ /* XXX: add pointer to internal data struct for ABI-safe extensions */ diff --git a/agent/agent.c b/agent/agent.c index cc1b5c8..b232690 100644 --- a/agent/agent.c +++ b/agent/agent.c @@ -81,9 +81,11 @@ #define DEFAULT_STUN_PORT 3478 #define DEFAULT_UPNP_TIMEOUT 200 /* milliseconds */ +#define DEFAULT_IDLE_TIMEOUT 5000 /* milliseconds */ #define MAX_TCP_MTU 1400 /* Use 1400 because of VPNs and we assume IEE 802.3 */ + static void nice_debug_input_message_composition (const NiceInputMessage *messages, guint n_messages); @@ -120,6 +122,7 @@ enum PROP_NOMINATION_MODE, PROP_ICE_TRICKLE, PROP_SUPPORT_RENOMINATION, + PROP_IDLE_TIMEOUT, }; @@ -484,6 +487,51 @@ nice_agent_class_init (NiceAgentClass *klass) FALSE, G_PARAM_READWRITE)); + /** + * NiceAgent:idle-timeout + * + * A final timeout in msec, launched when the agent becomes idle, + * before stopping its activity. + * + * This timer will delay the decision to set a component as failed. + * This delay is added to reduce the chance to see the agent receiving + * new stun activity just after the conncheck list has been declared + * failed (some valid pairs, no nominated pair, and no in-progress + * pairs), reactiviting conncheck activity, and causing a (valid) + * state transitions like that: connecting -> failed -> connecting -> + * connected -> ready. Such transitions are not buggy per-se, but may + * break the test-suite, that counts precisely the number of time each + * state has been set, and doesnt expect these transcient failed + * states. + * + * This timer is also useful when the agent is in controlled mode and + * the other controlling peer takes some time to elect its nominated + * pair (this may be the case for SfB peers). + * + * This timer is *NOT* part if the RFC5245, as this situation is not + * covered in sect 8.1.2 "Updating States", but deals with a real + * use-case, where a controlled agent can not wait forever for the + * other peer to make a nomination decision. + * + * Also note that the value of this timeout will not delay the + * emission of 'connected' and 'ready' agent signals, and will not + * slow down the behaviour of the agent when the peer agent works + * in a timely manner. + * + * Since: 0.1.17 + */ + + g_object_class_install_property (gobject_class, PROP_IDLE_TIMEOUT, + g_param_spec_uint ( + "idle-timeout", + "Timeout before stopping the agent when being idle", + "A final timeout in msecs, launched when the agent becomes idle, " + "with no in-progress pairs to wait for, before stopping its activity, " + "and declaring a component as failed in needed.", + 50, 60000, + DEFAULT_IDLE_TIMEOUT, + G_PARAM_READWRITE | G_PARAM_CONSTRUCT)); + /** * NiceAgent:proxy-ip: * @@ -1203,6 +1251,7 @@ nice_agent_init (NiceAgent *agent) agent->max_conn_checks = NICE_AGENT_MAX_CONNECTIVITY_CHECKS_DEFAULT; agent->nomination_mode = NICE_NOMINATION_MODE_AGGRESSIVE; agent->support_renomination = FALSE; + agent->idle_timeout = DEFAULT_IDLE_TIMEOUT; agent->discovery_list = NULL; agent->discovery_unsched_items = 0; @@ -1327,6 +1376,10 @@ nice_agent_get_property ( g_value_set_boolean (value, agent->support_renomination); break; + case PROP_IDLE_TIMEOUT: + g_value_set_uint (value, agent->idle_timeout); + break; + case PROP_PROXY_IP: g_value_set_string (value, agent->proxy_ip); break; @@ -1543,6 +1596,10 @@ nice_agent_set_property ( agent->support_renomination = g_value_get_boolean (value); break; + case PROP_IDLE_TIMEOUT: + agent->idle_timeout = g_value_get_uint (value); + break; + case PROP_PROXY_IP: g_free (agent->proxy_ip); agent->proxy_ip = g_value_dup_string (value); diff --git a/agent/conncheck.c b/agent/conncheck.c index 369dd0a..df298ea 100644 --- a/agent/conncheck.c +++ b/agent/conncheck.c @@ -1105,7 +1105,7 @@ conn_check_stop (NiceAgent *agent) g_source_destroy (agent->conncheck_timer_source); g_source_unref (agent->conncheck_timer_source); agent->conncheck_timer_source = NULL; - agent->conncheck_timer_grace_period = 0; + agent->conncheck_ongoing_idle_delay = 0; } @@ -1192,18 +1192,18 @@ static gboolean priv_conn_check_tick_agent_locked (NiceAgent *agent, * failed. Components marked connected, and then ready follow another * code path, and are not concerned by this grace period. */ - if (!keep_timer_going && agent->conncheck_timer_grace_period == 0) + if (!keep_timer_going && agent->conncheck_ongoing_idle_delay == 0) nice_debug ("Agent %p : waiting %d msecs before checking " - "for failed components.", agent, NICE_AGENT_MAX_TIMER_GRACE_PERIOD); + "for failed components.", agent, agent->idle_timeout); if (keep_timer_going) - agent->conncheck_timer_grace_period = 0; + agent->conncheck_ongoing_idle_delay = 0; else - agent->conncheck_timer_grace_period += agent->timer_ta; + agent->conncheck_ongoing_idle_delay += agent->timer_ta; /* step: stop timer if no work left */ if (!keep_timer_going && - agent->conncheck_timer_grace_period >= NICE_AGENT_MAX_TIMER_GRACE_PERIOD) { + agent->conncheck_ongoing_idle_delay >= agent->idle_timeout) { nice_debug ("Agent %p : checking for failed components now.", agent); for (i = agent->streams; i; i = i->next) { NiceStream *stream = i->data; -- cgit v1.2.1