Skip to content

Commit e460847

Browse files
jori-nordiccarlescufi
authored andcommitted
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 <jonathan.rico@nordicsemi.no>
1 parent 75dd2c9 commit e460847

File tree

3 files changed

+67
-6
lines changed

3 files changed

+67
-6
lines changed

subsys/bluetooth/host/conn.c

Lines changed: 54 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ struct tx_meta {
5252
* continuations).
5353
*/
5454
bool is_cont;
55+
/* Indicates whether the ISO PDU contains a timestamp */
56+
bool iso_has_ts;
5557
};
5658

5759
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)
410412
return k_fifo_get(&free_tx, K_FOREVER);
411413
}
412414

415+
int bt_conn_send_iso_cb(struct bt_conn *conn, struct net_buf *buf,
416+
bt_conn_tx_cb_t cb, bool has_ts)
417+
{
418+
int err = bt_conn_send_cb(conn, buf, cb, NULL);
419+
420+
if (err) {
421+
return err;
422+
}
423+
424+
/* Necessary for setting the TS_Flag bit when we pop the buffer from the
425+
* send queue.
426+
* Size check for the user_data is already done in `bt_conn_send_cb`.
427+
*/
428+
tx_data(buf)->iso_has_ts = has_ts;
429+
430+
return 0;
431+
}
432+
413433
int bt_conn_send_cb(struct bt_conn *conn, struct net_buf *buf,
414434
bt_conn_tx_cb_t cb, void *user_data)
415435
{
@@ -419,7 +439,9 @@ int bt_conn_send_cb(struct bt_conn *conn, struct net_buf *buf,
419439
user_data);
420440

421441
if (buf->user_data_size < CONFIG_BT_CONN_TX_USER_DATA_SIZE) {
422-
LOG_ERR("not enough room in user_data");
442+
LOG_ERR("not enough room in user_data %d < %d",
443+
buf->user_data_size,
444+
CONFIG_BT_CONN_TX_USER_DATA_SIZE);
423445
return -EINVAL;
424446
}
425447

@@ -493,6 +515,7 @@ static int send_acl(struct bt_conn *conn, struct net_buf *buf, uint8_t flags)
493515
static int send_iso(struct bt_conn *conn, struct net_buf *buf, uint8_t flags)
494516
{
495517
struct bt_hci_iso_hdr *hdr;
518+
bool ts;
496519

497520
switch (flags) {
498521
case FRAG_START:
@@ -512,8 +535,12 @@ static int send_iso(struct bt_conn *conn, struct net_buf *buf, uint8_t flags)
512535
}
513536

514537
hdr = net_buf_push(buf, sizeof(*hdr));
515-
hdr->handle = sys_cpu_to_le16(bt_iso_handle_pack(conn->handle, flags,
516-
0));
538+
539+
ts = tx_data(buf)->iso_has_ts &&
540+
(flags == BT_ISO_START || flags == BT_ISO_SINGLE);
541+
542+
hdr->handle = sys_cpu_to_le16(bt_iso_handle_pack(conn->handle, flags, ts));
543+
517544
hdr->len = sys_cpu_to_le16(buf->len - sizeof(*hdr));
518545

519546
bt_buf_set_type(buf, BT_BUF_ISO_OUT);
@@ -621,6 +648,21 @@ static int do_send_frag(struct bt_conn *conn, struct net_buf *buf, uint8_t flags
621648
return err;
622649
}
623650

651+
static size_t iso_hdr_len(struct net_buf *buf, struct bt_conn *conn)
652+
{
653+
#if defined(CONFIG_BT_ISO)
654+
if (conn->type == BT_CONN_TYPE_ISO) {
655+
if (tx_data(buf)->iso_has_ts) {
656+
return BT_HCI_ISO_TS_DATA_HDR_SIZE;
657+
} else {
658+
return BT_HCI_ISO_DATA_HDR_SIZE;
659+
}
660+
}
661+
#endif
662+
663+
return 0;
664+
}
665+
624666
static int send_frag(struct bt_conn *conn,
625667
struct net_buf *buf, struct net_buf *frag,
626668
uint8_t flags)
@@ -633,7 +675,9 @@ static int send_frag(struct bt_conn *conn,
633675

634676
/* Add the data to the buffer */
635677
if (frag) {
636-
uint16_t frag_len = MIN(conn_mtu(conn), net_buf_tailroom(frag));
678+
size_t iso_hdr = flags == FRAG_START ? iso_hdr_len(buf, conn) : 0;
679+
uint16_t frag_len = MIN(conn_mtu(conn) + iso_hdr,
680+
net_buf_tailroom(frag));
637681

638682
net_buf_add_mem(frag, buf->data, frag_len);
639683
net_buf_pull(buf, frag_len);
@@ -681,6 +725,11 @@ static struct net_buf *create_frag(struct bt_conn *conn, struct net_buf *buf)
681725
return frag;
682726
}
683727

728+
static bool fits_single_ctlr_buf(struct net_buf *buf, struct bt_conn *conn)
729+
{
730+
return buf->len - iso_hdr_len(buf, conn) <= conn_mtu(conn);
731+
}
732+
684733
static int send_buf(struct bt_conn *conn, struct net_buf *buf)
685734
{
686735
struct net_buf *frag;
@@ -690,7 +739,7 @@ static int send_buf(struct bt_conn *conn, struct net_buf *buf)
690739
LOG_DBG("conn %p buf %p len %u", conn, buf, buf->len);
691740

692741
/* Send directly if the packet fits the ACL MTU */
693-
if (buf->len <= conn_mtu(conn) && !tx_data(buf)->is_cont) {
742+
if (fits_single_ctlr_buf(buf, conn) && !tx_data(buf)->is_cont) {
694743
LOG_DBG("send single");
695744
return send_frag(conn, buf, NULL, FRAG_SINGLE);
696745
}

subsys/bluetooth/host/conn_internal.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,15 @@ void bt_conn_recv(struct bt_conn *conn, struct net_buf *buf, uint8_t flags);
254254
int bt_conn_send_cb(struct bt_conn *conn, struct net_buf *buf,
255255
bt_conn_tx_cb_t cb, void *user_data);
256256

257+
/* Thin wrapper over `bt_conn_send_cb`
258+
*
259+
* Used to set the TS_Flag bit in `buf`'s metadata.
260+
*
261+
* Return values & buf ownership same as parent.
262+
*/
263+
int bt_conn_send_iso_cb(struct bt_conn *conn, struct net_buf *buf,
264+
bt_conn_tx_cb_t cb, bool has_ts);
265+
257266
static inline int bt_conn_send(struct bt_conn *conn, struct net_buf *buf)
258267
{
259268
return bt_conn_send_cb(conn, buf, NULL, NULL);

subsys/bluetooth/host/iso.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -803,7 +803,10 @@ int bt_iso_chan_send(struct bt_iso_chan *chan, struct net_buf *buf,
803803
BT_ISO_DATA_VALID));
804804
}
805805

806-
return bt_conn_send_cb(iso_conn, buf, bt_iso_send_cb, NULL);
806+
return bt_conn_send_iso_cb(iso_conn,
807+
buf,
808+
bt_iso_send_cb,
809+
ts != BT_ISO_TIMESTAMP_NONE);
807810
}
808811

809812
#if defined(CONFIG_BT_ISO_CENTRAL) || defined(CONFIG_BT_ISO_BROADCASTER)

0 commit comments

Comments
 (0)