Skip to content
Snippets Groups Projects
Commit e9caa150 authored by Mohamed Chibani's avatar Mohamed Chibani
Browse files

ice: fix issue in nomination of triggered checks

There is an issue with the nomination of triggered checks in ICE
that causes ICE negotiation to conclude on sub-optimal pairs.
The issue was reported, and acknolegded by PJSIP maintainers.
This patch will fix the issue.

Gitlab: #622

Change-Id: I4bed7191692051d6215fbde8ee2ca9f94b76e0f9
parent e26f9399
No related branches found
No related tags found
No related merge requests found
diff --git a/pjnath/src/pjnath/ice_session.c b/pjnath/src/pjnath/ice_session.c
index ef88807a1..5feb32b21 100644
--- a/pjnath/src/pjnath/ice_session.c
+++ b/pjnath/src/pjnath/ice_session.c
@@ -3382,14 +3382,22 @@ static void handle_incoming_check(pj_ice_sess *ice,
if (c->state == PJ_ICE_SESS_CHECK_STATE_FROZEN ||
c->state == PJ_ICE_SESS_CHECK_STATE_WAITING)
{
- /* See if we shall nominate this check */
- pj_bool_t nominate = (c->nominated || ice->is_nominating);
-
- LOG5((ice->obj_name, "Performing triggered check for check %d",i));
- pj_log_push_indent();
- perform_check(ice, &ice->clist, i, nominate);
- pj_log_pop_indent();
+ /* If we are nominating in regular nomination, don't nominate this
+ * triggered check immediately, just wait for its scheduled check.
+ */
+ if (ice->is_nominating && !ice->opt.aggressive) {
+ LOG5((ice->obj_name, "Triggered check for check %d not "
+ "performed because nomination is in progress", i));
+ } else {
+ /* See if we shall nominate this check */
+ pj_bool_t nominate = (c->nominated || ice->is_nominating);
+ LOG5((ice->obj_name, "Performing triggered check for "
+ "check %d",i));
+ pj_log_push_indent();
+ perform_check(ice, &ice->clist, i, nominate);
+ pj_log_pop_indent();
+ }
} else if (c->state == PJ_ICE_SESS_CHECK_STATE_IN_PROGRESS) {
/* Should retransmit immediately
*/
@@ -3449,7 +3457,7 @@ static void handle_incoming_check(pj_ice_sess *ice,
else if (ice->clist.count < PJ_ICE_MAX_CHECKS) {
pj_ice_sess_check *c = &ice->clist.checks[ice->clist.count];
- pj_bool_t nominate;
+ unsigned check_id = ice->clist.count;
c->lcand = lcand;
c->rcand = rcand;
@@ -3457,19 +3465,38 @@ static void handle_incoming_check(pj_ice_sess *ice,
c->state = PJ_ICE_SESS_CHECK_STATE_WAITING;
c->nominated = rcheck->use_candidate;
c->err_code = PJ_SUCCESS;
+ ++ice->clist.count;
- nominate = (c->nominated || ice->is_nominating);
+ LOG4((ice->obj_name, "New triggered check added: %d", check_id));
- LOG4((ice->obj_name, "New triggered check added: %d",
- ice->clist.count));
- pj_log_push_indent();
+ /* If we are nominating in regular nomination, don't nominate this
+ * newly found pair.
+ */
+ if (ice->is_nominating && !ice->opt.aggressive) {
+ LOG5((ice->obj_name, "Triggered check for check %d not "
+ "performed because nomination is in progress", check_id));
+
+ /* Just in case the periodic check has been stopped (due to no more
+ * pair to check), let's restart it for this pair.
+ */
+ if (!pj_timer_entry_running(&ice->clist.timer)) {
+ pj_time_val delay = {0, 0};
+ pj_timer_heap_schedule_w_grp_lock(ice->stun_cfg.timer_heap,
+ &ice->clist.timer, &delay,
+ PJ_TRUE, ice->grp_lock);
+ }
+ } else {
+ pj_bool_t nominate;
+ nominate = (c->nominated || ice->is_nominating);
+
+ pj_log_push_indent();
+ perform_check(ice, &ice->clist, check_id, nominate);
+ pj_log_pop_indent();
+ }
- perform_check(ice, &ice->clist, ice->clist.count++, nominate);
/* Re-sort the list because of the newly added pair. */
sort_checklist(ice, &ice->clist);
- pj_log_pop_indent();
-
} else {
LOG4((ice->obj_name, "Error: unable to perform triggered check: "
"TOO MANY CHECKS IN CHECKLIST!"));
......@@ -23,6 +23,7 @@
"0017-auto-register-thread.patch",
"0018-fix-ioqueue-lock-acquire.patch",
"0019-resort-check-list-after-adding-prflx.patch",
"0020-avoid-immediate-nominating-triggered-check.patch",
"0001-win-config.patch",
"0002-win-vs-gnutls.patch",
"0003-win-vs2017-props.patch"
......
......@@ -64,7 +64,8 @@ pjproject: pjproject-$(PJPROJECT_VERSION).tar.gz .sum-pjproject
$(APPLY) $(SRC)/pjproject/0016-use-larger-Ta-interval.patch
$(APPLY) $(SRC)/pjproject/0017-auto-register-thread.patch
$(APPLY) $(SRC)/pjproject/0018-fix-ioqueue-lock-acquire.patch
$(APPLY) $(SRC)/pjproject/0019-resort-check-list-after-adding-prflx.patch
$(APPLY) $(SRC)/pjproject/0019-resort-check-list-after-adding-prflx.patch # TODO remove with 2.12 (https://github.com/pjsip/pjproject/pull/2806)
$(APPLY) $(SRC)/pjproject/0020-avoid-immediate-nominating-triggered-check.patch # TODO remove with 2.12 (https://github.com/pjsip/pjproject/issues/2812)
ifdef HAVE_ANDROID
$(APPLY) $(SRC)/pjproject/0001-android.patch
endif
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment