Re: [PATCH] Bluetooth: hci_core: lookup pa sync need check BIG sync state

[Yang Li <yang.li@amlogic.com>]

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