public inbox for linux-bluetooth@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Bluetooth: hci_core: lookup pa sync need check BIG sync state
@ 2025-07-02  1:18 Yang Li via B4 Relay
  2025-07-02  3:12 ` bluez.test.bot
  2025-07-02 13:12 ` [PATCH] " Luiz Augusto von Dentz
  0 siblings, 2 replies; 8+ messages in thread
From: Yang Li via B4 Relay @ 2025-07-02  1:18 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman
  Cc: linux-bluetooth, netdev, linux-kernel, Yang Li

From: Yang Li <yang.li@amlogic.com>

Ignore the big sync connections, we are looking for the PA
sync connection that was created as a result of the PA sync
established event.

Signed-off-by: Yang Li <yang.li@amlogic.com>
---
 include/net/bluetooth/hci_core.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 3ce1fb6f5822..646b0c5fd7a5 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1400,6 +1400,13 @@ hci_conn_hash_lookup_pa_sync_handle(struct hci_dev *hdev, __u16 sync_handle)
 		if (c->type != BIS_LINK)
 			continue;
 
+		/* Ignore the big sync connections, we are looking
+		 * for the PA sync connection that was created as
+		 * a result of the PA sync established event.
+		 */
+		if (test_bit(HCI_CONN_BIG_SYNC, &c->flags))
+			continue;
+
 		/* Ignore the listen hcon, we are looking
 		 * for the child hcon that was created as
 		 * a result of the PA sync established event.

---
base-commit: 3bc46213b81278f3a9df0324768e152de71eb9fe
change-id: 20250701-pa_sync-2fc7fc9f592c

Best regards,
-- 
Yang Li <yang.li@amlogic.com>



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* RE: Bluetooth: hci_core: lookup pa sync need check BIG sync state
  2025-07-02  1:18 [PATCH] Bluetooth: hci_core: lookup pa sync need check BIG sync state Yang Li via B4 Relay
@ 2025-07-02  3:12 ` bluez.test.bot
  2025-07-02 13:12 ` [PATCH] " Luiz Augusto von Dentz
  1 sibling, 0 replies; 8+ messages in thread
From: bluez.test.bot @ 2025-07-02  3:12 UTC (permalink / raw)
  To: linux-bluetooth, yang.li

[-- Attachment #1: Type: text/plain, Size: 3619 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=977970

---Test result---

Test Summary:
CheckPatch                    PENDING   0.27 seconds
GitLint                       PENDING   0.23 seconds
SubjectPrefix                 PASS      0.12 seconds
BuildKernel                   PASS      24.75 seconds
CheckAllWarning               PASS      27.00 seconds
CheckSparse                   PASS      30.59 seconds
BuildKernel32                 PASS      24.63 seconds
TestRunnerSetup               FAIL      102.22 seconds
TestRunner_l2cap-tester       FAIL      0.45 seconds
TestRunner_iso-tester         FAIL      0.12 seconds
TestRunner_bnep-tester        FAIL      0.12 seconds
TestRunner_mgmt-tester        FAIL      0.12 seconds
TestRunner_rfcomm-tester      FAIL      0.12 seconds
TestRunner_sco-tester         FAIL      0.12 seconds
TestRunner_ioctl-tester       FAIL      0.12 seconds
TestRunner_mesh-tester        FAIL      0.44 seconds
TestRunner_smp-tester         FAIL      0.12 seconds
TestRunner_userchan-tester    FAIL      0.12 seconds
IncrementalBuild              PENDING   0.71 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: TestRunnerSetup - FAIL
Desc: Setup kernel and bluez for test-runner
Output:
Bluez: 
src/device.c: In function ‘connect_profiles’:
src/device.c:2689:6: error: ‘ERR_BREDR_CONN_PROFILE_UNAVAILABLE’ undeclared (first use in this function)
 2689 |      ERR_BREDR_CONN_PROFILE_UNAVAILABLE);
      |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/device.c:2689:6: note: each undeclared identifier is reported only once for each function it appears in
make[1]: *** [Makefile:11162: src/bluetoothd-device.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:4690: all] Error 2
##############################
Test: TestRunner_l2cap-tester - FAIL
Desc: Run l2cap-tester with test-runner
Output:
No tester found
##############################
Test: TestRunner_iso-tester - FAIL
Desc: Run iso-tester with test-runner
Output:
No tester found
##############################
Test: TestRunner_bnep-tester - FAIL
Desc: Run bnep-tester with test-runner
Output:
No tester found
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
No tester found
##############################
Test: TestRunner_rfcomm-tester - FAIL
Desc: Run rfcomm-tester with test-runner
Output:
No tester found
##############################
Test: TestRunner_sco-tester - FAIL
Desc: Run sco-tester with test-runner
Output:
No tester found
##############################
Test: TestRunner_ioctl-tester - FAIL
Desc: Run ioctl-tester with test-runner
Output:
No tester found
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
No tester found
##############################
Test: TestRunner_smp-tester - FAIL
Desc: Run smp-tester with test-runner
Output:
No tester found
##############################
Test: TestRunner_userchan-tester - FAIL
Desc: Run userchan-tester with test-runner
Output:
No tester found
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Bluetooth: hci_core: lookup pa sync need check BIG sync state
  2025-07-02  1:18 [PATCH] Bluetooth: hci_core: lookup pa sync need check BIG sync state Yang Li via B4 Relay
  2025-07-02  3:12 ` bluez.test.bot
@ 2025-07-02 13:12 ` Luiz Augusto von Dentz
  2025-07-03  8:30   ` Yang Li
  2025-07-03  9:19   ` Yang Li
  1 sibling, 2 replies; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2025-07-02 13:12 UTC (permalink / raw)
  To: yang.li
  Cc: Marcel Holtmann, Johan Hedberg, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, linux-bluetooth,
	netdev, linux-kernel

Hi,

On Tue, Jul 1, 2025 at 9:18 PM Yang Li via B4 Relay
<devnull+yang.li.amlogic.com@kernel.org> wrote:
>
> From: Yang Li <yang.li@amlogic.com>
>
> Ignore the big sync connections, we are looking for the PA
> sync connection that was created as a result of the PA sync
> established event.

Were you seeing an issue with this, if you do please describe it and
add the traces, debug logs, etc.

> Signed-off-by: Yang Li <yang.li@amlogic.com>
> ---
>  include/net/bluetooth/hci_core.h | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 3ce1fb6f5822..646b0c5fd7a5 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -1400,6 +1400,13 @@ hci_conn_hash_lookup_pa_sync_handle(struct hci_dev *hdev, __u16 sync_handle)
>                 if (c->type != BIS_LINK)
>                         continue;
>
> +               /* Ignore the big sync connections, we are looking
> +                * for the PA sync connection that was created as
> +                * a result of the PA sync established event.
> +                */
> +               if (test_bit(HCI_CONN_BIG_SYNC, &c->flags))
> +                       continue;
> +

