nimble/ll: Indicate proper max TX/RX time to peer

Core 5.1, Vol 6, Part B, 5.1.9 states as follows: If the peer device does not support the LE Coded PHY feature, then the MaxRxTime and MaxTxTime fields in the LL_LENGTH_REQ and LL_LENGTH_RSP PDUs shall be set to a value less than or equal to 2120 microseconds. So we should do as spec says. This means that any request from host to update TxTime should be checked and adjusted if necessary. However, once we know that peer does support LE Coded PHY (either xplicitly by features exchange or implicitly by PHY change to coded) we can update our MaxRxTime and MaxTxTime and also indicate this to peer.
2026-08-22 17:59:58 +00:00 · 2019-12-16 13:46:18 +01:00
parent 946cfb7ff2
commit dd5c30b37d
4 changed files with 83 additions and 22 deletions
@@ -133,6 +133,7 @@ union ble_ll_conn_sm_flags {
        uint32_t aux_conn_req: 1;
        uint32_t rxd_features:1;
        uint32_t pending_hci_rd_features:1;
+        uint32_t pending_initiate_dle:1;
    } cfbit;
    uint32_t conn_flags;
 } __attribute__((packed));
@@ -237,6 +238,9 @@ struct ble_ll_conn_sm
    uint16_t eff_max_tx_time;
    uint16_t eff_max_rx_time;
    uint8_t max_tx_octets_phy_mode[BLE_PHY_NUM_MODE];
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LE_CODED_PHY)
+    uint16_t host_req_max_tx_time;
+#endif

 #if (BLE_LL_BT5_PHY_SUPPORTED == 1)
    struct ble_ll_conn_phy_data phy_data;
@@ -1563,6 +1563,8 @@ ble_ll_conn_init_phy(struct ble_ll_conn_sm *connsm, int phy)
        connsm->max_rx_time = BLE_LL_CONN_SUPP_TIME_MAX_CODED;
        connsm->rem_max_tx_time = BLE_LL_CONN_SUPP_TIME_MIN_CODED;
        connsm->rem_max_rx_time = BLE_LL_CONN_SUPP_TIME_MIN_CODED;
+        /* Assume peer does support coded */
+        connsm->remote_features[0] |= (BLE_LL_FEAT_LE_CODED_PHY >> 8);
    } else {
        connsm->max_tx_time = conngp->conn_init_max_tx_time_uncoded;
        connsm->max_rx_time = BLE_LL_CONN_SUPP_TIME_MAX_UNCODED;
@@ -1743,6 +1745,9 @@ ble_ll_conn_sm_new(struct ble_ll_conn_sm *connsm)
    connsm->rem_max_rx_octets = BLE_LL_CONN_SUPP_BYTES_MIN;
    connsm->eff_max_tx_octets = BLE_LL_CONN_SUPP_BYTES_MIN;
    connsm->eff_max_rx_octets = BLE_LL_CONN_SUPP_BYTES_MIN;
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LE_CODED_PHY)
+    connsm->host_req_max_tx_time = 0;
+#endif

    ble_ll_update_max_tx_octets_phy_mode(connsm);

@@ -2142,6 +2147,22 @@ ble_ll_conn_next_event(struct ble_ll_conn_sm *connsm)
        CONN_F_PHY_UPDATE_EVENT(connsm) = 1;

        ble_ll_ctrl_phy_update_proc_complete(connsm);
+
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LE_CODED_PHY)
+        /*
+         * If PHY in either direction was changed to coded, we assume that peer
+         * does support LE Coded PHY even if features were not exchanged yet.
+         * This means that MaxRxTime can be updated to supported max and we need
+         * initiate DLE to notify peer about the change.
+         */
+        if (((connsm->phy_data.cur_tx_phy == BLE_PHY_CODED) ||
+             (connsm->phy_data.cur_rx_phy == BLE_PHY_CODED)) &&
+            !(connsm->remote_features[0] & (BLE_LL_FEAT_LE_CODED_PHY >> 8))) {
+            connsm->remote_features[0] |= (BLE_LL_FEAT_LE_CODED_PHY >> 8);
+            connsm->max_rx_time = BLE_LL_CONN_SUPP_TIME_MAX_CODED;
+            ble_ll_ctrl_initiate_dle(connsm);
+        }
+#endif
    }
 #endif

@@ -1375,6 +1375,19 @@ ble_ll_conn_hci_set_data_len(const uint8_t *cmdbuf, uint8_t len,
        goto done;
    }

+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LE_CODED_PHY)
+    /*
+     * Keep original value requested by host since we may want to recalculate
+     * MaxTxTime after PHY changes between coded and uncoded.
+     */
+    connsm->host_req_max_tx_time = txtime;
+
+    /* If peer does not support coded, we cannot use value larger than 2120us */
+    if (!(connsm->remote_features[0] & (BLE_LL_FEAT_LE_CODED_PHY >> 8))) {
+        txtime = min(txtime, BLE_LL_CONN_SUPP_TIME_MAX_UNCODED);
+    }
+#endif
+
    rc = BLE_ERR_SUCCESS;
    if (connsm->max_tx_time != txtime ||
        connsm->max_tx_octets != txoctets) {
@@ -1735,6 +1735,38 @@ ble_ll_ctrl_initiate_dle(struct ble_ll_conn_sm *connsm)
    ble_ll_ctrl_proc_start(connsm, BLE_LL_CTRL_PROC_DATA_LEN_UPD);
 }

+static void
+ble_ll_ctrl_update_features(struct ble_ll_conn_sm *connsm, uint8_t *feat)
+{
+    connsm->conn_features = feat[0];
+    memcpy(connsm->remote_features, feat + 1, 7);
+
+    /* If we received peer's features for the 1st time, we should try DLE */
+    if (!connsm->csmflags.cfbit.rxd_features) {
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LE_CODED_PHY)
+        /*
+         * If connection was established on uncoded PHY, by default we use
+         * MaxTxTime and MaxRxTime applicable for that PHY since we are not
+         * allowed to indicate longer supported time if peer does not support
+         * LE Coded PHY. However, once we know that peer does support it we can
+         * update those values to ones applicable for coded PHY.
+         */
+        if (connsm->remote_features[0] & (BLE_LL_FEAT_LE_CODED_PHY >> 8)) {
+            if (connsm->host_req_max_tx_time) {
+                connsm->max_tx_time = max(connsm->max_tx_time,
+                                          connsm->host_req_max_tx_time);
+            } else {
+                connsm->max_tx_time = g_ble_ll_conn_params.conn_init_max_tx_time_coded;
+            }
+            connsm->max_rx_time = BLE_LL_CONN_SUPP_TIME_MAX_CODED;
+        }
+#endif
+
+        connsm->csmflags.cfbit.pending_initiate_dle = 1;
+        connsm->csmflags.cfbit.rxd_features = 1;
+    }
+}
+
 /**
 * Called when we receive a feature request or a slave initiated feature
 * request.
@@ -1750,7 +1782,7 @@ ble_ll_ctrl_initiate_dle(struct ble_ll_conn_sm *connsm)
 */
 static int
 ble_ll_ctrl_rx_feature_req(struct ble_ll_conn_sm *connsm, uint8_t *dptr,
-                           uint8_t *rspbuf, uint8_t opcode, uint8_t *new_features)
+                           uint8_t *rspbuf, uint8_t opcode)
 {
    uint8_t rsp_opcode;
    uint32_t our_feat;
@@ -1774,6 +1806,8 @@ ble_ll_ctrl_rx_feature_req(struct ble_ll_conn_sm *connsm, uint8_t *dptr,

    rsp_opcode = BLE_LL_CTRL_FEATURE_RSP;

+    ble_ll_ctrl_update_features(connsm, dptr);
+
    /*
     * 1st octet of features should be common features of local and remote
     * controller - we call this 'connection features'
@@ -1782,19 +1816,12 @@ ble_ll_ctrl_rx_feature_req(struct ble_ll_conn_sm *connsm, uint8_t *dptr,
     *
     * See: Vol 6, Part B, section 2.4.2.10
     */
+    connsm->conn_features &= our_feat;

-    connsm->conn_features = dptr[0] & our_feat;
-    memcpy(connsm->remote_features, dptr + 1, 7);
    memset(rspbuf + 1, 0, 8);
    put_le32(rspbuf + 1, our_feat);
    rspbuf[1] = connsm->conn_features;

-    /* If this is the first time we received remote features, try to start DLE */
-    if (!connsm->csmflags.cfbit.rxd_features) {
-        *new_features = 1;
-        connsm->csmflags.cfbit.rxd_features = 1;
-    }
-
    return rsp_opcode;
 }

@@ -1807,19 +1834,15 @@ ble_ll_ctrl_rx_feature_req(struct ble_ll_conn_sm *connsm, uint8_t *dptr,
 *
 */
 static void
-ble_ll_ctrl_rx_feature_rsp(struct ble_ll_conn_sm *connsm, uint8_t *dptr, uint8_t *new_features)
+ble_ll_ctrl_rx_feature_rsp(struct ble_ll_conn_sm *connsm, uint8_t *dptr)
 {
-    connsm->conn_features = dptr[0];
-    memcpy(connsm->remote_features, dptr + 1, 7);
-    /* If this is the first time we received remote features, try to start DLE */
-    if (!connsm->csmflags.cfbit.rxd_features) {
-        *new_features = 1;
-        connsm->csmflags.cfbit.rxd_features = 1;
-    }
+    ble_ll_ctrl_update_features(connsm, dptr);
+
    /* Stop the control procedure */
    if (IS_PENDING_CTRL_PROC(connsm, BLE_LL_CTRL_PROC_FEATURE_XCHG)) {
        ble_ll_ctrl_proc_stop(connsm, BLE_LL_CTRL_PROC_FEATURE_XCHG);
    }
+
    /* Send event to host if pending features read */
    if (connsm->csmflags.cfbit.pending_hci_rd_features) {
        ble_ll_hci_ev_rd_rem_used_feat(connsm, BLE_ERR_SUCCESS);
@@ -2298,7 +2321,6 @@ ble_ll_ctrl_rx_pdu(struct ble_ll_conn_sm *connsm, struct os_mbuf *om)
 #if MYNEWT_VAL(BLE_LL_CFG_FEAT_LE_ENCRYPTION)
    int restart_encryption;
 #endif
-    uint8_t new_features = 0;
    int rc = 0;

    /* XXX: where do we validate length received and packet header length?
@@ -2456,17 +2478,17 @@ ble_ll_ctrl_rx_pdu(struct ble_ll_conn_sm *connsm, struct os_mbuf *om)
        rsp_opcode = ble_ll_ctrl_proc_unk_rsp(connsm, dptr, rspdata);
        break;
    case BLE_LL_CTRL_FEATURE_REQ:
-        rsp_opcode = ble_ll_ctrl_rx_feature_req(connsm, dptr, rspbuf, opcode, &new_features);
+        rsp_opcode = ble_ll_ctrl_rx_feature_req(connsm, dptr, rspbuf, opcode);
        break;
    /* XXX: check to see if ctrl procedure was running? Do we care? */
    case BLE_LL_CTRL_FEATURE_RSP:
-        ble_ll_ctrl_rx_feature_rsp(connsm, dptr, &new_features);
+        ble_ll_ctrl_rx_feature_rsp(connsm, dptr);
        break;
    case BLE_LL_CTRL_VERSION_IND:
        rsp_opcode = ble_ll_ctrl_rx_version_ind(connsm, dptr, rspdata);
        break;
    case BLE_LL_CTRL_SLAVE_FEATURE_REQ:
-        rsp_opcode = ble_ll_ctrl_rx_feature_req(connsm, dptr, rspbuf, opcode, &new_features);
+        rsp_opcode = ble_ll_ctrl_rx_feature_req(connsm, dptr, rspbuf, opcode);
        break;
 #if MYNEWT_VAL(BLE_LL_CFG_FEAT_LE_ENCRYPTION)
    case BLE_LL_CTRL_ENC_REQ:
@@ -2554,7 +2576,8 @@ ll_ctrl_send_rsp:
 #endif
    }

-    if (new_features) {
+    if (connsm->csmflags.cfbit.pending_initiate_dle) {
+        connsm->csmflags.cfbit.pending_initiate_dle = 0;
        ble_ll_ctrl_initiate_dle(connsm);
    }