[PATCH 0/6 v2] Bluetooth: Add 32 and 128 bit EIR UUID support

[PATCH 0/6 v2] Bluetooth: Add 32 and 128 bit EIR UUID support

[Johan Hedberg]

Hi,

Here's an update set with a fix suggestion from Lizardo for patch 4/6 as
well as a simplification to patch 1/6.

>From the original cover letter:
"
There was another set of patches from Syam to add the same functionality
but it can be accomplished in a simpler way since we only need to follow
the ordering within each group of UUIDs.

I've added test cases to mgmt-tester to verify both the complete and
partial UUID list cases (more can be added, but there are enough that
I'm confident of the correctness of these patches).
"

Johan Hedberg (6):
      Bluetooth: Split UUIDs to three separate lists
      Bluetooth: Simplify UUID16 list generation for EIR
      Bluetooth: Remove useless eir_len variable from EIR creation
      Bluetooth: Refactor UUID-16 list generation into its own function
      Bluetooth: Add support for 32-bit UUIDs in EIR data
      Bluetooth: Add support for 128-bit UUIDs in EIR data

 include/net/bluetooth/hci_core.h |    4 +++
 net/bluetooth/hci_core.c         |   14 +++++-----
 net/bluetooth/hci_sysfs.c        |    2 +-
 net/bluetooth/mgmt.c             |  203 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------------
 4 files changed, 137 insertions(+), 86 deletions(-)

[PATCH 1/6 v2] Bluetooth: Split UUIDs to three separate lists

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

The primary purpose of the UUIDs is to eable generation of EIR and AD
data. In these data formats the UUIDs are split into separate fields
based on whether they're 16, 32 or 128 bit UUIDs. To make the generation
of these data fields simple this patch splits the UUIDs into three
separate lists based on their type.

To keep cases needing access to all UUIDs easy the UUIDs are also kept
in an "all" list. This is particularly useful for exposing the UUIDs
through sysfs as well as removing UUIDs (whether removing a specific one
or all UUIDs).

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
v2: Removed enum bt_uuid_t since all it was needed for was selecting
which list to add to.

 include/net/bluetooth/hci_core.h |    4 +++
 net/bluetooth/hci_core.c         |   14 ++++----
 net/bluetooth/hci_sysfs.c        |    2 +-
 net/bluetooth/mgmt.c             |   67 +++++++++++++++++++-------------------
 4 files changed, 45 insertions(+), 42 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index bcf8ffe..d3b1d1b 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -85,6 +85,7 @@ struct bdaddr_list {
 
 struct bt_uuid {
 	struct list_head list;
+	struct list_head all;
 	u8 uuid[16];
 	u8 svc_hint;
 };
@@ -256,6 +257,9 @@ struct hci_dev {
 	struct list_head	blacklist;
 
 	struct list_head	uuids;
+	struct list_head	uuid16;
+	struct list_head	uuid32;
+	struct list_head	uuid128;
 
 	struct list_head	link_keys;
 
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index e061b35..8818521 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1183,14 +1183,11 @@ static void hci_discov_off(struct work_struct *work)
 
 int hci_uuids_clear(struct hci_dev *hdev)
 {
-	struct list_head *p, *n;
-
-	list_for_each_safe(p, n, &hdev->uuids) {
-		struct bt_uuid *uuid;
+	struct bt_uuid *uuid, *tmp;
 
-		uuid = list_entry(p, struct bt_uuid, list);
-
-		list_del(p);
+	list_for_each_entry_safe(uuid, tmp, &hdev->uuids, all) {
+		list_del(&uuid->list);
+		list_del(&uuid->all);
 		kfree(uuid);
 	}
 
@@ -1718,6 +1715,9 @@ struct hci_dev *hci_alloc_dev(void)
 	INIT_LIST_HEAD(&hdev->mgmt_pending);
 	INIT_LIST_HEAD(&hdev->blacklist);
 	INIT_LIST_HEAD(&hdev->uuids);
+	INIT_LIST_HEAD(&hdev->uuid16);
+	INIT_LIST_HEAD(&hdev->uuid32);
+	INIT_LIST_HEAD(&hdev->uuid128);
 	INIT_LIST_HEAD(&hdev->link_keys);
 	INIT_LIST_HEAD(&hdev->long_term_keys);
 	INIT_LIST_HEAD(&hdev->remote_oob_data);
diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
index 23b4e24..9a60c74 100644
--- a/net/bluetooth/hci_sysfs.c
+++ b/net/bluetooth/hci_sysfs.c
@@ -483,7 +483,7 @@ static int uuids_show(struct seq_file *f, void *p)
 
 	hci_dev_lock(hdev);
 
-	list_for_each_entry(uuid, &hdev->uuids, list)
+	list_for_each_entry(uuid, &hdev->uuids, all)
 		print_bt_uuid(f, uuid->uuid);
 
 	hci_dev_unlock(hdev);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index e7f944f..1257ed0 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -435,28 +435,6 @@ static u32 get_current_settings(struct hci_dev *hdev)
 
 #define PNP_INFO_SVCLASS_ID		0x1200
 
-static u8 bluetooth_base_uuid[] = {
-			0xFB, 0x34, 0x9B, 0x5F, 0x80, 0x00, 0x00, 0x80,
-			0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-};
-
-static u16 get_uuid16(u8 *uuid128)
-{
-	u32 val;
-	int i;
-
-	for (i = 0; i < 12; i++) {
-		if (bluetooth_base_uuid[i] != uuid128[i])
-			return 0;
-	}
-
-	val = get_unaligned_le32(&uuid128[12]);
-	if (val > 0xffff)
-		return 0;
-
-	return (u16) val;
-}
-
 static void create_eir(struct hci_dev *hdev, u8 *data)
 {
 	u8 *ptr = data;
@@ -510,13 +488,10 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
 	memset(uuid16_list, 0, sizeof(uuid16_list));
 
 	/* Group all UUID16 types */
-	list_for_each_entry(uuid, &hdev->uuids, list) {
+	list_for_each_entry(uuid, &hdev->uuid16, list) {
 		u16 uuid16;
 
-		uuid16 = get_uuid16(uuid->uuid);
-		if (uuid16 == 0)
-			return;
-
+		uuid16 = get_unaligned_le16(&uuid->uuid[12]);
 		if (uuid16 < 0x1100)
 			continue;
 
@@ -592,7 +567,7 @@ static u8 get_service_classes(struct hci_dev *hdev)
 	struct bt_uuid *uuid;
 	u8 val = 0;
 
-	list_for_each_entry(uuid, &hdev->uuids, list)
+	list_for_each_entry(uuid, &hdev->uuids, all)
 		val |= uuid->svc_hint;
 
 	return val;
@@ -1304,11 +1279,34 @@ unlock:
 	return err;
 }
 
+static u8 bluetooth_base_uuid[] = {
+			0xfb, 0x34, 0x9b, 0x5f, 0x80, 0x00, 0x00, 0x80,
+			0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+};
+
+static struct list_head *select_uuid_list(struct hci_dev *hdev, u8 *uuid)
+{
+	u32 val;
+	int i;
+
+	for (i = 0; i < 12; i++) {
+		if (bluetooth_base_uuid[i] != uuid[i])
+			return &hdev->uuid128;
+	}
+
+	val = get_unaligned_le32(&uuid[12]);
+	if (val > 0xffff)
+		return &hdev->uuid32;
+
+	return &hdev->uuid16;
+}
+
 static int add_uuid(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 {
 	struct mgmt_cp_add_uuid *cp = data;
 	struct pending_cmd *cmd;
 	struct bt_uuid *uuid;
+	struct list_head *l;
 	int err;
 
 	BT_DBG("request for %s", hdev->name);
@@ -1330,7 +1328,10 @@ static int add_uuid(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 	memcpy(uuid->uuid, cp->uuid, 16);
 	uuid->svc_hint = cp->svc_hint;
 
-	list_add(&uuid->list, &hdev->uuids);
+	l = select_uuid_list(hdev, cp->uuid);
+	list_add_tail(&uuid->list, l);
+
+	list_add_tail(&uuid->all, &hdev->uuids);
 
 	err = update_class(hdev);
 	if (err < 0)
@@ -1374,7 +1375,7 @@ static int remove_uuid(struct sock *sk, struct hci_dev *hdev, void *data,
 {
 	struct mgmt_cp_remove_uuid *cp = data;
 	struct pending_cmd *cmd;
-	struct list_head *p, *n;
+	struct bt_uuid *match, *tmp;
 	u8 bt_uuid_any[] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };
 	int err, found;
 
@@ -1402,13 +1403,11 @@ static int remove_uuid(struct sock *sk, struct hci_dev *hdev, void *data,
 
 	found = 0;
 
-	list_for_each_safe(p, n, &hdev->uuids) {
-		struct bt_uuid *match = list_entry(p, struct bt_uuid, list);
-
+	list_for_each_entry_safe(match, tmp, &hdev->uuids, all) {
 		if (memcmp(match->uuid, cp->uuid, 16) != 0)
 			continue;
-
 		list_del(&match->list);
+		list_del(&match->all);
 		kfree(match);
 		found++;
 	}
-- 
1.7.10.4

[PATCH 2/6 v2] Bluetooth: Simplify UUID16 list generation for EIR

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

There's no need to use two separate loops to generate a UUID list for
the EIR data. This patch merges the two loops previously used for the
16-bit UUID list generation into a single loop, thus simplifying the
code a great deal.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/mgmt.c |   46 +++++++++++++++-------------------------------
 1 file changed, 15 insertions(+), 31 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 1257ed0..b3a1397 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -438,9 +438,8 @@ static u32 get_current_settings(struct hci_dev *hdev)
 static void create_eir(struct hci_dev *hdev, u8 *data)
 {
 	u8 *ptr = data;
+	u8 *uuids_start;
 	u16 eir_len = 0;
-	u16 uuid16_list[HCI_MAX_EIR_LENGTH / sizeof(u16)];
-	int i, truncated = 0;
 	struct bt_uuid *uuid;
 	size_t name_len;
 
@@ -485,7 +484,7 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
 		ptr += 10;
 	}
 
-	memset(uuid16_list, 0, sizeof(uuid16_list));
+	uuids_start = NULL;
 
 	/* Group all UUID16 types */
 	list_for_each_entry(uuid, &hdev->uuid16, list) {
@@ -498,39 +497,24 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
 		if (uuid16 == PNP_INFO_SVCLASS_ID)
 			continue;
 
+		if (!uuids_start) {
+			uuids_start = ptr;
+			uuids_start[0] = 1;
+			uuids_start[1] = EIR_UUID16_ALL;
+			ptr += 2;
+			eir_len += 2;
+		}
+
 		/* Stop if not enough space to put next UUID */
 		if (eir_len + 2 + sizeof(u16) > HCI_MAX_EIR_LENGTH) {
-			truncated = 1;
+			uuids_start[1] = EIR_UUID16_SOME;
 			break;
 		}
 
-		/* Check for duplicates */
-		for (i = 0; uuid16_list[i] != 0; i++)
-			if (uuid16_list[i] == uuid16)
-				break;
-
-		if (uuid16_list[i] == 0) {
-			uuid16_list[i] = uuid16;
-			eir_len += sizeof(u16);
-		}
-	}
-
-	if (uuid16_list[0] != 0) {
-		u8 *length = ptr;
-
-		/* EIR Data type */
-		ptr[1] = truncated ? EIR_UUID16_SOME : EIR_UUID16_ALL;
-
-		ptr += 2;
-		eir_len += 2;
-
-		for (i = 0; uuid16_list[i] != 0; i++) {
-			*ptr++ = (uuid16_list[i] & 0x00ff);
-			*ptr++ = (uuid16_list[i] & 0xff00) >> 8;
-		}
-
-		/* EIR Data length */
-		*length = (i * sizeof(u16)) + 1;
+		*ptr++ = (uuid16 & 0x00ff);
+		*ptr++ = (uuid16 & 0xff00) >> 8;
+		eir_len += sizeof(uuid16);
+		uuids_start[0] += sizeof(uuid16);
 	}
 }
 
-- 
1.7.10.4

[PATCH 3/6 v2] Bluetooth: Remove useless eir_len variable from EIR creation

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

The amount of data encoded so far in the create_eir() function can be
calculated simply through the difference between the data and ptr
pointer variables. The eir_len variable then becomes essentially
useless.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/mgmt.c |    8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index b3a1397..abf941d 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -439,7 +439,6 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
 {
 	u8 *ptr = data;
 	u8 *uuids_start;
-	u16 eir_len = 0;
 	struct bt_uuid *uuid;
 	size_t name_len;
 
@@ -458,7 +457,6 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
 
 		memcpy(ptr + 2, hdev->dev_name, name_len);
 
-		eir_len += (name_len + 2);
 		ptr += (name_len + 2);
 	}
 
@@ -467,7 +465,6 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
 		ptr[1] = EIR_TX_POWER;
 		ptr[2] = (u8) hdev->inq_tx_power;
 
-		eir_len += 3;
 		ptr += 3;
 	}
 
@@ -480,7 +477,6 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
 		put_unaligned_le16(hdev->devid_product, ptr + 6);
 		put_unaligned_le16(hdev->devid_version, ptr + 8);
 
-		eir_len += 10;
 		ptr += 10;
 	}
 
@@ -502,18 +498,16 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
 			uuids_start[0] = 1;
 			uuids_start[1] = EIR_UUID16_ALL;
 			ptr += 2;
-			eir_len += 2;
 		}
 
 		/* Stop if not enough space to put next UUID */
-		if (eir_len + 2 + sizeof(u16) > HCI_MAX_EIR_LENGTH) {
+		if ((ptr - data) + 2 + sizeof(u16) > HCI_MAX_EIR_LENGTH) {
 			uuids_start[1] = EIR_UUID16_SOME;
 			break;
 		}
 
 		*ptr++ = (uuid16 & 0x00ff);
 		*ptr++ = (uuid16 & 0xff00) >> 8;
-		eir_len += sizeof(uuid16);
 		uuids_start[0] += sizeof(uuid16);
 	}
 }
-- 
1.7.10.4

[PATCH 4/6 v2] Bluetooth: Refactor UUID-16 list generation into its own function

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

We will need to create three separate UUID lists in the EIR data (for
16, 32 and 128 bit UUIDs) so the code is easier to follow if each list
is generated in their own function.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/mgmt.c |   72 ++++++++++++++++++++++++++++----------------------
 1 file changed, 40 insertions(+), 32 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index abf941d..3c8e69f 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -435,11 +435,48 @@ static u32 get_current_settings(struct hci_dev *hdev)
 
 #define PNP_INFO_SVCLASS_ID		0x1200
 
+static u8 *create_uuid16_list(struct hci_dev *hdev, u8 *data, ptrdiff_t len)
+{
+	u8 *ptr = data, *uuids_start = NULL;
+	struct bt_uuid *uuid;
+
+	if (len < 4)
+		return ptr;
+
+	list_for_each_entry(uuid, &hdev->uuid16, list) {
+		u16 uuid16;
+
+		uuid16 = get_unaligned_le16(&uuid->uuid[12]);
+		if (uuid16 < 0x1100)
+			continue;
+
+		if (uuid16 == PNP_INFO_SVCLASS_ID)
+			continue;
+
+		if (!uuids_start) {
+			uuids_start = ptr;
+			uuids_start[0] = 1;
+			uuids_start[1] = EIR_UUID16_ALL;
+			ptr += 2;
+		}
+
+		/* Stop if not enough space to put next UUID */
+		if ((ptr - data) + sizeof(u16) > len) {
+			uuids_start[1] = EIR_UUID16_SOME;
+			break;
+		}
+
+		*ptr++ = (uuid16 & 0x00ff);
+		*ptr++ = (uuid16 & 0xff00) >> 8;
+		uuids_start[0] += sizeof(uuid16);
+	}
+
+	return ptr;
+}
+
 static void create_eir(struct hci_dev *hdev, u8 *data)
 {
 	u8 *ptr = data;
-	u8 *uuids_start;
-	struct bt_uuid *uuid;
 	size_t name_len;
 
 	name_len = strlen(hdev->dev_name);
@@ -480,36 +517,7 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
 		ptr += 10;
 	}
 
-	uuids_start = NULL;
-
-	/* Group all UUID16 types */
-	list_for_each_entry(uuid, &hdev->uuid16, list) {
-		u16 uuid16;
-
-		uuid16 = get_unaligned_le16(&uuid->uuid[12]);
-		if (uuid16 < 0x1100)
-			continue;
-
-		if (uuid16 == PNP_INFO_SVCLASS_ID)
-			continue;
-
-		if (!uuids_start) {
-			uuids_start = ptr;
-			uuids_start[0] = 1;
-			uuids_start[1] = EIR_UUID16_ALL;
-			ptr += 2;
-		}
-
-		/* Stop if not enough space to put next UUID */
-		if ((ptr - data) + 2 + sizeof(u16) > HCI_MAX_EIR_LENGTH) {
-			uuids_start[1] = EIR_UUID16_SOME;
-			break;
-		}
-
-		*ptr++ = (uuid16 & 0x00ff);
-		*ptr++ = (uuid16 & 0xff00) >> 8;
-		uuids_start[0] += sizeof(uuid16);
-	}
+	ptr = create_uuid16_list(hdev, ptr, HCI_MAX_EIR_LENGTH - (ptr - data));
 }
 
 static int update_eir(struct hci_dev *hdev)
-- 
1.7.10.4

[PATCH 5/6 v2] Bluetooth: Add support for 32-bit UUIDs in EIR data

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

This patch adds the necessary code for inserting a list of 32-bit UUIDs
into the EIR data.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/mgmt.c |   31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 3c8e69f..48d81fc 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -474,6 +474,36 @@ static u8 *create_uuid16_list(struct hci_dev *hdev, u8 *data, ptrdiff_t len)
 	return ptr;
 }
 
+static u8 *create_uuid32_list(struct hci_dev *hdev, u8 *data, ptrdiff_t len)
+{
+	u8 *ptr = data, *uuids_start = NULL;
+	struct bt_uuid *uuid;
+
+	if (len < 6)
+		return ptr;
+
+	list_for_each_entry(uuid, &hdev->uuid32, list) {
+		if (!uuids_start) {
+			uuids_start = ptr;
+			uuids_start[0] = 1;
+			uuids_start[1] = EIR_UUID32_ALL;
+			ptr += 2;
+		}
+
+		/* Stop if not enough space to put next UUID */
+		if ((ptr - data) + sizeof(u32) > len) {
+			uuids_start[1] = EIR_UUID32_SOME;
+			break;
+		}
+
+		memcpy(ptr, &uuid->uuid[12], sizeof(u32));
+		ptr += sizeof(u32);
+		uuids_start[0] += sizeof(u32);
+	}
+
+	return ptr;
+}
+
 static void create_eir(struct hci_dev *hdev, u8 *data)
 {
 	u8 *ptr = data;
@@ -518,6 +548,7 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
 	}
 
 	ptr = create_uuid16_list(hdev, ptr, HCI_MAX_EIR_LENGTH - (ptr - data));
+	ptr = create_uuid32_list(hdev, ptr, HCI_MAX_EIR_LENGTH - (ptr - data));
 }
 
 static int update_eir(struct hci_dev *hdev)
-- 
1.7.10.4

[PATCH 6/6 v2] Bluetooth: Add support for 128-bit UUIDs in EIR data

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

This patch adds the necessary code for encoding a list of 128-bit UUIDs
into the EIR data.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/mgmt.c |   31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 48d81fc..b13e83e 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -504,6 +504,36 @@ static u8 *create_uuid32_list(struct hci_dev *hdev, u8 *data, ptrdiff_t len)
 	return ptr;
 }
 
+static u8 *create_uuid128_list(struct hci_dev *hdev, u8 *data, ptrdiff_t len)
+{
+	u8 *ptr = data, *uuids_start = NULL;
+	struct bt_uuid *uuid;
+
+	if (len < 18)
+		return ptr;
+
+	list_for_each_entry(uuid, &hdev->uuid128, list) {
+		if (!uuids_start) {
+			uuids_start = ptr;
+			uuids_start[0] = 1;
+			uuids_start[1] = EIR_UUID128_ALL;
+			ptr += 2;
+		}
+
+		/* Stop if not enough space to put next UUID */
+		if ((ptr - data) + 16 > len) {
+			uuids_start[1] = EIR_UUID128_SOME;
+			break;
+		}
+
+		memcpy(ptr, uuid->uuid, 16);
+		ptr += 16;
+		uuids_start[0] += 16;
+	}
+
+	return ptr;
+}
+
 static void create_eir(struct hci_dev *hdev, u8 *data)
 {
 	u8 *ptr = data;
@@ -549,6 +579,7 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
 
 	ptr = create_uuid16_list(hdev, ptr, HCI_MAX_EIR_LENGTH - (ptr - data));
 	ptr = create_uuid32_list(hdev, ptr, HCI_MAX_EIR_LENGTH - (ptr - data));
+	ptr = create_uuid128_list(hdev, ptr, HCI_MAX_EIR_LENGTH - (ptr - data));
 }
 
 static int update_eir(struct hci_dev *hdev)
-- 
1.7.10.4

Re: [PATCH 1/6 v2] Bluetooth: Split UUIDs to three separate lists

[Marcel Holtmann]

Hi Johan,

> The primary purpose of the UUIDs is to eable generation of EIR and AD
> data. In these data formats the UUIDs are split into separate fields
> based on whether they're 16, 32 or 128 bit UUIDs. To make the generation
> of these data fields simple this patch splits the UUIDs into three
> separate lists based on their type.
> 
> To keep cases needing access to all UUIDs easy the UUIDs are also kept
> in an "all" list. This is particularly useful for exposing the UUIDs
> through sysfs as well as removing UUIDs (whether removing a specific one
> or all UUIDs).
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> v2: Removed enum bt_uuid_t since all it was needed for was selecting
> which list to add to.
> 
>  include/net/bluetooth/hci_core.h |    4 +++
>  net/bluetooth/hci_core.c         |   14 ++++----
>  net/bluetooth/hci_sysfs.c        |    2 +-
>  net/bluetooth/mgmt.c             |   67 +++++++++++++++++++-------------------
>  4 files changed, 45 insertions(+), 42 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index bcf8ffe..d3b1d1b 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -85,6 +85,7 @@ struct bdaddr_list {
>  
>  struct bt_uuid {
>  	struct list_head list;
> +	struct list_head all;
>  	u8 uuid[16];
>  	u8 svc_hint;
>  };

I am not 100% convinced we should be doing this actually. If we store
the UUIDs as 128-bit values anyway, why do we need three lists.

> @@ -256,6 +257,9 @@ struct hci_dev {
>  	struct list_head	blacklist;
>  
>  	struct list_head	uuids;
> +	struct list_head	uuid16;
> +	struct list_head	uuid32;
> +	struct list_head	uuid128;

I would name them uuid16_list etc. if we stick with this.

And are we adding each entry twice? Or how does that work? I am getting
a bad feeling here.

>  	struct list_head	link_keys;
>  
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index e061b35..8818521 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -1183,14 +1183,11 @@ static void hci_discov_off(struct work_struct *work)
>  
>  int hci_uuids_clear(struct hci_dev *hdev)
>  {
> -	struct list_head *p, *n;
> -
> -	list_for_each_safe(p, n, &hdev->uuids) {
> -		struct bt_uuid *uuid;
> +	struct bt_uuid *uuid, *tmp;
>  
> -		uuid = list_entry(p, struct bt_uuid, list);
> -
> -		list_del(p);
> +	list_for_each_entry_safe(uuid, tmp, &hdev->uuids, all) {
> +		list_del(&uuid->list);
> +		list_del(&uuid->all);
>  		kfree(uuid);
>  	}
>  
> @@ -1718,6 +1715,9 @@ struct hci_dev *hci_alloc_dev(void)
>  	INIT_LIST_HEAD(&hdev->mgmt_pending);
>  	INIT_LIST_HEAD(&hdev->blacklist);
>  	INIT_LIST_HEAD(&hdev->uuids);
> +	INIT_LIST_HEAD(&hdev->uuid16);
> +	INIT_LIST_HEAD(&hdev->uuid32);
> +	INIT_LIST_HEAD(&hdev->uuid128);
>  	INIT_LIST_HEAD(&hdev->link_keys);
>  	INIT_LIST_HEAD(&hdev->long_term_keys);
>  	INIT_LIST_HEAD(&hdev->remote_oob_data);
> diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
> index 23b4e24..9a60c74 100644
> --- a/net/bluetooth/hci_sysfs.c
> +++ b/net/bluetooth/hci_sysfs.c
> @@ -483,7 +483,7 @@ static int uuids_show(struct seq_file *f, void *p)
>  
>  	hci_dev_lock(hdev);
>  
> -	list_for_each_entry(uuid, &hdev->uuids, list)
> +	list_for_each_entry(uuid, &hdev->uuids, all)
>  		print_bt_uuid(f, uuid->uuid);

This all thing makes is not so pretty code. You would expect list here.

>  
>  	hci_dev_unlock(hdev);
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index e7f944f..1257ed0 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -435,28 +435,6 @@ static u32 get_current_settings(struct hci_dev *hdev)
>  
>  #define PNP_INFO_SVCLASS_ID		0x1200
>  
> -static u8 bluetooth_base_uuid[] = {
> -			0xFB, 0x34, 0x9B, 0x5F, 0x80, 0x00, 0x00, 0x80,
> -			0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> -};
> -
> -static u16 get_uuid16(u8 *uuid128)
> -{
> -	u32 val;
> -	int i;
> -
> -	for (i = 0; i < 12; i++) {
> -		if (bluetooth_base_uuid[i] != uuid128[i])
> -			return 0;
> -	}
> -
> -	val = get_unaligned_le32(&uuid128[12]);
> -	if (val > 0xffff)
> -		return 0;
> -
> -	return (u16) val;
> -}
> -
>  static void create_eir(struct hci_dev *hdev, u8 *data)
>  {
>  	u8 *ptr = data;
> @@ -510,13 +488,10 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
>  	memset(uuid16_list, 0, sizeof(uuid16_list));
>  
>  	/* Group all UUID16 types */
> -	list_for_each_entry(uuid, &hdev->uuids, list) {
> +	list_for_each_entry(uuid, &hdev->uuid16, list) {
>  		u16 uuid16;
>  
> -		uuid16 = get_uuid16(uuid->uuid);
> -		if (uuid16 == 0)
> -			return;
> -
> +		uuid16 = get_unaligned_le16(&uuid->uuid[12]);
>  		if (uuid16 < 0x1100)
>  			continue;

Why are we not storing the UUID size with it. So we just loop the same
list three times.

You have to keep the list of all UUIDs anyway. And we are not expecting
a bunch of hundred UUIDs. This list will be in the range of 10-20 max.
 
> @@ -592,7 +567,7 @@ static u8 get_service_classes(struct hci_dev *hdev)
>  	struct bt_uuid *uuid;
>  	u8 val = 0;
>  
> -	list_for_each_entry(uuid, &hdev->uuids, list)
> +	list_for_each_entry(uuid, &hdev->uuids, all)
>  		val |= uuid->svc_hint;
>  
>  	return val;
> @@ -1304,11 +1279,34 @@ unlock:
>  	return err;
>  }
>  
> +static u8 bluetooth_base_uuid[] = {
> +			0xfb, 0x34, 0x9b, 0x5f, 0x80, 0x00, 0x00, 0x80,
> +			0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +};
> +
> +static struct list_head *select_uuid_list(struct hci_dev *hdev, u8 *uuid)
> +{
> +	u32 val;
> +	int i;
> +
> +	for (i = 0; i < 12; i++) {
> +		if (bluetooth_base_uuid[i] != uuid[i])
> +			return &hdev->uuid128;
> +	}

I know it is in the original code as well, but what is wrong with
memcmp.

> +
> +	val = get_unaligned_le32(&uuid[12]);
> +	if (val > 0xffff)
> +		return &hdev->uuid32;
> +
> +	return &hdev->uuid16;
> +}
> +
>  static int add_uuid(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
>  {
>  	struct mgmt_cp_add_uuid *cp = data;
>  	struct pending_cmd *cmd;
>  	struct bt_uuid *uuid;
> +	struct list_head *l;
>  	int err;
>  
>  	BT_DBG("request for %s", hdev->name);
> @@ -1330,7 +1328,10 @@ static int add_uuid(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
>  	memcpy(uuid->uuid, cp->uuid, 16);
>  	uuid->svc_hint = cp->svc_hint;
>  
> -	list_add(&uuid->list, &hdev->uuids);
> +	l = select_uuid_list(hdev, cp->uuid);
> +	list_add_tail(&uuid->list, l);
> +
> +	list_add_tail(&uuid->all, &hdev->uuids);
>  
>  	err = update_class(hdev);
>  	if (err < 0)
> @@ -1374,7 +1375,7 @@ static int remove_uuid(struct sock *sk, struct hci_dev *hdev, void *data,
>  {
>  	struct mgmt_cp_remove_uuid *cp = data;
>  	struct pending_cmd *cmd;
> -	struct list_head *p, *n;
> +	struct bt_uuid *match, *tmp;
>  	u8 bt_uuid_any[] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };
>  	int err, found;
>  
> @@ -1402,13 +1403,11 @@ static int remove_uuid(struct sock *sk, struct hci_dev *hdev, void *data,
>  
>  	found = 0;
>  
> -	list_for_each_safe(p, n, &hdev->uuids) {
> -		struct bt_uuid *match = list_entry(p, struct bt_uuid, list);
> -
> +	list_for_each_entry_safe(match, tmp, &hdev->uuids, all) {
>  		if (memcmp(match->uuid, cp->uuid, 16) != 0)
>  			continue;
> -
>  		list_del(&match->list);
> +		list_del(&match->all);
>  		kfree(match);
>  		found++;
>  	}

Regards

Marcel