[v4] bluetooth: Add Adv Monitor Device Found/Lost events

Commit Message

Manish Mandlik Oct. 25, 2021, 7:53 p.m. UTC

Whenever the controller starts/stops monitoring a bt device, it sends
MSFT Monitor Device event. Handle this event and notify the bluetoothd
whenever the controller starts/stops monitoring a particular device.

Test performed:
- verified by logs that the MSFT Monitor Device is received from the
  controller and the bluetoothd is notified whenever the controller
  starts/stops monitoring a device.

Signed-off-by: Manish Mandlik <[e-mail]>
---
Hello Bt-Maintainers,

As mentioned in the bluez patch series [1], we need to capture the 'MSFT
Monitor Device' from the controller and pass it to the bluetoothd.

This is required to further optimize the power consumption by avoiding
handling of RSSI thresholds and timeouts in the user space and let the
controller do the RSSI tracking.

This patch adds support to read HCI_VS_MSFT_LE_Monitor_Device_Event and
introduces new MGMT events MGMT_EV_ADV_MONITOR_DEVICE_FOUND and
MGMT_EV_ADV_MONITOR_DEVICE_LOST to indicate that the controller has
started/stopped tracking a particular device.

Please let me know what you think about this or if you have any further
questions.

[1] https://patchwork.kernel.org/project/bluetooth/list/?series=569881

Thanks,
Manish.

Changes in v4:
- Add Advertisement Monitor Device Found event and update addr type.

Changes in v3:
- Discard changes to the Device Found event and notify bluetoothd only
  when the controller stops monitoring the device via new Device Lost
  event.

Changes in v2:
- Instead of creating a new 'Device Tracking' event, add a flag 'Device
  Tracked' in the existing 'Device Found' event and add a new 'Device
  Lost' event to indicate that the controller has stopped tracking that
  device.

 include/net/bluetooth/hci_core.h |   4 ++
 include/net/bluetooth/mgmt.h     |  12 ++++
 net/bluetooth/mgmt.c             |  28 ++++++++
 net/bluetooth/msft.c             | 109 +++++++++++++++++++++++++------
 4 files changed, 132 insertions(+), 21 deletions(-)

Comments

kernel test robot Oct. 26, 2021, 6:59 a.m. UTC | #1

