[PATCH 0/4] Bluetooth: Implement get/set device flags and device flags changed

[PATCH 0/4] Bluetooth: Implement get/set device flags and device flags changed

[Abhishek Pandit-Subedi]

Hi linux-bluetooth,

This series adds support for configuring the Remote Wakeup flag on
devices by implementing Get Device Flags, Set Device Flags and Device
Flags Changed.

This was tested with some userspace changes to update the Remote Wakeup
flag (these changes will be upstreamed as Bluez patches once they're
cleaned up). I verified that Add Device generates the Device Flags
changed on all mgmt interfaces and Set Device Flags skips the one that
requested it.

This was tested on a Chromebook running kernel 5.4.

Abhishek



Abhishek Pandit-Subedi (4):
  Bluetooth: Add bdaddr_list_with_flags for classic whitelist
  Bluetooth: Replace wakeable list with flag
  Bluetooth: Replace wakeable in hci_conn_params
  Bluetooth: Add get/set device flags mgmt op

 include/net/bluetooth/hci.h      |   1 +
 include/net/bluetooth/hci_core.h |  31 ++++++-
 include/net/bluetooth/mgmt.h     |  28 +++++++
 net/bluetooth/hci_core.c         |  59 ++++++++++++-
 net/bluetooth/hci_event.c        |   8 +-
 net/bluetooth/hci_request.c      |  15 ++--
 net/bluetooth/hci_sock.c         |   1 +
 net/bluetooth/mgmt.c             | 139 ++++++++++++++++++++++++++++++-
 8 files changed, 266 insertions(+), 16 deletions(-)

-- 
2.27.0.290.gba653c62da-goog

[PATCH 1/4] Bluetooth: Add bdaddr_list_with_flags for classic whitelist

[Abhishek Pandit-Subedi]

In order to more easily add device flags to classic devices, create
a new type of bdaddr_list that supports setting flags.

Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Reviewed-by: Alain Michaud <alainm@chromium.org>
---

 include/net/bluetooth/hci_core.h | 18 ++++++++--
 net/bluetooth/hci_core.c         | 58 ++++++++++++++++++++++++++++++++
 net/bluetooth/hci_event.c        |  8 ++---
 net/bluetooth/mgmt.c             |  5 +--
 4 files changed, 81 insertions(+), 8 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 0d5dbb6cb5a089..95a3935325bbbc 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -136,6 +136,13 @@ struct bdaddr_list_with_irk {
 	u8 local_irk[16];
 };
 
