Bluetooth: controller: Fix Data Length Update

Before a LL_FEATURE_REQ is sent, verify that the Data Length Update
procedure is still supported as it could have been unmasked as the result
of a previously completed Feature Exchange procedure.

Add unit test to verify the case of enqueuing both a
Feature Exchange procedure and Data Length Update procedure, where the
Feature Exchange procedure would result in the unmasking of the Data
Length Update procedure.

Signed-off-by: Thomas Ebert Hansen <[email protected]>

subsys/bluetooth/controller/ll_sw/ull_llcp.c

@@ -1032,6 +1032,16 @@ uint8_t ull_cp_data_length_update(struct ll_conn *conn, uint16_t max_tx_octets,
 {
 	struct proc_ctx *ctx;
 
+	if (!feature_dle(conn)) {
+		/* Data Length Update procedure not supported */
+
+		/* Returning BT_HCI_ERR_SUCCESS here might seem counter-intuitive,
+		 * but nothing in the specification seems to suggests
+		 * BT_HCI_ERR_UNSUPP_REMOTE_FEATURE.
+		 */
+		return BT_HCI_ERR_SUCCESS;
+	}
+
 	ctx = llcp_create_local_procedure(PROC_DATA_LENGTH_UPDATE);
 
 	if (!ctx) {

subsys/bluetooth/controller/ll_sw/ull_llcp_common.c

@@ -646,7 +646,7 @@ static void lp_comm_send_req(struct ll_conn *conn, struct proc_ctx *ctx, uint8_t
 #endif /* CONFIG_BT_CTLR_CENTRAL_ISO || CONFIG_BT_CTLR_PERIPHERAL_ISO */
 #if defined(CONFIG_BT_CTLR_DATA_LENGTH)
 	case PROC_DATA_LENGTH_UPDATE:
-		if (!ull_cp_remote_dle_pending(conn)) {
+		if (feature_dle(conn) && !ull_cp_remote_dle_pending(conn)) {
 			if (llcp_lr_ispaused(conn) || !llcp_tx_alloc_peek(conn, ctx)) {
 				ctx->state = LP_COMMON_STATE_WAIT_TX;
 			} else {
@@ -662,6 +662,10 @@ static void lp_comm_send_req(struct ll_conn *conn, struct proc_ctx *ctx, uint8_t
 			/* REQ was received from peer and RSP not yet sent
 			 * lets piggy-back on RSP instead af sending REQ
 			 * thus we can complete local req
+			 *
+			 * OR
+			 *
+			 * Data Length Update procedure no longer supported
 			 */
 			llcp_lr_complete(conn);
 			ctx->state = LP_COMMON_STATE_IDLE;

tests/bluetooth/controller/ctrl_data_length_update/src/main.c

@@ -195,6 +195,114 @@ ZTEST(dle_central, test_data_length_update_central_loc_unknown_rsp)
 		      "Free CTX buffers %d", llcp_ctx_buffers_free());
 }
 
+/*
+ * Locally triggered Data Length Update procedure
+ *
+ *
+ * Start a Feature Exchange procedure and Data Length Update procedure.
+ *
+ * The Feature Exchange procedure completes, removing Data Length Update
+ * procedure support.
+ *
+ * Expect that the already enqueued Data Length Update procedure completes
+ * without doing anything.
+ *
+ * +-----+                     +-------+                       +-----+
+ * | UT  |                     | LL_A  |                       | LT  |
+ * +-----+                     +-------+                       +-----+
+ *    |                            |                              |
+ *    | Start                      |                              |
+ *    | Feature Exchange Proc.     |                              |
+ *    |--------------------------->|                              |
+ *    |                            |                              |
+ *    | Start                      |                              |
+ *    | Data Length Update Proc.   |                              |
+ *    |--------------------------->|                              |
+ *    |                            |                              |
+ *    |                            | LL_FEATURE_REQ               |
+ *    |                            |----------------------------->|
+ *    |                            |                              |
+ *    |                            |               LL_FEATURE_RSP |
+ *    |                            |<-----------------------------|
+ *    |                            |                              |
+ *  ~~~~~~~~~~~~~~~~~~~~~~~  Unmask DLE support ~~~~~~~~~~~~~~~~~~~~
+ *    |                            |                              |
+ *    |     Feature Exchange Proc. |                              |
+ *    |                   Complete |                              |
+ *    |<---------------------------|                              |
+ *    |                            |                              |
+ */
+ZTEST(dle_central, test_data_length_update_central_loc_unsupported)
+{
+	uint8_t err;
+	struct node_tx *tx;
+	struct node_rx_pdu *ntf;
+
+	struct pdu_data_llctrl_feature_req local_feature_req;
+	struct pdu_data_llctrl_feature_rsp remote_feature_rsp;
+	struct pdu_data_llctrl_feature_rsp exp_remote_feature_rsp;
+
+	sys_put_le64(DEFAULT_FEATURE, local_feature_req.features);
+	sys_put_le64(0ULL, remote_feature_rsp.features);
+	sys_put_le64(0ULL, exp_remote_feature_rsp.features);
+
+
+	test_set_role(&conn, BT_HCI_ROLE_CENTRAL);
+	/* Connect */
+	ull_cp_state_set(&conn, ULL_CP_CONNECTED);
+	/* Init DLE data */
+	ull_conn_default_tx_octets_set(251);
+	ull_conn_default_tx_time_set(2120);
+	ull_dle_init(&conn, PHY_1M);
+
+	/* Confirm DLE is indicated as supported */
+	zassert_equal(feature_dle(&conn), true, "DLE Feature masked out");
+
+	/* Initiate a Feature Exchange Procedure */
+	err = ull_cp_feature_exchange(&conn, 1U);
+	zassert_equal(err, BT_HCI_ERR_SUCCESS);
+
+	/* Initiate a Data Length Update Procedure */
+	err = ull_cp_data_length_update(&conn, 211, 1800);
+	zassert_equal(err, BT_HCI_ERR_SUCCESS);
+
+	event_prepare(&conn);
+	/* Tx Queue should have one LL Control PDU */
+	lt_rx(LL_FEATURE_REQ, &conn, &tx, &local_feature_req);
+	lt_rx_q_is_empty(&conn);
+
+	/* Rx */
+	lt_tx(LL_FEATURE_RSP, &conn, &remote_feature_rsp);
+
+	event_done(&conn);
+	/* There should be one host notification */
+
+	ut_rx_pdu(LL_FEATURE_RSP, &ntf, &exp_remote_feature_rsp);
+
+	ut_rx_q_is_empty();
+
+	ull_cp_release_tx(&conn, tx);
+	release_ntf(ntf);
+
+	/* Confirm DLE is no longer indicated as supported */
+	zassert_equal(feature_dle(&conn), false, "DLE Feature not masked out");
+
+	/* Prepare another event for enqueued Data Length Update procedure */
+	event_prepare(&conn);
+	/* Tx Queue should have no LL Control PDU */
+	lt_rx_q_is_empty(&conn);
+	event_done(&conn);
+
+	/* Confirm DLE is no longer indicated as supported */
+	zassert_equal(feature_dle(&conn), false, "DLE Feature not masked out");
+
+	/* There should not be a host notifications */
+	ut_rx_q_is_empty();
+
+	zassert_equal(llcp_ctx_buffers_free(), test_ctx_buffers_cnt(),
+		      "Free CTX buffers %d", llcp_ctx_buffers_free());
+}
+
 /*
  * Locally triggered Data Length Update procedure
  *