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 zephyrproject-rtos#21103 Signed-off-by: Luiz Augusto von Dentz <[email protected]>
niziak · Jan 9, 2020 · d278cdc · d278cdc
1 parent f256625
commit d278cdc
Show file tree

Hide file tree

Showing 6 changed files with 73 additions and 42 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);
 	}