Bug 1004530 - Part 1: Allow a grace period for trickle candidates to arrive when all candidate pairs have failed. r=drno, r=ekr

media/mtransport/nricectx.cpp

@@ -419,6 +419,7 @@ RefPtr<NrIceCtx> NrIceCtx::Create(const std::string& name,
     }
 
     NR_reg_set_uint4((char *)"stun.client.maximum_transmits",4);
+    NR_reg_set_uint4((char *)NR_ICE_REG_TRICKLE_GRACE_PERIOD, 5000);
   }
 
   // Create the ICE context

media/mtransport/third_party/nICEr/src/ice/ice_candidate_pair.c

@@ -280,7 +280,7 @@ static void nr_ice_candidate_pair_stun_cb(NR_SOCKET s, int how, void *cb_arg)
 
           nr_ice_candidate_pair_set_state(pair->pctx,pair,NR_ICE_PAIR_STATE_SUCCEEDED);
 
-          if(r=nr_ice_candidate_pair_insert(&pair->remote->stream->check_list,pair))
+          if(r=nr_ice_component_insert_pair(pair->remote->component,pair))
             ABORT(r);
 
           /* If the original pair was nominated, make us nominated,

media/mtransport/third_party/nICEr/src/ice/ice_component.c

@@ -641,7 +641,7 @@ static int nr_ice_component_process_incoming_check(nr_ice_component *comp, nr_tr
         }
         nr_ice_candidate_pair_set_state(pair->pctx,pair,NR_ICE_PAIR_STATE_FROZEN);
 
-        if(r=nr_ice_candidate_pair_insert(&comp->stream->check_list,pair)) {
+        if(r=nr_ice_component_insert_pair(comp,pair)) {
           *error=(r==R_NO_MEMORY)?500:400;
           nr_ice_candidate_pair_destroy(&pair);
           ABORT(r);
@@ -805,8 +805,7 @@ int nr_ice_component_pair_candidate(nr_ice_peer_ctx *pctx, nr_ice_component *pco
         if(r=nr_ice_candidate_pair_create(pctx,lcand,pcand,&pair))
           ABORT(r);
 
-        if(r=nr_ice_candidate_pair_insert(&pcomp->stream->check_list,
-                                          pair))
+        if(r=nr_ice_component_insert_pair(pcomp, pair))
           ABORT(r);
       }
 
@@ -942,13 +941,10 @@ int nr_ice_component_nominated_pair(nr_ice_component *comp, nr_ice_cand_pair *pa
     return(_status);
   }
 
-int nr_ice_component_failed_pair(nr_ice_component *comp, nr_ice_cand_pair *pair)
+static int nr_ice_component_have_all_pairs_failed(nr_ice_component *comp)
   {
-    int r,_status;
     nr_ice_cand_pair *p2;
 
-    assert(pair->state == NR_ICE_PAIR_STATE_FAILED);
-
     p2=TAILQ_FIRST(&comp->stream->check_list);
     while(p2){
       if(comp->component_id==p2->local->component_id){
@@ -956,13 +952,8 @@ int nr_ice_component_failed_pair(nr_ice_component *comp, nr_ice_cand_pair *pair)
         case NR_ICE_PAIR_STATE_FROZEN:
         case NR_ICE_PAIR_STATE_WAITING:
         case NR_ICE_PAIR_STATE_IN_PROGRESS:
-            /* answer component status cannot be determined yet */
-            goto done;
-            break;
         case NR_ICE_PAIR_STATE_SUCCEEDED:
-            /* the component will succeed */
-            goto done;
-            break;
+            return(0);
         case NR_ICE_PAIR_STATE_FAILED:
         case NR_ICE_PAIR_STATE_CANCELLED:
             /* states that will never be recovered from */
@@ -976,17 +967,28 @@ int nr_ice_component_failed_pair(nr_ice_component *comp, nr_ice_cand_pair *pair)
       p2=TAILQ_NEXT(p2,entry);
     }
 
-    /* all the pairs in the component are in their final states with
-     * none of them being SUCCEEDED, so the component fails entirely,
-     * tell the media stream that this component has failed */
+    return(1);
+  }
 
-    if(r=nr_ice_media_stream_component_failed(comp->stream,comp))
-      ABORT(r);
+int nr_ice_component_failed_pair(nr_ice_component *comp, nr_ice_cand_pair *pair)
+  {
+    return nr_ice_component_check_if_failed(comp);
+  }
 
-  done:
-    _status=0;
-  abort:
-    return(_status);
+int nr_ice_component_check_if_failed(nr_ice_component *comp)
+  {
+    if (comp->state == NR_ICE_COMPONENT_RUNNING) {
+      /* Don't do anything to streams that aren't currently running */
+      r_log(LOG_ICE,LOG_DEBUG,"ICE-PEER(%s)/STREAM(%s)/COMP(%d): Checking whether component needs to be marked failed.",comp->stream->pctx->label,comp->stream->label,comp->component_id);
+
+      if (!comp->stream->pctx->trickle_grace_period_timer &&
+          nr_ice_component_have_all_pairs_failed(comp)) {
+        r_log(LOG_ICE,LOG_INFO,"ICE-PEER(%s)/STREAM(%s)/COMP(%d): All pairs are failed, and grace period has elapsed. Marking component as failed.",comp->stream->pctx->label,comp->stream->label,comp->component_id);
+        return nr_ice_media_stream_component_failed(comp->stream,comp);
+      }
+    }
+
+    return(0);
   }
 
 int nr_ice_component_select_pair(nr_ice_peer_ctx *pctx, nr_ice_component *comp)