hci_conn_hash_lookup_pa_sync_big_handle does:

        if (c->type != BIS_LINK ||
            !test_bit(HCI_CONN_PA_SYNC, &c->flags))

>                 /* Ignore the listen hcon, we are looking
>                  * for the child hcon that was created as
>                  * a result of the PA sync established event.
>
> ---
> base-commit: 3bc46213b81278f3a9df0324768e152de71eb9fe
> change-id: 20250701-pa_sync-2fc7fc9f592c
>
> Best regards,
> --
> Yang Li <yang.li@amlogic.com>
>
>


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Bluetooth: hci_core: lookup pa sync need check BIG sync state
  2025-07-02 13:12 ` [PATCH] " Luiz Augusto von Dentz
@ 2025-07-03  8:30   ` Yang Li
  2025-07-03  8:48     ` Yang Li
  2025-07-03  9:19   ` Yang Li
  1 sibling, 1 reply; 8+ messages in thread
From: Yang Li @ 2025-07-03  8:30 UTC (permalink / raw)
  To: Luiz Augusto von Dentz
  Cc: Marcel Holtmann, Johan Hedberg, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, linux-bluetooth,
	netdev, linux-kernel

Hi Luiz,
> [ EXTERNAL EMAIL ]
>
> Hi,
>
> On Tue, Jul 1, 2025 at 9:18 PM Yang Li via B4 Relay
> <devnull+yang.li.amlogic.com@kernel.org> wrote:
>> From: Yang Li <yang.li@amlogic.com>
>>
>> Ignore the big sync connections, we are looking for the PA
>> sync connection that was created as a result of the PA sync
>> established event.
> Were you seeing an issue with this, if you do please describe it and
> add the traces, debug logs, etc.

Since the PA sync connection is set to BT_CONNECTED in 
hci_le_big_sync_established_evt, if its status is BT_CONNECTED when 
hci_abort_conn_sync is called, hci_disconnect_sync() will be executed, 
which will cause the PA sync connection to be deleted.

int hci_abort_conn_sync(struct hci_dev *hdev, struct hci_conn *conn, u8 
reason)
{
...
     switch (conn->state) {
     case BT_CONNECTED:
     case BT_CONFIG:
         err = hci_disconnect_sync(hdev, conn, reason);
         break;
...

stack trace as below:

[   55.154495][0 T1966  d.] CPU: 0 PID: 1966 Comm: kworker/u9:0 Tainted: 
G           O       6.6.77 #104
[   55.155721][0 T1966  d.] Hardware name: Amlogic (DT)
[   55.156336][0 T1966  d.] Workqueue: hci0 hci_cmd_sync_work
[   55.157018][0 T1966  d.] Call trace:
[   55.157461][0 T1966  d.]  dump_backtrace+0x94/0xec
[   55.158056][0 T1966  d.]  show_stack+0x18/0x24
[   55.158607][0 T1966  d.]  dump_stack_lvl+0x48/0x60
[   55.159205][0 T1966  d.]  dump_stack+0x18/0x24
[   55.159756][0 T1966  d.]  hci_conn_del+0x1c/0x12c
[   55.160341][0 T1966  d.]  hci_conn_failed+0xdc/0x150
[   55.160958][0 T1966  d.]  hci_abort_conn_sync+0x204/0x388
[   55.161630][0 T1966  d.]  abort_conn_sync+0x58/0x80
[   55.162237][0 T1966  d.]  hci_cmd_sync_work+0x94/0x100
[   55.162877][0 T1966  d.]  process_one_work+0x168/0x444
[   55.163516][0 T1966  d.]  worker_thread+0x378/0x3f4
[   55.164122][0 T1966  d.]  kthread+0x108/0x10c
[   55.164664][0 T1966  d.]  ret_from_fork+0x10/0x20
[   55.165408][0 T1966  d.] hci0 hcon 000000004f36962c handle 3841 #PA 
sync connection


btmon trace:

< HCI Command: Disconnect (0x01|0x0006) plen 3             #75 [hci0] 
14.640630
         Handle: 3841
         Reason: Remote User Terminated Connection (0x13)
 > HCI Event: Command Status (0x0f) plen 4                  #76 [hci0] 
14.642103
       Disconnect (0x01|0x0006) ncmd 1
         Status: Invalid HCI Command Parameters (0x12)


So the current question is whether the PA sync connection, which is 
marked as BT_CONNECTED, really needs to be disconnected.
If it does need to be disconnected, then the PA sync terminate command 
must be executed.
However, in my opinion, the PA sync connection should not be disconnected.

>
>> Signed-off-by: Yang Li <yang.li@amlogic.com>
>> ---
>>   include/net/bluetooth/hci_core.h | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>> index 3ce1fb6f5822..646b0c5fd7a5 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -1400,6 +1400,13 @@ hci_conn_hash_lookup_pa_sync_handle(struct hci_dev *hdev, __u16 sync_handle)
>>                  if (c->type != BIS_LINK)
>>                          continue;
>>
>> +               /* Ignore the big sync connections, we are looking
>> +                * for the PA sync connection that was created as
>> +                * a result of the PA sync established event.
>> +                */
>> +               if (test_bit(HCI_CONN_BIG_SYNC, &c->flags))
>> +                       continue;
>> +
> hci_conn_hash_lookup_pa_sync_big_handle does:
>
>          if (c->type != BIS_LINK ||
>              !test_bit(HCI_CONN_PA_SYNC, &c->flags))
>
>>                  /* Ignore the listen hcon, we are looking
>>                   * for the child hcon that was created as
>>                   * a result of the PA sync established event.
>>
>> ---
>> base-commit: 3bc46213b81278f3a9df0324768e152de71eb9fe
>> change-id: 20250701-pa_sync-2fc7fc9f592c
>>
>> Best regards,
>> --
>> Yang Li <yang.li@amlogic.com>
>>
>>
>
> --
> Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Bluetooth: hci_core: lookup pa sync need check BIG sync state
  2025-07-03  8:30   ` Yang Li
@ 2025-07-03  8:48     ` Yang Li
  0 siblings, 0 replies; 8+ messages in thread
From: Yang Li @ 2025-07-03  8:48 UTC (permalink / raw)
  To: Luiz Augusto von Dentz
  Cc: Marcel Holtmann, Johan Hedberg, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, linux-bluetooth,
	netdev, linux-kernel

Hi,
Please forgive my oversight,
I replied to the wrong email. Kindly ignore that response.
> Hi Luiz,
>> [ EXTERNAL EMAIL ]
>>
>> Hi,
>>
>> On Tue, Jul 1, 2025 at 9:18 PM Yang Li via B4 Relay
>> <devnull+yang.li.amlogic.com@kernel.org> wrote:
>>> From: Yang Li <yang.li@amlogic.com>
>>>
>>> Ignore the big sync connections, we are looking for the PA
>>> sync connection that was created as a result of the PA sync
>>> established event.
>> Were you seeing an issue with this, if you do please describe it and
>> add the traces, debug logs, etc.
>
> Since the PA sync connection is set to BT_CONNECTED in 
> hci_le_big_sync_established_evt, if its status is BT_CONNECTED when 
> hci_abort_conn_sync is called, hci_disconnect_sync() will be executed, 
> which will cause the PA sync connection to be deleted.
>
> int hci_abort_conn_sync(struct hci_dev *hdev, struct hci_conn *conn, 
> u8 reason)
> {
> ...
>     switch (conn->state) {
>     case BT_CONNECTED:
>     case BT_CONFIG:
>         err = hci_disconnect_sync(hdev, conn, reason);
>         break;
> ...
>
> stack trace as below:
>
> [   55.154495][0 T1966  d.] CPU: 0 PID: 1966 Comm: kworker/u9:0 
> Tainted: G           O       6.6.77 #104
> [   55.155721][0 T1966  d.] Hardware name: Amlogic (DT)
> [   55.156336][0 T1966  d.] Workqueue: hci0 hci_cmd_sync_work
> [   55.157018][0 T1966  d.] Call trace:
> [   55.157461][0 T1966  d.]  dump_backtrace+0x94/0xec
> [   55.158056][0 T1966  d.]  show_stack+0x18/0x24
> [   55.158607][0 T1966  d.]  dump_stack_lvl+0x48/0x60
> [   55.159205][0 T1966  d.]  dump_stack+0x18/0x24
> [   55.159756][0 T1966  d.]  hci_conn_del+0x1c/0x12c
> [   55.160341][0 T1966  d.]  hci_conn_failed+0xdc/0x150
> [   55.160958][0 T1966  d.]  hci_abort_conn_sync+0x204/0x388
> [   55.161630][0 T1966  d.]  abort_conn_sync+0x58/0x80
> [   55.162237][0 T1966  d.]  hci_cmd_sync_work+0x94/0x100
> [   55.162877][0 T1966  d.]  process_one_work+0x168/0x444
> [   55.163516][0 T1966  d.]  worker_thread+0x378/0x3f4
> [   55.164122][0 T1966  d.]  kthread+0x108/0x10c
> [   55.164664][0 T1966  d.]  ret_from_fork+0x10/0x20
> [   55.165408][0 T1966  d.] hci0 hcon 000000004f36962c handle 3841 #PA 
> sync connection
>
>
> btmon trace:
>
> < HCI Command: Disconnect (0x01|0x0006) plen 3             #75 [hci0] 
> 14.640630
>         Handle: 3841
>         Reason: Remote User Terminated Connection (0x13)
> > HCI Event: Command Status (0x0f) plen 4                  #76 [hci0] 
> 14.642103
>       Disconnect (0x01|0x0006) ncmd 1
>         Status: Invalid HCI Command Parameters (0x12)
>
>
> So the current question is whether the PA sync connection, which is 
> marked as BT_CONNECTED, really needs to be disconnected.
> If it does need to be disconnected, then the PA sync terminate command 
> must be executed.
> However, in my opinion, the PA sync connection should not be 
> disconnected.
>
>>
>>> Signed-off-by: Yang Li <yang.li@amlogic.com>
>>> ---
>>>   include/net/bluetooth/hci_core.h | 7 +++++++
>>>   1 file changed, 7 insertions(+)
>>>
>>> diff --git a/include/net/bluetooth/hci_core.h 
>>> b/include/net/bluetooth/hci_core.h
>>> index 3ce1fb6f5822..646b0c5fd7a5 100644
>>> --- a/include/net/bluetooth/hci_core.h
>>> +++ b/include/net/bluetooth/hci_core.h
>>> @@ -1400,6 +1400,13 @@ hci_conn_hash_lookup_pa_sync_handle(struct 
>>> hci_dev *hdev, __u16 sync_handle)
>>>                  if (c->type != BIS_LINK)
>>>                          continue;
>>>
>>> +               /* Ignore the big sync connections, we are looking
>>> +                * for the PA sync connection that was created as
>>> +                * a result of the PA sync established event.
>>> +                */
>>> +               if (test_bit(HCI_CONN_BIG_SYNC, &c->flags))
>>> +                       continue;
>>> +
>> hci_conn_hash_lookup_pa_sync_big_handle does:
>>
>>          if (c->type != BIS_LINK ||
>>              !test_bit(HCI_CONN_PA_SYNC, &c->flags))
>>
>>>                  /* Ignore the listen hcon, we are looking
>>>                   * for the child hcon that was created as
>>>                   * a result of the PA sync established event.
>>>
>>> ---
>>> base-commit: 3bc46213b81278f3a9df0324768e152de71eb9fe
>>> change-id: 20250701-pa_sync-2fc7fc9f592c
>>>
>>> Best regards,
>>> -- 
>>> Yang Li <yang.li@amlogic.com>
>>>
>>>
>>
>> -- 
>> Luiz Augusto von Dentz



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Bluetooth: hci_core: lookup pa sync need check BIG sync state
  2025-07-02 13:12 ` [PATCH] " Luiz Augusto von Dentz
  2025-07-03  8:30   ` Yang Li
@ 2025-07-03  9:19   ` Yang Li
  2025-07-03 13:29     ` Luiz Augusto von Dentz
  1 sibling, 1 reply; 8+ messages in thread
From: Yang Li @ 2025-07-03  9:19 UTC (permalink / raw)
  To: Luiz Augusto von Dentz
  Cc: Marcel Holtmann, Johan Hedberg, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, linux-bluetooth,
	netdev, linux-kernel

Hi luiz,

> [ EXTERNAL EMAIL ]
>
> Hi,
>
> On Tue, Jul 1, 2025 at 9:18 PM Yang Li via B4 Relay
> <devnull+yang.li.amlogic.com@kernel.org> wrote:
>> From: Yang Li <yang.li@amlogic.com>
>>
>> Ignore the big sync connections, we are looking for the PA
>> sync connection that was created as a result of the PA sync
>> established event.
> Were you seeing an issue with this, if you do please describe it and
> add the traces, debug logs, etc.

connect list:

[   61.826679][2 T1974  d.] list conn: conn 00000000a6e8ac83 handle 
0x0f01 state 1, flags 0x40000220

pa_sync_conn.flags = HCI_CONN_PA_SYNC

[   61.827155][2 T1974  d.] list conn: conn 0000000073b03cb6 handle 
0x0100 state 1, flags 0x48000220
[   61.828254][2 T1974  d.] list conn: conn 00000000a7e091c9 handle 
0x0101 state 1, flags 0x48000220

big_sync_conn.flags = HCI_CONN_PA_SYNC | HCI_CONN_BIG_SYNC


If the PA sync connection is deleted, then when hci_le_big_sync_lost_evt 
is executed, hci_conn_hash_lookup_pa_sync_handle should return NULL, 
However, it currently returns the BIS1 connection instead, because bis 
conn also has HCI_CONN_PA_SYNC set.

Therefore, I added an HCI_CONN_BIG_SYNC check in 
hci_conn_hash_lookup_pa_sync_handle to filter out BIS connections.

>
>> Signed-off-by: Yang Li <yang.li@amlogic.com>
>> ---
>>   include/net/bluetooth/hci_core.h | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>> index 3ce1fb6f5822..646b0c5fd7a5 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -1400,6 +1400,13 @@ hci_conn_hash_lookup_pa_sync_handle(struct hci_dev *hdev, __u16 sync_handle)
>>                  if (c->type != BIS_LINK)
>>                          continue;
>>
>> +               /* Ignore the big sync connections, we are looking
>> +                * for the PA sync connection that was created as
>> +                * a result of the PA sync established event.
>> +                */
>> +               if (test_bit(HCI_CONN_BIG_SYNC, &c->flags))
>> +                       continue;
>> +
> hci_conn_hash_lookup_pa_sync_big_handle does:
>
>          if (c->type != BIS_LINK ||
>              !test_bit(HCI_CONN_PA_SYNC, &c->flags))


