From 767c92e17691d761b35a79447ca828d8423c32ee Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Sat, 7 Jan 2017 17:05:58 +0200 Subject: [PATCH] Bluetooth: Consolidate most outgoing ACL TX buffers into a single pool Having TX buffers split into numerous pools has the downside of increased memory consumption. This patch takes the initial step to consolidate these pools into a single one, saving about 248 bytes of RAM for a basic configuration. Change-Id: I449ba18b44a9a6af68e9a2c44f19a9286eb88b14 Signed-off-by: Johan Hedberg --- subsys/bluetooth/host/Kconfig | 38 ++++++++++-------- subsys/bluetooth/host/att.c | 54 +++----------------------- subsys/bluetooth/host/att_internal.h | 6 +++ subsys/bluetooth/host/conn.c | 27 ++++++------- subsys/bluetooth/host/gatt.c | 2 +- subsys/bluetooth/host/l2cap.c | 7 +--- subsys/bluetooth/host/l2cap_internal.h | 4 ++ subsys/bluetooth/host/smp.c | 6 +-- subsys/bluetooth/host/smp_null.c | 6 +-- tests/bluetooth/shell/arduino_101.conf | 2 +- tests/bluetooth/shell/prj.conf | 2 +- 11 files changed, 58 insertions(+), 96 deletions(-) diff --git a/subsys/bluetooth/host/Kconfig b/subsys/bluetooth/host/Kconfig index 293679afc5d..228dd33c339 100644 --- a/subsys/bluetooth/host/Kconfig +++ b/subsys/bluetooth/host/Kconfig @@ -135,15 +135,29 @@ config BLUETOOTH_CONN bool if BLUETOOTH_CONN -config BLUETOOTH_ATT_MTU - int "Attribute Protocol (ATT) channel MTU" - default 23 - default 50 if BLUETOOTH_SMP # BLUETOOTH_L2CAP_IN_MTU is big enough - # for two complete ACL packets - range 23 BLUETOOTH_RX_BUF_LEN +config BLUETOOTH_L2CAP_TX_BUF_COUNT + int "Number of L2CAP TX buffers" + default 3 + range 2 255 help - The MTU for the ATT channel. The minimum and default is 23, - whereas the maximum is limited by CONFIG_BLUETOOTH_RX_BUF_LEN. + Number of buffers available for outgoing L2CAP packets. + +config BLUETOOTH_L2CAP_TX_MTU + int "Maximum supported L2CAP MTU for L2CAP TX buffers" + default 23 + default 65 if BLUETOOTH_SMP + default 253 if BLUETOOTH_BREDR + range 23 2000 + range 65 2000 if BLUETOOTH_SMP + help + Maximum L2CAP MTU for L2CAP TX buffers. + +config BLUETOOTH_L2CAP_TX_USER_DATA_SIZE + int "Maximum supported user data size for L2CAP TX buffers" + default 4 + range 4 65535 + help + Maximum supported user data size for L2CAP TX buffers. config BLUETOOTH_ATT_PREPARE_COUNT int "Number of ATT prepare write buffers" @@ -153,14 +167,6 @@ config BLUETOOTH_ATT_PREPARE_COUNT Number of buffers available for ATT prepare write, setting this to 0 disables GATT long/reliable writes. -config BLUETOOTH_ATT_REQ_COUNT - int "Number of ATT request buffers" - default BLUETOOTH_MAX_CONN - range 1 64 - help - Number of outgoing buffers available for ATT requests, this controls - how many requests can be queued without blocking. - config BLUETOOTH_SMP bool "Security Manager Protocol support" select TINYCRYPT_AES diff --git a/subsys/bluetooth/host/att.c b/subsys/bluetooth/host/att.c index f8fe9ad4cde..76f9bedf714 100644 --- a/subsys/bluetooth/host/att.c +++ b/subsys/bluetooth/host/att.c @@ -66,10 +66,9 @@ struct bt_attr_data { uint16_t offset; }; -/* Pool for incoming ATT packets, MTU is 23 */ -NET_BUF_POOL_DEFINE(prep_pool, CONFIG_BLUETOOTH_ATT_PREPARE_COUNT, - CONFIG_BLUETOOTH_ATT_MTU, sizeof(struct bt_attr_data), - NULL); +/* Pool for incoming ATT packets */ +NET_BUF_POOL_DEFINE(prep_pool, CONFIG_BLUETOOTH_ATT_PREPARE_COUNT, BT_ATT_MTU, + sizeof(struct bt_attr_data), NULL); #endif /* CONFIG_BLUETOOTH_ATT_PREPARE_COUNT */ /* ATT channel specific context */ @@ -86,22 +85,6 @@ struct bt_att { static struct bt_att bt_req_pool[CONFIG_BLUETOOTH_MAX_CONN]; -/* - * Pool for outgoing ATT requests packets. - */ -NET_BUF_POOL_DEFINE(req_pool, CONFIG_BLUETOOTH_ATT_REQ_COUNT, - BT_L2CAP_BUF_SIZE(CONFIG_BLUETOOTH_ATT_MTU), - BT_BUF_USER_DATA_MIN, NULL); - -/* - * Pool for ougoing ATT responses packets. This is necessary in order not to - * block the RX thread since otherwise req_pool would have be used but buffers - * may only be freed after a response is received which would never happen if - * the RX thread is waiting a buffer causing a deadlock. - */ -NET_BUF_POOL_DEFINE(rsp_pool, 1, BT_L2CAP_BUF_SIZE(CONFIG_BLUETOOTH_ATT_MTU), - BT_BUF_USER_DATA_MIN, NULL); - static void att_req_destroy(struct bt_att_req *req) { if (req->buf) { @@ -163,7 +146,7 @@ static uint8_t att_mtu_req(struct bt_att *att, struct net_buf *buf) return BT_ATT_ERR_UNLIKELY; } - mtu_server = CONFIG_BLUETOOTH_ATT_MTU; + mtu_server = BT_ATT_MTU; BT_DBG("Server MTU %u", mtu_server); @@ -276,7 +259,7 @@ static uint8_t att_mtu_rsp(struct bt_att *att, struct net_buf *buf) return att_handle_rsp(att, NULL, 0, BT_ATT_ERR_INVALID_PDU); } - att->chan.rx.mtu = min(mtu, CONFIG_BLUETOOTH_ATT_MTU); + att->chan.rx.mtu = min(mtu, BT_ATT_MTU); /* BLUETOOTH SPECIFICATION Version 4.2 [Vol 3, Part F] page 484: * @@ -1763,32 +1746,7 @@ struct net_buf *bt_att_create_pdu(struct bt_conn *conn, uint8_t op, size_t len) return NULL; } - switch (op) { - case BT_ATT_OP_CONFIRM: - case BT_ATT_OP_ERROR_RSP: - case BT_ATT_OP_MTU_RSP: - case BT_ATT_OP_FIND_INFO_RSP: - case BT_ATT_OP_FIND_TYPE_RSP: - case BT_ATT_OP_READ_TYPE_RSP: - case BT_ATT_OP_READ_RSP: - case BT_ATT_OP_READ_BLOB_RSP: - case BT_ATT_OP_READ_MULT_RSP: - case BT_ATT_OP_READ_GROUP_RSP: - case BT_ATT_OP_WRITE_RSP: - case BT_ATT_OP_PREPARE_WRITE_RSP: - case BT_ATT_OP_EXEC_WRITE_RSP: - /* Use a different buffer pool for responses as this is - * usually sent from RX thread it shall never block. - */ - buf = bt_l2cap_create_pdu(&rsp_pool, 0); - break; - default: - buf = bt_l2cap_create_pdu(&req_pool, 0); - } - - if (!buf) { - return NULL; - } + buf = bt_l2cap_create_pdu(NULL, 0); hdr = net_buf_add(buf, sizeof(*hdr)); hdr->code = op; diff --git a/subsys/bluetooth/host/att_internal.h b/subsys/bluetooth/host/att_internal.h index 27855eb7efa..b70143dc836 100644 --- a/subsys/bluetooth/host/att_internal.h +++ b/subsys/bluetooth/host/att_internal.h @@ -18,6 +18,12 @@ #define BT_ATT_DEFAULT_LE_MTU 23 +#if BT_L2CAP_MTU(CONFIG_BLUETOOTH_RX_BUF_LEN) < CONFIG_BLUETOOTH_L2CAP_TX_MTU +#define BT_ATT_MTU BT_L2CAP_MTU(CONFIG_BLUETOOTH_RX_BUF_LEN) +#else +#define BT_ATT_MTU CONFIG_BLUETOOTH_L2CAP_TX_MTU +#endif + struct bt_att_hdr { uint8_t code; } __packed; diff --git a/subsys/bluetooth/host/conn.c b/subsys/bluetooth/host/conn.c index cc12c540051..cf3278c7004 100644 --- a/subsys/bluetooth/host/conn.c +++ b/subsys/bluetooth/host/conn.c @@ -43,12 +43,9 @@ #define BT_DBG(fmt, ...) #endif -/* Pool for outgoing ACL fragments */ -NET_BUF_POOL_DEFINE(frag_pool, 1, BT_L2CAP_BUF_SIZE(23), BT_BUF_USER_DATA_MIN, - NULL); - -/* Pool for dummy buffers to wake up the tx threads */ -NET_BUF_POOL_DEFINE(dummy_pool, CONFIG_BLUETOOTH_MAX_CONN, 0, 0, NULL); +NET_BUF_POOL_DEFINE(acl_tx_pool, CONFIG_BLUETOOTH_L2CAP_TX_BUF_COUNT, + BT_L2CAP_BUF_SIZE(CONFIG_BLUETOOTH_L2CAP_TX_MTU), + CONFIG_BLUETOOTH_L2CAP_TX_USER_DATA_SIZE, NULL); /* How long until we cancel HCI_LE_Create_Connection */ #define CONN_TIMEOUT K_SECONDS(3) @@ -948,7 +945,7 @@ static struct net_buf *create_frag(struct bt_conn *conn, struct net_buf *buf) struct net_buf *frag; uint16_t frag_len; - frag = bt_conn_create_pdu(&frag_pool, 0); + frag = bt_conn_create_pdu(NULL, 0); if (conn->state != BT_CONN_CONNECTED) { net_buf_unref(frag); @@ -1127,9 +1124,8 @@ void bt_conn_set_state(struct bt_conn *conn, bt_conn_state_t state) old_state == BT_CONN_DISCONNECT) { bt_l2cap_disconnected(conn); notify_disconnected(conn); - net_buf_put(&conn->tx_queue, - net_buf_alloc(&dummy_pool, K_NO_WAIT)); + bt_conn_create_pdu(NULL, 0)); } else if (old_state == BT_CONN_CONNECT) { /* conn->err will be set in this case */ notify_connected(conn); @@ -1575,13 +1571,18 @@ int bt_conn_le_conn_update(struct bt_conn *conn, struct net_buf *bt_conn_create_pdu(struct net_buf_pool *pool, size_t reserve) { - size_t head_reserve = reserve + sizeof(struct bt_hci_acl_hdr) + - CONFIG_BLUETOOTH_HCI_SEND_RESERVE; struct net_buf *buf; + if (!pool) { + pool = &acl_tx_pool; + } + buf = net_buf_alloc(pool, K_FOREVER); - /* NULL return is not possible because of K_FOREVER */ - net_buf_reserve(buf, head_reserve); + if (buf) { + reserve += sizeof(struct bt_hci_acl_hdr) + + CONFIG_BLUETOOTH_HCI_SEND_RESERVE; + net_buf_reserve(buf, reserve); + } return buf; } diff --git a/subsys/bluetooth/host/gatt.c b/subsys/bluetooth/host/gatt.c index c89610a3614..6b5cef93e20 100644 --- a/subsys/bluetooth/host/gatt.c +++ b/subsys/bluetooth/host/gatt.c @@ -785,7 +785,7 @@ int bt_gatt_exchange_mtu(struct bt_conn *conn, return -ENOMEM; } - mtu = CONFIG_BLUETOOTH_ATT_MTU; + mtu = BT_ATT_MTU; BT_DBG("Client MTU %u", mtu); diff --git a/subsys/bluetooth/host/l2cap.c b/subsys/bluetooth/host/l2cap.c index 7cf2074d631..22b8b8a4645 100644 --- a/subsys/bluetooth/host/l2cap.c +++ b/subsys/bluetooth/host/l2cap.c @@ -78,11 +78,6 @@ static struct bt_l2cap_fixed_chan *le_channels; static struct bt_l2cap_server *servers; #endif /* CONFIG_BLUETOOTH_L2CAP_DYNAMIC_CHANNEL */ -/* Pool for outgoing LE signaling packets, MTU is 23 */ -NET_BUF_POOL_DEFINE(le_sig_pool, CONFIG_BLUETOOTH_MAX_CONN, - BT_L2CAP_BUF_SIZE(L2CAP_LE_MIN_MTU), - BT_BUF_USER_DATA_MIN, NULL); - #if defined(CONFIG_BLUETOOTH_L2CAP_DYNAMIC_CHANNEL) /* Pool for outgoing LE data packets, MTU is 23 */ NET_BUF_POOL_DEFINE(le_data_pool, CONFIG_BLUETOOTH_MAX_CONN, @@ -401,7 +396,7 @@ static struct net_buf *l2cap_create_le_sig_pdu(uint8_t code, uint8_t ident, struct net_buf *buf; struct bt_l2cap_sig_hdr *hdr; - buf = bt_l2cap_create_pdu(&le_sig_pool, 0); + buf = bt_l2cap_create_pdu(NULL, 0); if (!buf) { return NULL; } diff --git a/subsys/bluetooth/host/l2cap_internal.h b/subsys/bluetooth/host/l2cap_internal.h index a110fd25228..ee63c22d2d4 100644 --- a/subsys/bluetooth/host/l2cap_internal.h +++ b/subsys/bluetooth/host/l2cap_internal.h @@ -207,6 +207,10 @@ struct bt_l2cap_le_credits { sizeof(struct bt_hci_acl_hdr) + \ sizeof(struct bt_l2cap_hdr) + (mtu)) +/* Helper to calculate max possible MTU from buffer size */ +#define BT_L2CAP_MTU(size) ((size) - CONFIG_BLUETOOTH_HCI_SEND_RESERVE - \ + 4 /* HCI ACL header */ - 4 /* L2CAP header */) + struct bt_l2cap_fixed_chan { uint16_t cid; int (*accept)(struct bt_conn *conn, struct bt_l2cap_chan **chan); diff --git a/subsys/bluetooth/host/smp.c b/subsys/bluetooth/host/smp.c index 46afa126458..a3c73aaf497 100644 --- a/subsys/bluetooth/host/smp.c +++ b/subsys/bluetooth/host/smp.c @@ -247,10 +247,6 @@ struct bt_smp_br { static struct bt_smp_br bt_smp_br_pool[CONFIG_BLUETOOTH_MAX_CONN]; #endif /* CONFIG_BLUETOOTH_BREDR */ -/* Pool for outgoing LE signaling packets, MTU is 65 */ -NET_BUF_POOL_DEFINE(smp_pool, CONFIG_BLUETOOTH_MAX_CONN, BT_L2CAP_BUF_SIZE(65), - BT_BUF_USER_DATA_MIN, NULL); - static struct bt_smp bt_smp_pool[CONFIG_BLUETOOTH_MAX_CONN]; static bool sc_supported; static bool sc_local_pkey_valid; @@ -336,7 +332,7 @@ static struct net_buf *smp_create_pdu(struct bt_conn *conn, uint8_t op, struct bt_smp_hdr *hdr; struct net_buf *buf; - buf = bt_l2cap_create_pdu(&smp_pool, 0); + buf = bt_l2cap_create_pdu(NULL, 0); /* NULL is not a possible return due to K_FOREVER */ hdr = net_buf_add(buf, sizeof(*hdr)); diff --git a/subsys/bluetooth/host/smp_null.c b/subsys/bluetooth/host/smp_null.c index 68406a1b2e3..9a791776bc2 100644 --- a/subsys/bluetooth/host/smp_null.c +++ b/subsys/bluetooth/host/smp_null.c @@ -36,10 +36,6 @@ static struct bt_l2cap_le_chan bt_smp_pool[CONFIG_BLUETOOTH_MAX_CONN]; -/* Pool for outgoing SMP signaling packets, MTU is 23 */ -NET_BUF_POOL_DEFINE(smp_pool, CONFIG_BLUETOOTH_MAX_CONN, BT_L2CAP_BUF_SIZE(23), - BT_BUF_USER_DATA_MIN, NULL); - int bt_smp_sign_verify(struct bt_conn *conn, struct net_buf *buf) { return -ENOTSUP; @@ -62,7 +58,7 @@ static void bt_smp_recv(struct bt_l2cap_chan *chan, struct net_buf *buf) * Core Specification Vol. 3, Part H, 3.3 */ - buf = bt_l2cap_create_pdu(&smp_pool, 0); + buf = bt_l2cap_create_pdu(NULL, 0); /* NULL is not a possible return due to K_FOREVER */ hdr = net_buf_add(buf, sizeof(*hdr)); diff --git a/tests/bluetooth/shell/arduino_101.conf b/tests/bluetooth/shell/arduino_101.conf index 9efb0361bdc..3480c43b121 100644 --- a/tests/bluetooth/shell/arduino_101.conf +++ b/tests/bluetooth/shell/arduino_101.conf @@ -12,7 +12,7 @@ CONFIG_BLUETOOTH_L2CAP_DYNAMIC_CHANNEL=y CONFIG_BLUETOOTH_TINYCRYPT_ECC=y CONFIG_CONSOLE_SHELL=y CONFIG_BLUETOOTH_BREDR_NAME="test shell" -CONFIG_BLUETOOTH_ATT_REQ_COUNT=5 +CONFIG_BLUETOOTH_L2CAP_TX_BUF_COUNT=6 CONFIG_BLUETOOTH_INTERNAL_STORAGE=y CONFIG_FLASH=y CONFIG_SPI=y diff --git a/tests/bluetooth/shell/prj.conf b/tests/bluetooth/shell/prj.conf index 878793e7ccc..0cdd29ce5ab 100644 --- a/tests/bluetooth/shell/prj.conf +++ b/tests/bluetooth/shell/prj.conf @@ -12,5 +12,5 @@ CONFIG_BLUETOOTH_L2CAP_DYNAMIC_CHANNEL=y CONFIG_BLUETOOTH_TINYCRYPT_ECC=y CONFIG_CONSOLE_SHELL=y CONFIG_BLUETOOTH_BREDR_NAME="test shell" -CONFIG_BLUETOOTH_ATT_REQ_COUNT=5 +CONFIG_BLUETOOTH_L2CAP_TX_BUF_COUNT=6