[PATCH v3 0/1] Bluetooth: ISO: Use defer setup to separate PA sync and BIG sync

[PATCH v3 0/1] Bluetooth: ISO: Use defer setup to separate PA sync and BIG sync

[Iulia Tanasescu]

Some scenarios require a Broadcast Sink to complete the PA sync and
BIG sync procedures separately.

For example, a Broadcast Sink might synchronize to the periodic
advertising transmitted by a Broadcast Source and it might detect
that the BIG is encrypted, by looking at the BIGInfo advertising
reports. If the Broadcast Sink doesn't know which Broadcast Code
to pass as parameter to the BIG Create Sync command, it might learn
it from a BASS Client through the Set Broadcast Code operation.
Only then will it be able to successfully sync to the BIG.

This patch implements defer setup support for the Broadcast Sink
scenario: By setting defer setup on a broadcast socket before calling
listen, the user is able to trigger the PA sync and BIG sync procedures
separately.

This patch contains a rebase against latest commits.

Iulia Tanasescu (1):
  Bluetooth: ISO: Use defer setup to separate PA sync and BIG sync

 include/net/bluetooth/hci_core.h |  30 +++++-
 net/bluetooth/hci_conn.c         |  13 ++-
 net/bluetooth/hci_event.c        |  41 +++++++-
 net/bluetooth/hci_sync.c         |  15 +++
 net/bluetooth/iso.c              | 160 ++++++++++++++++++++++++-------
 5 files changed, 218 insertions(+), 41 deletions(-)


base-commit: ef6d9b23aa589073cff4371135d017724728ae9f
-- 
2.34.1

[PATCH v3 1/1] Bluetooth: ISO: Use defer setup to separate PA sync and BIG sync

[Iulia Tanasescu]

This commit implements defer setup support for the Broadcast Sink
scenario: By setting defer setup on a broadcast socket before calling
listen, the user is able to trigger the PA sync and BIG sync procedures
separately.

This is useful if the user first wants to synchronize to the periodic
advertising transmitted by a Broadcast Source, and trigger the BIG sync
procedure later on.

If defer setup is set, once a PA sync established event arrives, a new
hcon is created and notified to the ISO layer. A child socket associated
with the PA sync connection will be added to the accept queue of the
listening socket.

Once the accept call returns the fd for the PA sync child socket, the
user should call read on that fd. This will trigger the BIG create sync
procedure, and the PA sync socket will become a listening socket itself.

When the BIG sync established event is notified to the ISO layer, the
bis connections will be added to the accept queue of the PA sync parent.
The user should call accept on the PA sync socket to get the final bis
connections.

Signed-off-by: Iulia Tanasescu <iulia.tanasescu@nxp.com>
---
 include/net/bluetooth/hci_core.h |  30 +++++-
 net/bluetooth/hci_conn.c         |  13 ++-
 net/bluetooth/hci_event.c        |  41 +++++++-
 net/bluetooth/hci_sync.c         |  15 +++
 net/bluetooth/iso.c              | 160 ++++++++++++++++++++++++-------
 5 files changed, 218 insertions(+), 41 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index c53d74236e3a..6fb055e3c595 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -978,6 +978,8 @@ enum {
 	HCI_CONN_CREATE_CIS,
 	HCI_CONN_BIG_SYNC,
 	HCI_CONN_BIG_SYNC_FAILED,
+	HCI_CONN_PA_SYNC,
+	HCI_CONN_PA_SYNC_FAILED,
 };
 
 static inline bool hci_conn_ssp_enabled(struct hci_conn *conn)
