Changeset 1140

Timestamp:

Apr 3, 2007 6:01:27 PM (17 years ago)

Author:

bennylp

Message:

Fixed misc bugs with ICE: (1) moved STUN session from candidate to component since it causes STUN response to wrong session, and (2) keep-alive transaction timed-out when ICE is active

Location:

pjproject/trunk

Files:

• pjmedia/src/pjmedia/transport_ice.c (modified) (7 diffs)
• pjnath/include/pjnath/config.h (modified) (1 diff)
• pjnath/include/pjnath/ice_session.h (modified) (6 diffs)
• pjnath/include/pjnath/ice_strans.h (modified) (1 diff)
• pjnath/include/pjnath/stun_session.h (modified) (2 diffs)
• pjnath/include/pjnath/stun_transaction.h (modified) (2 diffs)
• pjnath/src/pjnath/ice_session.c (modified) (22 diffs)
• pjnath/src/pjnath/ice_strans.c (modified) (13 diffs)
• pjnath/src/pjnath/stun_session.c (modified) (10 diffs)
• pjnath/src/pjnath/stun_transaction.c (modified) (12 diffs)

pjproject/trunk/pjmedia/src/pjmedia/transport_ice.c

@@ -435 +435 @@
     pj_bool_t remote_is_lite = PJ_FALSE;
     pj_bool_t ice_mismatch = PJ_FALSE;
+    pjmedia_sdp_conn *conn = NULL;
+    pj_sockaddr conn_addr;
+    pj_bool_t conn_found_in_candidate = PJ_FALSE;
     const pj_str_t STR_CANDIDATE = {"candidate", 9};
     const pj_str_t STR_ICE_LITE = {"ice-lite", 8};
@@ -445 +448 @@
 
     sdp_med = rem_sdp->media[media_index];
+
+    /* Get the SDP connection for the media stream.
+     * We'll verify later if the SDP connection address is specified 
+     * as one of the candidate.
+     */
+    conn = sdp_med->conn;
+    if (conn == NULL)
+        conn = rem_sdp->conn;
+
+    if (conn == NULL) {
+        /* Unable to find SDP connection */
+        return PJMEDIA_SDP_EMISSINGCONN;
+    }
+
+    pj_sockaddr_in_init(&conn_addr.ipv4, &conn->addr, 
+                        (pj_uint16_t)sdp_med->desc.port);
 
     /* Find ice-ufrag attribute in session descriptor */
@@ -481 +500 @@
         attr = sdp_med->attr[i];
 
+        /* Detect if remote is ICE lite */
         if (pj_strcmp(&attr->name, &STR_ICE_LITE)==0) {
             remote_is_lite = PJ_TRUE;
@@ -486 +506 @@
         }
 
+        /* Detect if remote has reported ICE mismatch */
         if (pj_strcmp(&attr->name, &STR_ICE_MISMATCH)==0) {
             ice_mismatch = PJ_TRUE;
@@ -494 +515 @@
             continue;
 
+        /* Parse candidate */
         status = parse_cand(pool, &attr->value, &cand[cand_cnt]);
         if (status != PJ_SUCCESS)
             return status;
 
+        /* Check if this candidate is equal to the connection line */
+        if (!conn_found_in_candidate &&
+            pj_memcmp(&conn_addr.ipv4, &cand[cand_cnt].addr.ipv4,
+                      sizeof(pj_sockaddr_in))==0)
+        {
+            conn_found_in_candidate = PJ_TRUE;
+        }
+
         cand_cnt++;
     }
@@ -503 +533 @@
     /* Handle ice-mismatch case */
     if (ice_mismatch) {
-        set_no_ice(tp_ice, "ice-mismatch detected");
+        set_no_ice(tp_ice, "remote reported ice-mismatch");
+        return PJ_SUCCESS;
+    }
+
+    /* Handle case where SDP connection address is not specified as
+     * one of the candidate.
+     */
+    if (!conn_found_in_candidate) {
+        set_no_ice(tp_ice, "local reported ice-mismatch");
         return PJ_SUCCESS;
     }
@@ -658 +696 @@
     char dst_addr[32];
 
+    pj_gettimeofday(&end_ice);
+    PJ_TIME_VAL_SUB(end_ice, tp_ice->start_ice);
+
     if (status != PJ_SUCCESS) {
         char errmsg[PJ_ERR_MSG_SIZE];
         pj_strerror(status, errmsg, sizeof(errmsg));
-        PJ_LOG(1,(ice_st->obj_name, "ICE negotiation failed: %s", errmsg));
+        PJ_LOG(1,(ice_st->obj_name, 
+                  "ICE negotiation failed after %d:%03ds: %s", 
+                  (int)end_ice.sec, (int)end_ice.msec,
+                  errmsg));
         return;
     }
-
-    pj_gettimeofday(&end_ice);
-    PJ_TIME_VAL_SUB(end_ice, tp_ice->start_ice);
 
     check = &ice_st->ice->valid_list.checks[0];

pjproject/trunk/pjnath/include/pjnath/config.h

