diff mbox series

[1/1] Bluetooth: ISO: Reassociate a socket with an active BIS

Message ID 20231030154318.4339-2-iulia.tanasescu@nxp.com (mailing list archive)
State Superseded
Headers show
Series Bluetooth: ISO: Reassociate a socket with an active BIS | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/CheckPatch success CheckPatch PASS
tedd_an/GitLint success Gitlint PASS
tedd_an/SubjectPrefix success Gitlint PASS
tedd_an/BuildKernel success BuildKernel PASS
tedd_an/CheckAllWarning success CheckAllWarning PASS
tedd_an/CheckSparse success CheckSparse PASS
tedd_an/CheckSmatch success CheckSparse PASS
tedd_an/BuildKernel32 success BuildKernel32 PASS
tedd_an/TestRunnerSetup success TestRunnerSetup PASS
tedd_an/TestRunner_l2cap-tester success TestRunner PASS
tedd_an/TestRunner_iso-tester success TestRunner PASS
tedd_an/TestRunner_bnep-tester success TestRunner PASS
tedd_an/TestRunner_mgmt-tester success TestRunner PASS
tedd_an/TestRunner_rfcomm-tester success TestRunner PASS
tedd_an/TestRunner_sco-tester success TestRunner PASS
tedd_an/TestRunner_ioctl-tester success TestRunner PASS
tedd_an/TestRunner_mesh-tester success TestRunner PASS
tedd_an/TestRunner_smp-tester success TestRunner PASS
tedd_an/TestRunner_userchan-tester success TestRunner PASS
tedd_an/IncrementalBuild success Incremental Build PASS

Commit Message

Iulia Tanasescu Oct. 30, 2023, 3:43 p.m. UTC
For ISO Broadcast, all BISes from a BIG have the same lifespan - they
cannot be created or terminated independently from each other.

This links together all BIS hcons that are part of the same BIG, so all
hcons are kept alive as long as the BIG is active.

If multiple BIS sockets are opened for a BIG handle, and only part of
them are closed at some point, the associated hcons will be marked as
open. If new sockets will later be opened for the same BIG, they will
be reassociated with the open BIS hcons.

All BIS hcons will be cleaned up and the BIG will be terminated when
the last BIS socket is closed from userspace.

Signed-off-by: Iulia Tanasescu <iulia.tanasescu@nxp.com>
---
 include/net/bluetooth/hci_core.h | 24 +++++++++++++
 net/bluetooth/hci_conn.c         | 27 ++++++++++++++
 net/bluetooth/iso.c              | 60 ++++++++++++++++++++++++++++++++
 3 files changed, 111 insertions(+)

Comments

bluez.test.bot@gmail.com Oct. 30, 2023, 4:39 p.m. UTC | #1
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=797654

---Test result---

Test Summary:
CheckPatch                    PASS      1.21 seconds
GitLint                       PASS      0.34 seconds
SubjectPrefix                 PASS      0.22 seconds
BuildKernel                   PASS      33.39 seconds
CheckAllWarning               PASS      36.48 seconds
CheckSparse                   PASS      42.11 seconds
CheckSmatch                   PASS      116.34 seconds
BuildKernel32                 PASS      32.24 seconds
TestRunnerSetup               PASS      509.79 seconds
TestRunner_l2cap-tester       PASS      29.52 seconds
TestRunner_iso-tester         PASS      51.81 seconds
TestRunner_bnep-tester        PASS      9.79 seconds
TestRunner_mgmt-tester        PASS      207.12 seconds
TestRunner_rfcomm-tester      PASS      15.10 seconds
TestRunner_sco-tester         PASS      18.39 seconds
TestRunner_ioctl-tester       PASS      17.42 seconds
TestRunner_mesh-tester        PASS      12.93 seconds
TestRunner_smp-tester         PASS      13.30 seconds
TestRunner_userchan-tester    PASS      10.37 seconds
IncrementalBuild              PASS      30.68 seconds



---
Regards,
Linux Bluetooth
Pauli Virtanen Oct. 30, 2023, 10:12 p.m. UTC | #2
Hi,

ma, 2023-10-30 kello 17:43 +0200, Iulia Tanasescu kirjoitti:
> For ISO Broadcast, all BISes from a BIG have the same lifespan - they
> cannot be created or terminated independently from each other.
>
> This links together all BIS hcons that are part of the same BIG, so all
> hcons are kept alive as long as the BIG is active.
> 
> If multiple BIS sockets are opened for a BIG handle, and only part of
> them are closed at some point, the associated hcons will be marked as
> open. If new sockets will later be opened for the same BIG, they will
> be reassociated with the open BIS hcons.

Can you explain here a bit the new BIS hci_conn refcounting scheme,
it's not straightforward to follow? Who owns their refcounts?

> 
> All BIS hcons will be cleaned up and the BIG will be terminated when
> the last BIS socket is closed from userspace.
> 
> Signed-off-by: Iulia Tanasescu <iulia.tanasescu@nxp.com>
> ---
>  include/net/bluetooth/hci_core.h | 24 +++++++++++++
>  net/bluetooth/hci_conn.c         | 27 ++++++++++++++
>  net/bluetooth/iso.c              | 60 ++++++++++++++++++++++++++++++++
>  3 files changed, 111 insertions(+)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 20988623c5cc..201c0809540a 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -1292,6 +1292,30 @@ static inline struct hci_conn *hci_conn_hash_lookup_big(struct hci_dev *hdev,
>  	return NULL;
>  }
>  
> +static inline struct hci_conn *
> +hci_conn_hash_lookup_big_state(struct hci_dev *hdev, __u8 handle,  __u16 state)
> +{
> +	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 (bacmp(&c->dst, BDADDR_ANY) || c->type != ISO_LINK ||
> +			c->state != state)
> +			continue;
> +
> +		if (handle == c->iso_qos.bcast.big) {
> +			rcu_read_unlock();
> +			return c;
> +		}
> +	}
> +
> +	rcu_read_unlock();
> +
> +	return NULL;
> +}
> +
>  static inline struct hci_conn *
>  hci_conn_hash_lookup_pa_sync_big_handle(struct hci_dev *hdev, __u8 big)
>  {
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 2cee330188ce..b8ab5c0cd48e 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -2228,7 +2228,17 @@ struct hci_conn *hci_bind_bis(struct hci_dev *hdev, bdaddr_t *dst,
>  			      __u8 base_len, __u8 *base)
>  {
>  	struct hci_conn *conn;
> +	struct hci_conn *parent;
>  	__u8 eir[HCI_MAX_PER_AD_LENGTH];
> +	struct hci_link *link;
> +
> +	/* Look for any BIS that is open for rebinding */
> +	conn = hci_conn_hash_lookup_big_state(hdev, qos->bcast.big, BT_OPEN);
> +	if (conn) {
> +		memcpy(qos, &conn->iso_qos, sizeof(*qos));
> +		conn->state = BT_CONNECTED;
> +		return conn;
> +	}
>  
>  	if (base_len && base)
>  		base_len = eir_append_service_data(eir, 0,  0x1851,
> @@ -2256,6 +2266,20 @@ struct hci_conn *hci_bind_bis(struct hci_dev *hdev, bdaddr_t *dst,
>  	conn->iso_qos = *qos;
>  	conn->state = BT_BOUND;
>  
> +	/* Link BISes together */
> +	parent = hci_conn_hash_lookup_big(hdev,
> +					  conn->iso_qos.bcast.big);
> +	if (parent && parent != conn) {
> +		link = hci_conn_link(parent, conn);
> +		if (!link) {
> +			hci_conn_drop(conn);
> +			return ERR_PTR(-ENOLINK);
> +		}
> +
> +		/* Link takes the refcount */
> +		hci_conn_drop(conn);
> +	}

So the first hci_conn added in a BIG is assigned as "parent". How does
the refcounting here work?

hci_conn_link takes refcount of the parent (cf. hci_conn_unlink), but
it's not incremented here so I guess it may go negative.

hci_conn_unlink does not drop child connections, and they're not
dropped by the socket closing below unless last, so I'm not sure if
they are then cleaned up if closing sockets in any order.

This change will also expose BIS hci_conns to hci_conn_cleanup_child,
which was written only for unicast, and might not be right for the use
case here.

> +
>  	return conn;
>  }
>  
> @@ -2287,6 +2311,9 @@ struct hci_conn *hci_connect_bis(struct hci_dev *hdev, bdaddr_t *dst,
>  	if (IS_ERR(conn))
>  		return conn;
>  
> +	if (conn->state == BT_CONNECTED)
> +		return conn;
> +
>  	data.big = qos->bcast.big;
>  	data.bis = qos->bcast.bis;
>  
> diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
> index e01b6abe36fb..13353d7dc4b1 100644
> --- a/net/bluetooth/iso.c
> +++ b/net/bluetooth/iso.c
> @@ -587,6 +587,44 @@ static struct sock *iso_get_sock_listen(bdaddr_t *src, bdaddr_t *dst,
>  	return sk ? sk : sk1;
>  }
>  
> +static struct sock *iso_get_sock_big(struct sock *match_sk, bdaddr_t *src,
> +				     bdaddr_t *dst, uint8_t big)
> +{
> +	struct sock *sk = NULL;
> +
> +	read_lock(&iso_sk_list.lock);
> +
> +	sk_for_each(sk, &iso_sk_list.head) {
> +		if (match_sk == sk)
> +			continue;
> +
> +		/* Look for sockets that have already been
> +		 * connected to the BIG
> +		 */
> +		if (sk->sk_state != BT_CONNECTED &&
> +		    sk->sk_state != BT_CONNECT)
> +			continue;
> +
> +		/* Match Broadcast destination */
> +		if (bacmp(&iso_pi(sk)->dst, dst))
> +			continue;
> +
> +		/* Match BIG handle */
> +		if (iso_pi(sk)->qos.bcast.big != big)
> +			continue;
> +
> +		/* Match source address */
> +		if (bacmp(&iso_pi(sk)->src, src))
> +			continue;
> +
> +		break;
> +	}
> +
> +	read_unlock(&iso_sk_list.lock);
> +
> +	return sk;

What keeps sk alive?

This pattern is used also in iso_get_sock_listen, but I don't
understand why it's OK. In unix/ & netrom/ & ax25/ I see sock_hold
before lock release in similar constructs.

> +}
> +
>  static void iso_sock_destruct(struct sock *sk)
>  {
>  	BT_DBG("sk %p", sk);
> @@ -639,6 +677,28 @@ static void iso_sock_kill(struct sock *sk)
>  
>  static void iso_sock_disconn(struct sock *sk)
>  {
> +	struct sock *bis_sk;
> +	struct hci_conn *hcon = iso_pi(sk)->conn->hcon;
> +
> +	if (test_bit(HCI_CONN_BIG_CREATED, &hcon->flags)) {
> +		bis_sk = iso_get_sock_big(sk, &iso_pi(sk)->src,
> +					  &iso_pi(sk)->dst,
> +					  iso_pi(sk)->qos.bcast.big);
> +
> +		/* If there are any other connected sockets for the
> +		 * same BIG, just delete the sk and leave the bis
> +		 * hcon active, in case later rebinding is needed.
> +		 */
> +		if (bis_sk) {
> +			hcon->state = BT_OPEN;
> +			iso_pi(sk)->conn->hcon = NULL;
> +			release_sock(sk);
> +			iso_conn_del(hcon, bt_to_errno(hcon->abort_reason));

iso_conn_del must be called with hdev->lock held, that's assumed in
iso_connect*.

Locks must be taken in order hci_dev_lock > lock_sock > iso_conn_lock.

Could this use iso_chan_del instead (plus maybe iso_sock_clear_timer)?
Lifetime of iso_conn currently follows hcon, not sure if that needs to
be changed?

> +			lock_sock(sk);
> +			return;
> +		}
> +	}
> +
>  	sk->sk_state = BT_DISCONN;
>  	iso_sock_set_timer(sk, ISO_DISCONN_TIMEOUT);
>  	iso_conn_lock(iso_pi(sk)->conn);

Not related to this patch precisely, but I suspect the disconnect
timeout is something that is not useful for ISO sockets and maybe we
should remove it, since closing sockets is used for CIG/BIG management.
Iulia Tanasescu Nov. 1, 2023, 3:34 p.m. UTC | #3
Hi Pauli,

> -----Original Message-----
> From: Pauli Virtanen <pav@iki.fi>
> Sent: Tuesday, October 31, 2023 12:13 AM
> To: Iulia Tanasescu <iulia.tanasescu@nxp.com>; linux-
> bluetooth@vger.kernel.org
> Subject: Re: [PATCH 1/1] Bluetooth: ISO: Reassociate a socket with an
> active BIS 
> 
> Hi,
> 
> ma, 2023-10-30 kello 17:43 +0200, Iulia Tanasescu kirjoitti:
> > For ISO Broadcast, all BISes from a BIG have the same lifespan - they
> > cannot be created or terminated independently from each other.
> >
> > This links together all BIS hcons that are part of the same BIG, so
> > all hcons are kept alive as long as the BIG is active.
> >
> > If multiple BIS sockets are opened for a BIG handle, and only part of
> > them are closed at some point, the associated hcons will be marked as
> > open. If new sockets will later be opened for the same BIG, they will
> > be reassociated with the open BIS hcons.
> 
> Can you explain here a bit the new BIS hci_conn refcounting scheme, it's
> not straightforward to follow? Who owns their refcounts?
> 
> >
> > All BIS hcons will be cleaned up and the BIG will be terminated when
> > the last BIS socket is closed from userspace.
> >
> > Signed-off-by: Iulia Tanasescu <iulia.tanasescu@nxp.com>
> > ---
> >  include/net/bluetooth/hci_core.h | 24 +++++++++++++
> >  net/bluetooth/hci_conn.c         | 27 ++++++++++++++
> >  net/bluetooth/iso.c              | 60 ++++++++++++++++++++++++++++++++
> >  3 files changed, 111 insertions(+)
> >
> > diff --git a/include/net/bluetooth/hci_core.h
> > b/include/net/bluetooth/hci_core.h
> > index 20988623c5cc..201c0809540a 100644
> > --- a/include/net/bluetooth/hci_core.h
> > +++ b/include/net/bluetooth/hci_core.h
> > @@ -1292,6 +1292,30 @@ static inline struct hci_conn
> *hci_conn_hash_lookup_big(struct hci_dev *hdev,
> >       return NULL;
> >  }
> >
> > +static inline struct hci_conn *
> > +hci_conn_hash_lookup_big_state(struct hci_dev *hdev, __u8 handle,
> > +__u16 state) {
> > +     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 (bacmp(&c->dst, BDADDR_ANY) || c->type != ISO_LINK ||
> > +                     c->state != state)
> > +                     continue;
> > +
> > +             if (handle == c->iso_qos.bcast.big) {
> > +                     rcu_read_unlock();
> > +                     return c;
> > +             }
> > +     }
> > +
> > +     rcu_read_unlock();
> > +
> > +     return NULL;
> > +}
> > +
> >  static inline struct hci_conn *
> >  hci_conn_hash_lookup_pa_sync_big_handle(struct hci_dev *hdev, __u8
> > big)  { diff --git a/net/bluetooth/hci_conn.c
> > b/net/bluetooth/hci_conn.c index 2cee330188ce..b8ab5c0cd48e 100644
> > --- a/net/bluetooth/hci_conn.c
> > +++ b/net/bluetooth/hci_conn.c
> > @@ -2228,7 +2228,17 @@ struct hci_conn *hci_bind_bis(struct hci_dev
> *hdev, bdaddr_t *dst,
> >                             __u8 base_len, __u8 *base)  {
> >       struct hci_conn *conn;
> > +     struct hci_conn *parent;
> >       __u8 eir[HCI_MAX_PER_AD_LENGTH];
> > +     struct hci_link *link;
> > +
> > +     /* Look for any BIS that is open for rebinding */
> > +     conn = hci_conn_hash_lookup_big_state(hdev, qos->bcast.big,
> BT_OPEN);
> > +     if (conn) {
> > +             memcpy(qos, &conn->iso_qos, sizeof(*qos));
> > +             conn->state = BT_CONNECTED;
> > +             return conn;
> > +     }
> >
> >       if (base_len && base)
> >               base_len = eir_append_service_data(eir, 0,  0x1851, @@
> > -2256,6 +2266,20 @@ struct hci_conn *hci_bind_bis(struct hci_dev *hdev,
> bdaddr_t *dst,
> >       conn->iso_qos = *qos;
> >       conn->state = BT_BOUND;
> >
> > +     /* Link BISes together */
> > +     parent = hci_conn_hash_lookup_big(hdev,
> > +                                       conn->iso_qos.bcast.big);
> > +     if (parent && parent != conn) {
> > +             link = hci_conn_link(parent, conn);
> > +             if (!link) {
> > +                     hci_conn_drop(conn);
> > +                     return ERR_PTR(-ENOLINK);
> > +             }
> > +
> > +             /* Link takes the refcount */
> > +             hci_conn_drop(conn);
> > +     }
> 
> So the first hci_conn added in a BIG is assigned as "parent". How does the
> refcounting here work?
> 
> hci_conn_link takes refcount of the parent (cf. hci_conn_unlink), but it's not
> incremented here so I guess it may go negative.

When adding a new BIS hcon, hci_add_bis will take the refcount.

The first BIS in a BIG will be assigned as parent. When new BISes are added,
they will be linked to the parent and hci_conn_link takes an extra refcount
for children:
https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/tree/net/bluetooth/hci_conn.c#n1724

This is why hci_conn_drop needs to be called for children BISes, to remove
the duplicate refcount.

> 
> hci_conn_unlink does not drop child connections, and they're not dropped
> by the socket closing below unless last, so I'm not sure if they are then
> cleaned up if closing sockets in any order.

If hci_conn_unlink is called for a child hcon, the child will be unlinked
and hci_conn_drop will be called on the parent:
https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/tree/net/bluetooth/hci_conn.c#n1130

hci_conn_unlink will then be called for the parent, and all other children
hcons will be cleaned up with hci_conn_cleanup_child.

So regardless if the parent is closed first or any child, all hcons will get
to be cleaned up.

> 
> This change will also expose BIS hci_conns to hci_conn_cleanup_child,
> which was written only for unicast, and might not be right for the use case
> here.

That is right, from the tests I performed I noticed the conditions in
hci_conn_cleanup_child are also met for BIS so it will reach
hci_conn_failed, but I submitted an updated patch to add a dedicated
condition for broadcast also, to cover all cases.

> 
> > +
> >       return conn;
> >  }
> >
> > @@ -2287,6 +2311,9 @@ struct hci_conn *hci_connect_bis(struct hci_dev
> *hdev, bdaddr_t *dst,
> >       if (IS_ERR(conn))
> >               return conn;
> >
> > +     if (conn->state == BT_CONNECTED)
> > +             return conn;
> > +
> >       data.big = qos->bcast.big;
> >       data.bis = qos->bcast.bis;
> >
> > diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c index
> > e01b6abe36fb..13353d7dc4b1 100644
> > --- a/net/bluetooth/iso.c
> > +++ b/net/bluetooth/iso.c
> > @@ -587,6 +587,44 @@ static struct sock *iso_get_sock_listen(bdaddr_t
> *src, bdaddr_t *dst,
> >       return sk ? sk : sk1;
> >  }
> >
> > +static struct sock *iso_get_sock_big(struct sock *match_sk, bdaddr_t *src,
> > +                                  bdaddr_t *dst, uint8_t big) {
> > +     struct sock *sk = NULL;
> > +
> > +     read_lock(&iso_sk_list.lock);
> > +
> > +     sk_for_each(sk, &iso_sk_list.head) {
> > +             if (match_sk == sk)
> > +                     continue;
> > +
> > +             /* Look for sockets that have already been
> > +              * connected to the BIG
> > +              */
> > +             if (sk->sk_state != BT_CONNECTED &&
> > +                 sk->sk_state != BT_CONNECT)
> > +                     continue;
> > +
> > +             /* Match Broadcast destination */
> > +             if (bacmp(&iso_pi(sk)->dst, dst))
> > +                     continue;
> > +
> > +             /* Match BIG handle */
> > +             if (iso_pi(sk)->qos.bcast.big != big)
> > +                     continue;
> > +
> > +             /* Match source address */
> > +             if (bacmp(&iso_pi(sk)->src, src))
> > +                     continue;
> > +
> > +             break;
> > +     }
> > +
> > +     read_unlock(&iso_sk_list.lock);
> > +
> > +     return sk;
> 
> What keeps sk alive?
> 
> This pattern is used also in iso_get_sock_listen, but I don't understand why
> it's OK. In unix/ & netrom/ & ax25/ I see sock_hold before lock release in
> similar constructs.

It seems like it might be necessary to add sock_hold to make sure the
socket is kept alive while it is being processed, so I added this update
in the new patch version.

> 
> > +}
> > +
> >  static void iso_sock_destruct(struct sock *sk)  {
> >       BT_DBG("sk %p", sk);
> > @@ -639,6 +677,28 @@ static void iso_sock_kill(struct sock *sk)
> >
> >  static void iso_sock_disconn(struct sock *sk)  {
> > +     struct sock *bis_sk;
> > +     struct hci_conn *hcon = iso_pi(sk)->conn->hcon;
> > +
> > +     if (test_bit(HCI_CONN_BIG_CREATED, &hcon->flags)) {
> > +             bis_sk = iso_get_sock_big(sk, &iso_pi(sk)->src,
> > +                                       &iso_pi(sk)->dst,
> > +                                       iso_pi(sk)->qos.bcast.big);
> > +
> > +             /* If there are any other connected sockets for the
> > +              * same BIG, just delete the sk and leave the bis
> > +              * hcon active, in case later rebinding is needed.
> > +              */
> > +             if (bis_sk) {
> > +                     hcon->state = BT_OPEN;
> > +                     iso_pi(sk)->conn->hcon = NULL;
> > +                     release_sock(sk);
> > +                     iso_conn_del(hcon,
> > + bt_to_errno(hcon->abort_reason));
> 
> iso_conn_del must be called with hdev->lock held, that's assumed in
> iso_connect*.
> 
> Locks must be taken in order hci_dev_lock > lock_sock > iso_conn_lock.
> 
> Could this use iso_chan_del instead (plus maybe iso_sock_clear_timer)?
> Lifetime of iso_conn currently follows hcon, not sure if that needs to be
> changed?

I agree, I could use iso_chan_del here, so only the socket is cleaned up,
and the ISO conn will remain allocated as long as hcon is alive. I updated
in the new patch.

> 
> > +                     lock_sock(sk);
> > +                     return;
> > +             }
> > +     }
> > +
> >       sk->sk_state = BT_DISCONN;
> >       iso_sock_set_timer(sk, ISO_DISCONN_TIMEOUT);
> >       iso_conn_lock(iso_pi(sk)->conn);
> 
> Not related to this patch precisely, but I suspect the disconnect timeout is
> something that is not useful for ISO sockets and maybe we should remove it,
> since closing sockets is used for CIG/BIG management.

I think the disconnect timeout makes more sense for CIS management, because
when a CIS socket is closed, HCI_OP_DISCONNECT is first sent to controller
to disconnect the CIS, and only after HCI_EV_DISCONN_COMPLETE is received,
the hcon will be cleaned up and the CIG will be removed.

> 
> --
> Pauli Virtanen

Regards,
Iulia
diff mbox series

Patch

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 20988623c5cc..201c0809540a 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1292,6 +1292,30 @@  static inline struct hci_conn *hci_conn_hash_lookup_big(struct hci_dev *hdev,
 	return NULL;
 }
 
+static inline struct hci_conn *
+hci_conn_hash_lookup_big_state(struct hci_dev *hdev, __u8 handle,  __u16 state)
+{
+	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 (bacmp(&c->dst, BDADDR_ANY) || c->type != ISO_LINK ||
+			c->state != state)
+			continue;
+
+		if (handle == c->iso_qos.bcast.big) {
+			rcu_read_unlock();
+			return c;
+		}
+	}
+
+	rcu_read_unlock();
+
+	return NULL;
+}
+
 static inline struct hci_conn *
 hci_conn_hash_lookup_pa_sync_big_handle(struct hci_dev *hdev, __u8 big)
 {
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 2cee330188ce..b8ab5c0cd48e 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -2228,7 +2228,17 @@  struct hci_conn *hci_bind_bis(struct hci_dev *hdev, bdaddr_t *dst,
 			      __u8 base_len, __u8 *base)
 {
 	struct hci_conn *conn;
+	struct hci_conn *parent;
 	__u8 eir[HCI_MAX_PER_AD_LENGTH];
+	struct hci_link *link;
+
+	/* Look for any BIS that is open for rebinding */
+	conn = hci_conn_hash_lookup_big_state(hdev, qos->bcast.big, BT_OPEN);
+	if (conn) {
+		memcpy(qos, &conn->iso_qos, sizeof(*qos));
+		conn->state = BT_CONNECTED;
+		return conn;
+	}
 
 	if (base_len && base)
 		base_len = eir_append_service_data(eir, 0,  0x1851,
@@ -2256,6 +2266,20 @@  struct hci_conn *hci_bind_bis(struct hci_dev *hdev, bdaddr_t *dst,
 	conn->iso_qos = *qos;
 	conn->state = BT_BOUND;
 
+	/* Link BISes together */
+	parent = hci_conn_hash_lookup_big(hdev,
+					  conn->iso_qos.bcast.big);
+	if (parent && parent != conn) {
+		link = hci_conn_link(parent, conn);
+		if (!link) {
+			hci_conn_drop(conn);
+			return ERR_PTR(-ENOLINK);
+		}
+
+		/* Link takes the refcount */
+		hci_conn_drop(conn);
+	}
+
 	return conn;
 }
 
@@ -2287,6 +2311,9 @@  struct hci_conn *hci_connect_bis(struct hci_dev *hdev, bdaddr_t *dst,
 	if (IS_ERR(conn))
 		return conn;
 
+	if (conn->state == BT_CONNECTED)
+		return conn;
+
 	data.big = qos->bcast.big;
 	data.bis = qos->bcast.bis;
 
diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
index e01b6abe36fb..13353d7dc4b1 100644
--- a/net/bluetooth/iso.c
+++ b/net/bluetooth/iso.c
@@ -587,6 +587,44 @@  static struct sock *iso_get_sock_listen(bdaddr_t *src, bdaddr_t *dst,
 	return sk ? sk : sk1;
 }
 
+static struct sock *iso_get_sock_big(struct sock *match_sk, bdaddr_t *src,
+				     bdaddr_t *dst, uint8_t big)
+{
+	struct sock *sk = NULL;
+
+	read_lock(&iso_sk_list.lock);
+
+	sk_for_each(sk, &iso_sk_list.head) {
+		if (match_sk == sk)
+			continue;
+
+		/* Look for sockets that have already been
+		 * connected to the BIG
+		 */
+		if (sk->sk_state != BT_CONNECTED &&
+		    sk->sk_state != BT_CONNECT)
+			continue;
+
+		/* Match Broadcast destination */
+		if (bacmp(&iso_pi(sk)->dst, dst))
+			continue;
+
+		/* Match BIG handle */
+		if (iso_pi(sk)->qos.bcast.big != big)
+			continue;
+
+		/* Match source address */
+		if (bacmp(&iso_pi(sk)->src, src))
+			continue;
+
+		break;
+	}
+
+	read_unlock(&iso_sk_list.lock);
+
+	return sk;
+}
+
 static void iso_sock_destruct(struct sock *sk)
 {
 	BT_DBG("sk %p", sk);
@@ -639,6 +677,28 @@  static void iso_sock_kill(struct sock *sk)
 
 static void iso_sock_disconn(struct sock *sk)
 {
+	struct sock *bis_sk;
+	struct hci_conn *hcon = iso_pi(sk)->conn->hcon;
+
+	if (test_bit(HCI_CONN_BIG_CREATED, &hcon->flags)) {
+		bis_sk = iso_get_sock_big(sk, &iso_pi(sk)->src,
+					  &iso_pi(sk)->dst,
+					  iso_pi(sk)->qos.bcast.big);
+
+		/* If there are any other connected sockets for the
+		 * same BIG, just delete the sk and leave the bis
+		 * hcon active, in case later rebinding is needed.
+		 */
+		if (bis_sk) {
+			hcon->state = BT_OPEN;
+			iso_pi(sk)->conn->hcon = NULL;
+			release_sock(sk);
+			iso_conn_del(hcon, bt_to_errno(hcon->abort_reason));
+			lock_sock(sk);
+			return;
+		}
+	}
+
 	sk->sk_state = BT_DISCONN;
 	iso_sock_set_timer(sk, ISO_DISCONN_TIMEOUT);
 	iso_conn_lock(iso_pi(sk)->conn);