Hi Manish,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on bluetooth-next/master]
[also build test ERROR on v5.15-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Manish-Mandlik/bluetooth-Add-Adv-Monitor-Device-Found-Lost-events/20211026-041928
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: parisc-buildonly-randconfig-r006-20211026 (attached as .config)
compiler: hppa-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/316f17a25f37ae394a494d39373fb881ab7c5377
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Manish-Mandlik/bluetooth-Add-Adv-Monitor-Device-Found-Lost-events/20211026-041928
        git checkout 316f17a25f37ae394a494d39373fb881ab7c5377
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=parisc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[e-mail]>

All errors (new ones prefixed by >>):

   In file included from net/bluetooth/msft.c:6:
   net/bluetooth/msft.c: In function 'msft_monitor_device_evt':
>> include/net/bluetooth/bluetooth.h:212:16: error: format '%x' expects a matching 'unsigned int' argument [-Werror=format=]
     212 |         BT_ERR("%s: " fmt, (hdev)->name, ##__VA_ARGS__)
         |                ^~~~~~
   include/net/bluetooth/bluetooth.h:199:40: note: in definition of macro 'BT_ERR'
     199 | #define BT_ERR(fmt, ...)        bt_err(fmt "\n", ##__VA_ARGS__)
         |                                        ^~~
   net/bluetooth/msft.c:404:17: note: in expansion of macro 'bt_dev_err'
     404 |                 bt_dev_err(hdev,
         |                 ^~~~~~~~~~
   cc1: all warnings being treated as errors


vim +212 include/net/bluetooth/bluetooth.h

^1da177e4c3f41 Linus Torvalds 2005-04-16  206  
6f558b70fb39fc Loic Poulain   2015-08-30  207  #define bt_dev_info(hdev, fmt, ...)				\
6f558b70fb39fc Loic Poulain   2015-08-30  208  	BT_INFO("%s: " fmt, (hdev)->name, ##__VA_ARGS__)
594b31ea7dc610 Frederic Danis 2015-09-23  209  #define bt_dev_warn(hdev, fmt, ...)				\
594b31ea7dc610 Frederic Danis 2015-09-23  210  	BT_WARN("%s: " fmt, (hdev)->name, ##__VA_ARGS__)
6f558b70fb39fc Loic Poulain   2015-08-30  211  #define bt_dev_err(hdev, fmt, ...)				\
6f558b70fb39fc Loic Poulain   2015-08-30 @212  	BT_ERR("%s: " fmt, (hdev)->name, ##__VA_ARGS__)
6f558b70fb39fc Loic Poulain   2015-08-30  213  #define bt_dev_dbg(hdev, fmt, ...)				\
6f558b70fb39fc Loic Poulain   2015-08-30  214  	BT_DBG("%s: " fmt, (hdev)->name, ##__VA_ARGS__)
6f558b70fb39fc Loic Poulain   2015-08-30  215  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[e-mail]

kernel test robot Oct. 27, 2021, 6:28 a.m. UTC | #2

Hi Manish,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on bluetooth-next/master]
[also build test WARNING on bluetooth/master v5.15-rc7 next-20211026]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Manish-Mandlik/bluetooth-Add-Adv-Monitor-Device-Found-Lost-events/20211026-041928
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: mips-randconfig-r033-20211027 (attached as .config)
compiler: mips64el-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/316f17a25f37ae394a494d39373fb881ab7c5377
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Manish-Mandlik/bluetooth-Add-Adv-Monitor-Device-Found-Lost-events/20211026-041928
        git checkout 316f17a25f37ae394a494d39373fb881ab7c5377
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=mips 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[e-mail]>

All warnings (new ones prefixed by >>):

   In file included from net/bluetooth/msft.c:6:
   net/bluetooth/msft.c: In function 'msft_monitor_device_evt':
>> include/net/bluetooth/bluetooth.h:212:16: warning: format '%x' expects a matching 'unsigned int' argument [-Wformat=]
     212 |         BT_ERR("%s: " fmt, (hdev)->name, ##__VA_ARGS__)
         |                ^~~~~~
   include/net/bluetooth/bluetooth.h:199:40: note: in definition of macro 'BT_ERR'
     199 | #define BT_ERR(fmt, ...)        bt_err(fmt "\n", ##__VA_ARGS__)
         |                                        ^~~
   net/bluetooth/msft.c:404:17: note: in expansion of macro 'bt_dev_err'
     404 |                 bt_dev_err(hdev,
         |                 ^~~~~~~~~~


vim +212 include/net/bluetooth/bluetooth.h

^1da177e4c3f41 Linus Torvalds 2005-04-16  206  
6f558b70fb39fc Loic Poulain   2015-08-30  207  #define bt_dev_info(hdev, fmt, ...)				\
6f558b70fb39fc Loic Poulain   2015-08-30  208  	BT_INFO("%s: " fmt, (hdev)->name, ##__VA_ARGS__)
594b31ea7dc610 Frederic Danis 2015-09-23  209  #define bt_dev_warn(hdev, fmt, ...)				\
594b31ea7dc610 Frederic Danis 2015-09-23  210  	BT_WARN("%s: " fmt, (hdev)->name, ##__VA_ARGS__)
6f558b70fb39fc Loic Poulain   2015-08-30  211  #define bt_dev_err(hdev, fmt, ...)				\
6f558b70fb39fc Loic Poulain   2015-08-30 @212  	BT_ERR("%s: " fmt, (hdev)->name, ##__VA_ARGS__)
6f558b70fb39fc Loic Poulain   2015-08-30  213  #define bt_dev_dbg(hdev, fmt, ...)				\
6f558b70fb39fc Loic Poulain   2015-08-30  214  	BT_DBG("%s: " fmt, (hdev)->name, ##__VA_ARGS__)
6f558b70fb39fc Loic Poulain   2015-08-30  215  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[e-mail]

Patch

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index dd8840e70e25..ed7d2780bdec 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1842,6 +1842,10 @@  void mgmt_adv_monitor_removed(struct hci_dev *hdev, u16 handle);
 int mgmt_phy_configuration_changed(struct hci_dev *hdev, struct sock *skip);
 int mgmt_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
 int mgmt_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status);
+void mgmt_adv_monitor_device_found(struct hci_dev *hdev, u16 handle,
+				   bdaddr_t *addr, u8 addr_type);
+void mgmt_adv_monitor_device_lost(struct hci_dev *hdev, u16 handle,
+				  bdaddr_t *addr, u8 addr_type);
 
 u8 hci_le_conn_update(struct hci_conn *conn, u16 min, u16 max, u16 latency,
 		      u16 to_multiplier);
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 23a0524061b7..d471aaaa2e4f 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -1103,3 +1103,15 @@  struct mgmt_ev_controller_resume {
 #define MGMT_WAKE_REASON_NON_BT_WAKE		0x0
 #define MGMT_WAKE_REASON_UNEXPECTED		0x1
 #define MGMT_WAKE_REASON_REMOTE_WAKE		0x2
+
+#define MGMT_EV_ADV_MONITOR_DEVICE_FOUND	0x002f
+struct mgmt_ev_adv_monitor_device_found {
+	__le16 monitor_handle;
+	struct mgmt_addr_info addr;
+} __packed;
+
+#define MGMT_EV_ADV_MONITOR_DEVICE_LOST		0x0030
+struct mgmt_ev_adv_monitor_device_lost {
+	__le16 monitor_handle;
+	struct mgmt_addr_info addr;
+} __packed;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 44683443300c..087e40761b26 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -173,6 +173,8 @@  static const u16 mgmt_events[] = {
 	MGMT_EV_ADV_MONITOR_REMOVED,
 	MGMT_EV_CONTROLLER_SUSPEND,
 	MGMT_EV_CONTROLLER_RESUME,
+	MGMT_EV_ADV_MONITOR_DEVICE_FOUND,
+	MGMT_EV_ADV_MONITOR_DEVICE_LOST,
 };
 
 static const u16 mgmt_untrusted_commands[] = {
@@ -4396,6 +4398,32 @@  static int set_device_flags(struct sock *sk, struct hci_dev *hdev, void *data,
 				 &cp->addr, sizeof(cp->addr));
 }
 
+void mgmt_adv_monitor_device_found(struct hci_dev *hdev, u16 handle,
+				   bdaddr_t *addr, u8 addr_type)
+{
+	struct mgmt_ev_adv_monitor_device_found ev;
+
+	ev.monitor_handle = cpu_to_le16(handle);
+	bacpy(&ev.addr.bdaddr, addr);
+	ev.addr.type = addr_type;
+
+	mgmt_event(MGMT_EV_ADV_MONITOR_DEVICE_FOUND, hdev, &ev, sizeof(ev),
+		   NULL);
+}
+
+void mgmt_adv_monitor_device_lost(struct hci_dev *hdev, u16 handle,
+				  bdaddr_t *addr, u8 addr_type)
+{
+	struct mgmt_ev_adv_monitor_device_lost ev;
+
+	ev.monitor_handle = cpu_to_le16(handle);
+	bacpy(&ev.addr.bdaddr, addr);
+	ev.addr.type = addr_type;
+
+	mgmt_event(MGMT_EV_ADV_MONITOR_DEVICE_LOST, hdev, &ev, sizeof(ev),
+		   NULL);
+}
+
 static void mgmt_adv_monitor_added(struct sock *sk, struct hci_dev *hdev,
 				   u16 handle)
 {
diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
index 255cffa554ee..88520273bd90 100644
--- a/net/bluetooth/msft.c
+++ b/net/bluetooth/msft.c
@@ -80,6 +80,14 @@  struct msft_rp_le_set_advertisement_filter_enable {
 	__u8 sub_opcode;
 } __packed;
 
+#define MSFT_EV_LE_MONITOR_DEVICE	0x02
+struct msft_ev_le_monitor_device {
+	__u8     addr_type;
+	bdaddr_t bdaddr;
+	__u8     monitor_handle;
+	__u8     monitor_state;
+} __packed;
+
 struct msft_monitor_advertisement_handle_data {
 	__u8  msft_handle;
 	__u16 mgmt_handle;
@@ -103,6 +111,26 @@  static int __msft_add_monitor_pattern(struct hci_dev *hdev,
 static int __msft_remove_monitor(struct hci_dev *hdev,
 				 struct adv_monitor *monitor, u16 handle);
 
+/* is_mgmt = true matches the handle exposed to userspace via mgmt.
+ * is_mgmt = false matches the handle used by the msft controller.
+ * This function requires the caller holds hdev->lock
+ */
+static struct msft_monitor_advertisement_handle_data *msft_find_handle_data
+				(struct hci_dev *hdev, u16 handle, bool is_mgmt)
+{
+	struct msft_monitor_advertisement_handle_data *entry;
+	struct msft_data *msft = hdev->msft_data;
+
+	list_for_each_entry(entry, &msft->handle_map, list) {
+		if (is_mgmt && entry->mgmt_handle == handle)
+			return entry;
+		if (!is_mgmt && entry->msft_handle == handle)
+			return entry;
+	}
+
+	return NULL;
+}
+
 bool msft_monitor_supported(struct hci_dev *hdev)
 {
 	return !!(msft_get_features(hdev) & MSFT_FEATURE_MASK_LE_ADV_MONITOR);
@@ -341,6 +369,53 @@  void msft_unregister(struct hci_dev *hdev)
 	kfree(msft);
 }
 
+/* This function requires the caller holds hdev->lock */
+static void msft_monitor_device_evt(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	struct msft_ev_le_monitor_device *ev = (void *)skb->data;
+	struct msft_monitor_advertisement_handle_data *handle_data;
+	u8 addr_type;
+
+	if (skb->len < sizeof(*ev)) {
+		bt_dev_err(hdev,
+			   "MSFT vendor event %u: insufficient data (len: %u)",
+			   MSFT_EV_LE_MONITOR_DEVICE, skb->len);
+		return;
+	}
+	skb_pull(skb, sizeof(*ev));
+
+	bt_dev_dbg(hdev,
+		   "MSFT vendor event %u: handle 0x%04x state %d addr %pMR",
+		   MSFT_EV_LE_MONITOR_DEVICE, ev->monitor_handle,
+		   ev->monitor_state, &ev->bdaddr);
+
+	handle_data = msft_find_handle_data(hdev, ev->monitor_handle, false);
+
+	switch (ev->addr_type) {
+	case ADDR_LE_DEV_PUBLIC:
+		addr_type = BDADDR_LE_PUBLIC;
+		break;
+
+	case ADDR_LE_DEV_RANDOM:
+		addr_type = BDADDR_LE_RANDOM;
+		break;
+
+	default:
+		bt_dev_err(hdev,
+			   "MSFT vendor event %u: unknown addr type 0x%02x",
+			   ev->addr_type);
+		return;
+	}
+
+	if (ev->monitor_state) {
+		mgmt_adv_monitor_device_found(hdev, handle_data->mgmt_handle,
+					      &ev->bdaddr, addr_type);
+	} else {
+		mgmt_adv_monitor_device_lost(hdev, handle_data->mgmt_handle,
+					     &ev->bdaddr, addr_type);
+	}
+}
+
 void msft_vendor_evt(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct msft_data *msft = hdev->msft_data;
@@ -368,37 +443,29 @@  void msft_vendor_evt(struct hci_dev *hdev, struct sk_buff *skb)
 	if (skb->len < 1)
 		return;
 
+	hci_dev_lock(hdev);
+
 	event = *skb->data;
 	skb_pull(skb, 1);
 
-	bt_dev_dbg(hdev, "MSFT vendor event %u", event);
-}
+	switch (event) {
+	case MSFT_EV_LE_MONITOR_DEVICE:
+		msft_monitor_device_evt(hdev, skb);
+		break;
 
-__u64 msft_get_features(struct hci_dev *hdev)
-{
-	struct msft_data *msft = hdev->msft_data;
+	default:
+		bt_dev_dbg(hdev, "MSFT vendor event %u", event);
+		break;
+	}
 
-	return msft ? msft->features : 0;
+	hci_dev_unlock(hdev);
 }
 
-/* is_mgmt = true matches the handle exposed to userspace via mgmt.
- * is_mgmt = false matches the handle used by the msft controller.
- * This function requires the caller holds hdev->lock
- */
-static struct msft_monitor_advertisement_handle_data *msft_find_handle_data
-				(struct hci_dev *hdev, u16 handle, bool is_mgmt)
+__u64 msft_get_features(struct hci_dev *hdev)
 {
-	struct msft_monitor_advertisement_handle_data *entry;
 	struct msft_data *msft = hdev->msft_data;
 
-	list_for_each_entry(entry, &msft->handle_map, list) {
-		if (is_mgmt && entry->mgmt_handle == handle)
-			return entry;
-		if (!is_mgmt && entry->msft_handle == handle)
-			return entry;
-	}
-
-	return NULL;
+	return msft ? msft->features : 0;
 }
 
 static void msft_le_monitor_advertisement_cb(struct hci_dev *hdev,