From e460847b6083c0bc57bd63f63bc13c756207c2c9 Mon Sep 17 00:00:00 2001 From: Jonathan Rico Date: Mon, 24 Apr 2023 11:05:06 +0200 Subject: [PATCH] Bluetooth: host: don't fragment ISO if len <= MTU MTU doesn't count against the ISO and ISO data headers. Then a config with CONFIG_BT_ISO_TX_MTU == CONFIG_BT_CTLR_ISO_TX_BUFFER_SIZE should not fragment SDUs over HCI. Also set the TS_Flag bit if a timestamp is present. Fixes #56749 Signed-off-by: Jonathan Rico --- subsys/bluetooth/host/conn.c | 59 ++++++++++++++++++++++++--- subsys/bluetooth/host/conn_internal.h | 9 ++++ subsys/bluetooth/host/iso.c | 5 ++- 3 files changed, 67 insertions(+), 6 deletions(-) diff --git a/subsys/bluetooth/host/conn.c b/subsys/bluetooth/host/conn.c index 10d894a4444..152c3aa5d10 100644 --- a/subsys/bluetooth/host/conn.c +++ b/subsys/bluetooth/host/conn.c @@ -52,6 +52,8 @@ struct tx_meta { * continuations). */ bool is_cont; + /* Indicates whether the ISO PDU contains a timestamp */ + bool iso_has_ts; }; BUILD_ASSERT(sizeof(struct tx_meta) == CONFIG_BT_CONN_TX_USER_DATA_SIZE, @@ -410,6 +412,24 @@ static struct bt_conn_tx *conn_tx_alloc(void) return k_fifo_get(&free_tx, K_FOREVER); } +int bt_conn_send_iso_cb(struct bt_conn *conn, struct net_buf *buf, + bt_conn_tx_cb_t cb, bool has_ts) +{ + int err = bt_conn_send_cb(conn, buf, cb, NULL); + + if (err) { + return err; + } + + /* Necessary for setting the TS_Flag bit when we pop the buffer from the + * send queue. + * Size check for the user_data is already done in `bt_conn_send_cb`. + */ + tx_data(buf)->iso_has_ts = has_ts; + + return 0; +} + int bt_conn_send_cb(struct bt_conn *conn, struct net_buf *buf, bt_conn_tx_cb_t cb, void *user_data) { @@ -419,7 +439,9 @@ int bt_conn_send_cb(struct bt_conn *conn, struct net_buf *buf, user_data); if (buf->user_data_size < CONFIG_BT_CONN_TX_USER_DATA_SIZE) { - LOG_ERR("not enough room in user_data"); + LOG_ERR("not enough room in user_data %d < %d", + buf->user_data_size, + CONFIG_BT_CONN_TX_USER_DATA_SIZE); return -EINVAL; } @@ -493,6 +515,7 @@ static int send_acl(struct bt_conn *conn, struct net_buf *buf, uint8_t flags) static int send_iso(struct bt_conn *conn, struct net_buf *buf, uint8_t flags) { struct bt_hci_iso_hdr *hdr; + bool ts; switch (flags) { case FRAG_START: @@ -512,8 +535,12 @@ static int send_iso(struct bt_conn *conn, struct net_buf *buf, uint8_t flags) } hdr = net_buf_push(buf, sizeof(*hdr)); - hdr->handle = sys_cpu_to_le16(bt_iso_handle_pack(conn->handle, flags, - 0)); + + ts = tx_data(buf)->iso_has_ts && + (flags == BT_ISO_START || flags == BT_ISO_SINGLE); + + hdr->handle = sys_cpu_to_le16(bt_iso_handle_pack(conn->handle, flags, ts)); + hdr->len = sys_cpu_to_le16(buf->len - sizeof(*hdr)); bt_buf_set_type(buf, BT_BUF_ISO_OUT); @@ -621,6 +648,21 @@ fail: return err; } +static size_t iso_hdr_len(struct net_buf *buf, struct bt_conn *conn) +{ +#if defined(CONFIG_BT_ISO) + if (conn->type == BT_CONN_TYPE_ISO) { + if (tx_data(buf)->iso_has_ts) { + return BT_HCI_ISO_TS_DATA_HDR_SIZE; + } else { + return BT_HCI_ISO_DATA_HDR_SIZE; + } + } +#endif + + return 0; +} + static int send_frag(struct bt_conn *conn, struct net_buf *buf, struct net_buf *frag, uint8_t flags) @@ -633,7 +675,9 @@ static int send_frag(struct bt_conn *conn, /* Add the data to the buffer */ if (frag) { - uint16_t frag_len = MIN(conn_mtu(conn), net_buf_tailroom(frag)); + size_t iso_hdr = flags == FRAG_START ? iso_hdr_len(buf, conn) : 0; + uint16_t frag_len = MIN(conn_mtu(conn) + iso_hdr, + net_buf_tailroom(frag)); net_buf_add_mem(frag, buf->data, frag_len); net_buf_pull(buf, frag_len); @@ -681,6 +725,11 @@ static struct net_buf *create_frag(struct bt_conn *conn, struct net_buf *buf) return frag; } +static bool fits_single_ctlr_buf(struct net_buf *buf, struct bt_conn *conn) +{ + return buf->len - iso_hdr_len(buf, conn) <= conn_mtu(conn); +} + static int send_buf(struct bt_conn *conn, struct net_buf *buf) { struct net_buf *frag; @@ -690,7 +739,7 @@ static int send_buf(struct bt_conn *conn, struct net_buf *buf) LOG_DBG("conn %p buf %p len %u", conn, buf, buf->len); /* Send directly if the packet fits the ACL MTU */ - if (buf->len <= conn_mtu(conn) && !tx_data(buf)->is_cont) { + if (fits_single_ctlr_buf(buf, conn) && !tx_data(buf)->is_cont) { LOG_DBG("send single"); return send_frag(conn, buf, NULL, FRAG_SINGLE); } diff --git a/subsys/bluetooth/host/conn_internal.h b/subsys/bluetooth/host/conn_internal.h index e34c786d382..ce89a65dd45 100644 --- a/subsys/bluetooth/host/conn_internal.h +++ b/subsys/bluetooth/host/conn_internal.h @@ -254,6 +254,15 @@ void bt_conn_recv(struct bt_conn *conn, struct net_buf *buf, uint8_t flags); int bt_conn_send_cb(struct bt_conn *conn, struct net_buf *buf, bt_conn_tx_cb_t cb, void *user_data); +/* Thin wrapper over `bt_conn_send_cb` + * + * Used to set the TS_Flag bit in `buf`'s metadata. + * + * Return values & buf ownership same as parent. + */ +int bt_conn_send_iso_cb(struct bt_conn *conn, struct net_buf *buf, + bt_conn_tx_cb_t cb, bool has_ts); + static inline int bt_conn_send(struct bt_conn *conn, struct net_buf *buf) { return bt_conn_send_cb(conn, buf, NULL, NULL); diff --git a/subsys/bluetooth/host/iso.c b/subsys/bluetooth/host/iso.c index 480ffddce4d..02bb0ce4844 100644 --- a/subsys/bluetooth/host/iso.c +++ b/subsys/bluetooth/host/iso.c @@ -803,7 +803,10 @@ int bt_iso_chan_send(struct bt_iso_chan *chan, struct net_buf *buf, BT_ISO_DATA_VALID)); } - return bt_conn_send_cb(iso_conn, buf, bt_iso_send_cb, NULL); + return bt_conn_send_iso_cb(iso_conn, + buf, + bt_iso_send_cb, + ts != BT_ISO_TIMESTAMP_NONE); } #if defined(CONFIG_BT_ISO_CENTRAL) || defined(CONFIG_BT_ISO_BROADCASTER)