[PATCH v3] Bluetooth: iso: fix socket matching ambiguity between BIS and CIS

[PATCH v3] Bluetooth: iso: fix socket matching ambiguity between BIS and CIS

[Yang Li]

When both BIS and CIS links exist, their sockets are in
the BT_LISTEN state.
dump sock:
  sk 000000001977ef51 state 6
  src 10:a5:62:31:05:cf dst 00:00:00:00:00:00
  sk 0000000031d28700 state 7
  src 10:a5:62:31:05:cf dst00:00:00:00:00:00
  sk 00000000613af00e state 4   # listen sock of bis
  src 10:a5:62:31:05:cf dst 54:00:00:d4:99:30
  sk 000000001710468c state 9
  src 10:a5:62:31:05:cf dst 54:00:00:d4:99:30
  sk 000000005d97dfde state 4   #listen sock of cis
  src 10:a5:62:31:05:cf dst 00:00:00:00:00:00

To locate the CIS socket correctly, check both the BT_LISTEN
state and whether dst addr is BDADDR_ANY.

Link: https://github.com/bluez/bluez/issues/1224

Signed-off-by: Yang Li <yang.li@amlogic.com>
---
 net/bluetooth/iso.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
index eaffd25570e3..9a4dea03af8c 100644
--- a/net/bluetooth/iso.c
+++ b/net/bluetooth/iso.c
@@ -1919,6 +1919,11 @@ static bool iso_match_pa_sync_flag(struct sock *sk, void *data)
 	return test_bit(BT_SK_PA_SYNC, &iso_pi(sk)->flags);
 }
 
+static bool iso_match_dst(struct sock *sk, void *data)
+{
+	return !bacmp(&iso_pi(sk)->dst, (bdaddr_t *)data);
+}
+
 static void iso_conn_ready(struct iso_conn *conn)
 {
 	struct sock *parent = NULL;
@@ -1981,7 +1986,7 @@ static void iso_conn_ready(struct iso_conn *conn)
 
 		if (!parent)
 			parent = iso_get_sock(&hcon->src, BDADDR_ANY,
-					      BT_LISTEN, NULL, NULL);
+					      BT_LISTEN, iso_match_dst, BDADDR_ANY);
 
 		if (!parent)
 			return;
@@ -2220,7 +2225,7 @@ int iso_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags)
 		}
 	} else {
 		sk = iso_get_sock(&hdev->bdaddr, BDADDR_ANY,
-				  BT_LISTEN, NULL, NULL);
+				  BT_LISTEN, iso_match_dst, BDADDR_ANY);
 	}
 
 done:

---
base-commit: 9c533991fe15be60ad9f9a7629c25dbc5b09788d
change-id: 20250731-bis_cis_coexist-717a442d5c42

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

[PATCH v3] Bluetooth: iso: fix socket matching ambiguity between BIS and CIS

[Yang Li via B4 Relay]

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

When both BIS and CIS links exist, their sockets are in
the BT_LISTEN state.
dump sock:
  sk 000000001977ef51 state 6
  src 10:a5:62:31:05:cf dst 00:00:00:00:00:00
  sk 0000000031d28700 state 7
  src 10:a5:62:31:05:cf dst00:00:00:00:00:00
  sk 00000000613af00e state 4   # listen sock of bis
  src 10:a5:62:31:05:cf dst 54:00:00:d4:99:30
  sk 000000001710468c state 9
  src 10:a5:62:31:05:cf dst 54:00:00:d4:99:30
  sk 000000005d97dfde state 4   #listen sock of cis
  src 10:a5:62:31:05:cf dst 00:00:00:00:00:00

To locate the CIS socket correctly, check both the BT_LISTEN
state and whether dst addr is BDADDR_ANY.

Link: https://github.com/bluez/bluez/issues/1224

Signed-off-by: Yang Li <yang.li@amlogic.com>
---
 net/bluetooth/iso.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
index eaffd25570e3..9a4dea03af8c 100644
--- a/net/bluetooth/iso.c
+++ b/net/bluetooth/iso.c
@@ -1919,6 +1919,11 @@ static bool iso_match_pa_sync_flag(struct sock *sk, void *data)
 	return test_bit(BT_SK_PA_SYNC, &iso_pi(sk)->flags);
 }
 
+static bool iso_match_dst(struct sock *sk, void *data)
+{
+	return !bacmp(&iso_pi(sk)->dst, (bdaddr_t *)data);
+}
+
 static void iso_conn_ready(struct iso_conn *conn)
 {
 	struct sock *parent = NULL;
@@ -1981,7 +1986,7 @@ static void iso_conn_ready(struct iso_conn *conn)
 
 		if (!parent)
 			parent = iso_get_sock(&hcon->src, BDADDR_ANY,
-					      BT_LISTEN, NULL, NULL);
+					      BT_LISTEN, iso_match_dst, BDADDR_ANY);
 
 		if (!parent)
 			return;
@@ -2220,7 +2225,7 @@ int iso_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags)
 		}
 	} else {
 		sk = iso_get_sock(&hdev->bdaddr, BDADDR_ANY,
-				  BT_LISTEN, NULL, NULL);
+				  BT_LISTEN, iso_match_dst, BDADDR_ANY);
 	}
 
 done:

---
base-commit: 9c533991fe15be60ad9f9a7629c25dbc5b09788d
change-id: 20250731-bis_cis_coexist-717a442d5c42

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

RE: [v3] Bluetooth: iso: fix socket matching ambiguity between BIS and CIS

[bluez.test.bot]

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

---Test result---

Test Summary:
CheckPatch                    PENDING   0.40 seconds
GitLint                       PENDING   0.35 seconds
SubjectPrefix                 PASS      0.09 seconds
BuildKernel                   PASS      24.55 seconds
CheckAllWarning               PASS      27.21 seconds
CheckSparse                   PASS      30.76 seconds
BuildKernel32                 PASS      24.68 seconds
TestRunnerSetup               PASS      481.00 seconds
TestRunner_l2cap-tester       PASS      24.72 seconds
TestRunner_iso-tester         PASS      40.13 seconds
TestRunner_bnep-tester        PASS      5.99 seconds
TestRunner_mgmt-tester        FAIL      126.50 seconds
TestRunner_rfcomm-tester      PASS      9.37 seconds
TestRunner_sco-tester         PASS      14.63 seconds
TestRunner_ioctl-tester       PASS      10.06 seconds
TestRunner_mesh-tester        FAIL      11.53 seconds
TestRunner_smp-tester         PASS      8.62 seconds
TestRunner_userchan-tester    PASS      6.23 seconds
IncrementalBuild              PENDING   0.98 seconds

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

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

##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 490, Passed: 485 (99.0%), Failed: 1, Not Run: 4

Failed Test Cases
LL Privacy - Set Flags 3 (2 Devices to RL)           Failed       0.200 seconds
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
Total: 10, Passed: 8 (80.0%), Failed: 2, Not Run: 0

Failed Test Cases
Mesh - Send cancel - 1                               Timed out    2.054 seconds
Mesh - Send cancel - 2                               Timed out    1.998 seconds
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth

Re: [PATCH v3] Bluetooth: iso: fix socket matching ambiguity between BIS and CIS

[Luiz Augusto von Dentz]

Hi Yang,

On Thu, Jul 31, 2025 at 4:00 AM Yang Li via B4 Relay
<devnull+yang.li.amlogic.com@kernel.org> wrote:
>
> From: Yang Li <yang.li@amlogic.com>
>
> When both BIS and CIS links exist, their sockets are in
> the BT_LISTEN state.

We probably need to introduce tests to iso-test that setup both then
to avoid reintroducing the problem.

> dump sock:
>   sk 000000001977ef51 state 6
>   src 10:a5:62:31:05:cf dst 00:00:00:00:00:00
>   sk 0000000031d28700 state 7
>   src 10:a5:62:31:05:cf dst00:00:00:00:00:00
>   sk 00000000613af00e state 4   # listen sock of bis
>   src 10:a5:62:31:05:cf dst 54:00:00:d4:99:30
>   sk 000000001710468c state 9
>   src 10:a5:62:31:05:cf dst 54:00:00:d4:99:30
>   sk 000000005d97dfde state 4   #listen sock of cis
>   src 10:a5:62:31:05:cf dst 00:00:00:00:00:00
>
> To locate the CIS socket correctly, check both the BT_LISTEN
> state and whether dst addr is BDADDR_ANY.
>
> Link: https://github.com/bluez/bluez/issues/1224
>
> Signed-off-by: Yang Li <yang.li@amlogic.com>
> ---
>  net/bluetooth/iso.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
> index eaffd25570e3..9a4dea03af8c 100644
> --- a/net/bluetooth/iso.c
> +++ b/net/bluetooth/iso.c
> @@ -1919,6 +1919,11 @@ static bool iso_match_pa_sync_flag(struct sock *sk, void *data)
>         return test_bit(BT_SK_PA_SYNC, &iso_pi(sk)->flags);
>  }
>
> +static bool iso_match_dst(struct sock *sk, void *data)
> +{
> +       return !bacmp(&iso_pi(sk)->dst, (bdaddr_t *)data);
> +}
> +
>  static void iso_conn_ready(struct iso_conn *conn)
>  {
>         struct sock *parent = NULL;
> @@ -1981,7 +1986,7 @@ static void iso_conn_ready(struct iso_conn *conn)
>
>                 if (!parent)
>                         parent = iso_get_sock(&hcon->src, BDADDR_ANY,
> -                                             BT_LISTEN, NULL, NULL);
> +                                             BT_LISTEN, iso_match_dst, BDADDR_ANY);
>
>                 if (!parent)
>                         return;
> @@ -2220,7 +2225,7 @@ int iso_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags)
>                 }
>         } else {
>                 sk = iso_get_sock(&hdev->bdaddr, BDADDR_ANY,
> -                                 BT_LISTEN, NULL, NULL);
> +                                 BT_LISTEN, iso_match_dst, BDADDR_ANY);

Perhaps we should add helper function that wrap the iso_get_sock (e.g.
iso_get_sock_cis and iso_get_sock_bis) to make it clearer what is the
expected socket type, also if the hcon has been set perhaps that
should be matched as well with CIS_LINK/BIS_LINK, or perhaps we
introduce a socket type to differentiate since the use of the address
can make the logic a little confusing when the socket types are mixed
together.

Now looking at the source code perhaps we can have a separate list for
cis and bis sockets instead of global iso_sk_list (e.g. cis_sk_list
and bis_sk_list), that way we don't need a type and there is no risk
of confusing the sockets since they would never be in the same list.

>         }
>
>  done:
>
> ---
> base-commit: 9c533991fe15be60ad9f9a7629c25dbc5b09788d
> change-id: 20250731-bis_cis_coexist-717a442d5c42
>
> Best regards,
> --
> Yang Li <yang.li@amlogic.com>
>
>


-- 
Luiz Augusto von Dentz

Re: [PATCH v3] Bluetooth: iso: fix socket matching ambiguity between BIS and CIS

[Yang Li]

Hi Luiz,
> [ EXTERNAL EMAIL ]
>
> Hi Yang,
>
> On Thu, Jul 31, 2025 at 4:00 AM Yang Li via B4 Relay
> <devnull+yang.li.amlogic.com@kernel.org> wrote:
>> From: Yang Li <yang.li@amlogic.com>
>>
>> When both BIS and CIS links exist, their sockets are in
>> the BT_LISTEN state.
> We probably need to introduce tests to iso-test that setup both then
> to avoid reintroducing the problem.


Since the coexistence of BIS sink and CIS sink is determined by 
application-level logic, it may be difficult to reproduce this scenario 
using iso-test.

Do you have any suggestions on how to simulate or test this case more 
effectively?

>
>> dump sock:
>>    sk 000000001977ef51 state 6
>>    src 10:a5:62:31:05:cf dst 00:00:00:00:00:00
>>    sk 0000000031d28700 state 7
>>    src 10:a5:62:31:05:cf dst00:00:00:00:00:00
>>    sk 00000000613af00e state 4   # listen sock of bis
>>    src 10:a5:62:31:05:cf dst 54:00:00:d4:99:30
>>    sk 000000001710468c state 9
>>    src 10:a5:62:31:05:cf dst 54:00:00:d4:99:30
>>    sk 000000005d97dfde state 4   #listen sock of cis
>>    src 10:a5:62:31:05:cf dst 00:00:00:00:00:00
>>
>> To locate the CIS socket correctly, check both the BT_LISTEN
>> state and whether dst addr is BDADDR_ANY.
>>
>> Link: https://github.com/bluez/bluez/issues/1224
>>
>> Signed-off-by: Yang Li <yang.li@amlogic.com>
>> ---
>>   net/bluetooth/iso.c | 9 +++++++--
>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
>> index eaffd25570e3..9a4dea03af8c 100644
>> --- a/net/bluetooth/iso.c
>> +++ b/net/bluetooth/iso.c
>> @@ -1919,6 +1919,11 @@ static bool iso_match_pa_sync_flag(struct sock *sk, void *data)
>>          return test_bit(BT_SK_PA_SYNC, &iso_pi(sk)->flags);
>>   }
>>
>> +static bool iso_match_dst(struct sock *sk, void *data)
>> +{
>> +       return !bacmp(&iso_pi(sk)->dst, (bdaddr_t *)data);
>> +}
>> +
>>   static void iso_conn_ready(struct iso_conn *conn)
>>   {
>>          struct sock *parent = NULL;
>> @@ -1981,7 +1986,7 @@ static void iso_conn_ready(struct iso_conn *conn)
>>
>>                  if (!parent)
>>                          parent = iso_get_sock(&hcon->src, BDADDR_ANY,
>> -                                             BT_LISTEN, NULL, NULL);
>> +                                             BT_LISTEN, iso_match_dst, BDADDR_ANY);
>>
>>                  if (!parent)
>>                          return;
>> @@ -2220,7 +2225,7 @@ int iso_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags)
>>                  }
>>          } else {
>>                  sk = iso_get_sock(&hdev->bdaddr, BDADDR_ANY,
>> -                                 BT_LISTEN, NULL, NULL);
>> +                                 BT_LISTEN, iso_match_dst, BDADDR_ANY);
> Perhaps we should add helper function that wrap the iso_get_sock (e.g.
> iso_get_sock_cis and iso_get_sock_bis) to make it clearer what is the
> expected socket type, also if the hcon has been set perhaps that
> should be matched as well with CIS_LINK/BIS_LINK, or perhaps we
> introduce a socket type to differentiate since the use of the address
> can make the logic a little confusing when the socket types are mixed
> together.
>
> Now looking at the source code perhaps we can have a separate list for
> cis and bis sockets instead of global iso_sk_list (e.g. cis_sk_list
> and bis_sk_list), that way we don't need a type and there is no risk
> of confusing the sockets since they would never be in the same list.


