Hi Pauli, On Wed, Jul 26, 2023 at 2:43 PM Pauli Virtanen <pav@xxxxxx> wrote: > > Dropped CIS that are in state BT_OPEN/BT_BOUND, and in state BT_CONNECT > with HCI_CONN_CREATE_CIS unset, should be cleaned up immediately. > Closing CIS ISO sockets should result to the hci_conn be deleted, so > that potentially pending CIG removal can run. > > hci_abort_conn cannot refer to them by handle, since their handle is > still unset if Set CIG Parameters has not yet completed. > > This fixes CIS not being terminated if the socket is shut down > immediately after connection, so that the hci_abort_conn runs before Set > CIG Parameters completes. See new BlueZ test "ISO Connect Close - Success" > > Signed-off-by: Pauli Virtanen <pav@xxxxxx> > --- > net/bluetooth/hci_sync.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c > index 101548fe81da..3926213c29e6 100644 > --- a/net/bluetooth/hci_sync.c > +++ b/net/bluetooth/hci_sync.c > @@ -5339,6 +5339,10 @@ static int hci_connect_cancel_sync(struct hci_dev *hdev, struct hci_conn *conn, > if (test_bit(HCI_CONN_CREATE_CIS, &conn->flags)) > return hci_disconnect_sync(hdev, conn, reason); > > + /* CIS with no Create CIS sent have nothing to cancel */ > + if (bacmp(&conn->dst, BDADDR_ANY)) > + return HCI_ERROR_LOCAL_HOST_TERM; > + > /* There is no way to cancel a BIS without terminating the BIG > * which is done later on connection cleanup. > */ > @@ -5426,9 +5430,17 @@ int hci_abort_conn_sync(struct hci_dev *hdev, struct hci_conn *conn, u8 reason) > case BT_CONNECT2: > return hci_reject_conn_sync(hdev, conn, reason); > case BT_OPEN: > - /* Cleanup bises that failed to be established */ > - if (test_and_clear_bit(HCI_CONN_BIG_SYNC_FAILED, &conn->flags)) > + /* Cleanup failed CIS, and BIS that failed to be established */ > + if (bacmp(&conn->dst, BDADDR_ANY) || > + test_and_clear_bit(HCI_CONN_BIG_SYNC_FAILED, &conn->flags)) bacmp(&conn->dst, BDADDR_ANY) will match connections other than ISO_LINK as well so I wonder if this is intentional? If it is then we need to update the commands to reflect that we are going to call hci_conn_failed, it seems we didn't call it before but perhaps this is a side effect of the other changes. > + hci_conn_failed(conn, reason); > + break; > + case BT_BOUND: > + /* Bound CIS should be cleaned up */ > + if (conn->type == ISO_LINK && bacmp(&conn->dst, BDADDR_ANY)) > hci_conn_failed(conn, reason); > + else > + conn->state = BT_CLOSED; > break; > default: > conn->state = BT_CLOSED; > -- > 2.41.0 > -- Luiz Augusto von Dentz
