netdev.vger.kernel.org archive mirror
 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 13:12 ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 7+ 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] 7+ 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 13:12 ` Luiz Augusto von Dentz
  2025-07-03  8:30   ` Yang Li
  2025-07-03  9:19   ` Yang Li
  0 siblings, 2 replies; 7+ 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] 7+ messages in thread

* Re: [PATCH] Bluetooth: hci_core: lookup pa sync need check BIG sync state
  2025-07-02 13:12 ` 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; 7+ 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] 7+ 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; 7+ 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] 7+ messages in thread

* Re: [PATCH] Bluetooth: hci_core: lookup pa sync need check BIG sync state
  2025-07-02 13:12 ` 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ messages in thread

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

Thread overview: 7+ 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 13:12 ` 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;
as well as URLs for NNTP newsgroup(s).