@@ -165 +165 @@
 
 
+/**
+ * Minimum interval value to be used for sending STUN keep-alive on the ICE
+ * stream transport, in seconds. This minimum interval, plus a random value
+ * which maximum is PJ_ICE_ST_KEEP_ALIVE_MAX_RAND, specify the actual interval
+ * of the STUN keep-alive.
+ *
+ * Default: 20 seconds
+ *
+ * @see PJ_ICE_ST_KEEP_ALIVE_MAX_RAND
+ */
+#ifndef PJ_ICE_ST_KEEP_ALIVE_MIN
+#   define PJ_ICE_ST_KEEP_ALIVE_MIN                 20
+#endif
+
+
+/**
+ * To prevent STUN keep-alives to be sent simultaneously, application should
+ * add random interval to minimum interval (PJ_ICE_ST_KEEP_ALIVE_MIN). This
+ * setting specifies the maximum random value to be added to the minimum
+ * interval, in seconds.
+ *
+ * Default: 5 seconds
+ *
+ * @see PJ_ICE_ST_KEEP_ALIVE_MIN
+ */
+#ifndef PJ_ICE_ST_KEEP_ALIVE_MAX_RAND
+#   define PJ_ICE_ST_KEEP_ALIVE_MAX_RAND            5
+#endif
+
+
+
 
 /**

pjproject/trunk/pjnath/include/pjnath/ice_session.h

@@ -120 +120 @@
      */
     pj_ice_sess_check   *valid_check;
+
+    /**
+     * The STUN session to be used to send and receive STUN messages for this
+     * component.
+     */
+    pj_stun_session     *stun_sess;
 
 } pj_ice_sess_comp;
@@ -188 +194 @@
     pj_sockaddr          rel_addr;
 
-    /**
-     * The STUN session to be used to send and receive STUN messages for this
-     * candidate.
-     */
-    pj_stun_session     *stun_sess;
-
 } pj_ice_sess_cand;
 
@@ -367 +367 @@
      * @param ice           The ICE session.
      * @param comp_id       ICE component ID.
-     * @param cand_id       ICE candidate ID.
      * @param pkt           The STUN packet.
      * @param size          The size of the packet.
@@ -374 +373 @@
      */
     pj_status_t (*on_tx_pkt)(pj_ice_sess *ice, unsigned comp_id, 
-                             unsigned cand_id,
                              const void *pkt, pj_size_t size,
                              const pj_sockaddr_t *dst_addr,
@@ -696 +694 @@
  * @param ice           The ICE session.
  * @param comp_id       Component ID.
- * @param cand_id       Candidate ID.
  * @param pkt           Incoming packet.
  * @param pkt_size      Size of incoming packet.
@@ -706 +703 @@
 PJ_DECL(pj_status_t) pj_ice_sess_on_rx_pkt(pj_ice_sess *ice,
                                            unsigned comp_id,
-                                           unsigned cand_id,
                                            void *pkt,
                                            pj_size_t pkt_size,

pjproject/trunk/pjnath/include/pjnath/ice_strans.h

@@ -275 +275 @@
 
     pj_stun_session     *stun_sess;     /**< STUN session.              */
+    pj_uint8_t           ka_tsx_id[12]; /**< ID for keep STUN alives    */
 
     pj_sockaddr          local_addr;    /**< Local/base address.        */

pjproject/trunk/pjnath/include/pjnath/stun_session.h

@@ -246 +246 @@
  * @param msg_type  The STUN request message type, from pj_stun_method_e or
  *                  from pj_stun_msg_type.
+ * @param tsx_id    Optional transaction ID.
  * @param p_tdata   Pointer to receive STUN transmit data instance containing
  *                  the request.
@@ -253 +254 @@
 PJ_DECL(pj_status_t) pj_stun_session_create_req(pj_stun_session *sess,
                                                 int msg_type,
+                                                const pj_uint8_t tsx_id[12],
                                                 pj_stun_tx_data **p_tdata);
 

pjproject/trunk/pjnath/include/pjnath/stun_transaction.h

@@ -91 +91 @@
                                pj_size_t pkt_size);
 
+    /**
+     * This callback is called after the timer that was scheduled by
+     * #pj_stun_client_tsx_schedule_destroy() has elapsed. Application
+     * should call #pj_stun_client_tsx_destroy() upon receiving this
+     * callback.
+     *
+     * This callback is optional if application will not call 
+     * #pj_stun_client_tsx_schedule_destroy().
+     *
+     * @param tsx           The STUN transaction instance.
+     */
+    void (*on_destroy)(pj_stun_client_tsx *tsx);
+
 } pj_stun_tsx_cb;
 
