Bluetooth: controller: Fix LLCP procedures issues

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;

subsys/bluetooth/controller/ll_sw/ull_llcp_phy.c

@@ -618,6 +618,7 @@ static void lp_pu_st_wait_rx_phy_rsp(struct ll_conn *conn, struct proc_ctx *ctx,
 		ctx->data.pu.error = BT_HCI_ERR_UNSUPP_REMOTE_FEATURE;
 		ctx->data.pu.ntf_pu = 1;
 		lp_pu_complete(conn, ctx, evt, param);
+		llcp_tx_resume_data(conn, LLCP_TX_QUEUE_PAUSE_DATA_PHY_UPDATE);
 		break;
 	default:
 		/* Ignore other evts */

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
  *

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

@@ -159,6 +159,9 @@ ZTEST(phy_central, test_phy_update_central_loc)
 	lt_rx(LL_PHY_REQ, &conn, &tx, &req);
 	lt_rx_q_is_empty(&conn);
 
+	/* Check that data tx was paused */
+	zassert_equal(conn.tx_q.pause_data, 1U, "Data tx is not paused");
+
 	/* Rx */
 	lt_tx(LL_PHY_RSP, &conn, &rsp);
 
@@ -279,6 +282,9 @@ ZTEST(phy_central, test_phy_update_central_loc_invalid)
 	lt_rx(LL_PHY_REQ, &conn, &tx, &req);
 	lt_rx_q_is_empty(&conn);
 
+	/* Check that data tx was paused */
+	zassert_equal(conn.tx_q.pause_data, 1U, "Data tx is not paused");
+
 	/* Rx */
 	lt_tx(LL_REJECT_IND, &conn, &reject_ind);
 
@@ -288,6 +294,9 @@ ZTEST(phy_central, test_phy_update_central_loc_invalid)
 	/* Done */
 	event_done(&conn);
 
+	/* Check that data tx was paused */
+	zassert_equal(conn.tx_q.pause_data, 1U, "Data tx is not paused");
+
 	/* Release Tx */
 	ull_cp_release_tx(&conn, tx);
 
@@ -330,6 +339,9 @@ ZTEST(phy_central, test_phy_update_central_loc_unsupp_feat)
 	lt_rx(LL_PHY_REQ, &conn, &tx, &req);
 	lt_rx_q_is_empty(&conn);
 
+	/* Check that data tx was paused */
+	zassert_equal(conn.tx_q.pause_data, 1U, "Data tx is not paused");
+
 	/* Rx */
 	lt_tx(LL_UNKNOWN_RSP, &conn, &unknown_rsp);
 
@@ -339,6 +351,9 @@ ZTEST(phy_central, test_phy_update_central_loc_unsupp_feat)
 	/* Done */
 	event_done(&conn);
 
+	/* Check that data tx is no longer paused */
+	zassert_equal(conn.tx_q.pause_data, 0U, "Data tx is paused");
+
 	/* Release Tx */
 	ull_cp_release_tx(&conn, tx);
 
@@ -502,9 +517,15 @@ ZTEST(phy_periph, test_phy_update_periph_loc)
 	lt_rx(LL_PHY_REQ, &conn, &tx, &req);
 	lt_rx_q_is_empty(&conn);
 
+	/* Check that data tx was paused */
+	zassert_equal(conn.tx_q.pause_data, 1U, "Data tx is not paused");
+
 	/* TX Ack */
 	event_tx_ack(&conn, tx);
 
+	/* Check that data tx is no longer paused */
+	zassert_equal(conn.tx_q.pause_data, 0U, "Data tx is paused");
+
 	/* Done */
 	event_done(&conn);
 
@@ -524,6 +545,9 @@ ZTEST(phy_periph, test_phy_update_periph_loc)
 	/* Done */
 	event_done(&conn);
 
+	/* Check that data tx is no longer paused */
+	zassert_equal(conn.tx_q.pause_data, 0U, "Data tx is paused");
+
 	/* */
 	while (!is_instant_reached(&conn, instant)) {
 		/* Prepare */
@@ -691,12 +715,21 @@ ZTEST(phy_periph, test_phy_update_periph_loc_unsupp_feat)
 	/* Rx */
 	lt_tx(LL_UNKNOWN_RSP, &conn, &unknown_rsp);
 
+	/* Check that data tx was paused */
+	zassert_equal(conn.tx_q.pause_data, 1U, "Data tx is not paused");
+
 	/* TX Ack */
 	event_tx_ack(&conn, tx);
 
+	/* Check that data tx is no longer paused */
+	zassert_equal(conn.tx_q.pause_data, 0U, "Data tx is paused");
+
 	/* Done */
 	event_done(&conn);
 
+	/* Check that data tx is no longer paused */
+	zassert_equal(conn.tx_q.pause_data, 0U, "Data tx is paused");
+
 	/* Release Tx */
 	ull_cp_release_tx(&conn, tx);
 
@@ -825,13 +858,13 @@ ZTEST(phy_central, test_phy_update_central_loc_collision)
 	/* TX Ack */
 	event_tx_ack(&conn, tx);
 
-	/* Check that data tx is not paused */
+	/* Check that data tx is paused */
 	zassert_equal(conn.tx_q.pause_data, 1U, "Data tx is not paused");
 
 	/* Done */
 	event_done(&conn);
 
-	/* Check that data tx is not paused */
+	/* Check that data tx is paused */
 	zassert_equal(conn.tx_q.pause_data, 1U, "Data tx is not paused");
 
 	/* Release Tx */
@@ -978,6 +1011,9 @@ ZTEST(phy_central, test_phy_update_central_rem_collision)
 	/* Done */
 	event_done(&conn);
 
+	/* Check that data tx was paused */
+	zassert_equal(conn.tx_q.pause_data, 1U, "Data tx is not paused");
+
 	/*** ***/
 
 	/* Initiate an PHY Update Procedure */
@@ -999,6 +1035,9 @@ ZTEST(phy_central, test_phy_update_central_rem_collision)
 	/* Done */
 	event_done(&conn);
 
+	/* Check that data tx is no longer paused */
+	zassert_equal(conn.tx_q.pause_data, 0U, "Data tx is paused");
+
 	/* Save Instant */
 	pdu = (struct pdu_data *)tx->pdu;
 	instant = sys_le16_to_cpu(pdu->llctrl.phy_upd_ind.instant);
@@ -1038,6 +1077,9 @@ ZTEST(phy_central, test_phy_update_central_rem_collision)
 	 * event due to completion of remote PHY update at end of the "at instant" conneciton event.
 	 */
 
+	/* Check that data tx is no longer paused */
+	zassert_equal(conn.tx_q.pause_data, 0U, "Data tx is paused");
+
 	/* Prepare */
 	event_prepare(&conn);
 
@@ -1054,6 +1096,9 @@ ZTEST(phy_central, test_phy_update_central_rem_collision)
 	/* Done */
 	event_done(&conn);
 
+	/* Check that data tx was paused */
+	zassert_equal(conn.tx_q.pause_data, 1U, "Data tx is not paused");
+
 	/* Release Tx */
 	ull_cp_release_tx(&conn, tx);
 
@@ -1077,6 +1122,9 @@ ZTEST(phy_central, test_phy_update_central_rem_collision)
 	/* Done */
 	event_done(&conn);
 
+	/* Check that data tx is no longer paused */
+	zassert_equal(conn.tx_q.pause_data, 0U, "Data tx is paused");
+
 	/* Save Instant */
 	pdu = (struct pdu_data *)tx->pdu;
 	instant = sys_le16_to_cpu(pdu->llctrl.phy_upd_ind.instant);
@@ -1161,9 +1209,15 @@ ZTEST(phy_periph, test_phy_update_periph_loc_collision)
 	/* Rx */
 	lt_tx(LL_PHY_REQ, &conn, &req_central);
 
+	/* Check that data tx was paused */
+	zassert_equal(conn.tx_q.pause_data, 1U, "Data tx is not paused");
+
 	/* TX Ack */
 	event_tx_ack(&conn, tx);
 
+	/* Check that data tx is no longer paused */
+	zassert_equal(conn.tx_q.pause_data, 0U, "Data tx is paused");
+
 	/* Done */
 	event_done(&conn);
 
@@ -1213,6 +1267,9 @@ ZTEST(phy_periph, test_phy_update_periph_loc_collision)
 	/* Release Tx */
 	ull_cp_release_tx(&conn, tx);
 
+	/* Check that data tx is no longer paused */
+	zassert_equal(conn.tx_q.pause_data, 0U, "Data tx is paused");
+
 	/* */
 	while (!is_instant_reached(&conn, instant)) {
 		/* Prepare */
@@ -1428,9 +1485,15 @@ ZTEST(phy_periph, test_phy_update_periph_loc_no_actual_change)
 	lt_rx(LL_PHY_REQ, &conn, &tx, &req);
 	lt_rx_q_is_empty(&conn);
 
+	/* Check that data tx was paused */
+	zassert_equal(conn.tx_q.pause_data, 1U, "Data tx is not paused");
+
 	/* TX Ack */
 	event_tx_ack(&conn, tx);
 
+	/* Check that data tx is no longer paused */
+	zassert_equal(conn.tx_q.pause_data, 0U, "Data tx is paused");
+
 	/* Done */
 	event_done(&conn);
 
@@ -1455,6 +1518,9 @@ ZTEST(phy_periph, test_phy_update_periph_loc_no_actual_change)
 	/* Done */
 	event_done(&conn);
 
+	/* Check that data tx is no longer paused */
+	zassert_equal(conn.tx_q.pause_data, 0U, "Data tx is paused");
+
 	/* There should be one notification due to Host initiated PHY UPD */
 	ut_rx_node(NODE_PHY_UPDATE, &ntf, &pu);
 	ut_rx_q_is_empty();