Alright, I will give it a try.

>
>>          }
>>
>>   done:
>>
>> ---
>> base-commit: 9c533991fe15be60ad9f9a7629c25dbc5b09788d
>> change-id: 20250731-bis_cis_coexist-717a442d5c42
>>
>> Best regards,
>> --
>> Yang Li <yang.li@amlogic.com>
>>
>>
>
> --
> Luiz Augusto von Dentz

Re: [PATCH v3] Bluetooth: iso: fix socket matching ambiguity between BIS and CIS

[Luiz Augusto von Dentz]

Hi Yang,

On Sun, Aug 3, 2025 at 9:07 PM Yang Li <yang.li@amlogic.com> wrote:
>
> Hi Luiz,
> > [ EXTERNAL EMAIL ]
> >
> > Hi Yang,
> >
> > On Thu, Jul 31, 2025 at 4:00 AM Yang Li via B4 Relay
> > <devnull+yang.li.amlogic.com@kernel.org> wrote:
> >> From: Yang Li <yang.li@amlogic.com>
> >>
> >> When both BIS and CIS links exist, their sockets are in
> >> the BT_LISTEN state.
> > We probably need to introduce tests to iso-test that setup both then
> > to avoid reintroducing the problem.
>
>
> Since the coexistence of BIS sink and CIS sink is determined by
> application-level logic, it may be difficult to reproduce this scenario
> using iso-test.

Looks like you haven't look at what iso-tester tools tests do, that is
not tight to bluetoothd, it directly operates at the socket layer so
we can create any scenario we want.

> Do you have any suggestions on how to simulate or test this case more
> effectively?
>
> >
> >> dump sock:
> >>    sk 000000001977ef51 state 6
> >>    src 10:a5:62:31:05:cf dst 00:00:00:00:00:00
> >>    sk 0000000031d28700 state 7
> >>    src 10:a5:62:31:05:cf dst00:00:00:00:00:00
> >>    sk 00000000613af00e state 4   # listen sock of bis
> >>    src 10:a5:62:31:05:cf dst 54:00:00:d4:99:30
> >>    sk 000000001710468c state 9
> >>    src 10:a5:62:31:05:cf dst 54:00:00:d4:99:30
> >>    sk 000000005d97dfde state 4   #listen sock of cis
> >>    src 10:a5:62:31:05:cf dst 00:00:00:00:00:00
> >>
> >> To locate the CIS socket correctly, check both the BT_LISTEN
> >> state and whether dst addr is BDADDR_ANY.
> >>
> >> Link: https://github.com/bluez/bluez/issues/1224
> >>
> >> Signed-off-by: Yang Li <yang.li@amlogic.com>
> >> ---
> >>   net/bluetooth/iso.c | 9 +++++++--
> >>   1 file changed, 7 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
> >> index eaffd25570e3..9a4dea03af8c 100644
> >> --- a/net/bluetooth/iso.c
> >> +++ b/net/bluetooth/iso.c
> >> @@ -1919,6 +1919,11 @@ static bool iso_match_pa_sync_flag(struct sock *sk, void *data)
> >>          return test_bit(BT_SK_PA_SYNC, &iso_pi(sk)->flags);
> >>   }
> >>
> >> +static bool iso_match_dst(struct sock *sk, void *data)
> >> +{
> >> +       return !bacmp(&iso_pi(sk)->dst, (bdaddr_t *)data);
> >> +}
> >> +
> >>   static void iso_conn_ready(struct iso_conn *conn)
> >>   {
> >>          struct sock *parent = NULL;
> >> @@ -1981,7 +1986,7 @@ static void iso_conn_ready(struct iso_conn *conn)
> >>
> >>                  if (!parent)
> >>                          parent = iso_get_sock(&hcon->src, BDADDR_ANY,
> >> -                                             BT_LISTEN, NULL, NULL);
> >> +                                             BT_LISTEN, iso_match_dst, BDADDR_ANY);
> >>
> >>                  if (!parent)
> >>                          return;
> >> @@ -2220,7 +2225,7 @@ int iso_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags)
> >>                  }
> >>          } else {
> >>                  sk = iso_get_sock(&hdev->bdaddr, BDADDR_ANY,
> >> -                                 BT_LISTEN, NULL, NULL);
> >> +                                 BT_LISTEN, iso_match_dst, BDADDR_ANY);
> > Perhaps we should add helper function that wrap the iso_get_sock (e.g.
> > iso_get_sock_cis and iso_get_sock_bis) to make it clearer what is the
> > expected socket type, also if the hcon has been set perhaps that
> > should be matched as well with CIS_LINK/BIS_LINK, or perhaps we
> > introduce a socket type to differentiate since the use of the address
> > can make the logic a little confusing when the socket types are mixed
> > together.
> >
> > Now looking at the source code perhaps we can have a separate list for
> > cis and bis sockets instead of global iso_sk_list (e.g. cis_sk_list
> > and bis_sk_list), that way we don't need a type and there is no risk
> > of confusing the sockets since they would never be in the same list.
>
>
> Alright, I will give it a try.
>
> >
> >>          }
> >>
> >>   done:
> >>
> >> ---
> >> base-commit: 9c533991fe15be60ad9f9a7629c25dbc5b09788d
> >> change-id: 20250731-bis_cis_coexist-717a442d5c42
> >>
> >> Best regards,
> >> --
> >> Yang Li <yang.li@amlogic.com>
> >>
> >>
> >
> > --
> > Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

Re: [PATCH v3] Bluetooth: iso: fix socket matching ambiguity between BIS and CIS

[Yang Li]

Hi Luiz,
> [ EXTERNAL EMAIL ]
>
> Hi Yang,
>
> On Sun, Aug 3, 2025 at 9:07 PM Yang Li <yang.li@amlogic.com> wrote:
>> Hi Luiz,
>>> [ EXTERNAL EMAIL ]
>>>
>>> Hi Yang,
>>>
>>> On Thu, Jul 31, 2025 at 4:00 AM Yang Li via B4 Relay
>>> <devnull+yang.li.amlogic.com@kernel.org> wrote:
>>>> From: Yang Li <yang.li@amlogic.com>
>>>>
>>>> When both BIS and CIS links exist, their sockets are in
>>>> the BT_LISTEN state.
>>> We probably need to introduce tests to iso-test that setup both then
>>> to avoid reintroducing the problem.
>>
>> Since the coexistence of BIS sink and CIS sink is determined by
>> application-level logic, it may be difficult to reproduce this scenario
>> using iso-test.
> Looks like you haven't look at what iso-tester tools tests do, that is
> not tight to bluetoothd, it directly operates at the socket layer so
> we can create any scenario we want.


Based on the current structure of iso-tester, it is not possible to 
implement test cases where CIS and BIS listen simultaneously. There are 
several issues:

 1.

    In struct iso_client_data, although both CIS and BIS related
    elements are defined, they are mutually exclusive. CIS and BIS
    cannot be used at the same time. For example, .bcast must explicitly
    specify whether it is broadcast or unicast.

 2.

    The setup_listen_many function also identifies BIS or CIS through
    .bcast.

