Bluetooth: adapt l2cap/userdata test

Modify the test so it checks that user_data is used and then cleared by
the stack.

Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>

tests/bsim/bluetooth/host/l2cap/compile.sh

@@ -14,6 +14,7 @@ app=tests/bsim/bluetooth/host/l2cap/many_conns compile
 app=tests/bsim/bluetooth/host/l2cap/multilink_peripheral compile
 app=tests/bsim/bluetooth/host/l2cap/general compile
 app=tests/bsim/bluetooth/host/l2cap/userdata compile
+app=tests/bsim/bluetooth/host/l2cap/userdata conf_file=prj_no_checks.conf compile
 app=tests/bsim/bluetooth/host/l2cap/stress compile
 app=tests/bsim/bluetooth/host/l2cap/stress conf_file=prj_nofrag.conf compile
 app=tests/bsim/bluetooth/host/l2cap/stress conf_file=prj_syswq.conf compile

tests/bsim/bluetooth/host/l2cap/userdata/prj.conf

@@ -5,6 +5,10 @@ CONFIG_BT_SMP=y
 CONFIG_BT_L2CAP_DYNAMIC_CHANNEL=y
 CONFIG_BT_L2CAP_ECRED=y
 
-CONFIG_BT_BUF_ACL_RX_SIZE=75
-
 CONFIG_ASSERT=y
+CONFIG_ARCH_POSIX_TRAP_ON_FATAL=y
+
+CONFIG_LOG=y
+CONFIG_LOG_THREAD_ID_PREFIX=y
+CONFIG_THREAD_NAME=y
+# CONFIG_BT_L2CAP_LOG_LEVEL_DBG=y

tests/bsim/bluetooth/host/l2cap/userdata/prj_no_checks.conf

@@ -0,0 +1,16 @@
+CONFIG_BT=y
+CONFIG_BT_CENTRAL=y
+CONFIG_BT_PERIPHERAL=y
+CONFIG_BT_SMP=y
+CONFIG_BT_L2CAP_DYNAMIC_CHANNEL=y
+CONFIG_BT_L2CAP_ECRED=y
+
+CONFIG_ASSERT=y
+CONFIG_ARCH_POSIX_TRAP_ON_FATAL=y
+
+CONFIG_LOG=y
+CONFIG_LOG_THREAD_ID_PREFIX=y
+CONFIG_THREAD_NAME=y
+# CONFIG_BT_L2CAP_LOG_LEVEL_DBG=y
+
+CONFIG_NO_RUNTIME_CHECKS=y

tests/bsim/bluetooth/host/l2cap/userdata/src/main_l2cap_userdata.c

@@ -14,8 +14,15 @@ extern enum bst_result_t bst_result;
 static struct bt_conn *default_conn;
 
 #define PSM 0x80
+#define DATA_SIZE 500
+#define USER_DATA_SIZE 10
+
+/* Pool to allocate a buffer that is too large to send */
+NET_BUF_POOL_DEFINE(buf_pool, 1, BT_L2CAP_SDU_BUF_SIZE(DATA_SIZE), USER_DATA_SIZE, NULL);
 
 CREATE_FLAG(is_connected);
+CREATE_FLAG(is_sent);
+CREATE_FLAG(has_received);
 CREATE_FLAG(chan_connected);
 
 static void chan_connected_cb(struct bt_l2cap_chan *l2cap_chan)
@@ -32,18 +39,32 @@ static void chan_disconnected_cb(struct bt_l2cap_chan *l2cap_chan)
 	UNSET_FLAG(chan_connected);
 }
 
+struct net_buf *alloc_buf_cb(struct bt_l2cap_chan *chan)
+{
+	return net_buf_alloc(&buf_pool, K_NO_WAIT);
+}
+
 static int chan_recv_cb(struct bt_l2cap_chan *chan, struct net_buf *buf)
 {
-	(void)chan;
-	(void)buf;
+	ARG_UNUSED(chan);
+	ARG_UNUSED(buf);
+
+	SET_FLAG(has_received);
 
 	return 0;
 }
 