@@ -1300,7 +1302,7 @@ static inline struct hci_conn *hci_conn_hash_lookup_big_any_dst(struct hci_dev *
 		if (c->type != ISO_LINK)
 			continue;
 
-		if (handle == c->iso_qos.bcast.big) {
+		if (handle != BT_ISO_QOS_BIG_UNSET && handle == c->iso_qos.bcast.big) {
 			rcu_read_unlock();
 			return c;
 		}
@@ -1311,6 +1313,29 @@ static inline struct hci_conn *hci_conn_hash_lookup_big_any_dst(struct hci_dev *
 	return NULL;
 }
 
+static inline struct hci_conn *
+hci_conn_hash_lookup_pa_sync(struct hci_dev *hdev, __u8 big)
+{
+	struct hci_conn_hash *h = &hdev->conn_hash;
+	struct hci_conn  *c;
+
+	rcu_read_lock();
+
+	list_for_each_entry_rcu(c, &h->list, list) {
+		if (c->type != ISO_LINK ||
+			!test_bit(HCI_CONN_PA_SYNC, &c->flags))
+			continue;
+
+		if (c->iso_qos.bcast.big == big) {
+			rcu_read_unlock();
+			return c;
+		}
+	}
+	rcu_read_unlock();
+
+	return NULL;
+}
+
 static inline struct hci_conn *hci_conn_hash_lookup_state(struct hci_dev *hdev,
 							__u8 type, __u16 state)
 {
@@ -1435,7 +1460,8 @@ struct hci_conn *hci_connect_bis(struct hci_dev *hdev, bdaddr_t *dst,
 				 __u8 data_len, __u8 *data);
 int hci_pa_create_sync(struct hci_dev *hdev, bdaddr_t *dst, __u8 dst_type,
 		       __u8 sid, struct bt_iso_qos *qos);
-int hci_le_big_create_sync(struct hci_dev *hdev, struct bt_iso_qos *qos,
+int hci_le_big_create_sync(struct hci_dev *hdev, struct hci_conn *hcon,
+			   struct bt_iso_qos *qos,
 			   __u16 sync_handle, __u8 num_bis, __u8 bis[]);
 int hci_conn_check_link_mode(struct hci_conn *conn);
 int hci_conn_check_secure(struct hci_conn *conn, __u8 sec_level);
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 95339623883c..8b0c8e631324 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -734,6 +734,7 @@ struct iso_list_data {
 	};
 	int count;
 	bool big_term;
+	bool pa_sync_term;
 	bool big_sync_term;
 };
 
@@ -807,7 +808,10 @@ static int big_terminate_sync(struct hci_dev *hdev, void *data)
 	if (d->big_sync_term)
 		hci_le_big_terminate_sync(hdev, d->big);
 
-	return hci_le_pa_terminate_sync(hdev, d->sync_handle);
+	if (d->pa_sync_term)
+		return hci_le_pa_terminate_sync(hdev, d->sync_handle);
+
+	return 0;
 }
 
 static int hci_le_big_terminate(struct hci_dev *hdev, u8 big, struct hci_conn *conn)
@@ -823,6 +827,7 @@ static int hci_le_big_terminate(struct hci_dev *hdev, u8 big, struct hci_conn *c
 
 	d->big = big;
 	d->sync_handle = conn->sync_handle;
+	d->pa_sync_term = test_and_clear_bit(HCI_CONN_PA_SYNC, &conn->flags);
 	d->big_sync_term = test_and_clear_bit(HCI_CONN_BIG_SYNC, &conn->flags);
 
 	ret = hci_cmd_sync_queue(hdev, big_terminate_sync, d,
@@ -2099,7 +2104,8 @@ int hci_pa_create_sync(struct hci_dev *hdev, bdaddr_t *dst, __u8 dst_type,
 	return hci_cmd_sync_queue(hdev, create_pa_sync, cp, create_pa_complete);
 }
 
-int hci_le_big_create_sync(struct hci_dev *hdev, struct bt_iso_qos *qos,
+int hci_le_big_create_sync(struct hci_dev *hdev, struct hci_conn *hcon,
+			   struct bt_iso_qos *qos,
 			   __u16 sync_handle, __u8 num_bis, __u8 bis[])
 {
 	struct _packed {
@@ -2115,6 +2121,9 @@ int hci_le_big_create_sync(struct hci_dev *hdev, struct bt_iso_qos *qos,
 	if (err)
 		return err;
 
+	if (hcon)
+		hcon->iso_qos.bcast.big = qos->bcast.big;
+
 	memset(&pdu, 0, sizeof(pdu));
 	pdu.cp.handle = qos->bcast.big;
 	pdu.cp.sync_handle = cpu_to_le16(sync_handle);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 559b6080706c..b4b72070f5f6 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -6581,20 +6581,39 @@ static void hci_le_pa_sync_estabilished_evt(struct hci_dev *hdev, void *data,
 	struct hci_ev_le_pa_sync_established *ev = data;
 	int mask = hdev->link_mode;
 	__u8 flags = 0;
+	struct hci_conn *bis;
 
 	bt_dev_dbg(hdev, "status 0x%2.2x", ev->status);
 
-	if (ev->status)
-		return;
-
 	hci_dev_lock(hdev);
 
 	hci_dev_clear_flag(hdev, HCI_PA_SYNC);
 
 	mask |= hci_proto_connect_ind(hdev, &ev->bdaddr, ISO_LINK, &flags);
-	if (!(mask & HCI_LM_ACCEPT))
+	if (!(mask & HCI_LM_ACCEPT)) {
 		hci_le_pa_term_sync(hdev, ev->handle);
+		goto unlock;
+	}
+
+	if (!(flags & HCI_PROTO_DEFER))
+		goto unlock;
+
+	/* Add connection to indicate the PA sync event */
+	bis = hci_conn_add(hdev, ISO_LINK, BDADDR_ANY,
+			   HCI_ROLE_SLAVE);
 
+	if (!bis)
+		goto unlock;
+
+	if (ev->status)
+		set_bit(HCI_CONN_PA_SYNC_FAILED, &bis->flags);
+	else
+		set_bit(HCI_CONN_PA_SYNC, &bis->flags);
+
+	/* Notify connection to iso layer */
+	hci_connect_cfm(bis, ev->status);
+
+unlock:
 	hci_dev_unlock(hdev);
 }
 
@@ -7045,6 +7064,7 @@ static void hci_le_big_sync_established_evt(struct hci_dev *hdev, void *data,
 {
 	struct hci_evt_le_big_sync_estabilished *ev = data;
 	struct hci_conn *bis;
+	struct hci_conn *pa_sync;
 	int i;
 
 	bt_dev_dbg(hdev, "status 0x%2.2x", ev->status);
@@ -7055,6 +7075,15 @@ static void hci_le_big_sync_established_evt(struct hci_dev *hdev, void *data,
 
 	hci_dev_lock(hdev);
 
+	if (!ev->status) {
+		pa_sync = hci_conn_hash_lookup_pa_sync(hdev, ev->handle);
+		if (pa_sync)
+			/* Also mark the BIG sync established event on the
+			 * associated PA sync hcon
+			 */
+			set_bit(HCI_CONN_BIG_SYNC, &pa_sync->flags);
+	}
+
 	for (i = 0; i < ev->num_bis; i++) {
 		u16 handle = le16_to_cpu(ev->bis[i]);
 		__le32 interval;
@@ -7068,6 +7097,10 @@ static void hci_le_big_sync_established_evt(struct hci_dev *hdev, void *data,
 			bis->handle = handle;
 		}
 
+		if (ev->status != 0x42)
+			/* Mark PA sync as established */
+			set_bit(HCI_CONN_PA_SYNC, &bis->flags);
+
 		bis->iso_qos.bcast.big = ev->handle;
 		memset(&interval, 0, sizeof(interval));
 		memcpy(&interval, ev->latency, sizeof(ev->latency));
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index d10a0f36b947..0cb780817198 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -5384,6 +5384,21 @@ int hci_abort_conn_sync(struct hci_dev *hdev, struct hci_conn *conn, u8 reason)
 		err = hci_reject_conn_sync(hdev, conn, reason);
 		break;
 	case BT_OPEN:
+		hci_dev_lock(hdev);
+
+		/* Cleanup bis or pa sync connections */
+		if (test_and_clear_bit(HCI_CONN_BIG_SYNC_FAILED, &conn->flags) ||
+		    test_and_clear_bit(HCI_CONN_PA_SYNC_FAILED, &conn->flags)) {
+			hci_conn_failed(conn, reason);
+		} else if (test_bit(HCI_CONN_PA_SYNC, &conn->flags) ||
+			   test_bit(HCI_CONN_BIG_SYNC, &conn->flags)) {
+			conn->state = BT_CLOSED;
+			hci_disconn_cfm(conn, reason);
+			hci_conn_del(conn);
+		}
+
+		hci_dev_unlock(hdev);
+		return 0;
 	case BT_BOUND:
 		hci_dev_lock(hdev);
 		hci_conn_failed(conn, reason);
diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
index 6b66d6a88b9a..9879f2349d48 100644
--- a/net/bluetooth/iso.c
+++ b/net/bluetooth/iso.c
@@ -51,6 +51,7 @@ static void iso_sock_kill(struct sock *sk);
 /* iso_pinfo flags values */
 enum {
 	BT_SK_BIG_SYNC,
+	BT_SK_PA_SYNC,
 };
 
 struct iso_pinfo {
@@ -75,6 +76,8 @@ static struct bt_iso_qos default_qos;
 
 static bool check_ucast_qos(struct bt_iso_qos *qos);
 static bool check_bcast_qos(struct bt_iso_qos *qos);
+static bool iso_match_sid(struct sock *sk, void *data);
+static void iso_sock_disconn(struct sock *sk);
 
 /* ---- ISO timers ---- */
 #define ISO_CONN_TIMEOUT	(HZ * 40)
@@ -598,6 +601,15 @@ static void iso_sock_cleanup_listen(struct sock *parent)
 		iso_sock_kill(sk);
 	}
 
+	/* If listening socket stands for a PA sync connection,
+	 * properly disconnect the hcon and socket.
+	 */
+	if (iso_pi(parent)->conn && iso_pi(parent)->conn->hcon &&
+	    test_bit(HCI_CONN_PA_SYNC, &iso_pi(parent)->conn->hcon->flags)) {
+		iso_sock_disconn(parent);
+		return;
+	}
+
 	parent->sk_state  = BT_CLOSED;
 	sock_set_flag(parent, SOCK_ZAPPED);
 }
@@ -619,6 +631,16 @@ static void iso_sock_kill(struct sock *sk)
 	sock_put(sk);
 }
 
+static void iso_sock_disconn(struct sock *sk)
+{
+	sk->sk_state = BT_DISCONN;
+	iso_sock_set_timer(sk, ISO_DISCONN_TIMEOUT);
+	iso_conn_lock(iso_pi(sk)->conn);
+	hci_conn_drop(iso_pi(sk)->conn->hcon);
+	iso_pi(sk)->conn->hcon = NULL;
+	iso_conn_unlock(iso_pi(sk)->conn);
+}
+
 static void __iso_sock_close(struct sock *sk)
 {
 	BT_DBG("sk %p state %d socket %p", sk, sk->sk_state, sk->sk_socket);
@@ -631,20 +653,19 @@ static void __iso_sock_close(struct sock *sk)
 	case BT_CONNECT:
 	case BT_CONNECTED:
 	case BT_CONFIG:
-		if (iso_pi(sk)->conn->hcon) {
-			sk->sk_state = BT_DISCONN;
-			iso_sock_set_timer(sk, ISO_DISCONN_TIMEOUT);
-			iso_conn_lock(iso_pi(sk)->conn);
-			hci_conn_drop(iso_pi(sk)->conn->hcon);
-			iso_pi(sk)->conn->hcon = NULL;
-			iso_conn_unlock(iso_pi(sk)->conn);
-		} else {
+		if (iso_pi(sk)->conn->hcon)
+			iso_sock_disconn(sk);
+		else
 			iso_chan_del(sk, ECONNRESET);
-		}
 		break;
 
 	case BT_CONNECT2:
-		iso_chan_del(sk, ECONNRESET);
+		if (iso_pi(sk)->conn->hcon &&
+		    (test_bit(HCI_CONN_PA_SYNC, &iso_pi(sk)->conn->hcon->flags) ||
+		    test_bit(HCI_CONN_PA_SYNC_FAILED, &iso_pi(sk)->conn->hcon->flags)))
+			iso_sock_disconn(sk);
+		else
+			iso_chan_del(sk, ECONNRESET);
 		break;
 	case BT_DISCONN:
 		iso_chan_del(sk, ECONNRESET);
@@ -1139,6 +1160,29 @@ static void iso_conn_defer_accept(struct hci_conn *conn)
 	hci_send_cmd(hdev, HCI_OP_LE_ACCEPT_CIS, sizeof(cp), &cp);
 }
 
+static void iso_conn_big_sync(struct sock *sk)
+{
+	int err;
+	struct hci_dev *hdev;
+
+	hdev = hci_get_route(&iso_pi(sk)->dst, &iso_pi(sk)->src,
+			     iso_pi(sk)->src_type);
+
+	if (!hdev)
+		return;
+
+	if (!test_and_set_bit(BT_SK_BIG_SYNC, &iso_pi(sk)->flags)) {
+		err = hci_le_big_create_sync(hdev, iso_pi(sk)->conn->hcon,
+					     &iso_pi(sk)->qos,
+					     iso_pi(sk)->sync_handle,
+					     iso_pi(sk)->bc_num_bis,
+					     iso_pi(sk)->bc_bis);
+		if (err)
+			bt_dev_err(hdev, "hci_le_big_create_sync: %d",
+				   err);
+	}
+}
+
 static int iso_sock_recvmsg(struct socket *sock, struct msghdr *msg,
 			    size_t len, int flags)
 {
@@ -1151,8 +1195,15 @@ static int iso_sock_recvmsg(struct socket *sock, struct msghdr *msg,
 		lock_sock(sk);
 		switch (sk->sk_state) {
 		case BT_CONNECT2:
-			iso_conn_defer_accept(pi->conn->hcon);
-			sk->sk_state = BT_CONFIG;
+			if (pi->conn->hcon &&
+			    test_bit(HCI_CONN_PA_SYNC, &pi->conn->hcon->flags)) {
+				iso_conn_big_sync(sk);
+				sk->sk_state = BT_LISTEN;
+				set_bit(BT_SK_PA_SYNC, &iso_pi(sk)->flags);
+			} else {
+				iso_conn_defer_accept(pi->conn->hcon);
+				sk->sk_state = BT_CONFIG;
+			}
 			release_sock(sk);
 			return 0;
 		case BT_CONNECT:
@@ -1513,11 +1564,17 @@ static bool iso_match_big(struct sock *sk, void *data)
 	return ev->handle == iso_pi(sk)->qos.bcast.big;
 }
 
+static bool iso_match_pa_sync_flag(struct sock *sk, void *data)
+{
+	return test_bit(BT_SK_PA_SYNC, &iso_pi(sk)->flags);
+}
+
 static void iso_conn_ready(struct iso_conn *conn)
 {
-	struct sock *parent;
+	struct sock *parent = NULL;
 	struct sock *sk = conn->sk;
-	struct hci_ev_le_big_sync_estabilished *ev;
+	struct hci_ev_le_big_sync_estabilished *ev = NULL;
+	struct hci_ev_le_pa_sync_established *ev2 = NULL;
 	struct hci_conn *hcon;
 
 	BT_DBG("conn %p", conn);
@@ -1529,15 +1586,32 @@ static void iso_conn_ready(struct iso_conn *conn)
 		if (!hcon)
 			return;
 
-		ev = hci_recv_event_data(hcon->hdev,
-					 HCI_EVT_LE_BIG_SYNC_ESTABILISHED);
-		if (ev)
+		if (test_bit(HCI_CONN_BIG_SYNC, &hcon->flags) ||
+		    test_bit(HCI_CONN_BIG_SYNC_FAILED, &hcon->flags)) {
+			ev = hci_recv_event_data(hcon->hdev,
+						 HCI_EVT_LE_BIG_SYNC_ESTABILISHED);
+
+			/* Get reference to PA sync parent socket, if it exists */
 			parent = iso_get_sock_listen(&hcon->src,
 						     &hcon->dst,
-						     iso_match_big, ev);
-		else
+						     iso_match_pa_sync_flag, NULL);
+			if (!parent && ev)
+				parent = iso_get_sock_listen(&hcon->src,
+							     &hcon->dst,
+							     iso_match_big, ev);
+		} else if (test_bit(HCI_CONN_PA_SYNC, &hcon->flags) ||
+				test_bit(HCI_CONN_PA_SYNC_FAILED, &hcon->flags)) {
+			ev2 = hci_recv_event_data(hcon->hdev,
+						  HCI_EV_LE_PA_SYNC_ESTABLISHED);
+			if (ev2)
+				parent = iso_get_sock_listen(&hcon->src,
+							     &hcon->dst,
+							     iso_match_sid, ev2);
+		}
+
+		if (!parent)
 			parent = iso_get_sock_listen(&hcon->src,
-						     BDADDR_ANY, NULL, NULL);
+							BDADDR_ANY, NULL, NULL);
 
 		if (!parent)
 			return;
@@ -1554,11 +1628,17 @@ static void iso_conn_ready(struct iso_conn *conn)
 		iso_sock_init(sk, parent);
 
 		bacpy(&iso_pi(sk)->src, &hcon->src);
-		iso_pi(sk)->src_type = hcon->src_type;
+
+		/* Convert from HCI to three-value type */
+		if (hcon->src_type == ADDR_LE_DEV_PUBLIC)
+			iso_pi(sk)->src_type = BDADDR_LE_PUBLIC;
+		else
+			iso_pi(sk)->src_type = BDADDR_LE_RANDOM;
 
 		/* If hcon has no destination address (BDADDR_ANY) it means it
-		 * was created by HCI_EV_LE_BIG_SYNC_ESTABILISHED so we need to
-		 * initialize using the parent socket destination address.
+		 * was created by HCI_EV_LE_BIG_SYNC_ESTABILISHED or
+		 * HCI_EV_LE_PA_SYNC_ESTABLISHED so we need to initialize using
+		 * the parent socket destination address.
 		 */
 		if (!bacmp(&hcon->dst, BDADDR_ANY)) {
 			bacpy(&hcon->dst, &iso_pi(parent)->dst);
@@ -1566,13 +1646,21 @@ static void iso_conn_ready(struct iso_conn *conn)
 			hcon->sync_handle = iso_pi(parent)->sync_handle;
 		}
 
+		if (ev2 && !ev2->status) {
+			iso_pi(sk)->sync_handle = iso_pi(parent)->sync_handle;
+			iso_pi(sk)->qos = iso_pi(parent)->qos;
+			iso_pi(sk)->bc_num_bis = iso_pi(parent)->bc_num_bis;
+			memcpy(iso_pi(sk)->bc_bis, iso_pi(parent)->bc_bis, ISO_MAX_NUM_BIS);
+		}
+
 		bacpy(&iso_pi(sk)->dst, &hcon->dst);
 		iso_pi(sk)->dst_type = hcon->dst_type;
 
 		hci_conn_hold(hcon);
 		iso_chan_add(conn, sk, parent);
 
-		if (ev && ((struct hci_evt_le_big_sync_estabilished *)ev)->status) {
+		if ((ev && ((struct hci_evt_le_big_sync_estabilished *)ev)->status) ||
+		    (ev2 && ev2->status)) {
 			/* Trigger error signal on child socket */
 			sk->sk_err = ECONNREFUSED;
 			sk->sk_error_report(sk);
@@ -1630,7 +1718,7 @@ int iso_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags)
 	if (ev1) {
 		sk = iso_get_sock_listen(&hdev->bdaddr, bdaddr, iso_match_sid,
 					 ev1);
-		if (sk)
+		if (sk && !ev1->status)
 			iso_pi(sk)->sync_handle = le16_to_cpu(ev1->handle);
 
 		goto done;
@@ -1638,16 +1726,21 @@ int iso_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags)
 
 	ev2 = hci_recv_event_data(hdev, HCI_EVT_LE_BIG_INFO_ADV_REPORT);
 	if (ev2) {
+		/* Try to get PA sync listening socket, if it exists */
 		sk = iso_get_sock_listen(&hdev->bdaddr, bdaddr,
-					 iso_match_sync_handle, ev2);
+						iso_match_pa_sync_flag, NULL);
+		if (!sk)
+			sk = iso_get_sock_listen(&hdev->bdaddr, bdaddr,
+						 iso_match_sync_handle, ev2);
 		if (sk) {
 			int err;
 
 			if (ev2->num_bis < iso_pi(sk)->bc_num_bis)
 				iso_pi(sk)->bc_num_bis = ev2->num_bis;
 
-			if (!test_and_set_bit(BT_SK_BIG_SYNC, &iso_pi(sk)->flags)) {
-				err = hci_le_big_create_sync(hdev,
+			if (!test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags) &&
+			    !test_and_set_bit(BT_SK_BIG_SYNC, &iso_pi(sk)->flags)) {
+				err = hci_le_big_create_sync(hdev, NULL,
 							     &iso_pi(sk)->qos,
 							     iso_pi(sk)->sync_handle,
 							     iso_pi(sk)->bc_num_bis,
@@ -1699,12 +1792,13 @@ static void iso_connect_cfm(struct hci_conn *hcon, __u8 status)
 
 	BT_DBG("hcon %p bdaddr %pMR status %d", hcon, &hcon->dst, status);
 
-	/* Similar to the success case, if HCI_CONN_BIG_SYNC_FAILED is set,
-	 * queue the failed bis connection into the accept queue of the
-	 * listening socket and wake up userspace, to inform the user about
-	 * the BIG sync failed event.
+	/* Similar to the success case, if HCI_CONN_BIG_SYNC_FAILED or
+	 * HCI_CONN_PA_SYNC_FAILED is set, queue the failed connection
+	 * into the accept queue of the listening socket and wake up
+	 * userspace, to inform the user about the event.
 	 */
-	if (!status || test_bit(HCI_CONN_BIG_SYNC_FAILED, &hcon->flags)) {
+	if (!status || test_bit(HCI_CONN_BIG_SYNC_FAILED, &hcon->flags) ||
+	    test_bit(HCI_CONN_PA_SYNC_FAILED, &hcon->flags)) {
 		struct iso_conn *conn;
 
 		conn = iso_conn_add(hcon);
-- 
2.34.1

RE: Bluetooth: ISO: Use defer setup to separate PA sync and BIG sync

[bluez.test.bot]

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

---Test result---

Test Summary:
CheckPatch                    PASS      1.57 seconds
GitLint                       PASS      0.28 seconds
SubjectPrefix                 PASS      0.10 seconds
BuildKernel                   PASS      32.76 seconds
CheckAllWarning               PASS      35.72 seconds
CheckSparse                   WARNING   40.23 seconds
CheckSmatch                   WARNING   113.18 seconds
BuildKernel32                 PASS      31.03 seconds
TestRunnerSetup               PASS      478.51 seconds
TestRunner_l2cap-tester       PASS      27.42 seconds
TestRunner_iso-tester         PASS      48.76 seconds
TestRunner_bnep-tester        PASS      11.03 seconds
TestRunner_mgmt-tester        PASS      220.60 seconds
TestRunner_rfcomm-tester      PASS      16.08 seconds
TestRunner_sco-tester         PASS      19.13 seconds
TestRunner_ioctl-tester       PASS      18.04 seconds
TestRunner_mesh-tester        PASS      13.44 seconds
TestRunner_smp-tester         PASS      14.34 seconds
TestRunner_userchan-tester    PASS      11.23 seconds
IncrementalBuild              PASS      30.12 seconds

Details
##############################
Test: CheckSparse - WARNING
Desc: Run sparse tool with linux kernel
Output:
net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):


---
Regards,
Linux Bluetooth

Re: [PATCH v3 1/1] Bluetooth: ISO: Use defer setup to separate PA sync and BIG sync

[Pauli Virtanen]

Hi,

to, 2023-08-17 kello 09:44 +0300, Iulia Tanasescu kirjoitti:
> This commit implements defer setup support for the Broadcast Sink
> scenario: By setting defer setup on a broadcast socket before calling
> listen, the user is able to trigger the PA sync and BIG sync procedures
> separately.
> 
> This is useful if the user first wants to synchronize to the periodic
> advertising transmitted by a Broadcast Source, and trigger the BIG sync
> procedure later on.
> 
> If defer setup is set, once a PA sync established event arrives, a new
> hcon is created and notified to the ISO layer. A child socket associated
> with the PA sync connection will be added to the accept queue of the
> listening socket.
> 
> Once the accept call returns the fd for the PA sync child socket, the
> user should call read on that fd. This will trigger the BIG create sync
> procedure, and the PA sync socket will become a listening socket itself.
> 
> When the BIG sync established event is notified to the ISO layer, the
> bis connections will be added to the accept queue of the PA sync parent.
> The user should call accept on the PA sync socket to get the final bis
> connections.
> 
> Signed-off-by: Iulia Tanasescu <iulia.tanasescu@nxp.com>
> ---
>  include/net/bluetooth/hci_core.h |  30 +++++-
>  net/bluetooth/hci_conn.c         |  13 ++-
>  net/bluetooth/hci_event.c        |  41 +++++++-
>  net/bluetooth/hci_sync.c         |  15 +++
>  net/bluetooth/iso.c              | 160 ++++++++++++++++++++++++-------
>  5 files changed, 218 insertions(+), 41 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index c53d74236e3a..6fb055e3c595 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -978,6 +978,8 @@ enum {
>  	HCI_CONN_CREATE_CIS,
>  	HCI_CONN_BIG_SYNC,
>  	HCI_CONN_BIG_SYNC_FAILED,
> +	HCI_CONN_PA_SYNC,
> +	HCI_CONN_PA_SYNC_FAILED,
>  };
>  
>  static inline bool hci_conn_ssp_enabled(struct hci_conn *conn)
> @@ -1300,7 +1302,7 @@ static inline struct hci_conn *hci_conn_hash_lookup_big_any_dst(struct hci_dev *
>  		if (c->type != ISO_LINK)
>  			continue;
>  
> -		if (handle == c->iso_qos.bcast.big) {
> +		if (handle != BT_ISO_QOS_BIG_UNSET && handle == c->iso_qos.bcast.big) {
>  			rcu_read_unlock();
>  			return c;
>  		}
> @@ -1311,6 +1313,29 @@ static inline struct hci_conn *hci_conn_hash_lookup_big_any_dst(struct hci_dev *
>  	return NULL;
>  }
>  
> +static inline struct hci_conn *
> +hci_conn_hash_lookup_pa_sync(struct hci_dev *hdev, __u8 big)
> +{
> +	struct hci_conn_hash *h = &hdev->conn_hash;
> +	struct hci_conn  *c;
> +
> +	rcu_read_lock();
> +
> +	list_for_each_entry_rcu(c, &h->list, list) {
> +		if (c->type != ISO_LINK ||
> +			!test_bit(HCI_CONN_PA_SYNC, &c->flags))
> +			continue;
> +
> +		if (c->iso_qos.bcast.big == big) {
> +			rcu_read_unlock();
> +			return c;
> +		}
> +	}
> +	rcu_read_unlock();
> +
> +	return NULL;
> +}
> +
>  static inline struct hci_conn *hci_conn_hash_lookup_state(struct hci_dev *hdev,
>  							__u8 type, __u16 state)
>  {
> @@ -1435,7 +1460,8 @@ struct hci_conn *hci_connect_bis(struct hci_dev *hdev, bdaddr_t *dst,
>  				 __u8 data_len, __u8 *data);
>  int hci_pa_create_sync(struct hci_dev *hdev, bdaddr_t *dst, __u8 dst_type,
>  		       __u8 sid, struct bt_iso_qos *qos);
> -int hci_le_big_create_sync(struct hci_dev *hdev, struct bt_iso_qos *qos,
> +int hci_le_big_create_sync(struct hci_dev *hdev, struct hci_conn *hcon,
> +			   struct bt_iso_qos *qos,
>  			   __u16 sync_handle, __u8 num_bis, __u8 bis[]);
>  int hci_conn_check_link_mode(struct hci_conn *conn);
>  int hci_conn_check_secure(struct hci_conn *conn, __u8 sec_level);
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 95339623883c..8b0c8e631324 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -734,6 +734,7 @@ struct iso_list_data {
>  	};
>  	int count;
>  	bool big_term;
> +	bool pa_sync_term;
>  	bool big_sync_term;
>  };
>  
> @@ -807,7 +808,10 @@ static int big_terminate_sync(struct hci_dev *hdev, void *data)
>  	if (d->big_sync_term)
>  		hci_le_big_terminate_sync(hdev, d->big);
>  
> -	return hci_le_pa_terminate_sync(hdev, d->sync_handle);
> +	if (d->pa_sync_term)
> +		return hci_le_pa_terminate_sync(hdev, d->sync_handle);
> +
> +	return 0;
>  }
>  
>  static int hci_le_big_terminate(struct hci_dev *hdev, u8 big, struct hci_conn *conn)
> @@ -823,6 +827,7 @@ static int hci_le_big_terminate(struct hci_dev *hdev, u8 big, struct hci_conn *c
>  
>  	d->big = big;
>  	d->sync_handle = conn->sync_handle;
> +	d->pa_sync_term = test_and_clear_bit(HCI_CONN_PA_SYNC, &conn->flags);
>  	d->big_sync_term = test_and_clear_bit(HCI_CONN_BIG_SYNC, &conn->flags);
>  
>  	ret = hci_cmd_sync_queue(hdev, big_terminate_sync, d,
> @@ -2099,7 +2104,8 @@ int hci_pa_create_sync(struct hci_dev *hdev, bdaddr_t *dst, __u8 dst_type,
>  	return hci_cmd_sync_queue(hdev, create_pa_sync, cp, create_pa_complete);
>  }
>  
> -int hci_le_big_create_sync(struct hci_dev *hdev, struct bt_iso_qos *qos,
> +int hci_le_big_create_sync(struct hci_dev *hdev, struct hci_conn *hcon,
> +			   struct bt_iso_qos *qos,
>  			   __u16 sync_handle, __u8 num_bis, __u8 bis[])
>  {
>  	struct _packed {
> @@ -2115,6 +2121,9 @@ int hci_le_big_create_sync(struct hci_dev *hdev, struct bt_iso_qos *qos,
>  	if (err)
>  		return err;
>  
> +	if (hcon)
> +		hcon->iso_qos.bcast.big = qos->bcast.big;
> +
>  	memset(&pdu, 0, sizeof(pdu));
>  	pdu.cp.handle = qos->bcast.big;
>  	pdu.cp.sync_handle = cpu_to_le16(sync_handle);
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 559b6080706c..b4b72070f5f6 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -6581,20 +6581,39 @@ static void hci_le_pa_sync_estabilished_evt(struct hci_dev *hdev, void *data,
>  	struct hci_ev_le_pa_sync_established *ev = data;
>  	int mask = hdev->link_mode;
>  	__u8 flags = 0;
> +	struct hci_conn *bis;
>  
>  	bt_dev_dbg(hdev, "status 0x%2.2x", ev->status);
>  
> -	if (ev->status)
> -		return;
> -
>  	hci_dev_lock(hdev);
>  
>  	hci_dev_clear_flag(hdev, HCI_PA_SYNC);
>  
>  	mask |= hci_proto_connect_ind(hdev, &ev->bdaddr, ISO_LINK, &flags);
> -	if (!(mask & HCI_LM_ACCEPT))
> +	if (!(mask & HCI_LM_ACCEPT)) {
>  		hci_le_pa_term_sync(hdev, ev->handle);
> +		goto unlock;
> +	}
> +
> +	if (!(flags & HCI_PROTO_DEFER))
> +		goto unlock;
> +
> +	/* Add connection to indicate the PA sync event */
> +	bis = hci_conn_add(hdev, ISO_LINK, BDADDR_ANY,
> +			   HCI_ROLE_SLAVE);
>  
> +	if (!bis)
> +		goto unlock;
> +
> +	if (ev->status)
> +		set_bit(HCI_CONN_PA_SYNC_FAILED, &bis->flags);
> +	else
> +		set_bit(HCI_CONN_PA_SYNC, &bis->flags);
> +
> +	/* Notify connection to iso layer */
> +	hci_connect_cfm(bis, ev->status);
> +
> +unlock:
>  	hci_dev_unlock(hdev);
>  }
>  
> @@ -7045,6 +7064,7 @@ static void hci_le_big_sync_established_evt(struct hci_dev *hdev, void *data,
>  {
>  	struct hci_evt_le_big_sync_estabilished *ev = data;
>  	struct hci_conn *bis;
> +	struct hci_conn *pa_sync;
>  	int i;
>  
>  	bt_dev_dbg(hdev, "status 0x%2.2x", ev->status);
> @@ -7055,6 +7075,15 @@ static void hci_le_big_sync_established_evt(struct hci_dev *hdev, void *data,
>  
>  	hci_dev_lock(hdev);
>  
> +	if (!ev->status) {
> +		pa_sync = hci_conn_hash_lookup_pa_sync(hdev, ev->handle);
> +		if (pa_sync)
> +			/* Also mark the BIG sync established event on the
> +			 * associated PA sync hcon
> +			 */
> +			set_bit(HCI_CONN_BIG_SYNC, &pa_sync->flags);
> +	}
> +
>  	for (i = 0; i < ev->num_bis; i++) {
>  		u16 handle = le16_to_cpu(ev->bis[i]);
>  		__le32 interval;
> @@ -7068,6 +7097,10 @@ static void hci_le_big_sync_established_evt(struct hci_dev *hdev, void *data,
>  			bis->handle = handle;
>  		}
>  
> +		if (ev->status != 0x42)
> +			/* Mark PA sync as established */
> +			set_bit(HCI_CONN_PA_SYNC, &bis->flags);
> +
>  		bis->iso_qos.bcast.big = ev->handle;
>  		memset(&interval, 0, sizeof(interval));
>  		memcpy(&interval, ev->latency, sizeof(ev->latency));
> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index d10a0f36b947..0cb780817198 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -5384,6 +5384,21 @@ int hci_abort_conn_sync(struct hci_dev *hdev, struct hci_conn *conn, u8 reason)
>  		err = hci_reject_conn_sync(hdev, conn, reason);
>  		break;
>  	case BT_OPEN:
> +		hci_dev_lock(hdev);
> +
> +		/* Cleanup bis or pa sync connections */
> +		if (test_and_clear_bit(HCI_CONN_BIG_SYNC_FAILED, &conn->flags) ||
> +		    test_and_clear_bit(HCI_CONN_PA_SYNC_FAILED, &conn->flags)) {
> +			hci_conn_failed(conn, reason);
> +		} else if (test_bit(HCI_CONN_PA_SYNC, &conn->flags) ||
> +			   test_bit(HCI_CONN_BIG_SYNC, &conn->flags)) {
> +			conn->state = BT_CLOSED;
> +			hci_disconn_cfm(conn, reason);
> +			hci_conn_del(conn);
> +		}
> +
> +		hci_dev_unlock(hdev);
> +		return 0;

Note the hci_disconnect_all_sync changes in 45c37c4e9c9aab5b now
require the conn is deleted in all cases, the above probably results to
busy loop if non-ISO broadcast conn is in BT_OPEN when controller is
supended.

>  	case BT_BOUND:
>  		hci_dev_lock(hdev);
>  		hci_conn_failed(conn, reason);
> diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
> index 6b66d6a88b9a..9879f2349d48 100644
> --- a/net/bluetooth/iso.c
> +++ b/net/bluetooth/iso.c
> @@ -51,6 +51,7 @@ static void iso_sock_kill(struct sock *sk);
>  /* iso_pinfo flags values */
>  enum {
>  	BT_SK_BIG_SYNC,
> +	BT_SK_PA_SYNC,
>  };
>  
>  struct iso_pinfo {
> @@ -75,6 +76,8 @@ static struct bt_iso_qos default_qos;
>  
>  static bool check_ucast_qos(struct bt_iso_qos *qos);
>  static bool check_bcast_qos(struct bt_iso_qos *qos);
> +static bool iso_match_sid(struct sock *sk, void *data);
> +static void iso_sock_disconn(struct sock *sk);
>  
>  /* ---- ISO timers ---- */
>  #define ISO_CONN_TIMEOUT	(HZ * 40)
> @@ -598,6 +601,15 @@ static void iso_sock_cleanup_listen(struct sock *parent)
>  		iso_sock_kill(sk);
>  	}
>  
> +	/* If listening socket stands for a PA sync connection,
> +	 * properly disconnect the hcon and socket.
> +	 */
> +	if (iso_pi(parent)->conn && iso_pi(parent)->conn->hcon &&
> +	    test_bit(HCI_CONN_PA_SYNC, &iso_pi(parent)->conn->hcon->flags)) {
> +		iso_sock_disconn(parent);
> +		return;
> +	}
> +
>  	parent->sk_state  = BT_CLOSED;
>  	sock_set_flag(parent, SOCK_ZAPPED);
>  }
> @@ -619,6 +631,16 @@ static void iso_sock_kill(struct sock *sk)
>  	sock_put(sk);
>  }
>  
> +static void iso_sock_disconn(struct sock *sk)
> +{
> +	sk->sk_state = BT_DISCONN;
> +	iso_sock_set_timer(sk, ISO_DISCONN_TIMEOUT);
> +	iso_conn_lock(iso_pi(sk)->conn);
> +	hci_conn_drop(iso_pi(sk)->conn->hcon);
> +	iso_pi(sk)->conn->hcon = NULL;
> +	iso_conn_unlock(iso_pi(sk)->conn);
> +}
> +
>  static void __iso_sock_close(struct sock *sk)
>  {
>  	BT_DBG("sk %p state %d socket %p", sk, sk->sk_state, sk->sk_socket);
> @@ -631,20 +653,19 @@ static void __iso_sock_close(struct sock *sk)
>  	case BT_CONNECT:
>  	case BT_CONNECTED:
>  	case BT_CONFIG:
> -		if (iso_pi(sk)->conn->hcon) {
> -			sk->sk_state = BT_DISCONN;
> -			iso_sock_set_timer(sk, ISO_DISCONN_TIMEOUT);
> -			iso_conn_lock(iso_pi(sk)->conn);
> -			hci_conn_drop(iso_pi(sk)->conn->hcon);
> -			iso_pi(sk)->conn->hcon = NULL;
> -			iso_conn_unlock(iso_pi(sk)->conn);
> -		} else {
> +		if (iso_pi(sk)->conn->hcon)
> +			iso_sock_disconn(sk);
> +		else
>  			iso_chan_del(sk, ECONNRESET);
> -		}
>  		break;
>  
>  	case BT_CONNECT2:
> -		iso_chan_del(sk, ECONNRESET);
> +		if (iso_pi(sk)->conn->hcon &&
> +		    (test_bit(HCI_CONN_PA_SYNC, &iso_pi(sk)->conn->hcon->flags) ||
> +		    test_bit(HCI_CONN_PA_SYNC_FAILED, &iso_pi(sk)->conn->hcon->flags)))
> +			iso_sock_disconn(sk);
> +		else
> +			iso_chan_del(sk, ECONNRESET);
>  		break;
>  	case BT_DISCONN:
>  		iso_chan_del(sk, ECONNRESET);
> @@ -1139,6 +1160,29 @@ static void iso_conn_defer_accept(struct hci_conn *conn)
>  	hci_send_cmd(hdev, HCI_OP_LE_ACCEPT_CIS, sizeof(cp), &cp);
>  }
>  
> +static void iso_conn_big_sync(struct sock *sk)
> +{
> +	int err;
> +	struct hci_dev *hdev;
> +
> +	hdev = hci_get_route(&iso_pi(sk)->dst, &iso_pi(sk)->src,
> +			     iso_pi(sk)->src_type);
> +
> +	if (!hdev)
> +		return;
> +
> +	if (!test_and_set_bit(BT_SK_BIG_SYNC, &iso_pi(sk)->flags)) {
> +		err = hci_le_big_create_sync(hdev, iso_pi(sk)->conn->hcon,
> +					     &iso_pi(sk)->qos,
> +					     iso_pi(sk)->sync_handle,
> +					     iso_pi(sk)->bc_num_bis,
> +					     iso_pi(sk)->bc_bis);
> +		if (err)
> +			bt_dev_err(hdev, "hci_le_big_create_sync: %d",
> +				   err);
> +	}
> +}
> +
>  static int iso_sock_recvmsg(struct socket *sock, struct msghdr *msg,
>  			    size_t len, int flags)
>  {
> @@ -1151,8 +1195,15 @@ static int iso_sock_recvmsg(struct socket *sock, struct msghdr *msg,
>  		lock_sock(sk);
>  		switch (sk->sk_state) {
>  		case BT_CONNECT2:
> -			iso_conn_defer_accept(pi->conn->hcon);
> -			sk->sk_state = BT_CONFIG;
> +			if (pi->conn->hcon &&
> +			    test_bit(HCI_CONN_PA_SYNC, &pi->conn->hcon->flags)) {
> +				iso_conn_big_sync(sk);
> +				sk->sk_state = BT_LISTEN;
> +				set_bit(BT_SK_PA_SYNC, &iso_pi(sk)->flags);
> +			} else {
> +				iso_conn_defer_accept(pi->conn->hcon);
> +				sk->sk_state = BT_CONFIG;
> +			}
>  			release_sock(sk);
>  			return 0;
>  		case BT_CONNECT:
> @@ -1513,11 +1564,17 @@ static bool iso_match_big(struct sock *sk, void *data)
>  	return ev->handle == iso_pi(sk)->qos.bcast.big;
>  }
>  
> +static bool iso_match_pa_sync_flag(struct sock *sk, void *data)
> +{
> +	return test_bit(BT_SK_PA_SYNC, &iso_pi(sk)->flags);
> +}
> +
>  static void iso_conn_ready(struct iso_conn *conn)
>  {
> -	struct sock *parent;
> +	struct sock *parent = NULL;
>  	struct sock *sk = conn->sk;
> -	struct hci_ev_le_big_sync_estabilished *ev;
> +	struct hci_ev_le_big_sync_estabilished *ev = NULL;
> +	struct hci_ev_le_pa_sync_established *ev2 = NULL;
>  	struct hci_conn *hcon;
>  
>  	BT_DBG("conn %p", conn);
> @@ -1529,15 +1586,32 @@ static void iso_conn_ready(struct iso_conn *conn)
>  		if (!hcon)
>  			return;
>  
> -		ev = hci_recv_event_data(hcon->hdev,
> -					 HCI_EVT_LE_BIG_SYNC_ESTABILISHED);
> -		if (ev)
> +		if (test_bit(HCI_CONN_BIG_SYNC, &hcon->flags) ||
> +		    test_bit(HCI_CONN_BIG_SYNC_FAILED, &hcon->flags)) {
> +			ev = hci_recv_event_data(hcon->hdev,
> +						 HCI_EVT_LE_BIG_SYNC_ESTABILISHED);
> +
> +			/* Get reference to PA sync parent socket, if it exists */
>  			parent = iso_get_sock_listen(&hcon->src,
>  						     &hcon->dst,
> -						     iso_match_big, ev);
> -		else
> +						     iso_match_pa_sync_flag, NULL);
> +			if (!parent && ev)
> +				parent = iso_get_sock_listen(&hcon->src,
> +							     &hcon->dst,
> +							     iso_match_big, ev);
> +		} else if (test_bit(HCI_CONN_PA_SYNC, &hcon->flags) ||
> +				test_bit(HCI_CONN_PA_SYNC_FAILED, &hcon->flags)) {
> +			ev2 = hci_recv_event_data(hcon->hdev,
> +						  HCI_EV_LE_PA_SYNC_ESTABLISHED);
> +			if (ev2)
> +				parent = iso_get_sock_listen(&hcon->src,
> +							     &hcon->dst,
> +							     iso_match_sid, ev2);
> +		}
> +
> +		if (!parent)
>  			parent = iso_get_sock_listen(&hcon->src,
> -						     BDADDR_ANY, NULL, NULL);
> +							BDADDR_ANY, NULL, NULL);
>  
>  		if (!parent)
>  			return;
> @@ -1554,11 +1628,17 @@ static void iso_conn_ready(struct iso_conn *conn)
>  		iso_sock_init(sk, parent);
>  
>  		bacpy(&iso_pi(sk)->src, &hcon->src);
> -		iso_pi(sk)->src_type = hcon->src_type;
> +
> +		/* Convert from HCI to three-value type */
> +		if (hcon->src_type == ADDR_LE_DEV_PUBLIC)
> +			iso_pi(sk)->src_type = BDADDR_LE_PUBLIC;
> +		else
> +			iso_pi(sk)->src_type = BDADDR_LE_RANDOM;
>  
>  		/* If hcon has no destination address (BDADDR_ANY) it means it
> -		 * was created by HCI_EV_LE_BIG_SYNC_ESTABILISHED so we need to
> -		 * initialize using the parent socket destination address.
> +		 * was created by HCI_EV_LE_BIG_SYNC_ESTABILISHED or
> +		 * HCI_EV_LE_PA_SYNC_ESTABLISHED so we need to initialize using
> +		 * the parent socket destination address.
>  		 */
>  		if (!bacmp(&hcon->dst, BDADDR_ANY)) {
>  			bacpy(&hcon->dst, &iso_pi(parent)->dst);
> @@ -1566,13 +1646,21 @@ static void iso_conn_ready(struct iso_conn *conn)
>  			hcon->sync_handle = iso_pi(parent)->sync_handle;
>  		}
>  
> +		if (ev2 && !ev2->status) {
> +			iso_pi(sk)->sync_handle = iso_pi(parent)->sync_handle;
> +			iso_pi(sk)->qos = iso_pi(parent)->qos;
> +			iso_pi(sk)->bc_num_bis = iso_pi(parent)->bc_num_bis;
> +			memcpy(iso_pi(sk)->bc_bis, iso_pi(parent)->bc_bis, ISO_MAX_NUM_BIS);
> +		}
> +
>  		bacpy(&iso_pi(sk)->dst, &hcon->dst);
>  		iso_pi(sk)->dst_type = hcon->dst_type;
>  
>  		hci_conn_hold(hcon);
>  		iso_chan_add(conn, sk, parent);
>  
> -		if (ev && ((struct hci_evt_le_big_sync_estabilished *)ev)->status) {
> +		if ((ev && ((struct hci_evt_le_big_sync_estabilished *)ev)->status) ||
> +		    (ev2 && ev2->status)) {
>  			/* Trigger error signal on child socket */
>  			sk->sk_err = ECONNREFUSED;
>  			sk->sk_error_report(sk);
> @@ -1630,7 +1718,7 @@ int iso_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags)
>  	if (ev1) {
>  		sk = iso_get_sock_listen(&hdev->bdaddr, bdaddr, iso_match_sid,
>  					 ev1);
> -		if (sk)
> +		if (sk && !ev1->status)
>  			iso_pi(sk)->sync_handle = le16_to_cpu(ev1->handle);
>  
>  		goto done;
> @@ -1638,16 +1726,21 @@ int iso_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags)
>  
>  	ev2 = hci_recv_event_data(hdev, HCI_EVT_LE_BIG_INFO_ADV_REPORT);
>  	if (ev2) {
> +		/* Try to get PA sync listening socket, if it exists */
>  		sk = iso_get_sock_listen(&hdev->bdaddr, bdaddr,
> -					 iso_match_sync_handle, ev2);
> +						iso_match_pa_sync_flag, NULL);
> +		if (!sk)
> +			sk = iso_get_sock_listen(&hdev->bdaddr, bdaddr,
> +						 iso_match_sync_handle, ev2);
>  		if (sk) {
>  			int err;
>  
>  			if (ev2->num_bis < iso_pi(sk)->bc_num_bis)
>  				iso_pi(sk)->bc_num_bis = ev2->num_bis;
>  
> -			if (!test_and_set_bit(BT_SK_BIG_SYNC, &iso_pi(sk)->flags)) {
> -				err = hci_le_big_create_sync(hdev,
> +			if (!test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags) &&
> +			    !test_and_set_bit(BT_SK_BIG_SYNC, &iso_pi(sk)->flags)) {
> +				err = hci_le_big_create_sync(hdev, NULL,
>  							     &iso_pi(sk)->qos,
>  							     iso_pi(sk)->sync_handle,
>  							     iso_pi(sk)->bc_num_bis,
> @@ -1699,12 +1792,13 @@ static void iso_connect_cfm(struct hci_conn *hcon, __u8 status)
>  
>  	BT_DBG("hcon %p bdaddr %pMR status %d", hcon, &hcon->dst, status);
>  
> -	/* Similar to the success case, if HCI_CONN_BIG_SYNC_FAILED is set,
> -	 * queue the failed bis connection into the accept queue of the
> -	 * listening socket and wake up userspace, to inform the user about
> -	 * the BIG sync failed event.
> +	/* Similar to the success case, if HCI_CONN_BIG_SYNC_FAILED or
> +	 * HCI_CONN_PA_SYNC_FAILED is set, queue the failed connection
> +	 * into the accept queue of the listening socket and wake up
> +	 * userspace, to inform the user about the event.
>  	 */
> -	if (!status || test_bit(HCI_CONN_BIG_SYNC_FAILED, &hcon->flags)) {
> +	if (!status || test_bit(HCI_CONN_BIG_SYNC_FAILED, &hcon->flags) ||
> +	    test_bit(HCI_CONN_PA_SYNC_FAILED, &hcon->flags)) {
>  		struct iso_conn *conn;
>  
>  		conn = iso_conn_add(hcon);

-- 
Pauli Virtanen

Re: [PATCH v3 1/1] Bluetooth: ISO: Use defer setup to separate PA sync and BIG sync

[Luiz Augusto von Dentz]

Hi Pauli,

On Thu, Aug 17, 2023 at 1:26 PM Pauli Virtanen <pav@iki.fi> wrote:
>
> Hi,
>
> to, 2023-08-17 kello 09:44 +0300, Iulia Tanasescu kirjoitti:
> > This commit implements defer setup support for the Broadcast Sink
> > scenario: By setting defer setup on a broadcast socket before calling
> > listen, the user is able to trigger the PA sync and BIG sync procedures
> > separately.
> >
> > This is useful if the user first wants to synchronize to the periodic
> > advertising transmitted by a Broadcast Source, and trigger the BIG sync
> > procedure later on.
> >
> > If defer setup is set, once a PA sync established event arrives, a new
> > hcon is created and notified to the ISO layer. A child socket associated
> > with the PA sync connection will be added to the accept queue of the
> > listening socket.
> >
> > Once the accept call returns the fd for the PA sync child socket, the
> > user should call read on that fd. This will trigger the BIG create sync
> > procedure, and the PA sync socket will become a listening socket itself.
> >
> > When the BIG sync established event is notified to the ISO layer, the
> > bis connections will be added to the accept queue of the PA sync parent.
> > The user should call accept on the PA sync socket to get the final bis
> > connections.
> >
> > Signed-off-by: Iulia Tanasescu <iulia.tanasescu@nxp.com>
> > ---
> >  include/net/bluetooth/hci_core.h |  30 +++++-
> >  net/bluetooth/hci_conn.c         |  13 ++-
> >  net/bluetooth/hci_event.c        |  41 +++++++-
> >  net/bluetooth/hci_sync.c         |  15 +++
> >  net/bluetooth/iso.c              | 160 ++++++++++++++++++++++++-------
> >  5 files changed, 218 insertions(+), 41 deletions(-)
> >
> > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> > index c53d74236e3a..6fb055e3c595 100644
> > --- a/include/net/bluetooth/hci_core.h
> > +++ b/include/net/bluetooth/hci_core.h
> > @@ -978,6 +978,8 @@ enum {
> >       HCI_CONN_CREATE_CIS,
> >       HCI_CONN_BIG_SYNC,
> >       HCI_CONN_BIG_SYNC_FAILED,
> > +     HCI_CONN_PA_SYNC,
> > +     HCI_CONN_PA_SYNC_FAILED,
> >  };
> >
> >  static inline bool hci_conn_ssp_enabled(struct hci_conn *conn)
> > @@ -1300,7 +1302,7 @@ static inline struct hci_conn *hci_conn_hash_lookup_big_any_dst(struct hci_dev *
> >               if (c->type != ISO_LINK)
> >                       continue;
> >
> > -             if (handle == c->iso_qos.bcast.big) {
> > +             if (handle != BT_ISO_QOS_BIG_UNSET && handle == c->iso_qos.bcast.big) {
> >                       rcu_read_unlock();
> >                       return c;
> >               }
> > @@ -1311,6 +1313,29 @@ static inline struct hci_conn *hci_conn_hash_lookup_big_any_dst(struct hci_dev *
> >       return NULL;
> >  }
> >
> > +static inline struct hci_conn *
> > +hci_conn_hash_lookup_pa_sync(struct hci_dev *hdev, __u8 big)
> > +{
> > +     struct hci_conn_hash *h = &hdev->conn_hash;
> > +     struct hci_conn  *c;
> > +
> > +     rcu_read_lock();
> > +
> > +     list_for_each_entry_rcu(c, &h->list, list) {
> > +             if (c->type != ISO_LINK ||
> > +                     !test_bit(HCI_CONN_PA_SYNC, &c->flags))
> > +                     continue;
> > +
> > +             if (c->iso_qos.bcast.big == big) {
> > +                     rcu_read_unlock();
> > +                     return c;
> > +             }
> > +     }
> > +     rcu_read_unlock();
> > +
> > +     return NULL;
> > +}
> > +
> >  static inline struct hci_conn *hci_conn_hash_lookup_state(struct hci_dev *hdev,
> >                                                       __u8 type, __u16 state)
> >  {
> > @@ -1435,7 +1460,8 @@ struct hci_conn *hci_connect_bis(struct hci_dev *hdev, bdaddr_t *dst,
> >                                __u8 data_len, __u8 *data);
> >  int hci_pa_create_sync(struct hci_dev *hdev, bdaddr_t *dst, __u8 dst_type,
> >                      __u8 sid, struct bt_iso_qos *qos);
> > -int hci_le_big_create_sync(struct hci_dev *hdev, struct bt_iso_qos *qos,
> > +int hci_le_big_create_sync(struct hci_dev *hdev, struct hci_conn *hcon,
> > +                        struct bt_iso_qos *qos,
> >                          __u16 sync_handle, __u8 num_bis, __u8 bis[]);
> >  int hci_conn_check_link_mode(struct hci_conn *conn);
> >  int hci_conn_check_secure(struct hci_conn *conn, __u8 sec_level);
> > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> > index 95339623883c..8b0c8e631324 100644
> > --- a/net/bluetooth/hci_conn.c
> > +++ b/net/bluetooth/hci_conn.c
> > @@ -734,6 +734,7 @@ struct iso_list_data {
> >       };
> >       int count;
> >       bool big_term;
> > +     bool pa_sync_term;
> >       bool big_sync_term;
> >  };
> >
> > @@ -807,7 +808,10 @@ static int big_terminate_sync(struct hci_dev *hdev, void *data)
> >       if (d->big_sync_term)
> >               hci_le_big_terminate_sync(hdev, d->big);
> >
> > -     return hci_le_pa_terminate_sync(hdev, d->sync_handle);
> > +     if (d->pa_sync_term)
> > +             return hci_le_pa_terminate_sync(hdev, d->sync_handle);
> > +
> > +     return 0;
> >  }
> >
> >  static int hci_le_big_terminate(struct hci_dev *hdev, u8 big, struct hci_conn *conn)
> > @@ -823,6 +827,7 @@ static int hci_le_big_terminate(struct hci_dev *hdev, u8 big, struct hci_conn *c
> >
> >       d->big = big;
> >       d->sync_handle = conn->sync_handle;
> > +     d->pa_sync_term = test_and_clear_bit(HCI_CONN_PA_SYNC, &conn->flags);
> >       d->big_sync_term = test_and_clear_bit(HCI_CONN_BIG_SYNC, &conn->flags);
> >
> >       ret = hci_cmd_sync_queue(hdev, big_terminate_sync, d,
> > @@ -2099,7 +2104,8 @@ int hci_pa_create_sync(struct hci_dev *hdev, bdaddr_t *dst, __u8 dst_type,
> >       return hci_cmd_sync_queue(hdev, create_pa_sync, cp, create_pa_complete);
> >  }
> >
> > -int hci_le_big_create_sync(struct hci_dev *hdev, struct bt_iso_qos *qos,
> > +int hci_le_big_create_sync(struct hci_dev *hdev, struct hci_conn *hcon,
> > +                        struct bt_iso_qos *qos,
> >                          __u16 sync_handle, __u8 num_bis, __u8 bis[])
> >  {
> >       struct _packed {
> > @@ -2115,6 +2121,9 @@ int hci_le_big_create_sync(struct hci_dev *hdev, struct bt_iso_qos *qos,
> >       if (err)
> >               return err;
> >
> > +     if (hcon)
> > +             hcon->iso_qos.bcast.big = qos->bcast.big;
> > +
> >       memset(&pdu, 0, sizeof(pdu));
> >       pdu.cp.handle = qos->bcast.big;
> >       pdu.cp.sync_handle = cpu_to_le16(sync_handle);
> > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > index 559b6080706c..b4b72070f5f6 100644
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -6581,20 +6581,39 @@ static void hci_le_pa_sync_estabilished_evt(struct hci_dev *hdev, void *data,
> >       struct hci_ev_le_pa_sync_established *ev = data;
> >       int mask = hdev->link_mode;
> >       __u8 flags = 0;
> > +     struct hci_conn *bis;
> >
> >       bt_dev_dbg(hdev, "status 0x%2.2x", ev->status);
> >
> > -     if (ev->status)
> > -             return;
> > -
> >       hci_dev_lock(hdev);
> >
> >       hci_dev_clear_flag(hdev, HCI_PA_SYNC);
> >
> >       mask |= hci_proto_connect_ind(hdev, &ev->bdaddr, ISO_LINK, &flags);
> > -     if (!(mask & HCI_LM_ACCEPT))
> > +     if (!(mask & HCI_LM_ACCEPT)) {
> >               hci_le_pa_term_sync(hdev, ev->handle);
> > +             goto unlock;
> > +     }
> > +
> > +     if (!(flags & HCI_PROTO_DEFER))
> > +             goto unlock;
> > +
> > +     /* Add connection to indicate the PA sync event */
> > +     bis = hci_conn_add(hdev, ISO_LINK, BDADDR_ANY,
> > +                        HCI_ROLE_SLAVE);
> >
> > +     if (!bis)
> > +             goto unlock;
> > +
> > +     if (ev->status)
> > +             set_bit(HCI_CONN_PA_SYNC_FAILED, &bis->flags);
> > +     else
> > +             set_bit(HCI_CONN_PA_SYNC, &bis->flags);
> > +
> > +     /* Notify connection to iso layer */
> > +     hci_connect_cfm(bis, ev->status);
> > +
> > +unlock:
> >       hci_dev_unlock(hdev);
> >  }
> >
> > @@ -7045,6 +7064,7 @@ static void hci_le_big_sync_established_evt(struct hci_dev *hdev, void *data,
> >  {
> >       struct hci_evt_le_big_sync_estabilished *ev = data;
> >       struct hci_conn *bis;
> > +     struct hci_conn *pa_sync;
> >       int i;
> >
> >       bt_dev_dbg(hdev, "status 0x%2.2x", ev->status);
> > @@ -7055,6 +7075,15 @@ static void hci_le_big_sync_established_evt(struct hci_dev *hdev, void *data,
> >
> >       hci_dev_lock(hdev);
> >
> > +     if (!ev->status) {
> > +             pa_sync = hci_conn_hash_lookup_pa_sync(hdev, ev->handle);
> > +             if (pa_sync)
> > +                     /* Also mark the BIG sync established event on the
> > +                      * associated PA sync hcon
> > +                      */
> > +                     set_bit(HCI_CONN_BIG_SYNC, &pa_sync->flags);
> > +     }
> > +
> >       for (i = 0; i < ev->num_bis; i++) {
> >               u16 handle = le16_to_cpu(ev->bis[i]);
> >               __le32 interval;
> > @@ -7068,6 +7097,10 @@ static void hci_le_big_sync_established_evt(struct hci_dev *hdev, void *data,
> >                       bis->handle = handle;
> >               }
> >
> > +             if (ev->status != 0x42)
> > +                     /* Mark PA sync as established */
> > +                     set_bit(HCI_CONN_PA_SYNC, &bis->flags);
> > +
> >               bis->iso_qos.bcast.big = ev->handle;
> >               memset(&interval, 0, sizeof(interval));
> >               memcpy(&interval, ev->latency, sizeof(ev->latency));
> > diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> > index d10a0f36b947..0cb780817198 100644
> > --- a/net/bluetooth/hci_sync.c
> > +++ b/net/bluetooth/hci_sync.c
> > @@ -5384,6 +5384,21 @@ int hci_abort_conn_sync(struct hci_dev *hdev, struct hci_conn *conn, u8 reason)
> >               err = hci_reject_conn_sync(hdev, conn, reason);
> >               break;
> >       case BT_OPEN:
> > +             hci_dev_lock(hdev);
> > +
> > +             /* Cleanup bis or pa sync connections */
> > +             if (test_and_clear_bit(HCI_CONN_BIG_SYNC_FAILED, &conn->flags) ||
> > +                 test_and_clear_bit(HCI_CONN_PA_SYNC_FAILED, &conn->flags)) {
> > +                     hci_conn_failed(conn, reason);
> > +             } else if (test_bit(HCI_CONN_PA_SYNC, &conn->flags) ||
> > +                        test_bit(HCI_CONN_BIG_SYNC, &conn->flags)) {
> > +                     conn->state = BT_CLOSED;
> > +                     hci_disconn_cfm(conn, reason);
> > +                     hci_conn_del(conn);
> > +             }
> > +
> > +             hci_dev_unlock(hdev);
> > +             return 0;
>
> Note the hci_disconnect_all_sync changes in 45c37c4e9c9aab5b now
> require the conn is deleted in all cases, the above probably results to
> busy loop if non-ISO broadcast conn is in BT_OPEN when controller is
> supended.

Yep, I wonder if we could just keep doing hci_conn_failed, it seems
the only difference seem that hci_disconn_cfm must be called in
certain cases where PA or BIG is being setup over hci_connnect_cfm
which is already handled by hci_conn_failed.

>
> >       case BT_BOUND:
> >               hci_dev_lock(hdev);
> >               hci_conn_failed(conn, reason);
> > diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
> > index 6b66d6a88b9a..9879f2349d48 100644
> > --- a/net/bluetooth/iso.c
> > +++ b/net/bluetooth/iso.c
> > @@ -51,6 +51,7 @@ static void iso_sock_kill(struct sock *sk);
> >  /* iso_pinfo flags values */
> >  enum {
> >       BT_SK_BIG_SYNC,
> > +     BT_SK_PA_SYNC,
> >  };
> >
> >  struct iso_pinfo {
> > @@ -75,6 +76,8 @@ static struct bt_iso_qos default_qos;
> >
> >  static bool check_ucast_qos(struct bt_iso_qos *qos);
> >  static bool check_bcast_qos(struct bt_iso_qos *qos);
> > +static bool iso_match_sid(struct sock *sk, void *data);
> > +static void iso_sock_disconn(struct sock *sk);
> >
> >  /* ---- ISO timers ---- */
> >  #define ISO_CONN_TIMEOUT     (HZ * 40)
> > @@ -598,6 +601,15 @@ static void iso_sock_cleanup_listen(struct sock *parent)
> >               iso_sock_kill(sk);
> >       }
> >
> > +     /* If listening socket stands for a PA sync connection,
> > +      * properly disconnect the hcon and socket.
> > +      */
> > +     if (iso_pi(parent)->conn && iso_pi(parent)->conn->hcon &&
> > +         test_bit(HCI_CONN_PA_SYNC, &iso_pi(parent)->conn->hcon->flags)) {
> > +             iso_sock_disconn(parent);
> > +             return;
> > +     }
> > +
> >       parent->sk_state  = BT_CLOSED;
> >       sock_set_flag(parent, SOCK_ZAPPED);
> >  }
> > @@ -619,6 +631,16 @@ static void iso_sock_kill(struct sock *sk)
> >       sock_put(sk);
> >  }
> >
> > +static void iso_sock_disconn(struct sock *sk)
> > +{
> > +     sk->sk_state = BT_DISCONN;
> > +     iso_sock_set_timer(sk, ISO_DISCONN_TIMEOUT);
> > +     iso_conn_lock(iso_pi(sk)->conn);
> > +     hci_conn_drop(iso_pi(sk)->conn->hcon);
> > +     iso_pi(sk)->conn->hcon = NULL;
> > +     iso_conn_unlock(iso_pi(sk)->conn);
> > +}
> > +
> >  static void __iso_sock_close(struct sock *sk)
> >  {
> >       BT_DBG("sk %p state %d socket %p", sk, sk->sk_state, sk->sk_socket);
> > @@ -631,20 +653,19 @@ static void __iso_sock_close(struct sock *sk)
> >       case BT_CONNECT:
> >       case BT_CONNECTED:
> >       case BT_CONFIG:
> > -             if (iso_pi(sk)->conn->hcon) {
> > -                     sk->sk_state = BT_DISCONN;
> > -                     iso_sock_set_timer(sk, ISO_DISCONN_TIMEOUT);
> > -                     iso_conn_lock(iso_pi(sk)->conn);
> > -                     hci_conn_drop(iso_pi(sk)->conn->hcon);
> > -                     iso_pi(sk)->conn->hcon = NULL;
> > -                     iso_conn_unlock(iso_pi(sk)->conn);
> > -             } else {
> > +             if (iso_pi(sk)->conn->hcon)
> > +                     iso_sock_disconn(sk);
> > +             else
> >                       iso_chan_del(sk, ECONNRESET);
> > -             }
> >               break;
> >
> >       case BT_CONNECT2:
> > -             iso_chan_del(sk, ECONNRESET);
> > +             if (iso_pi(sk)->conn->hcon &&
> > +                 (test_bit(HCI_CONN_PA_SYNC, &iso_pi(sk)->conn->hcon->flags) ||
> > +                 test_bit(HCI_CONN_PA_SYNC_FAILED, &iso_pi(sk)->conn->hcon->flags)))
> > +                     iso_sock_disconn(sk);
> > +             else
> > +                     iso_chan_del(sk, ECONNRESET);
> >               break;
> >       case BT_DISCONN:
> >               iso_chan_del(sk, ECONNRESET);
> > @@ -1139,6 +1160,29 @@ static void iso_conn_defer_accept(struct hci_conn *conn)
> >       hci_send_cmd(hdev, HCI_OP_LE_ACCEPT_CIS, sizeof(cp), &cp);
> >  }
> >
> > +static void iso_conn_big_sync(struct sock *sk)
> > +{
> > +     int err;
> > +     struct hci_dev *hdev;
> > +
> > +     hdev = hci_get_route(&iso_pi(sk)->dst, &iso_pi(sk)->src,
> > +                          iso_pi(sk)->src_type);
> > +
> > +     if (!hdev)
> > +             return;
> > +
> > +     if (!test_and_set_bit(BT_SK_BIG_SYNC, &iso_pi(sk)->flags)) {
> > +             err = hci_le_big_create_sync(hdev, iso_pi(sk)->conn->hcon,
> > +                                          &iso_pi(sk)->qos,
> > +                                          iso_pi(sk)->sync_handle,
> > +                                          iso_pi(sk)->bc_num_bis,
> > +                                          iso_pi(sk)->bc_bis);
> > +             if (err)
> > +                     bt_dev_err(hdev, "hci_le_big_create_sync: %d",
> > +                                err);
> > +     }
> > +}
> > +
> >  static int iso_sock_recvmsg(struct socket *sock, struct msghdr *msg,
> >                           size_t len, int flags)
> >  {
> > @@ -1151,8 +1195,15 @@ static int iso_sock_recvmsg(struct socket *sock, struct msghdr *msg,
> >               lock_sock(sk);
> >               switch (sk->sk_state) {
> >               case BT_CONNECT2:
> > -                     iso_conn_defer_accept(pi->conn->hcon);
> > -                     sk->sk_state = BT_CONFIG;
> > +                     if (pi->conn->hcon &&
> > +                         test_bit(HCI_CONN_PA_SYNC, &pi->conn->hcon->flags)) {
> > +                             iso_conn_big_sync(sk);
> > +                             sk->sk_state = BT_LISTEN;
> > +                             set_bit(BT_SK_PA_SYNC, &iso_pi(sk)->flags);
> > +                     } else {
> > +                             iso_conn_defer_accept(pi->conn->hcon);
> > +                             sk->sk_state = BT_CONFIG;
> > +                     }
> >                       release_sock(sk);
> >                       return 0;
> >               case BT_CONNECT:
> > @@ -1513,11 +1564,17 @@ static bool iso_match_big(struct sock *sk, void *data)
> >       return ev->handle == iso_pi(sk)->qos.bcast.big;
> >  }
> >
> > +static bool iso_match_pa_sync_flag(struct sock *sk, void *data)
> > +{
> > +     return test_bit(BT_SK_PA_SYNC, &iso_pi(sk)->flags);
> > +}
> > +
> >  static void iso_conn_ready(struct iso_conn *conn)
> >  {
> > -     struct sock *parent;
> > +     struct sock *parent = NULL;
> >       struct sock *sk = conn->sk;
> > -     struct hci_ev_le_big_sync_estabilished *ev;
> > +     struct hci_ev_le_big_sync_estabilished *ev = NULL;
> > +     struct hci_ev_le_pa_sync_established *ev2 = NULL;
> >       struct hci_conn *hcon;
> >
> >       BT_DBG("conn %p", conn);
> > @@ -1529,15 +1586,32 @@ static void iso_conn_ready(struct iso_conn *conn)
> >               if (!hcon)
> >                       return;
> >
> > -             ev = hci_recv_event_data(hcon->hdev,
> > -                                      HCI_EVT_LE_BIG_SYNC_ESTABILISHED);
> > -             if (ev)
> > +             if (test_bit(HCI_CONN_BIG_SYNC, &hcon->flags) ||
> > +                 test_bit(HCI_CONN_BIG_SYNC_FAILED, &hcon->flags)) {
> > +                     ev = hci_recv_event_data(hcon->hdev,
> > +                                              HCI_EVT_LE_BIG_SYNC_ESTABILISHED);
> > +
> > +                     /* Get reference to PA sync parent socket, if it exists */
> >                       parent = iso_get_sock_listen(&hcon->src,
> >                                                    &hcon->dst,
> > -                                                  iso_match_big, ev);
> > -             else
> > +                                                  iso_match_pa_sync_flag, NULL);
> > +                     if (!parent && ev)
> > +                             parent = iso_get_sock_listen(&hcon->src,
> > +                                                          &hcon->dst,
> > +                                                          iso_match_big, ev);
> > +             } else if (test_bit(HCI_CONN_PA_SYNC, &hcon->flags) ||
> > +                             test_bit(HCI_CONN_PA_SYNC_FAILED, &hcon->flags)) {
> > +                     ev2 = hci_recv_event_data(hcon->hdev,
> > +                                               HCI_EV_LE_PA_SYNC_ESTABLISHED);
> > +                     if (ev2)
> > +                             parent = iso_get_sock_listen(&hcon->src,
> > +                                                          &hcon->dst,
> > +                                                          iso_match_sid, ev2);
> > +             }
> > +
> > +             if (!parent)
> >                       parent = iso_get_sock_listen(&hcon->src,
> > -                                                  BDADDR_ANY, NULL, NULL);
> > +                                                     BDADDR_ANY, NULL, NULL);
> >
> >               if (!parent)
> >                       return;
> > @@ -1554,11 +1628,17 @@ static void iso_conn_ready(struct iso_conn *conn)
> >               iso_sock_init(sk, parent);
> >
> >               bacpy(&iso_pi(sk)->src, &hcon->src);
> > -             iso_pi(sk)->src_type = hcon->src_type;
> > +
> > +             /* Convert from HCI to three-value type */
> > +             if (hcon->src_type == ADDR_LE_DEV_PUBLIC)
> > +                     iso_pi(sk)->src_type = BDADDR_LE_PUBLIC;
> > +             else
> > +                     iso_pi(sk)->src_type = BDADDR_LE_RANDOM;
> >
> >               /* If hcon has no destination address (BDADDR_ANY) it means it
> > -              * was created by HCI_EV_LE_BIG_SYNC_ESTABILISHED so we need to
> > -              * initialize using the parent socket destination address.
> > +              * was created by HCI_EV_LE_BIG_SYNC_ESTABILISHED or
> > +              * HCI_EV_LE_PA_SYNC_ESTABLISHED so we need to initialize using
> > +              * the parent socket destination address.
> >                */
> >               if (!bacmp(&hcon->dst, BDADDR_ANY)) {
> >                       bacpy(&hcon->dst, &iso_pi(parent)->dst);
> > @@ -1566,13 +1646,21 @@ static void iso_conn_ready(struct iso_conn *conn)
> >                       hcon->sync_handle = iso_pi(parent)->sync_handle;
> >               }
> >
> > +             if (ev2 && !ev2->status) {
> > +                     iso_pi(sk)->sync_handle = iso_pi(parent)->sync_handle;
> > +                     iso_pi(sk)->qos = iso_pi(parent)->qos;
> > +                     iso_pi(sk)->bc_num_bis = iso_pi(parent)->bc_num_bis;
> > +                     memcpy(iso_pi(sk)->bc_bis, iso_pi(parent)->bc_bis, ISO_MAX_NUM_BIS);
> > +             }
> > +
> >               bacpy(&iso_pi(sk)->dst, &hcon->dst);
> >               iso_pi(sk)->dst_type = hcon->dst_type;
> >
> >               hci_conn_hold(hcon);
> >               iso_chan_add(conn, sk, parent);
> >
> > -             if (ev && ((struct hci_evt_le_big_sync_estabilished *)ev)->status) {
> > +             if ((ev && ((struct hci_evt_le_big_sync_estabilished *)ev)->status) ||
> > +                 (ev2 && ev2->status)) {
> >                       /* Trigger error signal on child socket */
> >                       sk->sk_err = ECONNREFUSED;
> >                       sk->sk_error_report(sk);
> > @@ -1630,7 +1718,7 @@ int iso_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags)
> >       if (ev1) {
> >               sk = iso_get_sock_listen(&hdev->bdaddr, bdaddr, iso_match_sid,
> >                                        ev1);
> > -             if (sk)
> > +             if (sk && !ev1->status)
> >                       iso_pi(sk)->sync_handle = le16_to_cpu(ev1->handle);
> >
> >               goto done;
> > @@ -1638,16 +1726,21 @@ int iso_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags)
> >
> >       ev2 = hci_recv_event_data(hdev, HCI_EVT_LE_BIG_INFO_ADV_REPORT);
> >       if (ev2) {
> > +             /* Try to get PA sync listening socket, if it exists */
> >               sk = iso_get_sock_listen(&hdev->bdaddr, bdaddr,
> > -                                      iso_match_sync_handle, ev2);
> > +                                             iso_match_pa_sync_flag, NULL);
> > +             if (!sk)
> > +                     sk = iso_get_sock_listen(&hdev->bdaddr, bdaddr,
> > +                                              iso_match_sync_handle, ev2);
> >               if (sk) {
> >                       int err;
> >
> >                       if (ev2->num_bis < iso_pi(sk)->bc_num_bis)
> >                               iso_pi(sk)->bc_num_bis = ev2->num_bis;
> >
> > -                     if (!test_and_set_bit(BT_SK_BIG_SYNC, &iso_pi(sk)->flags)) {
> > -                             err = hci_le_big_create_sync(hdev,
> > +                     if (!test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags) &&
> > +                         !test_and_set_bit(BT_SK_BIG_SYNC, &iso_pi(sk)->flags)) {
> > +                             err = hci_le_big_create_sync(hdev, NULL,
> >                                                            &iso_pi(sk)->qos,
> >                                                            iso_pi(sk)->sync_handle,
> >                                                            iso_pi(sk)->bc_num_bis,
> > @@ -1699,12 +1792,13 @@ static void iso_connect_cfm(struct hci_conn *hcon, __u8 status)
> >
> >       BT_DBG("hcon %p bdaddr %pMR status %d", hcon, &hcon->dst, status);
> >
> > -     /* Similar to the success case, if HCI_CONN_BIG_SYNC_FAILED is set,
> > -      * queue the failed bis connection into the accept queue of the
> > -      * listening socket and wake up userspace, to inform the user about
> > -      * the BIG sync failed event.
> > +     /* Similar to the success case, if HCI_CONN_BIG_SYNC_FAILED or
> > +      * HCI_CONN_PA_SYNC_FAILED is set, queue the failed connection
> > +      * into the accept queue of the listening socket and wake up
> > +      * userspace, to inform the user about the event.
> >        */
> > -     if (!status || test_bit(HCI_CONN_BIG_SYNC_FAILED, &hcon->flags)) {
> > +     if (!status || test_bit(HCI_CONN_BIG_SYNC_FAILED, &hcon->flags) ||
> > +         test_bit(HCI_CONN_PA_SYNC_FAILED, &hcon->flags)) {
> >               struct iso_conn *conn;
> >
> >               conn = iso_conn_add(hcon);
>
> --
> Pauli Virtanen



-- 
Luiz Augusto von Dentz

Re: [PATCH v3 1/1] Bluetooth: ISO: Use defer setup to separate PA sync and BIG sync

[Iulia Tanasescu]

Hi Luiz, Pauli

> -----Original Message-----
> From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
> Sent: Friday, August 18, 2023 12:39 AM
> To: Pauli Virtanen <pav@iki.fi>
> Cc: Iulia Tanasescu <iulia.tanasescu@nxp.com>; linux-
> bluetooth@vger.kernel.org
> Subject: Re: [PATCH v3 1/1] Bluetooth: ISO: Use defer setup to
> separate PA sync and BIG sync
> 
> Hi Pauli,
> 
> On Thu, Aug 17, 2023 at 1:26 PM Pauli Virtanen <pav@iki.fi> wrote:
> >
> > Hi,
> >
> > to, 2023-08-17 kello 09:44 +0300, Iulia Tanasescu kirjoitti:
> > > This commit implements defer setup support for the Broadcast Sink
> > > scenario: By setting defer setup on a broadcast socket before
> > > calling listen, the user is able to trigger the PA sync and BIG sync
> > > procedures separately.
> > >
> > > This is useful if the user first wants to synchronize to the
> > > periodic advertising transmitted by a Broadcast Source, and trigger
> > > the BIG sync procedure later on.
> > >
> > > If defer setup is set, once a PA sync established event arrives, a
> > > new hcon is created and notified to the ISO layer. A child socket
> > > associated with the PA sync connection will be added to the accept
> > > queue of the listening socket.
> > >
> > > Once the accept call returns the fd for the PA sync child socket,
> > > the user should call read on that fd. This will trigger the BIG
> > > create sync procedure, and the PA sync socket will become a listening
> socket itself.
> > >
> > > When the BIG sync established event is notified to the ISO layer,
> > > the bis connections will be added to the accept queue of the PA sync
> parent.
> > > The user should call accept on the PA sync socket to get the final
> > > bis connections.
> > >
> > > Signed-off-by: Iulia Tanasescu <iulia.tanasescu@nxp.com>
> > > ---
> > >  include/net/bluetooth/hci_core.h |  30 +++++-
> > >  net/bluetooth/hci_conn.c         |  13 ++-
> > >  net/bluetooth/hci_event.c        |  41 +++++++-
> > >  net/bluetooth/hci_sync.c         |  15 +++
> > >  net/bluetooth/iso.c              | 160 ++++++++++++++++++++++++-------
> > >  5 files changed, 218 insertions(+), 41 deletions(-)
> > >
> > > diff --git a/include/net/bluetooth/hci_core.h
> > > b/include/net/bluetooth/hci_core.h
> > > index c53d74236e3a..6fb055e3c595 100644
> > > --- a/include/net/bluetooth/hci_core.h
> > > +++ b/include/net/bluetooth/hci_core.h
> > > @@ -978,6 +978,8 @@ enum {
> > >       HCI_CONN_CREATE_CIS,
> > >       HCI_CONN_BIG_SYNC,
> > >       HCI_CONN_BIG_SYNC_FAILED,
> > > +     HCI_CONN_PA_SYNC,
> > > +     HCI_CONN_PA_SYNC_FAILED,
> > >  };
> > >
> > >  static inline bool hci_conn_ssp_enabled(struct hci_conn *conn) @@
> > > -1300,7 +1302,7 @@ static inline struct hci_conn
> *hci_conn_hash_lookup_big_any_dst(struct hci_dev *
> > >               if (c->type != ISO_LINK)
> > >                       continue;
> > >
> > > -             if (handle == c->iso_qos.bcast.big) {
> > > +             if (handle != BT_ISO_QOS_BIG_UNSET && handle ==
> > > + c->iso_qos.bcast.big) {
> > >                       rcu_read_unlock();
> > >                       return c;
> > >               }
> > > @@ -1311,6 +1313,29 @@ static inline struct hci_conn
> *hci_conn_hash_lookup_big_any_dst(struct hci_dev *
> > >       return NULL;
> > >  }
> > >
> > > +static inline struct hci_conn *
> > > +hci_conn_hash_lookup_pa_sync(struct hci_dev *hdev, __u8 big) {
> > > +     struct hci_conn_hash *h = &hdev->conn_hash;
> > > +     struct hci_conn  *c;
> > > +
> > > +     rcu_read_lock();
> > > +
> > > +     list_for_each_entry_rcu(c, &h->list, list) {
> > > +             if (c->type != ISO_LINK ||
> > > +                     !test_bit(HCI_CONN_PA_SYNC, &c->flags))
> > > +                     continue;
> > > +
> > > +             if (c->iso_qos.bcast.big == big) {
> > > +                     rcu_read_unlock();
> > > +                     return c;
> > > +             }
> > > +     }
> > > +     rcu_read_unlock();
> > > +
> > > +     return NULL;
> > > +}
> > > +
> > >  static inline struct hci_conn *hci_conn_hash_lookup_state(struct
> hci_dev *hdev,
> > >                                                       __u8 type,
> > > __u16 state)  { @@ -1435,7 +1460,8 @@ struct hci_conn
> > > *hci_connect_bis(struct hci_dev *hdev, bdaddr_t *dst,
> > >                                __u8 data_len, __u8 *data);  int
> > > hci_pa_create_sync(struct hci_dev *hdev, bdaddr_t *dst, __u8 dst_type,
> > >                      __u8 sid, struct bt_iso_qos *qos); -int
> > > hci_le_big_create_sync(struct hci_dev *hdev, struct bt_iso_qos *qos,
> > > +int hci_le_big_create_sync(struct hci_dev *hdev, struct hci_conn *hcon,
> > > +                        struct bt_iso_qos *qos,
> > >                          __u16 sync_handle, __u8 num_bis, __u8
> > > bis[]);  int hci_conn_check_link_mode(struct hci_conn *conn);  int
> > > hci_conn_check_secure(struct hci_conn *conn, __u8 sec_level); diff
> > > --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index
> > > 95339623883c..8b0c8e631324 100644
> > > --- a/net/bluetooth/hci_conn.c
> > > +++ b/net/bluetooth/hci_conn.c
> > > @@ -734,6 +734,7 @@ struct iso_list_data {
> > >       };
> > >       int count;
> > >       bool big_term;
> > > +     bool pa_sync_term;
> > >       bool big_sync_term;
> > >  };
> > >
> > > @@ -807,7 +808,10 @@ static int big_terminate_sync(struct hci_dev
> *hdev, void *data)
> > >       if (d->big_sync_term)
> > >               hci_le_big_terminate_sync(hdev, d->big);
> > >
> > > -     return hci_le_pa_terminate_sync(hdev, d->sync_handle);
> > > +     if (d->pa_sync_term)
> > > +             return hci_le_pa_terminate_sync(hdev, d->sync_handle);
> > > +
> > > +     return 0;
> > >  }
> > >
> > >  static int hci_le_big_terminate(struct hci_dev *hdev, u8 big,
> > > struct hci_conn *conn) @@ -823,6 +827,7 @@ static int
> > > hci_le_big_terminate(struct hci_dev *hdev, u8 big, struct hci_conn
> > > *c
> > >
> > >       d->big = big;
> > >       d->sync_handle = conn->sync_handle;
> > > +     d->pa_sync_term = test_and_clear_bit(HCI_CONN_PA_SYNC,
> > > + &conn->flags);
> > >       d->big_sync_term = test_and_clear_bit(HCI_CONN_BIG_SYNC,
> > > &conn->flags);
> > >
> > >       ret = hci_cmd_sync_queue(hdev, big_terminate_sync, d, @@
> > > -2099,7 +2104,8 @@ int hci_pa_create_sync(struct hci_dev *hdev,
> bdaddr_t *dst, __u8 dst_type,
> > >       return hci_cmd_sync_queue(hdev, create_pa_sync, cp,
> > > create_pa_complete);  }
> > >
> > > -int hci_le_big_create_sync(struct hci_dev *hdev, struct bt_iso_qos
> > > *qos,
> > > +int hci_le_big_create_sync(struct hci_dev *hdev, struct hci_conn *hcon,
> > > +                        struct bt_iso_qos *qos,
> > >                          __u16 sync_handle, __u8 num_bis, __u8
> > > bis[])  {
> > >       struct _packed {
> > > @@ -2115,6 +2121,9 @@ int hci_le_big_create_sync(struct hci_dev
> *hdev, struct bt_iso_qos *qos,
> > >       if (err)
> > >               return err;
> > >
> > > +     if (hcon)
> > > +             hcon->iso_qos.bcast.big = qos->bcast.big;
> > > +
> > >       memset(&pdu, 0, sizeof(pdu));
> > >       pdu.cp.handle = qos->bcast.big;
> > >       pdu.cp.sync_handle = cpu_to_le16(sync_handle); diff --git
> > > a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index
> > > 559b6080706c..b4b72070f5f6 100644
> > > --- a/net/bluetooth/hci_event.c
> > > +++ b/net/bluetooth/hci_event.c
> > > @@ -6581,20 +6581,39 @@ static void
> hci_le_pa_sync_estabilished_evt(struct hci_dev *hdev, void *data,
> > >       struct hci_ev_le_pa_sync_established *ev = data;
> > >       int mask = hdev->link_mode;
> > >       __u8 flags = 0;
> > > +     struct hci_conn *bis;
> > >
> > >       bt_dev_dbg(hdev, "status 0x%2.2x", ev->status);
> > >
> > > -     if (ev->status)
> > > -             return;
> > > -
> > >       hci_dev_lock(hdev);
> > >
> > >       hci_dev_clear_flag(hdev, HCI_PA_SYNC);
> > >
> > >       mask |= hci_proto_connect_ind(hdev, &ev->bdaddr, ISO_LINK,
> &flags);
> > > -     if (!(mask & HCI_LM_ACCEPT))
> > > +     if (!(mask & HCI_LM_ACCEPT)) {
> > >               hci_le_pa_term_sync(hdev, ev->handle);
> > > +             goto unlock;
> > > +     }
> > > +
> > > +     if (!(flags & HCI_PROTO_DEFER))
> > > +             goto unlock;
> > > +
> > > +     /* Add connection to indicate the PA sync event */
> > > +     bis = hci_conn_add(hdev, ISO_LINK, BDADDR_ANY,
> > > +                        HCI_ROLE_SLAVE);
> > >
> > > +     if (!bis)
> > > +             goto unlock;
> > > +
> > > +     if (ev->status)
> > > +             set_bit(HCI_CONN_PA_SYNC_FAILED, &bis->flags);
> > > +     else
> > > +             set_bit(HCI_CONN_PA_SYNC, &bis->flags);
> > > +
> > > +     /* Notify connection to iso layer */
> > > +     hci_connect_cfm(bis, ev->status);
> > > +
> > > +unlock:
> > >       hci_dev_unlock(hdev);
> > >  }
> > >
> > > @@ -7045,6 +7064,7 @@ static void
> > > hci_le_big_sync_established_evt(struct hci_dev *hdev, void *data,  {
> > >       struct hci_evt_le_big_sync_estabilished *ev = data;
> > >       struct hci_conn *bis;
> > > +     struct hci_conn *pa_sync;
> > >       int i;
> > >
> > >       bt_dev_dbg(hdev, "status 0x%2.2x", ev->status); @@ -7055,6
> > > +7075,15 @@ static void hci_le_big_sync_established_evt(struct
> > > hci_dev *hdev, void *data,
> > >
> > >       hci_dev_lock(hdev);
> > >
> > > +     if (!ev->status) {
> > > +             pa_sync = hci_conn_hash_lookup_pa_sync(hdev, ev->handle);
> > > +             if (pa_sync)
> > > +                     /* Also mark the BIG sync established event on the
> > > +                      * associated PA sync hcon
> > > +                      */
> > > +                     set_bit(HCI_CONN_BIG_SYNC, &pa_sync->flags);
> > > +     }
> > > +
> > >       for (i = 0; i < ev->num_bis; i++) {
> > >               u16 handle = le16_to_cpu(ev->bis[i]);
> > >               __le32 interval;
> > > @@ -7068,6 +7097,10 @@ static void
> hci_le_big_sync_established_evt(struct hci_dev *hdev, void *data,
> > >                       bis->handle = handle;
> > >               }
> > >
> > > +             if (ev->status != 0x42)
> > > +                     /* Mark PA sync as established */
> > > +                     set_bit(HCI_CONN_PA_SYNC, &bis->flags);
> > > +
> > >               bis->iso_qos.bcast.big = ev->handle;
> > >               memset(&interval, 0, sizeof(interval));
> > >               memcpy(&interval, ev->latency, sizeof(ev->latency));
> > > diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> > > index d10a0f36b947..0cb780817198 100644
> > > --- a/net/bluetooth/hci_sync.c
> > > +++ b/net/bluetooth/hci_sync.c
> > > @@ -5384,6 +5384,21 @@ int hci_abort_conn_sync(struct hci_dev *hdev,
> struct hci_conn *conn, u8 reason)
> > >               err = hci_reject_conn_sync(hdev, conn, reason);
> > >               break;
> > >       case BT_OPEN:
> > > +             hci_dev_lock(hdev);
> > > +
> > > +             /* Cleanup bis or pa sync connections */
> > > +             if (test_and_clear_bit(HCI_CONN_BIG_SYNC_FAILED, &conn-
> >flags) ||
> > > +                 test_and_clear_bit(HCI_CONN_PA_SYNC_FAILED, &conn-
> >flags)) {
> > > +                     hci_conn_failed(conn, reason);
> > > +             } else if (test_bit(HCI_CONN_PA_SYNC, &conn->flags) ||
> > > +                        test_bit(HCI_CONN_BIG_SYNC, &conn->flags)) {
> > > +                     conn->state = BT_CLOSED;
> > > +                     hci_disconn_cfm(conn, reason);
> > > +                     hci_conn_del(conn);
> > > +             }
> > > +
> > > +             hci_dev_unlock(hdev);
> > > +             return 0;
> >
> > Note the hci_disconnect_all_sync changes in 45c37c4e9c9aab5b now
> > require the conn is deleted in all cases, the above probably results
> > to busy loop if non-ISO broadcast conn is in BT_OPEN when controller
> > is supended.
> 
> Yep, I wonder if we could just keep doing hci_conn_failed, it seems the only
> difference seem that hci_disconn_cfm must be called in certain cases where
> PA or BIG is being setup over hci_connnect_cfm which is already handled by
> hci_conn_failed.
> 

I agree, we could just keep hci_conn_failed, since both hci_disconn_cfm
and hci_connect_cfm with error status will have the same effect on ISO
layer. I decided to use hci_disconn_cfm for the successful connections
just because I thought it might be confusing to use hci_conn_failed for
those cases. I can submit a new patch to only use hci_conn_failed like
before, if you agree.

> >
> > >       case BT_BOUND:
> > >               hci_dev_lock(hdev);
> > >               hci_conn_failed(conn, reason); diff --git
> > > a/net/bluetooth/iso.c b/net/bluetooth/iso.c index
> > > 6b66d6a88b9a..9879f2349d48 100644
> > > --- a/net/bluetooth/iso.c
> > > +++ b/net/bluetooth/iso.c
> > > @@ -51,6 +51,7 @@ static void iso_sock_kill(struct sock *sk);
> > >  /* iso_pinfo flags values */
> > >  enum {
> > >       BT_SK_BIG_SYNC,
> > > +     BT_SK_PA_SYNC,
> > >  };
> > >
> > >  struct iso_pinfo {
> > > @@ -75,6 +76,8 @@ static struct bt_iso_qos default_qos;
> > >
> > >  static bool check_ucast_qos(struct bt_iso_qos *qos);  static bool
> > > check_bcast_qos(struct bt_iso_qos *qos);
> > > +static bool iso_match_sid(struct sock *sk, void *data); static void
> > > +iso_sock_disconn(struct sock *sk);
> > >
> > >  /* ---- ISO timers ---- */
> > >  #define ISO_CONN_TIMEOUT     (HZ * 40)
> > > @@ -598,6 +601,15 @@ static void iso_sock_cleanup_listen(struct sock
> *parent)
> > >               iso_sock_kill(sk);
> > >       }
> > >
> > > +     /* If listening socket stands for a PA sync connection,
> > > +      * properly disconnect the hcon and socket.
> > > +      */
> > > +     if (iso_pi(parent)->conn && iso_pi(parent)->conn->hcon &&
> > > +         test_bit(HCI_CONN_PA_SYNC, &iso_pi(parent)->conn->hcon-
> >flags)) {
> > > +             iso_sock_disconn(parent);
> > > +             return;
> > > +     }
> > > +
> > >       parent->sk_state  = BT_CLOSED;
> > >       sock_set_flag(parent, SOCK_ZAPPED);  } @@ -619,6 +631,16 @@
> > > static void iso_sock_kill(struct sock *sk)
> > >       sock_put(sk);
> > >  }
> > >
> > > +static void iso_sock_disconn(struct sock *sk) {
> > > +     sk->sk_state = BT_DISCONN;
> > > +     iso_sock_set_timer(sk, ISO_DISCONN_TIMEOUT);
> > > +     iso_conn_lock(iso_pi(sk)->conn);
> > > +     hci_conn_drop(iso_pi(sk)->conn->hcon);
> > > +     iso_pi(sk)->conn->hcon = NULL;
> > > +     iso_conn_unlock(iso_pi(sk)->conn);
> > > +}
> > > +
> > >  static void __iso_sock_close(struct sock *sk)  {
> > >       BT_DBG("sk %p state %d socket %p", sk, sk->sk_state,
> > > sk->sk_socket); @@ -631,20 +653,19 @@ static void
> __iso_sock_close(struct sock *sk)
> > >       case BT_CONNECT:
> > >       case BT_CONNECTED:
> > >       case BT_CONFIG:
> > > -             if (iso_pi(sk)->conn->hcon) {
> > > -                     sk->sk_state = BT_DISCONN;
> > > -                     iso_sock_set_timer(sk, ISO_DISCONN_TIMEOUT);
> > > -                     iso_conn_lock(iso_pi(sk)->conn);
> > > -                     hci_conn_drop(iso_pi(sk)->conn->hcon);
> > > -                     iso_pi(sk)->conn->hcon = NULL;
> > > -                     iso_conn_unlock(iso_pi(sk)->conn);
> > > -             } else {
> > > +             if (iso_pi(sk)->conn->hcon)
> > > +                     iso_sock_disconn(sk);
> > > +             else
> > >                       iso_chan_del(sk, ECONNRESET);
> > > -             }
> > >               break;
> > >
> > >       case BT_CONNECT2:
> > > -             iso_chan_del(sk, ECONNRESET);
> > > +             if (iso_pi(sk)->conn->hcon &&
> > > +                 (test_bit(HCI_CONN_PA_SYNC, &iso_pi(sk)->conn->hcon-
> >flags) ||
> > > +                 test_bit(HCI_CONN_PA_SYNC_FAILED, &iso_pi(sk)->conn-
> >hcon->flags)))
> > > +                     iso_sock_disconn(sk);
> > > +             else
> > > +                     iso_chan_del(sk, ECONNRESET);
> > >               break;
> > >       case BT_DISCONN:
> > >               iso_chan_del(sk, ECONNRESET); @@ -1139,6 +1160,29 @@
> > > static void iso_conn_defer_accept(struct hci_conn *conn)
> > >       hci_send_cmd(hdev, HCI_OP_LE_ACCEPT_CIS, sizeof(cp), &cp);  }
> > >
> > > +static void iso_conn_big_sync(struct sock *sk) {
> > > +     int err;
> > > +     struct hci_dev *hdev;
> > > +
> > > +     hdev = hci_get_route(&iso_pi(sk)->dst, &iso_pi(sk)->src,
> > > +                          iso_pi(sk)->src_type);
> > > +
> > > +     if (!hdev)
> > > +             return;
> > > +
> > > +     if (!test_and_set_bit(BT_SK_BIG_SYNC, &iso_pi(sk)->flags)) {
> > > +             err = hci_le_big_create_sync(hdev, iso_pi(sk)->conn->hcon,
> > > +                                          &iso_pi(sk)->qos,
> > > +                                          iso_pi(sk)->sync_handle,
> > > +                                          iso_pi(sk)->bc_num_bis,
> > > +                                          iso_pi(sk)->bc_bis);
> > > +             if (err)
> > > +                     bt_dev_err(hdev, "hci_le_big_create_sync: %d",
> > > +                                err);
> > > +     }
> > > +}
> > > +
> > >  static int iso_sock_recvmsg(struct socket *sock, struct msghdr *msg,
> > >                           size_t len, int flags)  { @@ -1151,8
> > > +1195,15 @@ static int iso_sock_recvmsg(struct socket *sock, struct
> msghdr *msg,
> > >               lock_sock(sk);
> > >               switch (sk->sk_state) {
> > >               case BT_CONNECT2:
> > > -                     iso_conn_defer_accept(pi->conn->hcon);
> > > -                     sk->sk_state = BT_CONFIG;
> > > +                     if (pi->conn->hcon &&
> > > +                         test_bit(HCI_CONN_PA_SYNC, &pi->conn->hcon->flags)) {
> > > +                             iso_conn_big_sync(sk);
> > > +                             sk->sk_state = BT_LISTEN;
> > > +                             set_bit(BT_SK_PA_SYNC, &iso_pi(sk)->flags);
> > > +                     } else {
> > > +                             iso_conn_defer_accept(pi->conn->hcon);
> > > +                             sk->sk_state = BT_CONFIG;
> > > +                     }
> > >                       release_sock(sk);
> > >                       return 0;
> > >               case BT_CONNECT:
> > > @@ -1513,11 +1564,17 @@ static bool iso_match_big(struct sock *sk,
> void *data)
> > >       return ev->handle == iso_pi(sk)->qos.bcast.big;  }
> > >
> > > +static bool iso_match_pa_sync_flag(struct sock *sk, void *data) {
> > > +     return test_bit(BT_SK_PA_SYNC, &iso_pi(sk)->flags); }
> > > +
> > >  static void iso_conn_ready(struct iso_conn *conn)  {
> > > -     struct sock *parent;
> > > +     struct sock *parent = NULL;
> > >       struct sock *sk = conn->sk;
> > > -     struct hci_ev_le_big_sync_estabilished *ev;
> > > +     struct hci_ev_le_big_sync_estabilished *ev = NULL;
> > > +     struct hci_ev_le_pa_sync_established *ev2 = NULL;
> > >       struct hci_conn *hcon;
> > >
> > >       BT_DBG("conn %p", conn);
> > > @@ -1529,15 +1586,32 @@ static void iso_conn_ready(struct iso_conn
> *conn)
> > >               if (!hcon)
> > >                       return;
> > >
> > > -             ev = hci_recv_event_data(hcon->hdev,
> > > -                                      HCI_EVT_LE_BIG_SYNC_ESTABILISHED);
> > > -             if (ev)
> > > +             if (test_bit(HCI_CONN_BIG_SYNC, &hcon->flags) ||
> > > +                 test_bit(HCI_CONN_BIG_SYNC_FAILED, &hcon->flags)) {
> > > +                     ev = hci_recv_event_data(hcon->hdev,
> > > +
> > > + HCI_EVT_LE_BIG_SYNC_ESTABILISHED);
> > > +
> > > +                     /* Get reference to PA sync parent socket, if
> > > + it exists */
> > >                       parent = iso_get_sock_listen(&hcon->src,
> > >                                                    &hcon->dst,
> > > -                                                  iso_match_big, ev);
> > > -             else
> > > +                                                  iso_match_pa_sync_flag, NULL);
> > > +                     if (!parent && ev)
> > > +                             parent = iso_get_sock_listen(&hcon->src,
> > > +                                                          &hcon->dst,
> > > +                                                          iso_match_big, ev);
> > > +             } else if (test_bit(HCI_CONN_PA_SYNC, &hcon->flags) ||
> > > +                             test_bit(HCI_CONN_PA_SYNC_FAILED, &hcon->flags)) {
> > > +                     ev2 = hci_recv_event_data(hcon->hdev,
> > > +                                               HCI_EV_LE_PA_SYNC_ESTABLISHED);
> > > +                     if (ev2)
> > > +                             parent = iso_get_sock_listen(&hcon->src,
> > > +                                                          &hcon->dst,
> > > +                                                          iso_match_sid, ev2);
> > > +             }
> > > +
> > > +             if (!parent)
> > >                       parent = iso_get_sock_listen(&hcon->src,
> > > -                                                  BDADDR_ANY, NULL, NULL);
> > > +                                                     BDADDR_ANY,
> > > + NULL, NULL);
> > >
> > >               if (!parent)
> > >                       return;
> > > @@ -1554,11 +1628,17 @@ static void iso_conn_ready(struct iso_conn
> *conn)
> > >               iso_sock_init(sk, parent);
> > >
> > >               bacpy(&iso_pi(sk)->src, &hcon->src);
> > > -             iso_pi(sk)->src_type = hcon->src_type;
> > > +
> > > +             /* Convert from HCI to three-value type */
> > > +             if (hcon->src_type == ADDR_LE_DEV_PUBLIC)
> > > +                     iso_pi(sk)->src_type = BDADDR_LE_PUBLIC;
> > > +             else
> > > +                     iso_pi(sk)->src_type = BDADDR_LE_RANDOM;
> > >
> > >               /* If hcon has no destination address (BDADDR_ANY) it means it
> > > -              * was created by HCI_EV_LE_BIG_SYNC_ESTABILISHED so we
> need to
> > > -              * initialize using the parent socket destination address.
> > > +              * was created by HCI_EV_LE_BIG_SYNC_ESTABILISHED or
> > > +              * HCI_EV_LE_PA_SYNC_ESTABLISHED so we need to initialize
> using
> > > +              * the parent socket destination address.
> > >                */
> > >               if (!bacmp(&hcon->dst, BDADDR_ANY)) {
> > >                       bacpy(&hcon->dst, &iso_pi(parent)->dst); @@
> > > -1566,13 +1646,21 @@ static void iso_conn_ready(struct iso_conn *conn)
> > >                       hcon->sync_handle = iso_pi(parent)->sync_handle;
> > >               }
> > >
> > > +             if (ev2 && !ev2->status) {
> > > +                     iso_pi(sk)->sync_handle = iso_pi(parent)->sync_handle;
> > > +                     iso_pi(sk)->qos = iso_pi(parent)->qos;
> > > +                     iso_pi(sk)->bc_num_bis = iso_pi(parent)->bc_num_bis;
> > > +                     memcpy(iso_pi(sk)->bc_bis, iso_pi(parent)->bc_bis,
> ISO_MAX_NUM_BIS);
> > > +             }
> > > +
> > >               bacpy(&iso_pi(sk)->dst, &hcon->dst);
> > >               iso_pi(sk)->dst_type = hcon->dst_type;
> > >
> > >               hci_conn_hold(hcon);
> > >               iso_chan_add(conn, sk, parent);
> > >
> > > -             if (ev && ((struct hci_evt_le_big_sync_estabilished *)ev)->status)
> {
> > > +             if ((ev && ((struct hci_evt_le_big_sync_estabilished *)ev)-
> >status) ||
> > > +                 (ev2 && ev2->status)) {
> > >                       /* Trigger error signal on child socket */
> > >                       sk->sk_err = ECONNREFUSED;
> > >                       sk->sk_error_report(sk); @@ -1630,7 +1718,7 @@
> > > int iso_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8
> *flags)
> > >       if (ev1) {
> > >               sk = iso_get_sock_listen(&hdev->bdaddr, bdaddr, iso_match_sid,
> > >                                        ev1);
> > > -             if (sk)
> > > +             if (sk && !ev1->status)
> > >                       iso_pi(sk)->sync_handle =
> > > le16_to_cpu(ev1->handle);
> > >
> > >               goto done;
> > > @@ -1638,16 +1726,21 @@ int iso_connect_ind(struct hci_dev *hdev,
> > > bdaddr_t *bdaddr, __u8 *flags)
> > >
> > >       ev2 = hci_recv_event_data(hdev,
> HCI_EVT_LE_BIG_INFO_ADV_REPORT);
> > >       if (ev2) {
> > > +             /* Try to get PA sync listening socket, if it exists
> > > + */
> > >               sk = iso_get_sock_listen(&hdev->bdaddr, bdaddr,
> > > -                                      iso_match_sync_handle, ev2);
> > > +                                             iso_match_pa_sync_flag, NULL);
> > > +             if (!sk)
> > > +                     sk = iso_get_sock_listen(&hdev->bdaddr, bdaddr,
> > > +
> > > + iso_match_sync_handle, ev2);
> > >               if (sk) {
> > >                       int err;
> > >
> > >                       if (ev2->num_bis < iso_pi(sk)->bc_num_bis)
> > >                               iso_pi(sk)->bc_num_bis = ev2->num_bis;
> > >
> > > -                     if (!test_and_set_bit(BT_SK_BIG_SYNC, &iso_pi(sk)->flags)) {
> > > -                             err = hci_le_big_create_sync(hdev,
> > > +                     if (!test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags) &&
> > > +                         !test_and_set_bit(BT_SK_BIG_SYNC, &iso_pi(sk)->flags)) {
> > > +                             err = hci_le_big_create_sync(hdev,
> > > + NULL,
> > >                                                            &iso_pi(sk)->qos,
> > >                                                            iso_pi(sk)->sync_handle,
> > >
> > > iso_pi(sk)->bc_num_bis, @@ -1699,12 +1792,13 @@ static void
> > > iso_connect_cfm(struct hci_conn *hcon, __u8 status)
> > >
> > >       BT_DBG("hcon %p bdaddr %pMR status %d", hcon, &hcon->dst,
> > > status);
> > >
> > > -     /* Similar to the success case, if HCI_CONN_BIG_SYNC_FAILED is set,
> > > -      * queue the failed bis connection into the accept queue of the
> > > -      * listening socket and wake up userspace, to inform the user about
> > > -      * the BIG sync failed event.
> > > +     /* Similar to the success case, if HCI_CONN_BIG_SYNC_FAILED or
> > > +      * HCI_CONN_PA_SYNC_FAILED is set, queue the failed connection
> > > +      * into the accept queue of the listening socket and wake up
> > > +      * userspace, to inform the user about the event.
> > >        */
> > > -     if (!status || test_bit(HCI_CONN_BIG_SYNC_FAILED, &hcon->flags)) {
> > > +     if (!status || test_bit(HCI_CONN_BIG_SYNC_FAILED, &hcon->flags)
> ||
> > > +         test_bit(HCI_CONN_PA_SYNC_FAILED, &hcon->flags)) {
> > >               struct iso_conn *conn;
> > >
> > >               conn = iso_conn_add(hcon);
> >
> > --
> > Pauli Virtanen
> 
> 
> 
> --
> Luiz Augusto von Dentz


Regards,
Iulia

Re: [PATCH v3 0/1] Bluetooth: ISO: Use defer setup to separate PA sync and BIG sync

[patchwork-bot+bluetooth]

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Thu, 17 Aug 2023 09:44:26 +0300 you wrote:
> Some scenarios require a Broadcast Sink to complete the PA sync and
> BIG sync procedures separately.
> 
> For example, a Broadcast Sink might synchronize to the periodic
> advertising transmitted by a Broadcast Source and it might detect
> that the BIG is encrypted, by looking at the BIGInfo advertising
> reports. If the Broadcast Sink doesn't know which Broadcast Code
> to pass as parameter to the BIG Create Sync command, it might learn
> it from a BASS Client through the Set Broadcast Code operation.
> Only then will it be able to successfully sync to the BIG.
> 
> [...]

Here is the summary with links:
  - [v3,1/1] Bluetooth: ISO: Use defer setup to separate PA sync and BIG sync
    https://git.kernel.org/bluetooth/bluetooth-next/c/f0835e7404b7

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html