Therefore, if we want to add test cases for the coexistence of BIS and 
CIS, the current data structure needs to be modified to completely 
separate the elements for BIS and CIS.


>> Do you have any suggestions on how to simulate or test this case more
>> effectively?
>>
>>>> dump sock:
>>>>     sk 000000001977ef51 state 6
>>>>     src 10:a5:62:31:05:cf dst 00:00:00:00:00:00
>>>>     sk 0000000031d28700 state 7
>>>>     src 10:a5:62:31:05:cf dst00:00:00:00:00:00
>>>>     sk 00000000613af00e state 4   # listen sock of bis
>>>>     src 10:a5:62:31:05:cf dst 54:00:00:d4:99:30
>>>>     sk 000000001710468c state 9
>>>>     src 10:a5:62:31:05:cf dst 54:00:00:d4:99:30
>>>>     sk 000000005d97dfde state 4   #listen sock of cis
>>>>     src 10:a5:62:31:05:cf dst 00:00:00:00:00:00
>>>>
>>>> To locate the CIS socket correctly, check both the BT_LISTEN
>>>> state and whether dst addr is BDADDR_ANY.
>>>>
>>>> Link: https://github.com/bluez/bluez/issues/1224
>>>>
>>>> Signed-off-by: Yang Li <yang.li@amlogic.com>
>>>> ---
>>>>    net/bluetooth/iso.c | 9 +++++++--
>>>>    1 file changed, 7 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
>>>> index eaffd25570e3..9a4dea03af8c 100644
>>>> --- a/net/bluetooth/iso.c
>>>> +++ b/net/bluetooth/iso.c
>>>> @@ -1919,6 +1919,11 @@ static bool iso_match_pa_sync_flag(struct sock *sk, void *data)
>>>>           return test_bit(BT_SK_PA_SYNC, &iso_pi(sk)->flags);
>>>>    }
>>>>
>>>> +static bool iso_match_dst(struct sock *sk, void *data)
>>>> +{
>>>> +       return !bacmp(&iso_pi(sk)->dst, (bdaddr_t *)data);
>>>> +}
>>>> +
>>>>    static void iso_conn_ready(struct iso_conn *conn)
>>>>    {
>>>>           struct sock *parent = NULL;
>>>> @@ -1981,7 +1986,7 @@ static void iso_conn_ready(struct iso_conn *conn)
>>>>
>>>>                   if (!parent)
>>>>                           parent = iso_get_sock(&hcon->src, BDADDR_ANY,
>>>> -                                             BT_LISTEN, NULL, NULL);
>>>> +                                             BT_LISTEN, iso_match_dst, BDADDR_ANY);
>>>>
>>>>                   if (!parent)
>>>>                           return;
>>>> @@ -2220,7 +2225,7 @@ int iso_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags)
>>>>                   }
>>>>           } else {
>>>>                   sk = iso_get_sock(&hdev->bdaddr, BDADDR_ANY,
>>>> -                                 BT_LISTEN, NULL, NULL);
>>>> +                                 BT_LISTEN, iso_match_dst, BDADDR_ANY);
>>> Perhaps we should add helper function that wrap the iso_get_sock (e.g.
>>> iso_get_sock_cis and iso_get_sock_bis) to make it clearer what is the
>>> expected socket type, also if the hcon has been set perhaps that
>>> should be matched as well with CIS_LINK/BIS_LINK, or perhaps we
>>> introduce a socket type to differentiate since the use of the address
>>> can make the logic a little confusing when the socket types are mixed
>>> together.
>>>
>>> Now looking at the source code perhaps we can have a separate list for
>>> cis and bis sockets instead of global iso_sk_list (e.g. cis_sk_list
>>> and bis_sk_list), that way we don't need a type and there is no risk
>>> of confusing the sockets since they would never be in the same list.
>>
>> Alright, I will give it a try.
>>
>>>>           }
>>>>
>>>>    done:
>>>>
>>>> ---
>>>> base-commit: 9c533991fe15be60ad9f9a7629c25dbc5b09788d
>>>> change-id: 20250731-bis_cis_coexist-717a442d5c42
>>>>
>>>> Best regards,
>>>> --
>>>> Yang Li <yang.li@amlogic.com>
>>>>
>>>>
>>> --
>>> Luiz Augusto von Dentz
>
>
> --
> Luiz Augusto von Dentz

Re: [PATCH v3] Bluetooth: iso: fix socket matching ambiguity between BIS and CIS

[Yang Li]

Hi Luiz,
A gentle ping, thanks.

> Hi Luiz,
>> [ EXTERNAL EMAIL ]
>>
>> Hi Yang,
>>
>> On Sun, Aug 3, 2025 at 9:07 PM Yang Li <yang.li@amlogic.com> wrote:
>>> Hi Luiz,
>>>> [ EXTERNAL EMAIL ]
>>>>
>>>> Hi Yang,
>>>>
>>>> On Thu, Jul 31, 2025 at 4:00 AM Yang Li via B4 Relay
>>>> <devnull+yang.li.amlogic.com@kernel.org> wrote:
>>>>> From: Yang Li <yang.li@amlogic.com>
>>>>>
>>>>> When both BIS and CIS links exist, their sockets are in
>>>>> the BT_LISTEN state.
>>>> We probably need to introduce tests to iso-test that setup both then
>>>> to avoid reintroducing the problem.
>>>
>>> Since the coexistence of BIS sink and CIS sink is determined by
>>> application-level logic, it may be difficult to reproduce this scenario
>>> using iso-test.
>> Looks like you haven't look at what iso-tester tools tests do, that is
>> not tight to bluetoothd, it directly operates at the socket layer so
>> we can create any scenario we want.
>
>
> Based on the current structure of iso-tester, it is not possible to 
> implement test cases where CIS and BIS listen simultaneously. There 
> are several issues:
>
> 1.
>
>    In struct iso_client_data, although both CIS and BIS related
>    elements are defined, they are mutually exclusive. CIS and BIS
>    cannot be used at the same time. For example, .bcast must explicitly
>    specify whether it is broadcast or unicast.
>
> 2.
>
>    The setup_listen_many function also identifies BIS or CIS through
>    .bcast.
>
> Therefore, if we want to add test cases for the coexistence of BIS and 
> CIS, the current data structure needs to be modified to completely 
> separate the elements for BIS and CIS.
>
>

I'm not sure if my understanding is fully correct, so I would appreciate 
any guidance or suggestions.

Based on my testing, this patch does fix the issue on my side.
Please let me know if there is anything I may have missed or if further 
changes are needed.


