Bluetooth: GATT: Remove _peer member from bt_gatt_subscribe_params

This further reduce the overhead on each subscription at expense of
having a dedicated array to store subscriptions, the code now maintain
a separate list for each peer which should also scale better with large
number of subscriptions to different peers.

Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

include/bluetooth/gatt.h

@@ -1303,7 +1303,6 @@ enum {
 
 /** @brief GATT Subscribe parameters */
 struct bt_gatt_subscribe_params {
-	bt_addr_le_t _peer;
 	/** Notification value callback */
 	bt_gatt_notify_func_t notify;
 	/** Subscribe value handle */

subsys/bluetooth/host/gatt.c

@@ -59,7 +59,12 @@ struct ccc_store {
 };
 
 #if defined(CONFIG_BT_GATT_CLIENT)
-static sys_slist_t subscriptions;
+struct gatt_sub {
+	bt_addr_le_t peer;
+	sys_slist_t list;
+};
+
+static struct gatt_sub subscriptions[CONFIG_BT_MAX_PAIRED + CONFIG_BT_MAX_CONN];
 #endif /* CONFIG_BT_GATT_CLIENT */
 
 static const u16_t gap_appearance = CONFIG_BT_DEVICE_APPEARANCE;
@@ -2181,16 +2186,64 @@ bool bt_gatt_is_subscribed(struct bt_conn *conn,
 }
 
 #if defined(CONFIG_BT_GATT_CLIENT)
+static struct gatt_sub *gatt_sub_find_free(struct bt_conn *conn,
+					   struct gatt_sub **free_sub)
+{
+	int i;
+
+	if (free_sub) {
+		*free_sub = NULL;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(subscriptions); i++) {
+		struct gatt_sub *sub = &subscriptions[i];
+
+		if (!bt_conn_addr_le_cmp(conn, &sub->peer)) {
+			return sub;
+		} else if (free_sub &&
+			   !bt_addr_le_cmp(BT_ADDR_LE_ANY, &sub->peer)) {
+			*free_sub = sub;
+		}
+	}
+
+	return NULL;
+}
+
+#define gatt_sub_find(_conn) \
+	gatt_sub_find_free(_conn, NULL)
+
+static struct gatt_sub *gatt_sub_add(struct bt_conn *conn)
+{
+	struct gatt_sub *sub, *free_sub;
+
+	sub = gatt_sub_find_free(conn, &free_sub);
+	if (sub) {
+		return sub;
+	}
+
+	if (free_sub) {
+		bt_addr_le_copy(&free_sub->peer, &conn->le.dst);
+		return free_sub;
+	}
+
+	return NULL;
+}
+
 void bt_gatt_notification(struct bt_conn *conn, u16_t handle,
 			  const void *data, u16_t length)
 {
 	struct bt_gatt_subscribe_params *params, *tmp;
+	struct gatt_sub *sub;
 
 	BT_DBG("handle 0x%04x length %u", handle, length);
 
-	SYS_SLIST_FOR_EACH_CONTAINER_SAFE(&subscriptions, params, tmp, node) {
+	sub = gatt_sub_find(conn);
-		if (bt_conn_addr_le_cmp(conn, &params->_peer) ||
+	if (!sub) {
-		    handle != params->value_handle) {
+		return;
+	}
+
+	SYS_SLIST_FOR_EACH_CONTAINER_SAFE(&sub->list, params, tmp, node) {
+		if (handle != params->value_handle) {
 			continue;
 		}
 
@@ -2201,46 +2254,54 @@ void bt_gatt_notification(struct bt_conn *conn, u16_t handle,
 	}
 }
 
-static void update_subscription(struct bt_conn *conn,
+static void gatt_sub_update(struct bt_conn *conn, struct gatt_sub *sub)
-				 struct bt_gatt_subscribe_params *params)
 {
-	if (params->_peer.type == BT_ADDR_LE_PUBLIC) {
+	if (sub->peer.type == BT_ADDR_LE_PUBLIC) {
 		return;
 	}
 
 	/* Update address */
-	bt_addr_le_copy(&params->_peer, &conn->le.dst);
+	bt_addr_le_copy(&sub->peer, &conn->le.dst);
 }
 
-static void gatt_subscription_remove(struct bt_conn *conn, sys_snode_t *prev,
+static void gatt_sub_remove(struct bt_conn *conn, struct gatt_sub *sub,
-				     struct bt_gatt_subscribe_params *params)
+			    sys_snode_t *prev,
+			    struct bt_gatt_subscribe_params *params)
 {
-	/* Remove subscription from the list*/
+	if (params) {
-	sys_slist_remove(&subscriptions, prev, &params->node);
+		/* Remove subscription from the list*/
+		sys_slist_remove(&sub->list, prev, &params->node);
+		/* Notify removal */
+		params->notify(conn, params, NULL, 0);
+	}
 
-	params->notify(conn, params, NULL, 0);
+	if (sys_slist_is_empty(&sub->list)) {
+		/* Reset address if there are no subscription left */
+		bt_addr_le_copy(&sub->peer, BT_ADDR_LE_ANY);
+	}
 }
 
 static void remove_subscriptions(struct bt_conn *conn)
 {
+	struct gatt_sub *sub;
 	struct bt_gatt_subscribe_params *params, *tmp;
 	sys_snode_t *prev = NULL;
 
-	/* Lookup existing subscriptions */
+	sub = gatt_sub_find(conn);
-	SYS_SLIST_FOR_EACH_CONTAINER_SAFE(&subscriptions, params, tmp, node) {
+	if (!sub) {
-		if (bt_conn_addr_le_cmp(conn, &params->_peer)) {
+		return;
-			prev = &params->node;
+	}
-			continue;
-		}
 
+	/* Lookup existing subscriptions */
+	SYS_SLIST_FOR_EACH_CONTAINER_SAFE(&sub->list, params, tmp, node) {
 		if (!bt_addr_le_is_bonded(conn->id, &conn->le.dst) ||
 		    (atomic_test_bit(params->flags,
 				     BT_GATT_SUBSCRIBE_FLAG_VOLATILE))) {
 			/* Remove subscription */
 			params->value = 0U;
-			gatt_subscription_remove(conn, prev, params);
+			gatt_sub_remove(conn, sub, prev, params);
 		} else {
-			update_subscription(conn, params);
+			gatt_sub_update(conn, sub);
 			prev = &params->node;
 		}
 	}
@@ -3410,15 +3471,6 @@ int bt_gatt_write(struct bt_conn *conn, struct bt_gatt_write_params *params)
 	return gatt_send(conn, buf, gatt_write_rsp, params, NULL);
 }
 
-static void gatt_subscription_add(struct bt_conn *conn,
-				  struct bt_gatt_subscribe_params *params)
-{
-	bt_addr_le_copy(&params->_peer, &conn->le.dst);
-
-	/* Prepend subscription */
-	sys_slist_prepend(&subscriptions, &params->node);
-}
-
 static void gatt_write_ccc_rsp(struct bt_conn *conn, u8_t err,
 			       const void *pdu, u16_t length,
 			       void *user_data)
@@ -3431,11 +3483,17 @@ static void gatt_write_ccc_rsp(struct bt_conn *conn, u8_t err,
 
 	/* if write to CCC failed we remove subscription and notify app */
 	if (err) {
+		struct gatt_sub *sub;
 		sys_snode_t *node, *tmp, *prev = NULL;
 
-		SYS_SLIST_FOR_EACH_NODE_SAFE(&subscriptions, node, tmp) {
+		sub = gatt_sub_find(conn);
+		if (!sub) {
+			return;
+		}
+
+		SYS_SLIST_FOR_EACH_NODE_SAFE(&sub->list, node, tmp) {
 			if (node == &params->node) {
-				gatt_subscription_remove(conn, prev, params);
+				gatt_sub_remove(conn, sub, tmp, params);
 				break;
 			}
 
@@ -3474,6 +3532,7 @@ static int gatt_write_ccc(struct bt_conn *conn, u16_t handle, u16_t value,
 int bt_gatt_subscribe(struct bt_conn *conn,
 		      struct bt_gatt_subscribe_params *params)
 {
+	struct gatt_sub *sub;
 	struct bt_gatt_subscribe_params *tmp;
 	bool has_subscription = false;
 
@@ -3486,16 +3545,21 @@ int bt_gatt_subscribe(struct bt_conn *conn,
 		return -ENOTCONN;
 	}
 
+	sub = gatt_sub_add(conn);
+	if (!sub) {
+		return -ENOMEM;
+	}
+
 	/* Lookup existing subscriptions */
-	SYS_SLIST_FOR_EACH_CONTAINER(&subscriptions, tmp, node) {
+	SYS_SLIST_FOR_EACH_CONTAINER(&sub->list, tmp, node) {
 		/* Fail if entry already exists */
 		if (tmp == params) {
+			gatt_sub_remove(conn, sub, NULL, NULL);
 			return -EALREADY;
 		}
 
 		/* Check if another subscription exists */
-		if (!bt_conn_addr_le_cmp(conn, &tmp->_peer) &&
+		if (tmp->value_handle == params->value_handle &&
-		    tmp->value_handle == params->value_handle &&
 		    tmp->value >= params->value) {
 			has_subscription = true;
 		}
@@ -3508,6 +3572,7 @@ int bt_gatt_subscribe(struct bt_conn *conn,
 		err = gatt_write_ccc(conn, params->ccc_handle, params->value,
 				     gatt_write_ccc_rsp, params);
 		if (err) {
+			gatt_sub_remove(conn, sub, NULL, NULL);
 			return err;
 		}
 	}
@@ -3516,7 +3581,7 @@ int bt_gatt_subscribe(struct bt_conn *conn,
 	 * Add subscription before write complete as some implementation were
 	 * reported to send notification before reply to CCC write.
 	 */
-	gatt_subscription_add(conn, params);
+	sys_slist_prepend(&sub->list, &params->node);
 
 	return 0;
 }
@@ -3524,6 +3589,7 @@ int bt_gatt_subscribe(struct bt_conn *conn,
 int bt_gatt_unsubscribe(struct bt_conn *conn,
 			struct bt_gatt_subscribe_params *params)
 {
+	struct gatt_sub *sub;
 	struct bt_gatt_subscribe_params *tmp, *next;
 	bool has_subscription = false, found = false;
 	sys_snode_t *prev = NULL;
@@ -3535,12 +3601,17 @@ int bt_gatt_unsubscribe(struct bt_conn *conn,
 		return -ENOTCONN;
 	}
 
+	sub = gatt_sub_find(conn);
+	if (!sub) {
+		return -EINVAL;
+	}
+
 	/* Lookup existing subscriptions */
-	SYS_SLIST_FOR_EACH_CONTAINER_SAFE(&subscriptions, tmp, next, node) {
+	SYS_SLIST_FOR_EACH_CONTAINER_SAFE(&sub->list, tmp, next, node) {
 		/* Remove subscription */
 		if (params == tmp) {
 			found = true;
-			sys_slist_remove(&subscriptions, prev, &tmp->node);
+			sys_slist_remove(&sub->list, prev, &tmp->node);
 			/* Attempt to cancel if write is pending */
 			if (atomic_test_bit(params->flags,
 			    BT_GATT_SUBSCRIBE_FLAG_WRITE_PENDING)) {
@@ -3552,8 +3623,7 @@ int bt_gatt_unsubscribe(struct bt_conn *conn,
 		}
 
 		/* Check if there still remains any other subscription */
-		if (!bt_conn_addr_le_cmp(conn, &tmp->_peer) &&
+		if (tmp->value_handle == params->value_handle) {
-		    tmp->value_handle == params->value_handle) {
 			has_subscription = true;
 		}
 	}
@@ -3581,14 +3651,16 @@ void bt_gatt_cancel(struct bt_conn *conn, void *params)
 
 static void add_subscriptions(struct bt_conn *conn)
 {
+	struct gatt_sub *sub;
 	struct bt_gatt_subscribe_params *params;
 
-	/* Lookup existing subscriptions */
+	sub = gatt_sub_find(conn);
-	SYS_SLIST_FOR_EACH_CONTAINER(&subscriptions, params, node) {
+	if (!sub) {
-		if (bt_conn_addr_le_cmp(conn, &params->_peer)) {
+		return;
-			continue;
+	}
-		}
 
+	/* Lookup existing subscriptions */
+	SYS_SLIST_FOR_EACH_CONTAINER(&sub->list, params, node) {
 		if (bt_addr_le_is_bonded(conn->id, &conn->le.dst) &&
 		    !atomic_test_bit(params->flags,
 				     BT_GATT_SUBSCRIBE_FLAG_NO_RESUB)) {
@@ -4169,16 +4241,25 @@ static int sc_clear_by_addr(u8_t id, const bt_addr_le_t *addr)
 static void bt_gatt_clear_subscriptions(const bt_addr_le_t *addr)
 {
 #if defined(CONFIG_BT_GATT_CLIENT)
+	struct gatt_sub *sub = NULL;
 	struct bt_gatt_subscribe_params *params, *tmp;
 	sys_snode_t *prev = NULL;
+	int i;
 
-	SYS_SLIST_FOR_EACH_CONTAINER_SAFE(&subscriptions, params, tmp, node) {
+	for (i = 0; i < ARRAY_SIZE(subscriptions); i++) {
-		if (bt_addr_le_cmp(addr, &params->_peer)) {
+		if (!bt_addr_le_cmp(addr, &subscriptions[i].peer)) {
-			prev = &params->node;
+			sub = &subscriptions[i];
-			continue;
+			break;
 		}
+	}
+
+	if (!sub) {
+		return;
+	}
+
+	SYS_SLIST_FOR_EACH_CONTAINER_SAFE(&sub->list, params, tmp, node) {
 		params->value = 0U;
-		gatt_subscription_remove(NULL, prev, params);
+		gatt_sub_remove(NULL, sub, prev, params);
 	}
 #endif /* CONFIG_BT_GATT_CLIENT */
 }