Bluetooth: hci_conn: Fix not setting conn_timeout for Broadcast Receiver
authorLuiz Augusto von Dentz <luiz.von.dentz@intel.com>
Wed, 9 Apr 2025 20:08:48 +0000 (16:08 -0400)
committerLuiz Augusto von Dentz <luiz.von.dentz@intel.com>
Fri, 25 Apr 2025 19:03:19 +0000 (15:03 -0400)
Broadcast Receiver requires creating PA sync but the command just
generates a status so this makes use of __hci_cmd_sync_status_sk to wait
for HCI_EV_LE_PA_SYNC_ESTABLISHED, also because of this chance it is not
longer necessary to use a custom method to serialize the process of
creating the PA sync since the cmd_work_sync itself ensures only one
command would be pending which now awaits for
HCI_EV_LE_PA_SYNC_ESTABLISHED before proceeding to next connection.

Fixes: 4a5e0ba68676 ("Bluetooth: ISO: Do not emit LE PA Create Sync if previous is pending")
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
include/net/bluetooth/hci.h
include/net/bluetooth/hci_core.h
include/net/bluetooth/hci_sync.h
net/bluetooth/hci_conn.c
net/bluetooth/hci_event.c
net/bluetooth/hci_sync.c

index a8586c3058c7cd5eb3ff2472f07adfe8074a6e74..8ea7a063cc651056114325577941f97b723c4af0 100644 (file)
@@ -1931,6 +1931,8 @@ struct hci_cp_le_pa_create_sync {
        __u8      sync_cte_type;
 } __packed;
 
+#define HCI_OP_LE_PA_CREATE_SYNC_CANCEL        0x2045
+
 #define HCI_OP_LE_PA_TERM_SYNC         0x2046
 struct hci_cp_le_pa_term_sync {
        __le16    handle;
index 5115da34f881280de2b18ac37413840ce73ad138..f20368b9a5d20036fd05c076bbbbd8e8ff3c893d 100644 (file)
@@ -1113,10 +1113,8 @@ static inline struct hci_conn *hci_conn_hash_lookup_bis(struct hci_dev *hdev,
        return NULL;
 }
 
-static inline struct hci_conn *hci_conn_hash_lookup_sid(struct hci_dev *hdev,
-                                                       __u8 sid,
-                                                       bdaddr_t *dst,
-                                                       __u8 dst_type)
+static inline struct hci_conn *
+hci_conn_hash_lookup_create_pa_sync(struct hci_dev *hdev)
 {
        struct hci_conn_hash *h = &hdev->conn_hash;
        struct hci_conn  *c;
@@ -1124,8 +1122,10 @@ static inline struct hci_conn *hci_conn_hash_lookup_sid(struct hci_dev *hdev,
        rcu_read_lock();
 
        list_for_each_entry_rcu(c, &h->list, list) {
-               if (c->type != ISO_LINK  || bacmp(&c->dst, dst) ||
-                   c->dst_type != dst_type || c->sid != sid)
+               if (c->type != ISO_LINK)
+                       continue;
+
+               if (!test_bit(HCI_CONN_CREATE_PA_SYNC, &c->flags))
                        continue;
 
                rcu_read_unlock();
@@ -1524,7 +1524,6 @@ bool hci_setup_sync(struct hci_conn *conn, __u16 handle);
 void hci_sco_setup(struct hci_conn *conn, __u8 status);
 bool hci_iso_setup_path(struct hci_conn *conn);
 int hci_le_create_cis_pending(struct hci_dev *hdev);
-int hci_pa_create_sync_pending(struct hci_dev *hdev);
 int hci_le_big_create_sync_pending(struct hci_dev *hdev);
 int hci_conn_check_create_cis(struct hci_conn *conn);
 
index 7e2cf0cca939a1ff5234458941d55ec50ea9c089..93dac4c7f9e3efd630fa60eae017ee0dc2f5237b 100644 (file)
@@ -185,3 +185,5 @@ int hci_connect_le_sync(struct hci_dev *hdev, struct hci_conn *conn);
 int hci_cancel_connect_sync(struct hci_dev *hdev, struct hci_conn *conn);
 int hci_le_conn_update_sync(struct hci_dev *hdev, struct hci_conn *conn,
                            struct hci_conn_params *params);
+
+int hci_connect_pa_sync(struct hci_dev *hdev, struct hci_conn *conn);
index 7e1b53857648df5495a09c4877cfe601251a8719..c3112ce39f6724a76df8e547eeac12adad59c999 100644 (file)
@@ -2064,95 +2064,6 @@ static int create_big_sync(struct hci_dev *hdev, void *data)
        return hci_le_create_big(conn, &conn->iso_qos);
 }
 
-static void create_pa_complete(struct hci_dev *hdev, void *data, int err)
-{
-       bt_dev_dbg(hdev, "");
-
-       if (err)
-               bt_dev_err(hdev, "Unable to create PA: %d", err);
-}
-
-static bool hci_conn_check_create_pa_sync(struct hci_conn *conn)
-{
-       if (conn->type != ISO_LINK || conn->sid == HCI_SID_INVALID)
-               return false;
-
-       return true;
-}
-
-static int create_pa_sync(struct hci_dev *hdev, void *data)
-{
-       struct hci_cp_le_pa_create_sync cp = {0};
-       struct hci_conn *conn;
-       int err = 0;
-
-       hci_dev_lock(hdev);
-
-       rcu_read_lock();
-
-       /* The spec allows only one pending LE Periodic Advertising Create
-        * Sync command at a time. If the command is pending now, don't do
-        * anything. We check for pending connections after each PA Sync
-        * Established event.
-        *
-        * BLUETOOTH CORE SPECIFICATION Version 5.3 | Vol 4, Part E
-        * page 2493:
-        *
-        * If the Host issues this command when another HCI_LE_Periodic_
-        * Advertising_Create_Sync command is pending, the Controller shall
-        * return the error code Command Disallowed (0x0C).
-        */
-       list_for_each_entry_rcu(conn, &hdev->conn_hash.list, list) {
-               if (test_bit(HCI_CONN_CREATE_PA_SYNC, &conn->flags))
-                       goto unlock;
-       }
-
-       list_for_each_entry_rcu(conn, &hdev->conn_hash.list, list) {
-               if (hci_conn_check_create_pa_sync(conn)) {
-                       struct bt_iso_qos *qos = &conn->iso_qos;
-
-                       cp.options = qos->bcast.options;
-                       cp.sid = conn->sid;
-                       cp.addr_type = conn->dst_type;
-                       bacpy(&cp.addr, &conn->dst);
-                       cp.skip = cpu_to_le16(qos->bcast.skip);
-                       cp.sync_timeout = cpu_to_le16(qos->bcast.sync_timeout);
-                       cp.sync_cte_type = qos->bcast.sync_cte_type;
-
-                       break;
-               }
-       }
-
-unlock:
-       rcu_read_unlock();
-
-       hci_dev_unlock(hdev);
-
-       if (bacmp(&cp.addr, BDADDR_ANY)) {
-               hci_dev_set_flag(hdev, HCI_PA_SYNC);
-               set_bit(HCI_CONN_CREATE_PA_SYNC, &conn->flags);
-
-               err = __hci_cmd_sync_status(hdev, HCI_OP_LE_PA_CREATE_SYNC,
-                                           sizeof(cp), &cp, HCI_CMD_TIMEOUT);
-               if (!err)
-                       err = hci_update_passive_scan_sync(hdev);
-
-               if (err) {
-                       hci_dev_clear_flag(hdev, HCI_PA_SYNC);
-                       clear_bit(HCI_CONN_CREATE_PA_SYNC, &conn->flags);
-               }
-       }
-
-       return err;
-}
-
-int hci_pa_create_sync_pending(struct hci_dev *hdev)
-{
-       /* Queue start pa_create_sync and scan */
-       return hci_cmd_sync_queue(hdev, create_pa_sync,
-                                 NULL, create_pa_complete);
-}
-
 struct hci_conn *hci_pa_create_sync(struct hci_dev *hdev, bdaddr_t *dst,
                                    __u8 dst_type, __u8 sid,
                                    struct bt_iso_qos *qos)
@@ -2167,10 +2078,11 @@ struct hci_conn *hci_pa_create_sync(struct hci_dev *hdev, bdaddr_t *dst,
        conn->dst_type = dst_type;
        conn->sid = sid;
        conn->state = BT_LISTEN;
+       conn->conn_timeout = msecs_to_jiffies(qos->bcast.sync_timeout * 10);
 
        hci_conn_hold(conn);
 
-       hci_pa_create_sync_pending(hdev);
+       hci_connect_pa_sync(hdev, conn);
 
        return conn;
 }
index 5f808f0b0e9a2a18e982fce288ae258fbe53f478..ea7ccafd055a76b95773d17efab4b66b1dc0e195 100644 (file)
@@ -6378,8 +6378,7 @@ static void hci_le_pa_sync_estabilished_evt(struct hci_dev *hdev, void *data,
 
        hci_dev_clear_flag(hdev, HCI_PA_SYNC);
 
-       conn = hci_conn_hash_lookup_sid(hdev, ev->sid, &ev->bdaddr,
-                                       ev->bdaddr_type);
+       conn = hci_conn_hash_lookup_create_pa_sync(hdev);
        if (!conn) {
                bt_dev_err(hdev,
                           "Unable to find connection for dst %pMR sid 0x%2.2x",
@@ -6418,9 +6417,6 @@ static void hci_le_pa_sync_estabilished_evt(struct hci_dev *hdev, void *data,
        }
 
 unlock:
-       /* Handle any other pending PA sync command */
-       hci_pa_create_sync_pending(hdev);
-
        hci_dev_unlock(hdev);
 }
 
index 609b035e5c9041f032af6d33c055ecad70080030..99c116b056ceebbec06a494da834a8a5c525861f 100644 (file)
@@ -2693,16 +2693,16 @@ static u8 hci_update_accept_list_sync(struct hci_dev *hdev)
 
        /* Force address filtering if PA Sync is in progress */
        if (hci_dev_test_flag(hdev, HCI_PA_SYNC)) {
-               struct hci_cp_le_pa_create_sync *sent;
+               struct hci_conn *conn;
 
-               sent = hci_sent_cmd_data(hdev, HCI_OP_LE_PA_CREATE_SYNC);
-               if (sent) {
+               conn = hci_conn_hash_lookup_create_pa_sync(hdev);
+               if (conn) {
                        struct conn_params pa;
 
                        memset(&pa, 0, sizeof(pa));
 
-                       bacpy(&pa.addr, &sent->addr);
-                       pa.addr_type = sent->addr_type;
+                       bacpy(&pa.addr, &conn->dst);
+                       pa.addr_type = conn->dst_type;
 
                        /* Clear first since there could be addresses left
                         * behind.
@@ -6895,3 +6895,80 @@ int hci_le_conn_update_sync(struct hci_dev *hdev, struct hci_conn *conn,
        return __hci_cmd_sync_status(hdev, HCI_OP_LE_CONN_UPDATE,
                                     sizeof(cp), &cp, HCI_CMD_TIMEOUT);
 }
+
+static void create_pa_complete(struct hci_dev *hdev, void *data, int err)
+{
+       bt_dev_dbg(hdev, "err %d", err);
+
+       if (!err)
+               return;
+
+       hci_dev_clear_flag(hdev, HCI_PA_SYNC);
+
+       if (err == -ECANCELED)
+               return;
+
+       hci_dev_lock(hdev);
+
+       hci_update_passive_scan_sync(hdev);
+
+       hci_dev_unlock(hdev);
+}
+
+static int hci_le_pa_create_sync(struct hci_dev *hdev, void *data)
+{
+       struct hci_cp_le_pa_create_sync cp;
+       struct hci_conn *conn = data;
+       struct bt_iso_qos *qos = &conn->iso_qos;
+       int err;
+
+       if (!hci_conn_valid(hdev, conn))
+               return -ECANCELED;
+
+       if (hci_dev_test_and_set_flag(hdev, HCI_PA_SYNC))
+               return -EBUSY;
+
+       /* Mark HCI_CONN_CREATE_PA_SYNC so hci_update_passive_scan_sync can
+        * program the address in the allow list so PA advertisements can be
+        * received.
+        */
+       set_bit(HCI_CONN_CREATE_PA_SYNC, &conn->flags);
+
+       hci_update_passive_scan_sync(hdev);
+
+       memset(&cp, 0, sizeof(cp));
+       cp.options = qos->bcast.options;
+       cp.sid = conn->sid;
+       cp.addr_type = conn->dst_type;
+       bacpy(&cp.addr, &conn->dst);
+       cp.skip = cpu_to_le16(qos->bcast.skip);
+       cp.sync_timeout = cpu_to_le16(qos->bcast.sync_timeout);
+       cp.sync_cte_type = qos->bcast.sync_cte_type;
+
+       /* The spec allows only one pending LE Periodic Advertising Create
+        * Sync command at a time so we forcefully wait for PA Sync Established
+        * event since cmd_work can only schedule one command at a time.
+        *
+        * BLUETOOTH CORE SPECIFICATION Version 5.3 | Vol 4, Part E
+        * page 2493:
+        *
+        * If the Host issues this command when another HCI_LE_Periodic_
+        * Advertising_Create_Sync command is pending, the Controller shall
+        * return the error code Command Disallowed (0x0C).
+        */
+       err = __hci_cmd_sync_status_sk(hdev, HCI_OP_LE_PA_CREATE_SYNC,
+                                      sizeof(cp), &cp,
+                                      HCI_EV_LE_PA_SYNC_ESTABLISHED,
+                                      conn->conn_timeout, NULL);
+       if (err == -ETIMEDOUT)
+               __hci_cmd_sync_status(hdev, HCI_OP_LE_PA_CREATE_SYNC_CANCEL,
+                                     0, NULL, HCI_CMD_TIMEOUT);
+
+       return err;
+}
+
+int hci_connect_pa_sync(struct hci_dev *hdev, struct hci_conn *conn)
+{
+       return hci_cmd_sync_queue_once(hdev, hci_le_pa_create_sync, conn,
+                                      create_pa_complete);
+}