andrzej-kaczmarek closed pull request #614: nimble/ll: Fixes for Data Length
Extension
URL: https://github.com/apache/mynewt-core/pull/614
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/apps/btshell/src/cmd.c b/apps/btshell/src/cmd.c
index 0d0339235..f6a32dc24 100644
--- a/apps/btshell/src/cmd.c
+++ b/apps/btshell/src/cmd.c
@@ -2080,7 +2080,7 @@ cmd_conn_datalen(int argc, char **argv)
#if MYNEWT_VAL(SHELL_CMD_HELP)
static const struct shell_param conn_datalen_params[] = {
- {"conn", "conn_datalenion handle, usage: =<UINT16>"},
+ {"conn", "conn_datalen handle, usage: =<UINT16>"},
{"octets", "usage: =<UINT16>"},
{"time", "usage: =<UINT16>"},
{NULL, NULL}
diff --git a/net/nimble/controller/src/ble_ll_conn.c
b/net/nimble/controller/src/ble_ll_conn.c
index 03930660f..4c93391cd 100644
--- a/net/nimble/controller/src/ble_ll_conn.c
+++ b/net/nimble/controller/src/ble_ll_conn.c
@@ -3788,11 +3788,9 @@ ble_ll_conn_rx_isr_end(uint8_t *rxbuf, struct
ble_mbuf_hdr *rxhdr)
/* XXX: TODO need to check with phy update procedure.
* There are limitations if we have started update */
rem_bytes = OS_MBUF_PKTLEN(txpdu) -
txhdr->txinfo.offset;
- if (rem_bytes > connsm->eff_max_tx_octets) {
- txhdr->txinfo.pyld_len = connsm->eff_max_tx_octets;
- } else {
- txhdr->txinfo.pyld_len = rem_bytes;
- }
+ /* Adjust payload for max TX time and octets */
+ rem_bytes = ble_ll_conn_adjust_pyld_len(connsm,
rem_bytes);
+ txhdr->txinfo.pyld_len = rem_bytes;
}
}
}
@@ -4133,6 +4131,13 @@ ble_ll_conn_slave_start(uint8_t *rxbuf, uint8_t pat,
struct ble_mbuf_hdr *rxhdr,
return 0;
}
+#define MAX_TIME_UNCODED(_maxbytes) \
+ ble_ll_pdu_tx_time_get(_maxbytes + BLE_LL_DATA_MIC_LEN, \
+ BLE_PHY_MODE_1M);
+#define MAX_TIME_CODED(_maxbytes) \
+ ble_ll_pdu_tx_time_get(_maxbytes + BLE_LL_DATA_MIC_LEN, \
+ BLE_PHY_MODE_CODED_125KBPS);
+
/**
* Called to reset the connection module. When this function is called the
* scheduler has been stopped and the phy has been disabled. The LL should
@@ -4174,42 +4179,32 @@ ble_ll_conn_module_reset(void)
}
/* Get the maximum supported PHY PDU size from the PHY */
+ max_phy_pyld = ble_phy_max_data_pdu_pyld();
/* Configure the global LL parameters */
conn_params = &g_ble_ll_conn_params;
- max_phy_pyld = ble_phy_max_data_pdu_pyld();
- /* NOTE: this all assumes that the default phy is 1Mbps */
maxbytes = min(MYNEWT_VAL(BLE_LL_SUPP_MAX_RX_BYTES), max_phy_pyld);
conn_params->supp_max_rx_octets = maxbytes;
#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LE_CODED_PHY)
- conn_params->supp_max_rx_time =
- ble_ll_pdu_tx_time_get(maxbytes + BLE_LL_DATA_MIC_LEN,
- BLE_PHY_MODE_CODED_125KBPS);
+ conn_params->supp_max_rx_time = MAX_TIME_CODED(maxbytes);
#else
- conn_params->supp_max_rx_time =
- ble_ll_pdu_tx_time_get(maxbytes + BLE_LL_DATA_MIC_LEN,
BLE_PHY_MODE_1M);
+ conn_params->supp_max_rx_time = MAX_TIME_UNCODED(maxbytes);
#endif
maxbytes = min(MYNEWT_VAL(BLE_LL_SUPP_MAX_TX_BYTES), max_phy_pyld);
conn_params->supp_max_tx_octets = maxbytes;
#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LE_CODED_PHY)
- conn_params->supp_max_tx_time =
- ble_ll_pdu_tx_time_get(maxbytes + BLE_LL_DATA_MIC_LEN,
- BLE_PHY_MODE_CODED_125KBPS);
+ conn_params->supp_max_tx_time = MAX_TIME_CODED(maxbytes);
#else
- conn_params->supp_max_tx_time =
- ble_ll_pdu_tx_time_get(maxbytes + BLE_LL_DATA_MIC_LEN,
BLE_PHY_MODE_1M);
+ conn_params->supp_max_tx_time = MAX_TIME_UNCODED(maxbytes);
#endif
maxbytes = min(MYNEWT_VAL(BLE_LL_CONN_INIT_MAX_TX_BYTES), max_phy_pyld);
conn_params->conn_init_max_tx_octets = maxbytes;
- conn_params->conn_init_max_tx_time =
- ble_ll_pdu_tx_time_get(maxbytes + BLE_LL_DATA_MIC_LEN,
BLE_PHY_MODE_1M);
- conn_params->conn_init_max_tx_time_uncoded =
- ble_ll_pdu_tx_time_get(maxbytes + BLE_LL_DATA_MIC_LEN,
BLE_PHY_MODE_1M);
- conn_params->conn_init_max_tx_time_coded =
- ble_ll_pdu_tx_time_get(maxbytes + BLE_LL_DATA_MIC_LEN,
BLE_PHY_MODE_CODED_125KBPS);
+ conn_params->conn_init_max_tx_time = MAX_TIME_UNCODED(maxbytes);
+ conn_params->conn_init_max_tx_time_uncoded = MAX_TIME_UNCODED(maxbytes);
+ conn_params->conn_init_max_tx_time_coded = MAX_TIME_CODED(maxbytes);
conn_params->sugg_tx_octets = BLE_LL_CONN_SUPP_BYTES_MIN;
conn_params->sugg_tx_time = BLE_LL_CONN_SUPP_TIME_MIN;
diff --git a/net/nimble/controller/src/ble_ll_conn_priv.h
b/net/nimble/controller/src/ble_ll_conn_priv.h
index a03dc61ae..02e99ed8a 100644
--- a/net/nimble/controller/src/ble_ll_conn_priv.h
+++ b/net/nimble/controller/src/ble_ll_conn_priv.h
@@ -27,14 +27,11 @@ extern "C" {
#endif
/*
- * Definitions for max rx/tx time/bytes for connections
- * NOTE: you get 327 usecs from 27 bytes of payload by:
- * -> adding 4 bytes for MIC
- * -> adding 2 bytes for PDU header
- * -> adding 8 bytes for preamble (1), access address (4) and crc (3).
+ * Definitions for min/max RX/TX time/bytes values allowed for connections.
+ * Source: Core 5.0 specification, Vol 6, Part B, section 4.5.10
*/
#define BLE_LL_CONN_SUPP_TIME_MIN (328) /* usecs */
-#define BLE_LL_CONN_SUPP_TIME_MAX (2120) /* usecs */
+#define BLE_LL_CONN_SUPP_TIME_MAX (17040) /* usecs */
#define BLE_LL_CONN_SUPP_TIME_MIN_UNCODED (328) /* usecs */
#define BLE_LL_CONN_SUPP_TIME_MAX_UNCODED (2120) /* usecs */
#define BLE_LL_CONN_SUPP_TIME_MIN_CODED (2704) /* usecs */
diff --git a/net/nimble/controller/src/ble_ll_ctrl.c
b/net/nimble/controller/src/ble_ll_ctrl.c
index 49d14c7d7..072b5d7e9 100644
--- a/net/nimble/controller/src/ble_ll_ctrl.c
+++ b/net/nimble/controller/src/ble_ll_ctrl.c
@@ -161,35 +161,6 @@ ble_ll_ctrl_rej_ext_ind_make(uint8_t rej_opcode, uint8_t
err, uint8_t *ctrdata)
ctrdata[1] = err;
}
-static int
-ble_ll_ctrl_chk_supp_bytes(uint16_t bytes)
-{
- int rc;
-
- if ((bytes < BLE_LL_CONN_SUPP_BYTES_MIN) ||
- (bytes > BLE_LL_CONN_SUPP_BYTES_MAX)) {
- rc = 0;
- } else {
- rc = 1;
- }
-
- return rc;
-}
-
-static int
-ble_ll_ctrl_chk_supp_time(uint16_t t)
-{
- int rc;
-
- if ((t < BLE_LL_CONN_SUPP_TIME_MIN) || (t > BLE_LL_CONN_SUPP_TIME_MAX)) {
- rc = 0;
- } else {
- rc = 1;
- }
-
- return rc;
-}
-
#if (BLE_LL_BT5_PHY_SUPPORTED == 1)
/**
* Called to cancel a phy update procedure.
@@ -226,10 +197,10 @@ ble_ll_ctrl_len_proc(struct ble_ll_conn_sm *connsm,
uint8_t *dptr)
ctrl_req.max_tx_bytes = get_le16(dptr + 4);
ctrl_req.max_tx_time = get_le16(dptr + 6);
- if (!ble_ll_ctrl_chk_supp_bytes(ctrl_req.max_rx_bytes) ||
- !ble_ll_ctrl_chk_supp_bytes(ctrl_req.max_tx_bytes) ||
- !ble_ll_ctrl_chk_supp_time(ctrl_req.max_tx_time) ||
- !ble_ll_ctrl_chk_supp_time(ctrl_req.max_rx_time)) {
+ if ((ctrl_req.max_rx_bytes < BLE_LL_CONN_SUPP_BYTES_MIN) ||
+ (ctrl_req.max_rx_time < BLE_LL_CONN_SUPP_TIME_MIN) ||
+ (ctrl_req.max_tx_bytes < BLE_LL_CONN_SUPP_BYTES_MIN) ||
+ (ctrl_req.max_tx_time < BLE_LL_CONN_SUPP_TIME_MIN)) {
rc = 1;
} else {
/* Update the connection with the new parameters */
@@ -2228,11 +2199,14 @@ ble_ll_ctrl_rx_pdu(struct ble_ll_conn_sm *connsm,
struct os_mbuf *om)
case BLE_LL_CTRL_LENGTH_RSP:
/* According to specification, process this only if we asked for it. */
if (connsm->cur_ctrl_proc == BLE_LL_CTRL_PROC_DATA_LEN_UPD) {
- /* Process the received data */
+ /*
+ * Process the received data. If received data is invalid, we'll
+ * reply with LL_UNKNOWN_RSP as per spec, but we still need to stop
+ * control procedure to avoid timeout.
+ */
if (ble_ll_ctrl_len_proc(connsm, dptr)) {
rc = -1;
rsp_opcode = BLE_LL_CTRL_UNKNOWN_RSP;
- goto ll_ctrl_send_rsp;
}
/* Stop the control procedure */
diff --git a/net/nimble/include/nimble/hci_common.h
b/net/nimble/include/nimble/hci_common.h
index 4dd779b44..482889d7f 100644
--- a/net/nimble/include/nimble/hci_common.h
+++ b/net/nimble/include/nimble/hci_common.h
@@ -420,7 +420,7 @@ extern "C" {
#define BLE_HCI_SET_DATALEN_TX_OCTETS_MIN (0x001b)
#define BLE_HCI_SET_DATALEN_TX_OCTETS_MAX (0x00fb)
#define BLE_HCI_SET_DATALEN_TX_TIME_MIN (0x0148)
-#define BLE_HCI_SET_DATALEN_TX_TIME_MAX (0x0848)
+#define BLE_HCI_SET_DATALEN_TX_TIME_MAX (0x4290)
/* --- LE read suggested default data length (OCF 0x0023) */
#define BLE_HCI_RD_SUGG_DATALEN_RSPLEN (4)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