>>> Do you have any suggestions on how to simulate or test this case more
>>> effectively?
>>>
>>>>> dump sock:
>>>>>     sk 000000001977ef51 state 6
>>>>>     src 10:a5:62:31:05:cf dst 00:00:00:00:00:00
>>>>>     sk 0000000031d28700 state 7
>>>>>     src 10:a5:62:31:05:cf dst00:00:00:00:00:00
>>>>>     sk 00000000613af00e state 4   # listen sock of bis
>>>>>     src 10:a5:62:31:05:cf dst 54:00:00:d4:99:30
>>>>>     sk 000000001710468c state 9
>>>>>     src 10:a5:62:31:05:cf dst 54:00:00:d4:99:30
>>>>>     sk 000000005d97dfde state 4   #listen sock of cis
>>>>>     src 10:a5:62:31:05:cf dst 00:00:00:00:00:00
>>>>>
>>>>> To locate the CIS socket correctly, check both the BT_LISTEN
>>>>> state and whether dst addr is BDADDR_ANY.
>>>>>
>>>>> Link: https://github.com/bluez/bluez/issues/1224
>>>>>
>>>>> Signed-off-by: Yang Li <yang.li@amlogic.com>
>>>>> ---
>>>>>    net/bluetooth/iso.c | 9 +++++++--
>>>>>    1 file changed, 7 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
>>>>> index eaffd25570e3..9a4dea03af8c 100644
>>>>> --- a/net/bluetooth/iso.c
>>>>> +++ b/net/bluetooth/iso.c
>>>>> @@ -1919,6 +1919,11 @@ static bool iso_match_pa_sync_flag(struct 
>>>>> sock *sk, void *data)
>>>>>           return test_bit(BT_SK_PA_SYNC, &iso_pi(sk)->flags);
>>>>>    }
>>>>>
>>>>> +static bool iso_match_dst(struct sock *sk, void *data)
>>>>> +{
>>>>> +       return !bacmp(&iso_pi(sk)->dst, (bdaddr_t *)data);
>>>>> +}
>>>>> +
>>>>>    static void iso_conn_ready(struct iso_conn *conn)
>>>>>    {
>>>>>           struct sock *parent = NULL;
>>>>> @@ -1981,7 +1986,7 @@ static void iso_conn_ready(struct iso_conn 
>>>>> *conn)
>>>>>
>>>>>                   if (!parent)
>>>>>                           parent = iso_get_sock(&hcon->src, 
>>>>> BDADDR_ANY,
>>>>> -                                             BT_LISTEN, NULL, NULL);
>>>>> +                                             BT_LISTEN, 
>>>>> iso_match_dst, BDADDR_ANY);
>>>>>
>>>>>                   if (!parent)
>>>>>                           return;
>>>>> @@ -2220,7 +2225,7 @@ int iso_connect_ind(struct hci_dev *hdev, 
>>>>> bdaddr_t *bdaddr, __u8 *flags)
>>>>>                   }
>>>>>           } else {
>>>>>                   sk = iso_get_sock(&hdev->bdaddr, BDADDR_ANY,
>>>>> -                                 BT_LISTEN, NULL, NULL);
>>>>> +                                 BT_LISTEN, iso_match_dst, 
>>>>> BDADDR_ANY);
>>>> Perhaps we should add helper function that wrap the iso_get_sock (e.g.
>>>> iso_get_sock_cis and iso_get_sock_bis) to make it clearer what is the
>>>> expected socket type, also if the hcon has been set perhaps that
>>>> should be matched as well with CIS_LINK/BIS_LINK, or perhaps we
>>>> introduce a socket type to differentiate since the use of the address
>>>> can make the logic a little confusing when the socket types are mixed
>>>> together.
>>>>
>>>> Now looking at the source code perhaps we can have a separate list for
>>>> cis and bis sockets instead of global iso_sk_list (e.g. cis_sk_list
>>>> and bis_sk_list), that way we don't need a type and there is no risk
>>>> of confusing the sockets since they would never be in the same list.
>>>
>>> Alright, I will give it a try.
>>>
>>>>>           }
>>>>>
>>>>>    done:
>>>>>
>>>>> ---
>>>>> base-commit: 9c533991fe15be60ad9f9a7629c25dbc5b09788d
>>>>> change-id: 20250731-bis_cis_coexist-717a442d5c42
>>>>>
>>>>> Best regards,
>>>>> -- 
>>>>> Yang Li <yang.li@amlogic.com>
>>>>>
>>>>>
>>>> -- 
>>>> Luiz Augusto von Dentz
>>
>>
>> -- 
>> Luiz Augusto von Dentz

Re: [PATCH v3] Bluetooth: iso: fix socket matching ambiguity between BIS and CIS

[Luiz Augusto von Dentz]

Hi Yang,

On Thu, Oct 23, 2025 at 11:47 PM Yang Li <yang.li@amlogic.com> wrote:
>
> Hi Luiz,
> A gentle ping, thanks.
>
> > Hi Luiz,
> >> [ EXTERNAL EMAIL ]
> >>
> >> Hi Yang,
> >>
> >> On Sun, Aug 3, 2025 at 9:07 PM Yang Li <yang.li@amlogic.com> wrote:
> >>> Hi Luiz,
> >>>> [ EXTERNAL EMAIL ]
> >>>>
> >>>> Hi Yang,
> >>>>
> >>>> On Thu, Jul 31, 2025 at 4:00 AM Yang Li via B4 Relay
> >>>> <devnull+yang.li.amlogic.com@kernel.org> wrote:
> >>>>> From: Yang Li <yang.li@amlogic.com>
> >>>>>
> >>>>> When both BIS and CIS links exist, their sockets are in
> >>>>> the BT_LISTEN state.
> >>>> We probably need to introduce tests to iso-test that setup both then
> >>>> to avoid reintroducing the problem.
> >>>
> >>> Since the coexistence of BIS sink and CIS sink is determined by
> >>> application-level logic, it may be difficult to reproduce this scenario
> >>> using iso-test.
> >> Looks like you haven't look at what iso-tester tools tests do, that is
> >> not tight to bluetoothd, it directly operates at the socket layer so
> >> we can create any scenario we want.
> >
> >
> > Based on the current structure of iso-tester, it is not possible to
> > implement test cases where CIS and BIS listen simultaneously. There
> > are several issues:
> >
> > 1.
> >
> >    In struct iso_client_data, although both CIS and BIS related
> >    elements are defined, they are mutually exclusive. CIS and BIS
> >    cannot be used at the same time. For example, .bcast must explicitly
> >    specify whether it is broadcast or unicast.
> >
> > 2.
> >
> >    The setup_listen_many function also identifies BIS or CIS through
> >    .bcast.
> >
> > Therefore, if we want to add test cases for the coexistence of BIS and
> > CIS, the current data structure needs to be modified to completely
> > separate the elements for BIS and CIS.
> >
> >
>
> I'm not sure if my understanding is fully correct, so I would appreciate
> any guidance or suggestions.
>
> Based on my testing, this patch does fix the issue on my side.
> Please let me know if there is anything I may have missed or if further
> changes are needed.

I hope you are paying attention to the mailing list since I did add a
lot of new code that introduces support for PAST, including new test
cases for iso-tester, so I don't think asking for a test case for
having both BIS/CIS together is too much really. Works for me doesn't
really cut it since we do want to make sure we don't reintroduce the
problem later, but Im fine merging this now if it doesn't introduce
any problem existing tests in iso-tester.

>
> >>> Do you have any suggestions on how to simulate or test this case more
> >>> effectively?
> >>>
> >>>>> dump sock:
> >>>>>     sk 000000001977ef51 state 6
> >>>>>     src 10:a5:62:31:05:cf dst 00:00:00:00:00:00
> >>>>>     sk 0000000031d28700 state 7
> >>>>>     src 10:a5:62:31:05:cf dst00:00:00:00:00:00
> >>>>>     sk 00000000613af00e state 4   # listen sock of bis
> >>>>>     src 10:a5:62:31:05:cf dst 54:00:00:d4:99:30
> >>>>>     sk 000000001710468c state 9
> >>>>>     src 10:a5:62:31:05:cf dst 54:00:00:d4:99:30
> >>>>>     sk 000000005d97dfde state 4   #listen sock of cis
> >>>>>     src 10:a5:62:31:05:cf dst 00:00:00:00:00:00
> >>>>>
> >>>>> To locate the CIS socket correctly, check both the BT_LISTEN
> >>>>> state and whether dst addr is BDADDR_ANY.
> >>>>>
> >>>>> Link: https://github.com/bluez/bluez/issues/1224
> >>>>>
> >>>>> Signed-off-by: Yang Li <yang.li@amlogic.com>
> >>>>> ---
> >>>>>    net/bluetooth/iso.c | 9 +++++++--
> >>>>>    1 file changed, 7 insertions(+), 2 deletions(-)
> >>>>>
> >>>>> diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
> >>>>> index eaffd25570e3..9a4dea03af8c 100644
> >>>>> --- a/net/bluetooth/iso.c
> >>>>> +++ b/net/bluetooth/iso.c
> >>>>> @@ -1919,6 +1919,11 @@ static bool iso_match_pa_sync_flag(struct
> >>>>> sock *sk, void *data)
> >>>>>           return test_bit(BT_SK_PA_SYNC, &iso_pi(sk)->flags);
> >>>>>    }
> >>>>>
> >>>>> +static bool iso_match_dst(struct sock *sk, void *data)
> >>>>> +{
> >>>>> +       return !bacmp(&iso_pi(sk)->dst, (bdaddr_t *)data);
> >>>>> +}
> >>>>> +
> >>>>>    static void iso_conn_ready(struct iso_conn *conn)
> >>>>>    {
> >>>>>           struct sock *parent = NULL;
> >>>>> @@ -1981,7 +1986,7 @@ static void iso_conn_ready(struct iso_conn
> >>>>> *conn)
> >>>>>
> >>>>>                   if (!parent)
> >>>>>                           parent = iso_get_sock(&hcon->src,
> >>>>> BDADDR_ANY,
> >>>>> -                                             BT_LISTEN, NULL, NULL);
> >>>>> +                                             BT_LISTEN,
> >>>>> iso_match_dst, BDADDR_ANY);
> >>>>>
> >>>>>                   if (!parent)
> >>>>>                           return;
> >>>>> @@ -2220,7 +2225,7 @@ int iso_connect_ind(struct hci_dev *hdev,
> >>>>> bdaddr_t *bdaddr, __u8 *flags)
> >>>>>                   }
> >>>>>           } else {
> >>>>>                   sk = iso_get_sock(&hdev->bdaddr, BDADDR_ANY,
> >>>>> -                                 BT_LISTEN, NULL, NULL);
> >>>>> +                                 BT_LISTEN, iso_match_dst,
> >>>>> BDADDR_ANY);
> >>>> Perhaps we should add helper function that wrap the iso_get_sock (e.g.
> >>>> iso_get_sock_cis and iso_get_sock_bis) to make it clearer what is the
> >>>> expected socket type, also if the hcon has been set perhaps that
> >>>> should be matched as well with CIS_LINK/BIS_LINK, or perhaps we
> >>>> introduce a socket type to differentiate since the use of the address
> >>>> can make the logic a little confusing when the socket types are mixed
> >>>> together.
> >>>>
> >>>> Now looking at the source code perhaps we can have a separate list for
> >>>> cis and bis sockets instead of global iso_sk_list (e.g. cis_sk_list
> >>>> and bis_sk_list), that way we don't need a type and there is no risk
> >>>> of confusing the sockets since they would never be in the same list.
> >>>
> >>> Alright, I will give it a try.
> >>>
> >>>>>           }
> >>>>>
> >>>>>    done:
> >>>>>
> >>>>> ---
> >>>>> base-commit: 9c533991fe15be60ad9f9a7629c25dbc5b09788d
> >>>>> change-id: 20250731-bis_cis_coexist-717a442d5c42
> >>>>>
> >>>>> Best regards,
> >>>>> --
> >>>>> Yang Li <yang.li@amlogic.com>
> >>>>>
> >>>>>
> >>>> --
> >>>> Luiz Augusto von Dentz
> >>
> >>
> >> --
> >> Luiz Augusto von Dentz
>
>


-- 
Luiz Augusto von Dentz

Re: [PATCH v3] Bluetooth: iso: fix socket matching ambiguity between BIS and CIS

[Luiz Augusto von Dentz]

Hi Yang,

On Fri, Oct 24, 2025 at 9:45 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Yang,
>
> On Thu, Oct 23, 2025 at 11:47 PM Yang Li <yang.li@amlogic.com> wrote:
> >
> > Hi Luiz,
> > A gentle ping, thanks.
> >
> > > Hi Luiz,
> > >> [ EXTERNAL EMAIL ]
> > >>
> > >> Hi Yang,
> > >>
> > >> On Sun, Aug 3, 2025 at 9:07 PM Yang Li <yang.li@amlogic.com> wrote:
> > >>> Hi Luiz,
> > >>>> [ EXTERNAL EMAIL ]
> > >>>>
> > >>>> Hi Yang,
> > >>>>
> > >>>> On Thu, Jul 31, 2025 at 4:00 AM Yang Li via B4 Relay
> > >>>> <devnull+yang.li.amlogic.com@kernel.org> wrote:
> > >>>>> From: Yang Li <yang.li@amlogic.com>
> > >>>>>
> > >>>>> When both BIS and CIS links exist, their sockets are in
> > >>>>> the BT_LISTEN state.
> > >>>> We probably need to introduce tests to iso-test that setup both then
> > >>>> to avoid reintroducing the problem.
> > >>>
> > >>> Since the coexistence of BIS sink and CIS sink is determined by
> > >>> application-level logic, it may be difficult to reproduce this scenario
> > >>> using iso-test.
> > >> Looks like you haven't look at what iso-tester tools tests do, that is
> > >> not tight to bluetoothd, it directly operates at the socket layer so
> > >> we can create any scenario we want.
> > >
> > >
> > > Based on the current structure of iso-tester, it is not possible to
> > > implement test cases where CIS and BIS listen simultaneously. There
> > > are several issues:
> > >
> > > 1.
> > >
> > >    In struct iso_client_data, although both CIS and BIS related
> > >    elements are defined, they are mutually exclusive. CIS and BIS
> > >    cannot be used at the same time. For example, .bcast must explicitly
> > >    specify whether it is broadcast or unicast.
> > >
> > > 2.
> > >
> > >    The setup_listen_many function also identifies BIS or CIS through
> > >    .bcast.
> > >
> > > Therefore, if we want to add test cases for the coexistence of BIS and
> > > CIS, the current data structure needs to be modified to completely
> > > separate the elements for BIS and CIS.
> > >
> > >
> >
> > I'm not sure if my understanding is fully correct, so I would appreciate
> > any guidance or suggestions.
> >
> > Based on my testing, this patch does fix the issue on my side.
> > Please let me know if there is anything I may have missed or if further
> > changes are needed.
>
> I hope you are paying attention to the mailing list since I did add a
> lot of new code that introduces support for PAST, including new test
> cases for iso-tester, so I don't think asking for a test case for
> having both BIS/CIS together is too much really. Works for me doesn't
> really cut it since we do want to make sure we don't reintroduce the
> problem later, but Im fine merging this now if it doesn't introduce
> any problem existing tests in iso-tester.