Please forgive my misunderstanding.

>
>>                  /* Ignore the listen hcon, we are looking
>>                   * for the child hcon that was created as
>>                   * a result of the PA sync established event.
>>
>> ---
>> base-commit: 3bc46213b81278f3a9df0324768e152de71eb9fe
>> change-id: 20250701-pa_sync-2fc7fc9f592c
>>
>> Best regards,
>> --
>> Yang Li <yang.li@amlogic.com>
>>
>>
>
> --
> Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Bluetooth: hci_core: lookup pa sync need check BIG sync state
  2025-07-03  9:19   ` Yang Li
@ 2025-07-03 13:29     ` Luiz Augusto von Dentz
  2025-07-04  2:21       ` Yang Li
  0 siblings, 1 reply; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2025-07-03 13:29 UTC (permalink / raw)
  To: Yang Li
  Cc: Marcel Holtmann, Johan Hedberg, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, linux-bluetooth,
	netdev, linux-kernel

Hi,

On Thu, Jul 3, 2025 at 5:19 AM Yang Li <yang.li@amlogic.com> wrote:
>
> Hi luiz,
>
> > [ EXTERNAL EMAIL ]
> >
> > Hi,
> >
> > On Tue, Jul 1, 2025 at 9:18 PM Yang Li via B4 Relay
> > <devnull+yang.li.amlogic.com@kernel.org> wrote:
> >> From: Yang Li <yang.li@amlogic.com>
> >>
> >> Ignore the big sync connections, we are looking for the PA
> >> sync connection that was created as a result of the PA sync
> >> established event.
> > Were you seeing an issue with this, if you do please describe it and
> > add the traces, debug logs, etc.
>
> connect list:
>
> [   61.826679][2 T1974  d.] list conn: conn 00000000a6e8ac83 handle
> 0x0f01 state 1, flags 0x40000220
>
> pa_sync_conn.flags = HCI_CONN_PA_SYNC
>
> [   61.827155][2 T1974  d.] list conn: conn 0000000073b03cb6 handle
> 0x0100 state 1, flags 0x48000220
> [   61.828254][2 T1974  d.] list conn: conn 00000000a7e091c9 handle
> 0x0101 state 1, flags 0x48000220
>
> big_sync_conn.flags = HCI_CONN_PA_SYNC | HCI_CONN_BIG_SYNC

This is a bug then, it should have both PA_SYNC and BIG_SYNC together,
also I think we should probably disambiguate this by not using
BIS_LINK for PA_SYNC, byt introducing PA_LINK as conn->type.

>
> If the PA sync connection is deleted, then when hci_le_big_sync_lost_evt
> is executed, hci_conn_hash_lookup_pa_sync_handle should return NULL,
> However, it currently returns the BIS1 connection instead, because bis
> conn also has HCI_CONN_PA_SYNC set.
>
> Therefore, I added an HCI_CONN_BIG_SYNC check in
> hci_conn_hash_lookup_pa_sync_handle to filter out BIS connections.
>
> >
> >> Signed-off-by: Yang Li <yang.li@amlogic.com>
> >> ---
> >>   include/net/bluetooth/hci_core.h | 7 +++++++
> >>   1 file changed, 7 insertions(+)
> >>
> >> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> >> index 3ce1fb6f5822..646b0c5fd7a5 100644
> >> --- a/include/net/bluetooth/hci_core.h
> >> +++ b/include/net/bluetooth/hci_core.h
> >> @@ -1400,6 +1400,13 @@ hci_conn_hash_lookup_pa_sync_handle(struct hci_dev *hdev, __u16 sync_handle)
> >>                  if (c->type != BIS_LINK)
> >>                          continue;
> >>
> >> +               /* Ignore the big sync connections, we are looking
> >> +                * for the PA sync connection that was created as
> >> +                * a result of the PA sync established event.
> >> +                */
> >> +               if (test_bit(HCI_CONN_BIG_SYNC, &c->flags))
> >> +                       continue;
> >> +
> > hci_conn_hash_lookup_pa_sync_big_handle does:
> >
> >          if (c->type != BIS_LINK ||
> >              !test_bit(HCI_CONN_PA_SYNC, &c->flags))
>
>
> Please forgive my misunderstanding.
>
> >
> >>                  /* Ignore the listen hcon, we are looking
> >>                   * for the child hcon that was created as
> >>                   * a result of the PA sync established event.
> >>
> >> ---
> >> base-commit: 3bc46213b81278f3a9df0324768e152de71eb9fe
> >> change-id: 20250701-pa_sync-2fc7fc9f592c
> >>
> >> Best regards,
> >> --
> >> Yang Li <yang.li@amlogic.com>
> >>
> >>
> >
> > --
> > Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Bluetooth: hci_core: lookup pa sync need check BIG sync state
  2025-07-03 13:29     ` Luiz Augusto von Dentz
@ 2025-07-04  2:21       ` Yang Li
  0 siblings, 0 replies; 8+ messages in thread
From: Yang Li @ 2025-07-04  2:21 UTC (permalink / raw)
  To: Luiz Augusto von Dentz
  Cc: Marcel Holtmann, Johan Hedberg, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, linux-bluetooth,
	netdev, linux-kernel

Hi Luiz,
> [ EXTERNAL EMAIL ]
>
> Hi,
>
> On Thu, Jul 3, 2025 at 5:19 AM Yang Li <yang.li@amlogic.com> wrote:
>> Hi luiz,
>>
>>> [ EXTERNAL EMAIL ]
>>>
>>> Hi,
>>>
>>> On Tue, Jul 1, 2025 at 9:18 PM Yang Li via B4 Relay
>>> <devnull+yang.li.amlogic.com@kernel.org> wrote:
>>>> From: Yang Li <yang.li@amlogic.com>
>>>>
>>>> Ignore the big sync connections, we are looking for the PA
>>>> sync connection that was created as a result of the PA sync
>>>> established event.
>>> Were you seeing an issue with this, if you do please describe it and
>>> add the traces, debug logs, etc.
>> connect list:
>>
>> [   61.826679][2 T1974  d.] list conn: conn 00000000a6e8ac83 handle
>> 0x0f01 state 1, flags 0x40000220
>>
>> pa_sync_conn.flags = HCI_CONN_PA_SYNC
>>
>> [   61.827155][2 T1974  d.] list conn: conn 0000000073b03cb6 handle
>> 0x0100 state 1, flags 0x48000220
>> [   61.828254][2 T1974  d.] list conn: conn 00000000a7e091c9 handle
>> 0x0101 state 1, flags 0x48000220
>>
>> big_sync_conn.flags = HCI_CONN_PA_SYNC | HCI_CONN_BIG_SYNC
> This is a bug then, it should have both PA_SYNC and BIG_SYNC together,
> also I think we should probably disambiguate this by not using
> BIS_LINK for PA_SYNC, byt introducing PA_LINK as conn->type.


Yes, I agree with your point.

Adding PA_LINK can make it clearer to distinguish between PA sync and 
BIG sync links.

Let me try to update it accordingly.

>
>> If the PA sync connection is deleted, then when hci_le_big_sync_lost_evt
>> is executed, hci_conn_hash_lookup_pa_sync_handle should return NULL,
>> However, it currently returns the BIS1 connection instead, because bis
>> conn also has HCI_CONN_PA_SYNC set.
>>
>> Therefore, I added an HCI_CONN_BIG_SYNC check in
>> hci_conn_hash_lookup_pa_sync_handle to filter out BIS connections.
>>
>>>> Signed-off-by: Yang Li <yang.li@amlogic.com>
>>>> ---
>>>>    include/net/bluetooth/hci_core.h | 7 +++++++
>>>>    1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>>>> index 3ce1fb6f5822..646b0c5fd7a5 100644
>>>> --- a/include/net/bluetooth/hci_core.h
>>>> +++ b/include/net/bluetooth/hci_core.h
>>>> @@ -1400,6 +1400,13 @@ hci_conn_hash_lookup_pa_sync_handle(struct hci_dev *hdev, __u16 sync_handle)
>>>>                   if (c->type != BIS_LINK)
>>>>                           continue;
>>>>
>>>> +               /* Ignore the big sync connections, we are looking
>>>> +                * for the PA sync connection that was created as
>>>> +                * a result of the PA sync established event.
>>>> +                */
>>>> +               if (test_bit(HCI_CONN_BIG_SYNC, &c->flags))
>>>> +                       continue;
>>>> +
>>> hci_conn_hash_lookup_pa_sync_big_handle does:
>>>
>>>           if (c->type != BIS_LINK ||
>>>               !test_bit(HCI_CONN_PA_SYNC, &c->flags))
>>
>> Please forgive my misunderstanding.
>>
>>>>                   /* Ignore the listen hcon, we are looking
>>>>                    * for the child hcon that was created as
>>>>                    * a result of the PA sync established event.
>>>>
>>>> ---
>>>> base-commit: 3bc46213b81278f3a9df0324768e152de71eb9fe
>>>> change-id: 20250701-pa_sync-2fc7fc9f592c
>>>>
>>>> Best regards,
>>>> --
>>>> Yang Li <yang.li@amlogic.com>
>>>>
>>>>
>>> --
>>> Luiz Augusto von Dentz
>
>
> --
> Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-07-04  2:22 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-02  1:18 [PATCH] Bluetooth: hci_core: lookup pa sync need check BIG sync state Yang Li via B4 Relay
2025-07-02  3:12 ` bluez.test.bot
2025-07-02 13:12 ` [PATCH] " Luiz Augusto von Dentz
2025-07-03  8:30   ` Yang Li
2025-07-03  8:48     ` Yang Li
2025-07-03  9:19   ` Yang Li
2025-07-03 13:29     ` Luiz Augusto von Dentz
2025-07-04  2:21       ` Yang Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox