* [PATCH v3 1/5] Bluetooth: ISO: Don't initiate CIS connections if there are no buffers
@ 2025-08-13 21:15 Luiz Augusto von Dentz
2025-08-13 21:15 ` [PATCH v3 2/5] Bluetooth: HCI: Fix using LE/ACL buffers for ISO packets Luiz Augusto von Dentz
` (6 more replies)
0 siblings, 7 replies; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2025-08-13 21:15 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
If the controller has no buffers left return -ENOBUFF to indicate that
iso_cnt might be out of sync.
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
net/bluetooth/iso.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
index 5ce823ca3aaf..dff3fc4917d6 100644
--- a/net/bluetooth/iso.c
+++ b/net/bluetooth/iso.c
@@ -458,6 +458,13 @@ static int iso_connect_cis(struct sock *sk)
goto unlock;
}
+ /* Check if there are available buffers for output/TX. */
+ if (iso_pi(sk)->qos.ucast.out.sdu && !hci_conn_num(hdev, CIS_LINK) &&
+ (hdev->iso_pkts && !hdev->iso_cnt)) {
+ err = -ENOBUFS;
+ goto unlock;
+ }
+
/* Just bind if DEFER_SETUP has been set */
if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) {
hcon = hci_bind_cis(hdev, &iso_pi(sk)->dst,
--
2.50.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 2/5] Bluetooth: HCI: Fix using LE/ACL buffers for ISO packets 2025-08-13 21:15 [PATCH v3 1/5] Bluetooth: ISO: Don't initiate CIS connections if there are no buffers Luiz Augusto von Dentz @ 2025-08-13 21:15 ` Luiz Augusto von Dentz 2025-08-13 21:15 ` [PATCH v3 3/5] Bluetooth: hci_conn: Make unacked packet handling more robust Luiz Augusto von Dentz ` (5 subsequent siblings) 6 siblings, 0 replies; 11+ messages in thread From: Luiz Augusto von Dentz @ 2025-08-13 21:15 UTC (permalink / raw) To: linux-bluetooth From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> ISO packets shall not use LE/ACL buffer pool, that feature seem to be exclusive to LE-ACL only. Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> --- net/bluetooth/hci_conn.c | 33 ++++++++++++++------------------- net/bluetooth/hci_core.c | 6 ++---- net/bluetooth/hci_event.c | 16 +++------------- 3 files changed, 19 insertions(+), 36 deletions(-) diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index 7a879290dd28..9d2324eb1211 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -926,10 +926,9 @@ static struct hci_conn *__hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t case CIS_LINK: case BIS_LINK: case PA_LINK: - if (hdev->iso_mtu) - /* Dedicated ISO Buffer exists */ - break; - fallthrough; + if (!hdev->iso_mtu) + return ERR_PTR(-ECONNREFUSED); + break; case LE_LINK: if (hdev->le_mtu && hdev->le_mtu < HCI_MIN_LE_MTU) return ERR_PTR(-ECONNREFUSED); @@ -1152,28 +1151,24 @@ void hci_conn_del(struct hci_conn *conn) disable_delayed_work_sync(&conn->auto_accept_work); disable_delayed_work_sync(&conn->idle_work); - if (conn->type == ACL_LINK) { - /* Unacked frames */ + /* Handle unnacked frames */ + switch (conn->type) { + case ACL_LINK: hdev->acl_cnt += conn->sent; - } else if (conn->type == LE_LINK) { + break; + case LE_LINK: cancel_delayed_work(&conn->le_conn_timeout); if (hdev->le_pkts) hdev->le_cnt += conn->sent; else hdev->acl_cnt += conn->sent; - } else { - /* Unacked ISO frames */ - if (conn->type == CIS_LINK || - conn->type == BIS_LINK || - conn->type == PA_LINK) { - if (hdev->iso_pkts) - hdev->iso_cnt += conn->sent; - else if (hdev->le_pkts) - hdev->le_cnt += conn->sent; - else - hdev->acl_cnt += conn->sent; - } + break; + case CIS_LINK: + case BIS_LINK: + case PA_LINK: + hdev->iso_cnt += conn->sent; + break; } skb_queue_purge(&conn->data_q); diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 55e0722fd066..e2bffad9816f 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -3399,8 +3399,7 @@ static inline void hci_quote_sent(struct hci_conn *conn, int num, int *quote) case CIS_LINK: case BIS_LINK: case PA_LINK: - cnt = hdev->iso_mtu ? hdev->iso_cnt : - hdev->le_mtu ? hdev->le_cnt : hdev->acl_cnt; + cnt = hdev->iso_cnt; break; default: cnt = 0; @@ -3759,8 +3758,7 @@ static void hci_sched_iso(struct hci_dev *hdev, __u8 type) if (!hci_conn_num(hdev, type)) return; - cnt = hdev->iso_pkts ? &hdev->iso_cnt : - hdev->le_pkts ? &hdev->le_cnt : &hdev->acl_cnt; + cnt = &hdev->iso_cnt; while (*cnt && (conn = hci_low_sent(hdev, type, "e))) { while (quote-- && (skb = skb_dequeue(&conn->data_q))) { BT_DBG("skb %p len %d", skb, skb->len); diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index fe7cdd67ad2a..1686680a38c8 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -4433,19 +4433,9 @@ static void hci_num_comp_pkts_evt(struct hci_dev *hdev, void *data, case CIS_LINK: case BIS_LINK: case PA_LINK: - if (hdev->iso_pkts) { - hdev->iso_cnt += count; - if (hdev->iso_cnt > hdev->iso_pkts) - hdev->iso_cnt = hdev->iso_pkts; - } else if (hdev->le_pkts) { - hdev->le_cnt += count; - if (hdev->le_cnt > hdev->le_pkts) - hdev->le_cnt = hdev->le_pkts; - } else { - hdev->acl_cnt += count; - if (hdev->acl_cnt > hdev->acl_pkts) - hdev->acl_cnt = hdev->acl_pkts; - } + hdev->iso_cnt += count; + if (hdev->iso_cnt > hdev->iso_pkts) + hdev->iso_cnt = hdev->iso_pkts; break; default: -- 2.50.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 3/5] Bluetooth: hci_conn: Make unacked packet handling more robust 2025-08-13 21:15 [PATCH v3 1/5] Bluetooth: ISO: Don't initiate CIS connections if there are no buffers Luiz Augusto von Dentz 2025-08-13 21:15 ` [PATCH v3 2/5] Bluetooth: HCI: Fix using LE/ACL buffers for ISO packets Luiz Augusto von Dentz @ 2025-08-13 21:15 ` Luiz Augusto von Dentz 2025-08-14 15:30 ` Pauli Virtanen 2025-08-13 21:15 ` [PATCH v3 4/5] Bluetooth: ISO: Use sk_sndtimeo as conn_timeout Luiz Augusto von Dentz ` (4 subsequent siblings) 6 siblings, 1 reply; 11+ messages in thread From: Luiz Augusto von Dentz @ 2025-08-13 21:15 UTC (permalink / raw) To: linux-bluetooth From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> This attempts to make unacked packet handling more robust by detecting if there are no connections left then restore all buffers of the respective pool. Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> --- net/bluetooth/hci_conn.c | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index 9d2324eb1211..d2f0c3c0f0ae 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -1151,22 +1151,44 @@ void hci_conn_del(struct hci_conn *conn) disable_delayed_work_sync(&conn->auto_accept_work); disable_delayed_work_sync(&conn->idle_work); - /* Handle unnacked frames */ + /* Handle unnacked frames: + * + * - In case there are no connection restore all buffers to the pool + * - Otherwise restore just the buffers considered in transit for the + * hci_conn + */ switch (conn->type) { case ACL_LINK: - hdev->acl_cnt += conn->sent; + if (!hci_conn_num(hdev, ACL_LINK)) + hdev->acl_cnt = hdev->acl_pkts; + else + hdev->acl_cnt += conn->sent; break; case LE_LINK: cancel_delayed_work(&conn->le_conn_timeout); - if (hdev->le_pkts) - hdev->le_cnt += conn->sent; - else - hdev->acl_cnt += conn->sent; + if (hdev->le_pkts) { + if (!hci_conn_num(hdev, LE_LINK)) + hdev->le_cnt = hdev->le_pkts; + else + hdev->le_cnt += conn->sent; + } else { + if (!hci_conn_num(hdev, LE_LINK) && + !hci_conn_num(hdev, ACL_LINK)) + hdev->acl_cnt = hdev->acl_pkts; + else + hdev->acl_cnt += conn->sent; + } break; case CIS_LINK: case BIS_LINK: case PA_LINK: + if (!hci_conn_num(hdev, CIS_LINK) && + !hci_conn_num(hdev, BIS_LINK) && + !hci_conn_num(hdev, PA_LINK)) + hdev->iso_cnt = hdev->iso_pkts; + else + hdev->iso_cnt += conn->sent; hdev->iso_cnt += conn->sent; break; } -- 2.50.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 3/5] Bluetooth: hci_conn: Make unacked packet handling more robust 2025-08-13 21:15 ` [PATCH v3 3/5] Bluetooth: hci_conn: Make unacked packet handling more robust Luiz Augusto von Dentz @ 2025-08-14 15:30 ` Pauli Virtanen 2025-08-14 15:45 ` Luiz Augusto von Dentz 0 siblings, 1 reply; 11+ messages in thread From: Pauli Virtanen @ 2025-08-14 15:30 UTC (permalink / raw) To: Luiz Augusto von Dentz, linux-bluetooth ke, 2025-08-13 kello 17:15 -0400, Luiz Augusto von Dentz kirjoitti: > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > This attempts to make unacked packet handling more robust by detecting > if there are no connections left then restore all buffers of the > respective pool. > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > --- > net/bluetooth/hci_conn.c | 34 ++++++++++++++++++++++++++++------ > 1 file changed, 28 insertions(+), 6 deletions(-) > > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c > index 9d2324eb1211..d2f0c3c0f0ae 100644 > --- a/net/bluetooth/hci_conn.c > +++ b/net/bluetooth/hci_conn.c > @@ -1151,22 +1151,44 @@ void hci_conn_del(struct hci_conn *conn) > disable_delayed_work_sync(&conn->auto_accept_work); > disable_delayed_work_sync(&conn->idle_work); > > - /* Handle unnacked frames */ > + /* Handle unnacked frames: > + * > + * - In case there are no connection restore all buffers to the pool > + * - Otherwise restore just the buffers considered in transit for the > + * hci_conn > + */ > switch (conn->type) { > case ACL_LINK: > - hdev->acl_cnt += conn->sent; > + if (!hci_conn_num(hdev, ACL_LINK)) > + hdev->acl_cnt = hdev->acl_pkts; > + else > + hdev->acl_cnt += conn->sent; > break; > case LE_LINK: > cancel_delayed_work(&conn->le_conn_timeout); > > - if (hdev->le_pkts) > - hdev->le_cnt += conn->sent; > - else > - hdev->acl_cnt += conn->sent; > + if (hdev->le_pkts) { > + if (!hci_conn_num(hdev, LE_LINK)) > + hdev->le_cnt = hdev->le_pkts; > + else > + hdev->le_cnt += conn->sent; > + } else { > + if (!hci_conn_num(hdev, LE_LINK) && > + !hci_conn_num(hdev, ACL_LINK)) > + hdev->acl_cnt = hdev->acl_pkts; > + else > + hdev->acl_cnt += conn->sent; > + } > break; > case CIS_LINK: > case BIS_LINK: > case PA_LINK: > + if (!hci_conn_num(hdev, CIS_LINK) && > + !hci_conn_num(hdev, BIS_LINK) && > + !hci_conn_num(hdev, PA_LINK)) > + hdev->iso_cnt = hdev->iso_pkts; > + else > + hdev->iso_cnt += conn->sent; > hdev->iso_cnt += conn->sent; The last hdev->iso_cnt += conn->sent; probably should be removed. > break; > } -- Pauli Virtanen ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 3/5] Bluetooth: hci_conn: Make unacked packet handling more robust 2025-08-14 15:30 ` Pauli Virtanen @ 2025-08-14 15:45 ` Luiz Augusto von Dentz 0 siblings, 0 replies; 11+ messages in thread From: Luiz Augusto von Dentz @ 2025-08-14 15:45 UTC (permalink / raw) To: Pauli Virtanen; +Cc: linux-bluetooth Hi Pauli, On Thu, Aug 14, 2025 at 11:30 AM Pauli Virtanen <pav@iki.fi> wrote: > > ke, 2025-08-13 kello 17:15 -0400, Luiz Augusto von Dentz kirjoitti: > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > > > This attempts to make unacked packet handling more robust by detecting > > if there are no connections left then restore all buffers of the > > respective pool. > > > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > --- > > net/bluetooth/hci_conn.c | 34 ++++++++++++++++++++++++++++------ > > 1 file changed, 28 insertions(+), 6 deletions(-) > > > > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c > > index 9d2324eb1211..d2f0c3c0f0ae 100644 > > --- a/net/bluetooth/hci_conn.c > > +++ b/net/bluetooth/hci_conn.c > > @@ -1151,22 +1151,44 @@ void hci_conn_del(struct hci_conn *conn) > > disable_delayed_work_sync(&conn->auto_accept_work); > > disable_delayed_work_sync(&conn->idle_work); > > > > - /* Handle unnacked frames */ > > + /* Handle unnacked frames: > > + * > > + * - In case there are no connection restore all buffers to the pool > > + * - Otherwise restore just the buffers considered in transit for the > > + * hci_conn > > + */ > > switch (conn->type) { > > case ACL_LINK: > > - hdev->acl_cnt += conn->sent; > > + if (!hci_conn_num(hdev, ACL_LINK)) > > + hdev->acl_cnt = hdev->acl_pkts; > > + else > > + hdev->acl_cnt += conn->sent; > > break; > > case LE_LINK: > > cancel_delayed_work(&conn->le_conn_timeout); > > > > - if (hdev->le_pkts) > > - hdev->le_cnt += conn->sent; > > - else > > - hdev->acl_cnt += conn->sent; > > + if (hdev->le_pkts) { > > + if (!hci_conn_num(hdev, LE_LINK)) > > + hdev->le_cnt = hdev->le_pkts; > > + else > > + hdev->le_cnt += conn->sent; > > + } else { > > + if (!hci_conn_num(hdev, LE_LINK) && > > + !hci_conn_num(hdev, ACL_LINK)) > > + hdev->acl_cnt = hdev->acl_pkts; > > + else > > + hdev->acl_cnt += conn->sent; > > + } > > break; > > case CIS_LINK: > > case BIS_LINK: > > case PA_LINK: > > + if (!hci_conn_num(hdev, CIS_LINK) && > > + !hci_conn_num(hdev, BIS_LINK) && > > + !hci_conn_num(hdev, PA_LINK)) > > + hdev->iso_cnt = hdev->iso_pkts; > > + else > > + hdev->iso_cnt += conn->sent; > > hdev->iso_cnt += conn->sent; > > The last hdev->iso_cnt += conn->sent; probably should be removed. Opps, yeah that is a mistake, will fix that in v5. > > break; > > } > > -- > Pauli Virtanen -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 4/5] Bluetooth: ISO: Use sk_sndtimeo as conn_timeout 2025-08-13 21:15 [PATCH v3 1/5] Bluetooth: ISO: Don't initiate CIS connections if there are no buffers Luiz Augusto von Dentz 2025-08-13 21:15 ` [PATCH v3 2/5] Bluetooth: HCI: Fix using LE/ACL buffers for ISO packets Luiz Augusto von Dentz 2025-08-13 21:15 ` [PATCH v3 3/5] Bluetooth: hci_conn: Make unacked packet handling more robust Luiz Augusto von Dentz @ 2025-08-13 21:15 ` Luiz Augusto von Dentz 2025-08-13 21:15 ` [PATCH v3 5/5] Bluetooth: hci_core: Detect if an ISO link has stalled Luiz Augusto von Dentz ` (3 subsequent siblings) 6 siblings, 0 replies; 11+ messages in thread From: Luiz Augusto von Dentz @ 2025-08-13 21:15 UTC (permalink / raw) To: linux-bluetooth From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> This aligns the usage of socket sk_sndtimeo as conn_timeout when initiating a connection and then use it when scheduling the resulting HCI command, similar to what has been done in bf98feea5b65 ("Bluetooth: hci_conn: Always use sk_timeo as conn_timeout"). Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> --- include/net/bluetooth/hci_core.h | 10 ++++++---- net/bluetooth/hci_conn.c | 20 ++++++++++++-------- net/bluetooth/iso.c | 16 ++++++++++------ 3 files changed, 28 insertions(+), 18 deletions(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index bb30bde6f0e8..b060a6a76456 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -1546,16 +1546,18 @@ struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type, bdaddr_t *dst, __u16 setting, struct bt_codec *codec, u16 timeout); struct hci_conn *hci_bind_cis(struct hci_dev *hdev, bdaddr_t *dst, - __u8 dst_type, struct bt_iso_qos *qos); + __u8 dst_type, struct bt_iso_qos *qos, + u16 timeout); struct hci_conn *hci_bind_bis(struct hci_dev *hdev, bdaddr_t *dst, __u8 sid, struct bt_iso_qos *qos, - __u8 base_len, __u8 *base); + __u8 base_len, __u8 *base, u16 timeout); struct hci_conn *hci_connect_cis(struct hci_dev *hdev, bdaddr_t *dst, - __u8 dst_type, struct bt_iso_qos *qos); + __u8 dst_type, struct bt_iso_qos *qos, + u16 timeout); struct hci_conn *hci_connect_bis(struct hci_dev *hdev, bdaddr_t *dst, __u8 dst_type, __u8 sid, struct bt_iso_qos *qos, - __u8 data_len, __u8 *data); + __u8 data_len, __u8 *data, u16 timeout); struct hci_conn *hci_pa_create_sync(struct hci_dev *hdev, bdaddr_t *dst, __u8 dst_type, __u8 sid, struct bt_iso_qos *qos); int hci_conn_big_create_sync(struct hci_dev *hdev, struct hci_conn *hcon, diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index d2f0c3c0f0ae..05f97a17142e 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -1533,7 +1533,7 @@ static int qos_set_bis(struct hci_dev *hdev, struct bt_iso_qos *qos) /* This function requires the caller holds hdev->lock */ static struct hci_conn *hci_add_bis(struct hci_dev *hdev, bdaddr_t *dst, __u8 sid, struct bt_iso_qos *qos, - __u8 base_len, __u8 *base) + __u8 base_len, __u8 *base, u16 timeout) { struct hci_conn *conn; int err; @@ -1575,6 +1575,7 @@ static struct hci_conn *hci_add_bis(struct hci_dev *hdev, bdaddr_t *dst, conn->state = BT_CONNECT; conn->sid = sid; + conn->conn_timeout = timeout; hci_conn_hold(conn); return conn; @@ -1915,7 +1916,8 @@ static bool hci_le_set_cig_params(struct hci_conn *conn, struct bt_iso_qos *qos) } struct hci_conn *hci_bind_cis(struct hci_dev *hdev, bdaddr_t *dst, - __u8 dst_type, struct bt_iso_qos *qos) + __u8 dst_type, struct bt_iso_qos *qos, + u16 timeout) { struct hci_conn *cis; @@ -1930,6 +1932,7 @@ struct hci_conn *hci_bind_cis(struct hci_dev *hdev, bdaddr_t *dst, cis->dst_type = dst_type; cis->iso_qos.ucast.cig = BT_ISO_QOS_CIG_UNSET; cis->iso_qos.ucast.cis = BT_ISO_QOS_CIS_UNSET; + cis->conn_timeout = timeout; } if (cis->state == BT_CONNECTED) @@ -2169,7 +2172,7 @@ static void create_big_complete(struct hci_dev *hdev, void *data, int err) struct hci_conn *hci_bind_bis(struct hci_dev *hdev, bdaddr_t *dst, __u8 sid, struct bt_iso_qos *qos, - __u8 base_len, __u8 *base) + __u8 base_len, __u8 *base, u16 timeout) { struct hci_conn *conn; struct hci_conn *parent; @@ -2190,7 +2193,7 @@ struct hci_conn *hci_bind_bis(struct hci_dev *hdev, bdaddr_t *dst, __u8 sid, base, base_len); /* We need hci_conn object using the BDADDR_ANY as dst */ - conn = hci_add_bis(hdev, dst, sid, qos, base_len, eir); + conn = hci_add_bis(hdev, dst, sid, qos, base_len, eir, timeout); if (IS_ERR(conn)) return conn; @@ -2243,13 +2246,13 @@ static void bis_mark_per_adv(struct hci_conn *conn, void *data) struct hci_conn *hci_connect_bis(struct hci_dev *hdev, bdaddr_t *dst, __u8 dst_type, __u8 sid, struct bt_iso_qos *qos, - __u8 base_len, __u8 *base) + __u8 base_len, __u8 *base, u16 timeout) { struct hci_conn *conn; int err; struct iso_list_data data; - conn = hci_bind_bis(hdev, dst, sid, qos, base_len, base); + conn = hci_bind_bis(hdev, dst, sid, qos, base_len, base, timeout); if (IS_ERR(conn)) return conn; @@ -2292,7 +2295,8 @@ struct hci_conn *hci_connect_bis(struct hci_dev *hdev, bdaddr_t *dst, } struct hci_conn *hci_connect_cis(struct hci_dev *hdev, bdaddr_t *dst, - __u8 dst_type, struct bt_iso_qos *qos) + __u8 dst_type, struct bt_iso_qos *qos, + u16 timeout) { struct hci_conn *le; struct hci_conn *cis; @@ -2316,7 +2320,7 @@ struct hci_conn *hci_connect_cis(struct hci_dev *hdev, bdaddr_t *dst, hci_iso_qos_setup(hdev, le, &qos->ucast.in, le->le_rx_phy ? le->le_rx_phy : hdev->le_rx_def_phys); - cis = hci_bind_cis(hdev, dst, dst_type, qos); + cis = hci_bind_cis(hdev, dst, dst_type, qos, timeout); if (IS_ERR(cis)) { hci_conn_drop(le); return cis; diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c index dff3fc4917d6..672987e845d0 100644 --- a/net/bluetooth/iso.c +++ b/net/bluetooth/iso.c @@ -91,8 +91,8 @@ static struct sock *iso_get_sock(bdaddr_t *src, bdaddr_t *dst, iso_sock_match_t match, void *data); /* ---- ISO timers ---- */ -#define ISO_CONN_TIMEOUT (HZ * 40) -#define ISO_DISCONN_TIMEOUT (HZ * 2) +#define ISO_CONN_TIMEOUT secs_to_jiffies(20) +#define ISO_DISCONN_TIMEOUT secs_to_jiffies(2) static void iso_conn_free(struct kref *ref) { @@ -367,7 +367,8 @@ static int iso_connect_bis(struct sock *sk) if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) { hcon = hci_bind_bis(hdev, &iso_pi(sk)->dst, iso_pi(sk)->bc_sid, &iso_pi(sk)->qos, iso_pi(sk)->base_len, - iso_pi(sk)->base); + iso_pi(sk)->base, + READ_ONCE(sk->sk_sndtimeo)); if (IS_ERR(hcon)) { err = PTR_ERR(hcon); goto unlock; @@ -376,7 +377,8 @@ static int iso_connect_bis(struct sock *sk) hcon = hci_connect_bis(hdev, &iso_pi(sk)->dst, le_addr_type(iso_pi(sk)->dst_type), iso_pi(sk)->bc_sid, &iso_pi(sk)->qos, - iso_pi(sk)->base_len, iso_pi(sk)->base); + iso_pi(sk)->base_len, iso_pi(sk)->base, + READ_ONCE(sk->sk_sndtimeo)); if (IS_ERR(hcon)) { err = PTR_ERR(hcon); goto unlock; @@ -469,7 +471,8 @@ static int iso_connect_cis(struct sock *sk) if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) { hcon = hci_bind_cis(hdev, &iso_pi(sk)->dst, le_addr_type(iso_pi(sk)->dst_type), - &iso_pi(sk)->qos); + &iso_pi(sk)->qos, + READ_ONCE(sk->sk_sndtimeo)); if (IS_ERR(hcon)) { err = PTR_ERR(hcon); goto unlock; @@ -477,7 +480,8 @@ static int iso_connect_cis(struct sock *sk) } else { hcon = hci_connect_cis(hdev, &iso_pi(sk)->dst, le_addr_type(iso_pi(sk)->dst_type), - &iso_pi(sk)->qos); + &iso_pi(sk)->qos, + READ_ONCE(sk->sk_sndtimeo)); if (IS_ERR(hcon)) { err = PTR_ERR(hcon); goto unlock; -- 2.50.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 5/5] Bluetooth: hci_core: Detect if an ISO link has stalled 2025-08-13 21:15 [PATCH v3 1/5] Bluetooth: ISO: Don't initiate CIS connections if there are no buffers Luiz Augusto von Dentz ` (2 preceding siblings ...) 2025-08-13 21:15 ` [PATCH v3 4/5] Bluetooth: ISO: Use sk_sndtimeo as conn_timeout Luiz Augusto von Dentz @ 2025-08-13 21:15 ` Luiz Augusto von Dentz 2025-08-13 21:28 ` [PATCH v3 1/5] Bluetooth: ISO: Don't initiate CIS connections if there are no buffers Paul Menzel ` (2 subsequent siblings) 6 siblings, 0 replies; 11+ messages in thread From: Luiz Augusto von Dentz @ 2025-08-13 21:15 UTC (permalink / raw) To: linux-bluetooth From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> This attempts to detect if an ISO link has been waiting for an ISO buffer for longer than the maximum allowed transport latency then proceed to use hci_link_tx_to which prints an error and disconnects. Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> --- include/net/bluetooth/hci.h | 1 + include/net/bluetooth/hci_core.h | 1 + net/bluetooth/hci_core.c | 34 ++++++++++++++++++++++++-------- 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h index df1847b74e55..9ecc70baaca9 100644 --- a/include/net/bluetooth/hci.h +++ b/include/net/bluetooth/hci.h @@ -488,6 +488,7 @@ enum { #define HCI_AUTO_OFF_TIMEOUT msecs_to_jiffies(2000) /* 2 seconds */ #define HCI_ACL_CONN_TIMEOUT msecs_to_jiffies(20000) /* 20 seconds */ #define HCI_LE_CONN_TIMEOUT msecs_to_jiffies(20000) /* 20 seconds */ +#define HCI_ISO_TX_TIMEOUT usecs_to_jiffies(0x7fffff) /* 8388607 usecs */ /* HCI data types */ #define HCI_COMMAND_PKT 0x01 diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index b060a6a76456..242ff4033ac4 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -485,6 +485,7 @@ struct hci_dev { unsigned long acl_last_tx; unsigned long le_last_tx; + unsigned long iso_last_tx; __u8 le_tx_def_phys; __u8 le_rx_def_phys; diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index e2bffad9816f..4cf4bb1187dc 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -3585,24 +3585,37 @@ static void hci_prio_recalculate(struct hci_dev *hdev, __u8 type) static void __check_timeout(struct hci_dev *hdev, unsigned int cnt, u8 type) { - unsigned long last_tx; + unsigned long timeout; if (hci_dev_test_flag(hdev, HCI_UNCONFIGURED)) return; switch (type) { + case ACL_LINK: + /* tx timeout must be longer than maximum link supervision + * timeout (40.9 seconds) + */ + timeout = hdev->acl_last_tx + HCI_ACL_TX_TIMEOUT; + break; case LE_LINK: - last_tx = hdev->le_last_tx; + /* tx timeout must be longer than maximum link supervision + * timeout (40.9 seconds) + */ + timeout = hdev->le_last_tx + HCI_ACL_TX_TIMEOUT; + break; + case CIS_LINK: + case BIS_LINK: + case PA_LINK: + /* tx timeout must be longer than the maximum transport latency + * (8.388607 seconds) + */ + timeout = hdev->iso_last_tx + HCI_ISO_TX_TIMEOUT; break; default: - last_tx = hdev->acl_last_tx; - break; + return; } - /* tx timeout must be longer than maximum link supervision timeout - * (40.9 seconds) - */ - if (!cnt && time_after(jiffies, last_tx + HCI_ACL_TX_TIMEOUT)) + if (!cnt && time_after(jiffies, timeout)) hci_link_tx_to(hdev, type); } @@ -3759,10 +3772,15 @@ static void hci_sched_iso(struct hci_dev *hdev, __u8 type) return; cnt = &hdev->iso_cnt; + + __check_timeout(hdev, *cnt, type); + while (*cnt && (conn = hci_low_sent(hdev, type, "e))) { while (quote-- && (skb = skb_dequeue(&conn->data_q))) { BT_DBG("skb %p len %d", skb, skb->len); + hci_send_conn_frame(hdev, conn, skb); + hdev->iso_last_tx = jiffies; conn->sent++; if (conn->sent == ~0) -- 2.50.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/5] Bluetooth: ISO: Don't initiate CIS connections if there are no buffers 2025-08-13 21:15 [PATCH v3 1/5] Bluetooth: ISO: Don't initiate CIS connections if there are no buffers Luiz Augusto von Dentz ` (3 preceding siblings ...) 2025-08-13 21:15 ` [PATCH v3 5/5] Bluetooth: hci_core: Detect if an ISO link has stalled Luiz Augusto von Dentz @ 2025-08-13 21:28 ` Paul Menzel 2025-08-13 21:59 ` [v3,1/5] " bluez.test.bot 2025-08-14 15:26 ` [PATCH v3 1/5] " Pauli Virtanen 6 siblings, 0 replies; 11+ messages in thread From: Paul Menzel @ 2025-08-13 21:28 UTC (permalink / raw) To: Luiz Augusto von Dentz; +Cc: linux-bluetooth Dear Luiz, Thank you for your patch. Am 13.08.25 um 23:15 schrieb Luiz Augusto von Dentz: > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > If the controller has no buffers left return -ENOBUFF to indicate that > iso_cnt might be out of sync. > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > --- > net/bluetooth/iso.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c > index 5ce823ca3aaf..dff3fc4917d6 100644 > --- a/net/bluetooth/iso.c > +++ b/net/bluetooth/iso.c > @@ -458,6 +458,13 @@ static int iso_connect_cis(struct sock *sk) > goto unlock; > } > > + /* Check if there are available buffers for output/TX. */ > + if (iso_pi(sk)->qos.ucast.out.sdu && !hci_conn_num(hdev, CIS_LINK) && > + (hdev->iso_pkts && !hdev->iso_cnt)) { > + err = -ENOBUFS; > + goto unlock; > + } > + > /* Just bind if DEFER_SETUP has been set */ > if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) { > hcon = hci_bind_cis(hdev, &iso_pi(sk)->dst, Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de> Kind regards, Paul ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [v3,1/5] Bluetooth: ISO: Don't initiate CIS connections if there are no buffers 2025-08-13 21:15 [PATCH v3 1/5] Bluetooth: ISO: Don't initiate CIS connections if there are no buffers Luiz Augusto von Dentz ` (4 preceding siblings ...) 2025-08-13 21:28 ` [PATCH v3 1/5] Bluetooth: ISO: Don't initiate CIS connections if there are no buffers Paul Menzel @ 2025-08-13 21:59 ` bluez.test.bot 2025-08-14 15:26 ` [PATCH v3 1/5] " Pauli Virtanen 6 siblings, 0 replies; 11+ messages in thread From: bluez.test.bot @ 2025-08-13 21:59 UTC (permalink / raw) To: linux-bluetooth, luiz.dentz [-- Attachment #1: Type: text/plain, Size: 3408 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=991214 ---Test result--- Test Summary: CheckPatch PENDING 0.32 seconds GitLint PENDING 0.24 seconds SubjectPrefix PASS 0.57 seconds BuildKernel PASS 24.86 seconds CheckAllWarning PASS 27.83 seconds CheckSparse WARNING 31.55 seconds BuildKernel32 PASS 25.08 seconds TestRunnerSetup PASS 490.00 seconds TestRunner_l2cap-tester PASS 25.31 seconds TestRunner_iso-tester PASS 39.58 seconds TestRunner_bnep-tester PASS 6.19 seconds TestRunner_mgmt-tester FAIL 127.73 seconds TestRunner_rfcomm-tester PASS 9.44 seconds TestRunner_sco-tester PASS 15.11 seconds TestRunner_ioctl-tester PASS 10.26 seconds TestRunner_mesh-tester FAIL 9.61 seconds TestRunner_smp-tester PASS 8.59 seconds TestRunner_userchan-tester PASS 6.28 seconds IncrementalBuild PENDING 0.67 seconds Details ############################## Test: CheckPatch - PENDING Desc: Run checkpatch.pl script Output: ############################## Test: GitLint - PENDING Desc: Run gitlint Output: ############################## Test: CheckSparse - WARNING Desc: Run sparse tool with linux kernel Output: net/bluetooth/hci_core.c:85:9: warning: context imbalance in '__hci_dev_get' - different lock contexts for basic blocknet/bluetooth/hci_core.c: note: in included file (through include/linux/notifier.h, include/linux/memory_hotplug.h, include/linux/mmzone.h, include/linux/gfp.h, include/linux/xarray.h, include/linux/radix-tree.h, ...):net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):net/bluetooth/hci_core.c:85:9: warning: context imbalance in '__hci_dev_get' - different lock contexts for basic blocknet/bluetooth/hci_core.c: note: in included file (through include/linux/notifier.h, include/linux/memory_hotplug.h, include/linux/mmzone.h, include/linux/gfp.h, include/linux/xarray.h, include/linux/radix-tree.h, ...): ############################## Test: TestRunner_mgmt-tester - FAIL Desc: Run mgmt-tester with test-runner Output: Total: 490, Passed: 482 (98.4%), Failed: 4, Not Run: 4 Failed Test Cases Add Ext Advertising - Success 22 (LE -> off, Remove) Failed 0.130 seconds Set Device ID - Power off and Power on Failed 0.113 seconds Set Device ID - SSP off and Power on Failed 0.120 seconds LL Privacy - Add Device 3 (AL is full) Failed 0.220 seconds ############################## Test: TestRunner_mesh-tester - FAIL Desc: Run mesh-tester with test-runner Output: Total: 10, Passed: 8 (80.0%), Failed: 2, Not Run: 0 Failed Test Cases Mesh - Send cancel - 1 Timed out 1.926 seconds Mesh - Send cancel - 2 Failed 0.136 seconds ############################## Test: IncrementalBuild - PENDING Desc: Incremental build with the patches in the series Output: --- Regards, Linux Bluetooth ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/5] Bluetooth: ISO: Don't initiate CIS connections if there are no buffers 2025-08-13 21:15 [PATCH v3 1/5] Bluetooth: ISO: Don't initiate CIS connections if there are no buffers Luiz Augusto von Dentz ` (5 preceding siblings ...) 2025-08-13 21:59 ` [v3,1/5] " bluez.test.bot @ 2025-08-14 15:26 ` Pauli Virtanen 2025-08-14 15:44 ` Luiz Augusto von Dentz 6 siblings, 1 reply; 11+ messages in thread From: Pauli Virtanen @ 2025-08-14 15:26 UTC (permalink / raw) To: Luiz Augusto von Dentz, linux-bluetooth Hi, ke, 2025-08-13 kello 17:15 -0400, Luiz Augusto von Dentz kirjoitti: > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > If the controller has no buffers left return -ENOBUFF to indicate that > iso_cnt might be out of sync. > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > --- > net/bluetooth/iso.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c > index 5ce823ca3aaf..dff3fc4917d6 100644 > --- a/net/bluetooth/iso.c > +++ b/net/bluetooth/iso.c > @@ -458,6 +458,13 @@ static int iso_connect_cis(struct sock *sk) > goto unlock; > } > > + /* Check if there are available buffers for output/TX. */ > + if (iso_pi(sk)->qos.ucast.out.sdu && !hci_conn_num(hdev, CIS_LINK) && > + (hdev->iso_pkts && !hdev->iso_cnt)) { Also && !hci_conn_num(hdev, BIS_LINK) so it doesn't fail if a BIS is saturating the buffers? Does PA_LINK have ISO packet TX/RX? > + err = -ENOBUFS; > + goto unlock; > + } > + > /* Just bind if DEFER_SETUP has been set */ > if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) { > hcon = hci_bind_cis(hdev, &iso_pi(sk)->dst, -- Pauli Virtanen ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/5] Bluetooth: ISO: Don't initiate CIS connections if there are no buffers 2025-08-14 15:26 ` [PATCH v3 1/5] " Pauli Virtanen @ 2025-08-14 15:44 ` Luiz Augusto von Dentz 0 siblings, 0 replies; 11+ messages in thread From: Luiz Augusto von Dentz @ 2025-08-14 15:44 UTC (permalink / raw) To: Pauli Virtanen; +Cc: linux-bluetooth Hi Pauli, On Thu, Aug 14, 2025 at 11:26 AM Pauli Virtanen <pav@iki.fi> wrote: > > Hi, > > ke, 2025-08-13 kello 17:15 -0400, Luiz Augusto von Dentz kirjoitti: > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > > > If the controller has no buffers left return -ENOBUFF to indicate that > > iso_cnt might be out of sync. > > > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > --- > > net/bluetooth/iso.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c > > index 5ce823ca3aaf..dff3fc4917d6 100644 > > --- a/net/bluetooth/iso.c > > +++ b/net/bluetooth/iso.c > > @@ -458,6 +458,13 @@ static int iso_connect_cis(struct sock *sk) > > goto unlock; > > } > > > > + /* Check if there are available buffers for output/TX. */ > > + if (iso_pi(sk)->qos.ucast.out.sdu && !hci_conn_num(hdev, CIS_LINK) && > > + (hdev->iso_pkts && !hdev->iso_cnt)) { > > Also && !hci_conn_num(hdev, BIS_LINK) so it doesn't fail if a BIS is > saturating the buffers? Right, will add that on v5. > Does PA_LINK have ISO packet TX/RX? Don't think so but since it is used while establishing the BIS I guess it is safer that we check that as well. > > + err = -ENOBUFS; > > + goto unlock; > > + } > > + > > /* Just bind if DEFER_SETUP has been set */ > > if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) { > > hcon = hci_bind_cis(hdev, &iso_pi(sk)->dst, > > -- > Pauli Virtanen -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-08-14 15:45 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-08-13 21:15 [PATCH v3 1/5] Bluetooth: ISO: Don't initiate CIS connections if there are no buffers Luiz Augusto von Dentz 2025-08-13 21:15 ` [PATCH v3 2/5] Bluetooth: HCI: Fix using LE/ACL buffers for ISO packets Luiz Augusto von Dentz 2025-08-13 21:15 ` [PATCH v3 3/5] Bluetooth: hci_conn: Make unacked packet handling more robust Luiz Augusto von Dentz 2025-08-14 15:30 ` Pauli Virtanen 2025-08-14 15:45 ` Luiz Augusto von Dentz 2025-08-13 21:15 ` [PATCH v3 4/5] Bluetooth: ISO: Use sk_sndtimeo as conn_timeout Luiz Augusto von Dentz 2025-08-13 21:15 ` [PATCH v3 5/5] Bluetooth: hci_core: Detect if an ISO link has stalled Luiz Augusto von Dentz 2025-08-13 21:28 ` [PATCH v3 1/5] Bluetooth: ISO: Don't initiate CIS connections if there are no buffers Paul Menzel 2025-08-13 21:59 ` [v3,1/5] " bluez.test.bot 2025-08-14 15:26 ` [PATCH v3 1/5] " Pauli Virtanen 2025-08-14 15:44 ` Luiz Augusto von Dentz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox