* [PATCH BlueZ 1/4] iso-tester: Add tests for AC configuration reconnect
@ 2023-05-28 17:39 Pauli Virtanen
2023-05-28 17:39 ` [PATCH BlueZ 2/4] btdev: fix inactive CIG configurable status Pauli Virtanen
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Pauli Virtanen @ 2023-05-28 17:39 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Pauli Virtanen
Add test for reconnecting a CIG with two CIS, either both with same peer
or with different peers.
ISO Reconnect AC 6(i) - Success
ISO Reconnect AC 6(ii) - Success
---
Notes:
The intent here was to trigger the kernel to send Remove CIG and Set CIG
Parameters in the wrong order, but it doesn't hit the right timing in
the emulator. It occurs on real hardware when BlueZ recreates multiple
CIS at the same time.
This can occur because Remove CIG is sent via hci_sync queue and
queueing its request may be delayed until hci_cmd_sync_work runs,
whereas Set CIG Parameters request is queued directly via hci_send_cmd.
However, these tests were useful to have, as they hit Create CIS timing
issues if Set CIG Parameters is moved to hci_sync.
tools/iso-tester.c | 39 ++++++++++++++++++++++++++++++---------
1 file changed, 30 insertions(+), 9 deletions(-)
diff --git a/tools/iso-tester.c b/tools/iso-tester.c
index 63c37bd52..776d87fc1 100644
--- a/tools/iso-tester.c
+++ b/tools/iso-tester.c
@@ -839,6 +839,15 @@ static const struct iso_client_data connect_ac_6i = {
.defer = true,
};
+static const struct iso_client_data reconnect_ac_6i = {
+ .qos = AC_6i_1,
+ .qos_2 = AC_6i_2,
+ .expect_err = 0,
+ .mcis = true,
+ .defer = true,
+ .disconnect = true,
+};
+
static const struct iso_client_data connect_ac_6ii = {
.qos = AC_6ii_1,
.qos_2 = AC_6ii_2,
@@ -847,6 +856,15 @@ static const struct iso_client_data connect_ac_6ii = {
.defer = true,
};
+static const struct iso_client_data reconnect_ac_6ii = {
+ .qos = AC_6ii_1,
+ .qos_2 = AC_6ii_2,
+ .expect_err = 0,
+ .mcis = true,
+ .defer = true,
+ .disconnect = true,
+};
+
static const struct iso_client_data connect_ac_7i = {
.qos = AC_7i_1,
.qos_2 = AC_7i_2,
@@ -1626,7 +1644,7 @@ static void iso_send(struct test_data *data, GIOChannel *io)
iso_recv(data, io);
}
-static void setup_connect(struct test_data *data, uint8_t num, GIOFunc func);
+static void test_connect(const void *test_data);
static gboolean iso_connect_cb(GIOChannel *io, GIOCondition cond,
gpointer user_data);
@@ -1642,7 +1660,7 @@ static gboolean iso_disconnected(GIOChannel *io, GIOCondition cond,
if (data->reconnect) {
data->reconnect = false;
- setup_connect(data, 0, iso_connect_cb);
+ test_connect(data->test_data);
return FALSE;
}
@@ -1885,17 +1903,12 @@ static void test_connect(const void *test_data)
setup_connect_many(data, n, num, func);
}
-static void setup_reconnect(struct test_data *data, uint8_t num, GIOFunc func)
-{
- data->reconnect = true;
- setup_connect(data, num, func);
-}
-
static void test_reconnect(const void *test_data)
{
struct test_data *data = tester_get_data();
- setup_reconnect(data, 0, iso_connect_cb);
+ data->reconnect = true;
+ test_connect(test_data);
}
static void test_defer(const void *test_data)
@@ -2410,6 +2423,14 @@ int main(int argc, char *argv[])
setup_powered,
test_connect2_seq);
+ test_iso2("ISO Reconnect AC 6(i) - Success", &reconnect_ac_6i,
+ setup_powered,
+ test_reconnect);
+
+ test_iso2("ISO Reconnect AC 6(ii) - Success", &reconnect_ac_6ii,
+ setup_powered,
+ test_reconnect);
+
test_iso("ISO Broadcaster - Success", &bcast_16_2_1_send, setup_powered,
test_bcast);
test_iso("ISO Broadcaster Encrypted - Success", &bcast_enc_16_2_1_send,
--
2.40.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH BlueZ 2/4] btdev: fix inactive CIG configurable status
2023-05-28 17:39 [PATCH BlueZ 1/4] iso-tester: Add tests for AC configuration reconnect Pauli Virtanen
@ 2023-05-28 17:39 ` Pauli Virtanen
2023-05-28 17:39 ` [PATCH BlueZ 3/4] btdev: check LE Create CIS error conditions Pauli Virtanen
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Pauli Virtanen @ 2023-05-28 17:39 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Pauli Virtanen
CIG shall not be configurable after the first Create CIS until Remove
CIG is issued (Core v5.3 Vol 6 Part B Sec. 4.5.14.3). We currently have
it configurable in the inactive state (Create CIS done and all CIS
closed), which is incorrect.
Track CIG state and allow reconfigure only in nonexistent/configured
state, i.e., when no CIS have been created yet.
---
emulator/btdev.c | 32 +++++++++++++++-----------------
1 file changed, 15 insertions(+), 17 deletions(-)
diff --git a/emulator/btdev.c b/emulator/btdev.c
index f9260511a..98d7af99e 100644
--- a/emulator/btdev.c
+++ b/emulator/btdev.c
@@ -105,6 +105,7 @@ struct le_ext_adv {
struct le_cig {
struct bt_hci_cmd_le_set_cig_params params;
struct bt_hci_cis_params cis[CIS_SIZE];
+ bool activated;
} __attribute__ ((packed));
struct btdev {
@@ -5864,29 +5865,24 @@ static int cmd_set_cig_params(struct btdev *dev, const void *data,
goto done;
}
+ /* BLUETOOTH CORE SPECIFICATION Version 5.3 | Vol 4, Part E
+ * page 2553
+ *
+ * If the Host issues this command when the CIG is not in the
+ * configurable state, the Controller shall return the error
+ * code Command Disallowed (0x0C).
+ */
+ if (dev->le_cig[cig_idx].activated) {
+ rsp.params.status = BT_HCI_ERR_COMMAND_DISALLOWED;
+ goto done;
+ }
+
rsp.params.status = BT_HCI_ERR_SUCCESS;
rsp.params.cig_id = cmd->cig_id;
for (i = 0; i < cmd->num_cis; i++) {
- struct btdev_conn *iso;
-
rsp.params.num_handles++;
rsp.handle[i] = cpu_to_le16(make_cis_handle(cig_idx, i));
-
- /* BLUETOOTH CORE SPECIFICATION Version 5.3 | Vol 4, Part E
- * page 2553
- *
- * If the Host issues this command when the CIG is not in the
- * configurable state, the Controller shall return the error
- * code Command Disallowed (0x0C).
- */
- iso = queue_find(dev->conns, match_handle,
- UINT_TO_PTR(le16_to_cpu(rsp.handle[i])));
- if (iso) {
- rsp.params.status = BT_HCI_ERR_COMMAND_DISALLOWED;
- i = 0;
- goto done;
- }
}
memcpy(&dev->le_cig[cig_idx], data, len);
@@ -6006,6 +6002,8 @@ static int cmd_create_cis_complete(struct btdev *dev, const void *data,
evt.cig_id = le_cig->params.cig_id;
evt.cis_id = le_cig->cis[cis_idx].cis_id;
+ le_cig->activated = true;
+
le_meta_event(iso->link->dev, BT_HCI_EVT_LE_CIS_REQ, &evt,
sizeof(evt));
}
--
2.40.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH BlueZ 3/4] btdev: check LE Create CIS error conditions
2023-05-28 17:39 [PATCH BlueZ 1/4] iso-tester: Add tests for AC configuration reconnect Pauli Virtanen
2023-05-28 17:39 ` [PATCH BlueZ 2/4] btdev: fix inactive CIG configurable status Pauli Virtanen
@ 2023-05-28 17:39 ` Pauli Virtanen
2023-05-28 17:39 ` [PATCH BlueZ 4/4] test-runner: enable no_hash_pointers=1 Pauli Virtanen
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Pauli Virtanen @ 2023-05-28 17:39 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Pauli Virtanen
Check LE Create CIS input parameter are valid and return correct status
codes (Core v5.3 Vol 4 Part E Sec. 7.8.99).
On current bluetooth-next kernel, this results to
ISO AC 6(i) - Success Failed
ISO AC 7(i) - Success Failed
ISO AC 8(i) - Success Failed
ISO AC 9(i) - Success Failed
ISO AC 11(i) - Success Failed
as in these tests the kernel is sending new Create CIS commands before
it has seen all events from the previous, which is not allowed:
< HCI Command: LE Create Co.. (0x08|0x0064) plen 9 #129 [hci0]
Number of CIS: 2
CIS Handle: 257
ACL Handle: 42
CIS Handle: 258
ACL Handle: 42
> HCI Event: Command Status (0x0f) plen 4 #130 [hci0]
LE Create Connected Isochronous Stream (0x08|0x0064) ncmd 1
Status: Success (0x00)
> HCI Event: LE Meta Event (0x3e) plen 29 #131 [hci0]
LE Connected Isochronous Stream Established (0x19)
Status: Success (0x00)
Connection Handle: 257
...
< HCI Command: LE Setup Is.. (0x08|0x006e) plen 13 #132 [hci0]
...
> HCI Event: Command Complete (0x0e) plen 6 #133 [hci0]
LE Setup Isochronous Data Path (0x08|0x006e) ncmd 1
...
< HCI Command: LE Create Co.. (0x08|0x0064) plen 5 #134 [hci0]
Number of CIS: 1
CIS Handle: 258
ACL Handle: 42
> HCI Event: Command Status (0x0f) plen 4 #135 [hci0]
LE Create Connected Isochronous Stream (0x08|0x0064) ncmd 1
Status: ACL Connection Already Exists (0x0b)
> HCI Event: LE Meta Event (0x3e) plen 29 #136 [hci0]
LE Connected Isochronous Stream Established (0x19)
Status: Success (0x00)
Connection Handle: 258
...
The emulator uses Already Exists error code here, not Command
Disallowed, since the Established events are logically generated
immediately after the first status event, even though the kernel hasn't
yet processed them.
---
emulator/btdev.c | 38 ++++++++++++++++++++++++++++++++++++++
monitor/bt.h | 1 +
2 files changed, 39 insertions(+)
diff --git a/emulator/btdev.c b/emulator/btdev.c
index 98d7af99e..08506c66e 100644
--- a/emulator/btdev.c
+++ b/emulator/btdev.c
@@ -5903,6 +5903,38 @@ static int cmd_set_cig_params_test(struct btdev *dev, const void *data,
static int cmd_create_cis(struct btdev *dev, const void *data, uint8_t len)
{
+ const struct bt_hci_cmd_le_create_cis *cmd = data;
+ int i, j;
+
+ for (i = 0; i < cmd->num_cis; i++) {
+ const struct bt_hci_cis *cis = &cmd->cis[i];
+ struct btdev_conn *acl;
+ struct btdev_conn *iso;
+ int cig_idx, cis_idx;
+
+ /* Check for errors (Core v5.3 Vol 4 Part E Sec. 7.8.99) */
+ for (j = 0; j < i; j++)
+ if (cis->cis_handle == cmd->cis[j].cis_handle)
+ return -EINVAL;
+
+ cig_idx = parse_cis_handle(le16_to_cpu(cis->cis_handle),
+ &cis_idx);
+ if (cig_idx < 0)
+ return -ENOENT;
+ if (cis_idx >= dev->le_cig[cig_idx].params.num_cis)
+ return -ENOENT;
+
+ acl = queue_find(dev->conns, match_handle,
+ UINT_TO_PTR(le16_to_cpu(cis->acl_handle)));
+ if (!acl)
+ return -ENOENT;
+
+ iso = queue_find(dev->conns, match_handle,
+ UINT_TO_PTR(le16_to_cpu(cis->cis_handle)));
+ if (iso)
+ return -EEXIST;
+ }
+
cmd_status(dev, BT_HCI_ERR_SUCCESS, BT_HCI_CMD_LE_CREATE_CIS);
return 0;
@@ -7142,6 +7174,12 @@ static const struct btdev_cmd *run_cmd(struct btdev *btdev,
case -EPERM:
status = BT_HCI_ERR_COMMAND_DISALLOWED;
break;
+ case -EEXIST:
+ status = BT_HCI_ERR_CONN_ALREADY_EXISTS;
+ break;
+ case -ENOENT:
+ status = BT_HCI_ERR_UNKNOWN_CONN_ID;
+ break;
default:
status = BT_HCI_ERR_UNSPECIFIED_ERROR;
break;
diff --git a/monitor/bt.h b/monitor/bt.h
index b99ada0b2..37fcdaeaa 100644
--- a/monitor/bt.h
+++ b/monitor/bt.h
@@ -3713,6 +3713,7 @@ struct bt_hci_evt_le_big_info_adv_report {
#define BT_HCI_ERR_AUTH_FAILURE 0x05
#define BT_HCI_ERR_PIN_OR_KEY_MISSING 0x06
#define BT_HCI_ERR_MEM_CAPACITY_EXCEEDED 0x07
+#define BT_HCI_ERR_CONN_ALREADY_EXISTS 0x0b
#define BT_HCI_ERR_COMMAND_DISALLOWED 0x0c
#define BT_HCI_ERR_UNSUPPORTED_FEATURE 0x11
#define BT_HCI_ERR_INVALID_PARAMETERS 0x12
--
2.40.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH BlueZ 4/4] test-runner: enable no_hash_pointers=1
2023-05-28 17:39 [PATCH BlueZ 1/4] iso-tester: Add tests for AC configuration reconnect Pauli Virtanen
2023-05-28 17:39 ` [PATCH BlueZ 2/4] btdev: fix inactive CIG configurable status Pauli Virtanen
2023-05-28 17:39 ` [PATCH BlueZ 3/4] btdev: check LE Create CIS error conditions Pauli Virtanen
@ 2023-05-28 17:39 ` Pauli Virtanen
2023-05-28 20:07 ` [BlueZ,1/4] iso-tester: Add tests for AC configuration reconnect bluez.test.bot
2023-05-30 18:40 ` [PATCH BlueZ 1/4] " patchwork-bot+bluetooth
4 siblings, 0 replies; 6+ messages in thread
From: Pauli Virtanen @ 2023-05-28 17:39 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Pauli Virtanen
Set no_hash_pointers=1 to avoid printk printing "___ptrval___" for %p.
This happens with test-runner since the kernel is not running long
enough to gather entropy and so refuses to print addresses, and makes
debugging harder. As test-runner is only used for testing and
debugging, pointer hashing is not needed.
---
tools/test-runner.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/test-runner.c b/tools/test-runner.c
index 8b18f848a..d74bb1087 100644
--- a/tools/test-runner.c
+++ b/tools/test-runner.c
@@ -247,7 +247,7 @@ static void start_qemu(void)
snprintf(cmdline, sizeof(cmdline),
"console=ttyS0,115200n8 earlyprintk=serial "
- "rootfstype=9p "
+ "no_hash_pointers=1 rootfstype=9p "
"rootflags=trans=virtio,version=9p2000.u "
"acpi=off pci=noacpi noapic quiet ro init=%s "
"TESTHOME=%s TESTDBUS=%u TESTDAEMON=%u "
--
2.40.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [BlueZ,1/4] iso-tester: Add tests for AC configuration reconnect
2023-05-28 17:39 [PATCH BlueZ 1/4] iso-tester: Add tests for AC configuration reconnect Pauli Virtanen
` (2 preceding siblings ...)
2023-05-28 17:39 ` [PATCH BlueZ 4/4] test-runner: enable no_hash_pointers=1 Pauli Virtanen
@ 2023-05-28 20:07 ` bluez.test.bot
2023-05-30 18:40 ` [PATCH BlueZ 1/4] " patchwork-bot+bluetooth
4 siblings, 0 replies; 6+ messages in thread
From: bluez.test.bot @ 2023-05-28 20:07 UTC (permalink / raw)
To: linux-bluetooth, pav
[-- Attachment #1: Type: text/plain, Size: 2394 bytes --]
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=751682
---Test result---
Test Summary:
CheckPatch PASS 1.63 seconds
GitLint FAIL 1.35 seconds
BuildEll PASS 33.10 seconds
BluezMake PASS 1035.93 seconds
MakeCheck PASS 13.28 seconds
MakeDistcheck PASS 193.40 seconds
CheckValgrind PASS 305.90 seconds
CheckSmatch WARNING 407.90 seconds
bluezmakeextell PASS 124.24 seconds
IncrementalBuild PASS 3302.71 seconds
ScanBuild WARNING 1262.65 seconds
Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[BlueZ,1/4] iso-tester: Add tests for AC configuration reconnect
WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
15: B2 Line has trailing whitespace: " "
19: B2 Line has trailing whitespace: " "
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
emulator/btdev.c:417:29: warning: Variable length array is used.emulator/btdev.c:417:29: warning: Variable length array is used.
##############################
Test: ScanBuild - WARNING
Desc: Run Scan Build
Output:
emulator/btdev.c:1080:10: warning: Although the value stored to 'conn' is used in the enclosing expression, the value is never actually read from 'conn'
while ((conn = queue_find(dev->conns, match_handle,
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
emulator/btdev.c:1392:13: warning: Access to field 'dev' results in a dereference of a null pointer (loaded from variable 'conn')
send_event(conn->dev, BT_HCI_EVT_AUTH_COMPLETE, &ev, sizeof(ev));
^~~~~~~~~
2 warnings generated.
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH BlueZ 1/4] iso-tester: Add tests for AC configuration reconnect
2023-05-28 17:39 [PATCH BlueZ 1/4] iso-tester: Add tests for AC configuration reconnect Pauli Virtanen
` (3 preceding siblings ...)
2023-05-28 20:07 ` [BlueZ,1/4] iso-tester: Add tests for AC configuration reconnect bluez.test.bot
@ 2023-05-30 18:40 ` patchwork-bot+bluetooth
4 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+bluetooth @ 2023-05-30 18:40 UTC (permalink / raw)
To: Pauli Virtanen; +Cc: linux-bluetooth
Hello:
This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:
On Sun, 28 May 2023 17:39:13 +0000 you wrote:
> Add test for reconnecting a CIG with two CIS, either both with same peer
> or with different peers.
>
> ISO Reconnect AC 6(i) - Success
> ISO Reconnect AC 6(ii) - Success
> ---
>
> [...]
Here is the summary with links:
- [BlueZ,1/4] iso-tester: Add tests for AC configuration reconnect
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=000c2012f38a
- [BlueZ,2/4] btdev: fix inactive CIG configurable status
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=a8b927e34733
- [BlueZ,3/4] btdev: check LE Create CIS error conditions
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=d214fe5f7522
- [BlueZ,4/4] test-runner: enable no_hash_pointers=1
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=15eb57049b19
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-05-30 18:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-28 17:39 [PATCH BlueZ 1/4] iso-tester: Add tests for AC configuration reconnect Pauli Virtanen
2023-05-28 17:39 ` [PATCH BlueZ 2/4] btdev: fix inactive CIG configurable status Pauli Virtanen
2023-05-28 17:39 ` [PATCH BlueZ 3/4] btdev: check LE Create CIS error conditions Pauli Virtanen
2023-05-28 17:39 ` [PATCH BlueZ 4/4] test-runner: enable no_hash_pointers=1 Pauli Virtanen
2023-05-28 20:07 ` [BlueZ,1/4] iso-tester: Add tests for AC configuration reconnect bluez.test.bot
2023-05-30 18:40 ` [PATCH BlueZ 1/4] " patchwork-bot+bluetooth
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).