+void sent_cb(struct bt_l2cap_chan *chan)
+{
+	SET_FLAG(is_sent);
+}
+
 static const struct bt_l2cap_chan_ops l2cap_ops = {
 	.connected = chan_connected_cb,
 	.disconnected = chan_disconnected_cb,
 	.recv = chan_recv_cb,
+	.sent = sent_cb,
+	.alloc_buf = alloc_buf_cb,
 };
 
 static struct bt_l2cap_le_chan channel;
@@ -53,6 +74,7 @@ static int accept(struct bt_conn *conn, struct bt_l2cap_server *server,
 {
 	channel.chan.ops = &l2cap_ops;
 	*l2cap_chan = &channel.chan;
+	channel.rx.mtu = DATA_SIZE;
 
 	return 0;
 }
@@ -70,6 +92,8 @@ static void connect_l2cap_channel(void)
 
 	channel.chan.ops = &l2cap_ops;
 
+	channel.rx.mtu = DATA_SIZE;
+
 	err = bt_l2cap_ecred_chan_connect(default_conn, chans, server.psm);
 	if (err) {
 		FAIL("Failed to send ecred connection request (err %d)\n", err);
@@ -161,22 +185,44 @@ static void test_peripheral_main(void)
 
 	register_l2cap_server();
 
-	WAIT_FOR_FLAG_UNSET(is_connected);
+	if (!IS_ENABLED(CONFIG_NO_RUNTIME_CHECKS)) {
+		PASS("Peripheral done\n");
+		return;
+	}
+
+	WAIT_FOR_FLAG_SET(has_received);
+
+	/* /\* Wait until we get the credits *\/ */
+	/* k_sleep(K_MSEC(100)); */
+
+	err = bt_conn_disconnect(default_conn, BT_HCI_ERR_REMOTE_USER_TERM_CONN);
+	if (err) {
+		FAIL("Failed to disconnect (err %d)\n", err);
+		return;
+	}
 
 	PASS("Test passed\n");
 }
 
 #define FILL	       0xAA
-#define DATA_SIZE      BT_L2CAP_BUF_SIZE(BT_L2CAP_SDU_RX_MTU) + 1
-#define USER_DATA_SIZE 10
 
-/* Pool to allocate a buffer that is too large to send */
-NET_BUF_POOL_DEFINE(buf_pool, 1, DATA_SIZE, USER_DATA_SIZE, NULL);
+static void print_user_data(struct net_buf *buf)
+{
+	for (int i = 0; i < buf->user_data_size; i++) {
+		printk("%02X", buf->user_data[i]);
+	}
+	printk("\n");
+}
 
 static void test_central_main(void)
 {
 	struct net_buf *buf;
 	int err;
+	bool has_checks = !IS_ENABLED(CONFIG_NO_RUNTIME_CHECKS);
+
+	printk("##################\n");
+	printk("(%s-checks) Starting test\n",
+	       has_checks ? "Enabled" : "Disabled");
 
 	err = bt_enable(NULL);
 	if (err != 0) {
@@ -198,40 +244,57 @@ static void test_central_main(void)
 		FAIL("Buffer allcation failed\n");
 	}
 
-	/* Don't care about the content of the packet itself, just the length */
+	net_buf_reserve(buf, BT_L2CAP_SDU_CHAN_SEND_RESERVE);
+
 	(void)net_buf_add(buf, DATA_SIZE);
-	/* Fill the user data with a pattern to more easily see any changes */
+	/* Fill the user data with a non-zero pattern */
 	(void)memset(buf->user_data, FILL, buf->user_data_size);
 
-	/* Try to send a buffer larger than the MTU */
+	printk("Buffer user_data before\n");
+	print_user_data(buf);
+
+	/* Send the buffer. We don't care that the other side receives it.
+	 * Only about when we will get called.
+	 */
 	err = bt_l2cap_chan_send(&channel.chan, buf);
-	if (err != -EMSGSIZE) {
-		FAIL("Expected error code -EMSGSIZE, got %d\n", err);
-	}
 
-	printk("Buffer user data (Expecting all bytes to be " STRINGIFY(FILL) "): ");
-	for (int i = 0; i < USER_DATA_SIZE; i++) {
-		printk("%02X", buf->user_data[i]);
-	}
-
-	printk("\n");
-
-	/* Validate that the user data is unchanged */
-	for (int i = 0; i < USER_DATA_SIZE; i++) {
-		if (buf->user_data[i] != FILL) {
-			FAIL("Buffer user data should not change.\n");
-		}
-	}
-
-	err = bt_conn_disconnect(default_conn, BT_HCI_ERR_REMOTE_USER_TERM_CONN);
-	if (err) {
-		FAIL("Failed to disconnect (err %d)\n", err);
+	if (has_checks) {
+		/* The stack is supposed to reject `buf` if it has non-null
+		 * user_data.
+		 */
+		if (err == -EINVAL) {
+			PASS("(Enabled-checks) Test passed\n");
 			return;
 		}
 
-	WAIT_FOR_FLAG_UNSET(is_connected);
+		FAIL("Expected EINVAL (%d) got %d\n", -EINVAL, err);
+	}
 
-	PASS("Test passed\n");
+	/* We have bypassed runtime checks of user_data. L2CAP will take our
+	 * `buf` with non-null user_data. We verify that:
+	 * - it is cleared
+	 * - we don't segfault later (e.g. in `tx_notify`)
+	 */
+
+	if (err != 0) {
+		FAIL("Got error %d\n", err);
+	}
+
+	WAIT_FOR_FLAG_SET(is_sent);
+
+	printk("Buffer user_data after (should've been cleared)\n");
+	print_user_data(buf);
+
+	printk("\n");
+
+	/* Validate that the user data has changed */
+	for (int i = 0; i < USER_DATA_SIZE; i++) {
+		if (buf->user_data[i] == FILL) {
+			FAIL("Buffer user data should be reset by stack.\n");
+		}
+	}
+
+	PASS("(Disabled-checks) Test passed\n");
 }
 
 static const struct bst_test_instance test_def[] = {

tests/bsim/bluetooth/host/l2cap/userdata/tests_scripts/_compile.sh

@@ -0,0 +1,13 @@
+#!/usr/bin/env bash
+# Copyright 2023 Nordic Semiconductor ASA
+# SPDX-License-Identifier: Apache-2.0
+set -eu
+: "${ZEPHYR_BASE:?ZEPHYR_BASE must be defined}"
+
+INCR_BUILD=1
+source ${ZEPHYR_BASE}/tests/bsim/compile.source
+
+app="$(guess_test_relpath)" compile
+app="$(guess_test_relpath)" conf_file=prj_no_checks.conf compile
+
+wait_for_background_jobs

tests/bsim/bluetooth/host/l2cap/userdata/tests_scripts/l2cap.sh

@@ -17,6 +17,17 @@ Execute ./bs_${BOARD_TS}_tests_bsim_bluetooth_host_l2cap_userdata_prj_conf \
     -v=${verbosity_level} -s=${simulation_id} -d=1 -testid=peripheral
 
 Execute ./bs_2G4_phy_v1 -v=${verbosity_level} -s=${simulation_id} \
-    -D=2 -sim_length=30e6 $@
+    -D=2 -sim_length=5e6 $@
+
+wait_for_background_jobs
+
+Execute ./bs_${BOARD_TS}_tests_bsim_bluetooth_host_l2cap_userdata_prj_no_checks_conf \
+    -v=${verbosity_level} -s=${simulation_id} -d=0 -testid=central
+
+Execute ./bs_${BOARD_TS}_tests_bsim_bluetooth_host_l2cap_userdata_prj_no_checks_conf \
+    -v=${verbosity_level} -s=${simulation_id} -d=1 -testid=peripheral
+
+Execute ./bs_2G4_phy_v1 -v=${verbosity_level} -s=${simulation_id} \
+    -D=2 -sim_length=5e6 $@
 
 wait_for_background_jobs