+struct bdaddr_list_with_flags {
+	struct list_head list;
+	bdaddr_t bdaddr;
+	u8 bdaddr_type;
+	u32 current_flags;
+};
+
 struct bt_uuid {
 	struct list_head list;
 	u8 uuid[16];
@@ -1169,12 +1176,19 @@ struct bdaddr_list *hci_bdaddr_list_lookup(struct list_head *list,
 struct bdaddr_list_with_irk *hci_bdaddr_list_lookup_with_irk(
 				    struct list_head *list, bdaddr_t *bdaddr,
 				    u8 type);
+struct bdaddr_list_with_flags *
+hci_bdaddr_list_lookup_with_flags(struct list_head *list, bdaddr_t *bdaddr,
+				  u8 type);
 int hci_bdaddr_list_add(struct list_head *list, bdaddr_t *bdaddr, u8 type);
 int hci_bdaddr_list_add_with_irk(struct list_head *list, bdaddr_t *bdaddr,
-					u8 type, u8 *peer_irk, u8 *local_irk);
+				 u8 type, u8 *peer_irk, u8 *local_irk);
+int hci_bdaddr_list_add_with_flags(struct list_head *list, bdaddr_t *bdaddr,
+				   u8 type, u32 flags);
 int hci_bdaddr_list_del(struct list_head *list, bdaddr_t *bdaddr, u8 type);
 int hci_bdaddr_list_del_with_irk(struct list_head *list, bdaddr_t *bdaddr,
-								u8 type);
+				 u8 type);
+int hci_bdaddr_list_del_with_flags(struct list_head *list, bdaddr_t *bdaddr,
+				   u8 type);
 void hci_bdaddr_list_clear(struct list_head *list);
 
 struct hci_conn_params *hci_conn_params_lookup(struct hci_dev *hdev,
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 4f1052a7c488e5..8a471bec2731ed 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3023,6 +3023,20 @@ struct bdaddr_list_with_irk *hci_bdaddr_list_lookup_with_irk(
 	return NULL;
 }
 
+struct bdaddr_list_with_flags *
+hci_bdaddr_list_lookup_with_flags(struct list_head *bdaddr_list,
+				  bdaddr_t *bdaddr, u8 type)
+{
+	struct bdaddr_list_with_flags *b;
+
+	list_for_each_entry(b, bdaddr_list, list) {
+		if (!bacmp(&b->bdaddr, bdaddr) && b->bdaddr_type == type)
+			return b;
+	}
+
+	return NULL;
+}
+
 void hci_bdaddr_list_clear(struct list_head *bdaddr_list)
 {
 	struct bdaddr_list *b, *n;
@@ -3084,6 +3098,30 @@ int hci_bdaddr_list_add_with_irk(struct list_head *list, bdaddr_t *bdaddr,
 	return 0;
 }
 
+int hci_bdaddr_list_add_with_flags(struct list_head *list, bdaddr_t *bdaddr,
+				   u8 type, u32 flags)
+{
+	struct bdaddr_list_with_flags *entry;
+
+	if (!bacmp(bdaddr, BDADDR_ANY))
+		return -EBADF;
+
+	if (hci_bdaddr_list_lookup(list, bdaddr, type))
+		return -EEXIST;
+
+	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+	if (!entry)
+		return -ENOMEM;
+
+	bacpy(&entry->bdaddr, bdaddr);
+	entry->bdaddr_type = type;
+	entry->current_flags = flags;
+
+	list_add(&entry->list, list);
+
+	return 0;
+}
+
 int hci_bdaddr_list_del(struct list_head *list, bdaddr_t *bdaddr, u8 type)
 {
 	struct bdaddr_list *entry;
@@ -3123,6 +3161,26 @@ int hci_bdaddr_list_del_with_irk(struct list_head *list, bdaddr_t *bdaddr,
 	return 0;
 }
 
+int hci_bdaddr_list_del_with_flags(struct list_head *list, bdaddr_t *bdaddr,
+				   u8 type)
+{
+	struct bdaddr_list_with_flags *entry;
+
+	if (!bacmp(bdaddr, BDADDR_ANY)) {
+		hci_bdaddr_list_clear(list);
+		return 0;
+	}
+
+	entry = hci_bdaddr_list_lookup_with_flags(list, bdaddr, type);
+	if (!entry)
+		return -ENOENT;
+
+	list_del(&entry->list);
+	kfree(entry);
+
+	return 0;
+}
+
 /* This function requires the caller holds hdev->lock */
 struct hci_conn_params *hci_conn_params_lookup(struct hci_dev *hdev,
 					       bdaddr_t *addr, u8 addr_type)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index cfeaee347db32d..8981954ff4c47d 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2697,10 +2697,10 @@ static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
 	 */
 	if (hci_dev_test_flag(hdev, HCI_MGMT) &&
 	    !hci_dev_test_flag(hdev, HCI_CONNECTABLE) &&
-	    !hci_bdaddr_list_lookup(&hdev->whitelist, &ev->bdaddr,
-				    BDADDR_BREDR)) {
-		    hci_reject_conn(hdev, &ev->bdaddr);
-		    return;
+	    !hci_bdaddr_list_lookup_with_flags(&hdev->whitelist, &ev->bdaddr,
+					       BDADDR_BREDR)) {
+		hci_reject_conn(hdev, &ev->bdaddr);
+		return;
 	}
 
 	/* Connection accepted */
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 99fbfd467d0465..6d996e5e5bcc2d 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -5997,8 +5997,9 @@ static int add_device(struct sock *sk, struct hci_dev *hdev,
 			goto unlock;
 		}
 
-		err = hci_bdaddr_list_add(&hdev->whitelist, &cp->addr.bdaddr,
-					  cp->addr.type);
+		err = hci_bdaddr_list_add_with_flags(&hdev->whitelist,
+						     &cp->addr.bdaddr,
+						     cp->addr.type, 0);
 		if (err)
 			goto unlock;
 
-- 
2.27.0.290.gba653c62da-goog

[PATCH 2/4] Bluetooth: Replace wakeable list with flag

[Abhishek Pandit-Subedi]

Since the classic device list now supports flags, convert the wakeable
list into a flag on the existing device list.

Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Reviewed-by: Alain Michaud <alainm@chromium.org>
---

 include/net/bluetooth/hci_core.h | 11 ++++++++++-
 net/bluetooth/hci_core.c         |  1 -
 net/bluetooth/hci_request.c      | 12 ++++++++----
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 95a3935325bbbc..0643c737ba8528 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -143,6 +143,16 @@ struct bdaddr_list_with_flags {
 	u32 current_flags;
 };
 
+enum hci_conn_flags {
+	HCI_CONN_FLAG_REMOTE_WAKEUP,
+	HCI_CONN_FLAG_MAX
+};
+
+#define hci_conn_test_flag(nr, flags) ((flags) & (1U << nr))
+
+/* Make sure number of flags doesn't exceed sizeof(current_flags) */
+static_assert(HCI_CONN_FLAG_MAX < 32);
+
 struct bt_uuid {
 	struct list_head list;
 	u8 uuid[16];
@@ -463,7 +473,6 @@ struct hci_dev {
 	struct list_head	mgmt_pending;
 	struct list_head	blacklist;
 	struct list_head	whitelist;
-	struct list_head	wakeable;
 	struct list_head	uuids;
 	struct list_head	link_keys;
 	struct list_head	long_term_keys;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 8a471bec2731ed..8e01afb2ee8c5c 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3499,7 +3499,6 @@ struct hci_dev *hci_alloc_dev(void)
 	INIT_LIST_HEAD(&hdev->mgmt_pending);
 	INIT_LIST_HEAD(&hdev->blacklist);
 	INIT_LIST_HEAD(&hdev->whitelist);
-	INIT_LIST_HEAD(&hdev->wakeable);
 	INIT_LIST_HEAD(&hdev->uuids);
 	INIT_LIST_HEAD(&hdev->link_keys);
 	INIT_LIST_HEAD(&hdev->long_term_keys);
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index a7f572ad38ef08..a5b53d3ea50802 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -968,15 +968,19 @@ static void hci_req_clear_event_filter(struct hci_request *req)
 
 static void hci_req_set_event_filter(struct hci_request *req)
 {
-	struct bdaddr_list *b;
+	struct bdaddr_list_with_flags *b;
 	struct hci_cp_set_event_filter f;
 	struct hci_dev *hdev = req->hdev;
-	u8 scan;
+	u8 scan = SCAN_DISABLED;
 
 	/* Always clear event filter when starting */
 	hci_req_clear_event_filter(req);
 
-	list_for_each_entry(b, &hdev->wakeable, list) {
+	list_for_each_entry(b, &hdev->whitelist, list) {
+		if (!hci_conn_test_flag(HCI_CONN_FLAG_REMOTE_WAKEUP,
+					b->current_flags))
+			continue;
+
 		memset(&f, 0, sizeof(f));
 		bacpy(&f.addr_conn_flt.bdaddr, &b->bdaddr);
 		f.flt_type = HCI_FLT_CONN_SETUP;
@@ -985,9 +989,9 @@ static void hci_req_set_event_filter(struct hci_request *req)
 
 		bt_dev_dbg(hdev, "Adding event filters for %pMR", &b->bdaddr);
 		hci_req_add(req, HCI_OP_SET_EVENT_FLT, sizeof(f), &f);
+		scan = SCAN_PAGE;
 	}
 
-	scan = !list_empty(&hdev->wakeable) ? SCAN_PAGE : SCAN_DISABLED;
 	hci_req_add(req, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);
 }
 
-- 
2.27.0.290.gba653c62da-goog

[PATCH 3/4] Bluetooth: Replace wakeable in hci_conn_params

[Abhishek Pandit-Subedi]

Replace the wakeable boolean with flags in hci_conn_params and all users
of this boolean. This will be used by the get/set device flags mgmt op.

Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Reviewed-by: Alain Michaud <alainm@chromium.org>
---

 include/net/bluetooth/hci_core.h | 2 +-
 net/bluetooth/hci_request.c      | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 0643c737ba8528..6f88e5d81bd24f 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -660,7 +660,7 @@ struct hci_conn_params {
 
 	struct hci_conn *conn;
 	bool explicit_connect;
-	bool wakeable;
+	u32 current_flags;
 };
 
 extern struct list_head hci_dev_list;
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index a5b53d3ea50802..eee9c007a5fb40 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -710,7 +710,8 @@ static int add_to_white_list(struct hci_request *req,
 	}
 
 	/* During suspend, only wakeable devices can be in whitelist */
-	if (hdev->suspended && !params->wakeable)
+	if (hdev->suspended && !hci_conn_test_flag(HCI_CONN_FLAG_REMOTE_WAKEUP,
+						   params->current_flags))
 		return 0;
 
 	*num_entries += 1;
-- 
2.27.0.290.gba653c62da-goog

[PATCH 4/4] Bluetooth: Add get/set device flags mgmt op

[Abhishek Pandit-Subedi]

Add the get device flags and set device flags mgmt ops and the device
flags changed event. Their behavior is described in detail in
mgmt-api.txt in bluez.

Sample btmon trace when a HID device is added (trimmed to 75 chars):

@ MGMT Command: Unknown (0x0050) plen 11        {0x0001} [hci0] 18:06:14.98
        90 c5 13 cd f3 cd 02 01 00 00 00                 ...........
@ MGMT Event: Unknown (0x002a) plen 15          {0x0004} [hci0] 18:06:14.98
        90 c5 13 cd f3 cd 02 01 00 00 00 01 00 00 00     ...............
@ MGMT Event: Unknown (0x002a) plen 15          {0x0003} [hci0] 18:06:14.98
        90 c5 13 cd f3 cd 02 01 00 00 00 01 00 00 00     ...............
@ MGMT Event: Unknown (0x002a) plen 15          {0x0002} [hci0] 18:06:14.98
        90 c5 13 cd f3 cd 02 01 00 00 00 01 00 00 00     ...............
@ MGMT Event: Command Compl.. (0x0001) plen 10  {0x0001} [hci0] 18:06:14.98
      Unknown (0x0050) plen 7
        Status: Success (0x00)
        90 c5 13 cd f3 cd 02                             .......
@ MGMT Command: Add Device (0x0033) plen 8      {0x0001} [hci0] 18:06:14.98
        LE Address: CD:F3:CD:13:C5:90 (Static)
        Action: Auto-connect remote device (0x02)
@ MGMT Event: Device Added (0x001a) plen 8      {0x0004} [hci0] 18:06:14.98
        LE Address: CD:F3:CD:13:C5:90 (Static)
        Action: Auto-connect remote device (0x02)
@ MGMT Event: Device Added (0x001a) plen 8      {0x0003} [hci0] 18:06:14.98
        LE Address: CD:F3:CD:13:C5:90 (Static)
        Action: Auto-connect remote device (0x02)
@ MGMT Event: Device Added (0x001a) plen 8      {0x0002} [hci0] 18:06:14.98
        LE Address: CD:F3:CD:13:C5:90 (Static)
        Action: Auto-connect remote device (0x02)
@ MGMT Event: Unknown (0x002a) plen 15          {0x0004} [hci0] 18:06:14.98
        90 c5 13 cd f3 cd 02 01 00 00 00 01 00 00 00     ...............
@ MGMT Event: Unknown (0x002a) plen 15          {0x0003} [hci0] 18:06:14.98
        90 c5 13 cd f3 cd 02 01 00 00 00 01 00 00 00     ...............
@ MGMT Event: Unknown (0x002a) plen 15          {0x0002} [hci0] 18:06:14.98
        90 c5 13 cd f3 cd 02 01 00 00 00 01 00 00 00     ...............
@ MGMT Event: Unknown (0x002a) plen 15          {0x0001} [hci0] 18:06:14.98
        90 c5 13 cd f3 cd 02 01 00 00 00 01 00 00 00     ...............

Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Reviewed-by: Alain Michaud <alainm@chromium.org>
---

 include/net/bluetooth/hci.h  |   1 +
 include/net/bluetooth/mgmt.h |  28 ++++++++
 net/bluetooth/hci_sock.c     |   1 +
 net/bluetooth/mgmt.c         | 134 +++++++++++++++++++++++++++++++++++
 4 files changed, 164 insertions(+)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 16ab6ce8788341..5e03aac76ad47f 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -259,6 +259,7 @@ enum {
 	HCI_MGMT_LOCAL_NAME_EVENTS,
 	HCI_MGMT_OOB_DATA_EVENTS,
 	HCI_MGMT_EXP_FEATURE_EVENTS,
+	HCI_MGMT_DEVICE_FLAGS_EVENTS,
 };
 
 /*
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index e515288f328f47..8e47b0c5fe52bb 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -720,6 +720,27 @@ struct mgmt_rp_set_exp_feature {
 #define MGMT_OP_SET_DEF_RUNTIME_CONFIG	0x004e
 #define MGMT_SET_DEF_RUNTIME_CONFIG_SIZE	0
 
+#define MGMT_OP_GET_DEVICE_FLAGS	0x004F
+#define MGMT_GET_DEVICE_FLAGS_SIZE	7
+struct mgmt_cp_get_device_flags {
+	struct mgmt_addr_info addr;
+} __packed;
+struct mgmt_rp_get_device_flags {
+	struct mgmt_addr_info addr;
+	__le32 supported_flags;
+	__le32 current_flags;
+} __packed;
+
+#define MGMT_OP_SET_DEVICE_FLAGS	0x0050
+#define MGMT_SET_DEVICE_FLAGS_SIZE	11
+struct mgmt_cp_set_device_flags {
+	struct mgmt_addr_info addr;
+	__le32 current_flags;
+} __packed;
+struct mgmt_rp_set_device_flags {
+	struct mgmt_addr_info addr;
+} __packed;
+
 #define MGMT_EV_CMD_COMPLETE		0x0001
 struct mgmt_ev_cmd_complete {
 	__le16	opcode;
@@ -951,3 +972,10 @@ struct mgmt_ev_exp_feature_changed {
 	__u8	uuid[16];
 	__le32	flags;
 } __packed;
+
+#define MGMT_EV_DEVICE_FLAGS_CHANGED		0x002a
+struct mgmt_ev_device_flags_changed {
+	struct mgmt_addr_info addr;
+	__le32 supported_flags;
+	__le32 current_flags;
+} __packed;
diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index d5627967fc254f..a7903b6206620c 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -1354,6 +1354,7 @@ static int hci_sock_bind(struct socket *sock, struct sockaddr *addr,
 			hci_sock_set_flag(sk, HCI_MGMT_SETTING_EVENTS);
 			hci_sock_set_flag(sk, HCI_MGMT_DEV_CLASS_EVENTS);
 			hci_sock_set_flag(sk, HCI_MGMT_LOCAL_NAME_EVENTS);
+			hci_sock_set_flag(sk, HCI_MGMT_DEVICE_FLAGS_EVENTS);
 		}
 		break;
 	}
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 6d996e5e5bcc2d..2805f662d85695 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -114,6 +114,8 @@ static const u16 mgmt_commands[] = {
 	MGMT_OP_SET_EXP_FEATURE,
 	MGMT_OP_READ_DEF_SYSTEM_CONFIG,
 	MGMT_OP_SET_DEF_SYSTEM_CONFIG,
+	MGMT_OP_GET_DEVICE_FLAGS,
+	MGMT_OP_SET_DEVICE_FLAGS,
 };
 
 static const u16 mgmt_events[] = {
@@ -154,6 +156,7 @@ static const u16 mgmt_events[] = {
 	MGMT_EV_EXT_INFO_CHANGED,
 	MGMT_EV_PHY_CONFIGURATION_CHANGED,
 	MGMT_EV_EXP_FEATURE_CHANGED,
+	MGMT_EV_DEVICE_FLAGS_CHANGED,
 };
 
 static const u16 mgmt_untrusted_commands[] = {
@@ -3853,6 +3856,122 @@ static int set_exp_feature(struct sock *sk, struct hci_dev *hdev,
 			       MGMT_STATUS_NOT_SUPPORTED);
 }
 
+#define SUPPORTED_DEVICE_FLAGS() ((1U << HCI_CONN_FLAG_MAX) - 1)
+
+static int get_device_flags(struct sock *sk, struct hci_dev *hdev, void *data,
+			    u16 data_len)
+{
+	struct mgmt_cp_get_device_flags *cp = data;
+	struct mgmt_rp_get_device_flags rp;
+	struct bdaddr_list_with_flags *br_params;
+	struct hci_conn_params *params;
+	u32 supported_flags = SUPPORTED_DEVICE_FLAGS();
+	u32 current_flags = 0;
+	u8 status = MGMT_STATUS_INVALID_PARAMS;
+
+	bt_dev_dbg(hdev, "Get device flags %pMR (type 0x%x)\n",
+		   &cp->addr.bdaddr, cp->addr.type);
+
+	if (cp->addr.type == BDADDR_BREDR) {
+		br_params = hci_bdaddr_list_lookup_with_flags(&hdev->whitelist,
+							      &cp->addr.bdaddr,
+							      cp->addr.type);
+		if (!br_params)
+			goto done;
+
+		current_flags = br_params->current_flags;
+	} else {
+		params = hci_conn_params_lookup(hdev, &cp->addr.bdaddr,
+						le_addr_type(cp->addr.type));
+
+		if (!params)
+			goto done;
+
+		current_flags = params->current_flags;
+	}
+
+	bacpy(&rp.addr.bdaddr, &cp->addr.bdaddr);
+	rp.addr.type = cp->addr.type;
+	rp.supported_flags = cpu_to_le32(supported_flags);
+	rp.current_flags = cpu_to_le32(current_flags);
+
+	status = MGMT_STATUS_SUCCESS;
+
+done:
+	return mgmt_cmd_complete(sk, hdev->id, MGMT_OP_GET_DEVICE_FLAGS, status,
+				&rp, sizeof(rp));
+}
+
+static int device_flags_changed(struct hci_dev *hdev, bdaddr_t *bdaddr,
+				u8 bdaddr_type, u32 supported_flags,
+				u32 current_flags, struct sock *skip)
+{
+	struct mgmt_ev_device_flags_changed ev;
+
+	bacpy(&ev.addr.bdaddr, bdaddr);
+	ev.addr.type = bdaddr_type;
+	ev.supported_flags = cpu_to_le32(supported_flags);
+	ev.current_flags = cpu_to_le32(current_flags);
+
+	return mgmt_limited_event(MGMT_EV_DEVICE_FLAGS_CHANGED, hdev, &ev,
+				  sizeof(ev), HCI_MGMT_DEVICE_FLAGS_EVENTS,
+				  skip);
+}
+
+static int set_device_flags(struct sock *sk, struct hci_dev *hdev, void *data,
+			    u16 len)
+{
+	struct mgmt_cp_set_device_flags *cp = data;
+	struct bdaddr_list_with_flags *br_params;
+	struct hci_conn_params *params;
+	u8 status = MGMT_STATUS_INVALID_PARAMS;
+	u32 supported_flags = SUPPORTED_DEVICE_FLAGS();
+	u32 current_flags = __le32_to_cpu(cp->current_flags);
+
+	bt_dev_dbg(hdev, "Set device flags %pMR (type 0x%x) = 0x%x",
+		   &cp->addr.bdaddr, cp->addr.type,
+		   __le32_to_cpu(current_flags));
+
+	if ((supported_flags | current_flags) != supported_flags) {
+		bt_dev_warn(hdev, "Bad flag given (0x%x) vs supported (0x%0x)",
+			    current_flags, supported_flags);
+		goto done;
+	}
+
+	if (cp->addr.type == BDADDR_BREDR) {
+		br_params = hci_bdaddr_list_lookup_with_flags(&hdev->whitelist,
+							      &cp->addr.bdaddr,
+							      cp->addr.type);
+
+		if (br_params) {
+			br_params->current_flags = current_flags;
+			status = MGMT_STATUS_SUCCESS;
+		} else {
+			bt_dev_warn(hdev, "No such BR/EDR device %pMR (0x%x)",
+				    &cp->addr.bdaddr, cp->addr.type);
+		}
+	} else {
+		params = hci_conn_params_lookup(hdev, &cp->addr.bdaddr,
+						le_addr_type(cp->addr.type));
+		if (params) {
+			params->current_flags = current_flags;
+			status = MGMT_STATUS_SUCCESS;
+		} else {
+			bt_dev_warn(hdev, "No such LE device %pMR (0x%x)",
+				    &cp->addr.bdaddr,
+				    le_addr_type(cp->addr.type));
+		}
+	}
+
+done:
+	if (status == MGMT_STATUS_SUCCESS)
+		device_flags_changed(hdev, &cp->addr.bdaddr, cp->addr.type,
+				     supported_flags, current_flags, sk);
+
+	return mgmt_cmd_complete(sk, hdev->id, MGMT_OP_SET_DEVICE_FLAGS, status,
+				 &cp->addr, sizeof(cp->addr));
+}
+
 static void read_local_oob_data_complete(struct hci_dev *hdev, u8 status,
 				         u16 opcode, struct sk_buff *skb)
 {
@@ -5970,7 +6089,9 @@ static int add_device(struct sock *sk, struct hci_dev *hdev,
 {
 	struct mgmt_cp_add_device *cp = data;
 	u8 auto_conn, addr_type;
+	struct hci_conn_params *params;
 	int err;
+	u32 current_flags = 0;
 
 	bt_dev_dbg(hdev, "sock %p", sk);
 
@@ -6038,12 +6159,19 @@ static int add_device(struct sock *sk, struct hci_dev *hdev,
 					MGMT_STATUS_FAILED, &cp->addr,
 					sizeof(cp->addr));
 		goto unlock;
+	} else {
+		params = hci_conn_params_lookup(hdev, &cp->addr.bdaddr,
+						addr_type);
+		if (params)
+			current_flags = params->current_flags;
 	}
 
 	hci_update_background_scan(hdev);
 
 added:
 	device_added(sk, hdev, &cp->addr.bdaddr, cp->addr.type, cp->action);
+	device_flags_changed(hdev, &cp->addr.bdaddr, cp->addr.type,
+			     SUPPORTED_DEVICE_FLAGS(), current_flags, NULL);
 
 	err = mgmt_cmd_complete(sk, hdev->id, MGMT_OP_ADD_DEVICE,
 				MGMT_STATUS_SUCCESS, &cp->addr,
@@ -7306,6 +7434,12 @@ static const struct hci_mgmt_handler mgmt_handlers[] = {
 						HCI_MGMT_UNTRUSTED },
 	{ set_def_system_config,   MGMT_SET_DEF_SYSTEM_CONFIG_SIZE,
 						HCI_MGMT_VAR_LEN },
+
+	{ NULL }, /* Read default runtime config */
+	{ NULL }, /* Set default runtime config */
+
+	{ get_device_flags,        MGMT_GET_DEVICE_FLAGS_SIZE },
+	{ set_device_flags,        MGMT_SET_DEVICE_FLAGS_SIZE },
 };
 
 void mgmt_index_added(struct hci_dev *hdev)
-- 
2.27.0.290.gba653c62da-goog

Re: [PATCH 4/4] Bluetooth: Add get/set device flags mgmt op

[Marcel Holtmann]

Hi Abhishek,

> Add the get device flags and set device flags mgmt ops and the device
> flags changed event. Their behavior is described in detail in
> mgmt-api.txt in bluez.
> 
> Sample btmon trace when a HID device is added (trimmed to 75 chars):
> 
> @ MGMT Command: Unknown (0x0050) plen 11        {0x0001} [hci0] 18:06:14.98
>        90 c5 13 cd f3 cd 02 01 00 00 00                 ...........
> @ MGMT Event: Unknown (0x002a) plen 15          {0x0004} [hci0] 18:06:14.98
>        90 c5 13 cd f3 cd 02 01 00 00 00 01 00 00 00     ...............
> @ MGMT Event: Unknown (0x002a) plen 15          {0x0003} [hci0] 18:06:14.98
>        90 c5 13 cd f3 cd 02 01 00 00 00 01 00 00 00     ...............
> @ MGMT Event: Unknown (0x002a) plen 15          {0x0002} [hci0] 18:06:14.98
>        90 c5 13 cd f3 cd 02 01 00 00 00 01 00 00 00     ...............
> @ MGMT Event: Command Compl.. (0x0001) plen 10  {0x0001} [hci0] 18:06:14.98
>      Unknown (0x0050) plen 7
>        Status: Success (0x00)
>        90 c5 13 cd f3 cd 02                             .......
> @ MGMT Command: Add Device (0x0033) plen 8      {0x0001} [hci0] 18:06:14.98
>        LE Address: CD:F3:CD:13:C5:90 (Static)
>        Action: Auto-connect remote device (0x02)
> @ MGMT Event: Device Added (0x001a) plen 8      {0x0004} [hci0] 18:06:14.98
>        LE Address: CD:F3:CD:13:C5:90 (Static)
>        Action: Auto-connect remote device (0x02)
> @ MGMT Event: Device Added (0x001a) plen 8      {0x0003} [hci0] 18:06:14.98
>        LE Address: CD:F3:CD:13:C5:90 (Static)
>        Action: Auto-connect remote device (0x02)
> @ MGMT Event: Device Added (0x001a) plen 8      {0x0002} [hci0] 18:06:14.98
>        LE Address: CD:F3:CD:13:C5:90 (Static)
>        Action: Auto-connect remote device (0x02)
> @ MGMT Event: Unknown (0x002a) plen 15          {0x0004} [hci0] 18:06:14.98
>        90 c5 13 cd f3 cd 02 01 00 00 00 01 00 00 00     ...............
> @ MGMT Event: Unknown (0x002a) plen 15          {0x0003} [hci0] 18:06:14.98
>        90 c5 13 cd f3 cd 02 01 00 00 00 01 00 00 00     ...............
> @ MGMT Event: Unknown (0x002a) plen 15          {0x0002} [hci0] 18:06:14.98
>        90 c5 13 cd f3 cd 02 01 00 00 00 01 00 00 00     ...............
> @ MGMT Event: Unknown (0x002a) plen 15          {0x0001} [hci0] 18:06:14.98
>        90 c5 13 cd f3 cd 02 01 00 00 00 01 00 00 00     ...............
> 
> Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> Reviewed-by: Alain Michaud <alainm@chromium.org>
> ---
> 
> include/net/bluetooth/hci.h  |   1 +
> include/net/bluetooth/mgmt.h |  28 ++++++++
> net/bluetooth/hci_sock.c     |   1 +
> net/bluetooth/mgmt.c         | 134 +++++++++++++++++++++++++++++++++++
> 4 files changed, 164 insertions(+)
> 
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 16ab6ce8788341..5e03aac76ad47f 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -259,6 +259,7 @@ enum {
> 	HCI_MGMT_LOCAL_NAME_EVENTS,
> 	HCI_MGMT_OOB_DATA_EVENTS,
> 	HCI_MGMT_EXP_FEATURE_EVENTS,
> +	HCI_MGMT_DEVICE_FLAGS_EVENTS,

this part is not needed. We are doing this for commands where a client has to initiate a read command first before things get enabled. In this case the triggering command is Add Device and that has been there for a while. So no need to extra guard this.

> };
> 
> /*
> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> index e515288f328f47..8e47b0c5fe52bb 100644
> --- a/include/net/bluetooth/mgmt.h
> +++ b/include/net/bluetooth/mgmt.h
> @@ -720,6 +720,27 @@ struct mgmt_rp_set_exp_feature {
> #define MGMT_OP_SET_DEF_RUNTIME_CONFIG	0x004e
> #define MGMT_SET_DEF_RUNTIME_CONFIG_SIZE	0
> 
> +#define MGMT_OP_GET_DEVICE_FLAGS	0x004F
> +#define MGMT_GET_DEVICE_FLAGS_SIZE	7
> +struct mgmt_cp_get_device_flags {
> +	struct mgmt_addr_info addr;
> +} __packed;
> +struct mgmt_rp_get_device_flags {
> +	struct mgmt_addr_info addr;
> +	__le32 supported_flags;
> +	__le32 current_flags;
> +} __packed;
> +
> +#define MGMT_OP_SET_DEVICE_FLAGS	0x0050
> +#define MGMT_SET_DEVICE_FLAGS_SIZE	11
> +struct mgmt_cp_set_device_flags {
> +	struct mgmt_addr_info addr;
> +	__le32 current_flags;
> +} __packed;
> +struct mgmt_rp_set_device_flags {
> +	struct mgmt_addr_info addr;
> +} __packed;
> +
> #define MGMT_EV_CMD_COMPLETE		0x0001
> struct mgmt_ev_cmd_complete {
> 	__le16	opcode;
> @@ -951,3 +972,10 @@ struct mgmt_ev_exp_feature_changed {
> 	__u8	uuid[16];
> 	__le32	flags;
> } __packed;
> +
> +#define MGMT_EV_DEVICE_FLAGS_CHANGED		0x002a
> +struct mgmt_ev_device_flags_changed {
> +	struct mgmt_addr_info addr;
> +	__le32 supported_flags;
> +	__le32 current_flags;
> +} __packed;
> diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
> index d5627967fc254f..a7903b6206620c 100644
> --- a/net/bluetooth/hci_sock.c
> +++ b/net/bluetooth/hci_sock.c
> @@ -1354,6 +1354,7 @@ static int hci_sock_bind(struct socket *sock, struct sockaddr *addr,
> 			hci_sock_set_flag(sk, HCI_MGMT_SETTING_EVENTS);
> 			hci_sock_set_flag(sk, HCI_MGMT_DEV_CLASS_EVENTS);
> 			hci_sock_set_flag(sk, HCI_MGMT_LOCAL_NAME_EVENTS);
> +			hci_sock_set_flag(sk, HCI_MGMT_DEVICE_FLAGS_EVENTS);

This is actually wrong. The other flags are there for event where you have multiple versions of the same event. If we ever introduce an Add Extended Device command, then yes, we need to guard things here. Right now, we don’t.

> 		}
> 		break;
> 	}
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 6d996e5e5bcc2d..2805f662d85695 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -114,6 +114,8 @@ static const u16 mgmt_commands[] = {
> 	MGMT_OP_SET_EXP_FEATURE,
> 	MGMT_OP_READ_DEF_SYSTEM_CONFIG,
> 	MGMT_OP_SET_DEF_SYSTEM_CONFIG,
> +	MGMT_OP_GET_DEVICE_FLAGS,
> +	MGMT_OP_SET_DEVICE_FLAGS,
> };
> 
> static const u16 mgmt_events[] = {
> @@ -154,6 +156,7 @@ static const u16 mgmt_events[] = {
> 	MGMT_EV_EXT_INFO_CHANGED,
> 	MGMT_EV_PHY_CONFIGURATION_CHANGED,
> 	MGMT_EV_EXP_FEATURE_CHANGED,
> +	MGMT_EV_DEVICE_FLAGS_CHANGED,
> };
> 
> static const u16 mgmt_untrusted_commands[] = {
> @@ -3853,6 +3856,122 @@ static int set_exp_feature(struct sock *sk, struct hci_dev *hdev,
> 			       MGMT_STATUS_NOT_SUPPORTED);
> }
> 
> +#define SUPPORTED_DEVICE_FLAGS() ((1U << HCI_CONN_FLAG_MAX) - 1)
> +
> +static int get_device_flags(struct sock *sk, struct hci_dev *hdev, void *data,
> +			    u16 data_len)
> +{
> +	struct mgmt_cp_get_device_flags *cp = data;
> +	struct mgmt_rp_get_device_flags rp;
> +	struct bdaddr_list_with_flags *br_params;
> +	struct hci_conn_params *params;
> +	u32 supported_flags = SUPPORTED_DEVICE_FLAGS();
> +	u32 current_flags = 0;
> +	u8 status = MGMT_STATUS_INVALID_PARAMS;
> +
> +	bt_dev_dbg(hdev, "Get device flags %pMR (type 0x%x)\n",
> +		   &cp->addr.bdaddr, cp->addr.type);
> +
> +	if (cp->addr.type == BDADDR_BREDR) {
> +		br_params = hci_bdaddr_list_lookup_with_flags(&hdev->whitelist,
> +							      &cp->addr.bdaddr,
> +							      cp->addr.type);
> +		if (!br_params)
> +			goto done;
> +
> +		current_flags = br_params->current_flags;
> +	} else {
> +		params = hci_conn_params_lookup(hdev, &cp->addr.bdaddr,
> +						le_addr_type(cp->addr.type));
> +
> +		if (!params)
> +			goto done;
> +
> +		current_flags = params->current_flags;
> +	}
> +
> +	bacpy(&rp.addr.bdaddr, &cp->addr.bdaddr);
> +	rp.addr.type = cp->addr.type;
> +	rp.supported_flags = cpu_to_le32(supported_flags);
> +	rp.current_flags = cpu_to_le32(current_flags);
> +
> +	status = MGMT_STATUS_SUCCESS;
> +
> +done:
> +	return mgmt_cmd_complete(sk, hdev->id, MGMT_OP_GET_DEVICE_FLAGS, status,
> +				&rp, sizeof(rp));
> +}
> +
> +static int device_flags_changed(struct hci_dev *hdev, bdaddr_t *bdaddr,
> +				u8 bdaddr_type, u32 supported_flags,
> +				u32 current_flags, struct sock *skip)
> +{
> +	struct mgmt_ev_device_flags_changed ev;
> +
> +	bacpy(&ev.addr.bdaddr, bdaddr);
> +	ev.addr.type = bdaddr_type;
> +	ev.supported_flags = cpu_to_le32(supported_flags);
> +	ev.current_flags = cpu_to_le32(current_flags);
> +
> +	return mgmt_limited_event(MGMT_EV_DEVICE_FLAGS_CHANGED, hdev, &ev,
> +				  sizeof(ev), HCI_MGMT_DEVICE_FLAGS_EVENTS,
> +				  skip);
> +}
> +
> +static int set_device_flags(struct sock *sk, struct hci_dev *hdev, void *data,
> +			    u16 len)
> +{
> +	struct mgmt_cp_set_device_flags *cp = data;
> +	struct bdaddr_list_with_flags *br_params;
> +	struct hci_conn_params *params;
> +	u8 status = MGMT_STATUS_INVALID_PARAMS;
> +	u32 supported_flags = SUPPORTED_DEVICE_FLAGS();
> +	u32 current_flags = __le32_to_cpu(cp->current_flags);
> +
> +	bt_dev_dbg(hdev, "Set device flags %pMR (type 0x%x) = 0x%x",
> +		   &cp->addr.bdaddr, cp->addr.type,
> +		   __le32_to_cpu(current_flags));
> +
> +	if ((supported_flags | current_flags) != supported_flags) {
> +		bt_dev_warn(hdev, "Bad flag given (0x%x) vs supported (0x%0x)",
> +			    current_flags, supported_flags);
> +		goto done;
> +	}
> +
> +	if (cp->addr.type == BDADDR_BREDR) {
> +		br_params = hci_bdaddr_list_lookup_with_flags(&hdev->whitelist,
> +							      &cp->addr.bdaddr,
> +							      cp->addr.type);
> +
> +		if (br_params) {
> +			br_params->current_flags = current_flags;
> +			status = MGMT_STATUS_SUCCESS;
> +		} else {
> +			bt_dev_warn(hdev, "No such BR/EDR device %pMR (0x%x)",
> +				    &cp->addr.bdaddr, cp->addr.type);
> +		}
> +	} else {
> +		params = hci_conn_params_lookup(hdev, &cp->addr.bdaddr,
> +						le_addr_type(cp->addr.type));
> +		if (params) {
> +			params->current_flags = current_flags;
> +			status = MGMT_STATUS_SUCCESS;
> +		} else {
> +			bt_dev_warn(hdev, "No such LE device %pMR (0x%x)",
> +				    &cp->addr.bdaddr,
> +				    le_addr_type(cp->addr.type));
> +		}
> +	}
> +
> +done:
> +	if (status == MGMT_STATUS_SUCCESS)
> +		device_flags_changed(hdev, &cp->addr.bdaddr, cp->addr.type,
> +				     supported_flags, current_flags, sk);
> +
> +	return mgmt_cmd_complete(sk, hdev->id, MGMT_OP_SET_DEVICE_FLAGS, status,
> +				 &cp->addr, sizeof(cp->addr));
> +}
> +
> static void read_local_oob_data_complete(struct hci_dev *hdev, u8 status,
> 				         u16 opcode, struct sk_buff *skb)
> {
> @@ -5970,7 +6089,9 @@ static int add_device(struct sock *sk, struct hci_dev *hdev,
> {
> 	struct mgmt_cp_add_device *cp = data;
> 	u8 auto_conn, addr_type;
> +	struct hci_conn_params *params;
> 	int err;
> +	u32 current_flags = 0;
> 
> 	bt_dev_dbg(hdev, "sock %p", sk);
> 
> @@ -6038,12 +6159,19 @@ static int add_device(struct sock *sk, struct hci_dev *hdev,
> 					MGMT_STATUS_FAILED, &cp->addr,
> 					sizeof(cp->addr));
> 		goto unlock;
> +	} else {
> +		params = hci_conn_params_lookup(hdev, &cp->addr.bdaddr,
> +						addr_type);
> +		if (params)
> +			current_flags = params->current_flags;
> 	}
> 
> 	hci_update_background_scan(hdev);
> 
> added:
> 	device_added(sk, hdev, &cp->addr.bdaddr, cp->addr.type, cp->action);
> +	device_flags_changed(hdev, &cp->addr.bdaddr, cp->addr.type,
> +			     SUPPORTED_DEVICE_FLAGS(), current_flags, NULL);
> 
> 	err = mgmt_cmd_complete(sk, hdev->id, MGMT_OP_ADD_DEVICE,
> 				MGMT_STATUS_SUCCESS, &cp->addr,
> @@ -7306,6 +7434,12 @@ static const struct hci_mgmt_handler mgmt_handlers[] = {
> 						HCI_MGMT_UNTRUSTED },
> 	{ set_def_system_config,   MGMT_SET_DEF_SYSTEM_CONFIG_SIZE,
> 						HCI_MGMT_VAR_LEN },
> +
> +	{ NULL }, /* Read default runtime config */
> +	{ NULL }, /* Set default runtime config */
> +
> +	{ get_device_flags,        MGMT_GET_DEVICE_FLAGS_SIZE },
> +	{ set_device_flags,        MGMT_SET_DEVICE_FLAGS_SIZE },
> };

I have create a local tree that has the read/set runtime config commands already in there. I added your patches 1-3 to the tree already. I might just remove the HCI_MGMT_DEVICE_FLAGS_EVENTS and add this patch as well. Everything else looks good.

Regards

Marcel