2023-05-21 15:52:09

by Pauli Virtanen

[permalink] [raw]
Subject: [PATCH 1/2] Bluetooth: ISO: consider right CIS when removing CIG at cleanup

When looking for CIS blocking CIG removal, consider only the CIS with
the right CIG ID. Don't try to remove CIG with unset CIG ID.
---
net/bluetooth/hci_conn.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index f75ef12f18f7..2363477af89d 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -950,6 +950,8 @@ static void find_cis(struct hci_conn *conn, void *data)
/* Ignore broadcast */
if (!bacmp(&conn->dst, BDADDR_ANY))
return;
+ if (d->cig != conn->iso_qos.ucast.cig)
+ return;

d->count++;
}
@@ -963,6 +965,9 @@ static void cis_cleanup(struct hci_conn *conn)
struct hci_dev *hdev = conn->hdev;
struct iso_list_data d;

+ if (conn->iso_qos.ucast.cig == BT_ISO_QOS_CIG_UNSET)
+ return;
+
memset(&d, 0, sizeof(d));
d.cig = conn->iso_qos.ucast.cig;

--
2.40.1



2023-05-21 15:52:46

by Pauli Virtanen

[permalink] [raw]
Subject: [PATCH 2/2] Bluetooth: ISO: fix CIG auto-allocation to select configurable CIG

Make CIG auto-allocation to select the first CIG_ID that is still
configurable. Also use correct CIG_ID range (see Core v5.3 Vol 4 Part E
Sec 7.8.97 p.2553).

Previously, it would always select CIG_ID 0 regardless of anything,
because cis_list with data.cis == 0xff (BT_ISO_QOS_CIS_UNSET) would not
count any CIS. Since we are not adding CIS here, use find_cis instead.
---

Notes:
It could also make sense to always auto-allocate to an unused CIG_ID
instead. However, that changes current behavior, and would force
userspace to do the allocation themselves as they may want to use as few
CIG as possible. I think e.g Intel AX210 doesn't support multiple CIG.

With the patchset adding new BlueZ iso-tester CIG tests:

ISO Connect2 CIG auto/auto Seq - Success Passed 0.148 seconds

net/bluetooth/hci_conn.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 2363477af89d..99150d054a8d 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -1771,24 +1771,23 @@ static bool hci_le_set_cig_params(struct hci_conn *conn, struct bt_iso_qos *qos)

memset(&data, 0, sizeof(data));

- /* Allocate a CIG if not set */
+ /* Allocate first still reconfigurable CIG if not set */
if (qos->ucast.cig == BT_ISO_QOS_CIG_UNSET) {
- for (data.cig = 0x00; data.cig < 0xff; data.cig++) {
+ for (data.cig = 0x00; data.cig < 0xf0; data.cig++) {
data.count = 0;
- data.cis = 0xff;

- hci_conn_hash_list_state(hdev, cis_list, ISO_LINK,
- BT_BOUND, &data);
+ hci_conn_hash_list_state(hdev, find_cis, ISO_LINK,
+ BT_CONNECT, &data);
if (data.count)
continue;

- hci_conn_hash_list_state(hdev, cis_list, ISO_LINK,
+ hci_conn_hash_list_state(hdev, find_cis, ISO_LINK,
BT_CONNECTED, &data);
if (!data.count)
break;
}

- if (data.cig == 0xff)
+ if (data.cig == 0xf0)
return false;

/* Update CIG */
--
2.40.1


2023-05-21 16:54:51

by bluez.test.bot

[permalink] [raw]
Subject: RE: [1/2] Bluetooth: ISO: consider right CIS when removing CIG at cleanup

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=749545

---Test result---

Test Summary:
CheckPatch FAIL 1.64 seconds
GitLint FAIL 0.85 seconds
SubjectPrefix PASS 0.14 seconds
BuildKernel PASS 38.70 seconds
CheckAllWarning PASS 42.33 seconds
CheckSparse PASS 47.65 seconds
CheckSmatch PASS 129.33 seconds
BuildKernel32 PASS 37.24 seconds
TestRunnerSetup PASS 532.64 seconds
TestRunner_l2cap-tester PASS 19.71 seconds
TestRunner_iso-tester PASS 25.55 seconds
TestRunner_bnep-tester PASS 6.73 seconds
TestRunner_mgmt-tester PASS 131.70 seconds
TestRunner_rfcomm-tester PASS 10.63 seconds
TestRunner_sco-tester PASS 9.81 seconds
TestRunner_ioctl-tester PASS 11.61 seconds
TestRunner_mesh-tester PASS 8.48 seconds
TestRunner_smp-tester PASS 9.66 seconds
TestRunner_userchan-tester PASS 7.11 seconds
IncrementalBuild PASS 41.57 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[1/2] Bluetooth: ISO: consider right CIS when removing CIG at cleanup
ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 0 warnings, 0 checks, 17 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/src/13249430.patch has style problems, please review.

NOTE: Ignored message types: UNKNOWN_COMMIT_ID

NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.


[2/2] Bluetooth: ISO: fix CIG auto-allocation to select configurable CIG
ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 0 warnings, 0 checks, 30 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/src/13249431.patch has style problems, please review.

NOTE: Ignored message types: UNKNOWN_COMMIT_ID

NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[2/2] Bluetooth: ISO: fix CIG auto-allocation to select configurable CIG

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
17: B2 Line has trailing whitespace: " "
19: B2 Line has trailing whitespace: " "
20: B1 Line exceeds max length (83>80): " ISO Connect2 CIG auto/auto Seq - Success Passed 0.148 seconds"


---
Regards,
Linux Bluetooth

2023-05-22 19:01:48

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 1/2] Bluetooth: ISO: consider right CIS when removing CIG at cleanup

Hi Pauli,

On Sun, May 21, 2023 at 8:52 AM Pauli Virtanen <[email protected]> wrote:
>
> When looking for CIS blocking CIG removal, consider only the CIS with
> the right CIG ID. Don't try to remove CIG with unset CIG ID.

You forgot to add Signed-off-by, also we should probably add Fixes tag
as well since we might want to backport these changes.

> ---
> net/bluetooth/hci_conn.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index f75ef12f18f7..2363477af89d 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -950,6 +950,8 @@ static void find_cis(struct hci_conn *conn, void *data)
> /* Ignore broadcast */
> if (!bacmp(&conn->dst, BDADDR_ANY))
> return;
> + if (d->cig != conn->iso_qos.ucast.cig)
> + return;
>
> d->count++;
> }
> @@ -963,6 +965,9 @@ static void cis_cleanup(struct hci_conn *conn)
> struct hci_dev *hdev = conn->hdev;
> struct iso_list_data d;
>
> + if (conn->iso_qos.ucast.cig == BT_ISO_QOS_CIG_UNSET)
> + return;
> +
> memset(&d, 0, sizeof(d));
> d.cig = conn->iso_qos.ucast.cig;
>
> --
> 2.40.1
>


--
Luiz Augusto von Dentz