@@ -117 +130 @@
 
 /**
- * Destroy a STUN client transaction.
+ * Schedule timer to destroy the transaction after the transaction is 
+ * complete. Application normally calls this function in the on_complete()
+ * callback. When this timer elapsed, the on_destroy() callback will be 
+ * called.
+ *
+ * This is convenient to let the STUN transaction absorbs any response 
+ * for the previous request retransmissions. If application doesn't want
+ * this, it can destroy the transaction immediately by calling 
+ * #pj_stun_client_tsx_destroy().
+ *
+ * @param tsx           The STUN transaction.
+ * @param delay         The delay interval before on_destroy() callback
+ *                      is called.
+ *
+ * @return              PJ_SUCCESS on success, or the appropriate error code.
+ */
+PJ_DECL(pj_status_t) 
+pj_stun_client_tsx_schedule_destroy(pj_stun_client_tsx *tsx,
+                                    const pj_time_val *delay);
+
+
+/**
+ * Destroy a STUN client transaction immediately. This function can be 
+ * called at any time to stop the transaction and destroy it.
  *
  * @param tsx           The STUN transaction.

pjproject/trunk/pjnath/src/pjnath/ice_session.c

@@ -84 +84 @@
 {
     pj_ice_sess         *ice;
-    pj_ice_sess_cand    *lcand;
+    unsigned             comp_id;
+    pj_ice_sess_comp    *comp;
 } stun_data;
 
@@ -180 +181 @@
 
 
+/* Init component */
+static pj_status_t init_comp(pj_ice_sess *ice,
+                             unsigned comp_id,
+                             pj_ice_sess_comp *comp)
+{
+    pj_stun_session_cb sess_cb;
+    pj_stun_auth_cred auth_cred;
+    stun_data *sd;
+    pj_status_t status;
+
+    /* Init STUN callbacks */
+    pj_bzero(&sess_cb, sizeof(sess_cb));
+    sess_cb.on_request_complete = &on_stun_request_complete;
+    sess_cb.on_rx_indication = &on_stun_rx_indication;
+    sess_cb.on_rx_request = &on_stun_rx_request;
+    sess_cb.on_send_msg = &on_stun_send_msg;
+
+    /* Create STUN session for this candidate */
+    status = pj_stun_session_create(&ice->stun_cfg, NULL, 
+                                    &sess_cb, PJ_FALSE,
+                                    &comp->stun_sess);
+    if (status != PJ_SUCCESS)
+        return status;
+
+    /* Associate data with this STUN session */
+    sd = PJ_POOL_ZALLOC_T(ice->pool, struct stun_data);
+    sd->ice = ice;
+    sd->comp_id = comp_id;
+    sd->comp = comp;
+    pj_stun_session_set_user_data(comp->stun_sess, sd);
+
+    /* Init STUN authentication credential */
+    pj_bzero(&auth_cred, sizeof(auth_cred));
+    auth_cred.type = PJ_STUN_AUTH_CRED_DYNAMIC;
+    auth_cred.data.dyn_cred.get_auth = &stun_auth_get_auth;
+    auth_cred.data.dyn_cred.get_cred = &stun_auth_get_cred;
+    auth_cred.data.dyn_cred.get_password = &stun_auth_get_password;
+    auth_cred.data.dyn_cred.user_data = comp->stun_sess;
+    pj_stun_session_set_credential(comp->stun_sess, &auth_cred);
+
+    return PJ_SUCCESS;
+}
+
+
 /*
  * Create ICE session.
@@ -228 +273 @@
         comp = &ice->comp[i];
         comp->valid_check = NULL;
+
+        status = init_comp(ice, i+1, comp);
+        if (status != PJ_SUCCESS) {
+            destroy_ice(ice, status);
+            return status;
+        }
     }
 
@@ -233 +284 @@
         ice->rx_ufrag.ptr = pj_pool_alloc(ice->pool, 16);
         pj_create_random_string(ice->rx_ufrag.ptr, 16);
+        ice->rx_ufrag.slen = 16;
     } else {
         pj_strdup(ice->pool, &ice->rx_ufrag, local_ufrag);
@@ -240 +292 @@
         ice->rx_pass.ptr = pj_pool_alloc(ice->pool, 16);
         pj_create_random_string(ice->rx_pass.ptr, 16);
+        ice->rx_pass.slen = 16;
     } else {
         pj_strdup(ice->pool, &ice->rx_pass, local_passwd);
@@ -268 +321 @@
     }
 
-    for (i=0; i<ice->lcand_cnt; ++i) {
-        if (ice->lcand[i].stun_sess) {
-            pj_stun_session_destroy(ice->lcand[i].stun_sess);
-            ice->lcand[i].stun_sess = NULL;
+    for (i=0; i<ice->comp_cnt; ++i) {
+        if (ice->comp[i].stun_sess) {
+            pj_stun_session_destroy(ice->comp[i].stun_sess);
+            ice->comp[i].stun_sess = NULL;
         }
     }
@@ -466 +519 @@
 {
     pj_ice_sess_cand *lcand;
-    pj_stun_session_cb sess_cb;
-    pj_stun_auth_cred auth_cred;
-    stun_data *sd;
     pj_status_t status = PJ_SUCCESS;
     char tmp[128];
@@ -495 +545 @@
     else
         pj_bzero(&lcand->rel_addr, sizeof(lcand->rel_addr));
-
-
-    /* Init STUN callbacks */
-    pj_bzero(&sess_cb, sizeof(sess_cb));
-    sess_cb.on_request_complete = &on_stun_request_complete;
-    sess_cb.on_rx_indication = &on_stun_rx_indication;
-    sess_cb.on_rx_request = &on_stun_rx_request;
-    sess_cb.on_send_msg = &on_stun_send_msg;
-
-    /* Create STUN session for this candidate */
-    status = pj_stun_session_create(&ice->stun_cfg, ice->obj_name, 
-                                    &sess_cb, PJ_FALSE,
-                                    &lcand->stun_sess);
-    if (status != PJ_SUCCESS) {
-        pj_mutex_unlock(ice->mutex);
-        return status;
-    }
-
-    /* Associate data with this STUN session */
-    sd = PJ_POOL_ZALLOC_T(ice->pool, struct stun_data);
-    sd->ice = ice;
-    sd->lcand = lcand;
-    pj_stun_session_set_user_data(lcand->stun_sess, sd);
-
-    /* Init STUN authentication credential */
-    pj_bzero(&auth_cred, sizeof(auth_cred));
-    auth_cred.type = PJ_STUN_AUTH_CRED_DYNAMIC;
-    auth_cred.data.dyn_cred.get_auth = &stun_auth_get_auth;
-    auth_cred.data.dyn_cred.get_cred = &stun_auth_get_cred;
-    auth_cred.data.dyn_cred.get_password = &stun_auth_get_password;
-    auth_cred.data.dyn_cred.user_data = lcand->stun_sess;
-    pj_stun_session_set_credential(lcand->stun_sess, &auth_cred);
 
 
@@ -909 +927 @@
              GET_CHECK_ID(&ice->clist, check)));
 
