Skip to content

Commit

Permalink
ICE - rework TCP reconnection
Browse files Browse the repository at this point in the history 
Introduce two main changes to the TCP patch:
- limit the number of connection retries
- use a higher Ta for TCP connection retry

Gitlab: #634

Change-Id: Ifbddcceb2c4d15d6c2bed6c62ae8cf5c535892cf
  • Loading branch information
@mchibani-sfl
mchibani-sfl authored and Sébastien Blin committed Sep 7, 2023
1 parent 5774b54 commit 6eb7a17
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 66 deletions.
11 changes: 0 additions & 11 deletions pjlib/src/pj/ioqueue_common_abs.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,19 +89,11 @@ union operation_key
};

#if PJ_IOQUEUE_HAS_SAFE_UNREG
<<<<<<< HEAD
# define UNREG_FIELDS \
unsigned ref_count; \
pj_bool_t closing; \
pj_time_val free_time; \

=======
# define UNREG_FIELDS \
unsigned ref_count; \
pj_bool_t closing; \
pj_time_val free_time; \

>>>>>>> 26a1072f6 (0008-fix_ioqueue_ipv6_sendto)
#else
# define UNREG_FIELDS
#endif
Expand Down Expand Up @@ -148,10 +140,7 @@ static void ioqueue_add_to_set2(pj_ioqueue_t *ioqueue,
static void ioqueue_remove_from_set( pj_ioqueue_t *ioqueue,
pj_ioqueue_key_t *key,
enum ioqueue_event_type event_type);
<<<<<<< HEAD
static void ioqueue_remove_from_set2(pj_ioqueue_t *ioqueue,
pj_ioqueue_key_t *key,
unsigned event_types);

=======
>>>>>>> 26a1072f6 (0008-fix_ioqueue_ipv6_sendto)
9 changes: 8 additions & 1 deletion pjnath/include/pjnath/ice_session.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,14 @@ struct pj_ice_sess_check
* When the check failed, this will contain the failure status of the
* STUN transaction.
*/
pj_status_t err_code;
pj_status_t err_code;

#if PJ_HAS_TCP
/**
* TCP reconnection attemps counter.
*/
int reconnect_count;
#endif
};