You will need to resend it since it is no longer available in patchwork.

> >
> > >>> Do you have any suggestions on how to simulate or test this case more
> > >>> effectively?
> > >>>
> > >>>>> dump sock:
> > >>>>>     sk 000000001977ef51 state 6
> > >>>>>     src 10:a5:62:31:05:cf dst 00:00:00:00:00:00
> > >>>>>     sk 0000000031d28700 state 7
> > >>>>>     src 10:a5:62:31:05:cf dst00:00:00:00:00:00
> > >>>>>     sk 00000000613af00e state 4   # listen sock of bis
> > >>>>>     src 10:a5:62:31:05:cf dst 54:00:00:d4:99:30
> > >>>>>     sk 000000001710468c state 9
> > >>>>>     src 10:a5:62:31:05:cf dst 54:00:00:d4:99:30
> > >>>>>     sk 000000005d97dfde state 4   #listen sock of cis
> > >>>>>     src 10:a5:62:31:05:cf dst 00:00:00:00:00:00
> > >>>>>
> > >>>>> To locate the CIS socket correctly, check both the BT_LISTEN
> > >>>>> state and whether dst addr is BDADDR_ANY.
> > >>>>>
> > >>>>> Link: https://github.com/bluez/bluez/issues/1224
> > >>>>>
> > >>>>> Signed-off-by: Yang Li <yang.li@amlogic.com>
> > >>>>> ---
> > >>>>>    net/bluetooth/iso.c | 9 +++++++--
> > >>>>>    1 file changed, 7 insertions(+), 2 deletions(-)
> > >>>>>
> > >>>>> diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
> > >>>>> index eaffd25570e3..9a4dea03af8c 100644
> > >>>>> --- a/net/bluetooth/iso.c
> > >>>>> +++ b/net/bluetooth/iso.c
> > >>>>> @@ -1919,6 +1919,11 @@ static bool iso_match_pa_sync_flag(struct
> > >>>>> sock *sk, void *data)
> > >>>>>           return test_bit(BT_SK_PA_SYNC, &iso_pi(sk)->flags);
> > >>>>>    }
> > >>>>>
> > >>>>> +static bool iso_match_dst(struct sock *sk, void *data)
> > >>>>> +{
> > >>>>> +       return !bacmp(&iso_pi(sk)->dst, (bdaddr_t *)data);
> > >>>>> +}
> > >>>>> +
> > >>>>>    static void iso_conn_ready(struct iso_conn *conn)
> > >>>>>    {
> > >>>>>           struct sock *parent = NULL;
> > >>>>> @@ -1981,7 +1986,7 @@ static void iso_conn_ready(struct iso_conn
> > >>>>> *conn)
> > >>>>>
> > >>>>>                   if (!parent)
> > >>>>>                           parent = iso_get_sock(&hcon->src,
> > >>>>> BDADDR_ANY,
> > >>>>> -                                             BT_LISTEN, NULL, NULL);
> > >>>>> +                                             BT_LISTEN,
> > >>>>> iso_match_dst, BDADDR_ANY);
> > >>>>>
> > >>>>>                   if (!parent)
> > >>>>>                           return;
> > >>>>> @@ -2220,7 +2225,7 @@ int iso_connect_ind(struct hci_dev *hdev,
> > >>>>> bdaddr_t *bdaddr, __u8 *flags)
> > >>>>>                   }
> > >>>>>           } else {
> > >>>>>                   sk = iso_get_sock(&hdev->bdaddr, BDADDR_ANY,
> > >>>>> -                                 BT_LISTEN, NULL, NULL);
> > >>>>> +                                 BT_LISTEN, iso_match_dst,
> > >>>>> BDADDR_ANY);
> > >>>> Perhaps we should add helper function that wrap the iso_get_sock (e.g.
> > >>>> iso_get_sock_cis and iso_get_sock_bis) to make it clearer what is the
> > >>>> expected socket type, also if the hcon has been set perhaps that
> > >>>> should be matched as well with CIS_LINK/BIS_LINK, or perhaps we
> > >>>> introduce a socket type to differentiate since the use of the address
> > >>>> can make the logic a little confusing when the socket types are mixed
> > >>>> together.
> > >>>>
> > >>>> Now looking at the source code perhaps we can have a separate list for
> > >>>> cis and bis sockets instead of global iso_sk_list (e.g. cis_sk_list
> > >>>> and bis_sk_list), that way we don't need a type and there is no risk
> > >>>> of confusing the sockets since they would never be in the same list.
> > >>>
> > >>> Alright, I will give it a try.
> > >>>
> > >>>>>           }
> > >>>>>
> > >>>>>    done:
> > >>>>>
> > >>>>> ---
> > >>>>> base-commit: 9c533991fe15be60ad9f9a7629c25dbc5b09788d
> > >>>>> change-id: 20250731-bis_cis_coexist-717a442d5c42
> > >>>>>
> > >>>>> Best regards,
> > >>>>> --
> > >>>>> Yang Li <yang.li@amlogic.com>
> > >>>>>
> > >>>>>
> > >>>> --
> > >>>> Luiz Augusto von Dentz
> > >>
> > >>
> > >> --
> > >> Luiz Augusto von Dentz
> >
> >
>
>
> --
> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

Re: [PATCH v3] Bluetooth: iso: fix socket matching ambiguity between BIS and CIS

[Yang Li]

Hi Luiz,