+        comp = find_comp(ice, check->lcand->comp_id);
+
         for (i=0; i<ice->clist.count; ++i) {
             pj_ice_sess_check *c = &ice->clist.checks[i];
@@ -927 +947 @@
                              "Cancelling check %s (In Progress)",
                              dump_check(buf, sizeof(buf), &ice->clist, c)));
-                        pj_stun_session_cancel_req(c->lcand->stun_sess, 
+                        pj_stun_session_cancel_req(comp->stun_sess, 
                                                    c->tdata, PJ_FALSE, 0);
                         c->tdata = NULL;
@@ -938 +958 @@
 
         /* Update the nominated check for the component */
-        comp = find_comp(ice, check->lcand->comp_id);
         if (comp->valid_check == NULL) {
             comp->valid_check = check;
@@ -1165 +1184 @@
 
     /* Create request */
-    status = pj_stun_session_create_req(lcand->stun_sess, 
+    status = pj_stun_session_create_req(comp->stun_sess, 
                                         PJ_STUN_BINDING_REQUEST, 
-                                        &check->tdata);
+                                        NULL, &check->tdata);
     if (status != PJ_SUCCESS) {
         pjnath_perror(ice->obj_name, "Error creating STUN request", status);
@@ -1200 +1219 @@
 
     /* Initiate STUN transaction to send the request */
-    status = pj_stun_session_send_msg(lcand->stun_sess, PJ_FALSE, 
+    status = pj_stun_session_send_msg(comp->stun_sess, PJ_FALSE, 
                                       &rcand->addr, 
                                       sizeof(pj_sockaddr_in), check->tdata);
@@ -1400 +1419 @@
     stun_data *sd = (stun_data*) pj_stun_session_get_user_data(sess);
     pj_ice_sess *ice = sd->ice;
-    return (*sd->ice->cb.on_tx_pkt)(sd->ice, sd->lcand->comp_id, 
-                                    GET_LCAND_ID(sd->lcand),
-                                    pkt, pkt_size, 
-                                    dst_addr, addr_len);
+    return (*ice->cb.on_tx_pkt)(ice, sd->comp_id, 
+                                pkt, pkt_size, 
+                                dst_addr, addr_len);
 }
 
@@ -1600 +1618 @@
     pj_stun_use_candidate_attr *uc;
     pj_ice_sess_comp *comp;
-    pj_ice_sess_cand *lcand;
+    pj_ice_sess_cand *lcand = NULL;
     pj_ice_sess_cand *rcand;
     unsigned i;
@@ -1625 +1643 @@
     sd = (stun_data*) pj_stun_session_get_user_data(sess);
     ice = sd->ice;
-    lcand = sd->lcand;
-    comp = find_comp(ice, lcand->comp_id);
+    comp = sd->comp;
 
     pj_mutex_lock(ice->mutex);
@@ -1683 +1700 @@
     if (i == ice->rcand_cnt) {
         rcand = &ice->rcand[ice->rcand_cnt++];
-        rcand->comp_id = lcand->comp_id;
+        rcand->comp_id = sd->comp_id;
         rcand->type = PJ_ICE_CAND_TYPE_PRFLX;
         rcand->prio = ap->value;
@@ -1703 +1720 @@
         rcand = &ice->rcand[i];
     }
+
+#if 0
+    /* Find again the local candidate by matching the base address
+     * with the local candidates in the checklist. Checks may have
+     * been pruned before, so it's possible that if we use the lcand
+     * as it is, we wouldn't be able to find the check in the checklist
+     * and we will end up creating a new check unnecessarily.
+     */
+    for (i=0; i<ice->clist.count; ++i) {
+        pj_ice_sess_check *c = &ice->clist.checks[i];
+        if (/*c->lcand == lcand ||*/
+            sockaddr_cmp(&c->lcand->base_addr, &lcand->base_addr)==0)
+        {
+            lcand = c->lcand;
+            break;
+        }
+    }
+#else
+    /* Just get candidate with the highest priority for the specified 
+     * component ID in the checklist.
+     */
+    for (i=0; i<ice->clist.count; ++i) {
+        pj_ice_sess_check *c = &ice->clist.checks[i];
+        if (c->lcand->comp_id == sd->comp_id) {
+            lcand = c->lcand;
+            break;
+        }
+    }
+    pj_assert(lcand != NULL);
+#endif
 
     /* 
@@ -1823 +1870 @@
 
 
-PJ_DEF(pj_status_t) pj_ice_sess_send_data( pj_ice_sess *ice,
-                                      unsigned comp_id,
-                                      const void *data,
-                                      pj_size_t data_len)
+PJ_DEF(pj_status_t) pj_ice_sess_send_data(pj_ice_sess *ice,
+                                          unsigned comp_id,
+                                          const void *data,
+                                          pj_size_t data_len)
 {
     pj_status_t status = PJ_SUCCESS;
     pj_ice_sess_comp *comp;
-    unsigned cand_id;
 
     PJ_ASSERT_RETURN(ice && comp_id && comp_id <= ice->comp_cnt, PJ_EINVAL);
@@ -1847 +1893 @@
     }
 
-    cand_id = GET_LCAND_ID(comp->valid_check->lcand);
-    status = (*ice->cb.on_tx_pkt)(ice, comp_id, cand_id, data, data_len, 
+    status = (*ice->cb.on_tx_pkt)(ice, comp_id, data, data_len, 
                                   &comp->valid_check->rcand->addr, 
                                   sizeof(pj_sockaddr_in));
@@ -1858 +1903 @@
 
 
-PJ_DEF(pj_status_t) pj_ice_sess_on_rx_pkt( pj_ice_sess *ice,
-                                      unsigned comp_id,
-                                      unsigned cand_id,
-                                      void *pkt,
-                                      pj_size_t pkt_size,
-                                      const pj_sockaddr_t *src_addr,
-                                      int src_addr_len)
+PJ_DEF(pj_status_t) pj_ice_sess_on_rx_pkt(pj_ice_sess *ice,
+                                          unsigned comp_id,
+                                          void *pkt,
+                                          pj_size_t pkt_size,
+                                          const pj_sockaddr_t *src_addr,
+                                          int src_addr_len)
 {
     pj_status_t status = PJ_SUCCESS;
     pj_ice_sess_comp *comp;
-    pj_ice_sess_cand *lcand;
     pj_status_t stun_status;
 
@@ -1881 +1924 @@
     }
 
-    lcand = &ice->lcand[cand_id];
-
     stun_status = pj_stun_msg_check(pkt, pkt_size, PJ_STUN_IS_DATAGRAM);
     if (stun_status == PJ_SUCCESS) {
-        status = pj_stun_session_on_rx_pkt(lcand->stun_sess, pkt, pkt_size,
+        status = pj_stun_session_on_rx_pkt(comp->stun_sess, pkt, pkt_size,
                                            PJ_STUN_IS_DATAGRAM,
                                            NULL, src_addr, src_addr_len);

pjproject/trunk/pjnath/src/pjnath/ice_strans.c

@@ -24 +24 @@
 #include <pj/log.h>
 #include <pj/pool.h>
+#include <pj/rand.h>
 #include <pj/string.h>
+
+
+#if 0
+#  define TRACE_PKT(expr)           PJ_LOG(5,expr)
+#else
+#  define TRACE_PKT(expr)
+#endif
 
 
@@ -31 +39 @@
 static void        on_ice_complete(pj_ice_sess *ice, pj_status_t status);
 static pj_status_t ice_tx_pkt(pj_ice_sess *ice, 
-                              unsigned comp_id, unsigned cand_id,
+                              unsigned comp_id,
                               const void *pkt, pj_size_t size,
                               const pj_sockaddr_t *dst_addr,
@@ -85 +93 @@
 
     if (name == NULL)
-        name = "icest%p";
+        name = "icstr%p";
 
     pool = pj_pool_create(stun_cfg->pf, name, 4000, 4000, NULL);
@@ -243 +251 @@
     pj_ice_strans_comp *comp;
     int retry, addr_len;
+    struct {
+        pj_uint32_t a1, a2, a3;
+    } tsx_id;
     pj_status_t status;
 
@@ -251 +262 @@
     comp->sock = PJ_INVALID_SOCKET;
     comp->last_status = PJ_SUCCESS;
+
+    /* Create transaction ID for STUN keep alives */
+    tsx_id.a1 = 0;
+    tsx_id.a2 = comp_id;
+    tsx_id.a3 = (pj_uint32_t) ice_st;
+    pj_memcpy(comp->ka_tsx_id, &tsx_id, sizeof(comp->ka_tsx_id));
 
     /* Create socket */
@@ -438 +455 @@
          * The process below is just a workaround.
          */
-        if (ice_st->ice) {
-            PJ_TODO(DISTINGUISH_BETWEEN_LOCAL_AND_RELAY);
-            status = pj_ice_sess_on_rx_pkt(ice_st->ice, comp->comp_id, 
-                                      comp->cand_list[0].ice_cand_id,
-                                      comp->pkt, bytes_read,
-                                      &comp->src_addr, comp->src_addr_len);
-        } else if (comp->stun_sess) {
-            status = pj_stun_msg_check(comp->pkt, bytes_read, 
-                                       PJ_STUN_IS_DATAGRAM);
-            if (status == PJ_SUCCESS) {
+        status = pj_stun_msg_check(comp->pkt, bytes_read, 
+                                   PJ_STUN_IS_DATAGRAM);
+
+        if (status == PJ_SUCCESS) {
+            if (ice_st->ice==NULL ||
+                pj_memcmp(comp->pkt+8, comp->ka_tsx_id, 12) == 0) 
+            {
                 status = pj_stun_session_on_rx_pkt(comp->stun_sess, comp->pkt,
                                                    bytes_read, 
@@ -454 +468 @@
                                                    comp->src_addr_len);
             } else {
-                (*ice_st->cb.on_rx_data)(ice_st, comp->comp_id, 
-                                         comp->pkt, bytes_read, 
-                                         &comp->src_addr, comp->src_addr_len);
-
+                PJ_TODO(DISTINGUISH_BETWEEN_LOCAL_AND_RELAY);
+
+                TRACE_PKT((comp->ice_st->obj_name, 
+                          "Component %d RX packet from %s:%d",
+                          comp->comp_id,
+                          pj_inet_ntoa(comp->src_addr.ipv4.sin_addr),
+                          (int)pj_ntohs(comp->src_addr.ipv4.sin_port)));
+
+                status = pj_ice_sess_on_rx_pkt(ice_st->ice, comp->comp_id, 
+                                               comp->pkt, bytes_read,
+                                               &comp->src_addr, 
+                                               comp->src_addr_len);
             }
         } else {
@@ -528 +550 @@
         /* Create STUN binding request */
         status = pj_stun_session_create_req(comp->stun_sess,
-                                            PJ_STUN_BINDING_REQUEST, &tdata);
+                                            PJ_STUN_BINDING_REQUEST, 
+                                            comp->ka_tsx_id, &tdata);
         if (status != PJ_SUCCESS)
             continue;
@@ -555 +578 @@
         return;
 
-    delay.sec = 20;
-    delay.msec = 0;
+    delay.sec = PJ_ICE_ST_KEEP_ALIVE_MIN;
+    delay.msec = pj_rand() % (PJ_ICE_ST_KEEP_ALIVE_MAX_RAND * 1000);
+    pj_time_val_normalize(&delay);
 
     ice_st->ka_timer.cb = &ka_timer_cb;
@@ -617 +641 @@
     /* Create STUN binding request */
     status = pj_stun_session_create_req(comp->stun_sess, 
-                                        PJ_STUN_BINDING_REQUEST, &tdata);
+                                        PJ_STUN_BINDING_REQUEST, 
+                                        comp->ka_tsx_id, 
+                                        &tdata);
     if (status != PJ_SUCCESS)
         return status;
@@ -945 +971 @@
  */
 static pj_status_t ice_tx_pkt(pj_ice_sess *ice, 
-                              unsigned comp_id, unsigned cand_id,
+                              unsigned comp_id, 
                               const void *pkt, pj_size_t size,
                               const pj_sockaddr_t *dst_addr,
@@ -959 +985 @@
     PJ_ASSERT_RETURN(comp_id && comp_id <= ice_st->comp_cnt, PJ_EINVAL);
     comp = ice_st->comp[comp_id-1];
+
+    TRACE_PKT((comp->ice_st->obj_name, 
+              "Component %d TX packet to %s:%d",
+              comp_id,
+              pj_inet_ntoa(((pj_sockaddr_in*)dst_addr)->sin_addr),
+              (int)pj_ntohs(((pj_sockaddr_in*)dst_addr)->sin_port)));
 
     pkt_size = size;
@@ -1027 +1059 @@
     if (cand == NULL) {
         /* This is keep-alive */
+        if (status != PJ_SUCCESS) {
+            ice_st_perror(comp->ice_st, "STUN keep-alive request failed",
+                          status);
+        }
         return;
     }

pjproject/trunk/pjnath/src/pjnath/stun_session.c

@@ -50 +50 @@
 
 
-static void tsx_on_complete(pj_stun_client_tsx *tsx,
-                            pj_status_t status, 
-                            const pj_stun_msg *response);
-static pj_status_t tsx_on_send_msg(pj_stun_client_tsx *tsx,
-                                   const void *stun_pkt,
-                                   pj_size_t pkt_size);
+static void stun_tsx_on_complete(pj_stun_client_tsx *tsx,
+                                 pj_status_t status, 
+                                 const pj_stun_msg *response);
+static pj_status_t stun_tsx_on_send_msg(pj_stun_client_tsx *tsx,
+                                        const void *stun_pkt,
+                                        pj_size_t pkt_size);
+static void stun_tsx_on_destroy(pj_stun_client_tsx *tsx);
 
 static pj_stun_tsx_cb tsx_cb = 
 {
-    &tsx_on_complete,
-    &tsx_on_send_msg
+    &stun_tsx_on_complete,
+    &stun_tsx_on_send_msg,
+    &stun_tsx_on_destroy
 };
 
@@ -121 +123 @@
 static pj_status_t create_request_tdata(pj_stun_session *sess,
                                         unsigned msg_type,
+                                        const pj_uint8_t tsx_id[12],
                                         pj_stun_tx_data **p_tdata)
 {
@@ -132 +135 @@
     /* Create STUN message */
     status = pj_stun_msg_create(tdata->pool, msg_type,  PJ_STUN_MAGIC, 
-                                NULL, &tdata->msg);
+                                tsx_id, &tdata->msg);
     if (status != PJ_SUCCESS) {
         pj_pool_release(tdata->pool);
@@ -149 +152 @@
 }
 
+
+static void stun_tsx_on_destroy(pj_stun_client_tsx *tsx)
+{
+    pj_stun_tx_data *tdata;
+
+    tdata = (pj_stun_tx_data*) pj_stun_client_tsx_get_data(tsx);
+    pj_stun_client_tsx_destroy(tsx);
+    pj_pool_release(tdata->pool);
+}
+
 static void destroy_tdata(pj_stun_tx_data *tdata)
 {
-    if (tdata->client_tsx) {
-        tsx_erase(tdata->sess, tdata);
-        pj_stun_client_tsx_destroy(tdata->client_tsx);
-        tdata->client_tsx = NULL;
-    }
     if (tdata->res_timer.id != PJ_FALSE) {
         pj_timer_heap_cancel(tdata->sess->cfg->timer_heap, 
@@ -162 +170 @@
         pj_list_erase(tdata);
     }
-    pj_pool_release(tdata->pool);
+
+    if (tdata->client_tsx) {
+        pj_time_val delay = {2, 0};
+        tsx_erase(tdata->sess, tdata);
+        pj_stun_client_tsx_schedule_destroy(tdata->client_tsx, &delay);
+        tdata->client_tsx = NULL;
+
+    } else {
+        pj_pool_release(tdata->pool);
+    }
 }
 
@@ -307 +324 @@
 
 
-static void tsx_on_complete(pj_stun_client_tsx *tsx,
-                            pj_status_t status, 
-                            const pj_stun_msg *response)
+static void stun_tsx_on_complete(pj_stun_client_tsx *tsx,
+                                 pj_status_t status, 
+                                 const pj_stun_msg *response)
 {
     pj_stun_tx_data *tdata;
@@ -321 +338 @@
 }
 
-static pj_status_t tsx_on_send_msg(pj_stun_client_tsx *tsx,
-                                   const void *stun_pkt,
-                                   pj_size_t pkt_size)
+static pj_status_t stun_tsx_on_send_msg(pj_stun_client_tsx *tsx,
+                                        const void *stun_pkt,
+                                        pj_size_t pkt_size)
 {
     pj_stun_tx_data *tdata;
@@ -442 +459 @@
 PJ_DEF(pj_status_t) pj_stun_session_create_req(pj_stun_session *sess,
                                                int method,
+                                               const pj_uint8_t tsx_id[12],
                                                pj_stun_tx_data **p_tdata)
 {
@@ -449 +467 @@
     PJ_ASSERT_RETURN(sess && p_tdata, PJ_EINVAL);
 
-    status = create_request_tdata(sess, method, &tdata);
+    status = create_request_tdata(sess, method, tsx_id, &tdata);
     if (status != PJ_SUCCESS)
         return status;
@@ -766 +784 @@
     tdata = tsx_lookup(sess, msg);
     if (tdata == NULL) {
-        PJ_LOG(5,(SNAME(sess), 
+        PJ_LOG(4,(SNAME(sess), 
                   "Transaction not found, response silently discarded"));
         return PJ_SUCCESS;

pjproject/trunk/pjnath/src/pjnath/stun_transaction.c

@@ -37 +37 @@
 
     pj_bool_t            complete;
-    \
+
     pj_bool_t            require_retransmit;
-    pj_timer_entry       timer;
+    pj_timer_entry       retransmit_timer;
     unsigned             transmit_count;
     pj_time_val          retransmit_time;
+
+    pj_timer_entry       destroy_timer;
 
     void                *last_pkt;
@@ -50 +52 @@
 static void retransmit_timer_callback(pj_timer_heap_t *timer_heap, 
                                       pj_timer_entry *timer);
+static void destroy_timer_callback(pj_timer_heap_t *timer_heap, 
+                                   pj_timer_entry *timer);
 
 #define stun_perror(tsx,msg,rc) pjnath_perror(tsx->obj_name, msg, rc)
@@ -70 +74 @@
     pj_memcpy(&tsx->cb, cb, sizeof(*cb));
 
-    tsx->timer.cb = &retransmit_timer_callback;
-    tsx->timer.user_data = tsx;
+    tsx->retransmit_timer.cb = &retransmit_timer_callback;
+    tsx->retransmit_timer.user_data = tsx;
+
+    tsx->destroy_timer.cb = &destroy_timer_callback;
+    tsx->destroy_timer.user_data = tsx;
 
     pj_ansi_snprintf(tsx->obj_name, sizeof(tsx->obj_name), "stuntsx%p", tsx);
@@ -82 +89 @@
 
 
-/*
- * .
+PJ_DEF(pj_status_t) 
+pj_stun_client_tsx_schedule_destroy(pj_stun_client_tsx *tsx,
+                                    const pj_time_val *delay)
+{
+    pj_status_t status;
+
+    PJ_ASSERT_RETURN(tsx && delay, PJ_EINVAL);
+    PJ_ASSERT_RETURN(tsx->cb.on_destroy, PJ_EINVAL);
+
+    /* Cancel previously registered timer */
+    if (tsx->destroy_timer.id != 0) {
+        pj_timer_heap_cancel(tsx->cfg->timer_heap, &tsx->destroy_timer);
+        tsx->destroy_timer.id = 0;
+    }
+
+    /* Stop retransmission, just in case */
+    if (tsx->retransmit_timer.id != 0) {
+        pj_timer_heap_cancel(tsx->cfg->timer_heap, &tsx->retransmit_timer);
+        tsx->retransmit_timer.id = 0;
+    }
+
+    status = pj_timer_heap_schedule(tsx->cfg->timer_heap,
+                                    &tsx->destroy_timer, delay);
+    if (status != PJ_SUCCESS)
+        return status;
+
+    tsx->destroy_timer.id = TIMER_ACTIVE;
+
+    return PJ_SUCCESS;
+}
+
+
+/*
+ * Destroy transaction immediately.
  */
 PJ_DEF(pj_status_t) pj_stun_client_tsx_destroy(pj_stun_client_tsx *tsx)
@@ -89 +128 @@
     PJ_ASSERT_RETURN(tsx, PJ_EINVAL);
 
-    if (tsx->timer.id != 0) {
-        pj_timer_heap_cancel(tsx->cfg->timer_heap, &tsx->timer);
-        tsx->timer.id = 0;
-    }
+    if (tsx->retransmit_timer.id != 0) {
+        pj_timer_heap_cancel(tsx->cfg->timer_heap, &tsx->retransmit_timer);
+        tsx->retransmit_timer.id = 0;
+    }
+    if (tsx->destroy_timer.id != 0) {
+        pj_timer_heap_cancel(tsx->cfg->timer_heap, &tsx->destroy_timer);
+        tsx->destroy_timer.id = 0;
+    }
+
+    PJ_LOG(5,(tsx->obj_name, "STUN client transaction destroyed"));
     return PJ_SUCCESS;
 }
@@ -136 +181 @@
     pj_status_t status;
 
-    PJ_ASSERT_RETURN(tsx->timer.id == 0, PJ_EBUSY);
+    PJ_ASSERT_RETURN(tsx->retransmit_timer.id == 0, PJ_EBUSY);
 
     if (tsx->require_retransmit) {
@@ -161 +206 @@
          * cancel transmission).
          */;
-        status = pj_timer_heap_schedule(tsx->cfg->timer_heap, &tsx->timer,
+        status = pj_timer_heap_schedule(tsx->cfg->timer_heap, 
+                                        &tsx->retransmit_timer,
                                         &tsx->retransmit_time);
         if (status != PJ_SUCCESS) {
-            tsx->timer.id = 0;
+            tsx->retransmit_timer.id = 0;
             return status;
         }
-        tsx->timer.id = TIMER_ACTIVE;
+        tsx->retransmit_timer.id = TIMER_ACTIVE;
     }
 
@@ -174 +220 @@
     status = tsx->cb.on_send_msg(tsx, tsx->last_pkt, tsx->last_pkt_size);
     if (status != PJ_SUCCESS) {
-        if (tsx->timer.id != 0) {
-            pj_timer_heap_cancel(tsx->cfg->timer_heap, &tsx->timer);
-            tsx->timer.id = 0;
+        if (tsx->retransmit_timer.id != 0) {
+            pj_timer_heap_cancel(tsx->cfg->timer_heap, 
+                                 &tsx->retransmit_timer);
+            tsx->retransmit_timer.id = 0;
         }
         stun_perror(tsx, "STUN error sending message", status);
@@ -199 +246 @@
 {
     PJ_ASSERT_RETURN(tsx && pkt && pkt_len, PJ_EINVAL);
-    PJ_ASSERT_RETURN(tsx->timer.id == 0, PJ_EBUSY);
+    PJ_ASSERT_RETURN(tsx->retransmit_timer.id == 0, PJ_EBUSY);
 
     /* Encode message */
@@ -224 +271 @@
     if (tsx->transmit_count >= PJ_STUN_MAX_RETRANSMIT_COUNT) {
         /* Retransmission count exceeded. Transaction has failed */
-        tsx->timer.id = 0;
+        tsx->retransmit_timer.id = 0;
         PJ_LOG(4,(tsx->obj_name, "STUN timeout waiting for response"));
-        tsx->complete = PJ_TRUE;
-        if (tsx->cb.on_complete) {
-            tsx->cb.on_complete(tsx, PJNATH_ESTUNTIMEDOUT, NULL);
+        if (!tsx->complete) {
+            tsx->complete = PJ_TRUE;
+            if (tsx->cb.on_complete) {
+                tsx->cb.on_complete(tsx, PJNATH_ESTUNTIMEDOUT, NULL);
+            }
         }
         return;
     }
 
-    tsx->timer.id = 0;
+    tsx->retransmit_timer.id = 0;
     status = tsx_transmit_msg(tsx);
     if (status != PJ_SUCCESS) {
-        tsx->timer.id = 0;
-        tsx->complete = PJ_TRUE;
-        if (tsx->cb.on_complete) {
-            tsx->cb.on_complete(tsx, status, NULL);
-        }
-    }
-}
-
+        tsx->retransmit_timer.id = 0;
+        if (!tsx->complete) {
+            tsx->complete = PJ_TRUE;
+            if (tsx->cb.on_complete) {
+                tsx->cb.on_complete(tsx, status, NULL);
+            }
+        }
+    }
+}
+
+
+/* Timer callback to destroy transaction */
+static void destroy_timer_callback(pj_timer_heap_t *timer_heap, 
+                                   pj_timer_entry *timer)
+{
+    pj_stun_client_tsx *tsx = (pj_stun_client_tsx *) timer->user_data;
+
+    PJ_UNUSED_ARG(timer_heap);
+
+    tsx->destroy_timer.id = PJ_FALSE;
+    tsx->cb.on_destroy(tsx);
+    /* Don't access transaction after this */
+}
 
 
@@ -268 +332 @@
      * We can cancel retransmit timer now.
      */
-    if (tsx->timer.id) {
-        pj_timer_heap_cancel(tsx->cfg->timer_heap, &tsx->timer);
-        tsx->timer.id = 0;
+    if (tsx->retransmit_timer.id) {
+        pj_timer_heap_cancel(tsx->cfg->timer_heap, &tsx->retransmit_timer);
+        tsx->retransmit_timer.id = 0;
     }
 
@@ -297 +361 @@
 
     /* Call callback */
-    tsx->complete = PJ_TRUE;
-    if (tsx->cb.on_complete) {
-        tsx->cb.on_complete(tsx, status, msg);
-    }
-
-    return PJ_SUCCESS;
-
-}
-
+    if (!tsx->complete) {
+        tsx->complete = PJ_TRUE;
+        if (tsx->cb.on_complete) {
+            tsx->cb.on_complete(tsx, status, msg);
+        }
+    }
+
+    return PJ_SUCCESS;
+
+}
+