Expand Down
110 changes: 75 additions & 35 deletions pjnath/src/pjnath/ice_session.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2278,6 +2278,9 @@ static pj_status_t add_rcand_and_update_checklist(
chk->state = PJ_ICE_SESS_CHECK_STATE_FROZEN;
chk->foundation_idx = get_check_foundation_idx(ice, lcand, rcand,
PJ_TRUE);
#if PJ_HAS_TCP
chk->reconnect_count = 0;
#endif

/* Check if foundation cannot be added (e.g: list is full) */
if (chk->foundation_idx < 0)
Expand Down Expand Up @@ -2679,9 +2682,13 @@ static pj_status_t perform_check(pj_ice_sess *ice,
if (ice->timer_connect.id != TIMER_NONE) {
pj_assert(!"Not expected any timer active");
} else {
LOG5((ice->obj_name,
"Scheduling connection time-out for check %s",
dump_check(ice->tmp.txt, sizeof(ice->tmp.txt), clist, check)));

pj_time_val delay = {
.sec = 15,
.msec = 0,
.sec = 0,
.msec = PJ_ICE_TCP_CONNECTION_TIMEOUT,
};
pj_time_val_normalize(&delay);
pj_timer_heap_schedule_w_grp_lock(ice->stun_cfg.timer_heap,
Expand Down Expand Up @@ -2735,6 +2742,8 @@ static pj_status_t start_periodic_check(pj_timer_heap_t *th,
td = (struct timer_data*) te->user_data;
ice = td->ice;
clist = td->clist;
// Default Ta timer
pj_time_val timeout = {0, PJ_ICE_TA_VAL};

pj_grp_lock_acquire(ice->grp_lock);

Expand All @@ -2751,6 +2760,31 @@ static pj_status_t start_periodic_check(pj_timer_heap_t *th,

pj_ice_sess_check *check = NULL;

/* Send STUN Binding request for check with highest priority on
* Waiting state.
*/

if (start_count == 0) {
for (i = 0; i < clist->count; ++i) {
check = &clist->checks[i];

if (check->state == PJ_ICE_SESS_CHECK_STATE_WAITING) {
LOG5((ice->obj_name, "Starting periodic check for check %i (was waiting)", i));
pj_log_push_indent();

status = perform_check(ice, clist, i, ice->is_nominating);
if (status != PJ_SUCCESS && status != PJ_EPENDING) {
check_set_state(ice, check, PJ_ICE_SESS_CHECK_STATE_FAILED,
status);
on_check_complete(ice, check);
}
++start_count;
break;
}
}
}

#if PJ_HAS_TCP
/* Send STUN Binding request for check with highest priority on
* Retry state.
*/
Expand All @@ -2768,6 +2802,9 @@ static pj_status_t start_periodic_check(pj_timer_heap_t *th,
status);
on_check_complete(ice, check);
}
timeout.msec = PJ_ICE_TCP_RECONNECTION_DELAY;
timeout.sec = 0;

++start_count;
break;
}
Expand Down Expand Up @@ -2799,30 +2836,7 @@ static pj_status_t start_periodic_check(pj_timer_heap_t *th,
}
}
}

/* Send STUN Binding request for check with highest priority on
* Waiting state.
*/

if (start_count == 0) {
for (i = 0; i < clist->count; ++i) {
check = &clist->checks[i];

if (check->state == PJ_ICE_SESS_CHECK_STATE_WAITING) {
LOG5((ice->obj_name, "Starting periodic check for check %i (was waiting)", i));
pj_log_push_indent();

status = perform_check(ice, clist, i, ice->is_nominating);
if (status != PJ_SUCCESS && status != PJ_EPENDING) {
check_set_state(ice, check, PJ_ICE_SESS_CHECK_STATE_FAILED,
status);
on_check_complete(ice, check);
}
++start_count;
break;
}
}
}
#endif

/* If we don't have anything in Waiting state, perform check to
* highest priority pair that is in Frozen state.
Expand All @@ -2845,6 +2859,7 @@ static pj_status_t start_periodic_check(pj_timer_heap_t *th,
}
}

#if PJ_HAS_TCP
if (start_count == 0) {
// If all sockets are pending, do nothing
for (i = 0; i < clist->count; ++i) {
Expand All @@ -2855,13 +2870,12 @@ static pj_status_t start_periodic_check(pj_timer_heap_t *th,
}
}
}
#endif

// The checks are performed at the rate of 1 check per Ta
// interval. If a new check was started, we need to re-schedule
// for the next one (if any).
if (start_count!=0) {
pj_time_val timeout = {0, PJ_ICE_TA_VAL};

pj_time_val_normalize(&timeout);
pj_timer_heap_schedule_w_grp_lock(th, te, &timeout, PJ_TRUE,
ice->grp_lock);
Expand Down Expand Up @@ -3172,12 +3186,22 @@ void ice_sess_on_peer_connection(pj_ice_sess *ice,
* the relayed candidate. This is done by set_perm from the other case.
* But from this side, we can't know if the peer has authorized us. If it's
* not the case, the connection will got a CONNECTION RESET BY PEER status.
* In this case, we can try to reconnect a bit after and this until the check
* reached its timeout.
* In this case, we try to reconnect few times with a delay between two
* attempts.
*/
check->state = PJ_ICE_SESS_CHECK_STATE_NEEDS_RETRY;
check_set_state(ice, check,PJ_ICE_SESS_CHECK_STATE_NEEDS_RETRY,
status);
if (check->reconnect_count < PJ_ICE_TCP_MAX_RECONNECTION_COUNT) {
check->state = PJ_ICE_SESS_CHECK_STATE_NEEDS_RETRY;
check_set_state(ice, check,PJ_ICE_SESS_CHECK_STATE_NEEDS_RETRY,
status);
check->reconnect_count++;
} else {
// Max attempts reached. Fail this check.
LOG4((ice->obj_name, "Check %s: connection failed after %d attempts",
dump_check(ice->tmp.txt, sizeof(ice->tmp.txt), &ice->clist, check),
PJ_ICE_TCP_MAX_RECONNECTION_COUNT));
check_set_state(ice, check, PJ_ICE_SESS_CHECK_STATE_FAILED, status);
on_check_complete(ice, check);
}
pj_grp_lock_release(ice->grp_lock);
return;
} else if (status != PJ_SUCCESS) {
Expand Down Expand Up @@ -3235,8 +3259,21 @@ void ice_sess_on_peer_connection(pj_ice_sess *ice,
* In this case, we can try to reconnect a bit after and this until the check
* reached its timeout.
*/
check_set_state(ice, check,PJ_ICE_SESS_CHECK_STATE_NEEDS_RETRY,
status);

if (check->reconnect_count < PJ_ICE_TCP_MAX_RECONNECTION_COUNT) {
check_set_state(ice, check, PJ_ICE_SESS_CHECK_STATE_NEEDS_RETRY,
status);
check->reconnect_count++;
} else {
// Max attempts reached. Fail this check.
LOG4((ice->obj_name, "Check %s: connection failed after %d attempts",
dump_check(ice->tmp.txt, sizeof(ice->tmp.txt), &ice->clist, check),
PJ_ICE_TCP_MAX_RECONNECTION_COUNT));
check_set_state(ice, check, PJ_ICE_SESS_CHECK_STATE_FAILED, status);
on_check_complete(ice, check);
pj_grp_lock_release(ice->grp_lock);
return;
}
} else if (status == PJ_EBUSY /* EBUSY */) {
check_set_state(ice, check, PJ_ICE_SESS_CHECK_STATE_NEEDS_FIRST_PACKET,
status);
Expand Down Expand Up @@ -3682,6 +3719,9 @@ static void on_stun_request_complete(pj_stun_session *stun_sess,
new_check->state = PJ_ICE_SESS_CHECK_STATE_SUCCEEDED;
new_check->nominated = check->nominated;
new_check->err_code = PJ_SUCCESS;
#if PJ_HAS_TCP
new_check->reconnect_count = 0;
#endif
} else {
new_check = &ice->valid_list.checks[i];
ice->valid_list.checks[i].nominated = check->nominated;
Expand Down
2 changes: 1 addition & 1 deletion pjnath/src/pjnath/ice_strans.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -891,7 +891,7 @@ static pj_bool_t add_local_candidate(pj_ice_sess_cand *cand,
return PJ_SUCCESS;
}
else if (stun_cfg->af == pj_AF_INET6()) {
pj_in6_addr in6addr = {{0}};
pj_in6_addr in6addr = {0};
in6addr.s6_addr[15] = 1;
if (pj_memcmp(&in6addr, &addr->ipv6.sin6_addr,
sizeof(in6addr))==0)
Expand Down
18 changes: 0 additions & 18 deletions pjsip/src/pjsip/sip_transport.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2480,31 +2480,13 @@ PJ_DEF(pj_status_t) pjsip_tpmgr_acquire_transport2(pjsip_tpmgr *mgr,

} else {

<<<<<<< HEAD
/* Find factory with type matches the destination type */
factory = mgr->factory_list.next;
while (factory != &mgr->factory_list) {
if (factory->type == type)
break;
factory = factory->next;
}
=======
/* Make sure we don't use another factory than the one given if
secure flag is set */
if (flag & PJSIP_TRANSPORT_SECURE) {
TRACE_((THIS_FILE, "Can't create new TLS transport with no "
"provided suitable TLS listener."));
return PJSIP_ETPNOTSUITABLE;
}

/* Find factory with type matches the destination type */
factory = mgr->factory_list.next;
while (factory != &mgr->factory_list) {
if (factory->type == type)
break;
factory = factory->next;
}
>>>>>>> e6a22f11e (0004 multiple listeners)

if (factory == &mgr->factory_list) {
/* No factory can create the transport! */
Expand Down

0 comments on commit 6eb7a17

Please sign in to comment.