From d278cdc3d7a15f78130cf432167fc1f6b669cf91 Mon Sep 17 00:00:00 2001 From: Luiz Augusto von Dentz Date: Mon, 30 Dec 2019 10:45:35 -0800 Subject: [PATCH] Bluetooth: GATT: Allocate request from a memory slab This should reduce the footprint on applications that do a lot of requests i.e have a lot of subscriptions. Fixes #21103 Signed-off-by: Luiz Augusto von Dentz --- include/bluetooth/att.h | 17 ---------- include/bluetooth/gatt.h | 6 ---- subsys/bluetooth/host/Kconfig.gatt | 2 +- subsys/bluetooth/host/att.c | 49 +++++++++++++++++++++------- subsys/bluetooth/host/att_internal.h | 29 +++++++++++++--- subsys/bluetooth/host/gatt.c | 12 ++++++- 6 files changed, 73 insertions(+), 42 deletions(-) diff --git a/include/bluetooth/att.h b/include/bluetooth/att.h index 6e934503b7f..625478bd434 100644 --- a/include/bluetooth/att.h +++ b/include/bluetooth/att.h @@ -43,23 +43,6 @@ extern "C" { #define BT_ATT_ERR_PROCEDURE_IN_PROGRESS 0xfe #define BT_ATT_ERR_OUT_OF_RANGE 0xff -typedef void (*bt_att_func_t)(struct bt_conn *conn, u8_t err, - const void *pdu, u16_t length, - void *user_data); -typedef void (*bt_att_destroy_t)(void *user_data); - -/* ATT request context */ -struct bt_att_req { - sys_snode_t node; - bt_att_func_t func; - bt_att_destroy_t destroy; - struct net_buf_simple_state state; - struct net_buf *buf; -#if defined(CONFIG_BT_SMP) - bool retrying; -#endif /* CONFIG_BT_SMP */ -}; - #ifdef __cplusplus } #endif diff --git a/include/bluetooth/gatt.h b/include/bluetooth/gatt.h index 7446288c8ef..4b9750866ee 100644 --- a/include/bluetooth/gatt.h +++ b/include/bluetooth/gatt.h @@ -879,7 +879,6 @@ typedef void (*bt_gatt_indicate_func_t)(struct bt_conn *conn, /** @brief GATT Indicate Value parameters */ struct bt_gatt_indicate_params { - struct bt_att_req _req; /** Notification Attribute UUID type */ const struct bt_uuid *uuid; /** Indicate Attribute object*/ @@ -963,7 +962,6 @@ u16_t bt_gatt_get_mtu(struct bt_conn *conn); /** @brief GATT Exchange MTU parameters */ struct bt_gatt_exchange_params { - struct bt_att_req _req; /** Response callback */ void (*func)(struct bt_conn *conn, u8_t err, struct bt_gatt_exchange_params *params); @@ -1040,7 +1038,6 @@ enum { /** @brief GATT Discover Attributes parameters */ struct bt_gatt_discover_params { - struct bt_att_req _req; /** Discover UUID type */ struct bt_uuid *uuid; /** Discover attribute callback */ @@ -1121,7 +1118,6 @@ typedef u8_t (*bt_gatt_read_func_t)(struct bt_conn *conn, u8_t err, * @param uuid 2 or 16 octet UUID */ struct bt_gatt_read_params { - struct bt_att_req _req; bt_gatt_read_func_t func; size_t handle_count; union { @@ -1174,7 +1170,6 @@ typedef void (*bt_gatt_write_func_t)(struct bt_conn *conn, u8_t err, /** @brief GATT Write parameters */ struct bt_gatt_write_params { - struct bt_att_req _req; /** Response callback */ bt_gatt_write_func_t func; /** Attribute handle */ @@ -1308,7 +1303,6 @@ enum { /** @brief GATT Subscribe parameters */ struct bt_gatt_subscribe_params { - struct bt_att_req _req; bt_addr_le_t _peer; /** Notification value callback */ bt_gatt_notify_func_t notify; diff --git a/subsys/bluetooth/host/Kconfig.gatt b/subsys/bluetooth/host/Kconfig.gatt index 87e765b40c3..70eb5fe4505 100644 --- a/subsys/bluetooth/host/Kconfig.gatt +++ b/subsys/bluetooth/host/Kconfig.gatt @@ -26,7 +26,7 @@ config BT_ATT_PREPARE_COUNT config BT_ATT_TX_MAX int "Maximum number of queued outgoing ATT PDUs" - default 2 + default BT_L2CAP_TX_BUF_COUNT range 1 BT_L2CAP_TX_BUF_COUNT help Number of ATT PDUs that can be at a single moment queued for diff --git a/subsys/bluetooth/host/att.c b/subsys/bluetooth/host/att.c index de266b07930..28d1c9e376b 100644 --- a/subsys/bluetooth/host/att.c +++ b/subsys/bluetooth/host/att.c @@ -36,8 +36,6 @@ #define ATT_CMD_MASK 0x40 -#define ATT_TIMEOUT K_SECONDS(30) - typedef enum __packed { ATT_COMMAND, ATT_REQUEST, @@ -61,6 +59,9 @@ NET_BUF_POOL_DEFINE(prep_pool, CONFIG_BT_ATT_PREPARE_COUNT, BT_ATT_MTU, sizeof(struct bt_attr_data), NULL); #endif /* CONFIG_BT_ATT_PREPARE_COUNT */ +K_MEM_SLAB_DEFINE(req_slab, sizeof(struct bt_att_req), + CONFIG_BT_ATT_TX_MAX, 16); + enum { ATT_PENDING_RSP, ATT_PENDING_CFM, @@ -100,7 +101,7 @@ static void att_req_destroy(struct bt_att_req *req) req->destroy(req); } - (void)memset(req, 0, sizeof(*req)); + bt_att_req_free(req); } static struct bt_att *att_get(struct bt_conn *conn) @@ -197,7 +198,7 @@ void att_req_sent(struct bt_conn *conn, void *user_data) /* Start timeout work */ if (att->req) { - k_delayed_work_submit(&att->timeout_work, ATT_TIMEOUT); + k_delayed_work_submit(&att->timeout_work, BT_ATT_TIMEOUT); } att_pdu_sent(conn, user_data); @@ -343,6 +344,7 @@ static void att_process(struct bt_att *att) static u8_t att_handle_rsp(struct bt_att *att, void *pdu, u16_t len, u8_t err) { bt_att_func_t func; + void *params; BT_DBG("err 0x%02x len %u: %s", err, len, bt_hex(pdu, len)); @@ -369,16 +371,14 @@ static u8_t att_handle_rsp(struct bt_att *att, void *pdu, u16_t len, u8_t err) /* Reset func so it can be reused by the callback */ func = att->req->func; att->req->func = NULL; + params = att->req->user_data; - func(att->chan.chan.conn, err, pdu, len, att->req); - - /* Don't destroy if callback had reused the request */ - if (!att->req->func) { - att_req_destroy(att->req); - } - + /* free allocated request so its memory can be reused */ + att_req_destroy(att->req); att->req = NULL; + func(att->chan.chan.conn, err, pdu, len, params); + process: /* Process pending requests */ att_process(att); @@ -2045,7 +2045,7 @@ struct net_buf *bt_att_create_pdu(struct bt_conn *conn, u8_t op, size_t len) case ATT_RESPONSE: case ATT_CONFIRMATION: /* Use a timeout only when responding/confirming */ - buf = bt_l2cap_create_pdu_timeout(NULL, 0, ATT_TIMEOUT); + buf = bt_l2cap_create_pdu_timeout(NULL, 0, BT_ATT_TIMEOUT); break; default: buf = bt_l2cap_create_pdu(NULL, 0); @@ -2261,6 +2261,31 @@ u16_t bt_att_get_mtu(struct bt_conn *conn) return att->chan.tx.mtu; } +struct bt_att_req *bt_att_req_alloc(s32_t timeout) +{ + struct bt_att_req *req = NULL; + + /* Reserve space for request */ + if (k_mem_slab_alloc(&req_slab, (void **)&req, timeout)) { + return NULL; + } + + BT_DBG("req %p", req); + + req->func = NULL; + req->destroy = NULL; + req->user_data = NULL; + + return req; +} + +void bt_att_req_free(struct bt_att_req *req) +{ + BT_DBG("req %p", req); + + k_mem_slab_free(&req_slab, (void **)&req); +} + int bt_att_send(struct bt_conn *conn, struct net_buf *buf, bt_conn_tx_cb_t cb, void *user_data) { diff --git a/subsys/bluetooth/host/att_internal.h b/subsys/bluetooth/host/att_internal.h index 52f499a5a56..95eda16cfb4 100644 --- a/subsys/bluetooth/host/att_internal.h +++ b/subsys/bluetooth/host/att_internal.h @@ -7,6 +7,7 @@ */ #define BT_ATT_DEFAULT_LE_MTU 23 +#define BT_ATT_TIMEOUT K_SECONDS(30) #if BT_L2CAP_RX_MTU < CONFIG_BT_L2CAP_TX_MTU #define BT_ATT_MTU BT_L2CAP_RX_MTU @@ -236,19 +237,37 @@ struct bt_att_signed_write_cmd { u8_t value[0]; } __packed; -void att_pdu_sent(struct bt_conn *conn, void *user_data); +typedef void (*bt_att_func_t)(struct bt_conn *conn, u8_t err, + const void *pdu, u16_t length, + void *user_data); +typedef void (*bt_att_destroy_t)(void *user_data); -void att_cfm_sent(struct bt_conn *conn, void *user_data); +/* ATT request context */ +struct bt_att_req { + sys_snode_t node; + bt_att_func_t func; + bt_att_destroy_t destroy; + struct net_buf_simple_state state; + struct net_buf *buf; +#if defined(CONFIG_BT_SMP) + bool retrying; +#endif /* CONFIG_BT_SMP */ + void *user_data; +}; -void att_rsp_sent(struct bt_conn *conn, void *user_data); - -void att_req_sent(struct bt_conn *conn, void *user_data); +void att_sent(struct bt_conn *conn, void *user_data); void bt_att_init(void); u16_t bt_att_get_mtu(struct bt_conn *conn); struct net_buf *bt_att_create_pdu(struct bt_conn *conn, u8_t op, size_t len); +/* Allocate a new request */ +struct bt_att_req *bt_att_req_alloc(s32_t timeout); + +/* Free a request */ +void bt_att_req_free(struct bt_att_req *req); + /* Send ATT PDU over a connection */ int bt_att_send(struct bt_conn *conn, struct net_buf *buf, bt_conn_tx_cb_t cb, void *user_data); diff --git a/subsys/bluetooth/host/gatt.c b/subsys/bluetooth/host/gatt.c index f561507e0d4..a08462b4bf8 100644 --- a/subsys/bluetooth/host/gatt.c +++ b/subsys/bluetooth/host/gatt.c @@ -1602,13 +1602,23 @@ static int gatt_send(struct bt_conn *conn, struct net_buf *buf, int err; if (params) { - struct bt_att_req *req = params; + struct bt_att_req *req; + + /* Allocate new request */ + req = bt_att_req_alloc(BT_ATT_TIMEOUT); + if (!req) { + return -ENOMEM; + } req->buf = buf; req->func = func; req->destroy = destroy; + req->user_data = params; err = bt_att_req_send(conn, req); + if (err) { + bt_att_req_free(req); + } } else { err = bt_att_send(conn, buf, NULL, NULL); }