> [ EXTERNAL EMAIL ]
>
> Hi Yang,
>
> On Fri, Oct 24, 2025 at 9:45 AM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
>> Hi Yang,
>>
>> On Thu, Oct 23, 2025 at 11:47 PM Yang Li <yang.li@amlogic.com> wrote:
>>> Hi Luiz,
>>> A gentle ping, thanks.
>>>
>>>> Hi Luiz,
>>>>> [ EXTERNAL EMAIL ]
>>>>>
>>>>> Hi Yang,
>>>>>
>>>>> On Sun, Aug 3, 2025 at 9:07 PM Yang Li <yang.li@amlogic.com> wrote:
>>>>>> Hi Luiz,
>>>>>>> [ EXTERNAL EMAIL ]
>>>>>>>
>>>>>>> Hi Yang,
>>>>>>>
>>>>>>> On Thu, Jul 31, 2025 at 4:00 AM Yang Li via B4 Relay
>>>>>>> <devnull+yang.li.amlogic.com@kernel.org> wrote:
>>>>>>>> From: Yang Li <yang.li@amlogic.com>
>>>>>>>>
>>>>>>>> When both BIS and CIS links exist, their sockets are in
>>>>>>>> the BT_LISTEN state.
>>>>>>> We probably need to introduce tests to iso-test that setup both then
>>>>>>> to avoid reintroducing the problem.
>>>>>> Since the coexistence of BIS sink and CIS sink is determined by
>>>>>> application-level logic, it may be difficult to reproduce this scenario
>>>>>> using iso-test.
>>>>> Looks like you haven't look at what iso-tester tools tests do, that is
>>>>> not tight to bluetoothd, it directly operates at the socket layer so
>>>>> we can create any scenario we want.
>>>>
>>>> Based on the current structure of iso-tester, it is not possible to
>>>> implement test cases where CIS and BIS listen simultaneously. There
>>>> are several issues:
>>>>
>>>> 1.
>>>>
>>>>     In struct iso_client_data, although both CIS and BIS related
>>>>     elements are defined, they are mutually exclusive. CIS and BIS
>>>>     cannot be used at the same time. For example, .bcast must explicitly
>>>>     specify whether it is broadcast or unicast.
>>>>
>>>> 2.
>>>>
>>>>     The setup_listen_many function also identifies BIS or CIS through
>>>>     .bcast.
>>>>
>>>> Therefore, if we want to add test cases for the coexistence of BIS and
>>>> CIS, the current data structure needs to be modified to completely
>>>> separate the elements for BIS and CIS.
>>>>
>>>>
>>> I'm not sure if my understanding is fully correct, so I would appreciate
>>> any guidance or suggestions.
>>>
>>> Based on my testing, this patch does fix the issue on my side.
>>> Please let me know if there is anything I may have missed or if further
>>> changes are needed.
>> I hope you are paying attention to the mailing list since I did add a
>> lot of new code that introduces support for PAST, including new test
>> cases for iso-tester, so I don't think asking for a test case for
>> having both BIS/CIS together is too much really. Works for me doesn't
>> really cut it since we do want to make sure we don't reintroduce the
>> problem later, but Im fine merging this now if it doesn't introduce
>> any problem existing tests in iso-tester.
> You will need to resend it since it is no longer available in patchwork.


I completely understand the necessity of adding a test case for the 
coexistence of BIS and CIS. However, the current issue is that the data 
structure of iso-tester doesn't support listening to both BIS and CIS at 
the same time. I will keep an eye on the updates of iso-tester and add 
the test case at the appropriate time.

I will update this patch.

Thanks

>
>>>>>> Do you have any suggestions on how to simulate or test this case more
>>>>>> effectively?
>>>>>>
>>>>>>>> dump sock:
>>>>>>>>      sk 000000001977ef51 state 6
>>>>>>>>      src 10:a5:62:31:05:cf dst 00:00:00:00:00:00
>>>>>>>>      sk 0000000031d28700 state 7
>>>>>>>>      src 10:a5:62:31:05:cf dst00:00:00:00:00:00
>>>>>>>>      sk 00000000613af00e state 4   # listen sock of bis
>>>>>>>>      src 10:a5:62:31:05:cf dst 54:00:00:d4:99:30
>>>>>>>>      sk 000000001710468c state 9
>>>>>>>>      src 10:a5:62:31:05:cf dst 54:00:00:d4:99:30
>>>>>>>>      sk 000000005d97dfde state 4   #listen sock of cis
>>>>>>>>      src 10:a5:62:31:05:cf dst 00:00:00:00:00:00
>>>>>>>>
>>>>>>>> To locate the CIS socket correctly, check both the BT_LISTEN
>>>>>>>> state and whether dst addr is BDADDR_ANY.
>>>>>>>>
>>>>>>>> Link: https://github.com/bluez/bluez/issues/1224
>>>>>>>>
>>>>>>>> Signed-off-by: Yang Li <yang.li@amlogic.com>
>>>>>>>> ---
>>>>>>>>     net/bluetooth/iso.c | 9 +++++++--
>>>>>>>>     1 file changed, 7 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
>>>>>>>> index eaffd25570e3..9a4dea03af8c 100644
>>>>>>>> --- a/net/bluetooth/iso.c
>>>>>>>> +++ b/net/bluetooth/iso.c
>>>>>>>> @@ -1919,6 +1919,11 @@ static bool iso_match_pa_sync_flag(struct
>>>>>>>> sock *sk, void *data)
>>>>>>>>            return test_bit(BT_SK_PA_SYNC, &iso_pi(sk)->flags);
>>>>>>>>     }
>>>>>>>>
>>>>>>>> +static bool iso_match_dst(struct sock *sk, void *data)
>>>>>>>> +{
>>>>>>>> +       return !bacmp(&iso_pi(sk)->dst, (bdaddr_t *)data);
>>>>>>>> +}
>>>>>>>> +
>>>>>>>>     static void iso_conn_ready(struct iso_conn *conn)
>>>>>>>>     {
>>>>>>>>            struct sock *parent = NULL;
>>>>>>>> @@ -1981,7 +1986,7 @@ static void iso_conn_ready(struct iso_conn
>>>>>>>> *conn)
>>>>>>>>
>>>>>>>>                    if (!parent)
>>>>>>>>                            parent = iso_get_sock(&hcon->src,
>>>>>>>> BDADDR_ANY,
>>>>>>>> -                                             BT_LISTEN, NULL, NULL);
>>>>>>>> +                                             BT_LISTEN,
>>>>>>>> iso_match_dst, BDADDR_ANY);
>>>>>>>>
>>>>>>>>                    if (!parent)
>>>>>>>>                            return;
>>>>>>>> @@ -2220,7 +2225,7 @@ int iso_connect_ind(struct hci_dev *hdev,
>>>>>>>> bdaddr_t *bdaddr, __u8 *flags)
>>>>>>>>                    }
>>>>>>>>            } else {
>>>>>>>>                    sk = iso_get_sock(&hdev->bdaddr, BDADDR_ANY,
>>>>>>>> -                                 BT_LISTEN, NULL, NULL);
>>>>>>>> +                                 BT_LISTEN, iso_match_dst,
>>>>>>>> BDADDR_ANY);
>>>>>>> Perhaps we should add helper function that wrap the iso_get_sock (e.g.
>>>>>>> iso_get_sock_cis and iso_get_sock_bis) to make it clearer what is the
>>>>>>> expected socket type, also if the hcon has been set perhaps that
>>>>>>> should be matched as well with CIS_LINK/BIS_LINK, or perhaps we
>>>>>>> introduce a socket type to differentiate since the use of the address
>>>>>>> can make the logic a little confusing when the socket types are mixed
>>>>>>> together.
>>>>>>>
>>>>>>> Now looking at the source code perhaps we can have a separate list for
>>>>>>> cis and bis sockets instead of global iso_sk_list (e.g. cis_sk_list
>>>>>>> and bis_sk_list), that way we don't need a type and there is no risk
>>>>>>> of confusing the sockets since they would never be in the same list.
>>>>>> Alright, I will give it a try.
>>>>>>
>>>>>>>>            }
>>>>>>>>
>>>>>>>>     done:
>>>>>>>>
>>>>>>>> ---
>>>>>>>> base-commit: 9c533991fe15be60ad9f9a7629c25dbc5b09788d
>>>>>>>> change-id: 20250731-bis_cis_coexist-717a442d5c42
>>>>>>>>
>>>>>>>> Best regards,
>>>>>>>> --
>>>>>>>> Yang Li <yang.li@amlogic.com>
>>>>>>>>
>>>>>>>>
>>>>>>> --
>>>>>>> Luiz Augusto von Dentz
>>>>>
>>>>> --
>>>>> Luiz Augusto von Dentz
>>>
>>
>> --
>> Luiz Augusto von Dentz
>
>
> --
> Luiz Augusto von Dentz