@@ -1042,7 +1044,6 @@ static void nr_ice_component_keepalive_cb(NR_SOCKET s, int how, void *cb_arg)
       keepalive_timeout=15000; /* Default */
     }
 
-
     if(comp->keepalive_needed)
       nr_stun_client_force_retransmit(comp->keepalive_ctx);
 
@@ -1084,3 +1085,35 @@ int nr_ice_component_finalize(nr_ice_component *lcomp, nr_ice_component *rcomp)
 
     return(_status);
   }
+
+
+int nr_ice_component_insert_pair(nr_ice_component *pcomp, nr_ice_cand_pair *pair)
+  {
+    int r,_status;
+
+    /* Pairs for peer reflexive are marked SUCCEEDED immediately */
+    if (pair->state != NR_ICE_PAIR_STATE_FROZEN &&
+        pair->state != NR_ICE_PAIR_STATE_SUCCEEDED){
+      assert(0);
+      ABORT(R_BAD_ARGS);
+    }
+
+    if(r=nr_ice_candidate_pair_insert(&pair->remote->stream->check_list,pair))
+      ABORT(r);
+
+    /* Make sure the check timer is running, if the stream was previously
+     * started. We will not start streams just because a pair was created. */
+    r_log(LOG_ICE,LOG_DEBUG,"ICE-PEER(%s)/CAND-PAIR(%s): Ensure that check timer is running for new pair %s.",pair->remote->stream->pctx->label, pair->codeword, pair->as_string);
+
+    if(pair->remote->stream->ice_state == NR_ICE_MEDIA_STREAM_CHECKS_ACTIVE){
+      if(nr_ice_media_stream_start_checks(pair->remote->stream->pctx, pair->remote->stream)) {
+        r_log(LOG_ICE,LOG_WARNING,"ICE-PEER(%s)/CAND-PAIR(%s): Could not restart checks for new pair %s.",pair->remote->stream->pctx->label, pair->codeword, pair->as_string);
+        ABORT(R_INTERNAL);
+      }
+    }
+
+    _status=0;
+  abort:
+    return(_status);
+  }
+

media/mtransport/third_party/nICEr/src/ice/ice_component.h

@@ -88,9 +88,11 @@ int nr_ice_component_pair_candidates(nr_ice_peer_ctx *pctx, nr_ice_component *lc
 int nr_ice_component_service_pre_answer_requests(nr_ice_peer_ctx *pctx, nr_ice_component *pcomp, char *username, int *serviced);
 int nr_ice_component_nominated_pair(nr_ice_component *comp, nr_ice_cand_pair *pair);
 int nr_ice_component_failed_pair(nr_ice_component *comp, nr_ice_cand_pair *pair);
+int nr_ice_component_check_if_failed(nr_ice_component *comp);
 int nr_ice_component_select_pair(nr_ice_peer_ctx *pctx, nr_ice_component *comp);
 int nr_ice_component_set_failed(nr_ice_component *comp);
 int nr_ice_component_finalize(nr_ice_component *lcomp, nr_ice_component *rcomp);
+int nr_ice_component_insert_pair(nr_ice_component *pcomp, nr_ice_cand_pair *pair);
 
 #ifdef __cplusplus
 }

media/mtransport/third_party/nICEr/src/ice/ice_media_stream.c

@@ -394,10 +394,25 @@ static void nr_ice_media_stream_check_timer_cb(NR_SOCKET s, int h, void *cb_arg)
 /* Start checks for this media stream (aka check list) */
 int nr_ice_media_stream_start_checks(nr_ice_peer_ctx *pctx, nr_ice_media_stream *stream)
   {
-    nr_ice_media_stream_set_state(stream,NR_ICE_MEDIA_STREAM_CHECKS_ACTIVE);
-    nr_ice_media_stream_check_timer_cb(0,0,stream);
+    int r,_status;
 
-    return(0);
+    /* Don't start the check timer if the stream is done (failed/completed) */
+    if (stream->ice_state > NR_ICE_MEDIA_STREAM_CHECKS_ACTIVE) {
+      assert(0);
+      ABORT(R_INTERNAL);
+    }
+
+    if(r=nr_ice_media_stream_set_state(stream,NR_ICE_MEDIA_STREAM_CHECKS_ACTIVE))
+      ABORT(r);
+
+    if (!stream->timer) {
+      r_log(LOG_ICE,LOG_INFO,"ICE-PEER(%s)/ICE-STREAM(%s): Starting check timer for stream.",pctx->label,stream->label);
+      nr_ice_media_stream_check_timer_cb(0,0,stream);
+    }
+
+    _status=0;
+  abort:
+    return(_status);
   }
 
 /* Start checks for this media stream (aka check list) S 5.7 */

