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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Luiz,
[ EXTERNAL EMAIL ]

Hi Yang,

On Sun, Aug 3, 2025 at 9:07 PM Yang Li <yang.li@xxxxxxxxxxx> 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@xxxxxxxxxx> wrote:
From: Yang Li <yang.li@xxxxxxxxxxx>

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@xxxxxxxxxxx>
---
   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@xxxxxxxxxxx>


--
Luiz Augusto von Dentz


--
Luiz Augusto von Dentz




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux