Hi all,
I am just adding a new patch version, w.r.t. revision 5214, as an
attachment.
Regards,
Dusan Klinec (ph4r05)
D?a 10.12.15 o 20:09 Dusan Klinec nap?sal(a):
> Hi all,
>
> I would like to report a bug I am encountering and to contribute a patch
> fixing it.
>
> * Bug description: When answering a call on iOS, it gives error 488
> (Not Acceptable Here). Reason: too many ICE candidates.
>
> * Bug reproducibility: Low/medium
>
> * Environment:
> - The latest stable PJSIP version, 2.4.5.
> - iOS 9.1
> - PJSIP configured to use STUN, ICE, TURN.
> - #define PJ_IPHONE_OS_HAS_MULTITASKING_SUPPORT 1
> - ICE max candidates set to 16, SDP ICE max candidates set to 8
>
> * Detailed description:
> When making a voice call, ICE submodule builds list of a candidates. If
> host candidates are allowed, local host addresses are determined. In
> some cases we are experiencing a situation where there are multiple ICE
> host candidates with exactly the same IP address for the same ICE
> component. It is visible ICE is adding the same candidate to the
> candidate list from the logs. We had 3 duplicate occurrences of the same
> IP and 2 duplicates of another IP address.
>
> This happens when iPhone is connected to multiple networks (4G internet,
> wifi, VPN).
> I don't know why resolving local IP addresses returns duplicates (maybe
> due to aliasing?). But I append a patch (bottom of the mail) that checks
> for ICE candidate duplicity in the list, not adding a duplicate entry to
> candidate list (increasing MAX ICE candidate count is not a solution).
> Algorithm time complexity is not optimal (checks all previous entries on
> adding a new one), but very easy to implement and to understand.
>
> The patch is against rev5206. There could be minor modifications needed
> to apply the patch as we apply several patches on top of official
> repository. But I guess idea is pretty clear.
>
> Cheers.
>
> Dusan Klinec (ph4r05)
>
> diff --git a/pjnath/src/pjnath/ice_strans.c
> b/pjnath/src/pjnath/ice_strans.c
> index c251c7b..5fbbaf1 100644
> --- a/pjnath/src/pjnath/ice_strans.c
> +++ b/pjnath/src/pjnath/ice_strans.c
> @@ -347,6 +347,34 @@ static pj_status_t add_update_turn(pj_ice_strans
> *ice_st,
> return PJ_SUCCESS;
> }
>
> +static pj_bool_t pj_ice_cand_equals(pj_ice_sess_cand *lcand,
> pj_ice_sess_cand *rcand){
> + if (lcand == NULL && rcand == NULL){
> + return PJ_TRUE;
> + }
> + if (lcand == NULL || rcand == NULL){
> + return PJ_FALSE;
> + }
> +
> + if (lcand->type != rcand->type
> + || lcand->status != rcand->status
> + || lcand->comp_id != rcand->comp_id
> + || lcand->transport_id != rcand->transport_id
> + || lcand->local_pref != rcand->local_pref
> + || lcand->prio != rcand->prio
> + || pj_sockaddr_cmp(&lcand->addr, &rcand->addr) != 0
> + || pj_sockaddr_cmp(&lcand->base_addr, &rcand->base_addr) != 0)
> + {
> + return PJ_FALSE;
> + }
> +
> +
> + if (lcand->foundation.slen != rcand->foundation.slen
> + || strncmp(lcand->foundation.ptr, rcand->foundation.ptr,
> (size_t)lcand->foundation.slen) != 0){
> + return PJ_FALSE;
> + }
> +
> + return PJ_TRUE;
> +}
>
> /*
> * Create the component.
> @@ -471,7 +499,8 @@ static pj_status_t create_comp(pj_ice_strans
> *ice_st, unsigned comp_id)
> */
> if (ice_st->cfg.stun.max_host_cands &&
> !pj_ice_is_cand_blocked(&ice_st->cfg.opt, PJ_ICE_CAND_TYPE_HOST)) {
> pj_stun_sock_info stun_sock_info;
> - unsigned i;
> + unsigned i,j;
> + pj_bool_t cand_duplicate = PJ_FALSE;
>
> /* Enumerate addresses */
> status = pj_stun_sock_get_info(comp->stun_sock, &stun_sock_info);
> @@ -498,7 +527,7 @@ static pj_status_t create_comp(pj_ice_strans
> *ice_st, unsigned comp_id)
> continue;
> }
>
> - cand = &comp->cand_list[comp->cand_cnt++];
> + cand = &comp->cand_list[comp->cand_cnt];
>
> cand->type = PJ_ICE_CAND_TYPE_HOST;
> cand->status = PJ_SUCCESS;
> @@ -511,6 +540,27 @@ static pj_status_t create_comp(pj_ice_strans
> *ice_st, unsigned comp_id)
> pj_ice_calc_foundation(ice_st->pool, &cand->foundation,
> cand->type, &cand->base_addr);
>
> + /* Check if not already in list */
> + for (j=0; j<comp->cand_cnt; j++){
> + if (pj_ice_cand_equals(cand, &comp->cand_list[j])){
> + cand_duplicate = PJ_TRUE;
> + break;
> + }
> + }
> +
> + if (cand_duplicate){
> + PJ_LOG(4,(ice_st->obj_name,
> + "Comp %d: host candidate %s is a duplicate",
> + comp_id, pj_sockaddr_print(&cand->addr, addrinfo,
> + sizeof(addrinfo), 3)));
> +
> + pj_bzero(&cand->addr, sizeof(cand->addr));
> + pj_bzero(&cand->base_addr, sizeof(cand->base_addr));
> + continue;
> + } else {
> + comp->cand_cnt+=1;
> + }
> +
> PJ_LOG(4,(ice_st->obj_name,
> "Comp %d: host candidate %s added",
> comp_id, pj_sockaddr_print(&cand->addr, addrinfo,
>
>
> ============================================================
>
>
>
>
>
>
-------------- next part --------------
diff --git a/pjnath/src/pjnath/ice_strans.c b/pjnath/src/pjnath/ice_strans.c
index c20debe..a22ca4d 100644
--- a/pjnath/src/pjnath/ice_strans.c
+++ b/pjnath/src/pjnath/ice_strans.c
@@ -347,6 +347,28 @@ static pj_status_t add_update_turn(pj_ice_strans *ice_st,
return PJ_SUCCESS;
}
+static pj_bool_t pj_ice_cand_equals(pj_ice_sess_cand *lcand, pj_ice_sess_cand *rcand){
+ if (lcand == NULL && rcand == NULL){
+ return PJ_TRUE;
+ }
+ if (lcand == NULL || rcand == NULL){
+ return PJ_FALSE;
+ }
+
+ if (lcand->type != rcand->type
+ || lcand->status != rcand->status
+ || lcand->comp_id != rcand->comp_id
+ || lcand->transport_id != rcand->transport_id
+ || lcand->local_pref != rcand->local_pref
+ || lcand->prio != rcand->prio
+ || pj_sockaddr_cmp(&lcand->addr, &rcand->addr) != 0
+ || pj_sockaddr_cmp(&lcand->base_addr, &rcand->base_addr) != 0)
+ {
+ return PJ_FALSE;
+ }
+
+ return PJ_TRUE;
+}
/*
* Create the component.
@@ -468,7 +490,8 @@ static pj_status_t create_comp(pj_ice_strans *ice_st, unsigned comp_id)
*/
if (ice_st->cfg.stun.max_host_cands) {
pj_stun_sock_info stun_sock_info;
- unsigned i;
+ unsigned i,j;
+ pj_bool_t cand_duplicate = PJ_FALSE;
/* Enumerate addresses */
status = pj_stun_sock_get_info(comp->stun_sock, &stun_sock_info);
@@ -495,7 +518,7 @@ static pj_status_t create_comp(pj_ice_strans *ice_st, unsigned comp_id)
continue;
}
- cand = &comp->cand_list[comp->cand_cnt++];
+ cand = &comp->cand_list[comp->cand_cnt];
cand->type = PJ_ICE_CAND_TYPE_HOST;
cand->status = PJ_SUCCESS;
@@ -505,6 +528,28 @@ static pj_status_t create_comp(pj_ice_strans *ice_st, unsigned comp_id)
pj_sockaddr_cp(&cand->addr, addr);
pj_sockaddr_cp(&cand->base_addr, addr);
pj_bzero(&cand->rel_addr, sizeof(cand->rel_addr));
+
+ /* Check if not already in list */
+ for (j=0; j<comp->cand_cnt; j++){
+ if (pj_ice_cand_equals(cand, &comp->cand_list[j])){
+ cand_duplicate = PJ_TRUE;
+ break;
+ }
+ }
+
+ if (cand_duplicate){
+ PJ_LOG(4,(ice_st->obj_name,
+ "Comp %d: host candidate %s is a duplicate",
+ comp_id, pj_sockaddr_print(&cand->addr, addrinfo,
+ sizeof(addrinfo), 3)));
+
+ pj_bzero(&cand->addr, sizeof(cand->addr));
+ pj_bzero(&cand->base_addr, sizeof(cand->base_addr));
+ continue;
+ } else {
+ comp->cand_cnt+=1;
+ }
+
pj_ice_calc_foundation(ice_st->pool, &cand->foundation,
cand->type, &cand->base_addr);