media/mtransport/third_party/nICEr/src/ice/ice_peer_ctx.c

@@ -36,6 +36,7 @@ static char *RCSSTRING __UNUSED__="$Id: ice_peer_ctx.c,v 1.2 2008/04/28 17:59:01
 
 #include <string.h>
 #include <assert.h>
+#include <registry.h>
 #include <nr_api.h>
 #include "ice_ctx.h"
 #include "ice_peer_ctx.h"
@@ -43,6 +44,7 @@ static char *RCSSTRING __UNUSED__="$Id: ice_peer_ctx.c,v 1.2 2008/04/28 17:59:01
 #include "ice_util.h"
 #include "nr_crypto.h"
 #include "async_timer.h"
+#include "ice_reg.h"
 
 static void nr_ice_peer_ctx_destroy_cb(NR_SOCKET s, int how, void *cb_arg);
 static int nr_ice_peer_ctx_parse_stream_attributes_int(nr_ice_peer_ctx *pctx, nr_ice_media_stream *stream, nr_ice_media_stream *pstream, char **attrs, int attr_ct);
@@ -323,6 +325,40 @@ int nr_ice_peer_ctx_parse_trickle_candidate(nr_ice_peer_ctx *pctx, nr_ice_media_
   }
 
 
+static void nr_ice_peer_ctx_trickle_wait_cb(NR_SOCKET s, int how, void *cb_arg)
+  {
+    nr_ice_peer_ctx *pctx=cb_arg;
+    nr_ice_media_stream *stream;
+    nr_ice_component *comp;
+
+    pctx->trickle_grace_period_timer=0;
+
+    r_log(LOG_ICE,LOG_INFO,"ICE(%s): peer (%s) Trickle grace period is over; marking every component with only failed pairs as failed.",pctx->ctx->label,pctx->label);
+
+    stream=STAILQ_FIRST(&pctx->peer_streams);
+    while(stream){
+      comp=STAILQ_FIRST(&stream->components);
+      while(comp){
+        nr_ice_component_check_if_failed(comp);
+        comp=STAILQ_NEXT(comp,entry);
+      }
+      stream=STAILQ_NEXT(stream,entry);
+    }
+  }
+
+static void nr_ice_peer_ctx_start_trickle_timer(nr_ice_peer_ctx *pctx)
+  {
+    UINT4 grace_period_timeout=0;
+
+    NR_reg_get_uint4(NR_ICE_REG_TRICKLE_GRACE_PERIOD,&grace_period_timeout);
+
+    if (grace_period_timeout) {
+      /* If we're doing trickle, we need to allow a grace period for new
+       * trickle candidates to arrive in case the pairs we have fail quickly. */
+       NR_ASYNC_TIMER_SET(grace_period_timeout,nr_ice_peer_ctx_trickle_wait_cb,pctx,&pctx->trickle_grace_period_timer);
+    }
+  }
+
 int nr_ice_peer_ctx_pair_candidates(nr_ice_peer_ctx *pctx)
   {
     nr_ice_media_stream *stream;
@@ -340,6 +376,9 @@ int nr_ice_peer_ctx_pair_candidates(nr_ice_peer_ctx *pctx)
      * up in UNPAIRED after creating some pairs. */
     pctx->state = NR_ICE_PEER_STATE_PAIRED;
 
+    /* Start grace period timer for incoming trickle candidates */
+    nr_ice_peer_ctx_start_trickle_timer(pctx);
+
     stream=STAILQ_FIRST(&pctx->peer_streams);
     while(stream){
       if(r=nr_ice_media_stream_pair_candidates(pctx, stream->local_stream,
@@ -415,6 +454,11 @@ static void nr_ice_peer_ctx_destroy_cb(NR_SOCKET s, int how, void *cb_arg)
     if (pctx->ctx)
       STAILQ_REMOVE(&pctx->ctx->peers, pctx, nr_ice_peer_ctx_, entry);
 
+    if(pctx->trickle_grace_period_timer) {
+      NR_async_timer_cancel(pctx->trickle_grace_period_timer);
+      pctx->trickle_grace_period_timer=0;
+    }
+
     RFREE(pctx);
   }
 

media/mtransport/third_party/nICEr/src/ice/ice_peer_ctx.h

@@ -62,6 +62,7 @@ struct nr_ice_peer_ctx_ {
 
   void *done_cb_timer;
   UCHAR reported_done;
+  void *trickle_grace_period_timer;
 
   STAILQ_ENTRY(nr_ice_peer_ctx_) entry;
 };

media/mtransport/third_party/nICEr/src/ice/ice_reg.h

@@ -62,6 +62,7 @@ extern "C" {
 
 #define NR_ICE_REG_KEEPALIVE_TIMER      "ice.keepalive_timer"
 
+#define NR_ICE_REG_TRICKLE_GRACE_PERIOD "ice.trickle_grace_period"
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */